From 9b934277e8f4ba86bae4994b67ad38c9b4812c22 Mon Sep 17 00:00:00 2001 From: Benjamin Patch Date: Fri, 3 Nov 2023 15:44:31 +1100 Subject: [PATCH 1/3] Add IMX deposit limit --- README.md | 4 + script/DeployRootContracts.s.sol | 6 +- script/InitializeRootContracts.s.sol | 7 +- src/interfaces/root/IRootERC20Bridge.sol | 8 ++ src/root/RootERC20Bridge.sol | 36 ++++++- test/integration/root/RootERC20Bridge.t.sol | 6 +- test/unit/root/RootERC20Bridge.t.sol | 105 ++++++++++++++++++-- test/utils.t.sol | 6 +- 8 files changed, 159 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index b8d43b610..e7054c68b 100644 --- a/README.md +++ b/README.md @@ -83,9 +83,12 @@ CHILD_GAS_SERVICE_ADDRESS= ROOT_CHAIN_NAME="ROOT" CHILD_CHAIN_NAME="CHILD" ROOT_IMX_ADDRESS= +INITIAL_IMX_CUMULATIVE_DEPOSIT_LIMIT="0" ``` where `{ROOT,CHILD}_{GATEWAY,GAS_SERVICE}_ADDRESS` refers to the gateway and gas service addresses used by Axelar. +`INITIAL_IMX_CUMULATIVE_DEPOSIT_LIMIT` refers to the cumulative amount of IMX that can be deposited. A value of `0` indicated unlimited. + We can just use dummy gateway/gas service addresses if we only want to test the deployment, and not bridging functionality. If wanting to use dummy addresses, any valid Ethereum address can be used here. 4. Run the deploy script. @@ -138,6 +141,7 @@ ROOT_GATEWAY_ADDRESS="0x013459EC3E8Aeced878C5C4bFfe126A366cd19E9" CHILD_GATEWAY_ADDRESS="0xc7B788E88BAaB770A6d4936cdcCcd5250E1bbAd8" ROOT_GAS_SERVICE_ADDRESS="0x28f8B50E1Be6152da35e923602a2641491E71Ed8" CHILD_GAS_SERVICE_ADDRESS="0xC573c722e21eD7fadD38A8f189818433e01Ae466" +INITIAL_IMX_CUMULATIVE_DEPOSIT_LIMIT="0" ENVIRONMENT="local" ``` (Note that `{ROOT,CHILD}_PRIVATE_KEY` can be any of the standard localhost private keys that get funded) diff --git a/script/DeployRootContracts.s.sol b/script/DeployRootContracts.s.sol index a1f49fceb..fee8e860c 100644 --- a/script/DeployRootContracts.s.sol +++ b/script/DeployRootContracts.s.sol @@ -35,15 +35,15 @@ contract DeployRootContracts is Script { rootChainChildTokenTemplate.initialize(address(123), "TEMPLATE", "TPT", 18); RootERC20Bridge rootERC20BridgeImplementation = new RootERC20Bridge(); - rootERC20BridgeImplementation.initialize(address(1), address(1), "filler", address(1), address(1), address(1)); + rootERC20BridgeImplementation.initialize( + address(1), address(1), "filler", address(1), address(1), address(1), 1 + ); TransparentUpgradeableProxy rootERC20BridgeProxy = new TransparentUpgradeableProxy( address(rootERC20BridgeImplementation), address(proxyAdmin), "" ); - // TODO add dummy initialize of implementation contracts! - RootAxelarBridgeAdaptor rootBridgeAdaptorImplementation = new RootAxelarBridgeAdaptor(); rootBridgeAdaptorImplementation.initialize( address(rootERC20BridgeImplementation), "Filler", address(1), address(1) diff --git a/script/InitializeRootContracts.s.sol b/script/InitializeRootContracts.s.sol index c9a25c2c3..8ea85a593 100644 --- a/script/InitializeRootContracts.s.sol +++ b/script/InitializeRootContracts.s.sol @@ -26,6 +26,7 @@ struct InitializeRootContractsParams { string childChainName; address rootGateway; address rootGasService; + uint256 initialIMXCumulativeDepositLimit; } contract InitializeRootContracts is Script { @@ -42,7 +43,8 @@ contract InitializeRootContracts is Script { rootWETHToken: vm.envAddress("ROOT_WETH_ADDRESS"), childChainName: vm.envString("CHILD_CHAIN_NAME"), rootGateway: vm.envAddress("ROOT_GATEWAY_ADDRESS"), - rootGasService: vm.envAddress("ROOT_GAS_SERVICE_ADDRESS") + rootGasService: vm.envAddress("ROOT_GAS_SERVICE_ADDRESS"), + initialIMXCumulativeDepositLimit: vm.envUint("INITIAL_IMX_CUMULATIVE_DEPOSIT_LIMIT") }); string[] memory checksumInputs = Utils.getChecksumInputs(params.childBridgeAdaptor); @@ -60,7 +62,8 @@ contract InitializeRootContracts is Script { childBridgeAdaptorChecksum, params.rootChainChildTokenTemplate, params.rootIMXToken, - params.rootWETHToken + params.rootWETHToken, + params.initialIMXCumulativeDepositLimit ); params.rootBridgeAdaptor.initialize( diff --git a/src/interfaces/root/IRootERC20Bridge.sol b/src/interfaces/root/IRootERC20Bridge.sol index 75fa06c46..83837221b 100644 --- a/src/interfaces/root/IRootERC20Bridge.sol +++ b/src/interfaces/root/IRootERC20Bridge.sol @@ -35,6 +35,10 @@ interface IRootERC20Bridge { } interface IRootERC20BridgeEvents { + /// @notice Emitted when the child chain bridge adaptor is updated. + event NewRootBridgeAdaptor(address oldRootBridgeAdaptor, address newRootBridgeAdaptor); + /// @notice Emitted when the IMX deposit limit is updated. + event NewImxDepositLimit(uint256 oldImxDepositLimit, uint256 newImxDepositLimit); /// @notice Emitted when a map token message is sent to the child chain. event L1TokenMapped(address indexed rootToken, address indexed childToken); /// @notice Emitted when an ERC20 deposit message is sent to the child chain. @@ -83,4 +87,8 @@ interface IRootERC20BridgeErrors { error BalanceInvariantCheckFailed(uint256 actualBalance, uint256 expectedBalance); /// @notice Error when the given child chain bridge adaptor is invalid. error InvalidChildERC20BridgeAdaptor(); + /// @notice Error when the total IMX deposit limit is exceeded + error ImxDepositLimitExceeded(); + /// @notice Error when the IMX deposit limit is set below the amount of IMX already deposited + error ImxDepositLimitTooLow(); } diff --git a/src/root/RootERC20Bridge.sol b/src/root/RootERC20Bridge.sol index 23ae28359..6611ecf7b 100644 --- a/src/root/RootERC20Bridge.sol +++ b/src/root/RootERC20Bridge.sol @@ -52,6 +52,9 @@ contract RootERC20Bridge is address public childETHToken; /// @dev The address of the wETH ERC20 token on L1. address public rootWETHToken; + /// @dev The maximum cumulative amount of IMX that can be deposited into the bridge. + /// @dev A limit of zero indicates unlimited. + uint256 public imxCumulativeDepositLimit; /** * @notice Initilization function for RootERC20Bridge. @@ -61,6 +64,7 @@ contract RootERC20Bridge is * @param newChildTokenTemplate Address of child token template to clone. * @param newRootIMXToken Address of ERC20 IMX on the root chain. * @param newRootWETHToken Address of ERC20 WETH on the root chain. + * @param newImxCumulativeDepositLimit The cumulative IMX deposit limit. * @dev Can only be called once. */ function initialize( @@ -69,7 +73,8 @@ contract RootERC20Bridge is string memory newChildBridgeAdaptor, address newChildTokenTemplate, address newRootIMXToken, - address newRootWETHToken + address newRootWETHToken, + uint256 newImxCumulativeDepositLimit ) public initializer { if ( newRootBridgeAdaptor == address(0) || newChildERC20Bridge == address(0) @@ -89,15 +94,38 @@ contract RootERC20Bridge is ); rootBridgeAdaptor = IRootERC20BridgeAdaptor(newRootBridgeAdaptor); childBridgeAdaptor = newChildBridgeAdaptor; + imxCumulativeDepositLimit = newImxCumulativeDepositLimit; } + /** + * @notice Updates the root bridge adaptor. + * @param newRootBridgeAdaptor Address of new root bridge adaptor. + * @dev Can only be called by owner. + */ function updateRootBridgeAdaptor(address newRootBridgeAdaptor) external onlyOwner { if (newRootBridgeAdaptor == address(0)) { revert ZeroAddress(); } + emit NewRootBridgeAdaptor(address(rootBridgeAdaptor), newRootBridgeAdaptor); rootBridgeAdaptor = IRootERC20BridgeAdaptor(newRootBridgeAdaptor); } + // TODO add updating of child bridge adaptor. Part of SMR-1908 + + /** + * @notice Updates the IMX deposit limit. + * @param newImxCumulativeDepositLimit The new cumulative IMX deposit limit. + * @dev Can only be called by owner. + * @dev The limit can decrease, but it can never decrease to below the contract's IMX balance. + */ + function updateImxCumulativeDepositLimit(uint256 newImxCumulativeDepositLimit) external onlyOwner { + if (newImxCumulativeDepositLimit < IERC20Metadata(rootIMXToken).balanceOf(address(this))) { + revert ImxDepositLimitTooLow(); + } + emit NewImxDepositLimit(imxCumulativeDepositLimit, newImxCumulativeDepositLimit); + imxCumulativeDepositLimit = newImxCumulativeDepositLimit; + } + /** * @dev method to receive the ETH back from the WETH contract when it is unwrapped */ @@ -227,6 +255,12 @@ contract RootERC20Bridge is if (amount == 0) { revert ZeroAmount(); } + if ( + address(rootToken) == rootIMXToken && imxCumulativeDepositLimit != 0 + && IERC20Metadata(rootIMXToken).balanceOf(address(this)) + amount > imxCumulativeDepositLimit + ) { + revert ImxDepositLimitExceeded(); + } // ETH, WETH and IMX do not need to be mapped since it should have been mapped on initialization // ETH also cannot be transferred since it was received in the payable function call diff --git a/test/integration/root/RootERC20Bridge.t.sol b/test/integration/root/RootERC20Bridge.t.sol index 374ad9ff0..cb20d3954 100644 --- a/test/integration/root/RootERC20Bridge.t.sol +++ b/test/integration/root/RootERC20Bridge.t.sol @@ -20,6 +20,7 @@ contract RootERC20BridgeIntegrationTest is Test, IRootERC20BridgeEvents, IRootAx address constant IMX_TOKEN_ADDRESS = address(0xccc); address constant NATIVE_ETH = address(0xeee); address constant WRAPPED_ETH = address(0xddd); + uint256 constant unlimitedDepositLimit = 0; uint256 constant mapTokenFee = 300; uint256 constant depositFee = 200; @@ -34,8 +35,9 @@ contract RootERC20BridgeIntegrationTest is Test, IRootERC20BridgeEvents, IRootAx function setUp() public { deployCodeTo("WETH.sol", abi.encode("Wrapped ETH", "WETH"), WRAPPED_ETH); - (imxToken, token, rootBridge, axelarAdaptor, mockAxelarGateway, axelarGasService) = - integrationSetup(CHILD_BRIDGE, CHILD_BRIDGE_ADAPTOR, CHILD_CHAIN_NAME, IMX_TOKEN_ADDRESS, WRAPPED_ETH); + (imxToken, token, rootBridge, axelarAdaptor, mockAxelarGateway, axelarGasService) = integrationSetup( + CHILD_BRIDGE, CHILD_BRIDGE_ADAPTOR, CHILD_CHAIN_NAME, IMX_TOKEN_ADDRESS, WRAPPED_ETH, unlimitedDepositLimit + ); } /** diff --git a/test/unit/root/RootERC20Bridge.t.sol b/test/unit/root/RootERC20Bridge.t.sol index 901a8f0bf..b527ebff5 100644 --- a/test/unit/root/RootERC20Bridge.t.sol +++ b/test/unit/root/RootERC20Bridge.t.sol @@ -27,6 +27,7 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid address constant WRAPPED_ETH = address(0xddd); uint256 constant mapTokenFee = 300; uint256 constant depositFee = 200; + uint256 constant unlimitedIMXDeposits = 0; ERC20PresetMinterPauser public token; RootERC20Bridge public rootBridge; @@ -53,7 +54,8 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid CHILD_BRIDGE_ADAPTOR_STRING, address(token), IMX_TOKEN, - WRAPPED_ETH + WRAPPED_ETH, + unlimitedIMXDeposits ); } @@ -75,50 +77,112 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid CHILD_BRIDGE_ADAPTOR_STRING, address(token), IMX_TOKEN, - WRAPPED_ETH + WRAPPED_ETH, + unlimitedIMXDeposits ); } function test_RevertIf_InitializeWithAZeroAddressRootAdapter() public { RootERC20Bridge bridge = new RootERC20Bridge(); vm.expectRevert(ZeroAddress.selector); - bridge.initialize(address(0), address(1), CHILD_BRIDGE_ADAPTOR_STRING, address(1), address(1), address(1)); + bridge.initialize( + address(0), + address(1), + CHILD_BRIDGE_ADAPTOR_STRING, + address(1), + address(1), + address(1), + unlimitedIMXDeposits + ); } function test_RevertIf_InitializeWithAZeroAddressChildBridge() public { RootERC20Bridge bridge = new RootERC20Bridge(); vm.expectRevert(ZeroAddress.selector); - bridge.initialize(address(1), address(0), CHILD_BRIDGE_ADAPTOR_STRING, address(1), address(1), address(1)); + bridge.initialize( + address(1), + address(0), + CHILD_BRIDGE_ADAPTOR_STRING, + address(1), + address(1), + address(1), + unlimitedIMXDeposits + ); } function test_RevertIf_InitializeWithEmptyChildAdapter() public { RootERC20Bridge bridge = new RootERC20Bridge(); vm.expectRevert(InvalidChildERC20BridgeAdaptor.selector); - bridge.initialize(address(1), address(1), "", address(1), address(1), address(1)); + bridge.initialize(address(1), address(1), "", address(1), address(1), address(1), unlimitedIMXDeposits); } function test_RevertIf_InitializeWithAZeroAddressTokenTemplate() public { RootERC20Bridge bridge = new RootERC20Bridge(); vm.expectRevert(ZeroAddress.selector); - bridge.initialize(address(1), address(1), CHILD_BRIDGE_ADAPTOR_STRING, address(0), address(1), address(1)); + bridge.initialize( + address(1), + address(1), + CHILD_BRIDGE_ADAPTOR_STRING, + address(0), + address(1), + address(1), + unlimitedIMXDeposits + ); } function test_RevertIf_InitializeWithAZeroAddressIMXToken() public { RootERC20Bridge bridge = new RootERC20Bridge(); vm.expectRevert(ZeroAddress.selector); - bridge.initialize(address(1), address(1), CHILD_BRIDGE_ADAPTOR_STRING, address(1), address(0), address(1)); + bridge.initialize( + address(1), + address(1), + CHILD_BRIDGE_ADAPTOR_STRING, + address(1), + address(0), + address(1), + unlimitedIMXDeposits + ); } function test_RevertIf_InitializeWithAZeroAddressWETHToken() public { RootERC20Bridge bridge = new RootERC20Bridge(); vm.expectRevert(ZeroAddress.selector); - bridge.initialize(address(1), address(1), CHILD_BRIDGE_ADAPTOR_STRING, address(1), address(1), address(0)); + bridge.initialize( + address(1), + address(1), + CHILD_BRIDGE_ADAPTOR_STRING, + address(1), + address(1), + address(0), + unlimitedIMXDeposits + ); } function test_RevertIf_InitializeWithAZeroAddressAll() public { RootERC20Bridge bridge = new RootERC20Bridge(); vm.expectRevert(ZeroAddress.selector); - bridge.initialize(address(0), address(0), "", address(0), address(0), address(0)); + bridge.initialize(address(0), address(0), "", address(0), address(0), address(0), unlimitedIMXDeposits); + } + + /** + * UPDATE IMX CUMULATIVE DEPOSIT LIMIT + */ + function test_RevertsIf_IMXDepositLimitTooLow() public { + uint256 imxCumulativeDepositLimit = 700; + uint256 depositAmount = imxCumulativeDepositLimit + 1; + + rootBridge.updateImxCumulativeDepositLimit(imxCumulativeDepositLimit); + + setupDeposit(IMX_TOKEN, rootBridge, mapTokenFee, depositFee, depositAmount, false); + + IERC20Metadata(IMX_TOKEN).approve(address(rootBridge), type(uint256).max); + + rootBridge.updateImxCumulativeDepositLimit(depositAmount); + + rootBridge.deposit{value: depositFee}(IERC20Metadata(IMX_TOKEN), depositAmount); + + vm.expectRevert(ImxDepositLimitTooLow.selector); + rootBridge.updateImxCumulativeDepositLimit(imxCumulativeDepositLimit); } /** @@ -406,6 +470,29 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid * DEPOSIT TOKEN */ + function test_RevertsIf_IMXDepositLimitExceeded() public { + uint256 imxCumulativeDepositLimit = 700; + + uint256 amount = 300; + setupDeposit(IMX_TOKEN, rootBridge, mapTokenFee, depositFee, amount, false); + + IERC20Metadata(IMX_TOKEN).approve(address(rootBridge), type(uint256).max); + + rootBridge.updateImxCumulativeDepositLimit(imxCumulativeDepositLimit); + + // Valid + rootBridge.deposit{value: depositFee}(IERC20Metadata(IMX_TOKEN), amount); + + setupDeposit(IMX_TOKEN, rootBridge, mapTokenFee, depositFee, amount, false); + // Valid + rootBridge.deposit{value: depositFee}(IERC20Metadata(IMX_TOKEN), amount); + + setupDeposit(IMX_TOKEN, rootBridge, mapTokenFee, depositFee, amount, false); + // Invalid + vm.expectRevert(ImxDepositLimitExceeded.selector); + rootBridge.deposit{value: depositFee}(IERC20Metadata(IMX_TOKEN), amount); + } + function test_depositCallsSendMessage() public { uint256 amount = 100; (, bytes memory predictedPayload) = diff --git a/test/utils.t.sol b/test/utils.t.sol index 7151afd95..79ea01580 100644 --- a/test/utils.t.sol +++ b/test/utils.t.sol @@ -20,7 +20,8 @@ contract Utils is Test { address childBridgeAdaptor, string memory childBridgeName, address imxTokenAddress, - address wethTokenAddress + address wethTokenAddress, + uint256 imxCumulativeDepositLimit ) public returns ( @@ -55,7 +56,8 @@ contract Utils is Test { Strings.toHexString(childBridgeAdaptor), address(token), imxTokenAddress, - wethTokenAddress + wethTokenAddress, + imxCumulativeDepositLimit ); axelarAdaptor.initialize( From ff6b92de888a24ab7907fe8bbc33e83498b6a37e Mon Sep 17 00:00:00 2001 From: Benjamin Patch Date: Fri, 3 Nov 2023 15:50:59 +1100 Subject: [PATCH 2/3] Address PR comment --- .../ChildAxelarBridgeWithdraw.t.sol | 355 ------------------ 1 file changed, 355 deletions(-) diff --git a/test/integration/child/withdrawals/ChildAxelarBridgeWithdraw.t.sol b/test/integration/child/withdrawals/ChildAxelarBridgeWithdraw.t.sol index c2406f452..e52abd117 100644 --- a/test/integration/child/withdrawals/ChildAxelarBridgeWithdraw.t.sol +++ b/test/integration/child/withdrawals/ChildAxelarBridgeWithdraw.t.sol @@ -138,359 +138,4 @@ contract ChildERC20BridgeWithdrawIntegrationTest is vm.expectRevert(NoGas.selector); childBridge.withdraw(childToken, withdrawAmount); } - - // 7a8dc26796a1e50e6e190b70259f58f6a4edd5b22280ceecc82b687b8e982869 - // 000000000000000000000000000000000000000000000000000000000000ad9c - // 0000000000000000000000007fa9385be102ac3eac297483dd6233d62b3e1496 - // 0000000000000000000000007fa9385be102ac3eac297483dd6233d62b3e1496 - // 000000000000000000000000000000000000000000000000000000174876e7ff - // 0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496 - - // f20755ba - // 0000000000000000000000000000000000000000000000000000000000000040 - // 0000000000000000000000007fa9385be102ac3eac297483dd6233d62b3e1496 - // 00000000000000000000000000000000000000000000000000000000000000a0 - // 2cef46a936bdc5b7e6e8c71aa04560c41cf7d88bb26901a7e7f4936ff02accad - // 000000000000000000000000000000000000000000000000000000000000ad9c - // 0000000000000000000000007fa9385be102ac3eac297483dd6233d62b3e1496 - // 0000000000000000000000007fa9385be102ac3eac297483dd6233d62b3e1496 - // 000000000000000000000000000000000000000000000000000000174876e7ff - - // bytes memory payload = abi.encode(MAP_TOKEN_SIG, address(token), token.name(), token.symbol(), token.decimals()); - // vm.expectEmit(true, true, true, false, address(axelarAdaptor)); - // emit AxelarMessage(CHILD_CHAIN_NAME, Strings.toHexString(CHILD_BRIDGE_ADAPTOR), payload); - - // vm.expectEmit(true, true, false, false, address(rootBridge)); - // emit L1TokenMapped(address(token), childToken); - - // // Instead of using expectCalls, we could use expectEmit in combination with mock contracts emitting events. - // // expectCalls requires less boilerplate and is less dependant on mock code. - // vm.expectCall( - // address(axelarAdaptor), - // mapTokenFee, - // abi.encodeWithSelector(axelarAdaptor.sendMessage.selector, payload, address(this)) - // ); - - // // These are calls that the axelarAdaptor should make. - // vm.expectCall( - // address(axelarGasService), - // mapTokenFee, - // abi.encodeWithSelector( - // axelarGasService.payNativeGasForContractCall.selector, - // address(axelarAdaptor), - // CHILD_CHAIN_NAME, - // Strings.toHexString(CHILD_BRIDGE_ADAPTOR), - // payload, - // address(this) - // ) - // ); - - // vm.expectCall( - // address(mockAxelarGateway), - // 0, - // abi.encodeWithSelector( - // mockAxelarGateway.callContract.selector, - // CHILD_CHAIN_NAME, - // Strings.toHexString(CHILD_BRIDGE_ADAPTOR), - // payload - // ) - // ); - - // // Check that we pay mapTokenFee to the axelarGasService. - // uint256 thisPreBal = address(this).balance; - // uint256 axelarGasServicePreBal = address(axelarGasService).balance; - - // rootBridge.mapToken{value: mapTokenFee}(token); - - // // Should update ETH balances as gas payment for message. - // assertEq(address(this).balance, thisPreBal - mapTokenFee, "ETH balance not decreased"); - // assertEq(address(axelarGasService).balance, axelarGasServicePreBal + mapTokenFee, "ETH not paid to gas service"); - - // assertEq(rootBridge.rootTokenToChildToken(address(token)), childToken, "childToken not set"); - // } - - // // TODO split into multiple tests - // function test_depositETH() public { - // uint256 tokenAmount = 300; - // string memory childBridgeAdaptorString = Strings.toHexString(CHILD_BRIDGE_ADAPTOR); - - // (, bytes memory predictedPayload) = - // setupDeposit(NATIVE_ETH, rootBridge, mapTokenFee, depositFee, tokenAmount, false); - - // console2.logBytes(predictedPayload); - - // vm.expectEmit(address(axelarAdaptor)); - // emit AxelarMessage(CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload); - // vm.expectEmit(address(rootBridge)); - // emit NativeEthDeposit( - // address(NATIVE_ETH), rootBridge.childETHToken(), address(this), address(this), tokenAmount - // ); - - // vm.expectCall( - // address(axelarAdaptor), - // depositFee, - // abi.encodeWithSelector(axelarAdaptor.sendMessage.selector, predictedPayload, address(this)) - // ); - - // vm.expectCall( - // address(axelarGasService), - // depositFee, - // abi.encodeWithSelector( - // axelarGasService.payNativeGasForContractCall.selector, - // address(axelarAdaptor), - // CHILD_CHAIN_NAME, - // childBridgeAdaptorString, - // predictedPayload, - // address(this) - // ) - // ); - - // vm.expectCall( - // address(mockAxelarGateway), - // 0, - // abi.encodeWithSelector( - // mockAxelarGateway.callContract.selector, CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload - // ) - // ); - - // uint256 bridgePreBal = address(rootBridge).balance; - - // uint256 thisNativePreBal = address(this).balance; - // uint256 gasServiceNativePreBal = address(axelarGasService).balance; - - // rootBridge.depositETH{value: tokenAmount + depositFee}(tokenAmount); - - // // Check that tokens are transferred - // assertEq(bridgePreBal + tokenAmount, address(rootBridge).balance, "ETH not transferred to bridge"); - // // Check that native asset transferred to gas service - // assertEq(thisNativePreBal - (depositFee + tokenAmount), address(this).balance, "ETH not paid from user"); - // assertEq(gasServiceNativePreBal + depositFee, address(axelarGasService).balance, "ETH not paid to adaptor"); - // } - - // // TODO split into multiple tests - // function test_depositIMXToken() public { - // uint256 tokenAmount = 300; - // string memory childBridgeAdaptorString = Strings.toHexString(CHILD_BRIDGE_ADAPTOR); - - // (, bytes memory predictedPayload) = - // setupDeposit(IMX_TOKEN_ADDRESS, rootBridge, mapTokenFee, depositFee, tokenAmount, false); - - // vm.expectEmit(address(axelarAdaptor)); - // emit AxelarMessage(CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload); - // vm.expectEmit(address(rootBridge)); - // emit IMXDeposit(address(IMX_TOKEN_ADDRESS), address(this), address(this), tokenAmount); - - // vm.expectCall( - // address(axelarAdaptor), - // depositFee, - // abi.encodeWithSelector(axelarAdaptor.sendMessage.selector, predictedPayload, address(this)) - // ); - - // vm.expectCall( - // address(axelarGasService), - // depositFee, - // abi.encodeWithSelector( - // axelarGasService.payNativeGasForContractCall.selector, - // address(axelarAdaptor), - // CHILD_CHAIN_NAME, - // childBridgeAdaptorString, - // predictedPayload, - // address(this) - // ) - // ); - - // vm.expectCall( - // address(mockAxelarGateway), - // 0, - // abi.encodeWithSelector( - // mockAxelarGateway.callContract.selector, CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload - // ) - // ); - - // uint256 thisPreBal = imxToken.balanceOf(address(this)); - // uint256 bridgePreBal = imxToken.balanceOf(address(rootBridge)); - - // uint256 thisNativePreBal = address(this).balance; - // uint256 gasServiceNativePreBal = address(axelarGasService).balance; - - // rootBridge.deposit{value: depositFee}(IERC20Metadata(IMX_TOKEN_ADDRESS), tokenAmount); - - // // Check that tokens are transferred - // assertEq(thisPreBal - tokenAmount, imxToken.balanceOf(address(this)), "Tokens not transferred from user"); - // assertEq( - // bridgePreBal + tokenAmount, imxToken.balanceOf(address(rootBridge)), "Tokens not transferred to bridge" - // ); - // // Check that native asset transferred to gas service - // assertEq(thisNativePreBal - depositFee, address(this).balance, "ETH not paid from user"); - // assertEq(gasServiceNativePreBal + depositFee, address(axelarGasService).balance, "ETH not paid to adaptor"); - // } - - // // TODO split into multiple tests - // function test_depositWETH() public { - // uint256 tokenAmount = 300; - // string memory childBridgeAdaptorString = Strings.toHexString(CHILD_BRIDGE_ADAPTOR); - // (, bytes memory predictedPayload) = - // setupDeposit(WRAPPED_ETH, rootBridge, mapTokenFee, depositFee, tokenAmount, false); - - // vm.expectEmit(address(axelarAdaptor)); - // emit AxelarMessage(CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload); - // vm.expectEmit(address(rootBridge)); - // emit WETHDeposit(address(WRAPPED_ETH), rootBridge.childETHToken(), address(this), address(this), tokenAmount); - // vm.expectCall( - // address(axelarAdaptor), - // depositFee, - // abi.encodeWithSelector(axelarAdaptor.sendMessage.selector, predictedPayload, address(this)) - // ); - - // vm.expectCall( - // address(axelarGasService), - // depositFee, - // abi.encodeWithSelector( - // axelarGasService.payNativeGasForContractCall.selector, - // address(axelarAdaptor), - // CHILD_CHAIN_NAME, - // childBridgeAdaptorString, - // predictedPayload, - // address(this) - // ) - // ); - - // vm.expectCall( - // address(mockAxelarGateway), - // 0, - // abi.encodeWithSelector( - // mockAxelarGateway.callContract.selector, CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload - // ) - // ); - - // uint256 thisPreBal = IERC20Metadata(WRAPPED_ETH).balanceOf(address(this)); - // uint256 bridgePreBal = address(rootBridge).balance; - - // uint256 thisNativePreBal = address(this).balance; - // uint256 gasServiceNativePreBal = address(axelarGasService).balance; - - // rootBridge.deposit{value: depositFee}(IERC20Metadata(WRAPPED_ETH), tokenAmount); - - // // Check that tokens are transferred - // assertEq( - // thisPreBal - tokenAmount, - // IERC20Metadata(WRAPPED_ETH).balanceOf(address(this)), - // "Tokens not transferred from user" - // ); - // assertEq(bridgePreBal + tokenAmount, address(rootBridge).balance, "ETH not transferred to Bridge"); - // // Check that native asset transferred to gas service - // assertEq(thisNativePreBal - depositFee, address(this).balance, "ETH for fee not paid from user"); - // assertEq(gasServiceNativePreBal + depositFee, address(axelarGasService).balance, "ETH not paid to adaptor"); - // } - - // // TODO split into multiple tests - // function test_depositToken() public { - // uint256 tokenAmount = 300; - // string memory childBridgeAdaptorString = Strings.toHexString(CHILD_BRIDGE_ADAPTOR); - // (address childToken, bytes memory predictedPayload) = - // setupDeposit(address(token), rootBridge, mapTokenFee, depositFee, tokenAmount, true); - - // vm.expectEmit(address(axelarAdaptor)); - // emit AxelarMessage(CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload); - // vm.expectEmit(address(rootBridge)); - // emit ChildChainERC20Deposit(address(token), childToken, address(this), address(this), tokenAmount); - - // vm.expectCall( - // address(axelarAdaptor), - // depositFee, - // abi.encodeWithSelector(axelarAdaptor.sendMessage.selector, predictedPayload, address(this)) - // ); - - // vm.expectCall( - // address(axelarGasService), - // depositFee, - // abi.encodeWithSelector( - // axelarGasService.payNativeGasForContractCall.selector, - // address(axelarAdaptor), - // CHILD_CHAIN_NAME, - // childBridgeAdaptorString, - // predictedPayload, - // address(this) - // ) - // ); - - // vm.expectCall( - // address(mockAxelarGateway), - // 0, - // abi.encodeWithSelector( - // mockAxelarGateway.callContract.selector, CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload - // ) - // ); - - // uint256 thisPreBal = token.balanceOf(address(this)); - // uint256 bridgePreBal = token.balanceOf(address(rootBridge)); - - // uint256 thisNativePreBal = address(this).balance; - // uint256 gasServiceNativePreBal = address(axelarGasService).balance; - - // rootBridge.deposit{value: depositFee}(token, tokenAmount); - - // // Check that tokens are transferred - // assertEq(thisPreBal - tokenAmount, token.balanceOf(address(this)), "Tokens not transferred from user"); - // assertEq(bridgePreBal + tokenAmount, token.balanceOf(address(rootBridge)), "Tokens not transferred to bridge"); - // // Check that native asset transferred to gas service - // assertEq(thisNativePreBal - depositFee, address(this).balance, "ETH not paid from user"); - // assertEq(gasServiceNativePreBal + depositFee, address(axelarGasService).balance, "ETH not paid to adaptor"); - // } - - // // TODO split into multiple tests - // function test_depositTo() public { - // uint256 tokenAmount = 300; - // address recipient = address(9876); - // string memory childBridgeAdaptorString = Strings.toHexString(CHILD_BRIDGE_ADAPTOR); - // (address childToken, bytes memory predictedPayload) = - // setupDepositTo(address(token), rootBridge, mapTokenFee, depositFee, tokenAmount, recipient, true); - - // vm.expectEmit(address(axelarAdaptor)); - // emit AxelarMessage(CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload); - // vm.expectEmit(address(rootBridge)); - // emit ChildChainERC20Deposit(address(token), childToken, address(this), recipient, tokenAmount); - - // vm.expectCall( - // address(axelarAdaptor), - // depositFee, - // abi.encodeWithSelector(axelarAdaptor.sendMessage.selector, predictedPayload, address(this)) - // ); - - // vm.expectCall( - // address(axelarGasService), - // depositFee, - // abi.encodeWithSelector( - // axelarGasService.payNativeGasForContractCall.selector, - // address(axelarAdaptor), - // CHILD_CHAIN_NAME, - // childBridgeAdaptorString, - // predictedPayload, - // address(this) - // ) - // ); - - // vm.expectCall( - // address(mockAxelarGateway), - // 0, - // abi.encodeWithSelector( - // mockAxelarGateway.callContract.selector, CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload - // ) - // ); - - // uint256 thisPreBal = token.balanceOf(address(this)); - // uint256 bridgePreBal = token.balanceOf(address(rootBridge)); - - // uint256 thisNativePreBal = address(this).balance; - // uint256 gasServiceNativePreBal = address(axelarGasService).balance; - - // rootBridge.depositTo{value: depositFee}(token, recipient, tokenAmount); - - // // Check that tokens are transferred - // assertEq(thisPreBal - tokenAmount, token.balanceOf(address(this)), "Tokens not transferred from user"); - // assertEq(bridgePreBal + tokenAmount, token.balanceOf(address(rootBridge)), "Tokens not transferred to bridge"); - // // Check that native asset transferred to gas service - // assertEq(thisNativePreBal - depositFee, address(this).balance, "ETH not paid from user"); - // assertEq(gasServiceNativePreBal + depositFee, address(axelarGasService).balance, "ETH not paid to adaptor"); - // } } From c5450a617139202cdd1204650adafcc29476cab9 Mon Sep 17 00:00:00 2001 From: Benjamin Patch Date: Sat, 4 Nov 2023 08:20:25 +1100 Subject: [PATCH 3/3] Address PR comments --- src/root/RootERC20Bridge.sol | 8 ++- test/unit/root/RootERC20Bridge.t.sol | 76 ++++++++++++++++++++++++---- 2 files changed, 71 insertions(+), 13 deletions(-) diff --git a/src/root/RootERC20Bridge.sol b/src/root/RootERC20Bridge.sol index 6611ecf7b..1abc92f18 100644 --- a/src/root/RootERC20Bridge.sol +++ b/src/root/RootERC20Bridge.sol @@ -35,6 +35,7 @@ contract RootERC20Bridge is /// @dev leave this as the first param for the integration tests mapping(address => address) public rootTokenToChildToken; + uint256 public constant NO_DEPOSIT_LIMIT = 0; bytes32 public constant MAP_TOKEN_SIG = keccak256("MAP_TOKEN"); bytes32 public constant DEPOSIT_SIG = keccak256("DEPOSIT"); address public constant NATIVE_ETH = address(0xeee); @@ -119,7 +120,10 @@ contract RootERC20Bridge is * @dev The limit can decrease, but it can never decrease to below the contract's IMX balance. */ function updateImxCumulativeDepositLimit(uint256 newImxCumulativeDepositLimit) external onlyOwner { - if (newImxCumulativeDepositLimit < IERC20Metadata(rootIMXToken).balanceOf(address(this))) { + if ( + newImxCumulativeDepositLimit != NO_DEPOSIT_LIMIT + && newImxCumulativeDepositLimit < IERC20Metadata(rootIMXToken).balanceOf(address(this)) + ) { revert ImxDepositLimitTooLow(); } emit NewImxDepositLimit(imxCumulativeDepositLimit, newImxCumulativeDepositLimit); @@ -256,7 +260,7 @@ contract RootERC20Bridge is revert ZeroAmount(); } if ( - address(rootToken) == rootIMXToken && imxCumulativeDepositLimit != 0 + address(rootToken) == rootIMXToken && imxCumulativeDepositLimit != NO_DEPOSIT_LIMIT && IERC20Metadata(rootIMXToken).balanceOf(address(this)) + amount > imxCumulativeDepositLimit ) { revert ImxDepositLimitExceeded(); diff --git a/test/unit/root/RootERC20Bridge.t.sol b/test/unit/root/RootERC20Bridge.t.sol index b527ebff5..07e612e4d 100644 --- a/test/unit/root/RootERC20Bridge.t.sol +++ b/test/unit/root/RootERC20Bridge.t.sol @@ -27,7 +27,7 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid address constant WRAPPED_ETH = address(0xddd); uint256 constant mapTokenFee = 300; uint256 constant depositFee = 200; - uint256 constant unlimitedIMXDeposits = 0; + uint256 constant UNLIMITED_IMX_DEPOSITS = 0; ERC20PresetMinterPauser public token; RootERC20Bridge public rootBridge; @@ -55,7 +55,7 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid address(token), IMX_TOKEN, WRAPPED_ETH, - unlimitedIMXDeposits + UNLIMITED_IMX_DEPOSITS ); } @@ -78,7 +78,7 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid address(token), IMX_TOKEN, WRAPPED_ETH, - unlimitedIMXDeposits + UNLIMITED_IMX_DEPOSITS ); } @@ -92,7 +92,7 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid address(1), address(1), address(1), - unlimitedIMXDeposits + UNLIMITED_IMX_DEPOSITS ); } @@ -106,14 +106,14 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid address(1), address(1), address(1), - unlimitedIMXDeposits + UNLIMITED_IMX_DEPOSITS ); } function test_RevertIf_InitializeWithEmptyChildAdapter() public { RootERC20Bridge bridge = new RootERC20Bridge(); vm.expectRevert(InvalidChildERC20BridgeAdaptor.selector); - bridge.initialize(address(1), address(1), "", address(1), address(1), address(1), unlimitedIMXDeposits); + bridge.initialize(address(1), address(1), "", address(1), address(1), address(1), UNLIMITED_IMX_DEPOSITS); } function test_RevertIf_InitializeWithAZeroAddressTokenTemplate() public { @@ -126,7 +126,7 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid address(0), address(1), address(1), - unlimitedIMXDeposits + UNLIMITED_IMX_DEPOSITS ); } @@ -140,7 +140,7 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid address(1), address(0), address(1), - unlimitedIMXDeposits + UNLIMITED_IMX_DEPOSITS ); } @@ -154,14 +154,14 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid address(1), address(1), address(0), - unlimitedIMXDeposits + UNLIMITED_IMX_DEPOSITS ); } function test_RevertIf_InitializeWithAZeroAddressAll() public { RootERC20Bridge bridge = new RootERC20Bridge(); vm.expectRevert(ZeroAddress.selector); - bridge.initialize(address(0), address(0), "", address(0), address(0), address(0), unlimitedIMXDeposits); + bridge.initialize(address(0), address(0), "", address(0), address(0), address(0), UNLIMITED_IMX_DEPOSITS); } /** @@ -262,7 +262,7 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid rootBridge.mapToken{value: 300}(IERC20Metadata(NATIVE_ETH)); } - function test_updateRootBridgeAdaptor() public { + function test_updateRootBridgeAdaptor_UpdatesRootBridgeAdaptor() public { address newAdaptorAddress = address(0x11111); assertEq(address(rootBridge.rootBridgeAdaptor()), address(mockAxelarAdaptor), "bridgeAdaptor not set"); @@ -270,6 +270,15 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid assertEq(address(rootBridge.rootBridgeAdaptor()), newAdaptorAddress, "bridgeAdaptor not updated"); } + function test_updateRootBridgeAdaptor_EmitsNewRootBridgeAdaptorEvent() public { + address newAdaptorAddress = address(0x11111); + + vm.expectEmit(); + emit NewRootBridgeAdaptor(address(rootBridge.rootBridgeAdaptor()), newAdaptorAddress); + + rootBridge.updateRootBridgeAdaptor(newAdaptorAddress); + } + function test_RevertIf_updateRootBridgeAdaptorCalledByNonOwner() public { vm.prank(address(0xf00f00)); vm.expectRevert("Ownable: caller is not the owner"); @@ -493,6 +502,51 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid rootBridge.deposit{value: depositFee}(IERC20Metadata(IMX_TOKEN), amount); } + function test_deposit_whenSettingImxDepositLimitToUnlimited() public { + uint256 imxCumulativeDepositLimit = 700; + + uint256 amount = 300; + setupDeposit(IMX_TOKEN, rootBridge, mapTokenFee, depositFee, amount, false); + + IERC20Metadata(IMX_TOKEN).approve(address(rootBridge), type(uint256).max); + + rootBridge.updateImxCumulativeDepositLimit(imxCumulativeDepositLimit); + + // Valid + rootBridge.deposit{value: depositFee}(IERC20Metadata(IMX_TOKEN), amount); + + setupDeposit(IMX_TOKEN, rootBridge, mapTokenFee, depositFee, amount, false); + // Valid + rootBridge.deposit{value: depositFee}(IERC20Metadata(IMX_TOKEN), amount); + + setupDeposit(IMX_TOKEN, rootBridge, mapTokenFee, depositFee, amount, false); + // Invalid + vm.expectRevert(ImxDepositLimitExceeded.selector); + rootBridge.deposit{value: depositFee}(IERC20Metadata(IMX_TOKEN), amount); + + rootBridge.updateImxCumulativeDepositLimit(UNLIMITED_IMX_DEPOSITS); + + uint256 bigDepositAmount = 999999999999 ether; + setupDeposit(IMX_TOKEN, rootBridge, mapTokenFee, depositFee, bigDepositAmount, false); + + uint256 thisPreBal = IERC20Metadata(IMX_TOKEN).balanceOf(address(this)); + uint256 bridgePreBal = IERC20Metadata(IMX_TOKEN).balanceOf(address(rootBridge)); + + rootBridge.deposit{value: depositFee}(IERC20Metadata(IMX_TOKEN), bigDepositAmount); + + // Check that tokens are transferred + assertEq( + thisPreBal - bigDepositAmount, + IERC20Metadata(IMX_TOKEN).balanceOf(address(this)), + "Tokens not transferred from user" + ); + assertEq( + bridgePreBal + bigDepositAmount, + IERC20Metadata(IMX_TOKEN).balanceOf(address(rootBridge)), + "Tokens not transferred to bridge" + ); + } + function test_depositCallsSendMessage() public { uint256 amount = 100; (, bytes memory predictedPayload) =