From c88ad563a8a14cac43f507b024cda00cf8b26de0 Mon Sep 17 00:00:00 2001 From: Benjamin Patch Date: Tue, 24 Oct 2023 11:01:15 +1100 Subject: [PATCH 1/2] Add lint CI workflow --- .github/workflows/lint.yml | 28 +++++++++++++ script/InitializeChildContracts.s.sol | 8 ++-- script/InitializeRootContracts.s.sol | 2 +- src/child/ChildERC20Bridge.sol | 6 +-- src/interfaces/child/IChildERC20Bridge.sol | 7 +--- src/interfaces/root/IRootERC20Bridge.sol | 7 +--- src/root/RootERC20Bridge.sol | 32 +++++++-------- test/integration/root/RootERC20Bridge.t.sol | 3 +- test/unit/child/ChildERC20Bridge.t.sol | 15 +++---- test/unit/root/RootERC20Bridge.t.sol | 45 +++++++++++++-------- test/utils.t.sol | 16 ++++---- 11 files changed, 99 insertions(+), 70 deletions(-) create mode 100644 .github/workflows/lint.yml diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 00000000..d5804b89 --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,28 @@ +name: Lint Check + +on: [push] + +env: + FOUNDRY_PROFILE: ci + +jobs: + check: + strategy: + fail-fast: true + + name: Forge Lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + with: + submodules: recursive + + - name: Install Foundry + uses: foundry-rs/foundry-toolchain@v1 + with: + version: nightly + + - name: Run Forge fmt --check + run: | + forge fmt --check + id: fmt diff --git a/script/InitializeChildContracts.s.sol b/script/InitializeChildContracts.s.sol index 513d0b2e..52cc6994 100644 --- a/script/InitializeChildContracts.s.sol +++ b/script/InitializeChildContracts.s.sol @@ -13,7 +13,8 @@ contract InitializeChildContracts is Script { function run() public { uint256 deployerPrivateKey = vm.envUint("CHILD_PRIVATE_KEY"); ChildERC20Bridge childERC20Bridge = ChildERC20Bridge(vm.envAddress("CHILD_ERC20_BRIDGE")); - ChildAxelarBridgeAdaptor childAxelarBridgeAdaptor = ChildAxelarBridgeAdaptor(vm.envAddress("CHILD_BRIDGE_ADAPTOR")); + ChildAxelarBridgeAdaptor childAxelarBridgeAdaptor = + ChildAxelarBridgeAdaptor(vm.envAddress("CHILD_BRIDGE_ADAPTOR")); address childTokenTemplate = vm.envAddress("CHILDCHAIN_CHILD_TOKEN_TEMPLATE"); address rootERC20BridgeAdaptor = vm.envAddress("ROOT_BRIDGE_ADAPTOR"); string memory childRpcUrl = vm.envString("CHILD_RPC_URL"); @@ -27,8 +28,8 @@ contract InitializeChildContracts is Script { vm.startBroadcast(deployerPrivateKey); childERC20Bridge.initialize( - address(childAxelarBridgeAdaptor), - Strings.toHexString(rootERC20BridgeAdaptor), + address(childAxelarBridgeAdaptor), + Strings.toHexString(rootERC20BridgeAdaptor), childTokenTemplate, rootChainName, rootIMXToken @@ -37,6 +38,5 @@ contract InitializeChildContracts is Script { childAxelarBridgeAdaptor.setRootBridgeAdaptor(); vm.stopBroadcast(); - } } diff --git a/script/InitializeRootContracts.s.sol b/script/InitializeRootContracts.s.sol index 66a53115..515f122d 100644 --- a/script/InitializeRootContracts.s.sol +++ b/script/InitializeRootContracts.s.sol @@ -22,7 +22,7 @@ contract InitializeRootContracts is Script { string memory rootRpcUrl = vm.envString("ROOT_RPC_URL"); uint256 rootPrivateKey = vm.envUint("ROOT_PRIVATE_KEY"); address rootIMXToken = vm.envAddress("ROOT_IMX_ADDRESS"); - address childETHToken = vm.envAddress("CHILD_ETH_ADDRESS"); + address childETHToken = vm.envAddress("CHILD_ETH_ADDRESS"); /** * INITIALIZE ROOT CHAIN CONTRACTS diff --git a/src/child/ChildERC20Bridge.sol b/src/child/ChildERC20Bridge.sol index ec91db47..2b8352f4 100644 --- a/src/child/ChildERC20Bridge.sol +++ b/src/child/ChildERC20Bridge.sol @@ -62,9 +62,7 @@ contract ChildERC20Bridge is string memory newRootChain, address newIMXToken ) public initializer { - if (newBridgeAdaptor == address(0) - || newChildTokenTemplate == address(0) - || newIMXToken == address(0)) { + if (newBridgeAdaptor == address(0) || newChildTokenTemplate == address(0) || newIMXToken == address(0)) { revert ZeroAddress(); } @@ -165,7 +163,7 @@ contract ChildERC20Bridge is } else { Address.sendValue(payable(receiver), amount); emit IMXDeposit(address(rootToken), sender, receiver, amount); - } + } } function updateBridgeAdaptor(address newBridgeAdaptor) external override onlyOwner { diff --git a/src/interfaces/child/IChildERC20Bridge.sol b/src/interfaces/child/IChildERC20Bridge.sol index e20e1f92..da1e9de6 100644 --- a/src/interfaces/child/IChildERC20Bridge.sol +++ b/src/interfaces/child/IChildERC20Bridge.sol @@ -32,12 +32,7 @@ interface IChildERC20BridgeEvents { address indexed receiver, uint256 amount ); - event IMXDeposit( - address indexed rootToken, - address depositor, - address indexed receiver, - uint256 amount - ); + event IMXDeposit(address indexed rootToken, address depositor, address indexed receiver, uint256 amount); event NativeDeposit( address indexed rootToken, address indexed childToken, diff --git a/src/interfaces/root/IRootERC20Bridge.sol b/src/interfaces/root/IRootERC20Bridge.sol index 0752b2cb..1dbe07ad 100644 --- a/src/interfaces/root/IRootERC20Bridge.sol +++ b/src/interfaces/root/IRootERC20Bridge.sol @@ -45,12 +45,7 @@ interface IRootERC20BridgeEvents { address indexed receiver, uint256 amount ); - event IMXDeposit( - address indexed rootToken, - address depositor, - address indexed receiver, - uint256 amount - ); + event IMXDeposit(address indexed rootToken, address depositor, address indexed receiver, uint256 amount); event NativeDeposit( address indexed rootToken, address indexed childToken, diff --git a/src/root/RootERC20Bridge.sol b/src/root/RootERC20Bridge.sol index ddb8f287..e2d0bd5a 100644 --- a/src/root/RootERC20Bridge.sol +++ b/src/root/RootERC20Bridge.sol @@ -61,19 +61,15 @@ contract RootERC20Bridge is address newRootBridgeAdaptor, address newChildERC20Bridge, address newChildBridgeAdaptor, - address newChildTokenTemplate, + address newChildTokenTemplate, address newRootIMXToken, - address newChildETHToken) - public - initializer - { - if (newRootBridgeAdaptor == address(0) - || newChildERC20Bridge == address(0) - || newChildTokenTemplate == address(0) - || newChildBridgeAdaptor == address(0) - || newRootIMXToken == address(0) - || newChildETHToken == address(0)) - { + address newChildETHToken + ) public initializer { + if ( + newRootBridgeAdaptor == address(0) || newChildERC20Bridge == address(0) + || newChildTokenTemplate == address(0) || newChildBridgeAdaptor == address(0) + || newRootIMXToken == address(0) || newChildETHToken == address(0) + ) { revert ZeroAddress(); } childERC20Bridge = newChildERC20Bridge; @@ -95,11 +91,13 @@ contract RootERC20Bridge is return _mapToken(rootToken); } - function depositETH(uint256 amount) external payable { //override removed? + function depositETH(uint256 amount) external payable { + //override removed? _depositETH(msg.sender, amount); } - function depositToETH(address receiver, uint256 amount) external payable { //override removed? + function depositToETH(address receiver, uint256 amount) external payable { + //override removed? _depositETH(receiver, amount); } @@ -109,7 +107,7 @@ contract RootERC20Bridge is } uint256 expectedBalance = address(this).balance - (msg.value - amount); - + _deposit(IERC20Metadata(NATIVE_TOKEN), receiver, amount); // invariant check to ensure that the root native balance has increased by the amount deposited @@ -186,7 +184,7 @@ contract RootERC20Bridge is // TODO We can call _mapToken here, but ordering in the GMP is not guaranteed. // Therefore, we need to decide how to handle this and it may be a UI decision to wait until map token message is executed on child chain. // Discuss this, and add this decision to the design doc. - if (address(rootToken) != NATIVE_TOKEN) { + if (address(rootToken) != NATIVE_TOKEN) { if (address(rootToken) != rootIMXToken) { childToken = rootTokenToChildToken[address(rootToken)]; if (childToken == address(0)) { @@ -199,7 +197,7 @@ contract RootERC20Bridge is } else { feeAmount = msg.value - amount; } - + // Deposit sig, root token address, depositor, receiver, amount bytes memory payload = abi.encode(DEPOSIT_SIG, rootToken, msg.sender, receiver, amount); // TODO investigate using delegatecall to keep the axelar message sender as the bridge contract, since adaptor can change. diff --git a/test/integration/root/RootERC20Bridge.t.sol b/test/integration/root/RootERC20Bridge.t.sol index d6bfd4de..8b0c38af 100644 --- a/test/integration/root/RootERC20Bridge.t.sol +++ b/test/integration/root/RootERC20Bridge.t.sol @@ -99,7 +99,8 @@ contract RootERC20BridgeIntegrationTest is Test, IRootERC20BridgeEvents, IRootAx function test_depositToken() public { uint256 tokenAmount = 300; string memory childBridgeAdaptorString = Strings.toHexString(CHILD_BRIDGE_ADAPTOR); - (address childToken, bytes memory predictedPayload) = setupDeposit(token, rootBridge, mapTokenFee, depositFee, tokenAmount, true); + (address childToken, bytes memory predictedPayload) = + setupDeposit(token, rootBridge, mapTokenFee, depositFee, tokenAmount, true); vm.expectEmit(address(axelarAdaptor)); emit MapTokenAxelarMessage(CHILD_CHAIN_NAME, childBridgeAdaptorString, predictedPayload); diff --git a/test/unit/child/ChildERC20Bridge.t.sol b/test/unit/child/ChildERC20Bridge.t.sol index aa7a2aeb..2838caac 100644 --- a/test/unit/child/ChildERC20Bridge.t.sol +++ b/test/unit/child/ChildERC20Bridge.t.sol @@ -33,7 +33,9 @@ contract ChildERC20BridgeUnitTest is Test, IChildERC20BridgeEvents, IChildERC20B childBridge = new ChildERC20Bridge(); - childBridge.initialize(address(this), ROOT_BRIDGE_ADAPTOR, address(childTokenTemplate), ROOT_CHAIN_NAME, IMX_TOKEN); + childBridge.initialize( + address(this), ROOT_BRIDGE_ADAPTOR, address(childTokenTemplate), ROOT_CHAIN_NAME, IMX_TOKEN + ); } function test_Initialize() public { @@ -45,7 +47,9 @@ contract ChildERC20BridgeUnitTest is Test, IChildERC20BridgeEvents, IChildERC20B function test_RevertIfInitializeTwice() public { vm.expectRevert("Initializable: contract is already initialized"); - childBridge.initialize(address(this), ROOT_BRIDGE_ADAPTOR, address(childTokenTemplate), ROOT_CHAIN_NAME, IMX_TOKEN); + childBridge.initialize( + address(this), ROOT_BRIDGE_ADAPTOR, address(childTokenTemplate), ROOT_CHAIN_NAME, IMX_TOKEN + ); } function test_RevertIf_InitializeWithAZeroAddressAdapter() public { @@ -182,9 +186,7 @@ contract ChildERC20BridgeUnitTest is Test, IChildERC20BridgeEvents, IChildERC20B } function test_RevertIf_mapTokenCalledWithIMXAddress() public { - bytes memory data = abi.encode( - childBridge.MAP_TOKEN_SIG(), IMX_TOKEN, "ImmutableX", "IMX", 18 - ); + bytes memory data = abi.encode(childBridge.MAP_TOKEN_SIG(), IMX_TOKEN, "ImmutableX", "IMX", 18); vm.expectRevert(CantMapIMX.selector); childBridge.onMessageReceive(ROOT_CHAIN_NAME, ROOT_BRIDGE_ADAPTOR, data); } @@ -220,7 +222,6 @@ contract ChildERC20BridgeUnitTest is Test, IChildERC20BridgeEvents, IChildERC20B //Deposit function test_onMessageReceive_DepositIMX_EmitsIMXDepositEvent() public { - uint256 fundedAmount = 10 ether; vm.deal(address(childBridge), fundedAmount); @@ -251,7 +252,7 @@ contract ChildERC20BridgeUnitTest is Test, IChildERC20BridgeEvents, IChildERC20B assertEq(receiver.balance, amount, "receiver balance not increased"); } - function test_RevertIf_onMessageReceive_DepositIMX_InsufficientBalance() public { + function test_RevertIf_onMessageReceive_DepositIMX_InsufficientBalance() public { uint256 fundedAmount = 1 ether; vm.deal(address(childBridge), fundedAmount); diff --git a/test/unit/root/RootERC20Bridge.t.sol b/test/unit/root/RootERC20Bridge.t.sol index 90c5c5a3..c6319b85 100644 --- a/test/unit/root/RootERC20Bridge.t.sol +++ b/test/unit/root/RootERC20Bridge.t.sol @@ -43,7 +43,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, CHILD_ETH_TOKEN); + rootBridge.initialize( + address(mockAxelarAdaptor), CHILD_BRIDGE, CHILD_BRIDGE_ADAPTOR, address(token), IMX_TOKEN, CHILD_ETH_TOKEN + ); } /** @@ -58,7 +60,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, CHILD_ETH_TOKEN); + rootBridge.initialize( + address(mockAxelarAdaptor), CHILD_BRIDGE, CHILD_BRIDGE_ADAPTOR, address(token), IMX_TOKEN, CHILD_ETH_TOKEN + ); } function test_RevertIf_InitializeWithAZeroAddressRootAdapter() public { @@ -118,7 +122,6 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid } function test_mapToken_CallsAdaptor() public { - bytes memory payload = abi.encode(rootBridge.MAP_TOKEN_SIG(), token, token.name(), token.symbol(), token.decimals()); @@ -201,7 +204,8 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid function test_depositETHCallsSendMessage() public { uint256 amount = 1000; - (, bytes memory predictedPayload) = setupDeposit(ERC20PresetMinterPauser(NATIVE_TOKEN), rootBridge, mapTokenFee, depositFee, amount, false); + (, bytes memory predictedPayload) = + setupDeposit(ERC20PresetMinterPauser(NATIVE_TOKEN), rootBridge, mapTokenFee, depositFee, amount, false); vm.expectCall( address(mockAxelarAdaptor), @@ -209,7 +213,7 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid abi.encodeWithSelector(mockAxelarAdaptor.sendMessage.selector, predictedPayload, address(this)) ); - rootBridge.depositETH{value: amount+depositFee}(amount); + rootBridge.depositETH{value: amount + depositFee}(amount); } function test_depositETHEmitsNativeDepositEvent() public { @@ -218,7 +222,7 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid vm.expectEmit(); emit NativeDeposit(NATIVE_TOKEN, CHILD_ETH_TOKEN, address(this), address(this), amount); - rootBridge.depositETH{value: amount+depositFee}(amount); + rootBridge.depositETH{value: amount + depositFee}(amount); } function test_RevertIf_depositETHInsufficientValue() public { @@ -226,43 +230,49 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid setupDeposit(ERC20PresetMinterPauser(NATIVE_TOKEN), rootBridge, mapTokenFee, depositFee, amount, false); vm.expectRevert(InsufficientValue.selector); - rootBridge.depositETH{value: (amount/2)+depositFee}(amount); + rootBridge.depositETH{value: (amount / 2) + depositFee}(amount); } - /** + /** * DEPOSIT TO ETH */ function test_depositToETHCallsSendMessage() public { uint256 amount = 1000; address receiver = address(12345); - (, bytes memory predictedPayload) = setupDepositTo(ERC20PresetMinterPauser(NATIVE_TOKEN), rootBridge, mapTokenFee, depositFee, amount, receiver, false); + (, bytes memory predictedPayload) = setupDepositTo( + ERC20PresetMinterPauser(NATIVE_TOKEN), rootBridge, mapTokenFee, depositFee, amount, receiver, false + ); vm.expectCall( address(mockAxelarAdaptor), depositFee, abi.encodeWithSelector(mockAxelarAdaptor.sendMessage.selector, predictedPayload, address(this)) ); - rootBridge.depositToETH{value: amount+depositFee}(receiver, amount); + rootBridge.depositToETH{value: amount + depositFee}(receiver, amount); } function test_depositToETHEmitsNativeDepositEvent() public { uint256 amount = 1000; address receiver = address(12345); - setupDepositTo(ERC20PresetMinterPauser(NATIVE_TOKEN), rootBridge, mapTokenFee, depositFee, amount, receiver, false); + setupDepositTo( + ERC20PresetMinterPauser(NATIVE_TOKEN), rootBridge, mapTokenFee, depositFee, amount, receiver, false + ); vm.expectEmit(); emit NativeDeposit(NATIVE_TOKEN, CHILD_ETH_TOKEN, address(this), receiver, amount); - rootBridge.depositToETH{value: amount+depositFee}(receiver, amount); + rootBridge.depositToETH{value: amount + depositFee}(receiver, amount); } function test_RevertIf_depositToETHInsufficientValue() public { uint256 amount = 1000; address receiver = address(12345); - setupDepositTo(ERC20PresetMinterPauser(NATIVE_TOKEN), rootBridge, mapTokenFee, depositFee, amount, receiver, false); + setupDepositTo( + ERC20PresetMinterPauser(NATIVE_TOKEN), rootBridge, mapTokenFee, depositFee, amount, receiver, false + ); vm.expectRevert(InsufficientValue.selector); - rootBridge.depositToETH{value: (amount/2)+depositFee}(receiver, amount); + rootBridge.depositToETH{value: (amount / 2) + depositFee}(receiver, amount); } /** @@ -274,7 +284,7 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid setupDeposit(ERC20PresetMinterPauser(NATIVE_TOKEN), rootBridge, mapTokenFee, depositFee, amount, false); vm.expectRevert(ZeroAmount.selector); - rootBridge.depositETH{value: amount+depositFee}(amount); + rootBridge.depositETH{value: amount + depositFee}(amount); } function test_RevertIf_depositToETHAmountIsZero() public { @@ -284,7 +294,7 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid setupDeposit(ERC20PresetMinterPauser(NATIVE_TOKEN), rootBridge, mapTokenFee, depositFee, amount, false); vm.expectRevert(ZeroAmount.selector); - rootBridge.depositToETH{value: amount+depositFee}(receiver, amount); + rootBridge.depositToETH{value: amount + depositFee}(receiver, amount); } function test_RevertIf_depositAmountIsZero() public { @@ -406,7 +416,8 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid uint256 amount = 100; address receiver = address(12345); - (, bytes memory predictedPayload) = setupDepositTo(token, rootBridge, mapTokenFee, depositFee, amount, receiver, true); + (, bytes memory predictedPayload) = + setupDepositTo(token, rootBridge, mapTokenFee, depositFee, amount, receiver, true); vm.expectCall( address(mockAxelarAdaptor), diff --git a/test/utils.t.sol b/test/utils.t.sol index 5da0769f..a7373846 100644 --- a/test/utils.t.sol +++ b/test/utils.t.sol @@ -12,13 +12,13 @@ import {IChildERC20, ChildERC20} from "../src/child/ChildERC20.sol"; import {RootAxelarBridgeAdaptor} from "../src/root/RootAxelarBridgeAdaptor.sol"; contract Utils is Test { - function integrationSetup( - address childBridge, - address childBridgeAdaptor, + address childBridge, + address childBridgeAdaptor, string memory childBridgeName, address imxTokenAddress, - address ethTokenAddress) + address ethTokenAddress + ) public returns ( ERC20PresetMinterPauser token, @@ -42,7 +42,9 @@ contract Utils is Test { address(axelarGasService) ); - rootBridge.initialize(address(axelarAdaptor), childBridge, childBridgeAdaptor, address(token), imxTokenAddress, ethTokenAddress); + rootBridge.initialize( + address(axelarAdaptor), childBridge, childBridgeAdaptor, address(token), imxTokenAddress, ethTokenAddress + ); axelarAdaptor.setChildBridgeAdaptor(); } @@ -83,12 +85,12 @@ contract Utils is Test { childToken = rootBridge.mapToken{value: mapTokenFee}(token); } if (address(token) == address(0xeee)) { - vm.deal(to, tokenAmount+depositFee); + vm.deal(to, tokenAmount + depositFee); } else { token.mint(address(this), tokenAmount); token.approve(address(rootBridge), tokenAmount); } - + return (childToken, predictedPayload); } From 558df259cde9691bef95f70f70e65ca0a777c095 Mon Sep 17 00:00:00 2001 From: Benjamin Patch Date: Tue, 24 Oct 2023 11:02:03 +1100 Subject: [PATCH 2/2] fix --- .github/workflows/static-analysis.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index 9be10507..58281c32 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -10,4 +10,5 @@ jobs: - uses: crytic/slither-action@v0.3.0 with: fail-on: high - slither-args: --filter-paths "./lib|./test" \ No newline at end of file + slither-args: --filter-paths "./lib|./test" + node-version: 18 \ No newline at end of file