From 02c3abc5a452c789a04d7d8e28ba0b454dd6eb41 Mon Sep 17 00:00:00 2001 From: Sam MacPherson Date: Wed, 12 Jun 2024 23:01:32 +0900 Subject: [PATCH] [SC-456] Use generic receivers and upgrade xchain-helpers (#18) * use proper wrapping of calls * use fallback style receiver * update to patched xchain-helpers * unused imports * formatting --- lib/forge-std | 2 +- lib/xchain-helpers | 2 +- .../BridgeExecutorReceiverArbitrum.sol | 35 ------------- .../BridgeExecutorReceiverGnosis.sol | 37 -------------- .../BridgeExecutorReceiverOptimism.sol | 35 ------------- test/ArbitrumOneCrosschainTest.t.sol | 44 +++++++--------- test/BaseChainCrosschainTest.t.sol | 38 +++++++------- test/CrosschainTestBase.sol | 24 +++++---- test/GnosisCrosschainTest.t.sol | 51 +++++++------------ test/OptimismCrosschainTest.t.sol | 44 +++++++--------- 10 files changed, 94 insertions(+), 218 deletions(-) delete mode 100644 src/receivers/BridgeExecutorReceiverArbitrum.sol delete mode 100644 src/receivers/BridgeExecutorReceiverGnosis.sol delete mode 100644 src/receivers/BridgeExecutorReceiverOptimism.sol diff --git a/lib/forge-std b/lib/forge-std index e4aef94..52715a2 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit e4aef94c1768803a16fe19f7ce8b65defd027cfd +Subproject commit 52715a217dc51d0de15877878ab8213f6cbbbab5 diff --git a/lib/xchain-helpers b/lib/xchain-helpers index 6d1d9c7..6be2397 160000 --- a/lib/xchain-helpers +++ b/lib/xchain-helpers @@ -1 +1 @@ -Subproject commit 6d1d9c76765fcf90f1ea09a5815f9dbc01347a37 +Subproject commit 6be2397aa4f6e9465a763f609e25a9111ffff200 diff --git a/src/receivers/BridgeExecutorReceiverArbitrum.sol b/src/receivers/BridgeExecutorReceiverArbitrum.sol deleted file mode 100644 index c0f3e8e..0000000 --- a/src/receivers/BridgeExecutorReceiverArbitrum.sol +++ /dev/null @@ -1,35 +0,0 @@ -// SPDX-License-Identifier: AGPL-3.0 -pragma solidity ^0.8.0; - -import { ArbitrumReceiver } from 'xchain-helpers/ArbitrumReceiver.sol'; - -import { IAuthBridgeExecutor } from 'src/interfaces/IAuthBridgeExecutor.sol'; - -contract BridgeExecutorReceiverArbitrum is ArbitrumReceiver { - - IAuthBridgeExecutor public executor; - - constructor( - address _l1Authority, - IAuthBridgeExecutor _executor - ) ArbitrumReceiver(_l1Authority) { - executor = _executor; - } - - function queue( - address[] memory targets, - uint256[] memory values, - string[] memory signatures, - bytes[] memory calldatas, - bool[] memory withDelegatecalls - ) external onlyCrossChainMessage { - executor.queue( - targets, - values, - signatures, - calldatas, - withDelegatecalls - ); - } - -} diff --git a/src/receivers/BridgeExecutorReceiverGnosis.sol b/src/receivers/BridgeExecutorReceiverGnosis.sol deleted file mode 100644 index 43178fc..0000000 --- a/src/receivers/BridgeExecutorReceiverGnosis.sol +++ /dev/null @@ -1,37 +0,0 @@ -// SPDX-License-Identifier: AGPL-3.0 -pragma solidity ^0.8.0; - -import { GnosisReceiver } from 'xchain-helpers/GnosisReceiver.sol'; - -import { IAuthBridgeExecutor } from 'src/interfaces/IAuthBridgeExecutor.sol'; - -contract BridgeExecutorReceiverGnosis is GnosisReceiver { - - IAuthBridgeExecutor public executor; - - constructor( - address _l2CrossDomain, - uint256 _chainId, - address _l1Authority, - IAuthBridgeExecutor _executor - ) GnosisReceiver(_l2CrossDomain, _chainId, _l1Authority) { - executor = _executor; - } - - function queue( - address[] memory targets, - uint256[] memory values, - string[] memory signatures, - bytes[] memory calldatas, - bool[] memory withDelegatecalls - ) external onlyCrossChainMessage { - executor.queue( - targets, - values, - signatures, - calldatas, - withDelegatecalls - ); - } - -} diff --git a/src/receivers/BridgeExecutorReceiverOptimism.sol b/src/receivers/BridgeExecutorReceiverOptimism.sol deleted file mode 100644 index 40a9f43..0000000 --- a/src/receivers/BridgeExecutorReceiverOptimism.sol +++ /dev/null @@ -1,35 +0,0 @@ -// SPDX-License-Identifier: AGPL-3.0 -pragma solidity ^0.8.0; - -import { OptimismReceiver } from 'xchain-helpers/OptimismReceiver.sol'; - -import { IAuthBridgeExecutor } from 'src/interfaces/IAuthBridgeExecutor.sol'; - -contract BridgeExecutorReceiverOptimism is OptimismReceiver { - - IAuthBridgeExecutor public executor; - - constructor( - address _l1Authority, - IAuthBridgeExecutor _executor - ) OptimismReceiver(_l1Authority) { - executor = _executor; - } - - function queue( - address[] memory targets, - uint256[] memory values, - string[] memory signatures, - bytes[] memory calldatas, - bool[] memory withDelegatecalls - ) external onlyCrossChainMessage { - executor.queue( - targets, - values, - signatures, - calldatas, - withDelegatecalls - ); - } - -} diff --git a/test/ArbitrumOneCrosschainTest.t.sol b/test/ArbitrumOneCrosschainTest.t.sol index 27f9628..6cc3d06 100644 --- a/test/ArbitrumOneCrosschainTest.t.sol +++ b/test/ArbitrumOneCrosschainTest.t.sol @@ -1,17 +1,11 @@ // SPDX-License-Identifier: AGPL-3.0 pragma solidity ^0.8.0; -import 'forge-std/Test.sol'; +import './CrosschainTestBase.sol'; -import { Domain, ArbitrumDomain } from 'xchain-helpers/testing/ArbitrumDomain.sol'; -import { XChainForwarders } from 'xchain-helpers/XChainForwarders.sol'; - -import { AuthBridgeExecutor } from 'src/executors/AuthBridgeExecutor.sol'; -import { BridgeExecutorReceiverArbitrum } from 'src/receivers/BridgeExecutorReceiverArbitrum.sol'; - -import { IPayload } from './interfaces/IPayload.sol'; - -import { CrosschainPayload, CrosschainTestBase } from './CrosschainTestBase.sol'; +import { ArbitrumBridgeTesting } from 'lib/xchain-helpers/src/testing/bridges/ArbitrumBridgeTesting.sol'; +import { ArbitrumForwarder } from 'lib/xchain-helpers/src/forwarders/ArbitrumForwarder.sol'; +import { ArbitrumReceiver } from 'lib/xchain-helpers/src/receivers/ArbitrumReceiver.sol'; contract ArbitrumOneCrosschainPayload is CrosschainPayload { @@ -19,7 +13,8 @@ contract ArbitrumOneCrosschainPayload is CrosschainPayload { CrosschainPayload(_targetPayload, _bridgeReceiver) {} function execute() external override { - XChainForwarders.sendMessageArbitrumOne( + ArbitrumForwarder.sendMessageL1toL2( + ArbitrumForwarder.L1_CROSS_DOMAIN_ARBITRUM_ONE, bridgeReceiver, encodeCrosschainExecutionMessage(), 1_000_000, @@ -32,6 +27,9 @@ contract ArbitrumOneCrosschainPayload is CrosschainPayload { contract ArbitrumOneCrosschainTest is CrosschainTestBase { + using DomainHelpers for *; + using ArbitrumBridgeTesting for *; + function deployCrosschainPayload(IPayload targetPayload, address bridgeReceiver) public override returns (IPayload) { @@ -39,33 +37,29 @@ contract ArbitrumOneCrosschainTest is CrosschainTestBase { } function setUp() public { - hostDomain = new Domain(getChain('mainnet')); - bridgedDomain = new ArbitrumDomain(getChain('arbitrum_one'), hostDomain); + bridge = ArbitrumBridgeTesting.createNativeBridge( + getChain('mainnet').createFork(), + getChain('arbitrum_one').createFork() + ); - bridgedDomain.selectFork(); + bridge.destination.selectFork(); bridgeExecutor = new AuthBridgeExecutor( defaultL2BridgeExecutorArgs.delay, defaultL2BridgeExecutorArgs.gracePeriod, defaultL2BridgeExecutorArgs.guardian ); - bridgeReceiver = address(new BridgeExecutorReceiverArbitrum( + bridgeReceiver = address(new ArbitrumReceiver( defaultL2BridgeExecutorArgs.ethereumGovernanceExecutor, - bridgeExecutor + address(bridgeExecutor) )); bridgeExecutor.grantRole(bridgeExecutor.DEFAULT_ADMIN_ROLE(), bridgeReceiver); - hostDomain.selectFork(); + bridge.source.selectFork(); vm.deal(L1_EXECUTOR, 0.01 ether); } - function test_constructor_receiver() public { - BridgeExecutorReceiverArbitrum receiver = new BridgeExecutorReceiverArbitrum( - defaultL2BridgeExecutorArgs.ethereumGovernanceExecutor, - bridgeExecutor - ); - - assertEq(receiver.l1Authority(), defaultL2BridgeExecutorArgs.ethereumGovernanceExecutor); - assertEq(address(receiver.executor()), address(bridgeExecutor)); + function relayMessagesAcrossBridge() internal override { + bridge.relayMessagesToDestination(true); } } diff --git a/test/BaseChainCrosschainTest.t.sol b/test/BaseChainCrosschainTest.t.sol index 85f0924..b87f747 100644 --- a/test/BaseChainCrosschainTest.t.sol +++ b/test/BaseChainCrosschainTest.t.sol @@ -1,17 +1,11 @@ // SPDX-License-Identifier: AGPL-3.0 pragma solidity ^0.8.0; -import 'forge-std/Test.sol'; +import './CrosschainTestBase.sol'; -import { Domain, OptimismDomain } from 'xchain-helpers/testing/OptimismDomain.sol'; -import { XChainForwarders } from 'xchain-helpers/XChainForwarders.sol'; - -import { AuthBridgeExecutor } from 'src/executors/AuthBridgeExecutor.sol'; -import { BridgeExecutorReceiverOptimism } from 'src/receivers/BridgeExecutorReceiverOptimism.sol'; - -import { IPayload } from './interfaces/IPayload.sol'; - -import { CrosschainPayload, CrosschainTestBase } from './CrosschainTestBase.sol'; +import { OptimismBridgeTesting } from 'lib/xchain-helpers/src/testing/bridges/OptimismBridgeTesting.sol'; +import { OptimismForwarder } from 'lib/xchain-helpers/src/forwarders/OptimismForwarder.sol'; +import { OptimismReceiver } from 'lib/xchain-helpers/src/receivers/OptimismReceiver.sol'; contract BaseChainCrosschainPayload is CrosschainPayload { @@ -19,7 +13,8 @@ contract BaseChainCrosschainPayload is CrosschainPayload { CrosschainPayload(_targetPayload, _bridgeReceiver) {} function execute() external override { - XChainForwarders.sendMessageBase( + OptimismForwarder.sendMessageL1toL2( + OptimismForwarder.L1_CROSS_DOMAIN_BASE, bridgeReceiver, encodeCrosschainExecutionMessage(), 1_000_000 @@ -30,6 +25,9 @@ contract BaseChainCrosschainPayload is CrosschainPayload { contract BaseChainCrosschainTest is CrosschainTestBase { + using DomainHelpers for *; + using OptimismBridgeTesting for *; + function deployCrosschainPayload(IPayload targetPayload, address bridgeReceiver) public override returns (IPayload) { @@ -37,22 +35,28 @@ contract BaseChainCrosschainTest is CrosschainTestBase { } function setUp() public { - hostDomain = new Domain(getChain('mainnet')); - bridgedDomain = new OptimismDomain(getChain('base'), hostDomain); + bridge = OptimismBridgeTesting.createNativeBridge( + getChain('mainnet').createFork(), + getChain('base').createFork() + ); - bridgedDomain.selectFork(); + bridge.destination.selectFork(); bridgeExecutor = new AuthBridgeExecutor( defaultL2BridgeExecutorArgs.delay, defaultL2BridgeExecutorArgs.gracePeriod, defaultL2BridgeExecutorArgs.guardian ); - bridgeReceiver = address(new BridgeExecutorReceiverOptimism( + bridgeReceiver = address(new OptimismReceiver( defaultL2BridgeExecutorArgs.ethereumGovernanceExecutor, - bridgeExecutor + address(bridgeExecutor) )); bridgeExecutor.grantRole(bridgeExecutor.DEFAULT_ADMIN_ROLE(), bridgeReceiver); - hostDomain.selectFork(); + bridge.source.selectFork(); + } + + function relayMessagesAcrossBridge() internal override { + bridge.relayMessagesToDestination(true); } } diff --git a/test/CrosschainTestBase.sol b/test/CrosschainTestBase.sol index 6fc2261..7249ec5 100644 --- a/test/CrosschainTestBase.sol +++ b/test/CrosschainTestBase.sol @@ -3,8 +3,8 @@ pragma solidity ^0.8.0; import 'forge-std/Test.sol'; -import { BridgedDomain } from 'xchain-helpers/testing/BridgedDomain.sol'; -import { Domain } from 'xchain-helpers/testing/Domain.sol'; +import { Bridge } from 'lib/xchain-helpers/src/testing/Bridge.sol'; +import { DomainHelpers } from 'lib/xchain-helpers/src/testing/Domain.sol'; import { IAuthBridgeExecutor } from 'src/interfaces/IAuthBridgeExecutor.sol'; import { IExecutorBase } from 'src/interfaces/IExecutorBase.sol'; @@ -60,6 +60,9 @@ abstract contract CrosschainPayload is IPayload { } abstract contract CrosschainTestBase is Test { + + using DomainHelpers for *; + event TestEvent(); address public constant L1_EXECUTOR = 0x3300f198988e4C9C63F75dF86De36421f06af8c4; @@ -73,20 +76,20 @@ abstract contract CrosschainTestBase is Test { guardian: GUARDIAN }); - Domain public hostDomain; - BridgedDomain public bridgedDomain; + Bridge public bridge; AuthBridgeExecutor public bridgeExecutor; address public bridgeReceiver; function deployCrosschainPayload(IPayload targetPayload, address bridgeReceiver) public virtual returns (IPayload); + function relayMessagesAcrossBridge() internal virtual; function preparePayloadExecution() public { - bridgedDomain.selectFork(); + bridge.destination.selectFork(); IPayload targetPayload = IPayload(new PayloadWithEmit()); - hostDomain.selectFork(); + bridge.source.selectFork(); IPayload crosschainPayload = deployCrosschainPayload( targetPayload, @@ -99,7 +102,7 @@ abstract contract CrosschainTestBase is Test { abi.encodeWithSelector(IPayload.execute.selector) ); - bridgedDomain.relayFromHost(true); + relayMessagesAcrossBridge(); } function testFuzz_basicCrosschainPayloadExecution(uint256 delay) public { @@ -302,7 +305,7 @@ abstract contract CrosschainTestBase is Test { } function test_selfReconfiguration() public { - bridgedDomain.selectFork(); + bridge.destination.selectFork(); assertEq( bridgeExecutor.getDelay(), @@ -330,7 +333,7 @@ abstract contract CrosschainTestBase is Test { newL2BridgeExecutorParams.guardian )); - hostDomain.selectFork(); + bridge.source.selectFork(); IPayload crosschainPayload = deployCrosschainPayload( reconfigurationPayload, @@ -343,7 +346,7 @@ abstract contract CrosschainTestBase is Test { abi.encodeWithSelector(IPayload.execute.selector) ); - bridgedDomain.relayFromHost(true); + relayMessagesAcrossBridge(); skip(defaultL2BridgeExecutorArgs.delay); @@ -362,4 +365,5 @@ abstract contract CrosschainTestBase is Test { newL2BridgeExecutorParams.guardian ); } + } diff --git a/test/GnosisCrosschainTest.t.sol b/test/GnosisCrosschainTest.t.sol index 5ae376c..1707e8e 100644 --- a/test/GnosisCrosschainTest.t.sol +++ b/test/GnosisCrosschainTest.t.sol @@ -1,17 +1,11 @@ // SPDX-License-Identifier: AGPL-3.0 pragma solidity ^0.8.0; -import 'forge-std/Test.sol'; +import './CrosschainTestBase.sol'; -import { Domain, GnosisDomain } from 'xchain-helpers/testing/GnosisDomain.sol'; -import { XChainForwarders } from 'xchain-helpers/XChainForwarders.sol'; - -import { AuthBridgeExecutor } from 'src/executors/AuthBridgeExecutor.sol'; -import { BridgeExecutorReceiverGnosis } from 'src/receivers/BridgeExecutorReceiverGnosis.sol'; - -import { IPayload } from './interfaces/IPayload.sol'; - -import { CrosschainPayload, CrosschainTestBase } from './CrosschainTestBase.sol'; +import { AMBBridgeTesting } from 'lib/xchain-helpers/src/testing/bridges/AMBBridgeTesting.sol'; +import { AMBForwarder } from 'lib/xchain-helpers/src/forwarders/AMBForwarder.sol'; +import { AMBReceiver } from 'lib/xchain-helpers/src/receivers/AMBReceiver.sol'; contract GnosisCrosschainPayload is CrosschainPayload { @@ -19,7 +13,7 @@ contract GnosisCrosschainPayload is CrosschainPayload { CrosschainPayload(_targetPayload, _bridgeReceiver) {} function execute() external override { - XChainForwarders.sendMessageGnosis( + AMBForwarder.sendMessageEthereumToGnosisChain( bridgeReceiver, encodeCrosschainExecutionMessage(), 1_000_000 @@ -30,7 +24,8 @@ contract GnosisCrosschainPayload is CrosschainPayload { contract GnosisCrosschainTest is CrosschainTestBase { - address constant AMB = 0x75Df5AF045d91108662D8080fD1FEFAd6aA0bb59; + using DomainHelpers for *; + using AMBBridgeTesting for *; function deployCrosschainPayload(IPayload targetPayload, address bridgeReceiver) public override returns (IPayload) @@ -39,38 +34,30 @@ contract GnosisCrosschainTest is CrosschainTestBase { } function setUp() public { - hostDomain = new Domain(getChain('mainnet')); - bridgedDomain = new GnosisDomain(getChain('gnosis_chain'), hostDomain); + bridge = AMBBridgeTesting.createGnosisBridge( + getChain('mainnet').createFork(), + getChain('gnosis_chain').createFork() + ); - bridgedDomain.selectFork(); + bridge.destination.selectFork(); bridgeExecutor = new AuthBridgeExecutor( defaultL2BridgeExecutorArgs.delay, defaultL2BridgeExecutorArgs.gracePeriod, defaultL2BridgeExecutorArgs.guardian ); - bridgeReceiver = address(new BridgeExecutorReceiverGnosis( - AMB, - 1, // Ethereum chainid + bridgeReceiver = address(new AMBReceiver( + AMBBridgeTesting.getGnosisMessengerFromChainAlias(bridge.destination.chain.chainAlias), + bytes32(uint256(1)), // Ethereum chainid defaultL2BridgeExecutorArgs.ethereumGovernanceExecutor, - bridgeExecutor + address(bridgeExecutor) )); bridgeExecutor.grantRole(bridgeExecutor.DEFAULT_ADMIN_ROLE(), bridgeReceiver); - hostDomain.selectFork(); + bridge.source.selectFork(); } - function test_constructor_receiver() public { - BridgeExecutorReceiverGnosis receiver = new BridgeExecutorReceiverGnosis( - AMB, - 1, - defaultL2BridgeExecutorArgs.ethereumGovernanceExecutor, - bridgeExecutor - ); - - assertEq(address(receiver.l2CrossDomain()), AMB); - assertEq(receiver.chainId(), bytes32(uint256(1))); - assertEq(receiver.l1Authority(), defaultL2BridgeExecutorArgs.ethereumGovernanceExecutor); - assertEq(address(receiver.executor()), address(bridgeExecutor)); + function relayMessagesAcrossBridge() internal override { + bridge.relayMessagesToDestination(true); } } diff --git a/test/OptimismCrosschainTest.t.sol b/test/OptimismCrosschainTest.t.sol index 4fd3f77..6895e1e 100644 --- a/test/OptimismCrosschainTest.t.sol +++ b/test/OptimismCrosschainTest.t.sol @@ -1,17 +1,11 @@ // SPDX-License-Identifier: AGPL-3.0 pragma solidity ^0.8.0; -import 'forge-std/Test.sol'; +import './CrosschainTestBase.sol'; -import { Domain, OptimismDomain } from 'xchain-helpers/testing/OptimismDomain.sol'; -import { XChainForwarders } from 'xchain-helpers/XChainForwarders.sol'; - -import { AuthBridgeExecutor } from 'src/executors/AuthBridgeExecutor.sol'; -import { BridgeExecutorReceiverOptimism } from 'src/receivers/BridgeExecutorReceiverOptimism.sol'; - -import { IPayload } from './interfaces/IPayload.sol'; - -import { CrosschainPayload, CrosschainTestBase } from './CrosschainTestBase.sol'; +import { OptimismBridgeTesting } from 'lib/xchain-helpers/src/testing/bridges/OptimismBridgeTesting.sol'; +import { OptimismForwarder } from 'lib/xchain-helpers/src/forwarders/OptimismForwarder.sol'; +import { OptimismReceiver } from 'lib/xchain-helpers/src/receivers/OptimismReceiver.sol'; contract OptimismCrosschainPayload is CrosschainPayload { @@ -19,7 +13,8 @@ contract OptimismCrosschainPayload is CrosschainPayload { CrosschainPayload(_targetPayload, _bridgeReceiver) {} function execute() external override { - XChainForwarders.sendMessageOptimismMainnet( + OptimismForwarder.sendMessageL1toL2( + OptimismForwarder.L1_CROSS_DOMAIN_OPTIMISM, bridgeReceiver, encodeCrosschainExecutionMessage(), 1_000_000 @@ -30,6 +25,9 @@ contract OptimismCrosschainPayload is CrosschainPayload { contract OptimismCrosschainTest is CrosschainTestBase { + using DomainHelpers for *; + using OptimismBridgeTesting for *; + function deployCrosschainPayload(IPayload targetPayload, address bridgeReceiver) public override returns (IPayload) { @@ -37,32 +35,28 @@ contract OptimismCrosschainTest is CrosschainTestBase { } function setUp() public { - hostDomain = new Domain(getChain('mainnet')); - bridgedDomain = new OptimismDomain(getChain('optimism'), hostDomain); + bridge = OptimismBridgeTesting.createNativeBridge( + getChain('mainnet').createFork(), + getChain('optimism').createFork() + ); - bridgedDomain.selectFork(); + bridge.destination.selectFork(); bridgeExecutor = new AuthBridgeExecutor( defaultL2BridgeExecutorArgs.delay, defaultL2BridgeExecutorArgs.gracePeriod, defaultL2BridgeExecutorArgs.guardian ); - bridgeReceiver = address(new BridgeExecutorReceiverOptimism( + bridgeReceiver = address(new OptimismReceiver( defaultL2BridgeExecutorArgs.ethereumGovernanceExecutor, - bridgeExecutor + address(bridgeExecutor) )); bridgeExecutor.grantRole(bridgeExecutor.DEFAULT_ADMIN_ROLE(), bridgeReceiver); - hostDomain.selectFork(); + bridge.source.selectFork(); } - function test_constructor_receiver() public { - BridgeExecutorReceiverOptimism receiver = new BridgeExecutorReceiverOptimism( - defaultL2BridgeExecutorArgs.ethereumGovernanceExecutor, - bridgeExecutor - ); - - assertEq(receiver.l1Authority(), defaultL2BridgeExecutorArgs.ethereumGovernanceExecutor); - assertEq(address(receiver.executor()), address(bridgeExecutor)); + function relayMessagesAcrossBridge() internal override { + bridge.relayMessagesToDestination(true); } }