fix(oft): audit fixes C-01, H-01, H-02, H-03, M-01a, L-05

packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol

@@ -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);
@@ -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 {
     using OFTMsgCodec for bytes;
     using OFTMsgCodec for bytes32;
     /// @dev Struct for storing bridge fees
@@ -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)
@@ -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);
@@ -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.
@@ -409,7 +425,7 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, OFTCoreUpgradeable {
             emit FeeCollected(feeRecipient, fee);
         }
 
-        return _amountLD;
+        return recipientAmount;
     }
 
     /**

packages/bridge-contracts/contracts/test/ReentrancyPoC.sol

@@ -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).
@@ -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 {

packages/bridge-contracts/deploy/deployOFT.ts

@@ -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;
@@ -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,
@@ -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,
@@ -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(
+    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);
+
+  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,
+    ]),
+  );
 
-  // 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');
   }
 
@@ -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');
 };

packages/bridge-contracts/deploy/utils/verifyOftDeployment.ts

@@ -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})`);
+}

packages/bridge-contracts/scripts/oft/configure-oft.ts

@@ -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;
@@ -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]);
+
   if (!flags.skipWire) wireLayerZero(networkName);
   if (!flags.skipMinter) await grantMinterRole();
   if (!flags.skipLimits) await setBridgeLimits();