From ea4dcdc159fe7ddd4ccc146fd561832299633df1 Mon Sep 17 00:00:00 2001 From: Ermyas Abebe Date: Wed, 25 Oct 2023 16:16:43 +1100 Subject: [PATCH] Fix failing test --- .gitignore | 3 +- script/InitializeRootContracts.s.sol | 9 ++++- src/interfaces/root/IERC20.sol | 6 +-- src/root/RootERC20Bridge.sol | 31 ++++++++------- test/integration/root/RootERC20Bridge.t.sol | 5 +-- test/unit/root/RootERC20Bridge.t.sol | 42 +++++++++++---------- test/utils.t.sol | 6 ++- 7 files changed, 53 insertions(+), 49 deletions(-) diff --git a/.gitignore b/.gitignore index dc2e9557..f7152630 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ # Compiler files cache/ out/ +.idea/ # Ignores development broadcast logs !/broadcast @@ -22,4 +23,4 @@ docs/ # Contract addresses addresses.json -/node_modules \ No newline at end of file +/node_modules diff --git a/script/InitializeRootContracts.s.sol b/script/InitializeRootContracts.s.sol index 5c01698a..0a61d1c8 100644 --- a/script/InitializeRootContracts.s.sol +++ b/script/InitializeRootContracts.s.sol @@ -14,7 +14,7 @@ import {Strings} from "@openzeppelin/contracts/utils/Strings.sol"; contract InitializeRootContracts is Script { function run() public { - RootERC20Bridge rootERC20Bridge = RootERC20Bridge(vm.envAddress("ROOT_ERC20_BRIDGE")); + RootERC20Bridge rootERC20Bridge = RootERC20Bridge(payable(vm.envAddress("ROOT_ERC20_BRIDGE"))); RootAxelarBridgeAdaptor rootBridgeAdaptor = RootAxelarBridgeAdaptor(vm.envAddress("ROOT_BRIDGE_ADAPTOR")); address rootChainChildTokenTemplate = vm.envAddress("ROOTCHAIN_CHILD_TOKEN_TEMPLATE"); address childBridgeAdaptor = vm.envAddress("CHILD_BRIDGE_ADAPTOR"); @@ -31,7 +31,12 @@ contract InitializeRootContracts is Script { vm.startBroadcast(rootPrivateKey); rootERC20Bridge.initialize( - address(rootBridgeAdaptor), childERC20Bridge, childBridgeAdaptor, rootChainChildTokenTemplate, rootIMXToken, rootWETHToken + address(rootBridgeAdaptor), + childERC20Bridge, + childBridgeAdaptor, + rootChainChildTokenTemplate, + rootIMXToken, + rootWETHToken ); rootBridgeAdaptor.setChildBridgeAdaptor(); diff --git a/src/interfaces/root/IERC20.sol b/src/interfaces/root/IERC20.sol index 3d0aa3bc..1d05ecf7 100644 --- a/src/interfaces/root/IERC20.sol +++ b/src/interfaces/root/IERC20.sol @@ -61,11 +61,7 @@ interface IERC20 { * * Emits a {Transfer} event. */ - function transferFrom( - address sender, - address recipient, - uint256 amount - ) external returns (bool); + function transferFrom(address sender, address recipient, uint256 amount) external returns (bool); /** * @dev Emitted when `value` tokens are moved from one account (`from`) to diff --git a/src/root/RootERC20Bridge.sol b/src/root/RootERC20Bridge.sol index d7f794a7..a208f534 100644 --- a/src/root/RootERC20Bridge.sol +++ b/src/root/RootERC20Bridge.sol @@ -13,8 +13,7 @@ import {IRootERC20BridgeEvents, IRootERC20BridgeErrors} from "../interfaces/root import {IRootERC20BridgeAdaptor} from "../interfaces/root/IRootERC20BridgeAdaptor.sol"; import {IChildERC20} from "../interfaces/child/IChildERC20.sol"; import {IWETH} from "../interfaces/root/IWETH.sol"; -import {console2} from "forge-std/Test.sol"; - +import "forge-std/console.sol"; /** * @notice RootERC20Bridge is a bridge that allows ERC20 tokens to be transferred from the root chain to the child chain. * @dev This contract is designed to be upgradeable. @@ -23,6 +22,7 @@ import {console2} from "forge-std/Test.sol"; * @dev Because of this pattern, any checks or logic that is agnostic to the messaging protocol should be done in RootERC20Bridge. * @dev Any checks or logic that is specific to the underlying messaging protocol should be done in the bridge adaptor. */ + contract RootERC20Bridge is Ownable2Step, Initializable, @@ -101,15 +101,17 @@ contract RootERC20Bridge is } function depositETH(uint256 amount) external payable { - //override removed? _depositETH(msg.sender, amount); } function depositToETH(address receiver, uint256 amount) external payable { - //override removed? _depositETH(receiver, amount); } + function _depositUnwrappedETH(address receiver, uint256 amount) private { + _deposit(IERC20Metadata(NATIVE_ETH), receiver, amount, msg.value); + } + function _depositETH(address receiver, uint256 amount) private { if (msg.value < amount) { revert InsufficientValue(); @@ -117,7 +119,7 @@ contract RootERC20Bridge is uint256 expectedBalance = address(this).balance - (msg.value - amount); - _deposit(IERC20Metadata(NATIVE_ETH), receiver, amount); + _deposit(IERC20Metadata(NATIVE_ETH), receiver, amount, msg.value - amount); // invariant check to ensure that the root native balance has increased by the amount deposited if (address(this).balance != expectedBalance) { @@ -143,20 +145,20 @@ contract RootERC20Bridge is * @inheritdoc IRootERC20Bridge */ function deposit(IERC20Metadata rootToken, uint256 amount) external payable override { - _depositWETHorERC20(rootToken, msg.sender, amount); + _depositToken(rootToken, msg.sender, amount); } /** * @inheritdoc IRootERC20Bridge */ function depositTo(IERC20Metadata rootToken, address receiver, uint256 amount) external payable override { - _depositWETHorERC20(rootToken, receiver, amount); + _depositToken(rootToken, receiver, amount); } - function _depositWETHorERC20(IERC20Metadata rootToken, address receiver, uint256 amount) private { + function _depositToken(IERC20Metadata rootToken, address receiver, uint256 amount) private { if (address(rootToken) == rootWETHToken) { _unwrapWETH(amount); - _depositETH(receiver, amount); + _depositUnwrappedETH(receiver, amount); } else { _depositERC20(rootToken, receiver, amount); } @@ -164,7 +166,7 @@ contract RootERC20Bridge is function _depositERC20(IERC20Metadata rootToken, address receiver, uint256 amount) private { uint256 expectedBalance = rootToken.balanceOf(address(this)) + amount; - _deposit(rootToken, receiver, amount); + _deposit(rootToken, receiver, amount, msg.value); // invariant check to ensure that the root token balance has increased by the amount deposited // slither-disable-next-line incorrect-equality if (rootToken.balanceOf(address(this)) != expectedBalance) { @@ -208,17 +210,15 @@ contract RootERC20Bridge is return childToken; } - function _deposit(IERC20Metadata rootToken, address receiver, uint256 amount) private { + function _deposit(IERC20Metadata rootToken, address receiver, uint256 amount, uint256 feeAmount) private { if (receiver == address(0) || address(rootToken) == address(0)) { revert ZeroAddress(); } - if (amount == 0) { revert ZeroAmount(); } address childToken; - uint256 feeAmount; // The native token does not need to be mapped since it should have been mapped on initialization // The native token also cannot be transferred since it was received in the payable function call @@ -234,9 +234,6 @@ contract RootERC20Bridge is } // ERC20 must be transferred explicitly rootToken.safeTransferFrom(msg.sender, address(this), amount); - feeAmount = msg.value; - } else { - feeAmount = msg.value - amount; } // Deposit sig, root token address, depositor, receiver, amount @@ -260,4 +257,6 @@ contract RootERC20Bridge is } rootBridgeAdaptor = IRootERC20BridgeAdaptor(newRootBridgeAdaptor); } + + receive() external payable {} } diff --git a/test/integration/root/RootERC20Bridge.t.sol b/test/integration/root/RootERC20Bridge.t.sol index a3741723..a9569caf 100644 --- a/test/integration/root/RootERC20Bridge.t.sol +++ b/test/integration/root/RootERC20Bridge.t.sol @@ -161,9 +161,8 @@ contract RootERC20BridgeIntegrationTest is Test, IRootERC20BridgeEvents, IRootAx uint256 tokenAmount = 300; string memory childBridgeAdaptorString = Strings.toHexString(CHILD_BRIDGE_ADAPTOR); - (, bytes memory predictedPayload) = setupDeposit( - IMX_TOKEN_ADDRESS, rootBridge, mapTokenFee, depositFee, tokenAmount, false - ); + (, bytes memory predictedPayload) = + setupDeposit(IMX_TOKEN_ADDRESS, rootBridge, mapTokenFee, depositFee, tokenAmount, false); vm.expectEmit(address(axelarAdaptor)); emit MapTokenAxelarMessage(CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload); diff --git a/test/unit/root/RootERC20Bridge.t.sol b/test/unit/root/RootERC20Bridge.t.sol index bbdbef38..87c81cbf 100644 --- a/test/unit/root/RootERC20Bridge.t.sol +++ b/test/unit/root/RootERC20Bridge.t.sol @@ -46,7 +46,9 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid mockAxelarAdaptor = new MockAdaptor(); // The specific ERC20 token template does not matter for these unit tests - rootBridge.initialize(address(mockAxelarAdaptor), CHILD_BRIDGE, CHILD_BRIDGE_ADAPTOR, address(token), IMX_TOKEN, WRAPPED_ETH); + rootBridge.initialize( + address(mockAxelarAdaptor), CHILD_BRIDGE, CHILD_BRIDGE_ADAPTOR, address(token), IMX_TOKEN, WRAPPED_ETH + ); } /** @@ -61,7 +63,9 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid function test_RevertIfInitializeTwice() public { vm.expectRevert("Initializable: contract is already initialized"); - rootBridge.initialize(address(mockAxelarAdaptor), CHILD_BRIDGE, CHILD_BRIDGE_ADAPTOR, address(token), IMX_TOKEN, WRAPPED_ETH); + rootBridge.initialize( + address(mockAxelarAdaptor), CHILD_BRIDGE, CHILD_BRIDGE_ADAPTOR, address(token), IMX_TOKEN, WRAPPED_ETH + ); } function test_RevertIf_InitializeWithAZeroAddressRootAdapter() public { @@ -208,8 +212,7 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid function test_depositETHCallsSendMessage() public { uint256 amount = 1000; - (, bytes memory predictedPayload) = - setupDeposit(NATIVE_ETH, rootBridge, mapTokenFee, depositFee, amount, false); + (, bytes memory predictedPayload) = setupDeposit(NATIVE_ETH, rootBridge, mapTokenFee, depositFee, amount, false); vm.expectCall( address(mockAxelarAdaptor), @@ -244,9 +247,8 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid function test_depositToETHCallsSendMessage() public { uint256 amount = 1000; address receiver = address(12345); - (, bytes memory predictedPayload) = setupDepositTo( - NATIVE_ETH, rootBridge, mapTokenFee, depositFee, amount, receiver, false - ); + (, bytes memory predictedPayload) = + setupDepositTo(NATIVE_ETH, rootBridge, mapTokenFee, depositFee, amount, receiver, false); vm.expectCall( address(mockAxelarAdaptor), depositFee, @@ -259,9 +261,7 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid function test_depositToETHEmitsNativeEthDepositEvent() public { uint256 amount = 1000; address receiver = address(12345); - setupDepositTo( - NATIVE_ETH, rootBridge, mapTokenFee, depositFee, amount, receiver, false - ); + setupDepositTo(NATIVE_ETH, rootBridge, mapTokenFee, depositFee, amount, receiver, false); vm.expectEmit(); emit NativeEthDeposit(NATIVE_ETH, rootBridge.childETHToken(), address(this), receiver, amount); @@ -271,9 +271,7 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid function test_RevertIf_depositToETHInsufficientValue() public { uint256 amount = 1000; address receiver = address(12345); - setupDepositTo( - NATIVE_ETH, rootBridge, mapTokenFee, depositFee, amount, receiver, false - ); + setupDepositTo(NATIVE_ETH, rootBridge, mapTokenFee, depositFee, amount, receiver, false); vm.expectRevert(InsufficientValue.selector); rootBridge.depositToETH{value: (amount / 2) + depositFee}(receiver, amount); @@ -318,21 +316,23 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid rootBridge.depositTo{value: depositFee}(token, receiver, amount); } - /** + /** * DEPOSIT WETH */ function test_depositWETHCallsSendMessage() public { uint256 amount = 100; - (, bytes memory predictedPayload) = setupDeposit(WRAPPED_ETH, rootBridge, mapTokenFee, depositFee, amount, false); + (, bytes memory predictedPayload) = + setupDeposit(WRAPPED_ETH, rootBridge, mapTokenFee, depositFee, amount, false); + + bytes memory pp = abi.encode(rootBridge.DEPOSIT_SIG(), NATIVE_ETH, address(this), address(this), amount); vm.expectCall( address(mockAxelarAdaptor), - depositFee, - abi.encodeWithSelector(mockAxelarAdaptor.sendMessage.selector, predictedPayload, address(this)) + abi.encodeWithSelector(mockAxelarAdaptor.sendMessage.selector, pp, address(this)) ); - rootBridge.deposit{value: depositFee}(ERC20PresetMinterPauser(WRAPPED_ETH), amount); + rootBridge.deposit{value: depositFee}(IERC20Metadata(WRAPPED_ETH), amount); } /** @@ -341,7 +341,8 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid function test_depositCallsSendMessage() public { uint256 amount = 100; - (, bytes memory predictedPayload) = setupDeposit(address(token), rootBridge, mapTokenFee, depositFee, amount, true); + (, bytes memory predictedPayload) = + setupDeposit(address(token), rootBridge, mapTokenFee, depositFee, amount, true); vm.expectCall( address(mockAxelarAdaptor), @@ -453,7 +454,8 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid uint256 amount = 100; address receiver = address(12345); - (address childToken,) = setupDepositTo(address(token), rootBridge, mapTokenFee, depositFee, amount, receiver, true); + (address childToken,) = + setupDepositTo(address(token), rootBridge, mapTokenFee, depositFee, amount, receiver, true); vm.expectEmit(); emit ERC20Deposit(address(token), childToken, address(this), receiver, amount); diff --git a/test/utils.t.sol b/test/utils.t.sol index 94182e7e..3475aa5c 100644 --- a/test/utils.t.sol +++ b/test/utils.t.sol @@ -53,7 +53,9 @@ contract Utils is Test { address(axelarGasService) ); - rootBridge.initialize(address(axelarAdaptor), childBridge, childBridgeAdaptor, address(token), imxTokenAddress, wethTokenAddress); + rootBridge.initialize( + address(axelarAdaptor), childBridge, childBridgeAdaptor, address(token), imxTokenAddress, wethTokenAddress + ); axelarAdaptor.setChildBridgeAdaptor(); } @@ -96,7 +98,7 @@ contract Utils is Test { if (token == address(0xeee)) { vm.deal(to, tokenAmount + depositFee); - } else if(address(token) == address(0xddd)) { + } else if (address(token) == address(0xddd)) { vm.deal(to, tokenAmount + depositFee); IWETH(token).deposit{value: tokenAmount}(); IWETH(token).approve(address(rootBridge), tokenAmount);