Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import "@layerzerolabs/oft-evm-upgradeable/contracts/oft/OFTCoreUpgradeable.sol"
import { IMintableBurnable } from "@layerzerolabs/oft-evm/contracts/interfaces/IMintableBurnable.sol";
import { INameService } from "@gooddollar/goodprotocol/contracts/utils/DAOUpgradeableContract.sol";
import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
import { ReentrancyGuardUpgradeable } from "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";

interface IIdentity {
function isWhitelisted(address) external view returns (bool);
Expand All @@ -16,7 +17,7 @@ interface IIdentity {
* @notice Upgradeable OFT adapter that uses mint/burn mechanisms for cross-chain transfers
* @dev Inherits from OFTCoreUpgradeable (which already includes OwnableUpgradeable) and implements mint/burn logic similar to MintBurnOFTAdapter
*/
contract GoodDollarOFTAdapter is UUPSUpgradeable, OFTCoreUpgradeable {
contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OFTCoreUpgradeable {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High · Legacy adapter storage shifts on upgrade

When a proxy deployed with the pre-PR layout is upgraded through upgradeOFT.ts, ReentrancyGuardUpgradeable adds storage before OFTCoreUpgradeable and the adapter’s fields, so upgradeOFT.ts upgrades a pre-PR proxy with a shifted layout; its token wiring, limits, and failed-request records are then read from the wrong slots, breaking the bridge and making the old state inaccessible.

Keep the old storage layout for in-place upgrades or use a fresh proxy and explicitly migrate state instead of upgrading a pre-PR proxy to this implementation.

Prompt for AI agents
In `packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol` at line 20:

**Legacy adapter storage shifts on upgrade**

When a proxy deployed with the pre-PR layout is upgraded through `upgradeOFT.ts`, `ReentrancyGuardUpgradeable` adds storage before `OFTCoreUpgradeable` and the adapter’s fields, so `upgradeOFT.ts` upgrades a pre-PR proxy with a shifted layout; its token wiring, limits, and failed-request records are then read from the wrong slots, breaking the bridge and making the old state inaccessible.

Keep the old storage layout for in-place upgrades or use a fresh proxy and explicitly migrate state instead of upgrading a pre-PR proxy to this implementation.

using OFTMsgCodec for bytes;
using OFTMsgCodec for bytes32;
/// @dev Struct for storing bridge fees
Expand Down Expand Up @@ -122,6 +123,9 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, OFTCoreUpgradeable {
/// @dev Event emitted when a failed receive request is approved
event FailedReceiveRequestApproved(bytes32 indexed guid);

/// @dev Event emitted when a failed receive request is rejected by the owner
event FailedReceiveRequestRejected(bytes32 indexed guid);

/**
* @dev Constructor for the upgradeable implementation; token is used for decimals, init is disabled.
* @param _token The underlying ERC20 token (used to get decimals for parent)
Expand Down Expand Up @@ -154,6 +158,7 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, OFTCoreUpgradeable {
require(IERC20Metadata(_token).decimals() == innerTokenDecimals, 'token decimals mismatch');

__UUPSUpgradeable_init();
__ReentrancyGuard_init();
__Ownable_init();
__OAppSender_init(_owner);
__OAppReceiver_init(_owner);
Expand Down Expand Up @@ -251,15 +256,26 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, OFTCoreUpgradeable {
/**
* @notice Approves a failed receive request with optimistic window or by owner
*/
function approveFailedRequest(bytes32 _guid) external {
function approveFailedRequest(bytes32 _guid) external nonReentrant {
require(!isClosed, 'closed');
FailedReceiveRequest memory request = failedReceiveRequests[_guid];
require(request.timestamp + OPTIMISTIC_WINDOW < block.timestamp || msg.sender == owner(), 'optimistic period not ended or not owner');
require(request.failed, 'request not failed');
_credit(request.toAddress, request.amount, request.srcEid);
// G$ mint calls the recipient's ERC777 hook, so state must be final before _credit
failedReceiveRequests[_guid].failed = false;
_credit(request.toAddress, request.amount, request.srcEid);
emit FailedReceiveRequestApproved(_guid);
}

/**
* @notice Rejects a failed receive request so it can never be approved (e.g. a forged or suspicious message)
*/
function rejectFailedRequest(bytes32 _guid) external onlyOwner {
require(failedReceiveRequests[_guid].failed, 'request not failed');
failedReceiveRequests[_guid].failed = false;
emit FailedReceiveRequestRejected(_guid);
}

/**
* @notice Bridge closed / whitelist check only (no limit checks).
* @dev Revert on this path does not store to failedReceiveRequests.
Expand Down Expand Up @@ -409,7 +425,7 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, OFTCoreUpgradeable {
emit FeeCollected(feeRecipient, fee);
}

return _amountLD;
return recipientAmount;
}

/**
Expand Down
13 changes: 13 additions & 0 deletions packages/bridge-contracts/contracts/test/ReentrancyPoC.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ interface IFailedRequestApprover {
function approveFailedRequest(bytes32 guid) external;
}

interface IERC1820RegistryLike {
function setInterfaceImplementer(address account, bytes32 interfaceHash, address implementer) external;
}

/**
* @dev Mimics SuperGoodDollar/SuperToken._mint: the recipient's ERC777 tokensReceived hook is invoked after
* balances are updated (SuperToken resolves the implementer via ERC1820; here a contract recipient is its own implementer).
Expand Down Expand Up @@ -46,6 +50,15 @@ contract FailedRequestReentrancyAttacker {

constructor(IFailedRequestApprover _adapter) {
adapter = _adapter;
// Registers as its own ERC777 recipient so the real SuperGoodDollar (fork tests) calls tokensReceived
address registry = 0x1820a4B7618BdE71Dce8cdc73aAB6C95905faD24;
if (registry.code.length > 0) {
IERC1820RegistryLike(registry).setInterfaceImplementer(
address(this),
keccak256('ERC777TokensRecipient'),
address(this)
);
}
}

function attack(bytes32 _guid, uint256 _maxReentries) external {
Expand Down
163 changes: 70 additions & 93 deletions packages/bridge-contracts/deploy/deployOFT.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,29 @@
/***
* Hardhat-deploy script for GoodDollar OFT (Omnichain Fungible Token) contracts
*
* Deploys (same pattern as MessageBridge: deterministic proxy + implementation + execute initialize):
* Deploys:
* 1. GoodDollarOFTMinterBurner - DAO-upgradeable contract that handles minting and burning of GoodDollar tokens for OFT
* 2. GoodDollarOFTAdapter - Upgradeable LayerZero OFT adapter that wraps GoodDollar token for cross-chain transfers
*
* Steps:
* 1. Deploy ERC1967Proxy (deterministic) for GoodDollarOFTMinterBurner, deploy implementation
* then execute initialize(nameService, adapter) once the adapter proxy address is known
* 2. Deploy ERC1967Proxy (deterministic) for GoodDollarOFTAdapter, deploy implementation (constructor: token, lzEndpoint), execute initialize(token, minterBurner, owner, feeRecipient)
* 1. Deploy both implementations (deterministic, salt = bytecode hash)
* 2. Predict both proxy addresses via GoodProtocol ProxyFactory1967.getDeploymentAddress(salt, deployer)
* 3. Deploy + initialize each proxy atomically via ProxyFactory1967.deployProxy(salt, impl, initData)
* - MinterBurner.initialize(nameService, adapter)
* - Adapter.initialize(token, minterBurner, deployer, Avatar as feeRecipient)
* 4. verifyOftDeployment: impl slots, owner, token, minterBurner, feeRecipient, operators(adapter)
*
* Note: GoodDollarOFTMinterBurner.initialize wires the adapter as operator.
* Note: proxy addresses depend on the per-chain ProxyFactory address, so they differ between chains.
*/

import { DeployFunction } from 'hardhat-deploy/types';
import { ethers } from 'hardhat';
import { BigNumber } from 'ethers';
import Contracts from '@gooddollar/goodprotocol/releases/deployment.json';
import { getImplementationAddress } from '@openzeppelin/upgrades-core';
import { verifyContract } from './utils/verifyContract';
import { getLzEndpoint } from './utils/getLzEndpoint';
import { verifyOftDeployment } from './utils/verifyOftDeployment';

const func: DeployFunction = async function (hre) {
const { deployments, network } = hre;
Expand Down Expand Up @@ -103,22 +108,6 @@ const func: DeployFunction = async function (hre) {
const oftAdapterArtifact = await hre.artifacts.readArtifact('GoodDollarOFTAdapter');
const oftAdapterImplSalt = ethers.utils.keccak256(oftAdapterArtifact.bytecode);

// --- GoodDollarOFTMinterBurner (hardhat-deploy: deterministic proxy + implementation + execute initialize) ---
const minterBurnerProxySalt = ethers.utils.keccak256(
ethers.utils.toUtf8Bytes(
isDevelopment ? 'Development-GoodDollarOFTMinterBurnerV1' : 'Production-GoodDollarOFTMinterBurnerV1'
),
);
const minterBurnerProxyDeploy = await deployments.deterministic('GoodDollarOFTMinterBurner', {
contract: 'ERC1967Proxy',
from: root.address,
salt: minterBurnerProxySalt,
log: true,
});
const minterBurnerProxy = await minterBurnerProxyDeploy.deploy();
const minterBurnerAddress = minterBurnerProxy.address;
console.log('GoodDollarOFTMinterBurner proxy', minterBurnerAddress);

const minterBurnerImpl = await deployments.deploy('GoodDollarOFTMinterBurner_Implementation', {
contract: 'GoodDollarOFTMinterBurner',
from: root.address,
Expand All @@ -127,54 +116,6 @@ const func: DeployFunction = async function (hre) {
});
console.log('GoodDollarOFTMinterBurner implementation', minterBurnerImpl.address);

const minterBurnerContract = await ethers.getContractAt('GoodDollarOFTMinterBurner', minterBurnerAddress);
const minterBurnerInitialized = await minterBurnerContract
.token()
.then((addr: string) => addr !== ethers.constants.AddressZero)
.catch(() => false);

if (!minterBurnerInitialized) {
console.log('GoodDollarOFTMinterBurner not initialized yet; will initialize after deploying adapter...');
} else {
console.log('GoodDollarOFTMinterBurner already initialized');
}

// Verify GoodDollarOFTMinterBurner implementation (no constructor args) on non-local networks (skip: 'hardhat', 'localhost')
if (!['hardhat', 'localhost'].includes(networkName)) {
await verifyContract(hre as any, minterBurnerImpl.address, [], 'GoodDollarOFTMinterBurner');
}

// --- GoodDollarOFTAdapter (hardhat-deploy: deterministic proxy + implementation with constructor + execute initialize) ---
const oftAdapterProxySalt = ethers.utils.keccak256(
ethers.utils.toUtf8Bytes(isDevelopment ? 'Development-GoodDollarOFTAdapterV1' : 'Production-GoodDollarOFTAdapterV1'),
);
const oftAdapterProxyDeploy = await deployments.deterministic('GoodDollarOFTAdapter', {
contract: 'ERC1967Proxy',
from: root.address,
salt: oftAdapterProxySalt,
log: true,
});
const oftAdapterProxy = await oftAdapterProxyDeploy.deploy();
const oftAdapterAddress = oftAdapterProxy.address;
console.log('GoodDollarOFTAdapter proxy', oftAdapterAddress);

// Initialize minter/burner after we know the adapter address
if (!minterBurnerInitialized) {
console.log('Initializing GoodDollarOFTMinterBurner with adapter address...');
const minterBurnerInitData = minterBurnerContract.interface.encodeFunctionData('initialize', [
nameServiceAddress,
oftAdapterAddress,
]);
await deployments.execute(
'GoodDollarOFTMinterBurner',
{ from: root.address },
'initialize',
minterBurnerImpl.address,
minterBurnerInitData,
);
console.log('GoodDollarOFTMinterBurner initialized');
}

const oftAdapterImpl = await deployments.deploy('GoodDollarOFTAdapter_Implementation', {
contract: 'GoodDollarOFTAdapter',
from: root.address,
Expand All @@ -184,35 +125,70 @@ const func: DeployFunction = async function (hre) {
});
console.log('GoodDollarOFTAdapter implementation', oftAdapterImpl.address);

const oftAdapterContract = await ethers.getContractAt('GoodDollarOFTAdapter', oftAdapterAddress);
const oftAdapterInitialized = await oftAdapterContract
.minterBurner()
.then((addr: string) => addr !== ethers.constants.AddressZero)
.catch(() => false);
// ProxyFactory1967 deploys and initializes in one tx, with a CREATE2 salt bound to the deployer (no init front-running)
const proxyFactoryAddress = goodProtocolContracts.ProxyFactory;
if (!proxyFactoryAddress) {
throw new Error(`ProxyFactory address not found in GoodProtocol deployment for network ${networkName}`);
}
const proxyFactory = await ethers.getContractAt(
[
'function deployProxy(uint256 _salt, address _logic, bytes _data) returns (address)',
'function getDeploymentAddress(uint256 _salt, address _sender) view returns (address)',
],
proxyFactoryAddress,
);

const envPrefix = isDevelopment ? 'Development' : 'Production';
const minterBurnerProxySalt = ethers.BigNumber.from(
ethers.utils.keccak256(ethers.utils.toUtf8Bytes(`${envPrefix}-GoodDollarOFTMinterBurnerV1`)),
);
const oftAdapterProxySalt = ethers.BigNumber.from(
Comment on lines +142 to +145

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚪ Low · Proxy addresses misdescribed

When operators use the configuring guide to predict or compare proxy addresses across chains, readers of the configuring guide are told that the proxy salt gives a fixed address per environment, but these salts are passed to each chain’s ProxyFactory and the factory address affects the resulting proxy address. Operators therefore expect the Celo and XDC proxies to share an address when they do not.

Update the guide to state that proxy addresses depend on the per-chain ProxyFactory and deployer, and differ between chains.

Prompt for AI agents
In `packages/bridge-contracts/deploy/deployOFT.ts` at lines 142-145:

**Proxy addresses misdescribed**

When operators use the configuring guide to predict or compare proxy addresses across chains, readers of the configuring guide are told that the proxy salt gives a fixed address per environment, but these salts are passed to each chain’s ProxyFactory and the factory address affects the resulting proxy address. Operators therefore expect the Celo and XDC proxies to share an address when they do not.

Update the guide to state that proxy addresses depend on the per-chain ProxyFactory and deployer, and differ between chains.

ethers.utils.keccak256(ethers.utils.toUtf8Bytes(`${envPrefix}-GoodDollarOFTAdapterV1`)),
);
// Both proxy addresses are known before deployment, which breaks the MinterBurner <-> adapter init cycle
const minterBurnerAddress: string = await proxyFactory.getDeploymentAddress(minterBurnerProxySalt, root.address);
const oftAdapterAddress: string = await proxyFactory.getDeploymentAddress(oftAdapterProxySalt, root.address);
Comment on lines +149 to +150

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High · Reruns create a second proxy pair

When a different signer resumes or reruns deployment after the original proxies have been deployed, getDeploymentAddress predicts addresses from the new root.address, so deployProxyIfMissing deploys a second pair and deployments.save replaces the local records without disabling the original pair. Later scripts target the new pair while the original can remain authorized or wired.

Reuse existing deployment records on reruns, and do not replace them with proxies predicted from a different signer.

Prompt for AI agents
In `packages/bridge-contracts/deploy/deployOFT.ts` at lines 149-150:

**Reruns create a second proxy pair**

When a different signer resumes or reruns deployment after the original proxies have been deployed, `getDeploymentAddress` predicts addresses from the new `root.address`, so `deployProxyIfMissing` deploys a second pair and `deployments.save` replaces the local records without disabling the original pair. Later scripts target the new pair while the original can remain authorized or wired.

Reuse existing deployment records on reruns, and do not replace them with proxies predicted from a different signer.


const deployProxyIfMissing = async (name: string, address: string, salt: BigNumber, logic: string, initData: string) => {
if ((await ethers.provider.getCode(address)) !== '0x') {
console.log(`${name} proxy already deployed`, address);
} else {
const tx = await proxyFactory.deployProxy(salt, logic, initData);
await tx.wait();
console.log(`${name} proxy deployed and initialized`, address, tx.hash);
}
await deployments.save(name, { address, abi: (await hre.artifacts.readArtifact(name)).abi });
};

const minterBurnerInterface = (await ethers.getContractFactory('GoodDollarOFTMinterBurner')).interface;
await deployProxyIfMissing(
'GoodDollarOFTMinterBurner',
minterBurnerAddress,
minterBurnerProxySalt,
minterBurnerImpl.address,
minterBurnerInterface.encodeFunctionData('initialize', [nameServiceAddress, oftAdapterAddress]),
);

if (!oftAdapterInitialized) {
console.log('Initializing GoodDollarOFTAdapter...');
const oftAdapterInitData = oftAdapterContract.interface.encodeFunctionData('initialize', [
// Deployer owns the adapter until oft:configure hands owner + LZ delegate to the Avatar; fees go to the Avatar
const oftAdapterInterface = (await ethers.getContractFactory('GoodDollarOFTAdapter')).interface;
await deployProxyIfMissing(
'GoodDollarOFTAdapter',
oftAdapterAddress,
oftAdapterProxySalt,
oftAdapterImpl.address,
oftAdapterInterface.encodeFunctionData('initialize', [
tokenAddress,
minterBurnerAddress,
root.address,
root.address,
]);
await deployments.execute(
'GoodDollarOFTAdapter',
{ from: root.address },
'initialize',
oftAdapterImpl.address,
oftAdapterInitData,
);
console.log('GoodDollarOFTAdapter initialized');
console.log('Fee recipient:', avatarAddress);
} else {
console.log('GoodDollarOFTAdapter already initialized');
}
avatarAddress,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High · Fee-bearing receives revert

When the Avatar has no ERC777 recipient implementer registered and bridge fees are positive, _credit sends the fee to the Avatar through minterBurner.mint; SuperGoodDollar rejects the mint, so the entire receive reverts and incoming transfers remain undelivered.

Register the Avatar as an ERC777 recipient or route fees to a recipient that can receive ERC777 tokens.

Prompt for AI agents
In `packages/bridge-contracts/deploy/deployOFT.ts` at line 183:

**Fee-bearing receives revert**

When the Avatar has no ERC777 recipient implementer registered and bridge fees are positive, `_credit` sends the fee to the Avatar through `minterBurner.mint`; SuperGoodDollar rejects the mint, so the entire receive reverts and incoming transfers remain undelivered.

Register the Avatar as an ERC777 recipient or route fees to a recipient that can receive ERC777 tokens.

]),
);

// Verify GoodDollarOFTAdapter implementation (constructor: tokenAddress, lzEndpoint) on non-local networks (skip: 'hardhat', 'localhost')
await verifyOftDeployment(networkName, [root.address, avatarAddress]);

// Verify implementations on non-local networks (skip: 'hardhat', 'localhost')
if (!['hardhat', 'localhost'].includes(networkName)) {
await verifyContract(hre as any, minterBurnerImpl.address, [], 'GoodDollarOFTMinterBurner');
await verifyContract(hre as any, oftAdapterImpl.address, [tokenAddress, lzEndpoint], 'GoodDollarOFTAdapter');
}

Expand All @@ -234,7 +210,8 @@ const func: DeployFunction = async function (hre) {
console.log(' Implementation:', oftAdapterImplAddress);
}
console.log('Token:', tokenAddress);
console.log('OFT Adapter Owner (Avatar):', avatarAddress);
console.log('OFT Adapter Owner (deployer, until oft:configure):', root.address);
console.log('Fee recipient (Avatar):', avatarAddress);
console.log('LayerZero Endpoint:', lzEndpoint);
console.log('========================\n');
};
Expand Down
56 changes: 56 additions & 0 deletions packages/bridge-contracts/deploy/utils/verifyOftDeployment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { deployments, ethers } from 'hardhat';
import Contracts from '@gooddollar/goodprotocol/releases/deployment.json';

const EIP1967_IMPLEMENTATION_SLOT = '0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc';

async function readProxyImplementation(proxy: string): Promise<string> {
const raw = await ethers.provider.getStorageAt(proxy, EIP1967_IMPLEMENTATION_SLOT);
return ethers.utils.getAddress(ethers.utils.hexDataSlice(raw, 12));
}

function expectEqual(label: string, actual: string, expected: string) {
if (actual.toLowerCase() !== expected.toLowerCase()) {
throw new Error(`OFT deployment check failed: ${label} is ${actual}, expected ${expected}`);
}
}

/**
* Verifies the OFT proxies point at our implementations and were initialized with the expected wiring.
* Must pass before granting MINTER_ROLE or setting LZ peers.
* @param allowedOwners adapter owners accepted at this stage (deployer before handover, Avatar after)
*/
export async function verifyOftDeployment(networkName: string, allowedOwners: string[]) {
const gp = Contracts[networkName as keyof typeof Contracts] as any;
if (!gp?.GoodDollar || !gp?.Avatar) throw new Error(`Missing GoodDollar/Avatar for ${networkName}`);

const minterBurnerAddress = (await deployments.get('GoodDollarOFTMinterBurner')).address;
const adapterAddress = (await deployments.get('GoodDollarOFTAdapter')).address;

expectEqual(
'GoodDollarOFTAdapter implementation',
await readProxyImplementation(adapterAddress),
(await deployments.get('GoodDollarOFTAdapter_Implementation')).address,
);
expectEqual(
'GoodDollarOFTMinterBurner implementation',
await readProxyImplementation(minterBurnerAddress),
(await deployments.get('GoodDollarOFTMinterBurner_Implementation')).address,
);

const adapter = await ethers.getContractAt('GoodDollarOFTAdapter', adapterAddress);
const owner: string = await adapter.owner();
if (!allowedOwners.some((o) => o.toLowerCase() === owner.toLowerCase())) {
throw new Error(`OFT deployment check failed: adapter owner ${owner} not in [${allowedOwners.join(', ')}]`);
}
expectEqual('adapter.token()', await adapter.token(), gp.GoodDollar);
expectEqual('adapter.minterBurner()', await adapter.minterBurner(), minterBurnerAddress);
expectEqual('adapter.feeRecipient()', await adapter.feeRecipient(), gp.Avatar);

const minterBurner = await ethers.getContractAt('GoodDollarOFTMinterBurner', minterBurnerAddress);
expectEqual('minterBurner.token()', await minterBurner.token(), gp.GoodDollar);
if (!(await minterBurner.operators(adapterAddress))) {
throw new Error('OFT deployment check failed: adapter is not a MinterBurner operator');
}

console.log(`OFT deployment verified on ${networkName} (adapter owner ${owner})`);
}
8 changes: 8 additions & 0 deletions packages/bridge-contracts/scripts/oft/configure-oft.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { spawnSync } from "child_process";
import path from "path";
import { task } from "hardhat/config";
import type { HardhatRuntimeEnvironment } from "hardhat/types";
import Contracts from "@gooddollar/goodprotocol/releases/deployment.json";

export type ConfigureFlags = {
skipMinter: boolean;
Expand Down Expand Up @@ -62,10 +63,17 @@ export async function configureOft(hre: HardhatRuntimeEnvironment, flags: Config
const { main: grantMinterRole } = await import("./steps/grant-minter-role");
const { main: setBridgeLimits } = await import("./steps/set-bridge-limits");
const { main: transferAdapterOwnership } = await import("./steps/adapter-ownership");
const { verifyOftDeployment } = await import("../../deploy/utils/verifyOftDeployment");

const networkName = hre.network.name;
console.log(`oft:configure ${networkName}`, flags);

// Never wire peers or grant MINTER_ROLE to proxies we can't prove are ours
const [signer] = await hre.ethers.getSigners();
const avatar = (Contracts[networkName as keyof typeof Contracts] as any)?.Avatar;
if (!avatar) throw new Error(`Avatar not found for ${networkName}`);
await verifyOftDeployment(networkName, [signer.address, avatar]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High · Configuration mutates before delegate check

When the adapter is already Avatar-owned, its endpoint delegate is not the Avatar, and configuration runs with minter or limit steps enabled, verifyOftDeployment accepts the Avatar as owner without checking the endpoint delegate, so configureOft can grant the minter role and set limits before adapter-ownership detects the bad delegate and throws. The command fails with the bridge partly configured and the delegate mismatch unresolved.

Check the endpoint delegate during the initial preflight and abort before any configuration mutations when it is inconsistent.

Also at packages/bridge-contracts/scripts/oft/steps/adapter-ownership.ts:26.

Prompt for AI agents
In `packages/bridge-contracts/scripts/oft/configure-oft.ts` at line 75:

**Configuration mutates before delegate check**

When the adapter is already Avatar-owned, its endpoint delegate is not the Avatar, and configuration runs with minter or limit steps enabled, `verifyOftDeployment` accepts the Avatar as owner without checking the endpoint delegate, so `configureOft` can grant the minter role and set limits before `adapter-ownership` detects the bad delegate and throws. The command fails with the bridge partly configured and the delegate mismatch unresolved.

Check the endpoint delegate during the initial preflight and abort before any configuration mutations when it is inconsistent.

Also at `packages/bridge-contracts/scripts/oft/steps/adapter-ownership.ts:26`.


if (!flags.skipWire) wireLayerZero(networkName);
if (!flags.skipMinter) await grantMinterRole();
if (!flags.skipLimits) await setBridgeLimits();
Expand Down
Loading
Loading