From 394da6443ac210e9ca7f22a41d4c1a5b861b9709 Mon Sep 17 00:00:00 2001 From: sirpy Date: Sun, 11 Oct 2026 16:50:57 +0300 Subject: [PATCH] fix(oft): audit medium/low remediations - M-01: cap fee at MAX_FEE_BPS, require minFee <= maxFee, clamp fee to amount - M-02: revert composed messages on limit failure instead of parking - M-03: separate inbound/outbound daily counters - M-05: require LayerZero Labs + Nethermind DVNs; add eids to production networks - L-01/L-02: adapter is the spender (approvalRequired true), MinterBurner burns only caller tokens with balance-delta check - L-03: onlyOperators checks the live Controller avatar - L-04: disable initializers on the MinterBurner implementation - L-06: fail closed when whitelist identity is unresolvable; emit NameServiceSet - L-07: validate setBridgeLimits - L-08: enforce limits on the dust-free amount - L-09: quoteOFT returns real min/max limits - upgradeOFT routes Avatar-owned upgrades through the DAO - add oft:verify post-deploy invariant task - remove dead generateGuid --- .../contracts/oft/GoodDollarOFTAdapter.sol | 167 +++++--- .../oft/GoodDollarOFTMinterBurner.sol | 15 +- .../oft/interfaces/ISuperGoodDollar.sol | 2 + .../test/GoodDollarOFTAdapterHarness.sol | 10 + .../bridge-contracts/deploy/upgradeOFT.ts | 63 ++- packages/bridge-contracts/hardhat.config.ts | 7 +- packages/bridge-contracts/package.json | 1 + .../scripts/oft/bridge-oft-token.ts | 37 +- .../scripts/oft/layerzero.config.ts | 3 +- .../scripts/oft/steps/set-bridge-limits.ts | 2 +- .../scripts/oft/verify-invariants.ts | 111 +++++ .../test/GoodDollarOFT.e2e.test.ts | 117 ++++-- .../test/GoodDollarOFT.reentrancy.test.ts | 3 +- .../test/GoodDollarOFT.test.ts | 389 ++++++++++++++++-- .../test/helpers/deployMinterBurnerProxy.ts | 11 + 15 files changed, 776 insertions(+), 162 deletions(-) create mode 100644 packages/bridge-contracts/scripts/oft/verify-invariants.ts create mode 100644 packages/bridge-contracts/test/helpers/deployMinterBurnerProxy.ts diff --git a/packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol b/packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol index 7486656..241e616 100644 --- a/packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol +++ b/packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol @@ -2,6 +2,7 @@ pragma solidity >=0.8.0; import { IERC20, IERC20Metadata } from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; +import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; 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"; @@ -20,6 +21,11 @@ interface IIdentity { contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OFTCoreUpgradeable { using OFTMsgCodec for bytes; using OFTMsgCodec for bytes32; + using SafeERC20 for IERC20; + + /// @dev Upper bound for bridgeFees.fee (5%) + uint256 public constant MAX_FEE_BPS = 500; + /// @dev Struct for storing bridge fees struct BridgeFees { uint256 minFee; @@ -39,7 +45,7 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OF uint256 bridged24Hours; } - /// @dev Struct for storing bridge limits + /// @dev Struct for storing bridge limits. Windows are tumbling (not sliding), so up to 2x a limit can pass around a reset struct BridgeLimits { uint256 dailyLimit; uint256 txLimit; @@ -66,10 +72,10 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OF /// @dev Bridge limits structure BridgeLimits public bridgeLimits; - /// @dev Bridge daily limit tracking + /// @dev Outbound (send) bridge daily limit tracking BridgeDailyLimit public bridgeDailyLimit; - /// @dev Account-specific daily limit tracking + /// @dev Outbound (send) account daily limit tracking mapping(address => AccountLimit) public accountsDailyLimit; struct FailedReceiveRequest { @@ -90,6 +96,12 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OF /// @dev NameService for identity checks (optional, can be address(0)) INameService public nameService; + /// @dev Inbound (receive) bridge daily limit tracking, separate so sends can't exhaust the receive budget + BridgeDailyLimit public bridgeReceiveDailyLimit; + + /// @dev Inbound (receive) account daily limit tracking + mapping(address => AccountLimit) public accountsReceiveDailyLimit; + /// @dev Error for bridge limits violations error BRIDGE_LIMITS(string reason); @@ -126,6 +138,9 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OF /// @dev Event emitted when a failed receive request is rejected by the owner event FailedReceiveRequestRejected(bytes32 indexed guid); + /// @dev Event emitted when the NameService is updated + event NameServiceSet(address indexed nameService); + /** * @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) @@ -180,18 +195,19 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OF /** * @notice Indicates whether the OFT contract requires approval of the underlying token to send - * @return requiresApproval False because this adapter uses mint and burn privileges + * @return requiresApproval True: the adapter pulls tokens from the sender before burning them */ function approvalRequired() external pure returns (bool) { - return false; + return true; } /** * @notice Sets the bridge fees configuration - * @param _fees The bridge fees struct containing minFee, maxFee, and fee (in basis points) + * @param _fees The bridge fees struct containing minFee, maxFee (0 = no cap), and fee (in basis points) */ function setBridgeFees(BridgeFees memory _fees) external onlyOwner { - require(_fees.fee <= 10000, 'invalid fee'); + require(_fees.fee <= MAX_FEE_BPS, 'invalid fee'); + require(_fees.maxFee == 0 || _fees.minFee <= _fees.maxFee, 'minFee > maxFee'); bridgeFees = _fees; emit BridgeFeesSet(_fees.minFee, _fees.maxFee, _fees.fee); } @@ -209,7 +225,7 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OF /** * @notice Calculates the fee amount from the given amount * @param amount The amount to calculate fee from - * @return fee The calculated fee amount (enforced to be between minFee and maxFee if set) + * @return fee The calculated fee amount (enforced to be between minFee and maxFee if set, never above amount) */ function _takeFee(uint256 amount) internal view returns (uint256 fee) { fee = (amount * bridgeFees.fee) / 10000; @@ -219,6 +235,10 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OF if (bridgeFees.maxFee > 0 && fee > bridgeFees.maxFee) { fee = bridgeFees.maxFee; } + // a minFee above the amount must not underflow and brick the message + if (fee > amount) { + fee = amount; + } } /** @@ -226,6 +246,9 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OF * @param _limits The bridge limits struct */ function setBridgeLimits(BridgeLimits memory _limits) external onlyOwner { + require(_limits.minAmount <= _limits.txLimit, 'minAmount > txLimit'); + require(_limits.txLimit <= _limits.dailyLimit, 'txLimit > dailyLimit'); + require(_limits.accountDailyLimit <= _limits.dailyLimit, 'accountDailyLimit > dailyLimit'); bridgeLimits = _limits; emit BridgeLimitsSet( _limits.dailyLimit, @@ -242,6 +265,7 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OF */ function setNameService(INameService _nameService) external onlyOwner { nameService = _nameService; + emit NameServiceSet(address(_nameService)); } /** @@ -255,6 +279,7 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OF /** * @notice Approves a failed receive request with optimistic window or by owner + * @dev Approved amounts intentionally skip the daily limit counters */ function approveFailedRequest(bytes32 _guid) external nonReentrant { require(!isClosed, 'closed'); @@ -282,11 +307,12 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OF */ function _checkBridgeClosedAndWhitelisted(address _from) internal view returns (bool ok, string memory reason) { if (isClosed) return (false, 'closed'); - if (bridgeLimits.onlyWhitelisted && address(nameService) != address(0)) { + if (bridgeLimits.onlyWhitelisted) { + // fail closed: whitelist mode without a resolvable Identity blocks transfers + if (address(nameService) == address(0)) return (false, 'identity not set'); IIdentity id = IIdentity(nameService.getAddress("IDENTITY")); - if (address(id) != address(0) && !id.isWhitelisted(_from)) { - return (false, 'not whitelisted'); - } + if (address(id) == address(0)) return (false, 'identity not set'); + if (!id.isWhitelisted(_from)) return (false, 'not whitelisted'); } return (true, ''); } @@ -295,11 +321,15 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OF * @notice Bridge limits check only (minAmount, accountDailyLimit, txLimit, dailyLimit). * @dev Assumes daily limit resets have already been applied. Failure on receive is stored in failedReceiveRequests. */ - function _checkBridgeLimits(address _from, uint256 _amount) internal view returns (bool ok, string memory reason) { + function _checkBridgeLimits( + uint256 _amount, + BridgeDailyLimit storage _daily, + AccountLimit storage _account + ) internal view returns (bool ok, string memory reason) { if (_amount < bridgeLimits.minAmount) return (false, 'minAmount'); - uint256 account24hours = accountsDailyLimit[_from].bridged24Hours; - if (accountsDailyLimit[_from].lastTransferReset < block.timestamp - 1 days) { + uint256 account24hours = _account.bridged24Hours; + if (_account.lastTransferReset < block.timestamp - 1 days) { account24hours = _amount; } else { account24hours += _amount; @@ -308,45 +338,86 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OF if (_amount > bridgeLimits.txLimit) return (false, 'txLimit'); - if (bridgeDailyLimit.lastTransferReset < block.timestamp - 1 days) { + if (_daily.lastTransferReset < block.timestamp - 1 days) { if (_amount > bridgeLimits.dailyLimit) return (false, 'dailyLimit'); } else { - if (bridgeDailyLimit.bridged24Hours + _amount > bridgeLimits.dailyLimit) return (false, 'dailyLimit'); + if (_daily.bridged24Hours + _amount > bridgeLimits.dailyLimit) return (false, 'dailyLimit'); } return (true, ''); } /** * @notice Resets bridge and account daily limit counters if the 24h window has elapsed. - * @param _address The account address for which to reset account daily limit. */ - function _resetDailyLimitsIfNeeded(address _address) internal { - if (bridgeDailyLimit.lastTransferReset < block.timestamp - 1 days) { - bridgeDailyLimit.lastTransferReset = block.timestamp; - bridgeDailyLimit.bridged24Hours = 0; + function _resetDailyLimitsIfNeeded(BridgeDailyLimit storage _daily, AccountLimit storage _account) internal { + if (_daily.lastTransferReset < block.timestamp - 1 days) { + _daily.lastTransferReset = block.timestamp; + _daily.bridged24Hours = 0; } - if (accountsDailyLimit[_address].lastTransferReset < block.timestamp - 1 days) { - accountsDailyLimit[_address].lastTransferReset = block.timestamp; - accountsDailyLimit[_address].bridged24Hours = 0; + if (_account.lastTransferReset < block.timestamp - 1 days) { + _account.lastTransferReset = block.timestamp; + _account.bridged24Hours = 0; } } /** * @notice Enforces transfer limits: bridge closed/whitelisted check then bridge limits check. - * @dev Used on send path. Resets daily windows, then checks; on success updates counters. + * @dev Used on send path. Resets daily windows, then checks; on success updates outbound counters. */ function _enforceLimits(address _address, uint256 _amount) internal returns (bool isValid, string memory reason) { - _resetDailyLimitsIfNeeded(_address); + AccountLimit storage account = accountsDailyLimit[_address]; + _resetDailyLimitsIfNeeded(bridgeDailyLimit, account); (isValid, reason) = _checkBridgeClosedAndWhitelisted(_address); if (!isValid) return (false, reason); - (isValid, reason) = _checkBridgeLimits(_address, _amount); + (isValid, reason) = _checkBridgeLimits(_amount, bridgeDailyLimit, account); if (!isValid) return (false, reason); bridgeDailyLimit.bridged24Hours += _amount; - accountsDailyLimit[_address].bridged24Hours += _amount; + account.bridged24Hours += _amount; return (true, ''); } + + /** + * @notice Quotes the OFT send with the real bridge limits for msg.sender + * @dev maxAmountLD = min(txLimit, remaining daily, remaining account daily); 0 when closed + */ + function quoteOFT( + SendParam calldata _sendParam + ) + external + view + virtual + override + returns (OFTLimit memory oftLimit, OFTFeeDetail[] memory oftFeeDetails, OFTReceipt memory oftReceipt) + { + uint256 maxAmount; + if (!isClosed) { + BridgeLimits memory limits = bridgeLimits; + uint256 dailyUsed = bridgeDailyLimit.lastTransferReset < block.timestamp - 1 days + ? 0 + : bridgeDailyLimit.bridged24Hours; + AccountLimit memory account = accountsDailyLimit[msg.sender]; + uint256 accountUsed = account.lastTransferReset < block.timestamp - 1 days ? 0 : account.bridged24Hours; + + maxAmount = limits.txLimit; + uint256 dailyLeft = limits.dailyLimit > dailyUsed ? limits.dailyLimit - dailyUsed : 0; + uint256 accountLeft = limits.accountDailyLimit > accountUsed ? limits.accountDailyLimit - accountUsed : 0; + if (dailyLeft < maxAmount) maxAmount = dailyLeft; + if (accountLeft < maxAmount) maxAmount = accountLeft; + } + oftLimit = OFTLimit(bridgeLimits.minAmount, maxAmount); + + // TODO: destination-chain bridge fee quoting is an open decision; fees are left out of the quote for now + oftFeeDetails = new OFTFeeDetail[](0); + + (uint256 amountSentLD, uint256 amountReceivedLD) = _debitView( + _sendParam.amountLD, + _sendParam.minAmountLD, + _sendParam.dstEid + ); + oftReceipt = OFTReceipt(amountSentLD, amountReceivedLD); + } /** * @notice Overrides the default _send function to enforce limits on sending side @@ -358,7 +429,8 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OF ) internal virtual override returns (MessagingReceipt memory msgReceipt, OFTReceipt memory oftReceipt) { /// @dev revert if sending to zero address require(_sendParam.to.bytes32ToAddress() != address(0), 'GoodDollarOFTAdapter: sending to zero address'); - (bool isValid, string memory reason) = _enforceLimits(msg.sender, _sendParam.amountLD); + // limits apply to the dust-free amount that is actually burned and bridged + (bool isValid, string memory reason) = _enforceLimits(msg.sender, _removeDust(_sendParam.amountLD)); if (!isValid) { revert BRIDGE_LIMITS(reason); } @@ -367,7 +439,7 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OF /** * @notice Overrides the default _lzReceive function to enforce limits on receiving side. - * @dev Bridge closed/whitelisted check: revert only. Bridge limits check: store in failedReceiveRequests then revert. + * @dev Closed/not whitelisted: revert. Limits exceeded: store in failedReceiveRequests (composed messages revert instead). */ function _lzReceive( Origin calldata _origin, @@ -384,10 +456,13 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OF revert BRIDGE_NOT_ALLOWED(reason); } - _resetDailyLimitsIfNeeded(toAddress); + AccountLimit storage account = accountsReceiveDailyLimit[toAddress]; + _resetDailyLimitsIfNeeded(bridgeReceiveDailyLimit, account); - (ok, reason) = _checkBridgeLimits(toAddress, amountLD); + (ok, reason) = _checkBridgeLimits(amountLD, bridgeReceiveDailyLimit, account); if (!ok) { + // parking would drop the compose payload; reverting keeps the message retryable at the endpoint + if (_message.isComposed()) revert BRIDGE_LIMITS(reason); failedReceiveRequests[_guid] = FailedReceiveRequest( true, toAddress, @@ -397,8 +472,8 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OF ); emit ReceiveRequestFailed(_guid, toAddress, amountLD, _origin.srcEid); } else{ - bridgeDailyLimit.bridged24Hours += amountLD; - accountsDailyLimit[toAddress].bridged24Hours += amountLD; + bridgeReceiveDailyLimit.bridged24Hours += amountLD; + account.bridged24Hours += amountLD; super._lzReceive(_origin, _guid, _message, _executor, _extraData); } } @@ -429,7 +504,7 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OF } /** - * @notice Burns tokens from the sender's balance to prepare for sending + * @notice Pulls tokens from the sender (adapter is the spender) and burns them via the MinterBurner * @param _from The address to debit the tokens from * @param _amountLD The amount of tokens to send in local decimals * @param _minAmountLD The minimum amount to send in local decimals @@ -444,21 +519,11 @@ contract GoodDollarOFTAdapter is UUPSUpgradeable, ReentrancyGuardUpgradeable, OF uint32 _dstEid ) internal virtual override returns (uint256 amountSentLD, uint256 amountReceivedLD) { (amountSentLD, amountReceivedLD) = _debitView(_amountLD, _minAmountLD, _dstEid); - minterBurner.burn(_from, amountSentLD); - } - - function generateGuid( - uint64 _nonce, - uint32 _srcEid, - address _sender, - uint32 _dstEid, - bytes32 _receiver - ) public pure returns (bytes32) { - return keccak256(abi.encodePacked(_nonce, _srcEid, _toBytes32(_sender), _dstEid, _receiver)); - } - - function _toBytes32(address _address) internal pure returns (bytes32) { - return bytes32(uint256(uint160(_address))); + uint256 balanceBefore = innerToken.balanceOf(address(this)); + innerToken.safeTransferFrom(_from, address(this), amountSentLD); + require(innerToken.balanceOf(address(this)) - balanceBefore == amountSentLD, 'received amount mismatch'); + innerToken.forceApprove(address(minterBurner), amountSentLD); + require(minterBurner.burn(address(this), amountSentLD), 'GoodDollarOFTAdapter: Burn failed'); } function _authorizeUpgrade(address) internal virtual override onlyOwner {} diff --git a/packages/bridge-contracts/contracts/oft/GoodDollarOFTMinterBurner.sol b/packages/bridge-contracts/contracts/oft/GoodDollarOFTMinterBurner.sol index bfbc1d9..646237c 100644 --- a/packages/bridge-contracts/contracts/oft/GoodDollarOFTMinterBurner.sol +++ b/packages/bridge-contracts/contracts/oft/GoodDollarOFTMinterBurner.sol @@ -22,11 +22,17 @@ contract GoodDollarOFTMinterBurner is DAOUpgradeableContract { event TokensBurned(address indexed from, uint256 amount, address indexed operator); modifier onlyOperators() { - require(operators[msg.sender] || msg.sender == avatar, 'Not authorized'); + // live avatar: the cached `avatar` can go stale after a DAO avatar change + require(operators[msg.sender] || msg.sender == address(dao.avatar()), 'Not authorized'); require(!paused, 'Contract is paused'); _; } + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() { + _disableInitializers(); + } + /** * @dev Initialize the MinterBurner contract * @param _nameService The NameService contract for DAO integration @@ -55,13 +61,16 @@ contract GoodDollarOFTMinterBurner is DAOUpgradeableContract { } /** - * @dev Burn tokens from an address - * @param _from The address to burn tokens from + * @dev Burn the caller's tokens; the caller must have approved this contract + * @param _from Must be msg.sender (kept for IMintableBurnable compatibility) * @param _amount The amount of tokens to burn * @return success True if the burn was successful */ function burn(address _from, uint256 _amount) external onlyOperators returns (bool) { + require(_from == msg.sender, 'can only burn own tokens'); + uint256 balanceBefore = token.balanceOf(address(this)); require(token.transferFrom(_from, address(this), _amount), 'TRANSFER_FROM_FAILED'); + require(token.balanceOf(address(this)) - balanceBefore == _amount, 'received amount mismatch'); token.burn(_amount); emit TokensBurned(_from, _amount, msg.sender); diff --git a/packages/bridge-contracts/contracts/oft/interfaces/ISuperGoodDollar.sol b/packages/bridge-contracts/contracts/oft/interfaces/ISuperGoodDollar.sol index 3686c6a..0d62928 100644 --- a/packages/bridge-contracts/contracts/oft/interfaces/ISuperGoodDollar.sol +++ b/packages/bridge-contracts/contracts/oft/interfaces/ISuperGoodDollar.sol @@ -15,5 +15,7 @@ interface ISuperGoodDollar { function transferFrom(address from, address to, uint256 amount) external returns (bool); + function balanceOf(address account) external view returns (uint256); + function addMinter(address _minter) external; } diff --git a/packages/bridge-contracts/contracts/test/GoodDollarOFTAdapterHarness.sol b/packages/bridge-contracts/contracts/test/GoodDollarOFTAdapterHarness.sol index 9c51c54..699d0f9 100644 --- a/packages/bridge-contracts/contracts/test/GoodDollarOFTAdapterHarness.sol +++ b/packages/bridge-contracts/contracts/test/GoodDollarOFTAdapterHarness.sol @@ -31,4 +31,14 @@ contract GoodDollarOFTAdapterHarness is GoodDollarOFTAdapter { ) external returns (uint256) { return _credit(_to, _amountLD, _srcEid); } + + /// @dev Calls internal _debit to test the pull-and-burn path without a full LayerZero send + function exposed_debit( + address _from, + uint256 _amountLD, + uint256 _minAmountLD, + uint32 _dstEid + ) external returns (uint256 amountSentLD, uint256 amountReceivedLD) { + return _debit(_from, _amountLD, _minAmountLD, _dstEid); + } } diff --git a/packages/bridge-contracts/deploy/upgradeOFT.ts b/packages/bridge-contracts/deploy/upgradeOFT.ts index a547976..5a65150 100644 --- a/packages/bridge-contracts/deploy/upgradeOFT.ts +++ b/packages/bridge-contracts/deploy/upgradeOFT.ts @@ -5,7 +5,8 @@ * - Reads proxy addresses from hardhat-deploy artifacts (`deployments/`) for the current network * - Resolves token and LayerZero endpoint from GoodProtocol deployment (same as deployOFT) * - Deploys new implementations (OFT adapter with constructor args, MinterBurner with none) - * - Calls upgradeTo(newImplementation) on each proxy (UUPS) + * - Upgrades each proxy (UUPS) through the DAO: Controller.genericCall when the deployer is a registered scheme, + * otherwise prints the genericCall payloads to submit as a DAO proposal * * Usage: * npx hardhat deploy --tags OFT-Upgrade --network development-xdc @@ -91,6 +92,25 @@ const func: DeployFunction = async function (hre) { console.log("Controller:", controllerAddress); console.log("Avatar:", avatarAddress); + const isScheme: boolean = await Controller.isSchemeRegistered(root.address, avatarAddress); + const pendingProposals: { target: string; data: string }[] = []; + + // Avatar-owned proxies are upgraded via the DAO; a deployer-owned adapter (before handover) is upgraded directly + const upgradeProxy = async (name: string, proxy: string, newImpl: string, ownedByAvatar: boolean) => { + const contract = await ethers.getContractAt(name, proxy); + const data = contract.interface.encodeFunctionData("upgradeTo", [newImpl]); + if (!ownedByAvatar) { + await (await contract.upgradeTo(newImpl)).wait(); + console.log(`✅ ${name} upgraded by owner`); + } else if (isScheme) { + await (await Controller.genericCall(proxy, data, avatarAddress, 0)).wait(); + console.log(`✅ ${name} upgraded via DAO avatar`); + } else { + pendingProposals.push({ target: proxy, data }); + console.log(`⏳ ${name} upgrade requires a DAO proposal`); + } + }; + // CREATE2 salt for implementations: // hardhat-deploy uses `deterministicDeployment` as the CREATE2 salt. // We derive it from the contract's compiled bytecode. @@ -111,14 +131,16 @@ const func: DeployFunction = async function (hre) { }); console.log("GoodDollarOFTAdapter implementation", oftAdapterImpl.address); - console.log("Upgrading GoodDollarOFTAdapter proxy via upgradeTo..."); + console.log("Upgrading GoodDollarOFTAdapter proxy..."); const oftAdapterProxyContract = await ethers.getContractAt( "GoodDollarOFTAdapter", oftAdapterProxy ); - const txOft = await oftAdapterProxyContract.upgradeTo(oftAdapterImpl.address); - await txOft.wait(); - console.log("✅ GoodDollarOFTAdapter upgraded"); + const adapterOwner: string = await oftAdapterProxyContract.owner(); + if (adapterOwner !== avatarAddress && adapterOwner !== root.address) { + throw new Error(`GoodDollarOFTAdapter owner ${adapterOwner} is neither the Avatar nor the deployer`); + } + await upgradeProxy("GoodDollarOFTAdapter", oftAdapterProxy, oftAdapterImpl.address, adapterOwner === avatarAddress); // --- GoodDollarOFTMinterBurner: deploy new implementation via hardhat-deploy, then upgrade proxy --- console.log("\nDeploying new GoodDollarOFTMinterBurner implementation..."); @@ -133,22 +155,21 @@ const func: DeployFunction = async function (hre) { ); console.log("GoodDollarOFTMinterBurner implementation", minterBurnerImpl.address); - console.log("Upgrading GoodDollarOFTMinterBurner proxy via Controller.genericCall (avatar)..."); - const minterBurnerProxyContract = await ethers.getContractAt( - "GoodDollarOFTMinterBurner", - minterBurnerProxy - ); - const upgradeData = minterBurnerProxyContract.interface.encodeFunctionData("upgradeTo", [ - minterBurnerImpl.address, - ]); - const txMb = await Controller.genericCall( - minterBurnerProxy, - upgradeData, - avatarAddress, - 0 - ); - await txMb.wait(); - console.log("✅ GoodDollarOFTMinterBurner upgraded via DAO avatar"); + console.log("Upgrading GoodDollarOFTMinterBurner proxy via the DAO avatar..."); + await upgradeProxy("GoodDollarOFTMinterBurner", minterBurnerProxy, minterBurnerImpl.address, true); + + if (pendingProposals.length > 0) { + console.log("\n=== DAO proposal required (Controller.genericCall(target, data, avatar, 0)) ==="); + console.log("Controller:", controllerAddress, "Avatar:", avatarAddress); + for (const p of pendingProposals) { + console.log( + JSON.stringify({ + ...p, + genericCall: Controller.interface.encodeFunctionData("genericCall", [p.target, p.data, avatarAddress, 0]), + }) + ); + } + } const minterBurnerImplAddress = await getImplementationAddress( ethers.provider, diff --git a/packages/bridge-contracts/hardhat.config.ts b/packages/bridge-contracts/hardhat.config.ts index f149325..4f90462 100644 --- a/packages/bridge-contracts/hardhat.config.ts +++ b/packages/bridge-contracts/hardhat.config.ts @@ -16,6 +16,7 @@ import '@layerzerolabs/toolbox-hardhat'; import { EndpointId } from '@layerzerolabs/lz-definitions'; import './scripts/oft/configure-oft'; +import './scripts/oft/verify-invariants'; configDotenv(); envEnc.config(); @@ -144,12 +145,14 @@ const config: HardhatUserConfig = { gas: 8000000, gasPrice: 26e9, chainId: 42220, - }, + eid: EndpointId.CELO_V2_MAINNET, + } as any, 'production-xdc': { accounts: accounts as HttpNetworkAccountsConfig, chainId: 50, url: 'https://rpc.xdc.org', - }, + eid: EndpointId.XDC_V2_MAINNET, + } as any, 'development-xdc': { accounts: accounts as HttpNetworkAccountsConfig, chainId: 50, diff --git a/packages/bridge-contracts/package.json b/packages/bridge-contracts/package.json index 6dc79ea..5a8f9a6 100644 --- a/packages/bridge-contracts/package.json +++ b/packages/bridge-contracts/package.json @@ -9,6 +9,7 @@ "test": "hardhat test", "test:oft": "hardhat test ./test/GoodDollarOFT.test.ts", "oft:configure": "hardhat oft:configure", + "oft:verify": "hardhat oft:verify", "updateabi": "find ./artifacts/contracts/ -iregex '.*/[a-z]*.json' -exec cp \"{}\" ../bridge-app/src/abi \\;", "start:dev": "hardhat node & yarn build && yarn updateabi && hardhat run scripts/deployDevEnv.ts --network localhost", "mpb:deploy": "hardhat deploy --export-all ./release/mpb.json" diff --git a/packages/bridge-contracts/scripts/oft/bridge-oft-token.ts b/packages/bridge-contracts/scripts/oft/bridge-oft-token.ts index 2400fbf..e9556fb 100644 --- a/packages/bridge-contracts/scripts/oft/bridge-oft-token.ts +++ b/packages/bridge-contracts/scripts/oft/bridge-oft-token.ts @@ -99,20 +99,18 @@ const main = async () => { throw new Error(`Insufficient balance. Need ${ethers.utils.formatEther(amount)} G$, have ${ethers.utils.formatEther(balance)} G$`); } - // Check and approve MinterBurner if needed (required for burning tokens) - // The OFT adapter calls minterBurner.burn(), which calls token.burnFrom() requiring approval - // Note: OFT adapter itself doesn't need approval since approvalRequired() returns false - const minterBurnerAllowance = await token.allowance(sender.address, minterBurnerAddress); - console.log("\nChecking MinterBurner allowance..."); - console.log("Current MinterBurner allowance:", ethers.utils.formatEther(minterBurnerAllowance), "G$"); - - if (minterBurnerAllowance.lt(amount)) { - console.log("\nApproving MinterBurner to burn tokens..."); - const approveMinterBurnerTx = await token.approve(minterBurnerAddress, amount); - await approveMinterBurnerTx.wait(); - console.log("MinterBurner approval confirmed:", approveMinterBurnerTx.hash); + // The OFT adapter is the spender (approvalRequired() returns true): it pulls the tokens and burns them via the MinterBurner + const adapterAllowance = await token.allowance(sender.address, oftAdapterAddress); + console.log("\nChecking OFT adapter allowance..."); + console.log("Current OFT adapter allowance:", ethers.utils.formatEther(adapterAllowance), "G$"); + + if (adapterAllowance.lt(amount)) { + console.log("\nApproving OFT adapter..."); + const approveAdapterTx = await token.approve(oftAdapterAddress, amount); + await approveAdapterTx.wait(); + console.log("OFT adapter approval confirmed:", approveAdapterTx.hash); } else { - console.log("Sufficient MinterBurner allowance already set"); + console.log("Sufficient OFT adapter allowance already set"); } // Recipient address (same address on destination chain) @@ -159,15 +157,14 @@ const main = async () => { console.log(`✅ ${destNetwork} peer is configured correctly`); - await token.approve(minterBurnerAddress, amount); - // Double-check MinterBurner approval before calling quoteSend - console.log("\nVerifying MinterBurner approval before quoteSend..."); - const finalMinterBurnerAllowance = await token.allowance(sender.address, minterBurnerAddress); - console.log("Final MinterBurner allowance:", ethers.utils.formatEther(finalMinterBurnerAllowance), "G$"); + // Double-check adapter approval before calling quoteSend + console.log("\nVerifying OFT adapter approval before quoteSend..."); + const finalAdapterAllowance = await token.allowance(sender.address, oftAdapterAddress); + console.log("Final OFT adapter allowance:", ethers.utils.formatEther(finalAdapterAllowance), "G$"); - if (finalMinterBurnerAllowance.lt(amount)) { + if (finalAdapterAllowance.lt(amount)) { throw new Error( - `MinterBurner allowance insufficient. Need ${ethers.utils.formatEther(amount)} G$, have ${ethers.utils.formatEther(finalMinterBurnerAllowance)} G$` + `OFT adapter allowance insufficient. Need ${ethers.utils.formatEther(amount)} G$, have ${ethers.utils.formatEther(finalAdapterAllowance)} G$` ); } diff --git a/packages/bridge-contracts/scripts/oft/layerzero.config.ts b/packages/bridge-contracts/scripts/oft/layerzero.config.ts index 7c0ef4c..9c296e8 100644 --- a/packages/bridge-contracts/scripts/oft/layerzero.config.ts +++ b/packages/bridge-contracts/scripts/oft/layerzero.config.ts @@ -36,7 +36,8 @@ const pathways: TwoWayConfig[] = [ [ celoContract, xdcContract, - [["LayerZero Labs"], []], + // 2 required DVNs, both live on Celo and XDC (LayerZero metadata) + [["LayerZero Labs", "Nethermind"], []], [20, 20], [EVM_ENFORCED_OPTIONS, EVM_ENFORCED_OPTIONS], ], diff --git a/packages/bridge-contracts/scripts/oft/steps/set-bridge-limits.ts b/packages/bridge-contracts/scripts/oft/steps/set-bridge-limits.ts index 9af6837..3651bb2 100644 --- a/packages/bridge-contracts/scripts/oft/steps/set-bridge-limits.ts +++ b/packages/bridge-contracts/scripts/oft/steps/set-bridge-limits.ts @@ -3,7 +3,7 @@ import { BigNumber } from "ethers"; import { getOftDeploymentAddresses } from "../../../deploy/utils/getOftDeploymentAddresses"; import config from "../oft.config.json"; -function parseLimit(value: string | undefined): BigNumber | null { +export function parseLimit(value: string | undefined): BigNumber | null { if (!value) return null; const numValue = value.trim(); if (numValue.includes(".") || numValue.length < 15) { diff --git a/packages/bridge-contracts/scripts/oft/verify-invariants.ts b/packages/bridge-contracts/scripts/oft/verify-invariants.ts new file mode 100644 index 0000000..321969d --- /dev/null +++ b/packages/bridge-contracts/scripts/oft/verify-invariants.ts @@ -0,0 +1,111 @@ +import { task } from "hardhat/config"; +import type { HardhatRuntimeEnvironment } from "hardhat/types"; +import { EndpointId } from "@layerzerolabs/lz-definitions"; +import Contracts from "@gooddollar/goodprotocol/releases/deployment.json"; +import config from "./oft.config.json"; +import { getOftDeploymentAddresses } from "../../deploy/utils/getOftDeploymentAddresses"; + +const REMOTE_NETWORK: Record = { + "production-xdc": "production-celo", + "production-celo": "production-xdc", + "development-xdc": "development-celo", + "development-celo": "development-xdc", +}; + +const MIN_REQUIRED_DVNS = 2; +const ULN_CONFIG_TYPE = 2; +const ULN_CONFIG_ABI = [ + "tuple(uint64 confirmations, uint8 requiredDVNCount, uint8 optionalDVNCount, uint8 optionalDVNThreshold, address[] requiredDVNs, address[] optionalDVNs)", +]; +const ENDPOINT_ABI = [ + "function delegates(address) view returns (address)", + "function getSendLibrary(address _sender, uint32 _dstEid) view returns (address)", + "function getReceiveLibrary(address _receiver, uint32 _srcEid) view returns (address lib, bool isDefault)", + "function getConfig(address _oapp, address _lib, uint32 _eid, uint32 _configType) view returns (bytes)", +]; + +const eidOf = (networkName: string) => + networkName.includes("xdc") ? EndpointId.XDC_V2_MAINNET : EndpointId.CELO_V2_MAINNET; + +/** + * Post-deploy invariants: Avatar owns/delegates the adapter, peers, limits, DVNs and minting rights are as expected. + * Collects every failure and throws once at the end. + */ +export async function verifyOftInvariants(hre: HardhatRuntimeEnvironment) { + const { ethers, network } = hre; + const { verifyOftDeployment } = await import("../../deploy/utils/verifyOftDeployment"); + const { parseLimit } = await import("./steps/set-bridge-limits"); + + const networkName = network.name; + const remoteNetwork = REMOTE_NETWORK[networkName]; + if (!remoteNetwork) throw new Error(`Unknown network pairing for "${networkName}"`); + const gp = (Contracts as any)[networkName]; + const avatar: string = gp?.Avatar; + if (!avatar) throw new Error(`Avatar not found for ${networkName}`); + + // implementation slots, token/minterBurner/feeRecipient wiring, operators(adapter); owner must be the Avatar + await verifyOftDeployment(networkName, [avatar]); + + const failures: string[] = []; + const check = (ok: boolean, label: string) => { + console.log(`${ok ? "ok " : "FAIL"} ${label}`); + if (!ok) failures.push(label); + }; + const same = (a: string, b: string) => a.toLowerCase() === b.toLowerCase(); + + const { GoodDollarOFTAdapter: adapterAddress, GoodDollarOFTMinterBurner: minterBurnerAddress } = + getOftDeploymentAddresses(networkName); + const { GoodDollarOFTAdapter: remoteAdapter } = getOftDeploymentAddresses(remoteNetwork); + const adapter = await ethers.getContractAt("GoodDollarOFTAdapter", adapterAddress); + const endpoint = await ethers.getContractAt(ENDPOINT_ABI, await adapter.endpoint()); + const remoteEid = eidOf(remoteNetwork); + + check(same(await adapter.owner(), avatar), `adapter.owner() == Avatar ${avatar}`); + check(same(await endpoint.delegates(adapterAddress), avatar), "endpoint.delegates(adapter) == Avatar"); + check( + same(await adapter.peers(remoteEid), ethers.utils.hexZeroPad(remoteAdapter, 32)), + `adapter.peers(${remoteEid}) == ${remoteNetwork} adapter ${remoteAdapter}` + ); + + const token = await ethers.getContractAt( + "contracts/oft/interfaces/ISuperGoodDollar.sol:ISuperGoodDollar", + gp.GoodDollar + ); + check(await token.isMinter(minterBurnerAddress), "G$.isMinter(minterBurner)"); + + const expected = (config as any)[networkName]?.limits; + if (!expected) { + check(false, `limits for ${networkName} in oft.config.json`); + } else { + const actual = await adapter.bridgeLimits(); + for (const key of ["dailyLimit", "txLimit", "accountDailyLimit", "minAmount"]) { + check(parseLimit(expected[key])!.eq(actual[key]), `bridgeLimits.${key} == ${expected[key]}`); + } + check(actual.onlyWhitelisted === expected.onlyWhitelisted, `bridgeLimits.onlyWhitelisted == ${expected.onlyWhitelisted}`); + } + + const sendLib: string = await endpoint.getSendLibrary(adapterAddress, remoteEid); + const [receiveLib] = await endpoint.getReceiveLibrary(adapterAddress, remoteEid); + for (const [side, lib] of [ + ["send", sendLib], + ["receive", receiveLib], + ]) { + const raw = await endpoint.getConfig(adapterAddress, lib, remoteEid, ULN_CONFIG_TYPE); + const [uln] = ethers.utils.defaultAbiCoder.decode(ULN_CONFIG_ABI, raw); + check( + uln.requiredDVNCount >= MIN_REQUIRED_DVNS && uln.requiredDVNCount !== 255, + `${side} ULN requiredDVNCount ${uln.requiredDVNCount} >= ${MIN_REQUIRED_DVNS} (${uln.requiredDVNs.join(", ")})` + ); + } + + if (failures.length > 0) { + throw new Error(`OFT invariants failed on ${networkName}:\n- ${failures.join("\n- ")}`); + } + console.log(`OFT invariants hold on ${networkName}`); +} + +task("oft:verify", "Check post-deploy OFT invariants (ownership, delegate, peers, limits, DVNs, minter)").setAction( + async (_, hre) => { + await verifyOftInvariants(hre); + } +); diff --git a/packages/bridge-contracts/test/GoodDollarOFT.e2e.test.ts b/packages/bridge-contracts/test/GoodDollarOFT.e2e.test.ts index 59efa27..d74c573 100644 --- a/packages/bridge-contracts/test/GoodDollarOFT.e2e.test.ts +++ b/packages/bridge-contracts/test/GoodDollarOFT.e2e.test.ts @@ -5,6 +5,7 @@ import { GoodDollarOFTAdapter } from '../typechain-types'; import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'; import contracts from '@gooddollar/goodprotocol/releases/deployment.json'; import * as networkHelpers from "@nomicfoundation/hardhat-network-helpers"; +import { deployMinterBurnerProxy } from './helpers/deployMinterBurnerProxy'; const CELO_CHAIN_URL = 'https://forno.celo.org'; /** @@ -68,9 +69,7 @@ describe('GoodDollarOFTAdapter Fork Tests (Celo Production)', () => { const nameService = await ethers.getContractAt(nameServiceAbi, nameServiceAddress); // Deploy minter/burner without initializing yet (adapter address is required) - const MinterBurnerFactory = await ethers.getContractFactory('GoodDollarOFTMinterBurner'); - const minterBurner = await MinterBurnerFactory.deploy(); - await minterBurner.deployed(); + const minterBurner = await deployMinterBurnerProxy(); // Deploy adapter using upgrades plugin with constructor args const AdapterFactory = await ethers.getContractFactory('GoodDollarOFTAdapter'); @@ -107,9 +106,7 @@ describe('GoodDollarOFTAdapter Fork Tests (Celo Production)', () => { // Deploy MinterBurner using production NameService // Note: This requires NameService to have GOODDOLLAR registered - const MinterBurnerFactory = await ethers.getContractFactory('GoodDollarOFTMinterBurner'); - const minterBurner = await MinterBurnerFactory.deploy(); - await minterBurner.deployed(); + const minterBurner = await deployMinterBurnerProxy(); // Deploy adapter using upgrades plugin with constructor args const AdapterFactory = await ethers.getContractFactory('GoodDollarOFTAdapter'); @@ -137,7 +134,7 @@ describe('GoodDollarOFTAdapter Fork Tests (Celo Production)', () => { expect(await adapter.token()).to.equal(goodDollarAddress); expect(await adapter.minterBurner()).to.equal(minterBurner.address); expect(await adapter.owner()).to.equal(owner.address); - expect(await adapter.approvalRequired()).to.be.false; + expect(await adapter.approvalRequired()).to.be.true; console.log('Adapter deployed:', { address: adapter.address, @@ -272,18 +269,16 @@ describe('GoodDollarOFTAdapter Fork Tests (Celo Production)', () => { ).to.be.revertedWithCustomError(adapter, 'BRIDGE_LIMITS').withArgs('accountDailyLimit'); }); - it('Should revert with dailyLimit when bridge daily limit exceeded', async function () { - await adapter.setBridgeLimits({ - dailyLimit: ethers.utils.parseEther('1000'), - txLimit: ethers.utils.parseEther('10000'), - accountDailyLimit: ethers.utils.parseEther('100000'), - minAmount: ethers.utils.parseEther('100'), - onlyWhitelisted: false, - }); - const amount = ethers.utils.parseEther('1001'); + it('Should reject limits where txLimit exceeds dailyLimit', async function () { await expect( - adapter.connect(user).send(makeSendParam(amount), messagingFee, user.address, { value: 0 }) - ).to.be.revertedWithCustomError(adapter, 'BRIDGE_LIMITS').withArgs('dailyLimit'); + adapter.setBridgeLimits({ + dailyLimit: ethers.utils.parseEther('1000'), + txLimit: ethers.utils.parseEther('10000'), + accountDailyLimit: ethers.utils.parseEther('1000'), + minAmount: ethers.utils.parseEther('100'), + onlyWhitelisted: false, + }) + ).to.be.revertedWith('txLimit > dailyLimit'); }); }); @@ -339,20 +334,20 @@ describe('GoodDollarOFTAdapter Fork Tests (Celo Production)', () => { }); it('Should enforce maxFee when calculated fee exceeds maxFee', async function () { - // Set fee to 10% with maxFee of 50 tokens + // Set fee to 5% with maxFee of 20 tokens await adapter.setBridgeFees({ minFee: ethers.utils.parseEther('0'), - maxFee: ethers.utils.parseEther('50'), - fee: 1000, // 10% + maxFee: ethers.utils.parseEther('20'), + fee: 500, // 5% }); - // For 1000 tokens: 1000 * 1000 / 10000 = 100 tokens (above maxFee of 50) - // So fee should be 50 tokens (maxFee) + // For 1000 tokens: 1000 * 500 / 10000 = 50 tokens (above maxFee of 20) + // So fee should be 20 tokens (maxFee) const amount = ethers.utils.parseEther('1000'); - const calculatedFee = amount.mul(1000).div(10000); - const maxFee = ethers.utils.parseEther('50'); + const calculatedFee = amount.mul(500).div(10000); + const maxFee = ethers.utils.parseEther('20'); - // The actual fee should be maxFee (50) since calculated (100) > maxFee (50) + // The actual fee should be maxFee (20) since calculated (50) > maxFee (20) expect(calculatedFee.gt(maxFee)).to.be.true; }); @@ -368,16 +363,70 @@ describe('GoodDollarOFTAdapter Fork Tests (Celo Production)', () => { expect(expectedFee).to.equal(ethers.utils.parseEther('0')); }); - it('Should handle 100% fee (10000 basis points)', async function () { - await adapter.setBridgeFees({ - minFee: ethers.utils.parseEther('0'), - maxFee: ethers.utils.parseEther('0'), - fee: 10000, // 100% + it('Should reject fees above MAX_FEE_BPS', async function () { + const maxFeeBps = await adapter.MAX_FEE_BPS(); + await expect(adapter.setBridgeFees({ minFee: 0, maxFee: 0, fee: 10000 })).to.be.revertedWith('invalid fee'); + await expect(adapter.setBridgeFees({ minFee: 0, maxFee: 0, fee: maxFeeBps.add(1) })).to.be.revertedWith( + 'invalid fee' + ); + await expect(adapter.setBridgeFees({ minFee: 0, maxFee: 0, fee: maxFeeBps })).to.emit(adapter, 'BridgeFeesSet'); + }); + }); + + // L-01/L-02: the adapter is the spender and burns through the MinterBurner, with real G$ and the real LZ endpoint + describe('Send path with real SuperGoodDollar (L-01/L-02)', () => { + // Celo->XDC defaults use a dead DVN until configured; Ethereum has live defaults so a raw send succeeds + const DST_EID = 30101; + + it('pulls approved tokens via the adapter and burns them; the MinterBurner allowance is not used', async function () { + const { adapter, minterBurner, goodDollarAddress } = await loadFixture(setupFixture); + const token = await ethers.getContractAt( + 'contracts/oft/interfaces/ISuperGoodDollar.sol:ISuperGoodDollar', + goodDollarAddress, + ); + const erc20 = await ethers.getContractAt('@openzeppelin/contracts/token/ERC20/IERC20.sol:IERC20', goodDollarAddress); + + await networkHelpers.impersonateAccount(celoContracts.Avatar); + await networkHelpers.setBalance(celoContracts.Avatar, ethers.utils.parseEther('10')); + const avatarSigner = await ethers.getSigner(celoContracts.Avatar); + await token.connect(avatarSigner).addMinter(minterBurner.address); + const amount = ethers.utils.parseEther('100'); + await minterBurner.connect(avatarSigner).mint(user.address, amount); + await networkHelpers.stopImpersonatingAccount(celoContracts.Avatar); + + await adapter.setPeer(DST_EID, ethers.utils.hexZeroPad(adapter.address, 32)); + await adapter.setBridgeLimits({ + dailyLimit: ethers.utils.parseEther('1000'), + txLimit: ethers.utils.parseEther('1000'), + accountDailyLimit: ethers.utils.parseEther('1000'), + minAmount: ethers.utils.parseEther('1'), + onlyWhitelisted: false, }); - const amount = ethers.utils.parseEther('1000'); - const expectedFee = amount.mul(10000).div(10000); - expect(expectedFee).to.equal(amount); + const sendParam = { + dstEid: DST_EID, + to: ethers.utils.hexZeroPad(user.address, 32), + amountLD: amount, + minAmountLD: amount, + extraOptions: '0x00030100110100000000000000000000000000030d40', // type 3, lzReceive 200k gas + composeMsg: '0x', + oftCmd: '0x', + }; + const fee = await adapter.quoteSend(sendParam, false); + + // approving only the MinterBurner (old flow) is not enough anymore + await erc20.connect(user).approve(minterBurner.address, amount); + await expect(adapter.connect(user).send(sendParam, fee, user.address, { value: fee.nativeFee })).to.be.reverted; + + await erc20.connect(user).approve(adapter.address, amount); + const supplyBefore = await erc20.totalSupply(); + await adapter.connect(user).send(sendParam, fee, user.address, { value: fee.nativeFee }); + + expect(await erc20.balanceOf(user.address)).to.equal(0); + expect(await erc20.totalSupply()).to.equal(supplyBefore.sub(amount)); + expect(await erc20.balanceOf(adapter.address)).to.equal(0); + expect(await erc20.balanceOf(minterBurner.address)).to.equal(0); + expect(await erc20.allowance(user.address, minterBurner.address)).to.equal(amount); }); }); diff --git a/packages/bridge-contracts/test/GoodDollarOFT.reentrancy.test.ts b/packages/bridge-contracts/test/GoodDollarOFT.reentrancy.test.ts index ffc14cf..104a7a4 100644 --- a/packages/bridge-contracts/test/GoodDollarOFT.reentrancy.test.ts +++ b/packages/bridge-contracts/test/GoodDollarOFT.reentrancy.test.ts @@ -2,6 +2,7 @@ import { ethers, upgrades } from 'hardhat'; import { expect } from 'chai'; import * as networkHelpers from '@nomicfoundation/hardhat-network-helpers'; import contracts from '@gooddollar/goodprotocol/releases/deployment.json'; +import { deployMinterBurnerProxy } from './helpers/deployMinterBurnerProxy'; // Regression for C-01: G$ (SuperGoodDollar) calls the recipient's ERC777 tokensReceived hook on mint, // so approveFailedRequest must not be re-enterable from inside _credit. @@ -17,7 +18,7 @@ function encodeOFTMessage(toAddress: string, amountLD: ethers.BigNumber): string async function deployHarness(tokenAddress: string, nameServiceAddress: string, endpointAddress: string) { const [owner, feeRecipient] = await ethers.getSigners(); - const minterBurner = await (await ethers.getContractFactory('GoodDollarOFTMinterBurner')).deploy(); + const minterBurner = await deployMinterBurnerProxy(); const harness = await upgrades.deployProxy( await ethers.getContractFactory('GoodDollarOFTAdapterHarness'), [tokenAddress, minterBurner.address, owner.address, feeRecipient.address], diff --git a/packages/bridge-contracts/test/GoodDollarOFT.test.ts b/packages/bridge-contracts/test/GoodDollarOFT.test.ts index 61a36d2..7772d4a 100644 --- a/packages/bridge-contracts/test/GoodDollarOFT.test.ts +++ b/packages/bridge-contracts/test/GoodDollarOFT.test.ts @@ -1,6 +1,7 @@ import { ethers, upgrades } from "hardhat"; import { expect } from "chai"; import { loadFixture, time } from "@nomicfoundation/hardhat-network-helpers"; +import { deployMinterBurnerProxy } from "./helpers/deployMinterBurnerProxy"; // OFT message encoding helpers (mirrors OFTMsgCodec library) // message = abi.encodePacked(bytes32(to), uint64(amountSD)) @@ -43,10 +44,8 @@ describe("OFT (unit, no fork)", () => { const endpoint = await Endpoint.deploy(); await endpoint.deployed(); - // Deploy minter/burner (do not initialize yet; adapter address is required) - const MinterBurner = await ethers.getContractFactory("GoodDollarOFTMinterBurner"); - const minterBurner = await MinterBurner.deploy(); - await minterBurner.deployed(); + // Deploy minter/burner proxy (do not initialize yet; adapter address is required) + const minterBurner = await deployMinterBurnerProxy(); // Deploy adapter via UUPS proxy (implementation disables initializers) const Adapter = await ethers.getContractFactory("GoodDollarOFTAdapter"); @@ -99,21 +98,38 @@ describe("OFT (unit, no fork)", () => { const amount = ethers.utils.parseEther("10"); - await expect(minterBurner.connect(owner).mint(user.address, amount)).to.emit( + await expect(minterBurner.connect(owner).mint(owner.address, amount)).to.emit( minterBurner, "TokensMinted" ); - await expect(minterBurner.connect(owner).burn(user.address, amount)).to.be.revertedWith( + await expect(minterBurner.connect(owner).burn(owner.address, amount)).to.be.revertedWith( "ERC20: insufficient allowance" ); - await token.connect(user).approve(minterBurner.address, amount); + await token.connect(owner).approve(minterBurner.address, amount); - await expect(minterBurner.connect(owner).burn(user.address, amount)).to.emit( + const supplyBefore = await token.totalSupply(); + await expect(minterBurner.connect(owner).burn(owner.address, amount)).to.emit( minterBurner, "TokensBurned" ); + expect(await token.totalSupply()).to.equal(supplyBefore.sub(amount)); + }); + + // L-01: an operator can no longer burn tokens of an arbitrary approver + it("operator cannot burn another account's tokens even with allowance", async () => { + const { minterBurner, token, user, avatar, owner } = await loadFixture(fixture); + + await minterBurner.connect(avatar).setOperator(owner.address, true); + const amount = ethers.utils.parseEther("10"); + await minterBurner.connect(owner).mint(user.address, amount); + await token.connect(user).approve(minterBurner.address, amount); + + await expect(minterBurner.connect(owner).burn(user.address, amount)).to.be.revertedWith( + "can only burn own tokens" + ); + expect(await token.balanceOf(user.address)).to.equal(amount); }); it("pause blocks operator mint/burn", async () => { @@ -126,11 +142,34 @@ describe("OFT (unit, no fork)", () => { "Contract is paused" ); - await token.connect(user).approve(minterBurner.address, 1); - await expect(minterBurner.connect(owner).burn(user.address, 1)).to.be.revertedWith( + await token.connect(owner).approve(minterBurner.address, 1); + await expect(minterBurner.connect(owner).burn(owner.address, 1)).to.be.revertedWith( "Contract is paused" ); }); + + // L-03: authorization follows the live Controller avatar + it("onlyOperators uses the current Controller avatar", async () => { + const { minterBurner, controller, user, avatar } = await loadFixture(fixture); + const [, , , , newAvatar] = await ethers.getSigners(); + + await controller.setAvatar(newAvatar.address); + + await expect(minterBurner.connect(avatar).mint(user.address, 1)).to.be.revertedWith("Not authorized"); + await expect(minterBurner.connect(newAvatar).mint(user.address, 1)).to.emit(minterBurner, "TokensMinted"); + }); + + // L-04: the implementation cannot be initialized directly + it("implementation has initializers disabled", async () => { + const { nameService, adapter } = await loadFixture(fixture); + const MinterBurner = await ethers.getContractFactory("GoodDollarOFTMinterBurner"); + const impl = await MinterBurner.deploy(); + await impl.deployed(); + + await expect(impl.initialize(nameService.address, adapter.address)).to.be.revertedWith( + "Initializable: contract is already initialized" + ); + }); }); describe("GoodDollarOFTAdapter basic config", () => { @@ -140,7 +179,102 @@ describe("OFT (unit, no fork)", () => { expect(await adapter.minterBurner()).to.equal(minterBurner.address); expect(await adapter.owner()).to.equal(owner.address); expect(await adapter.feeRecipient()).to.equal(feeRecipient.address); - expect(await adapter.approvalRequired()).to.equal(false); + expect(await adapter.approvalRequired()).to.equal(true); + }); + + // M-01: fee configuration bounds + it("rejects fee above MAX_FEE_BPS and minFee above maxFee", async () => { + const { adapter } = await loadFixture(fixture); + const maxBps = await adapter.MAX_FEE_BPS(); + + await expect(adapter.setBridgeFees({ minFee: 0, maxFee: 0, fee: maxBps.add(1) })).to.be.revertedWith( + "invalid fee" + ); + await expect(adapter.setBridgeFees({ minFee: 0, maxFee: 0, fee: 10000 })).to.be.revertedWith("invalid fee"); + await expect(adapter.setBridgeFees({ minFee: 10, maxFee: 5, fee: 100 })).to.be.revertedWith("minFee > maxFee"); + + await expect(adapter.setBridgeFees({ minFee: 10, maxFee: 0, fee: maxBps })).to.emit(adapter, "BridgeFeesSet"); + await expect(adapter.setBridgeFees({ minFee: 5, maxFee: 5, fee: 100 })).to.emit(adapter, "BridgeFeesSet"); + }); + + // L-07: limits must be internally consistent + it("rejects inconsistent bridge limits", async () => { + const { adapter } = await loadFixture(fixture); + const base = { + dailyLimit: ethers.utils.parseEther("1000"), + txLimit: ethers.utils.parseEther("100"), + accountDailyLimit: ethers.utils.parseEther("500"), + minAmount: ethers.utils.parseEther("1"), + onlyWhitelisted: false, + }; + + await expect(adapter.setBridgeLimits({ ...base, minAmount: ethers.utils.parseEther("101") })).to.be.revertedWith( + "minAmount > txLimit" + ); + await expect(adapter.setBridgeLimits({ ...base, txLimit: ethers.utils.parseEther("1001") })).to.be.revertedWith( + "txLimit > dailyLimit" + ); + await expect( + adapter.setBridgeLimits({ ...base, accountDailyLimit: ethers.utils.parseEther("1001") }) + ).to.be.revertedWith("accountDailyLimit > dailyLimit"); + await expect(adapter.setBridgeLimits(base)).to.emit(adapter, "BridgeLimitsSet"); + }); + + // L-06: setNameService emits an event + it("emits NameServiceSet", async () => { + const { adapter, nameService } = await loadFixture(fixture); + await expect(adapter.setNameService(nameService.address)) + .to.emit(adapter, "NameServiceSet") + .withArgs(nameService.address); + }); + + // L-09: quoteOFT reports the real limits for msg.sender + it("quoteOFT returns min/max from bridge limits and dust-free receipt", async () => { + const { adapter, user } = await loadFixture(fixture); + const sendParam = (amountLD: any) => ({ + dstEid: 1, + to: ethers.utils.hexZeroPad(user.address, 32), + amountLD, + minAmountLD: 0, + extraOptions: "0x", + composeMsg: "0x", + oftCmd: "0x", + }); + + let [limit] = await adapter.connect(user).quoteOFT(sendParam(ethers.utils.parseEther("10"))); + expect(limit.minAmountLD).to.equal(0); + expect(limit.maxAmountLD).to.equal(0); + + await adapter.setBridgeLimits({ + dailyLimit: ethers.utils.parseEther("1000"), + txLimit: ethers.utils.parseEther("100"), + accountDailyLimit: ethers.utils.parseEther("50"), + minAmount: ethers.utils.parseEther("1"), + onlyWhitelisted: false, + }); + + const dust = DECIMAL_CONVERSION_RATE.sub(1); + const amount = ethers.utils.parseEther("10").add(dust); + const [limit2, feeDetails, receipt] = await adapter.connect(user).quoteOFT(sendParam(amount)); + expect(limit2.minAmountLD).to.equal(ethers.utils.parseEther("1")); + expect(limit2.maxAmountLD).to.equal(ethers.utils.parseEther("50")); + expect(feeDetails.length).to.equal(0); + expect(receipt.amountSentLD).to.equal(ethers.utils.parseEther("10")); + expect(receipt.amountReceivedLD).to.equal(ethers.utils.parseEther("10")); + + await adapter.setBridgeLimits({ + dailyLimit: ethers.utils.parseEther("1000"), + txLimit: ethers.utils.parseEther("100"), + accountDailyLimit: ethers.utils.parseEther("1000"), + minAmount: ethers.utils.parseEther("1"), + onlyWhitelisted: false, + }); + [limit] = await adapter.connect(user).quoteOFT(sendParam(amount)); + expect(limit.maxAmountLD).to.equal(ethers.utils.parseEther("100")); + + await adapter.pauseBridge(true); + [limit] = await adapter.connect(user).quoteOFT(sendParam(amount)); + expect(limit.maxAmountLD).to.equal(0); }); it("sets bridge fees / feeRecipient / limits / pause", async () => { @@ -240,7 +374,7 @@ describe("OFT (unit, no fork)", () => { oftCmd: "0x", }); - it("reverts with minAmount / txLimit / accountDailyLimit / dailyLimit", async () => { + it("reverts with minAmount / txLimit / accountDailyLimit", async () => { const { adapter, user } = await loadFixture(fixture); await adapter.setBridgeLimits({ @@ -268,20 +402,61 @@ describe("OFT (unit, no fork)", () => { .connect(user) .send(makeSendParam(user.address, ethers.utils.parseEther("201")), messagingFee, user.address, { value: 0 }) ).to.be.revertedWithCustomError(adapter, "BRIDGE_LIMITS").withArgs("accountDailyLimit"); + }); + + // L-08: limits are checked on the dust-free amount + it("does not count dust against txLimit", async () => { + const { adapter, user } = await loadFixture(fixture); - // dailyLimit check on new window: amount > dailyLimit (raise other limits so dailyLimit is the first failure) await adapter.setBridgeLimits({ dailyLimit: ethers.utils.parseEther("1000"), - txLimit: ethers.utils.parseEther("10000"), - accountDailyLimit: ethers.utils.parseEther("10000"), - minAmount: ethers.utils.parseEther("10"), + txLimit: ethers.utils.parseEther("100"), + accountDailyLimit: ethers.utils.parseEther("1000"), + minAmount: ethers.utils.parseEther("1"), onlyWhitelisted: false, }); + + const amount = ethers.utils.parseEther("100").add(DECIMAL_CONVERSION_RATE.sub(1)); + await expect( + adapter + .connect(user) + .send({ ...makeSendParam(user.address, amount), minAmountLD: 0 }, messagingFee, user.address, { value: 0 }) + ).to.not.be.revertedWithCustomError(adapter, "BRIDGE_LIMITS"); + await expect( adapter .connect(user) - .send(makeSendParam(user.address, ethers.utils.parseEther("1001")), messagingFee, user.address, { value: 0 }) - ).to.be.revertedWithCustomError(adapter, "BRIDGE_LIMITS").withArgs("dailyLimit"); + .send( + { ...makeSendParam(user.address, amount.add(DECIMAL_CONVERSION_RATE)), minAmountLD: 0 }, + messagingFee, + user.address, + { value: 0 } + ) + ).to.be.revertedWithCustomError(adapter, "BRIDGE_LIMITS").withArgs("txLimit"); + }); + + // L-06: whitelist mode fails closed when Identity cannot be resolved + it("reverts with 'identity not set' when onlyWhitelisted and no NameService/IDENTITY", async () => { + const { adapter, user, nameService } = await loadFixture(fixture); + + await adapter.setBridgeLimits({ + dailyLimit: ethers.utils.parseEther("1000"), + txLimit: ethers.utils.parseEther("1000"), + accountDailyLimit: ethers.utils.parseEther("1000"), + minAmount: ethers.utils.parseEther("1"), + onlyWhitelisted: true, + }); + + const send = () => + adapter + .connect(user) + .send(makeSendParam(user.address, ethers.utils.parseEther("10")), messagingFee, user.address, { value: 0 }); + + await expect(send()).to.be.revertedWithCustomError(adapter, "BRIDGE_LIMITS").withArgs("identity not set"); + + // NameService set but IDENTITY missing + await adapter.setNameService(nameService.address); + await expect(send()).to.be.revertedWithCustomError(adapter, "BRIDGE_LIMITS").withArgs("identity not set"); }); it("bridging failed when bridge limits exceeded on send", async () => { @@ -432,9 +607,7 @@ describe("OFT (unit, no fork)", () => { const endpoint = await Endpoint.deploy(); await endpoint.deployed(); - const MinterBurner = await ethers.getContractFactory("GoodDollarOFTMinterBurner"); - const minterBurner = await MinterBurner.deploy(); - await minterBurner.deployed(); + const minterBurner = await deployMinterBurnerProxy(); // Deploy harness via UUPS proxy const Harness = await ethers.getContractFactory("GoodDollarOFTAdapterHarness"); @@ -692,6 +865,89 @@ describe("OFT (unit, no fork)", () => { .withArgs(guid, 1, user.address, netAmount); expect(await token.balanceOf(user.address)).to.equal(netAmount); }); + + // M-02: composed messages must not be parked (the compose payload would be lost) + it("reverts composed messages that exceed limits instead of parking them", async () => { + const { harness, user } = await loadFixture(harnessFixture); + await harness.setBridgeLimits({ + dailyLimit: ethers.utils.parseEther("1000"), + txLimit: ethers.utils.parseEther("50"), + accountDailyLimit: ethers.utils.parseEther("1000"), + minAmount: ethers.utils.parseEther("1"), + onlyWhitelisted: false, + }); + + const guid = ethers.utils.formatBytes32String("guid-composed"); + const composeFrom = ethers.utils.hexZeroPad(user.address, 32); + const message = + encodeOFTMessage(user.address, toSD(ethers.utils.parseEther("100"))) + composeFrom.slice(2) + "deadbeef"; + + await expect(harness.exposed_lzReceive(origin(), guid, message, ethers.constants.AddressZero, "0x")) + .to.be.revertedWithCustomError(harness, "BRIDGE_LIMITS") + .withArgs("txLimit"); + expect((await harness.failedReceiveRequests(guid)).failed).to.equal(false); + }); + + // M-03: inbound and outbound traffic use separate counters + it("receive updates only the inbound counters", async () => { + const { harness, user } = await loadFixture(harnessFixture); + await harness.setBridgeLimits({ + dailyLimit: ethers.utils.parseEther("1000"), + txLimit: ethers.utils.parseEther("1000"), + accountDailyLimit: ethers.utils.parseEther("1000"), + minAmount: ethers.utils.parseEther("1"), + onlyWhitelisted: false, + }); + + const amountLD = ethers.utils.parseEther("100"); + const guid = ethers.utils.formatBytes32String("guid-counters"); + await harness.exposed_lzReceive( + origin(), + guid, + encodeOFTMessage(user.address, toSD(amountLD)), + ethers.constants.AddressZero, + "0x" + ); + + expect((await harness.bridgeReceiveDailyLimit()).bridged24Hours).to.equal(amountLD); + expect((await harness.accountsReceiveDailyLimit(user.address)).bridged24Hours).to.equal(amountLD); + expect((await harness.bridgeDailyLimit()).bridged24Hours).to.equal(0); + expect((await harness.accountsDailyLimit(user.address)).bridged24Hours).to.equal(0); + }); + + it("parks a receive that exceeds the inbound dailyLimit", async () => { + const { harness, user, owner } = await loadFixture(harnessFixture); + await harness.setBridgeLimits({ + dailyLimit: ethers.utils.parseEther("150"), + txLimit: ethers.utils.parseEther("100"), + accountDailyLimit: ethers.utils.parseEther("150"), + minAmount: ethers.utils.parseEther("1"), + onlyWhitelisted: false, + }); + + const amountLD = ethers.utils.parseEther("100"); + const guid1 = ethers.utils.formatBytes32String("guid-daily-1"); + const guid2 = ethers.utils.formatBytes32String("guid-daily-2"); + await harness.exposed_lzReceive( + origin(), + guid1, + encodeOFTMessage(user.address, toSD(amountLD)), + ethers.constants.AddressZero, + "0x" + ); + await expect( + harness.exposed_lzReceive( + origin(), + guid2, + encodeOFTMessage(owner.address, toSD(amountLD)), + ethers.constants.AddressZero, + "0x" + ) + ).to.emit(harness, "ReceiveRequestFailed"); + + expect((await harness.failedReceiveRequests(guid1)).failed).to.equal(false); + expect((await harness.failedReceiveRequests(guid2)).failed).to.equal(true); + }); }); // ───────────────────────────────────────────────────────────────────────── @@ -720,9 +976,7 @@ describe("OFT (unit, no fork)", () => { const endpoint = await Endpoint.deploy(); await endpoint.deployed(); - const MinterBurner = await ethers.getContractFactory("GoodDollarOFTMinterBurner"); - const minterBurner = await MinterBurner.deploy(); - await minterBurner.deployed(); + const minterBurner = await deployMinterBurnerProxy(); const Harness = await ethers.getContractFactory("GoodDollarOFTAdapterHarness"); const harness = await upgrades.deployProxy( @@ -737,7 +991,7 @@ describe("OFT (unit, no fork)", () => { await minterBurner.initialize(nameService.address, harness.address); - return { owner, user, feeRecipient, token, harness }; + return { owner, user, feeRecipient, token, harness, minterBurner }; } it("mints full amount to recipient when no fee is configured", async () => { @@ -807,18 +1061,97 @@ describe("OFT (unit, no fork)", () => { it("enforces maxFee when calculated fee exceeds maximum", async () => { const { harness, user, token, feeRecipient } = await loadFixture(feeHarnessFixture); - const maxFee = ethers.utils.parseEther("50"); + const maxFee = ethers.utils.parseEther("20"); await harness.setBridgeFees({ minFee: 0, maxFee, - fee: 1000, // 10% + fee: 500, // 5% }); - const amount = ethers.utils.parseEther("1000"); // calculated: 100 tokens, above maxFee of 50 + const amount = ethers.utils.parseEther("1000"); // calculated: 50 tokens, above maxFee of 20 await harness.exposed_credit(user.address, amount, 1); expect(await token.balanceOf(feeRecipient.address)).to.equal(maxFee); expect(await token.balanceOf(user.address)).to.equal(amount.sub(maxFee)); }); + + // M-01: minFee above the amount is clamped so the credit cannot underflow + it("clamps the fee to the amount when minFee exceeds it", async () => { + const { harness, user, token, feeRecipient } = await loadFixture(feeHarnessFixture); + + await harness.setBridgeFees({ minFee: ethers.utils.parseEther("5"), maxFee: 0, fee: 10 }); + + const amount = ethers.utils.parseEther("2"); + expect(await harness.callStatic.exposed_credit(user.address, amount, 1)).to.equal(0); + await harness.exposed_credit(user.address, amount, 1); + + expect(await token.balanceOf(user.address)).to.equal(0); + expect(await token.balanceOf(feeRecipient.address)).to.equal(amount); + }); + }); + + // ───────────────────────────────────────────────────────────────────────── + // L-01/L-02: the adapter is the spender and burns through the MinterBurner + // ───────────────────────────────────────────────────────────────────────── + describe("Debit (_debit)", () => { + async function debitFixture() { + const [owner, user, feeRecipient, avatar] = await ethers.getSigners(); + + const NameService = await ethers.getContractFactory("NameServiceMock"); + const nameService = await NameService.deploy(); + const Controller = await ethers.getContractFactory("ControllerMock"); + const controller = await Controller.deploy(avatar.address); + const Token = await ethers.getContractFactory("MockGoodDollar"); + const token = await Token.deploy("GoodDollar", "G$"); + await nameService.setAddress("CONTROLLER", controller.address); + await nameService.setAddress("GOODDOLLAR", token.address); + const Endpoint = await ethers.getContractFactory("LayerZeroEndpointMock"); + const endpoint = await Endpoint.deploy(); + + const minterBurner = await deployMinterBurnerProxy(); + const Harness = await ethers.getContractFactory("GoodDollarOFTAdapterHarness"); + const harness = await upgrades.deployProxy( + Harness, + [token.address, minterBurner.address, owner.address, feeRecipient.address], + { + kind: "uups", + constructorArgs: [token.address, endpoint.address], + unsafeAllow: ["constructor", "state-variable-immutable", "duplicate-initializer-call", "missing-initializer"], + } + ); + await minterBurner.initialize(nameService.address, harness.address); + + return { user, token, harness, minterBurner }; + } + + it("pulls approved tokens into the adapter and burns them", async () => { + const { user, token, harness, minterBurner } = await loadFixture(debitFixture); + const amount = ethers.utils.parseEther("10"); + await token.mint(user.address, amount); + + await token.connect(user).approve(harness.address, amount); + const supplyBefore = await token.totalSupply(); + await expect(harness.exposed_debit(user.address, amount, amount, 1)) + .to.emit(minterBurner, "TokensBurned") + .withArgs(harness.address, amount, harness.address); + + expect(await token.balanceOf(user.address)).to.equal(0); + expect(await token.balanceOf(harness.address)).to.equal(0); + expect(await token.balanceOf(minterBurner.address)).to.equal(0); + expect(await token.totalSupply()).to.equal(supplyBefore.sub(amount)); + expect(await token.allowance(harness.address, minterBurner.address)).to.equal(0); + }); + + it("reverts without an allowance to the adapter (MinterBurner allowance is not enough)", async () => { + const { user, token, harness, minterBurner } = await loadFixture(debitFixture); + const amount = ethers.utils.parseEther("10"); + await token.mint(user.address, amount); + + await token.connect(user).approve(minterBurner.address, amount); + await expect(harness.exposed_debit(user.address, amount, amount, 1)).to.be.revertedWith( + "ERC20: insufficient allowance" + ); + expect(await token.balanceOf(user.address)).to.equal(amount); + }); }); }); diff --git a/packages/bridge-contracts/test/helpers/deployMinterBurnerProxy.ts b/packages/bridge-contracts/test/helpers/deployMinterBurnerProxy.ts new file mode 100644 index 0000000..758a1c4 --- /dev/null +++ b/packages/bridge-contracts/test/helpers/deployMinterBurnerProxy.ts @@ -0,0 +1,11 @@ +import { ethers } from 'hardhat'; + +// The MinterBurner implementation disables initializers, so tests put it behind a proxy and initialize it later +export async function deployMinterBurnerProxy() { + const impl = await (await ethers.getContractFactory('GoodDollarOFTMinterBurner')).deploy(); + const proxy = await ( + await ethers.getContractFactory('@gooddollar/goodprotocol/contracts/utils/ProxyFactory1967.sol:ERC1967Proxy') + ).deploy(); + await proxy.initialize(impl.address, '0x'); + return ethers.getContractAt('GoodDollarOFTMinterBurner', proxy.address); +}