From 31184eb03bfb10467a482702afea174c1ec42008 Mon Sep 17 00:00:00 2001 From: Sam MacPherson Date: Tue, 7 May 2024 15:04:41 +0900 Subject: [PATCH 01/13] slight refactor to bring in line with other repos style --- src/executors/AuthBridgeExecutor.sol | 2 +- src/executors/BridgeExecutorBase.sol | 2 +- src/receivers/BridgeExecutorReceiverArbitrum.sol | 2 +- src/receivers/BridgeExecutorReceiverGnosis.sol | 2 +- src/receivers/BridgeExecutorReceiverOptimism.sol | 2 +- ...chainTest.t.sol => ArbitrumOneCrosschainTest.t.sol} | 10 +++++----- ...sschainTest.t.sol => BaseChainCrosschainTest.t.sol} | 10 +++++----- test/CrosschainTestBase.sol | 6 +++--- test/GnosisCrosschainTest.t.sol | 4 ++-- test/OptimismCrosschainTest.t.sol | 4 ++-- 10 files changed, 22 insertions(+), 22 deletions(-) rename test/{ArbitrumCrosschainTest.t.sol => ArbitrumOneCrosschainTest.t.sol} (81%) rename test/{BaseCrosschainTest.t.sol => BaseChainCrosschainTest.t.sol} (81%) diff --git a/src/executors/AuthBridgeExecutor.sol b/src/executors/AuthBridgeExecutor.sol index d8ce754..72db024 100644 --- a/src/executors/AuthBridgeExecutor.sol +++ b/src/executors/AuthBridgeExecutor.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.10; import { AccessControl } from 'lib/openzeppelin-contracts/contracts/access/AccessControl.sol'; -import { IAuthBridgeExecutor } from '../interfaces/IAuthBridgeExecutor.sol'; +import { IAuthBridgeExecutor } from 'src/interfaces/IAuthBridgeExecutor.sol'; import { BridgeExecutorBase } from './BridgeExecutorBase.sol'; /** diff --git a/src/executors/BridgeExecutorBase.sol b/src/executors/BridgeExecutorBase.sol index 3c1c435..f2d8f3a 100644 --- a/src/executors/BridgeExecutorBase.sol +++ b/src/executors/BridgeExecutorBase.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: AGPL-3.0 pragma solidity ^0.8.10; -import { IExecutorBase } from '../interfaces/IExecutorBase.sol'; +import { IExecutorBase } from 'src/interfaces/IExecutorBase.sol'; /** * @title BridgeExecutorBase diff --git a/src/receivers/BridgeExecutorReceiverArbitrum.sol b/src/receivers/BridgeExecutorReceiverArbitrum.sol index 5a7accb..4dd6930 100644 --- a/src/receivers/BridgeExecutorReceiverArbitrum.sol +++ b/src/receivers/BridgeExecutorReceiverArbitrum.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.0; import { ArbitrumReceiver } from 'xchain-helpers/ArbitrumReceiver.sol'; -import { IAuthBridgeExecutor } from '../interfaces/IAuthBridgeExecutor.sol'; +import { IAuthBridgeExecutor } from 'src/interfaces/IAuthBridgeExecutor.sol'; contract BridgeExecutorReceiverArbitrum is ArbitrumReceiver { diff --git a/src/receivers/BridgeExecutorReceiverGnosis.sol b/src/receivers/BridgeExecutorReceiverGnosis.sol index 553039f..032d907 100644 --- a/src/receivers/BridgeExecutorReceiverGnosis.sol +++ b/src/receivers/BridgeExecutorReceiverGnosis.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.0; import { GnosisReceiver } from 'xchain-helpers/GnosisReceiver.sol'; -import { IAuthBridgeExecutor } from '../interfaces/IAuthBridgeExecutor.sol'; +import { IAuthBridgeExecutor } from 'src/interfaces/IAuthBridgeExecutor.sol'; contract BridgeExecutorReceiverGnosis is GnosisReceiver { diff --git a/src/receivers/BridgeExecutorReceiverOptimism.sol b/src/receivers/BridgeExecutorReceiverOptimism.sol index f5f483c..b8b8fa4 100644 --- a/src/receivers/BridgeExecutorReceiverOptimism.sol +++ b/src/receivers/BridgeExecutorReceiverOptimism.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.0; import { OptimismReceiver } from 'xchain-helpers/OptimismReceiver.sol'; -import { IAuthBridgeExecutor } from '../interfaces/IAuthBridgeExecutor.sol'; +import { IAuthBridgeExecutor } from 'src/interfaces/IAuthBridgeExecutor.sol'; contract BridgeExecutorReceiverOptimism is OptimismReceiver { diff --git a/test/ArbitrumCrosschainTest.t.sol b/test/ArbitrumOneCrosschainTest.t.sol similarity index 81% rename from test/ArbitrumCrosschainTest.t.sol rename to test/ArbitrumOneCrosschainTest.t.sol index 44a55e3..ad06c8d 100644 --- a/test/ArbitrumCrosschainTest.t.sol +++ b/test/ArbitrumOneCrosschainTest.t.sol @@ -6,14 +6,14 @@ import 'forge-std/Test.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 { 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'; -contract ArbitrumCrosschainPayload is CrosschainPayload { +contract ArbitrumOneCrosschainPayload is CrosschainPayload { constructor(IPayload _targetPayload, address _bridgeReceiver) CrosschainPayload(_targetPayload, _bridgeReceiver) {} @@ -30,12 +30,12 @@ contract ArbitrumCrosschainPayload is CrosschainPayload { } -contract ArbitrumCrosschainTest is CrosschainTestBase { +contract ArbitrumOneCrosschainTest is CrosschainTestBase { function deployCrosschainPayload(IPayload targetPayload, address bridgeReceiver) public override returns (IPayload) { - return IPayload(new ArbitrumCrosschainPayload(targetPayload, bridgeReceiver)); + return IPayload(new ArbitrumOneCrosschainPayload(targetPayload, bridgeReceiver)); } function setUp() public { diff --git a/test/BaseCrosschainTest.t.sol b/test/BaseChainCrosschainTest.t.sol similarity index 81% rename from test/BaseCrosschainTest.t.sol rename to test/BaseChainCrosschainTest.t.sol index 4213fbd..2cac3c0 100644 --- a/test/BaseCrosschainTest.t.sol +++ b/test/BaseChainCrosschainTest.t.sol @@ -6,14 +6,14 @@ import 'forge-std/Test.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 { 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'; -contract BaseCrosschainPayload is CrosschainPayload { +contract BaseChainCrosschainPayload is CrosschainPayload { constructor(IPayload _targetPayload, address _bridgeReceiver) CrosschainPayload(_targetPayload, _bridgeReceiver) {} @@ -28,12 +28,12 @@ contract BaseCrosschainPayload is CrosschainPayload { } -contract BaseCrosschainTest is CrosschainTestBase { +contract BaseChainCrosschainTest is CrosschainTestBase { function deployCrosschainPayload(IPayload targetPayload, address bridgeReceiver) public override returns (IPayload) { - return IPayload(new BaseCrosschainPayload(targetPayload, bridgeReceiver)); + return IPayload(new BaseChainCrosschainPayload(targetPayload, bridgeReceiver)); } function setUp() public { diff --git a/test/CrosschainTestBase.sol b/test/CrosschainTestBase.sol index ca08c68..d011d12 100644 --- a/test/CrosschainTestBase.sol +++ b/test/CrosschainTestBase.sol @@ -6,9 +6,9 @@ import 'forge-std/Test.sol'; import { BridgedDomain } from 'xchain-helpers/testing/BridgedDomain.sol'; import { Domain } from 'xchain-helpers/testing/Domain.sol'; -import { IAuthBridgeExecutor } from '../src/interfaces/IAuthBridgeExecutor.sol'; -import { IExecutorBase } from '../src/interfaces/IExecutorBase.sol'; -import { AuthBridgeExecutor } from '../src/executors/AuthBridgeExecutor.sol'; +import { IAuthBridgeExecutor } from 'src/interfaces/IAuthBridgeExecutor.sol'; +import { IExecutorBase } from 'src/interfaces/IExecutorBase.sol'; +import { AuthBridgeExecutor } from 'src/executors/AuthBridgeExecutor.sol'; import { IL1Executor } from './interfaces/IL1Executor.sol'; import { IPayload } from './interfaces/IPayload.sol'; diff --git a/test/GnosisCrosschainTest.t.sol b/test/GnosisCrosschainTest.t.sol index f1e7879..5e4c55b 100644 --- a/test/GnosisCrosschainTest.t.sol +++ b/test/GnosisCrosschainTest.t.sol @@ -6,8 +6,8 @@ import 'forge-std/Test.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 { AuthBridgeExecutor } from 'src/executors/AuthBridgeExecutor.sol'; +import { BridgeExecutorReceiverGnosis } from 'src/receivers/BridgeExecutorReceiverGnosis.sol'; import { IPayload } from './interfaces/IPayload.sol'; diff --git a/test/OptimismCrosschainTest.t.sol b/test/OptimismCrosschainTest.t.sol index 47c34ec..6c05ece 100644 --- a/test/OptimismCrosschainTest.t.sol +++ b/test/OptimismCrosschainTest.t.sol @@ -6,8 +6,8 @@ import 'forge-std/Test.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 { AuthBridgeExecutor } from 'src/executors/AuthBridgeExecutor.sol'; +import { BridgeExecutorReceiverOptimism } from 'src/receivers/BridgeExecutorReceiverOptimism.sol'; import { IPayload } from './interfaces/IPayload.sol'; From b71c27731dcf86023ffbc38b4504fa379e35c347 Mon Sep 17 00:00:00 2001 From: Sam MacPherson Date: Tue, 7 May 2024 15:21:28 +0900 Subject: [PATCH 02/13] coverage on receivers --- test/ArbitrumOneCrosschainTest.t.sol | 10 ++++++++++ test/GnosisCrosschainTest.t.sol | 14 ++++++++++++++ test/OptimismCrosschainTest.t.sol | 10 ++++++++++ 3 files changed, 34 insertions(+) diff --git a/test/ArbitrumOneCrosschainTest.t.sol b/test/ArbitrumOneCrosschainTest.t.sol index ad06c8d..d685f93 100644 --- a/test/ArbitrumOneCrosschainTest.t.sol +++ b/test/ArbitrumOneCrosschainTest.t.sol @@ -60,4 +60,14 @@ contract ArbitrumOneCrosschainTest is CrosschainTestBase { 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)); + } + } diff --git a/test/GnosisCrosschainTest.t.sol b/test/GnosisCrosschainTest.t.sol index 5e4c55b..2bbbe33 100644 --- a/test/GnosisCrosschainTest.t.sol +++ b/test/GnosisCrosschainTest.t.sol @@ -61,4 +61,18 @@ contract GnosisCrosschainTest is CrosschainTestBase { hostDomain.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)); + } + } diff --git a/test/OptimismCrosschainTest.t.sol b/test/OptimismCrosschainTest.t.sol index 6c05ece..f81ea6e 100644 --- a/test/OptimismCrosschainTest.t.sol +++ b/test/OptimismCrosschainTest.t.sol @@ -57,4 +57,14 @@ contract OptimismCrosschainTest is CrosschainTestBase { hostDomain.selectFork(); } + function test_constructor_receiver() public { + BridgeExecutorReceiverOptimism receiver = new BridgeExecutorReceiverOptimism( + defaultL2BridgeExecutorArgs.ethereumGovernanceExecutor, + bridgeExecutor + ); + + assertEq(receiver.l1Authority(), defaultL2BridgeExecutorArgs.ethereumGovernanceExecutor); + assertEq(address(receiver.executor()), address(bridgeExecutor)); + } + } From 67c0d77fad541dc432bd3931da91fb1f6a22edac Mon Sep 17 00:00:00 2001 From: Sam MacPherson Date: Tue, 7 May 2024 15:31:10 +0900 Subject: [PATCH 03/13] fix license; add auth bridge unit test scaffold; add specific compiler settings in foundry.toml --- foundry.toml | 3 +++ src/receivers/BridgeExecutorReceiverArbitrum.sol | 2 +- src/receivers/BridgeExecutorReceiverOptimism.sol | 2 +- test/ArbitrumOneCrosschainTest.t.sol | 2 +- test/AuthBridgeExecutor.t.sol | 11 +++++++++++ test/BaseChainCrosschainTest.t.sol | 2 +- test/CrosschainTestBase.sol | 2 +- test/GnosisCrosschainTest.t.sol | 2 +- test/OptimismCrosschainTest.t.sol | 2 +- test/interfaces/IL1Executor.sol | 2 +- test/interfaces/IPayload.sol | 2 +- test/mocks/PayloadWithEmit.sol | 2 +- test/mocks/ReconfigurationPayload.sol | 2 +- 13 files changed, 25 insertions(+), 11 deletions(-) create mode 100644 test/AuthBridgeExecutor.t.sol diff --git a/foundry.toml b/foundry.toml index 2bc70ba..1f8f15b 100644 --- a/foundry.toml +++ b/foundry.toml @@ -2,3 +2,6 @@ src = "src" out = "out" libs = ["lib"] +solc_version = '0.8.25' +optimizer = true +optimizer_runs = 200 diff --git a/src/receivers/BridgeExecutorReceiverArbitrum.sol b/src/receivers/BridgeExecutorReceiverArbitrum.sol index 4dd6930..c0f3e8e 100644 --- a/src/receivers/BridgeExecutorReceiverArbitrum.sol +++ b/src/receivers/BridgeExecutorReceiverArbitrum.sol @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: AGPL-3.0-or-later +// SPDX-License-Identifier: AGPL-3.0 pragma solidity ^0.8.0; import { ArbitrumReceiver } from 'xchain-helpers/ArbitrumReceiver.sol'; diff --git a/src/receivers/BridgeExecutorReceiverOptimism.sol b/src/receivers/BridgeExecutorReceiverOptimism.sol index b8b8fa4..40a9f43 100644 --- a/src/receivers/BridgeExecutorReceiverOptimism.sol +++ b/src/receivers/BridgeExecutorReceiverOptimism.sol @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: AGPL-3.0-or-later +// SPDX-License-Identifier: AGPL-3.0 pragma solidity ^0.8.0; import { OptimismReceiver } from 'xchain-helpers/OptimismReceiver.sol'; diff --git a/test/ArbitrumOneCrosschainTest.t.sol b/test/ArbitrumOneCrosschainTest.t.sol index d685f93..d285664 100644 --- a/test/ArbitrumOneCrosschainTest.t.sol +++ b/test/ArbitrumOneCrosschainTest.t.sol @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: MIT +// SPDX-License-Identifier: AGPL-3.0 pragma solidity ^0.8.0; import 'forge-std/Test.sol'; diff --git a/test/AuthBridgeExecutor.t.sol b/test/AuthBridgeExecutor.t.sol new file mode 100644 index 0000000..1c3c640 --- /dev/null +++ b/test/AuthBridgeExecutor.t.sol @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: AGPL-3.0 +pragma solidity ^0.8.0; + +import 'forge-std/Test.sol'; + +contract AuthBridgeExecutorTest is Test { + + function setUp() public { + } + +} diff --git a/test/BaseChainCrosschainTest.t.sol b/test/BaseChainCrosschainTest.t.sol index 2cac3c0..820b915 100644 --- a/test/BaseChainCrosschainTest.t.sol +++ b/test/BaseChainCrosschainTest.t.sol @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: MIT +// SPDX-License-Identifier: AGPL-3.0 pragma solidity ^0.8.0; import 'forge-std/Test.sol'; diff --git a/test/CrosschainTestBase.sol b/test/CrosschainTestBase.sol index d011d12..b43f77c 100644 --- a/test/CrosschainTestBase.sol +++ b/test/CrosschainTestBase.sol @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: MIT +// SPDX-License-Identifier: AGPL-3.0 pragma solidity ^0.8.0; import 'forge-std/Test.sol'; diff --git a/test/GnosisCrosschainTest.t.sol b/test/GnosisCrosschainTest.t.sol index 2bbbe33..9d2ccd8 100644 --- a/test/GnosisCrosschainTest.t.sol +++ b/test/GnosisCrosschainTest.t.sol @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: MIT +// SPDX-License-Identifier: AGPL-3.0 pragma solidity ^0.8.0; import 'forge-std/Test.sol'; diff --git a/test/OptimismCrosschainTest.t.sol b/test/OptimismCrosschainTest.t.sol index f81ea6e..8d029e0 100644 --- a/test/OptimismCrosschainTest.t.sol +++ b/test/OptimismCrosschainTest.t.sol @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: MIT +// SPDX-License-Identifier: AGPL-3.0 pragma solidity ^0.8.0; import 'forge-std/Test.sol'; diff --git a/test/interfaces/IL1Executor.sol b/test/interfaces/IL1Executor.sol index 30ef2e3..4362916 100644 --- a/test/interfaces/IL1Executor.sol +++ b/test/interfaces/IL1Executor.sol @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: MIT +// SPDX-License-Identifier: AGPL-3.0 pragma solidity ^0.8.0; interface IL1Executor { diff --git a/test/interfaces/IPayload.sol b/test/interfaces/IPayload.sol index 4e96a47..55b76db 100644 --- a/test/interfaces/IPayload.sol +++ b/test/interfaces/IPayload.sol @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: MIT +// SPDX-License-Identifier: AGPL-3.0 pragma solidity ^0.8.10; interface IPayload { diff --git a/test/mocks/PayloadWithEmit.sol b/test/mocks/PayloadWithEmit.sol index edd7bd5..5ed6771 100644 --- a/test/mocks/PayloadWithEmit.sol +++ b/test/mocks/PayloadWithEmit.sol @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: MIT +// SPDX-License-Identifier: AGPL-3.0 pragma solidity ^0.8.10; import { IPayload } from '../interfaces/IPayload.sol'; diff --git a/test/mocks/ReconfigurationPayload.sol b/test/mocks/ReconfigurationPayload.sol index 380b061..4e4e571 100644 --- a/test/mocks/ReconfigurationPayload.sol +++ b/test/mocks/ReconfigurationPayload.sol @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: MIT +// SPDX-License-Identifier: AGPL-3.0 pragma solidity ^0.8.10; import { IExecutorBase } from '../../src/interfaces/IExecutorBase.sol'; From bfaa4ae8a839a422ae715d20603694f1dbdd221c Mon Sep 17 00:00:00 2001 From: Sam MacPherson Date: Tue, 7 May 2024 15:32:39 +0900 Subject: [PATCH 04/13] license --- src/receivers/BridgeExecutorReceiverGnosis.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/receivers/BridgeExecutorReceiverGnosis.sol b/src/receivers/BridgeExecutorReceiverGnosis.sol index 032d907..43178fc 100644 --- a/src/receivers/BridgeExecutorReceiverGnosis.sol +++ b/src/receivers/BridgeExecutorReceiverGnosis.sol @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: AGPL-3.0-or-later +// SPDX-License-Identifier: AGPL-3.0 pragma solidity ^0.8.0; import { GnosisReceiver } from 'xchain-helpers/GnosisReceiver.sol'; From 5e9269537640658ab496f98f4e1358b64e5d6871 Mon Sep 17 00:00:00 2001 From: Sam MacPherson Date: Tue, 7 May 2024 17:23:07 +0900 Subject: [PATCH 05/13] add execute tests --- test/AuthBridgeExecutor.t.sol | 260 ++++++++++++++++++++++++++++++++++ 1 file changed, 260 insertions(+) diff --git a/test/AuthBridgeExecutor.t.sol b/test/AuthBridgeExecutor.t.sol index 1c3c640..83b5638 100644 --- a/test/AuthBridgeExecutor.t.sol +++ b/test/AuthBridgeExecutor.t.sol @@ -3,9 +3,269 @@ pragma solidity ^0.8.0; import 'forge-std/Test.sol'; +import { AuthBridgeExecutor } from 'src/executors/AuthBridgeExecutor.sol'; + +contract DefaultPayload { + event TestEvent(); + function execute() external { + emit TestEvent(); + } +} + +contract PayablePayload { + event TestEvent(); + function execute() external payable { + emit TestEvent(); + } +} + +contract RevertingPayload { + function execute() external pure { + revert("An error occurred"); + } +} + contract AuthBridgeExecutorTest is Test { + struct Action { + address[] targets; + uint256[] values; + string[] signatures; + bytes[] calldatas; + bool[] withDelegatecalls; + } + + event ActionsSetExecuted( + uint256 indexed id, + address indexed initiatorExecution, + bytes[] returnedData + ); + event TestEvent(); + + uint256 constant DELAY = 1 days; + uint256 constant GRACE_PERIOD = 30 days; + + address bridge = makeAddr("bridge"); + address guardian = makeAddr("guardian"); + + AuthBridgeExecutor executor; + function setUp() public { + executor = new AuthBridgeExecutor({ + delay: DELAY, + gracePeriod: GRACE_PERIOD, + minimumDelay: 0, // TODO: removing this in next PR + maximumDelay: 365 days, // TODO: removing this in next PR + guardian: guardian + }); + executor.grantRole(executor.AUTHORIZED_BRIDGE_ROLE(), bridge); + executor.grantRole(executor.DEFAULT_ADMIN_ROLE(), bridge); + executor.revokeRole(executor.DEFAULT_ADMIN_ROLE(), address(this)); + } + + function test_constructor() public { + executor = new AuthBridgeExecutor({ + delay: DELAY, + gracePeriod: GRACE_PERIOD, + minimumDelay: 0, + maximumDelay: 365 days, + guardian: guardian + }); + + assertEq(executor.getDelay(), DELAY); + assertEq(executor.getGracePeriod(), GRACE_PERIOD); + assertEq(executor.getGuardian(), guardian); + + assertEq(executor.hasRole(executor.DEFAULT_ADMIN_ROLE(), address(this)), true); + assertEq(executor.getRoleAdmin(executor.AUTHORIZED_BRIDGE_ROLE()), executor.DEFAULT_ADMIN_ROLE()); + } + + function test_execute_actionsSetId_too_high_boundary() public { + assertEq(executor.getActionsSetCount(), 0); + vm.expectRevert(abi.encodeWithSignature("InvalidActionsSetId()")); + executor.execute(0); + + _queueAction(); + skip(DELAY); + + assertEq(executor.getActionsSetCount(), 1); + executor.execute(0); + } + + function test_execute_notQueued_cancelled() public { + _queueAction(); + vm.prank(guardian); + executor.cancel(0); + + vm.expectRevert(abi.encodeWithSignature("OnlyQueuedActions()")); + executor.execute(0); + } + + function test_execute_notQueued_executed() public { + _queueAction(); + skip(DELAY); + + executor.execute(0); + + vm.expectRevert(abi.encodeWithSignature("OnlyQueuedActions()")); + executor.execute(0); + } + + function test_execute_notQueued_expired_boundary() public { + _queueAction(); + skip(DELAY + GRACE_PERIOD + 1); + + vm.expectRevert(abi.encodeWithSignature("OnlyQueuedActions()")); + executor.execute(0); + + vm.warp(block.timestamp - 1); + + executor.execute(0); + } + + function test_execute_timelock_not_finished_boundary() public { + _queueAction(); + skip(DELAY - 1); + + vm.expectRevert(abi.encodeWithSignature("TimelockNotFinished()")); + executor.execute(0); + + skip(1); + + executor.execute(0); + } + + function test_execute_balance_too_low_boundary() public { + _queueActionWithValue(1 ether); + skip(DELAY); + + vm.deal(address(executor), 1 ether - 1); + vm.expectRevert(abi.encodeWithSignature("InsufficientBalance()")); + executor.execute(0); + + vm.deal(address(executor), 1 ether); + + executor.execute(0); + } + + function test_execute_evm_error() public { + // Trigger some evm error like trying to call a non-payable function + Action memory action = _getDefaultAction(); + action.values[0] = 1 ether; + _queueAction(action); + skip(DELAY); + vm.deal(address(executor), 1 ether); + + vm.expectRevert(abi.encodeWithSignature("FailedActionExecution()")); + executor.execute(0); + } + + function test_execute_revert_error() public { + Action memory action = _getDefaultAction(); + action.targets[0] = address(new RevertingPayload()); + _queueAction(action); + skip(DELAY); + + // Should return the underlying error message + vm.expectRevert("An error occurred"); + executor.execute(0); + } + + function test_execute_delegateCall() public { + Action memory action = _getDefaultAction(); + bytes32 actionHash = _encodeHash(action, block.timestamp + DELAY); + _queueAction(action); + skip(DELAY); + + assertEq(executor.isActionQueued(actionHash), true); + + bytes[] memory returnedData = new bytes[](1); + returnedData[0] = ""; + vm.expectEmit(address(executor)); + emit TestEvent(); + vm.expectEmit(address(executor)); + emit ActionsSetExecuted(0, address(this), returnedData); + executor.execute(0); + + assertEq(executor.isActionQueued(actionHash), false); + } + + function test_execute_call() public { + Action memory action = _getDefaultAction(); + action.withDelegatecalls[0] = false; + bytes32 actionHash = _encodeHash(action, block.timestamp + DELAY); + _queueAction(action); + skip(DELAY); + + assertEq(executor.isActionQueued(actionHash), true); + + bytes[] memory returnedData = new bytes[](1); + returnedData[0] = ""; + vm.expectEmit(action.targets[0]); + emit TestEvent(); + vm.expectEmit(address(executor)); + emit ActionsSetExecuted(0, address(this), returnedData); + executor.execute(0); + + assertEq(executor.isActionQueued(actionHash), false); + } + + function _getDefaultAction() internal returns (Action memory) { + address[] memory targets = new address[](1); + targets[0] = address(new DefaultPayload()); + uint256[] memory values = new uint256[](1); + values[0] = 0; + string[] memory signatures = new string[](1); + signatures[0] = 'execute()'; + bytes[] memory calldatas = new bytes[](1); + calldatas[0] = ''; + bool[] memory withDelegatecalls = new bool[](1); + withDelegatecalls[0] = true; + + return Action({ + targets: targets, + values: values, + signatures: signatures, + calldatas: calldatas, + withDelegatecalls: withDelegatecalls + }); + } + + function _queueAction(Action memory action) internal { + vm.prank(bridge); + executor.queue( + action.targets, + action.values, + action.signatures, + action.calldatas, + action.withDelegatecalls + ); + } + + function _queueAction() internal { + _queueAction(_getDefaultAction()); + } + + function _queueActionWithValue(uint256 value) internal { + Action memory action = _getDefaultAction(); + action.targets[0] = address(new PayablePayload()); + action.values[0] = value; + _queueAction(action); + } + + function _encodeHash(Action memory action, uint256 index, uint256 executionTime) internal pure returns (bytes32) { + return keccak256(abi.encode( + action.targets[index], + action.values[index], + action.signatures[index], + action.calldatas[index], + executionTime, + action.withDelegatecalls[index] + )); + } + + function _encodeHash(Action memory action, uint256 executionTime) internal pure returns (bytes32) { + return _encodeHash(action, 0, executionTime); } } From 6eb964511d5edc06b2f04e0faab1e2f7d179a473 Mon Sep 17 00:00:00 2001 From: Sam MacPherson Date: Tue, 7 May 2024 17:43:49 +0900 Subject: [PATCH 06/13] add cancel tests --- test/AuthBridgeExecutor.t.sol | 95 +++++++++++++++++++++++++++++++++-- 1 file changed, 91 insertions(+), 4 deletions(-) diff --git a/test/AuthBridgeExecutor.t.sol b/test/AuthBridgeExecutor.t.sol index 83b5638..ac6b273 100644 --- a/test/AuthBridgeExecutor.t.sol +++ b/test/AuthBridgeExecutor.t.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.0; import 'forge-std/Test.sol'; import { AuthBridgeExecutor } from 'src/executors/AuthBridgeExecutor.sol'; +import { IExecutorBase } from 'src/interfaces/IExecutorBase.sol'; contract DefaultPayload { event TestEvent(); @@ -40,6 +41,7 @@ contract AuthBridgeExecutorTest is Test { address indexed initiatorExecution, bytes[] returnedData ); + event ActionsSetCanceled(uint256 indexed id); event TestEvent(); uint256 constant DELAY = 1 days; @@ -177,7 +179,9 @@ contract AuthBridgeExecutorTest is Test { _queueAction(action); skip(DELAY); - assertEq(executor.isActionQueued(actionHash), true); + assertEq(executor.isActionQueued(actionHash), true); + assertEq(executor.getActionsSetById(0).executed, false); + assertEq(uint8(executor.getCurrentState(0)), uint8(IExecutorBase.ActionsSetState.Queued)); bytes[] memory returnedData = new bytes[](1); returnedData[0] = ""; @@ -187,7 +191,9 @@ contract AuthBridgeExecutorTest is Test { emit ActionsSetExecuted(0, address(this), returnedData); executor.execute(0); - assertEq(executor.isActionQueued(actionHash), false); + assertEq(executor.isActionQueued(actionHash), false); + assertEq(executor.getActionsSetById(0).executed, true); + assertEq(uint8(executor.getCurrentState(0)), uint8(IExecutorBase.ActionsSetState.Executed)); } function test_execute_call() public { @@ -197,7 +203,9 @@ contract AuthBridgeExecutorTest is Test { _queueAction(action); skip(DELAY); - assertEq(executor.isActionQueued(actionHash), true); + assertEq(executor.isActionQueued(actionHash), true); + assertEq(executor.getActionsSetById(0).executed, false); + assertEq(uint8(executor.getCurrentState(0)), uint8(IExecutorBase.ActionsSetState.Queued)); bytes[] memory returnedData = new bytes[](1); returnedData[0] = ""; @@ -207,7 +215,86 @@ contract AuthBridgeExecutorTest is Test { emit ActionsSetExecuted(0, address(this), returnedData); executor.execute(0); - assertEq(executor.isActionQueued(actionHash), false); + assertEq(executor.isActionQueued(actionHash), false); + assertEq(executor.getActionsSetById(0).executed, true); + assertEq(uint8(executor.getCurrentState(0)), uint8(IExecutorBase.ActionsSetState.Executed)); + } + + function test_cancel_notGuardian() public { + _queueAction(); + skip(DELAY); + + vm.expectRevert(abi.encodeWithSignature("NotGuardian()")); + executor.cancel(0); + } + + function test_cancel_actionsSetId_too_high_boundary() public { + assertEq(executor.getActionsSetCount(), 0); + vm.expectRevert(abi.encodeWithSignature("InvalidActionsSetId()")); + vm.prank(guardian); + executor.cancel(0); + + _queueAction(); + skip(DELAY); + + assertEq(executor.getActionsSetCount(), 1); + vm.prank(guardian); + executor.cancel(0); + } + + function test_cancel_notQueued_cancelled() public { + _queueAction(); + vm.prank(guardian); + executor.cancel(0); + + vm.expectRevert(abi.encodeWithSignature("OnlyQueuedActions()")); + vm.prank(guardian); + executor.cancel(0); + } + + function test_cancel_notQueued_executed() public { + _queueAction(); + skip(DELAY); + + vm.prank(guardian); + executor.cancel(0); + + vm.expectRevert(abi.encodeWithSignature("OnlyQueuedActions()")); + vm.prank(guardian); + executor.cancel(0); + } + + function test_cancel_notQueued_expired_boundary() public { + _queueAction(); + skip(DELAY + GRACE_PERIOD + 1); + + vm.expectRevert(abi.encodeWithSignature("OnlyQueuedActions()")); + vm.prank(guardian); + executor.cancel(0); + + vm.warp(block.timestamp - 1); + + vm.prank(guardian); + executor.cancel(0); + } + + function test_cancel() public { + Action memory action = _getDefaultAction(); + bytes32 actionHash = _encodeHash(action, block.timestamp + DELAY); + _queueAction(action); + + assertEq(executor.isActionQueued(actionHash), true); + assertEq(executor.getActionsSetById(0).canceled, false); + assertEq(uint8(executor.getCurrentState(0)), uint8(IExecutorBase.ActionsSetState.Queued)); + + vm.expectEmit(address(executor)); + emit ActionsSetCanceled(0); + vm.prank(guardian); + executor.cancel(0); + + assertEq(executor.isActionQueued(actionHash), false); + assertEq(executor.getActionsSetById(0).canceled, true); + assertEq(uint8(executor.getCurrentState(0)), uint8(IExecutorBase.ActionsSetState.Canceled)); } function _getDefaultAction() internal returns (Action memory) { From 31bd206c4e72fe5c5ac9551b8cfa09e35b9b39f0 Mon Sep 17 00:00:00 2001 From: Sam MacPherson Date: Tue, 7 May 2024 17:49:55 +0900 Subject: [PATCH 07/13] add update guardian/delay/gracePeriod tets --- test/AuthBridgeExecutor.t.sol | 53 +++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/test/AuthBridgeExecutor.t.sol b/test/AuthBridgeExecutor.t.sol index ac6b273..2051c5c 100644 --- a/test/AuthBridgeExecutor.t.sol +++ b/test/AuthBridgeExecutor.t.sol @@ -42,6 +42,9 @@ contract AuthBridgeExecutorTest is Test { bytes[] returnedData ); event ActionsSetCanceled(uint256 indexed id); + event GuardianUpdate(address oldGuardian, address newGuardian); + event DelayUpdate(uint256 oldDelay, uint256 newDelay); + event GracePeriodUpdate(uint256 oldGracePeriod, uint256 newGracePeriod); event TestEvent(); uint256 constant DELAY = 1 days; @@ -297,6 +300,56 @@ contract AuthBridgeExecutorTest is Test { assertEq(uint8(executor.getCurrentState(0)), uint8(IExecutorBase.ActionsSetState.Canceled)); } + function test_updateGuardian_notSelf() public { + vm.expectRevert(abi.encodeWithSignature("OnlyCallableByThis()")); + executor.updateGuardian(guardian); + } + + function test_updateGuardian() public { + address newGuardian = makeAddr("newGuardian"); + + assertEq(executor.getGuardian(), guardian); + + vm.expectEmit(address(executor)); + emit GuardianUpdate(guardian, newGuardian); + vm.prank(address(executor)); + executor.updateGuardian(newGuardian); + + assertEq(executor.getGuardian(), newGuardian); + } + + function test_updateDelay_notSelf() public { + vm.expectRevert(abi.encodeWithSignature("OnlyCallableByThis()")); + executor.updateDelay(2 days); + } + + function test_updateDelay() public { + assertEq(executor.getDelay(), 1 days); + + vm.expectEmit(address(executor)); + emit DelayUpdate(1 days, 2 days); + vm.prank(address(executor)); + executor.updateDelay(2 days); + + assertEq(executor.getDelay(), 2 days); + } + + function test_updateGracePeriod_notSelf() public { + vm.expectRevert(abi.encodeWithSignature("OnlyCallableByThis()")); + executor.updateGracePeriod(60 days); + } + + function test_updateGracePeriod() public { + assertEq(executor.getGracePeriod(), 30 days); + + vm.expectEmit(address(executor)); + emit GracePeriodUpdate(30 days, 60 days); + vm.prank(address(executor)); + executor.updateGracePeriod(60 days); + + assertEq(executor.getGracePeriod(), 60 days); + } + function _getDefaultAction() internal returns (Action memory) { address[] memory targets = new address[](1); targets[0] = address(new DefaultPayload()); From aec30fb0104969eac05f4148fde3fc3ede5ad3a8 Mon Sep 17 00:00:00 2001 From: Sam MacPherson Date: Tue, 7 May 2024 23:00:11 +0900 Subject: [PATCH 08/13] add queue tests --- test/AuthBridgeExecutor.t.sol | 95 +++++++++++++++++++++++++++++++++-- 1 file changed, 91 insertions(+), 4 deletions(-) diff --git a/test/AuthBridgeExecutor.t.sol b/test/AuthBridgeExecutor.t.sol index 2051c5c..295ce64 100644 --- a/test/AuthBridgeExecutor.t.sol +++ b/test/AuthBridgeExecutor.t.sol @@ -85,7 +85,72 @@ contract AuthBridgeExecutorTest is Test { assertEq(executor.getRoleAdmin(executor.AUTHORIZED_BRIDGE_ROLE()), executor.DEFAULT_ADMIN_ROLE()); } - function test_execute_actionsSetId_too_high_boundary() public { + function test_queue_onlyBridge() public { + vm.expectRevert(abi.encodeWithSignature("AccessControlUnauthorizedAccount(address,bytes32)", address(this), executor.AUTHORIZED_BRIDGE_ROLE())); + executor.queue(new address[](0), new uint256[](0), new string[](0), new bytes[](0), new bool[](0)); + } + + function test_queue_lengthZero() public { + vm.expectRevert(abi.encodeWithSignature("EmptyTargets()")); + vm.prank(bridge); + executor.queue(new address[](0), new uint256[](0), new string[](0), new bytes[](0), new bool[](0)); + } + + function test_queue_inconsistentParamsLength() public { + vm.expectRevert(abi.encodeWithSignature("InconsistentParamsLength()")); + vm.prank(bridge); + executor.queue(new address[](2), new uint256[](1), new string[](1), new bytes[](1), new bool[](1)); + + vm.expectRevert(abi.encodeWithSignature("InconsistentParamsLength()")); + vm.prank(bridge); + executor.queue(new address[](1), new uint256[](2), new string[](1), new bytes[](1), new bool[](1)); + + vm.expectRevert(abi.encodeWithSignature("InconsistentParamsLength()")); + vm.prank(bridge); + executor.queue(new address[](1), new uint256[](1), new string[](2), new bytes[](1), new bool[](1)); + + vm.expectRevert(abi.encodeWithSignature("InconsistentParamsLength()")); + vm.prank(bridge); + executor.queue(new address[](1), new uint256[](1), new string[](1), new bytes[](2), new bool[](1)); + + vm.expectRevert(abi.encodeWithSignature("InconsistentParamsLength()")); + vm.prank(bridge); + executor.queue(new address[](1), new uint256[](1), new string[](1), new bytes[](1), new bool[](2)); + } + + function test_queue_duplicateAction() public { + Action memory action = _getDefaultAction(); + _queueAction(action); + + vm.expectRevert(abi.encodeWithSignature("DuplicateAction()")); + _queueAction(action); + } + + function test_queue() public { + Action memory action = _getDefaultAction(); + bytes32 actionHash1 = _encodeHash(action, block.timestamp + DELAY); + bytes32 actionHash2 = _encodeHash(action, block.timestamp + DELAY + 1); + + assertEq(executor.getActionsSetCount(), 0); + assertEq(executor.isActionQueued(actionHash1), false); + assertEq(executor.isActionQueued(actionHash2), false); + + _queueAction(action); + + assertEq(executor.getActionsSetCount(), 1); + assertEq(executor.isActionQueued(actionHash1), true); + assertEq(executor.isActionQueued(actionHash2), false); + + // Can queue up the same action 1 second later + skip(1); + _queueAction(action); + + assertEq(executor.getActionsSetCount(), 2); + assertEq(executor.isActionQueued(actionHash1), true); + assertEq(executor.isActionQueued(actionHash2), true); + } + + function test_execute_actionsSetIdTooHigh_boundary() public { assertEq(executor.getActionsSetCount(), 0); vm.expectRevert(abi.encodeWithSignature("InvalidActionsSetId()")); executor.execute(0); @@ -231,7 +296,7 @@ contract AuthBridgeExecutorTest is Test { executor.cancel(0); } - function test_cancel_actionsSetId_too_high_boundary() public { + function test_cancel_actionsSetIdTooHigh_boundary() public { assertEq(executor.getActionsSetCount(), 0); vm.expectRevert(abi.encodeWithSignature("InvalidActionsSetId()")); vm.prank(guardian); @@ -350,15 +415,37 @@ contract AuthBridgeExecutorTest is Test { assertEq(executor.getGracePeriod(), 60 days); } + function test_executeDelegateCall_notSelf() public { + vm.expectRevert(abi.encodeWithSignature("OnlyCallableByThis()")); + executor.executeDelegateCall(address(0), ""); + } + + function test_executeDelegateCall() public { + address target = address(new DefaultPayload()); + + vm.expectEmit(address(executor)); + emit TestEvent(); + vm.prank(address(executor)); + executor.executeDelegateCall(target, abi.encodeCall(DefaultPayload.execute, ())); + } + + function test_receiveFunds() public { + assertEq(address(executor).balance, 0); + + executor.receiveFunds{value:1 ether}(); + + assertEq(address(executor).balance, 1 ether); + } + function _getDefaultAction() internal returns (Action memory) { address[] memory targets = new address[](1); targets[0] = address(new DefaultPayload()); uint256[] memory values = new uint256[](1); values[0] = 0; string[] memory signatures = new string[](1); - signatures[0] = 'execute()'; + signatures[0] = "execute()"; bytes[] memory calldatas = new bytes[](1); - calldatas[0] = ''; + calldatas[0] = ""; bool[] memory withDelegatecalls = new bool[](1); withDelegatecalls[0] = true; From 1771a7805a62aad1cf17fe723ee2f5885c334d12 Mon Sep 17 00:00:00 2001 From: Sam MacPherson Date: Tue, 7 May 2024 23:11:00 +0900 Subject: [PATCH 09/13] cover grace period minimum --- test/AuthBridgeExecutor.t.sol | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/AuthBridgeExecutor.t.sol b/test/AuthBridgeExecutor.t.sol index 295ce64..12ca36c 100644 --- a/test/AuthBridgeExecutor.t.sol +++ b/test/AuthBridgeExecutor.t.sol @@ -404,6 +404,15 @@ contract AuthBridgeExecutorTest is Test { executor.updateGracePeriod(60 days); } + function test_updateGracePeriod_underMinimum_boundary() public { + vm.expectRevert(abi.encodeWithSignature("GracePeriodTooShort()")); + vm.prank(address(executor)); + executor.updateGracePeriod(10 minutes - 1); + + vm.prank(address(executor)); + executor.updateGracePeriod(10 minutes); + } + function test_updateGracePeriod() public { assertEq(executor.getGracePeriod(), 30 days); From a2649e425a4d9aa038e49e868df826d897a609e6 Mon Sep 17 00:00:00 2001 From: Sam MacPherson Date: Tue, 7 May 2024 23:31:29 +0900 Subject: [PATCH 10/13] init params coverage --- test/AuthBridgeExecutor.t.sol | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/AuthBridgeExecutor.t.sol b/test/AuthBridgeExecutor.t.sol index 12ca36c..0c91152 100644 --- a/test/AuthBridgeExecutor.t.sol +++ b/test/AuthBridgeExecutor.t.sol @@ -68,6 +68,17 @@ contract AuthBridgeExecutorTest is Test { executor.revokeRole(executor.DEFAULT_ADMIN_ROLE(), address(this)); } + function test_constructor_invalidInitParams() public { + vm.expectRevert(abi.encodeWithSignature("InvalidInitParams()")); + executor = new AuthBridgeExecutor({ + delay: DELAY, + gracePeriod: 10 minutes - 1, + minimumDelay: 0, + maximumDelay: 365 days, + guardian: guardian + }); + } + function test_constructor() public { executor = new AuthBridgeExecutor({ delay: DELAY, From ac5f4edb856f219bec3159dd31a787c9d94288cd Mon Sep 17 00:00:00 2001 From: Sam MacPherson Date: Tue, 7 May 2024 23:51:01 +0900 Subject: [PATCH 11/13] coverage of signature being empty --- test/AuthBridgeExecutor.t.sol | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/AuthBridgeExecutor.t.sol b/test/AuthBridgeExecutor.t.sol index 0c91152..26c81b5 100644 --- a/test/AuthBridgeExecutor.t.sol +++ b/test/AuthBridgeExecutor.t.sol @@ -299,6 +299,31 @@ contract AuthBridgeExecutorTest is Test { assertEq(uint8(executor.getCurrentState(0)), uint8(IExecutorBase.ActionsSetState.Executed)); } + function test_execute_delegateCallWithCalldata() public { + Action memory action = _getDefaultAction(); + action.signatures[0] = ""; + action.calldatas[0] = abi.encodeWithSignature("execute()"); + bytes32 actionHash = _encodeHash(action, block.timestamp + DELAY); + _queueAction(action); + skip(DELAY); + + assertEq(executor.isActionQueued(actionHash), true); + assertEq(executor.getActionsSetById(0).executed, false); + assertEq(uint8(executor.getCurrentState(0)), uint8(IExecutorBase.ActionsSetState.Queued)); + + bytes[] memory returnedData = new bytes[](1); + returnedData[0] = ""; + vm.expectEmit(address(executor)); + emit TestEvent(); + vm.expectEmit(address(executor)); + emit ActionsSetExecuted(0, address(this), returnedData); + executor.execute(0); + + assertEq(executor.isActionQueued(actionHash), false); + assertEq(executor.getActionsSetById(0).executed, true); + assertEq(uint8(executor.getCurrentState(0)), uint8(IExecutorBase.ActionsSetState.Executed)); + } + function test_cancel_notGuardian() public { _queueAction(); skip(DELAY); From ddc20afb5745330a7667e96c319dfb37c892329c Mon Sep 17 00:00:00 2001 From: Sam MacPherson Date: Thu, 16 May 2024 15:09:43 +0900 Subject: [PATCH 12/13] review fixes --- test/AuthBridgeExecutor.t.sol | 252 ++++++++++++++++++++++-------- test/OptimismCrosschainTest.t.sol | 2 +- 2 files changed, 188 insertions(+), 66 deletions(-) diff --git a/test/AuthBridgeExecutor.t.sol b/test/AuthBridgeExecutor.t.sol index 26c81b5..81844f2 100644 --- a/test/AuthBridgeExecutor.t.sol +++ b/test/AuthBridgeExecutor.t.sol @@ -26,7 +26,7 @@ contract RevertingPayload { } } -contract AuthBridgeExecutorTest is Test { +contract AuthBridgeExecutorTestBase is Test { struct Action { address[] targets; @@ -36,6 +36,15 @@ contract AuthBridgeExecutorTest is Test { bool[] withDelegatecalls; } + event ActionsSetQueued( + uint256 indexed id, + address[] targets, + uint256[] values, + string[] signatures, + bytes[] calldatas, + bool[] withDelegatecalls, + uint256 executionTime + ); event ActionsSetExecuted( uint256 indexed id, address indexed initiatorExecution, @@ -68,7 +77,85 @@ contract AuthBridgeExecutorTest is Test { executor.revokeRole(executor.DEFAULT_ADMIN_ROLE(), address(this)); } - function test_constructor_invalidInitParams() public { + /******************************************************************************************************************/ + /*** Helper functions ***/ + /******************************************************************************************************************/ + + function _getDefaultAction() internal returns (Action memory) { + address[] memory targets = new address[](1); + targets[0] = address(new DefaultPayload()); + uint256[] memory values = new uint256[](1); + values[0] = 0; + string[] memory signatures = new string[](1); + signatures[0] = "execute()"; + bytes[] memory calldatas = new bytes[](1); + calldatas[0] = ""; + bool[] memory withDelegatecalls = new bool[](1); + withDelegatecalls[0] = true; + + return Action({ + targets: targets, + values: values, + signatures: signatures, + calldatas: calldatas, + withDelegatecalls: withDelegatecalls + }); + } + + function _queueAction(Action memory action) internal { + vm.prank(bridge); + executor.queue( + action.targets, + action.values, + action.signatures, + action.calldatas, + action.withDelegatecalls + ); + } + + function _queueAction() internal { + _queueAction(_getDefaultAction()); + } + + function _queueActionWithValue(uint256 value) internal { + Action memory action = _getDefaultAction(); + action.targets[0] = address(new PayablePayload()); + action.values[0] = value; + _queueAction(action); + } + + function _encodeHash(Action memory action, uint256 index, uint256 executionTime) internal pure returns (bytes32) { + return keccak256(abi.encode( + action.targets[index], + action.values[index], + action.signatures[index], + action.calldatas[index], + executionTime, + action.withDelegatecalls[index] + )); + } + + function _encodeHash(Action memory action, uint256 executionTime) internal pure returns (bytes32) { + return _encodeHash(action, 0, executionTime); + } + + function _assertActionSet(uint256 id, bool executed, bool canceled, uint256 executionTime, Action memory actions) internal view { + IExecutorBase.ActionsSet memory actionsSet = executor.getActionsSetById(id); + assertEq(actionsSet.targets, actions.targets); + assertEq(actionsSet.values, actions.values); + assertEq(actionsSet.signatures, actions.signatures); + assertEq(actionsSet.calldatas, actions.calldatas); + assertEq(actionsSet.withDelegatecalls, actions.withDelegatecalls); + assertEq(actionsSet.executionTime, executionTime); + assertEq(actionsSet.executed, executed); + assertEq(actionsSet.canceled, canceled); + } + +} + +contract AuthBridgeExecutorConstructorTests is AuthBridgeExecutorTestBase { + + function test_constructor_invalidInitParams_boundary() public { vm.expectRevert(abi.encodeWithSignature("InvalidInitParams()")); executor = new AuthBridgeExecutor({ delay: DELAY, @@ -77,9 +164,23 @@ contract AuthBridgeExecutorTest is Test { maximumDelay: 365 days, guardian: guardian }); + + executor = new AuthBridgeExecutor({ + delay: DELAY, + gracePeriod: 10 minutes, + minimumDelay: 0, + maximumDelay: 365 days, + guardian: guardian + }); } function test_constructor() public { + vm.expectEmit(); + emit DelayUpdate(0, DELAY); + vm.expectEmit(); + emit GracePeriodUpdate(0, GRACE_PERIOD); + vm.expectEmit(); + emit GuardianUpdate(address(0), guardian); executor = new AuthBridgeExecutor({ delay: DELAY, gracePeriod: GRACE_PERIOD, @@ -93,9 +194,13 @@ contract AuthBridgeExecutorTest is Test { assertEq(executor.getGuardian(), guardian); assertEq(executor.hasRole(executor.DEFAULT_ADMIN_ROLE(), address(this)), true); - assertEq(executor.getRoleAdmin(executor.AUTHORIZED_BRIDGE_ROLE()), executor.DEFAULT_ADMIN_ROLE()); + assertEq(executor.getRoleAdmin(executor.AUTHORIZED_BRIDGE_ROLE()), executor.DEFAULT_ADMIN_ROLE()); } +} + +contract AuthBridgeExecutorQueueTests is AuthBridgeExecutorTestBase { + function test_queue_onlyBridge() public { vm.expectRevert(abi.encodeWithSignature("AccessControlUnauthorizedAccount(address,bytes32)", address(this), executor.AUTHORIZED_BRIDGE_ROLE())); executor.queue(new address[](0), new uint256[](0), new string[](0), new bytes[](0), new bool[](0)); @@ -146,21 +251,59 @@ contract AuthBridgeExecutorTest is Test { assertEq(executor.isActionQueued(actionHash1), false); assertEq(executor.isActionQueued(actionHash2), false); + vm.expectEmit(address(executor)); + emit ActionsSetQueued( + 0, + action.targets, + action.values, + action.signatures, + action.calldatas, + action.withDelegatecalls, + block.timestamp + DELAY + ); _queueAction(action); assertEq(executor.getActionsSetCount(), 1); assertEq(executor.isActionQueued(actionHash1), true); assertEq(executor.isActionQueued(actionHash2), false); + _assertActionSet({ + id: 0, + executed: false, + canceled: false, + executionTime: block.timestamp + DELAY, + actions: action + }); // Can queue up the same action 1 second later skip(1); + vm.expectEmit(address(executor)); + emit ActionsSetQueued( + 1, + action.targets, + action.values, + action.signatures, + action.calldatas, + action.withDelegatecalls, + block.timestamp + DELAY + ); _queueAction(action); assertEq(executor.getActionsSetCount(), 2); assertEq(executor.isActionQueued(actionHash1), true); assertEq(executor.isActionQueued(actionHash2), true); + _assertActionSet({ + id: 1, + executed: false, + canceled: false, + executionTime: block.timestamp + DELAY, + actions: action + }); } +} + +contract AuthBridgeExecutorExecutorTests is AuthBridgeExecutorTestBase { + function test_execute_actionsSetIdTooHigh_boundary() public { assertEq(executor.getActionsSetCount(), 0); vm.expectRevert(abi.encodeWithSignature("InvalidActionsSetId()")); @@ -204,7 +347,7 @@ contract AuthBridgeExecutorTest is Test { executor.execute(0); } - function test_execute_timelock_not_finished_boundary() public { + function test_execute_timelockNotFinished_boundary() public { _queueAction(); skip(DELAY - 1); @@ -216,7 +359,7 @@ contract AuthBridgeExecutorTest is Test { executor.execute(0); } - function test_execute_balance_too_low_boundary() public { + function test_execute_balanceTooLow_boundary() public { _queueActionWithValue(1 ether); skip(DELAY); @@ -324,6 +467,36 @@ contract AuthBridgeExecutorTest is Test { assertEq(uint8(executor.getCurrentState(0)), uint8(IExecutorBase.ActionsSetState.Executed)); } + function test_execute_callWithCalldata() public { + Action memory action = _getDefaultAction(); + action.signatures[0] = ""; + action.calldatas[0] = abi.encodeWithSignature("execute()"); + action.withDelegatecalls[0] = false; + bytes32 actionHash = _encodeHash(action, block.timestamp + DELAY); + _queueAction(action); + skip(DELAY); + + assertEq(executor.isActionQueued(actionHash), true); + assertEq(executor.getActionsSetById(0).executed, false); + assertEq(uint8(executor.getCurrentState(0)), uint8(IExecutorBase.ActionsSetState.Queued)); + + bytes[] memory returnedData = new bytes[](1); + returnedData[0] = ""; + vm.expectEmit(action.targets[0]); + emit TestEvent(); + vm.expectEmit(address(executor)); + emit ActionsSetExecuted(0, address(this), returnedData); + executor.execute(0); + + assertEq(executor.isActionQueued(actionHash), false); + assertEq(executor.getActionsSetById(0).executed, true); + assertEq(uint8(executor.getCurrentState(0)), uint8(IExecutorBase.ActionsSetState.Executed)); + } + +} + +contract AuthBridgeExecutorCancelTests is AuthBridgeExecutorTestBase { + function test_cancel_notGuardian() public { _queueAction(); skip(DELAY); @@ -360,8 +533,7 @@ contract AuthBridgeExecutorTest is Test { _queueAction(); skip(DELAY); - vm.prank(guardian); - executor.cancel(0); + executor.execute(0); vm.expectRevert(abi.encodeWithSignature("OnlyQueuedActions()")); vm.prank(guardian); @@ -401,6 +573,10 @@ contract AuthBridgeExecutorTest is Test { assertEq(uint8(executor.getCurrentState(0)), uint8(IExecutorBase.ActionsSetState.Canceled)); } +} + +contract AuthBridgeExecutorUpdateTests is AuthBridgeExecutorTestBase { + function test_updateGuardian_notSelf() public { vm.expectRevert(abi.encodeWithSignature("OnlyCallableByThis()")); executor.updateGuardian(guardian); @@ -460,6 +636,10 @@ contract AuthBridgeExecutorTest is Test { assertEq(executor.getGracePeriod(), 60 days); } +} + +contract AuthBridgeExecutorMiscTests is AuthBridgeExecutorTestBase { + function test_executeDelegateCall_notSelf() public { vm.expectRevert(abi.encodeWithSignature("OnlyCallableByThis()")); executor.executeDelegateCall(address(0), ""); @@ -482,62 +662,4 @@ contract AuthBridgeExecutorTest is Test { assertEq(address(executor).balance, 1 ether); } - function _getDefaultAction() internal returns (Action memory) { - address[] memory targets = new address[](1); - targets[0] = address(new DefaultPayload()); - uint256[] memory values = new uint256[](1); - values[0] = 0; - string[] memory signatures = new string[](1); - signatures[0] = "execute()"; - bytes[] memory calldatas = new bytes[](1); - calldatas[0] = ""; - bool[] memory withDelegatecalls = new bool[](1); - withDelegatecalls[0] = true; - - return Action({ - targets: targets, - values: values, - signatures: signatures, - calldatas: calldatas, - withDelegatecalls: withDelegatecalls - }); - } - - function _queueAction(Action memory action) internal { - vm.prank(bridge); - executor.queue( - action.targets, - action.values, - action.signatures, - action.calldatas, - action.withDelegatecalls - ); - } - - function _queueAction() internal { - _queueAction(_getDefaultAction()); - } - - function _queueActionWithValue(uint256 value) internal { - Action memory action = _getDefaultAction(); - action.targets[0] = address(new PayablePayload()); - action.values[0] = value; - _queueAction(action); - } - - function _encodeHash(Action memory action, uint256 index, uint256 executionTime) internal pure returns (bytes32) { - return keccak256(abi.encode( - action.targets[index], - action.values[index], - action.signatures[index], - action.calldatas[index], - executionTime, - action.withDelegatecalls[index] - )); - } - - function _encodeHash(Action memory action, uint256 executionTime) internal pure returns (bytes32) { - return _encodeHash(action, 0, executionTime); - } - } diff --git a/test/OptimismCrosschainTest.t.sol b/test/OptimismCrosschainTest.t.sol index 8d029e0..21fe8f7 100644 --- a/test/OptimismCrosschainTest.t.sol +++ b/test/OptimismCrosschainTest.t.sol @@ -63,7 +63,7 @@ contract OptimismCrosschainTest is CrosschainTestBase { bridgeExecutor ); - assertEq(receiver.l1Authority(), defaultL2BridgeExecutorArgs.ethereumGovernanceExecutor); + assertEq(receiver.l1Authority(), defaultL2BridgeExecutorArgs.ethereumGovernanceExecutor); assertEq(address(receiver.executor()), address(bridgeExecutor)); } From cde34adac03de75d84898b06f482382e1f68ebea Mon Sep 17 00:00:00 2001 From: Sam MacPherson Date: Fri, 17 May 2024 17:23:50 +0900 Subject: [PATCH 13/13] review fixes --- test/AuthBridgeExecutor.t.sol | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/AuthBridgeExecutor.t.sol b/test/AuthBridgeExecutor.t.sol index 81844f2..5f6b752 100644 --- a/test/AuthBridgeExecutor.t.sol +++ b/test/AuthBridgeExecutor.t.sol @@ -139,13 +139,13 @@ contract AuthBridgeExecutorTestBase is Test { return _encodeHash(action, 0, executionTime); } - function _assertActionSet(uint256 id, bool executed, bool canceled, uint256 executionTime, Action memory actions) internal view { + function _assertActionSet(uint256 id, bool executed, bool canceled, uint256 executionTime, Action memory action) internal view { IExecutorBase.ActionsSet memory actionsSet = executor.getActionsSetById(id); - assertEq(actionsSet.targets, actions.targets); - assertEq(actionsSet.values, actions.values); - assertEq(actionsSet.signatures, actions.signatures); - assertEq(actionsSet.calldatas, actions.calldatas); - assertEq(actionsSet.withDelegatecalls, actions.withDelegatecalls); + assertEq(actionsSet.targets, action.targets); + assertEq(actionsSet.values, action.values); + assertEq(actionsSet.signatures, action.signatures); + assertEq(actionsSet.calldatas, action.calldatas); + assertEq(actionsSet.withDelegatecalls, action.withDelegatecalls); assertEq(actionsSet.executionTime, executionTime); assertEq(actionsSet.executed, executed); assertEq(actionsSet.canceled, canceled); @@ -271,7 +271,7 @@ contract AuthBridgeExecutorQueueTests is AuthBridgeExecutorTestBase { executed: false, canceled: false, executionTime: block.timestamp + DELAY, - actions: action + action: action }); // Can queue up the same action 1 second later @@ -296,13 +296,13 @@ contract AuthBridgeExecutorQueueTests is AuthBridgeExecutorTestBase { executed: false, canceled: false, executionTime: block.timestamp + DELAY, - actions: action + action: action }); } } -contract AuthBridgeExecutorExecutorTests is AuthBridgeExecutorTestBase { +contract AuthBridgeExecutorExecuteTests is AuthBridgeExecutorTestBase { function test_execute_actionsSetIdTooHigh_boundary() public { assertEq(executor.getActionsSetCount(), 0);