From 1bf7911676aca9ca015184b205757d40a84b4d72 Mon Sep 17 00:00:00 2001 From: Benjamin Patch Date: Fri, 3 Nov 2023 13:58:30 +1100 Subject: [PATCH 1/9] Tests almost complete --- script/DeployRootContracts.s.sol | 11 +-- script/InitializeRootContracts.s.sol | 6 +- src/child/ChildAxelarBridgeAdaptor.sol | 4 +- src/interfaces/root/IRootERC20Bridge.sol | 26 ++++++++ src/root/RootAxelarBridgeAdaptor.sol | 32 +++++---- src/root/RootERC20Bridge.sol | 70 +++++++++++++++++++- src/test/child/ChildERC20FailOnBurn.sol | 2 +- src/test/root/MockAxelarGateway.sol | 4 ++ src/test/root/StubRootBridge.sol | 1 + test/unit/root/RootAxelarBridgeAdaptor.t.sol | 18 +++-- test/unit/root/RootERC20Bridge.t.sol | 41 +++++++++--- test/utils.t.sol | 9 ++- 12 files changed, 174 insertions(+), 50 deletions(-) diff --git a/script/DeployRootContracts.s.sol b/script/DeployRootContracts.s.sol index a1f49fceb..8c04b8b5f 100644 --- a/script/DeployRootContracts.s.sol +++ b/script/DeployRootContracts.s.sol @@ -21,6 +21,7 @@ contract DeployRootContracts is Script { uint256 rootPrivateKey = vm.envUint("ROOT_PRIVATE_KEY"); string memory rootRpcUrl = vm.envString("ROOT_RPC_URL"); string memory deployEnvironment = vm.envString("ENVIRONMENT"); + address rootGateway = vm.envAddress("ROOT_GATEWAY_ADDRESS"); /** * DEPLOY ROOT CHAIN CONTRACTS @@ -35,7 +36,9 @@ 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), "filler_child_name" + ); TransparentUpgradeableProxy rootERC20BridgeProxy = new TransparentUpgradeableProxy( address(rootERC20BridgeImplementation), address(proxyAdmin), @@ -44,10 +47,8 @@ contract DeployRootContracts is Script { // TODO add dummy initialize of implementation contracts! - RootAxelarBridgeAdaptor rootBridgeAdaptorImplementation = new RootAxelarBridgeAdaptor(); - rootBridgeAdaptorImplementation.initialize( - address(rootERC20BridgeImplementation), "Filler", address(1), address(1) - ); + RootAxelarBridgeAdaptor rootBridgeAdaptorImplementation = new RootAxelarBridgeAdaptor(rootGateway); + rootBridgeAdaptorImplementation.initialize(address(rootERC20BridgeImplementation), "Filler", address(1)); TransparentUpgradeableProxy rootBridgeAdaptorProxy = new TransparentUpgradeableProxy( address(rootBridgeAdaptorImplementation), address(proxyAdmin), diff --git a/script/InitializeRootContracts.s.sol b/script/InitializeRootContracts.s.sol index c9a25c2c3..d29ee55cd 100644 --- a/script/InitializeRootContracts.s.sol +++ b/script/InitializeRootContracts.s.sol @@ -24,7 +24,6 @@ struct InitializeRootContractsParams { address rootIMXToken; address rootWETHToken; string childChainName; - address rootGateway; address rootGasService; } @@ -41,7 +40,6 @@ contract InitializeRootContracts is Script { rootIMXToken: vm.envAddress("ROOT_IMX_ADDRESS"), 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") }); @@ -60,13 +58,13 @@ contract InitializeRootContracts is Script { childBridgeAdaptorChecksum, params.rootChainChildTokenTemplate, params.rootIMXToken, - params.rootWETHToken + params.rootWETHToken, + params.childChainName ); params.rootBridgeAdaptor.initialize( address(params.rootERC20Bridge), // root bridge params.childChainName, // child chain name - params.rootGateway, // axelar gateway params.rootGasService // axelar gas service ); diff --git a/src/child/ChildAxelarBridgeAdaptor.sol b/src/child/ChildAxelarBridgeAdaptor.sol index 144f6cac3..cb80d2456 100644 --- a/src/child/ChildAxelarBridgeAdaptor.sol +++ b/src/child/ChildAxelarBridgeAdaptor.sol @@ -22,7 +22,6 @@ contract ChildAxelarBridgeAdaptor is /// @notice Address of bridge to relay messages to. IChildERC20Bridge public childBridge; IAxelarGasService public gasService; - string public rootBridgeAdaptor; string public rootChain; constructor(address _gateway) AxelarExecutable(_gateway) {} @@ -39,7 +38,6 @@ contract ChildAxelarBridgeAdaptor is childBridge = IChildERC20Bridge(_childBridge); rootChain = _rootChain; gasService = IAxelarGasService(_gasService); - rootBridgeAdaptor = childBridge.rootERC20BridgeAdaptor(); } /** @@ -54,7 +52,7 @@ contract ChildAxelarBridgeAdaptor is } // Load from storage. - string memory _rootBridgeAdaptor = rootBridgeAdaptor; + string memory _rootBridgeAdaptor = childBridge.rootERC20BridgeAdaptor(); string memory _rootChain = rootChain; gasService.payNativeGasForContractCall{value: msg.value}( diff --git a/src/interfaces/root/IRootERC20Bridge.sol b/src/interfaces/root/IRootERC20Bridge.sol index edea801e9..e3fe11299 100644 --- a/src/interfaces/root/IRootERC20Bridge.sol +++ b/src/interfaces/root/IRootERC20Bridge.sol @@ -5,6 +5,14 @@ import {IERC20Metadata} from "@openzeppelin/contracts/token/ERC20/extensions/IER interface IRootERC20Bridge { function childBridgeAdaptor() external view returns (string memory); + /** + * @notice Receives a bridge message from child chain, parsing the message type then executing. + * @param sourceChain The chain the message originated from. + * @param sourceAddress The address the message originated from. + * @param data The data payload of the message. + */ + function onMessageReceive(string calldata sourceChain, string calldata sourceAddress, bytes calldata data) + external; /** * @notice Initiate sending a mapToken message to the child chain. @@ -60,6 +68,14 @@ interface IRootERC20BridgeEvents { address indexed receiver, uint256 amount ); + + event RootChainERC20Withdraw( + address indexed rootToken, + address indexed childToken, + address withdrawer, + address indexed receiver, + uint256 amount + ); } interface IRootERC20BridgeErrors { @@ -69,6 +85,8 @@ interface IRootERC20BridgeErrors { error ZeroAmount(); /// @notice Error when a zero address is given when not valid. error ZeroAddress(); + /// @notice Error when the child chain name is invalid. + error InvalidChildChain(); /// @notice Error when a token is already mapped. error AlreadyMapped(); /// @notice Error when a token is not mapped when it should be. @@ -83,4 +101,12 @@ interface IRootERC20BridgeErrors { error BalanceInvariantCheckFailed(uint256 actualBalance, uint256 expectedBalance); /// @notice Error when the given child chain bridge adaptor is invalid. error InvalidChildERC20BridgeAdaptor(); + /// @notice Error when a message received has invalid data. + error InvalidData(); + /// @notice Error when a message received has invalid source address. + error InvalidSourceAddress(); + /// @notice Error when a message received has invalid source chain. + error InvalidSourceChain(); + /// @notice Error when caller is not the root bridge adaptor but should be. + error NotBridgeAdaptor(); } diff --git a/src/root/RootAxelarBridgeAdaptor.sol b/src/root/RootAxelarBridgeAdaptor.sol index 8c769959d..7c9d3877c 100644 --- a/src/root/RootAxelarBridgeAdaptor.sol +++ b/src/root/RootAxelarBridgeAdaptor.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: Apache 2.0 pragma solidity ^0.8.21; +import {AxelarExecutable} from "@axelar-gmp-sdk-solidity/contracts/executable/AxelarExecutable.sol"; import {Initializable} from "@openzeppelin/contracts/proxy/utils/Initializable.sol"; import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import {IERC20Metadata} from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; @@ -20,6 +21,7 @@ import {IRootERC20Bridge} from "../interfaces/root/IRootERC20Bridge.sol"; * @notice RootAxelarBridgeAdaptor is a bridge adaptor that allows the RootERC20Bridge to communicate with the Axelar Gateway. */ contract RootAxelarBridgeAdaptor is + AxelarExecutable, Initializable, IRootERC20BridgeAdaptor, IRootAxelarBridgeAdaptorEvents, @@ -27,34 +29,30 @@ contract RootAxelarBridgeAdaptor is { using SafeERC20 for IERC20Metadata; - address public rootBridge; + IRootERC20Bridge public rootBridge; string public childBridgeAdaptor; string public childChain; - IAxelarGateway public axelarGateway; IAxelarGasService public gasService; mapping(uint256 => string) public chainIdToChainName; + constructor(address _gateway) AxelarExecutable(_gateway) {} + /** * @notice Initilization function for RootAxelarBridgeAdaptor. * @param _rootBridge Address of root bridge contract. * @param _childChain Name of child chain. - * @param _axelarGateway Address of Axelar Gateway contract. * @param _gasService Address of Axelar Gas Service contract. */ - function initialize(address _rootBridge, string memory _childChain, address _axelarGateway, address _gasService) - public - initializer - { - if (_rootBridge == address(0) || _axelarGateway == address(0) || _gasService == address(0)) { + function initialize(address _rootBridge, string memory _childChain, address _gasService) public initializer { + if (_rootBridge == address(0) || _gasService == address(0)) { revert ZeroAddresses(); } if (bytes(_childChain).length == 0) { revert InvalidChildChain(); } - rootBridge = _rootBridge; + rootBridge = IRootERC20Bridge(_rootBridge); childChain = _childChain; - axelarGateway = IAxelarGateway(_axelarGateway); gasService = IAxelarGasService(_gasService); } @@ -66,7 +64,7 @@ contract RootAxelarBridgeAdaptor is if (msg.value == 0) { revert NoGas(); } - if (msg.sender != rootBridge) { + if (msg.sender != address(rootBridge)) { revert CallerNotBridge(); } @@ -79,7 +77,17 @@ contract RootAxelarBridgeAdaptor is address(this), _childChain, _childBridgeAdaptor, payload, refundRecipient ); - axelarGateway.callContract(_childChain, _childBridgeAdaptor, payload); + gateway.callContract(_childChain, _childBridgeAdaptor, payload); emit AxelarMessage(_childChain, _childBridgeAdaptor, payload); } + + /** + * @dev This function is called by the parent `AxelarExecutable` contract to execute the payload. + */ + function _execute(string calldata sourceChain_, string calldata sourceAddress_, bytes calldata payload_) + internal + override + { + rootBridge.onMessageReceive(sourceChain_, sourceAddress_, payload_); + } } diff --git a/src/root/RootERC20Bridge.sol b/src/root/RootERC20Bridge.sol index 263a1ea8f..5ba1511cc 100644 --- a/src/root/RootERC20Bridge.sol +++ b/src/root/RootERC20Bridge.sol @@ -7,6 +7,7 @@ import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; import {Address} from "@openzeppelin/contracts/utils/Address.sol"; import {Strings} from "@openzeppelin/contracts/utils/Strings.sol"; import {Ownable2Step} from "@openzeppelin/contracts/access/Ownable2Step.sol"; +import {Address} from "@openzeppelin/contracts/utils/Address.sol"; import {IAxelarGateway} from "@axelar-cgp-solidity/contracts/interfaces/IAxelarGateway.sol"; import {IRootERC20Bridge, IERC20Metadata} from "../interfaces/root/IRootERC20Bridge.sol"; import {IRootERC20BridgeEvents, IRootERC20BridgeErrors} from "../interfaces/root/IRootERC20Bridge.sol"; @@ -37,6 +38,7 @@ contract RootERC20Bridge is bytes32 public constant MAP_TOKEN_SIG = keccak256("MAP_TOKEN"); bytes32 public constant DEPOSIT_SIG = keccak256("DEPOSIT"); + bytes32 public constant WITHDRAW_SIG = keccak256("WITHDRAW"); address public constant NATIVE_ETH = address(0xeee); IRootERC20BridgeAdaptor public rootBridgeAdaptor; @@ -52,6 +54,8 @@ contract RootERC20Bridge is address public childETHToken; /// @dev The address of the wETH ERC20 token on L1. address public rootWETHToken; + /// @dev The name of the chain that this bridge is connected to. + string public childChain; /** * @notice Initilization function for RootERC20Bridge. @@ -61,6 +65,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 newChildChain Name of child chain. * @dev Can only be called once. */ function initialize( @@ -69,7 +74,8 @@ contract RootERC20Bridge is string memory newChildBridgeAdaptor, address newChildTokenTemplate, address newRootIMXToken, - address newRootWETHToken + address newRootWETHToken, + string memory newChildChain ) public initializer { if ( newRootBridgeAdaptor == address(0) || newChildERC20Bridge == address(0) @@ -80,6 +86,10 @@ contract RootERC20Bridge is if (bytes(newChildBridgeAdaptor).length == 0) { revert InvalidChildERC20BridgeAdaptor(); } + if (bytes(newChildChain).length == 0) { + revert InvalidChildChain(); + } + childERC20Bridge = newChildERC20Bridge; childTokenTemplate = newChildTokenTemplate; rootIMXToken = newRootIMXToken; @@ -89,6 +99,7 @@ contract RootERC20Bridge is ); rootBridgeAdaptor = IRootERC20BridgeAdaptor(newRootBridgeAdaptor); childBridgeAdaptor = newChildBridgeAdaptor; + childChain = newChildChain; } function updateRootBridgeAdaptor(address newRootBridgeAdaptor) external onlyOwner { @@ -103,6 +114,35 @@ contract RootERC20Bridge is */ receive() external payable {} + /** + * @inheritdoc IRootERC20Bridge + * @dev This is only callable by the root chain bridge adaptor. + * @dev Validates `sourceAddress` is the child chain's bridgeAdaptor. + */ + function onMessageReceive(string calldata messageSourceChain, string calldata sourceAddress, bytes calldata data) + external + override + { + if (msg.sender != address(rootBridgeAdaptor)) { + revert NotBridgeAdaptor(); + } + if (!Strings.equal(messageSourceChain, childChain)) { + revert InvalidSourceChain(); + } + if (!Strings.equal(sourceAddress, childBridgeAdaptor)) { + revert InvalidSourceAddress(); + } + if (data.length == 0) { + revert InvalidData(); + } + + if (bytes32(data[:32]) == WITHDRAW_SIG) { + _withdraw(data[32:]); + } else { + revert InvalidData(); + } + } + /** * @inheritdoc IRootERC20Bridge * @dev TODO when this becomes part of the deposit flow on a token's first bridge, this logic will need to be mostly moved into an internal function. @@ -270,4 +310,32 @@ contract RootERC20Bridge is emit ChildChainERC20Deposit(address(rootToken), childToken, msg.sender, receiver, amount); } } + + function _withdraw(bytes memory data) private { + (address rootToken, address withdrawer, address receiver, uint256 amount) = + abi.decode(data, (address, address, address, uint256)); + address childToken = rootTokenToChildToken[rootToken]; + if (childToken == address(0)) { + revert NotMapped(); + } + _executeTransfer(rootToken, childToken, withdrawer, receiver, amount); + } + + function _executeTransfer( + address rootToken, + address childToken, + address withdrawer, + address receiver, + uint256 amount + ) internal { + // TODO when withdrawing ETH/WETH, this next section will also need to check for the withdrawal of WETH (i.e. rootToken == NATIVE_ETH || rootToken == CHILD_WETH) + // Tests for this NATIVE_ETH branch not yet written. This should come as part of that PR. + if (rootToken == NATIVE_ETH) { + Address.sendValue(payable(receiver), amount); + } else { + IERC20Metadata(rootToken).safeTransfer(receiver, amount); + } + // slither-disable-next-line reentrancy-events + emit RootChainERC20Withdraw(address(rootToken), childToken, withdrawer, receiver, amount); + } } diff --git a/src/test/child/ChildERC20FailOnBurn.sol b/src/test/child/ChildERC20FailOnBurn.sol index f287319ca..5fd11ad86 100644 --- a/src/test/child/ChildERC20FailOnBurn.sol +++ b/src/test/child/ChildERC20FailOnBurn.sol @@ -12,7 +12,7 @@ import "../../child/ChildERC20.sol"; */ // solhint-disable reason-string contract ChildERC20FailOnBurn is ChildERC20 { - function burn(address account, uint256 amount) public virtual override returns (bool) { + function burn(address, /*account*/ uint256 /*amount*/ ) public virtual override returns (bool) { return false; } } diff --git a/src/test/root/MockAxelarGateway.sol b/src/test/root/MockAxelarGateway.sol index 756a61a62..8b16342a2 100644 --- a/src/test/root/MockAxelarGateway.sol +++ b/src/test/root/MockAxelarGateway.sol @@ -4,4 +4,8 @@ pragma solidity ^0.8.21; // @dev A contract for ensuring the Axelar Gateway is called correctly during unit tests. contract MockAxelarGateway { function callContract(string memory childChain, string memory childBridgeAdaptor, bytes memory payload) external {} + + function validateContractCall(bytes32, string calldata, string calldata, bytes32) external pure returns (bool) { + return true; + } } diff --git a/src/test/root/StubRootBridge.sol b/src/test/root/StubRootBridge.sol index da4081155..25806be91 100644 --- a/src/test/root/StubRootBridge.sol +++ b/src/test/root/StubRootBridge.sol @@ -7,4 +7,5 @@ contract StubRootBridge { function childBridgeAdaptor() external pure returns (string memory) { return Strings.toHexString(address(9999)); } + function onMessageReceive(string calldata, string calldata, bytes calldata) external {} } diff --git a/test/unit/root/RootAxelarBridgeAdaptor.t.sol b/test/unit/root/RootAxelarBridgeAdaptor.t.sol index 2374409f2..5bd34aa86 100644 --- a/test/unit/root/RootAxelarBridgeAdaptor.t.sol +++ b/test/unit/root/RootAxelarBridgeAdaptor.t.sol @@ -32,30 +32,28 @@ contract RootAxelarBridgeAdaptorTest is Test, IRootAxelarBridgeAdaptorEvents, IR stubRootBridge = new StubRootBridge(); childBridgeAdaptor = stubRootBridge.childBridgeAdaptor(); - axelarAdaptor = new RootAxelarBridgeAdaptor(); - axelarAdaptor.initialize( - address(stubRootBridge), CHILD_CHAIN_NAME, address(mockAxelarGateway), address(axelarGasService) - ); + axelarAdaptor = new RootAxelarBridgeAdaptor(address(mockAxelarGateway)); + axelarAdaptor.initialize(address(stubRootBridge), CHILD_CHAIN_NAME, address(axelarGasService)); vm.deal(address(stubRootBridge), 99999999999); } function test_Constructor() public { - assertEq(axelarAdaptor.rootBridge(), address(stubRootBridge), "rootBridge not set"); + assertEq(address(axelarAdaptor.rootBridge()), address(stubRootBridge), "rootBridge not set"); assertEq(axelarAdaptor.childChain(), CHILD_CHAIN_NAME, "childChain not set"); - assertEq(address(axelarAdaptor.axelarGateway()), address(mockAxelarGateway), "axelarGateway not set"); + assertEq(address(axelarAdaptor.gateway()), address(mockAxelarGateway), "axelarGateway not set"); assertEq(address(axelarAdaptor.gasService()), address(axelarGasService), "axelarGasService not set"); } function test_RevertWhen_InitializerGivenZeroAddress() public { - RootAxelarBridgeAdaptor newAdaptor = new RootAxelarBridgeAdaptor(); + RootAxelarBridgeAdaptor newAdaptor = new RootAxelarBridgeAdaptor(address(mockAxelarGateway)); vm.expectRevert(ZeroAddresses.selector); - newAdaptor.initialize(address(0), CHILD_CHAIN_NAME, address(mockAxelarGateway), address(axelarGasService)); + newAdaptor.initialize(address(0), CHILD_CHAIN_NAME, address(axelarGasService)); } function test_RevertWhen_ConstructorGivenEmptyChildChainName() public { - RootAxelarBridgeAdaptor newAdaptor = new RootAxelarBridgeAdaptor(); + RootAxelarBridgeAdaptor newAdaptor = new RootAxelarBridgeAdaptor(address(mockAxelarGateway)); vm.expectRevert(InvalidChildChain.selector); - newAdaptor.initialize(address(this), "", address(mockAxelarGateway), address(axelarGasService)); + newAdaptor.initialize(address(this), "", address(axelarGasService)); } /// @dev For this unit test we just want to make sure the correct functions are called on the Axelar Gateway and Gas Service. diff --git a/test/unit/root/RootERC20Bridge.t.sol b/test/unit/root/RootERC20Bridge.t.sol index ef8ff0353..07d316ba2 100644 --- a/test/unit/root/RootERC20Bridge.t.sol +++ b/test/unit/root/RootERC20Bridge.t.sol @@ -53,7 +53,8 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid CHILD_BRIDGE_ADAPTOR_STRING, address(token), IMX_TOKEN, - WRAPPED_ETH + WRAPPED_ETH, + CHILD_CHAIN_NAME ); } @@ -65,6 +66,11 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid assertEq(address(rootBridge.rootBridgeAdaptor()), address(mockAxelarAdaptor), "bridgeAdaptor not set"); assertEq(rootBridge.childERC20Bridge(), CHILD_BRIDGE, "childERC20Bridge not set"); assertEq(rootBridge.childTokenTemplate(), address(token), "childTokenTemplate not set"); + assert(Strings.equal(rootBridge.childChain(), CHILD_CHAIN_NAME)); + assert(Strings.equal(CHILD_BRIDGE_ADAPTOR_STRING, rootBridge.childBridgeAdaptor())); + assertEq(address(token), rootBridge.childTokenTemplate(), "childTokenTemplate not set"); + assertEq(rootBridge.rootIMXToken(), IMX_TOKEN, "rootIMXToken not set"); + assertEq(rootBridge.rootWETHToken(), WRAPPED_ETH, "rootWETHToken not set"); } function test_RevertIfInitializeTwice() public { @@ -75,50 +81,67 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid CHILD_BRIDGE_ADAPTOR_STRING, address(token), IMX_TOKEN, - WRAPPED_ETH + WRAPPED_ETH, + CHILD_CHAIN_NAME ); } 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), CHILD_CHAIN_NAME + ); } 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), CHILD_CHAIN_NAME + ); } 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), CHILD_CHAIN_NAME); } 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), CHILD_CHAIN_NAME + ); } 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), CHILD_CHAIN_NAME + ); } 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), CHILD_CHAIN_NAME + ); } 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), CHILD_CHAIN_NAME); + } + + function test_RevertIf_InitializeWithEmptyChildName() public { + RootERC20Bridge bridge = new RootERC20Bridge(); + vm.expectRevert(InvalidChildChain.selector); + bridge.initialize(address(1), address(1), CHILD_BRIDGE_ADAPTOR_STRING, address(1), address(1), address(1), ""); } /** diff --git a/test/utils.t.sol b/test/utils.t.sol index b186fbbc4..9e9b86cea 100644 --- a/test/utils.t.sol +++ b/test/utils.t.sol @@ -86,7 +86,7 @@ contract Utils is Test { mockAxelarGateway = new MockAxelarGateway(); axelarGasService = new MockAxelarGasService(); - axelarAdaptor = new RootAxelarBridgeAdaptor(); + axelarAdaptor = new RootAxelarBridgeAdaptor(address(mockAxelarGateway)); rootBridge.initialize( address(axelarAdaptor), @@ -94,12 +94,11 @@ contract Utils is Test { Strings.toHexString(childBridgeAdaptor), address(token), imxTokenAddress, - wethTokenAddress + wethTokenAddress, + "CHILD" ); - axelarAdaptor.initialize( - address(rootBridge), childBridgeName, address(mockAxelarGateway), address(axelarGasService) - ); + axelarAdaptor.initialize(address(rootBridge), childBridgeName, address(axelarGasService)); } function setupDeposit( From e3efc7d9aaf9ccad29377b1b5c752e925cce4044 Mon Sep 17 00:00:00 2001 From: Benjamin Patch Date: Fri, 3 Nov 2023 13:59:25 +1100 Subject: [PATCH 2/9] Add new test files --- .../RootERC20BridgeWithdraw.t.sol | 91 ++++++++++ .../RootAxelarBridgeAdaptorWithdraw.t.sol | 54 ++++++ .../withdrawals/RootERC20BridgeWithdraw.t.sol | 167 ++++++++++++++++++ 3 files changed, 312 insertions(+) create mode 100644 test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol create mode 100644 test/unit/root/withdrawals/RootAxelarBridgeAdaptorWithdraw.t.sol create mode 100644 test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol diff --git a/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol b/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol new file mode 100644 index 000000000..68cf79d6d --- /dev/null +++ b/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: Apache 2.0 +pragma solidity ^0.8.21; + +import {Test, console2} from "forge-std/Test.sol"; +import {ERC20PresetMinterPauser} from "@openzeppelin/contracts/token/ERC20/presets/ERC20PresetMinterPauser.sol"; +import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; +import {Strings} from "@openzeppelin/contracts/utils/Strings.sol"; +import {MockAxelarGateway} from "../../../../src/test/root/MockAxelarGateway.sol"; +import {MockAxelarGasService} from "../../../../src/test/root/MockAxelarGasService.sol"; +import {RootERC20Bridge, IRootERC20BridgeEvents, IERC20Metadata} from "../../../../src/root/RootERC20Bridge.sol"; +import {RootAxelarBridgeAdaptor, IRootAxelarBridgeAdaptorEvents} from "../../../../src/root/RootAxelarBridgeAdaptor.sol"; +import {Utils} from "../../../utils.t.sol"; +import {WETH} from "../../../../src/test/root/WETH.sol"; + +contract RootERC20BridgeWithdrawIntegrationTest is Test, IRootERC20BridgeEvents, IRootAxelarBridgeAdaptorEvents, Utils { + address constant CHILD_BRIDGE = address(3); + address constant CHILD_BRIDGE_ADAPTOR = address(4); + string constant CHILD_CHAIN_NAME = "CHILD"; + address constant IMX_TOKEN_ADDRESS = address(0xccc); + address constant NATIVE_ETH = address(0xeee); + address constant WRAPPED_ETH = address(0xddd); + + uint256 constant withdrawAmount = 0.5 ether; + + ERC20PresetMinterPauser public token; + ERC20PresetMinterPauser public imxToken; + RootERC20Bridge public rootBridge; + RootAxelarBridgeAdaptor public axelarAdaptor; + MockAxelarGateway public mockAxelarGateway; + MockAxelarGasService public axelarGasService; + + function setUp() public { + deployCodeTo("WETH.sol", abi.encode("Wrapped ETH", "WETH"), WRAPPED_ETH); + + (imxToken, token, rootBridge, axelarAdaptor, mockAxelarGateway, axelarGasService) = + rootIntegrationSetup(CHILD_BRIDGE, CHILD_BRIDGE_ADAPTOR, CHILD_CHAIN_NAME, IMX_TOKEN_ADDRESS, WRAPPED_ETH); + + // Need to first map the token. + rootBridge.mapToken{value:1}(token); + // And give the bridge some tokens + token.transfer(address(rootBridge), 100 ether); + } + + function test_withdraw_TransfersTokens() public { + bytes memory data = abi.encode(WITHDRAW_SIG, address(token), address(this), address(this), withdrawAmount); + + bytes32 commandId = bytes32("testCommandId"); + string memory sourceAddress = rootBridge.childBridgeAdaptor(); + + uint256 thisPreBal = token.balanceOf(address(this)); + uint256 bridgePreBal = token.balanceOf(address(rootBridge)); + + axelarAdaptor.execute(commandId, CHILD_CHAIN_NAME, sourceAddress, data); + + uint256 thisPostBal = token.balanceOf(address(this)); + uint256 bridgePostBal = token.balanceOf(address(rootBridge)); + + assertEq(thisPostBal, thisPreBal + withdrawAmount, "Incorrect user balance after withdraw"); + assertEq(bridgePostBal, bridgePreBal - withdrawAmount, "Incorrect bridge balance after withdraw"); + } + + function test_withdraw_TransfersTokens_DifferentReceiver() public { + address receiver = address(987654321); + bytes memory data = abi.encode(WITHDRAW_SIG, address(token), address(this), receiver, withdrawAmount); + + bytes32 commandId = bytes32("testCommandId"); + string memory sourceAddress = rootBridge.childBridgeAdaptor(); + + uint256 receiverPreBal = token.balanceOf(receiver); + uint256 bridgePreBal = token.balanceOf(address(rootBridge)); + + axelarAdaptor.execute(commandId, CHILD_CHAIN_NAME, sourceAddress, data); + + uint256 receiverPostBal = token.balanceOf(receiver); + uint256 bridgePostBal = token.balanceOf(address(rootBridge)); + + assertEq(receiverPostBal, receiverPreBal + withdrawAmount, "Incorrect user balance after withdraw"); + assertEq(bridgePostBal, bridgePreBal - withdrawAmount, "Incorrect bridge balance after withdraw"); + } + + function test_withdraw_EmitsRootChainERC20WithdrawEvent() public { + bytes memory data = abi.encode(WITHDRAW_SIG, address(token), address(this), address(this), withdrawAmount); + + bytes32 commandId = bytes32("testCommandId"); + string memory sourceAddress = rootBridge.childBridgeAdaptor(); + + vm.expectEmit(); + emit RootChainERC20Withdraw(address(token), rootBridge.rootTokenToChildToken(address(token)), address(this), address(this), withdrawAmount); + axelarAdaptor.execute(commandId, CHILD_CHAIN_NAME, sourceAddress, data); + } +} diff --git a/test/unit/root/withdrawals/RootAxelarBridgeAdaptorWithdraw.t.sol b/test/unit/root/withdrawals/RootAxelarBridgeAdaptorWithdraw.t.sol new file mode 100644 index 000000000..e162c59f7 --- /dev/null +++ b/test/unit/root/withdrawals/RootAxelarBridgeAdaptorWithdraw.t.sol @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: Apache 2.0 +pragma solidity ^0.8.21; + +import {Test, console2} from "forge-std/Test.sol"; +import {ERC20PresetMinterPauser} from "@openzeppelin/contracts/token/ERC20/presets/ERC20PresetMinterPauser.sol"; +import {Strings} from "@openzeppelin/contracts/utils/Strings.sol"; +import {MockAxelarGateway} from "../../../../src/test/root/MockAxelarGateway.sol"; +import {MockAxelarGasService} from "../../../../src/test/root/MockAxelarGasService.sol"; +import { + RootAxelarBridgeAdaptor, + IRootAxelarBridgeAdaptorEvents, + IRootAxelarBridgeAdaptorErrors +} from "../../../../src/root/RootAxelarBridgeAdaptor.sol"; +import {StubRootBridge} from "../../../../src/test/root/StubRootBridge.sol"; + +contract RootAxelarBridgeWithdrawAdaptorTest is Test, IRootAxelarBridgeAdaptorEvents, IRootAxelarBridgeAdaptorErrors { + address constant CHILD_BRIDGE = address(3); + string public childBridgeAdaptor; + string constant CHILD_CHAIN_NAME = "test"; + bytes32 public constant MAP_TOKEN_SIG = keccak256("MAP_TOKEN"); + + ERC20PresetMinterPauser public token; + RootAxelarBridgeAdaptor public axelarAdaptor; + MockAxelarGateway public mockAxelarGateway; + MockAxelarGasService public axelarGasService; + StubRootBridge public stubRootBridge; + + function setUp() public { + token = new ERC20PresetMinterPauser("Test", "TST"); + mockAxelarGateway = new MockAxelarGateway(); + axelarGasService = new MockAxelarGasService(); + stubRootBridge = new StubRootBridge(); + childBridgeAdaptor = stubRootBridge.childBridgeAdaptor(); + + axelarAdaptor = new RootAxelarBridgeAdaptor(address(mockAxelarGateway)); + axelarAdaptor.initialize(address(stubRootBridge), CHILD_CHAIN_NAME, address(axelarGasService)); + vm.deal(address(stubRootBridge), 99999999999); + } + + + function test_execute_callsBridge() public { + bytes32 commandId = bytes32("testCommandId"); + string memory sourceChain = "test"; + string memory sourceAddress = Strings.toHexString(address(123)); + bytes memory payload = abi.encodePacked("payload"); + + // We expect to call the bridge's onMessageReceive function. + vm.expectCall( + address(stubRootBridge), + abi.encodeWithSelector(stubRootBridge.onMessageReceive.selector, sourceChain, sourceAddress, payload) + ); + axelarAdaptor.execute(commandId, sourceChain, sourceAddress, payload); + } +} diff --git a/test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol b/test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol new file mode 100644 index 000000000..ec33afda5 --- /dev/null +++ b/test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol @@ -0,0 +1,167 @@ +// SPDX-License-Identifier: Apache 2.0 +pragma solidity ^0.8.21; + +import {Test, console2} from "forge-std/Test.sol"; +import {ERC20PresetMinterPauser} from "@openzeppelin/contracts/token/ERC20/presets/ERC20PresetMinterPauser.sol"; +import {Strings} from "@openzeppelin/contracts/utils/Strings.sol"; +import { + RootERC20Bridge, + IRootERC20BridgeEvents, + IRootERC20BridgeErrors +} from "../../../../src/root/RootERC20Bridge.sol"; +import {MockAxelarGateway} from "../../../../src/test/root/MockAxelarGateway.sol"; +import {MockAxelarGasService} from "../../../../src/test/root/MockAxelarGasService.sol"; +import {MockAdaptor} from "../../../../src/test/root/MockAdaptor.sol"; +import {Utils} from "../../../utils.t.sol"; + +contract RootERC20BridgeWithdrawUnitTest is Test, IRootERC20BridgeEvents, IRootERC20BridgeErrors, Utils { + address constant CHILD_BRIDGE = address(3); + address constant CHILD_BRIDGE_ADAPTOR = address(4); + string CHILD_BRIDGE_ADAPTOR_STRING = Strings.toHexString(CHILD_BRIDGE_ADAPTOR); + string constant CHILD_CHAIN_NAME = "test"; + address constant IMX_TOKEN = address(0xccc); + address constant WRAPPED_ETH = address(0xddd); + uint256 constant mapTokenFee = 300; + uint256 constant withdrawAmount = 0.5 ether; + + ERC20PresetMinterPauser public token; + RootERC20Bridge public rootBridge; + MockAdaptor public mockAxelarAdaptor; + MockAxelarGateway public mockAxelarGateway; + MockAxelarGasService public axelarGasService; + + function setUp() public { + token = new ERC20PresetMinterPauser("Test", "TST"); + token.mint(address(this), 100 ether); + deployCodeTo("ERC20PresetMinterPauser.sol", abi.encode("ImmutableX", "IMX"), IMX_TOKEN); + + deployCodeTo("WETH.sol", abi.encode("Wrapped ETH", "WETH"), WRAPPED_ETH); + + rootBridge = new RootERC20Bridge(); + mockAxelarGateway = new MockAxelarGateway(); + axelarGasService = new MockAxelarGasService(); + + mockAxelarAdaptor = new MockAdaptor(); + + // The specific ERC20 token template does not matter for these unit tests + rootBridge.initialize( + address(mockAxelarAdaptor), + CHILD_BRIDGE, + CHILD_BRIDGE_ADAPTOR_STRING, + address(token), + IMX_TOKEN, + WRAPPED_ETH, + CHILD_CHAIN_NAME + ); + } + + function test_RevertsIf_WithdrawWithInvalidSender() public { + bytes memory data = abi.encode(WITHDRAW_SIG, IMX_TOKEN, address(this), address(this), withdrawAmount); + + vm.expectRevert(NotBridgeAdaptor.selector); + rootBridge.onMessageReceive(CHILD_CHAIN_NAME, CHILD_BRIDGE_ADAPTOR_STRING, data); + } + + function test_RevertsIf_OnMessageReceiveWithInvalidSourceChain() public { + bytes memory data = abi.encode(WITHDRAW_SIG, IMX_TOKEN, address(this), address(this), withdrawAmount); + + vm.prank(address(mockAxelarAdaptor)); + vm.expectRevert(InvalidSourceChain.selector); + rootBridge.onMessageReceive("ding_dong", CHILD_BRIDGE_ADAPTOR_STRING, data); + } + + function test_RevertsIf_OnMessageReceiveWithInvalidSourceAddress() public { + bytes memory data = abi.encode(WITHDRAW_SIG, IMX_TOKEN, address(this), address(this), withdrawAmount); + + console2.log(CHILD_CHAIN_NAME); + console2.log(rootBridge.childChain()); + vm.prank(address(mockAxelarAdaptor)); + vm.expectRevert(InvalidSourceAddress.selector); + rootBridge.onMessageReceive(CHILD_CHAIN_NAME, "DING_DONG", data); + } + + function test_RevertsIf_OnMessageReceiveWithZeroDataLength() public { + bytes memory data; + + vm.prank(address(mockAxelarAdaptor)); + vm.expectRevert(InvalidData.selector); + rootBridge.onMessageReceive(CHILD_CHAIN_NAME, CHILD_BRIDGE_ADAPTOR_STRING, data); + } + + function test_RevertsIf_OnMessageReceiveWithInvalidSignature() public { + bytes memory data = abi.encode(keccak256("RANDOM"), IMX_TOKEN, address(this), address(this), withdrawAmount); + vm.prank(address(mockAxelarAdaptor)); + vm.expectRevert(InvalidData.selector); + rootBridge.onMessageReceive(CHILD_CHAIN_NAME, CHILD_BRIDGE_ADAPTOR_STRING, data); + } + + function test_RevertsIf_OnMessageReceiveWithUnmappedToken() public { + bytes memory data = abi.encode(WITHDRAW_SIG, IMX_TOKEN, address(this), address(this), withdrawAmount); + vm.prank(address(mockAxelarAdaptor)); + vm.expectRevert(NotMapped.selector); + rootBridge.onMessageReceive(CHILD_CHAIN_NAME, CHILD_BRIDGE_ADAPTOR_STRING, data); + } + + function test_onMessageReceive_TransfersTokens() public { + // Need to first map the token. + rootBridge.mapToken(token); + // And give the bridge some tokens + token.transfer(address(rootBridge), 100 ether); + + uint256 thisPreBal = token.balanceOf(address(this)); + uint256 bridgePreBal = token.balanceOf(address(rootBridge)); + + bytes memory data = abi.encode(WITHDRAW_SIG, token, address(this), address(this), withdrawAmount); + vm.prank(address(mockAxelarAdaptor)); + rootBridge.onMessageReceive(CHILD_CHAIN_NAME, CHILD_BRIDGE_ADAPTOR_STRING, data); + + assertEq(token.balanceOf(address(this)), thisPreBal + withdrawAmount, "Tokens not transferred to receiver"); + assertEq(token.balanceOf(address(rootBridge)), bridgePreBal - withdrawAmount, "Tokens not transferred from bridge"); + } + + function test_onMessageReceive_TransfersTokens_DifferentReceiver() public { + address receiver = address(123456); + // Need to first map the token. + rootBridge.mapToken(token); + // And give the bridge some tokens + token.transfer(address(rootBridge), 100 ether); + + uint256 receiverPreBal = token.balanceOf(receiver); + uint256 bridgePreBal = token.balanceOf(address(rootBridge)); + + bytes memory data = abi.encode(WITHDRAW_SIG, token, address(this), receiver, withdrawAmount); + vm.prank(address(mockAxelarAdaptor)); + rootBridge.onMessageReceive(CHILD_CHAIN_NAME, CHILD_BRIDGE_ADAPTOR_STRING, data); + + assertEq(token.balanceOf(receiver), receiverPreBal + withdrawAmount, "Tokens not transferred to receiver"); + assertEq(token.balanceOf(address(rootBridge)), bridgePreBal - withdrawAmount, "Tokens not transferred from bridge"); + } + + function test_onMessageReceive_EmitsRootChainERC20WithdrawEvent() public { + // Need to first map the token. + rootBridge.mapToken(token); + // And give the bridge some tokens + token.transfer(address(rootBridge), 100 ether); + + bytes memory data = abi.encode(WITHDRAW_SIG, token, address(this), address(this), withdrawAmount); + vm.expectEmit(); + emit RootChainERC20Withdraw(address(token), rootBridge.rootTokenToChildToken(address(token)), address(this), address(this), withdrawAmount); + vm.prank(address(mockAxelarAdaptor)); + rootBridge.onMessageReceive(CHILD_CHAIN_NAME, CHILD_BRIDGE_ADAPTOR_STRING, data); + } + + function test_onMessageReceive_EmitsRootChainERC20WithdrawEvent_DifferentReceiver() public { + address receiver = address(123456); + // Need to first map the token. + rootBridge.mapToken(token); + // And give the bridge some tokens + token.transfer(address(rootBridge), 100 ether); + + bytes memory data = abi.encode(WITHDRAW_SIG, token, address(this), receiver, withdrawAmount); + vm.expectEmit(); + emit RootChainERC20Withdraw(address(token), rootBridge.rootTokenToChildToken(address(token)), address(this), receiver, withdrawAmount); + vm.prank(address(mockAxelarAdaptor)); + rootBridge.onMessageReceive(CHILD_CHAIN_NAME, CHILD_BRIDGE_ADAPTOR_STRING, data); + } + +} From 15e208b65d70bb4c5b050843dc6e0d3e29750edd Mon Sep 17 00:00:00 2001 From: Benjamin Patch Date: Fri, 3 Nov 2023 14:06:54 +1100 Subject: [PATCH 3/9] Integration tests complete --- src/test/root/StubRootBridge.sol | 1 + .../RootERC20BridgeWithdraw.t.sol | 83 +++++++++++++++++-- .../RootAxelarBridgeAdaptorWithdraw.t.sol | 1 - .../withdrawals/RootERC20BridgeWithdraw.t.sol | 25 ++++-- 4 files changed, 96 insertions(+), 14 deletions(-) diff --git a/src/test/root/StubRootBridge.sol b/src/test/root/StubRootBridge.sol index 25806be91..e8fa980c0 100644 --- a/src/test/root/StubRootBridge.sol +++ b/src/test/root/StubRootBridge.sol @@ -7,5 +7,6 @@ contract StubRootBridge { function childBridgeAdaptor() external pure returns (string memory) { return Strings.toHexString(address(9999)); } + function onMessageReceive(string calldata, string calldata, bytes calldata) external {} } diff --git a/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol b/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol index 68cf79d6d..8e54d0ed6 100644 --- a/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol +++ b/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol @@ -7,12 +7,25 @@ import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; import {Strings} from "@openzeppelin/contracts/utils/Strings.sol"; import {MockAxelarGateway} from "../../../../src/test/root/MockAxelarGateway.sol"; import {MockAxelarGasService} from "../../../../src/test/root/MockAxelarGasService.sol"; -import {RootERC20Bridge, IRootERC20BridgeEvents, IERC20Metadata} from "../../../../src/root/RootERC20Bridge.sol"; -import {RootAxelarBridgeAdaptor, IRootAxelarBridgeAdaptorEvents} from "../../../../src/root/RootAxelarBridgeAdaptor.sol"; +import { + RootERC20Bridge, + IRootERC20BridgeEvents, + IERC20Metadata, + IRootERC20BridgeErrors +} from "../../../../src/root/RootERC20Bridge.sol"; +import { + RootAxelarBridgeAdaptor, IRootAxelarBridgeAdaptorEvents +} from "../../../../src/root/RootAxelarBridgeAdaptor.sol"; import {Utils} from "../../../utils.t.sol"; import {WETH} from "../../../../src/test/root/WETH.sol"; -contract RootERC20BridgeWithdrawIntegrationTest is Test, IRootERC20BridgeEvents, IRootAxelarBridgeAdaptorEvents, Utils { +contract RootERC20BridgeWithdrawIntegrationTest is + Test, + IRootERC20BridgeErrors, + IRootERC20BridgeEvents, + IRootAxelarBridgeAdaptorEvents, + Utils +{ address constant CHILD_BRIDGE = address(3); address constant CHILD_BRIDGE_ADAPTOR = address(4); string constant CHILD_CHAIN_NAME = "CHILD"; @@ -36,11 +49,51 @@ contract RootERC20BridgeWithdrawIntegrationTest is Test, IRootERC20BridgeEvents, rootIntegrationSetup(CHILD_BRIDGE, CHILD_BRIDGE_ADAPTOR, CHILD_CHAIN_NAME, IMX_TOKEN_ADDRESS, WRAPPED_ETH); // Need to first map the token. - rootBridge.mapToken{value:1}(token); + rootBridge.mapToken{value: 1}(token); // And give the bridge some tokens token.transfer(address(rootBridge), 100 ether); } + function test_RevertsIf_WithdrawWithInvalidSourceAddress() public { + bytes memory data = abi.encode(WITHDRAW_SIG, address(token), address(this), address(this), withdrawAmount); + + bytes32 commandId = bytes32("testCommandId"); + string memory sourceAddress = rootBridge.childBridgeAdaptor(); + + vm.expectRevert(InvalidSourceChain.selector); + axelarAdaptor.execute(commandId, "INVALID", sourceAddress, data); + } + + function test_RevertsIf_WithdrawWithInvalidSourceChain() public { + bytes memory data = abi.encode(WITHDRAW_SIG, address(token), address(this), address(this), withdrawAmount); + + bytes32 commandId = bytes32("testCommandId"); + string memory sourceAddress = Strings.toHexString(address(123)); + + vm.expectRevert(InvalidSourceAddress.selector); + axelarAdaptor.execute(commandId, CHILD_CHAIN_NAME, sourceAddress, data); + } + + function test_RevertsIf_MessageWithEmptyData() public { + bytes memory data; + + bytes32 commandId = bytes32("testCommandId"); + string memory sourceAddress = rootBridge.childBridgeAdaptor(); + + vm.expectRevert(InvalidData.selector); + axelarAdaptor.execute(commandId, CHILD_CHAIN_NAME, sourceAddress, data); + } + + function test_RevertsIf_MessageWithInvalidSignature() public { + bytes memory data = abi.encode("INVALID_SIG", address(token), address(this), address(this), withdrawAmount); + + bytes32 commandId = bytes32("testCommandId"); + string memory sourceAddress = rootBridge.childBridgeAdaptor(); + + vm.expectRevert(InvalidData.selector); + axelarAdaptor.execute(commandId, CHILD_CHAIN_NAME, sourceAddress, data); + } + function test_withdraw_TransfersTokens() public { bytes memory data = abi.encode(WITHDRAW_SIG, address(token), address(this), address(this), withdrawAmount); @@ -85,7 +138,27 @@ contract RootERC20BridgeWithdrawIntegrationTest is Test, IRootERC20BridgeEvents, string memory sourceAddress = rootBridge.childBridgeAdaptor(); vm.expectEmit(); - emit RootChainERC20Withdraw(address(token), rootBridge.rootTokenToChildToken(address(token)), address(this), address(this), withdrawAmount); + emit RootChainERC20Withdraw( + address(token), + rootBridge.rootTokenToChildToken(address(token)), + address(this), + address(this), + withdrawAmount + ); + axelarAdaptor.execute(commandId, CHILD_CHAIN_NAME, sourceAddress, data); + } + + function test_withdraw_EmitsRootChainERC20WithdrawEvent_DifferentReceiver() public { + address receiver = address(987654321); + bytes memory data = abi.encode(WITHDRAW_SIG, address(token), address(this), receiver, withdrawAmount); + + bytes32 commandId = bytes32("testCommandId"); + string memory sourceAddress = rootBridge.childBridgeAdaptor(); + + vm.expectEmit(); + emit RootChainERC20Withdraw( + address(token), rootBridge.rootTokenToChildToken(address(token)), address(this), receiver, withdrawAmount + ); axelarAdaptor.execute(commandId, CHILD_CHAIN_NAME, sourceAddress, data); } } diff --git a/test/unit/root/withdrawals/RootAxelarBridgeAdaptorWithdraw.t.sol b/test/unit/root/withdrawals/RootAxelarBridgeAdaptorWithdraw.t.sol index e162c59f7..6b997c686 100644 --- a/test/unit/root/withdrawals/RootAxelarBridgeAdaptorWithdraw.t.sol +++ b/test/unit/root/withdrawals/RootAxelarBridgeAdaptorWithdraw.t.sol @@ -37,7 +37,6 @@ contract RootAxelarBridgeWithdrawAdaptorTest is Test, IRootAxelarBridgeAdaptorEv vm.deal(address(stubRootBridge), 99999999999); } - function test_execute_callsBridge() public { bytes32 commandId = bytes32("testCommandId"); string memory sourceChain = "test"; diff --git a/test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol b/test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol index ec33afda5..bc1c1cbac 100644 --- a/test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol +++ b/test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol @@ -5,9 +5,7 @@ import {Test, console2} from "forge-std/Test.sol"; import {ERC20PresetMinterPauser} from "@openzeppelin/contracts/token/ERC20/presets/ERC20PresetMinterPauser.sol"; import {Strings} from "@openzeppelin/contracts/utils/Strings.sol"; import { - RootERC20Bridge, - IRootERC20BridgeEvents, - IRootERC20BridgeErrors + RootERC20Bridge, IRootERC20BridgeEvents, IRootERC20BridgeErrors } from "../../../../src/root/RootERC20Bridge.sol"; import {MockAxelarGateway} from "../../../../src/test/root/MockAxelarGateway.sol"; import {MockAxelarGasService} from "../../../../src/test/root/MockAxelarGasService.sol"; @@ -116,7 +114,9 @@ contract RootERC20BridgeWithdrawUnitTest is Test, IRootERC20BridgeEvents, IRootE rootBridge.onMessageReceive(CHILD_CHAIN_NAME, CHILD_BRIDGE_ADAPTOR_STRING, data); assertEq(token.balanceOf(address(this)), thisPreBal + withdrawAmount, "Tokens not transferred to receiver"); - assertEq(token.balanceOf(address(rootBridge)), bridgePreBal - withdrawAmount, "Tokens not transferred from bridge"); + assertEq( + token.balanceOf(address(rootBridge)), bridgePreBal - withdrawAmount, "Tokens not transferred from bridge" + ); } function test_onMessageReceive_TransfersTokens_DifferentReceiver() public { @@ -134,7 +134,9 @@ contract RootERC20BridgeWithdrawUnitTest is Test, IRootERC20BridgeEvents, IRootE rootBridge.onMessageReceive(CHILD_CHAIN_NAME, CHILD_BRIDGE_ADAPTOR_STRING, data); assertEq(token.balanceOf(receiver), receiverPreBal + withdrawAmount, "Tokens not transferred to receiver"); - assertEq(token.balanceOf(address(rootBridge)), bridgePreBal - withdrawAmount, "Tokens not transferred from bridge"); + assertEq( + token.balanceOf(address(rootBridge)), bridgePreBal - withdrawAmount, "Tokens not transferred from bridge" + ); } function test_onMessageReceive_EmitsRootChainERC20WithdrawEvent() public { @@ -145,7 +147,13 @@ contract RootERC20BridgeWithdrawUnitTest is Test, IRootERC20BridgeEvents, IRootE bytes memory data = abi.encode(WITHDRAW_SIG, token, address(this), address(this), withdrawAmount); vm.expectEmit(); - emit RootChainERC20Withdraw(address(token), rootBridge.rootTokenToChildToken(address(token)), address(this), address(this), withdrawAmount); + emit RootChainERC20Withdraw( + address(token), + rootBridge.rootTokenToChildToken(address(token)), + address(this), + address(this), + withdrawAmount + ); vm.prank(address(mockAxelarAdaptor)); rootBridge.onMessageReceive(CHILD_CHAIN_NAME, CHILD_BRIDGE_ADAPTOR_STRING, data); } @@ -159,9 +167,10 @@ contract RootERC20BridgeWithdrawUnitTest is Test, IRootERC20BridgeEvents, IRootE bytes memory data = abi.encode(WITHDRAW_SIG, token, address(this), receiver, withdrawAmount); vm.expectEmit(); - emit RootChainERC20Withdraw(address(token), rootBridge.rootTokenToChildToken(address(token)), address(this), receiver, withdrawAmount); + emit RootChainERC20Withdraw( + address(token), rootBridge.rootTokenToChildToken(address(token)), address(this), receiver, withdrawAmount + ); vm.prank(address(mockAxelarAdaptor)); rootBridge.onMessageReceive(CHILD_CHAIN_NAME, CHILD_BRIDGE_ADAPTOR_STRING, data); } - } From dec0180bad7b2c4420eb08d18004c884c597bb78 Mon Sep 17 00:00:00 2001 From: Benjamin Patch Date: Mon, 6 Nov 2023 08:40:44 +1100 Subject: [PATCH 4/9] Add length <32 check to child bridge --- src/child/ChildERC20Bridge.sol | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/child/ChildERC20Bridge.sol b/src/child/ChildERC20Bridge.sol index 9a9e6212e..851de8249 100644 --- a/src/child/ChildERC20Bridge.sol +++ b/src/child/ChildERC20Bridge.sol @@ -109,11 +109,12 @@ contract ChildERC20Bridge is if (!Strings.equal(messageSourceChain, rootChain)) { revert InvalidSourceChain(); } - if (!Strings.equal(sourceAddress, rootERC20BridgeAdaptor)) { revert InvalidSourceAddress(); } - if (data.length == 0) { + if (data.length <= 32) { + // Data must always be greater than 32. + // 32 bytes for the signature, and at least some information for the payload revert InvalidData(); } From 1fc0d9770e83004ddec1b83f50882483b28ceb06 Mon Sep 17 00:00:00 2001 From: Benjamin Patch Date: Mon, 6 Nov 2023 08:43:29 +1100 Subject: [PATCH 5/9] Add checks and update tests --- src/child/ChildERC20Bridge.sol | 2 +- src/interfaces/child/IChildERC20Bridge.sol | 2 ++ src/interfaces/root/IRootERC20Bridge.sol | 2 ++ src/root/RootERC20Bridge.sol | 6 ++++-- test/integration/child/ChildAxelarBridge.t.sol | 2 +- .../root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol | 2 +- test/unit/child/ChildERC20Bridge.t.sol | 2 +- test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol | 2 +- 8 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/child/ChildERC20Bridge.sol b/src/child/ChildERC20Bridge.sol index 851de8249..9037215dd 100644 --- a/src/child/ChildERC20Bridge.sol +++ b/src/child/ChildERC20Bridge.sol @@ -115,7 +115,7 @@ contract ChildERC20Bridge is if (data.length <= 32) { // Data must always be greater than 32. // 32 bytes for the signature, and at least some information for the payload - revert InvalidData(); + revert DataTooShort(); } if (bytes32(data[:32]) == MAP_TOKEN_SIG) { diff --git a/src/interfaces/child/IChildERC20Bridge.sol b/src/interfaces/child/IChildERC20Bridge.sol index 087daa893..62d7cfe77 100644 --- a/src/interfaces/child/IChildERC20Bridge.sol +++ b/src/interfaces/child/IChildERC20Bridge.sol @@ -72,6 +72,8 @@ interface IChildERC20BridgeErrors { error AlreadyMapped(); /// @notice Error when a message is given to the bridge from an address not the designated bridge adaptor. error NotBridgeAdaptor(); + /// @notice Error when the message's payload is too short. + error DataTooShort(); /// @notice Error when the message's payload is not valid. error InvalidData(); /// @notice Error when the message's source chain is not valid. diff --git a/src/interfaces/root/IRootERC20Bridge.sol b/src/interfaces/root/IRootERC20Bridge.sol index 13bfb1e82..a0dd34dd9 100644 --- a/src/interfaces/root/IRootERC20Bridge.sol +++ b/src/interfaces/root/IRootERC20Bridge.sol @@ -105,6 +105,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 message's payload is too short. + error DataTooShort(); /// @notice Error when a message received has invalid data. error InvalidData(); /// @notice Error when a message received has invalid source address. diff --git a/src/root/RootERC20Bridge.sol b/src/root/RootERC20Bridge.sol index bf72034dd..b67a3ddda 100644 --- a/src/root/RootERC20Bridge.sol +++ b/src/root/RootERC20Bridge.sol @@ -164,8 +164,10 @@ contract RootERC20Bridge is if (!Strings.equal(sourceAddress, childBridgeAdaptor)) { revert InvalidSourceAddress(); } - if (data.length == 0) { - revert InvalidData(); + if (data.length <= 32) { + // Data must always be greater than 32. + // 32 bytes for the signature, and at least some information for the payload + revert DataTooShort(); } if (bytes32(data[:32]) == WITHDRAW_SIG) { diff --git a/test/integration/child/ChildAxelarBridge.t.sol b/test/integration/child/ChildAxelarBridge.t.sol index c73f41a68..cad11ba88 100644 --- a/test/integration/child/ChildAxelarBridge.t.sol +++ b/test/integration/child/ChildAxelarBridge.t.sol @@ -109,7 +109,7 @@ contract ChildERC20BridgeIntegrationTest is Test, IChildERC20BridgeEvents, IChil bytes32 commandId = bytes32("testCommandId"); bytes memory payload = ""; - vm.expectRevert(InvalidData.selector); + vm.expectRevert(DataTooShort.selector); childAxelarBridgeAdaptor.execute(commandId, ROOT_CHAIN_NAME, ROOT_ADAPTOR_ADDRESS, payload); } diff --git a/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol b/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol index 70b0d1400..319dc104f 100644 --- a/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol +++ b/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol @@ -87,7 +87,7 @@ contract RootERC20BridgeWithdrawIntegrationTest is bytes32 commandId = bytes32("testCommandId"); string memory sourceAddress = rootBridge.childBridgeAdaptor(); - vm.expectRevert(InvalidData.selector); + vm.expectRevert(DataTooShort.selector); axelarAdaptor.execute(commandId, CHILD_CHAIN_NAME, sourceAddress, data); } diff --git a/test/unit/child/ChildERC20Bridge.t.sol b/test/unit/child/ChildERC20Bridge.t.sol index 7bb37a263..95d72974f 100644 --- a/test/unit/child/ChildERC20Bridge.t.sol +++ b/test/unit/child/ChildERC20Bridge.t.sol @@ -168,7 +168,7 @@ contract ChildERC20BridgeUnitTest is Test, IChildERC20BridgeEvents, IChildERC20B function test_RevertIf_onMessageReceiveCalledWithDataLengthZero() public { bytes memory data = ""; - vm.expectRevert(InvalidData.selector); + vm.expectRevert(DataTooShort.selector); childBridge.onMessageReceive(ROOT_CHAIN_NAME, ROOT_BRIDGE_ADAPTOR, data); } diff --git a/test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol b/test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol index 5ea3c0e43..9fc1813da 100644 --- a/test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol +++ b/test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol @@ -84,7 +84,7 @@ contract RootERC20BridgeWithdrawUnitTest is Test, IRootERC20BridgeEvents, IRootE bytes memory data; vm.prank(address(mockAxelarAdaptor)); - vm.expectRevert(InvalidData.selector); + vm.expectRevert(DataTooShort.selector); rootBridge.onMessageReceive(CHILD_CHAIN_NAME, CHILD_BRIDGE_ADAPTOR_STRING, data); } From 2759eddcebb2d7f2186683872fb9a3540a63a778 Mon Sep 17 00:00:00 2001 From: Benjamin Patch Date: Tue, 7 Nov 2023 09:04:26 +1100 Subject: [PATCH 6/9] InvalidData with string reason --- src/child/ChildERC20Bridge.sol | 4 ++-- src/interfaces/child/IChildERC20Bridge.sol | 4 +--- src/interfaces/root/IRootERC20Bridge.sol | 4 +--- src/root/RootERC20Bridge.sol | 4 ++-- test/integration/child/ChildAxelarBridge.t.sol | 4 ++-- .../root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol | 4 ++-- test/unit/child/ChildERC20Bridge.t.sol | 4 ++-- test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol | 6 ++++-- 8 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/child/ChildERC20Bridge.sol b/src/child/ChildERC20Bridge.sol index 9037215dd..d4eae07a4 100644 --- a/src/child/ChildERC20Bridge.sol +++ b/src/child/ChildERC20Bridge.sol @@ -115,7 +115,7 @@ contract ChildERC20Bridge is if (data.length <= 32) { // Data must always be greater than 32. // 32 bytes for the signature, and at least some information for the payload - revert DataTooShort(); + revert InvalidData("Data too short"); } if (bytes32(data[:32]) == MAP_TOKEN_SIG) { @@ -123,7 +123,7 @@ contract ChildERC20Bridge is } else if (bytes32(data[:32]) == DEPOSIT_SIG) { _deposit(data[32:]); } else { - revert InvalidData(); + revert InvalidData("Unsupported action signature"); } } diff --git a/src/interfaces/child/IChildERC20Bridge.sol b/src/interfaces/child/IChildERC20Bridge.sol index 62d7cfe77..aaf4195d7 100644 --- a/src/interfaces/child/IChildERC20Bridge.sol +++ b/src/interfaces/child/IChildERC20Bridge.sol @@ -72,10 +72,8 @@ interface IChildERC20BridgeErrors { error AlreadyMapped(); /// @notice Error when a message is given to the bridge from an address not the designated bridge adaptor. error NotBridgeAdaptor(); - /// @notice Error when the message's payload is too short. - error DataTooShort(); /// @notice Error when the message's payload is not valid. - error InvalidData(); + error InvalidData(string reason); /// @notice Error when the message's source chain is not valid. error InvalidSourceChain(); /// @notice Error when the source chain's message sender is not a recognised address. diff --git a/src/interfaces/root/IRootERC20Bridge.sol b/src/interfaces/root/IRootERC20Bridge.sol index a0dd34dd9..51d235d05 100644 --- a/src/interfaces/root/IRootERC20Bridge.sol +++ b/src/interfaces/root/IRootERC20Bridge.sol @@ -105,10 +105,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 message's payload is too short. - error DataTooShort(); /// @notice Error when a message received has invalid data. - error InvalidData(); + error InvalidData(string reason); /// @notice Error when a message received has invalid source address. error InvalidSourceAddress(); /// @notice Error when a message received has invalid source chain. diff --git a/src/root/RootERC20Bridge.sol b/src/root/RootERC20Bridge.sol index b67a3ddda..266777a8a 100644 --- a/src/root/RootERC20Bridge.sol +++ b/src/root/RootERC20Bridge.sol @@ -167,13 +167,13 @@ contract RootERC20Bridge is if (data.length <= 32) { // Data must always be greater than 32. // 32 bytes for the signature, and at least some information for the payload - revert DataTooShort(); + revert InvalidData("Data too short"); } if (bytes32(data[:32]) == WITHDRAW_SIG) { _withdraw(data[32:]); } else { - revert InvalidData(); + revert InvalidData("Unsupported action signature"); } } diff --git a/test/integration/child/ChildAxelarBridge.t.sol b/test/integration/child/ChildAxelarBridge.t.sol index cad11ba88..bf8e0da28 100644 --- a/test/integration/child/ChildAxelarBridge.t.sol +++ b/test/integration/child/ChildAxelarBridge.t.sol @@ -83,7 +83,7 @@ contract ChildERC20BridgeIntegrationTest is Test, IChildERC20BridgeEvents, IChil bytes32 commandId = bytes32("testCommandId"); bytes memory payload = abi.encode("invalid payload"); - vm.expectRevert(InvalidData.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidData.selector, "Unsupported action signature")); childAxelarBridgeAdaptor.execute(commandId, ROOT_CHAIN_NAME, ROOT_ADAPTOR_ADDRESS, payload); } @@ -109,7 +109,7 @@ contract ChildERC20BridgeIntegrationTest is Test, IChildERC20BridgeEvents, IChil bytes32 commandId = bytes32("testCommandId"); bytes memory payload = ""; - vm.expectRevert(DataTooShort.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidData.selector, "Data too short")); childAxelarBridgeAdaptor.execute(commandId, ROOT_CHAIN_NAME, ROOT_ADAPTOR_ADDRESS, payload); } diff --git a/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol b/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol index 319dc104f..bedf6d979 100644 --- a/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol +++ b/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol @@ -87,7 +87,7 @@ contract RootERC20BridgeWithdrawIntegrationTest is bytes32 commandId = bytes32("testCommandId"); string memory sourceAddress = rootBridge.childBridgeAdaptor(); - vm.expectRevert(DataTooShort.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidData.selector, "Data too short")); axelarAdaptor.execute(commandId, CHILD_CHAIN_NAME, sourceAddress, data); } @@ -97,7 +97,7 @@ contract RootERC20BridgeWithdrawIntegrationTest is bytes32 commandId = bytes32("testCommandId"); string memory sourceAddress = rootBridge.childBridgeAdaptor(); - vm.expectRevert(InvalidData.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidData.selector, "Unsupported action signature")); axelarAdaptor.execute(commandId, CHILD_CHAIN_NAME, sourceAddress, data); } diff --git a/test/unit/child/ChildERC20Bridge.t.sol b/test/unit/child/ChildERC20Bridge.t.sol index 95d72974f..72694bcac 100644 --- a/test/unit/child/ChildERC20Bridge.t.sol +++ b/test/unit/child/ChildERC20Bridge.t.sol @@ -168,7 +168,7 @@ contract ChildERC20BridgeUnitTest is Test, IChildERC20BridgeEvents, IChildERC20B function test_RevertIf_onMessageReceiveCalledWithDataLengthZero() public { bytes memory data = ""; - vm.expectRevert(DataTooShort.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidData.selector, "Data too short")); childBridge.onMessageReceive(ROOT_CHAIN_NAME, ROOT_BRIDGE_ADAPTOR, data); } @@ -176,7 +176,7 @@ contract ChildERC20BridgeUnitTest is Test, IChildERC20BridgeEvents, IChildERC20B bytes memory data = abi.encode("FAKEDATA", address(rootToken), rootToken.name(), rootToken.symbol(), rootToken.decimals()); - vm.expectRevert(InvalidData.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidData.selector, "Unsupported action signature")); childBridge.onMessageReceive(ROOT_CHAIN_NAME, ROOT_BRIDGE_ADAPTOR, data); } diff --git a/test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol b/test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol index 9fc1813da..be1999cdc 100644 --- a/test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol +++ b/test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol @@ -84,14 +84,16 @@ contract RootERC20BridgeWithdrawUnitTest is Test, IRootERC20BridgeEvents, IRootE bytes memory data; vm.prank(address(mockAxelarAdaptor)); - vm.expectRevert(DataTooShort.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidData.selector, "Data too short")); + rootBridge.onMessageReceive(CHILD_CHAIN_NAME, CHILD_BRIDGE_ADAPTOR_STRING, data); } function test_RevertsIf_OnMessageReceiveWithInvalidSignature() public { bytes memory data = abi.encode(keccak256("RANDOM"), IMX_TOKEN, address(this), address(this), withdrawAmount); + vm.prank(address(mockAxelarAdaptor)); - vm.expectRevert(InvalidData.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidData.selector, "Unsupported action signature")); rootBridge.onMessageReceive(CHILD_CHAIN_NAME, CHILD_BRIDGE_ADAPTOR_STRING, data); } From 3ae15f5c0ae103f22d4771c38f376a0c0f510ddd Mon Sep 17 00:00:00 2001 From: Benjimmutable <123430337+Benjimmutable@users.noreply.github.com> Date: Tue, 7 Nov 2023 09:16:34 +1100 Subject: [PATCH 7/9] Update src/root/RootERC20Bridge.sol Co-authored-by: Ermyas Abebe --- src/root/RootERC20Bridge.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/root/RootERC20Bridge.sol b/src/root/RootERC20Bridge.sol index 266777a8a..132fda725 100644 --- a/src/root/RootERC20Bridge.sol +++ b/src/root/RootERC20Bridge.sol @@ -376,6 +376,6 @@ contract RootERC20Bridge is IERC20Metadata(rootToken).safeTransfer(receiver, amount); } // slither-disable-next-line reentrancy-events - emit RootChainERC20Withdraw(address(rootToken), childToken, withdrawer, receiver, amount); + emit RootChainERC20Withdraw(rootToken, childToken, withdrawer, receiver, amount); } } From ad5a3d84c967330952eff279bcafc840b46a84ea Mon Sep 17 00:00:00 2001 From: Benjamin Patch Date: Tue, 7 Nov 2023 09:27:39 +1100 Subject: [PATCH 8/9] Add event to _execute --- src/child/ChildAxelarBridgeAdaptor.sol | 1 + .../child/IChildAxelarBridgeAdaptor.sol | 2 ++ .../root/IRootAxelarBridgeAdaptor.sol | 2 ++ src/root/RootAxelarBridgeAdaptor.sol | 1 + .../RootERC20BridgeWithdraw.t.sol | 4 +-- .../unit/child/ChildAxelarBridgeAdaptor.t.sol | 14 +++++++++- test/unit/root/RootAxelarBridgeAdaptor.t.sol | 27 +++++++++++++++++++ 7 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/child/ChildAxelarBridgeAdaptor.sol b/src/child/ChildAxelarBridgeAdaptor.sol index cb80d2456..48dbad40a 100644 --- a/src/child/ChildAxelarBridgeAdaptor.sol +++ b/src/child/ChildAxelarBridgeAdaptor.sol @@ -71,6 +71,7 @@ contract ChildAxelarBridgeAdaptor is internal override { + emit AdaptorExecute(sourceChain_, sourceAddress_, payload_); childBridge.onMessageReceive(sourceChain_, sourceAddress_, payload_); } } diff --git a/src/interfaces/child/IChildAxelarBridgeAdaptor.sol b/src/interfaces/child/IChildAxelarBridgeAdaptor.sol index f7cd63330..ae86f45a1 100644 --- a/src/interfaces/child/IChildAxelarBridgeAdaptor.sol +++ b/src/interfaces/child/IChildAxelarBridgeAdaptor.sol @@ -13,4 +13,6 @@ interface IChildAxelarBridgeAdaptorErrors { interface IChildAxelarBridgeAdaptorEvents { /// @notice Emitted when an Axelar message is sent to the root chain. event AxelarMessage(string indexed rootChain, string indexed rootBridgeAdaptor, bytes indexed payload); + /// @notice Emitted when an Axelar message is received from the root chain. + event AdaptorExecute(string sourceChain, string sourceAddress_, bytes payload_); } diff --git a/src/interfaces/root/IRootAxelarBridgeAdaptor.sol b/src/interfaces/root/IRootAxelarBridgeAdaptor.sol index d134c9410..1876f24df 100644 --- a/src/interfaces/root/IRootAxelarBridgeAdaptor.sol +++ b/src/interfaces/root/IRootAxelarBridgeAdaptor.sol @@ -15,4 +15,6 @@ interface IRootAxelarBridgeAdaptorErrors { interface IRootAxelarBridgeAdaptorEvents { /// @notice Emitted when an Axelar message is sent to the child chain. event AxelarMessage(string indexed childChain, string indexed childBridgeAdaptor, bytes indexed payload); + /// @notice Emitted when an Axelar message is received from the child chain. + event AdaptorExecute(string sourceChain, string sourceAddress_, bytes payload_); } diff --git a/src/root/RootAxelarBridgeAdaptor.sol b/src/root/RootAxelarBridgeAdaptor.sol index 7c9d3877c..4460ec19a 100644 --- a/src/root/RootAxelarBridgeAdaptor.sol +++ b/src/root/RootAxelarBridgeAdaptor.sol @@ -88,6 +88,7 @@ contract RootAxelarBridgeAdaptor is internal override { + emit AdaptorExecute(sourceChain_, sourceAddress_, payload_); rootBridge.onMessageReceive(sourceChain_, sourceAddress_, payload_); } } diff --git a/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol b/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol index 70b0d1400..cf306eb07 100644 --- a/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol +++ b/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol @@ -61,7 +61,7 @@ contract RootERC20BridgeWithdrawIntegrationTest is token.transfer(address(rootBridge), 100 ether); } - function test_RevertsIf_WithdrawWithInvalidSourceAddress() public { + function test_RevertsIf_WithdrawWithInvalidSourceChain() public { bytes memory data = abi.encode(WITHDRAW_SIG, address(token), address(this), address(this), withdrawAmount); bytes32 commandId = bytes32("testCommandId"); @@ -71,7 +71,7 @@ contract RootERC20BridgeWithdrawIntegrationTest is axelarAdaptor.execute(commandId, "INVALID", sourceAddress, data); } - function test_RevertsIf_WithdrawWithInvalidSourceChain() public { + function test_RevertsIf_WithdrawWithInvalidSourceAddress() public { bytes memory data = abi.encode(WITHDRAW_SIG, address(token), address(this), address(this), withdrawAmount); bytes32 commandId = bytes32("testCommandId"); diff --git a/test/unit/child/ChildAxelarBridgeAdaptor.t.sol b/test/unit/child/ChildAxelarBridgeAdaptor.t.sol index 5cdf8e049..40bcfad2b 100644 --- a/test/unit/child/ChildAxelarBridgeAdaptor.t.sol +++ b/test/unit/child/ChildAxelarBridgeAdaptor.t.sol @@ -47,7 +47,7 @@ contract ChildAxelarBridgeAdaptorUnitTest is Test, IChildAxelarBridgeAdaptorErro newAdaptor.initialize("root", address(0), address(mockChildAxelarGasService)); } - function test_Execute() public { + function test_Execute_CallsBridge() public { bytes32 commandId = bytes32("testCommandId"); string memory sourceChain = "test"; string memory sourceAddress = Strings.toHexString(address(123)); @@ -61,6 +61,18 @@ contract ChildAxelarBridgeAdaptorUnitTest is Test, IChildAxelarBridgeAdaptorErro axelarAdaptor.execute(commandId, sourceChain, sourceAddress, payload); } + function test_Execute_EmitsAdaptorExecuteEvent() public { + bytes32 commandId = bytes32("testCommandId"); + string memory sourceChain = "test"; + string memory sourceAddress = Strings.toHexString(address(123)); + bytes memory payload = abi.encodePacked("payload"); + + // We expect to call the bridge's onMessageReceive function. + vm.expectEmit(); + emit AdaptorExecute(sourceChain, sourceAddress, payload); + axelarAdaptor.execute(commandId, sourceChain, sourceAddress, payload); + } + function test_sendMessage_CallsGasService() public { address refundRecipient = address(123); bytes memory payload = abi.encode(WITHDRAW_SIG, address(token), address(this), address(999), 11111); diff --git a/test/unit/root/RootAxelarBridgeAdaptor.t.sol b/test/unit/root/RootAxelarBridgeAdaptor.t.sol index 5bd34aa86..6323008a6 100644 --- a/test/unit/root/RootAxelarBridgeAdaptor.t.sol +++ b/test/unit/root/RootAxelarBridgeAdaptor.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.21; import {Test, console2} from "forge-std/Test.sol"; import {ERC20PresetMinterPauser} from "@openzeppelin/contracts/token/ERC20/presets/ERC20PresetMinterPauser.sol"; +import {Strings} from "@openzeppelin/contracts/utils/Strings.sol"; import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; import {MockAxelarGateway} from "../../../src/test/root/MockAxelarGateway.sol"; import {MockAxelarGasService} from "../../../src/test/root/MockAxelarGasService.sol"; @@ -56,6 +57,32 @@ contract RootAxelarBridgeAdaptorTest is Test, IRootAxelarBridgeAdaptorEvents, IR newAdaptor.initialize(address(this), "", address(axelarGasService)); } + function test_Execute_CallsBridge() public { + bytes32 commandId = bytes32("testCommandId"); + string memory sourceChain = "test"; + string memory sourceAddress = Strings.toHexString(address(123)); + bytes memory payload = abi.encodePacked("payload"); + + // We expect to call the bridge's onMessageReceive function. + vm.expectCall( + address(stubRootBridge), + abi.encodeWithSelector(stubRootBridge.onMessageReceive.selector, sourceChain, sourceAddress, payload) + ); + axelarAdaptor.execute(commandId, sourceChain, sourceAddress, payload); + } + + function test_Execute_EmitsAdaptorExecuteEvent() public { + bytes32 commandId = bytes32("testCommandId"); + string memory sourceChain = "test"; + string memory sourceAddress = Strings.toHexString(address(123)); + bytes memory payload = abi.encodePacked("payload"); + + // We expect to call the bridge's onMessageReceive function. + vm.expectEmit(); + emit AdaptorExecute(sourceChain, sourceAddress, payload); + axelarAdaptor.execute(commandId, sourceChain, sourceAddress, payload); + } + /// @dev For this unit test we just want to make sure the correct functions are called on the Axelar Gateway and Gas Service. function test_sendMessage_CallsGasService() public { address refundRecipient = address(123); From 7da7c83fed983f4ab61408d8644c4335ab400500 Mon Sep 17 00:00:00 2001 From: Benjamin Patch Date: Tue, 7 Nov 2023 09:29:26 +1100 Subject: [PATCH 9/9] AxelarMessageSent --- src/child/ChildAxelarBridgeAdaptor.sol | 2 +- src/interfaces/child/IChildAxelarBridgeAdaptor.sol | 2 +- src/interfaces/root/IRootAxelarBridgeAdaptor.sol | 2 +- src/root/RootAxelarBridgeAdaptor.sol | 2 +- .../withdrawals/ChildAxelarBridgeWithdraw.t.sol | 4 ++-- test/integration/root/RootERC20Bridge.t.sol | 12 ++++++------ test/unit/child/ChildAxelarBridgeAdaptor.t.sol | 4 ++-- test/unit/root/RootAxelarBridgeAdaptor.t.sol | 4 ++-- 8 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/child/ChildAxelarBridgeAdaptor.sol b/src/child/ChildAxelarBridgeAdaptor.sol index 48dbad40a..5e725257e 100644 --- a/src/child/ChildAxelarBridgeAdaptor.sol +++ b/src/child/ChildAxelarBridgeAdaptor.sol @@ -60,7 +60,7 @@ contract ChildAxelarBridgeAdaptor is ); gateway.callContract(_rootChain, _rootBridgeAdaptor, payload); - emit AxelarMessage(_rootChain, _rootBridgeAdaptor, payload); + emit AxelarMessageSent(_rootChain, _rootBridgeAdaptor, payload); } /** diff --git a/src/interfaces/child/IChildAxelarBridgeAdaptor.sol b/src/interfaces/child/IChildAxelarBridgeAdaptor.sol index ae86f45a1..cca42edcf 100644 --- a/src/interfaces/child/IChildAxelarBridgeAdaptor.sol +++ b/src/interfaces/child/IChildAxelarBridgeAdaptor.sol @@ -12,7 +12,7 @@ interface IChildAxelarBridgeAdaptorErrors { interface IChildAxelarBridgeAdaptorEvents { /// @notice Emitted when an Axelar message is sent to the root chain. - event AxelarMessage(string indexed rootChain, string indexed rootBridgeAdaptor, bytes indexed payload); + event AxelarMessageSent(string indexed rootChain, string indexed rootBridgeAdaptor, bytes indexed payload); /// @notice Emitted when an Axelar message is received from the root chain. event AdaptorExecute(string sourceChain, string sourceAddress_, bytes payload_); } diff --git a/src/interfaces/root/IRootAxelarBridgeAdaptor.sol b/src/interfaces/root/IRootAxelarBridgeAdaptor.sol index 1876f24df..e1cddac17 100644 --- a/src/interfaces/root/IRootAxelarBridgeAdaptor.sol +++ b/src/interfaces/root/IRootAxelarBridgeAdaptor.sol @@ -14,7 +14,7 @@ interface IRootAxelarBridgeAdaptorErrors { interface IRootAxelarBridgeAdaptorEvents { /// @notice Emitted when an Axelar message is sent to the child chain. - event AxelarMessage(string indexed childChain, string indexed childBridgeAdaptor, bytes indexed payload); + event AxelarMessageSent(string indexed childChain, string indexed childBridgeAdaptor, bytes indexed payload); /// @notice Emitted when an Axelar message is received from the child chain. event AdaptorExecute(string sourceChain, string sourceAddress_, bytes payload_); } diff --git a/src/root/RootAxelarBridgeAdaptor.sol b/src/root/RootAxelarBridgeAdaptor.sol index 4460ec19a..e603dadba 100644 --- a/src/root/RootAxelarBridgeAdaptor.sol +++ b/src/root/RootAxelarBridgeAdaptor.sol @@ -78,7 +78,7 @@ contract RootAxelarBridgeAdaptor is ); gateway.callContract(_childChain, _childBridgeAdaptor, payload); - emit AxelarMessage(_childChain, _childBridgeAdaptor, payload); + emit AxelarMessageSent(_childChain, _childBridgeAdaptor, payload); } /** diff --git a/test/integration/child/withdrawals/ChildAxelarBridgeWithdraw.t.sol b/test/integration/child/withdrawals/ChildAxelarBridgeWithdraw.t.sol index e52abd117..5b098ac91 100644 --- a/test/integration/child/withdrawals/ChildAxelarBridgeWithdraw.t.sol +++ b/test/integration/child/withdrawals/ChildAxelarBridgeWithdraw.t.sol @@ -106,14 +106,14 @@ contract ChildERC20BridgeWithdrawIntegrationTest is childBridge.withdraw{value: withdrawFee}(childToken, withdrawAmount); } - function test_withdraw_emits_AxelarMessageEvent() public { + function test_withdraw_emits_AxelarMessageSentEvent() public { ChildERC20 childToken = ChildERC20(childBridge.rootTokenToChildToken(rootToken)); bytes memory predictedPayload = abi.encode(WITHDRAW_SIG, rootToken, address(this), address(this), withdrawAmount); vm.expectEmit(address(axelarAdaptor)); - emit AxelarMessage(childBridge.rootChain(), childBridge.rootERC20BridgeAdaptor(), predictedPayload); + emit AxelarMessageSent(childBridge.rootChain(), childBridge.rootERC20BridgeAdaptor(), predictedPayload); childBridge.withdraw{value: withdrawFee}(childToken, withdrawAmount); } diff --git a/test/integration/root/RootERC20Bridge.t.sol b/test/integration/root/RootERC20Bridge.t.sol index 9728d5959..3a767d299 100644 --- a/test/integration/root/RootERC20Bridge.t.sol +++ b/test/integration/root/RootERC20Bridge.t.sol @@ -51,7 +51,7 @@ contract RootERC20BridgeIntegrationTest is Test, IRootERC20BridgeEvents, IRootAx 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); + emit AxelarMessageSent(CHILD_CHAIN_NAME, Strings.toHexString(CHILD_BRIDGE_ADAPTOR), payload); vm.expectEmit(true, true, false, false, address(rootBridge)); emit L1TokenMapped(address(token), childToken); @@ -113,7 +113,7 @@ contract RootERC20BridgeIntegrationTest is Test, IRootERC20BridgeEvents, IRootAx console2.logBytes(predictedPayload); vm.expectEmit(address(axelarAdaptor)); - emit AxelarMessage(CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload); + emit AxelarMessageSent(CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload); vm.expectEmit(address(rootBridge)); emit NativeEthDeposit( address(NATIVE_ETH), rootBridge.childETHToken(), address(this), address(this), tokenAmount @@ -169,7 +169,7 @@ contract RootERC20BridgeIntegrationTest is Test, IRootERC20BridgeEvents, IRootAx setupDeposit(IMX_TOKEN_ADDRESS, rootBridge, mapTokenFee, depositFee, tokenAmount, false); vm.expectEmit(address(axelarAdaptor)); - emit AxelarMessage(CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload); + emit AxelarMessageSent(CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload); vm.expectEmit(address(rootBridge)); emit IMXDeposit(address(IMX_TOKEN_ADDRESS), address(this), address(this), tokenAmount); @@ -226,7 +226,7 @@ contract RootERC20BridgeIntegrationTest is Test, IRootERC20BridgeEvents, IRootAx setupDeposit(WRAPPED_ETH, rootBridge, mapTokenFee, depositFee, tokenAmount, false); vm.expectEmit(address(axelarAdaptor)); - emit AxelarMessage(CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload); + emit AxelarMessageSent(CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload); vm.expectEmit(address(rootBridge)); emit WETHDeposit(address(WRAPPED_ETH), rootBridge.childETHToken(), address(this), address(this), tokenAmount); vm.expectCall( @@ -284,7 +284,7 @@ contract RootERC20BridgeIntegrationTest is Test, IRootERC20BridgeEvents, IRootAx setupDeposit(address(token), rootBridge, mapTokenFee, depositFee, tokenAmount, true); vm.expectEmit(address(axelarAdaptor)); - emit AxelarMessage(CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload); + emit AxelarMessageSent(CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload); vm.expectEmit(address(rootBridge)); emit ChildChainERC20Deposit(address(token), childToken, address(this), address(this), tokenAmount); @@ -340,7 +340,7 @@ contract RootERC20BridgeIntegrationTest is Test, IRootERC20BridgeEvents, IRootAx setupDepositTo(address(token), rootBridge, mapTokenFee, depositFee, tokenAmount, recipient, true); vm.expectEmit(address(axelarAdaptor)); - emit AxelarMessage(CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload); + emit AxelarMessageSent(CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload); vm.expectEmit(address(rootBridge)); emit ChildChainERC20Deposit(address(token), childToken, address(this), recipient, tokenAmount); diff --git a/test/unit/child/ChildAxelarBridgeAdaptor.t.sol b/test/unit/child/ChildAxelarBridgeAdaptor.t.sol index 40bcfad2b..148445c60 100644 --- a/test/unit/child/ChildAxelarBridgeAdaptor.t.sol +++ b/test/unit/child/ChildAxelarBridgeAdaptor.t.sol @@ -115,12 +115,12 @@ contract ChildAxelarBridgeAdaptorUnitTest is Test, IChildAxelarBridgeAdaptorErro axelarAdaptor.sendMessage{value: callValue}(payload, address(123)); } - function test_sendMessage_EmitsAxelarMessageEvent() public { + function test_sendMessage_EmitsAxelarMessageSentEvent() public { bytes memory payload = abi.encode(WITHDRAW_SIG, address(token), address(this), address(999), 11111); uint256 callValue = 300; vm.expectEmit(); - emit AxelarMessage(ROOT_CHAIN_NAME, mockChildERC20Bridge.rootERC20BridgeAdaptor(), payload); + emit AxelarMessageSent(ROOT_CHAIN_NAME, mockChildERC20Bridge.rootERC20BridgeAdaptor(), payload); vm.deal(address(mockChildERC20Bridge), callValue); vm.prank(address(mockChildERC20Bridge)); diff --git a/test/unit/root/RootAxelarBridgeAdaptor.t.sol b/test/unit/root/RootAxelarBridgeAdaptor.t.sol index 6323008a6..76281fce0 100644 --- a/test/unit/root/RootAxelarBridgeAdaptor.t.sol +++ b/test/unit/root/RootAxelarBridgeAdaptor.t.sol @@ -121,12 +121,12 @@ contract RootAxelarBridgeAdaptorTest is Test, IRootAxelarBridgeAdaptorEvents, IR axelarAdaptor.sendMessage{value: callValue}(payload, address(123)); } - function test_sendMessage_EmitsAxelarMessageEvent() public { + function test_sendMessage_EmitsAxelarMessageSentEvent() public { bytes memory payload = abi.encode(MAP_TOKEN_SIG, address(token), token.name(), token.symbol(), token.decimals()); uint256 callValue = 300; vm.expectEmit(true, true, true, false, address(axelarAdaptor)); - emit AxelarMessage(CHILD_CHAIN_NAME, childBridgeAdaptor, payload); + emit AxelarMessageSent(CHILD_CHAIN_NAME, childBridgeAdaptor, payload); vm.prank(address(stubRootBridge)); axelarAdaptor.sendMessage{value: callValue}(payload, address(123)); }