From d22798fa3953cf76df5e7d7b76e323985f850a9c Mon Sep 17 00:00:00 2001 From: Michael Zhou Date: Mon, 9 Oct 2023 13:09:06 -0700 Subject: [PATCH] Store hash of execution data Reduces storage gas cost, but requires caller to pass in full execution data. Fixes #35. --- src/MultiBridgeMessageReceiver.sol | 29 +++-- .../IMultiBridgeMessageReceiver.sol | 2 +- src/libraries/Error.sol | 3 + .../integration-tests/GracePeriodExpiry.t.sol | 24 +++- .../MultiMessageAggregation.t.sol | 24 +++- test/integration-tests/RemoteAdapterAdd.t.sol | 44 +++++-- .../RemoteAdapterRemove.t.sol | 92 +++++++++------ test/integration-tests/RemoteSetQuorum.t.sol | 34 ++++-- .../RemoteTimelockUpdate.t.sol | 35 ++++-- test/integration-tests/TimelockCheck.t.sol | 24 +++- .../MultiBridgeMessageReceiver.t.sol | 110 +++++++++++++++--- 11 files changed, 320 insertions(+), 101 deletions(-) diff --git a/src/MultiBridgeMessageReceiver.sol b/src/MultiBridgeMessageReceiver.sol index 5df92c0..584a365 100644 --- a/src/MultiBridgeMessageReceiver.sol +++ b/src/MultiBridgeMessageReceiver.sol @@ -42,8 +42,8 @@ contract MultiBridgeMessageReceiver is IMultiBridgeMessageReceiver, ExecutorAwar /// @notice count of bridge adapters that have delivered each message mapping(bytes32 msgId => uint256 deliveryCount) public msgDeliveryCount; - /// @notice the data required for executing a message - mapping(bytes32 msgId => ExecutionData execData) public msgExecData; + /// @notice the hash of the data required for executing a message + mapping(bytes32 msgId => bytes32 execDataHash) public msgExecDataHash; /// @notice whether a message has been sent to the governance timelock for execution mapping(bytes32 msgId => bool scheduled) public isExecutionScheduled; @@ -130,13 +130,15 @@ contract MultiBridgeMessageReceiver is IMultiBridgeMessageReceiver, ExecutorAwar /// increment quorum ++msgDeliveryCount[msgId]; - /// stores the execution data required - ExecutionData memory prevStored = msgExecData[msgId]; + /// stores the hash of the execution data required + bytes32 prevStoredHash = msgExecDataHash[msgId]; /// stores the message if the amb is the first one delivering the message - if (prevStored.target == address(0)) { - msgExecData[msgId] = ExecutionData( - _message.target, _message.callData, _message.nativeValue, _message.nonce, _message.expiration + if (prevStoredHash == bytes32(0)) { + msgExecDataHash[msgId] = keccak256( + abi.encodePacked( + _message.target, _message.callData, _message.nativeValue, _message.nonce, _message.expiration + ) ); } @@ -145,8 +147,17 @@ contract MultiBridgeMessageReceiver is IMultiBridgeMessageReceiver, ExecutorAwar } /// @inheritdoc IMultiBridgeMessageReceiver - function scheduleMessageExecution(bytes32 _msgId) external override { - ExecutionData memory _execData = msgExecData[_msgId]; + function scheduleMessageExecution(bytes32 _msgId, ExecutionData calldata _execData) external override { + bytes32 execDataHash = msgExecDataHash[_msgId]; + if ( + keccak256( + abi.encodePacked( + _execData.target, _execData.callData, _execData.value, _execData.nonce, _execData.expiration + ) + ) != execDataHash + ) { + revert Error.EXEC_DATA_HASH_MISMATCH(); + } /// @dev validates if msg execution is not beyond expiration if (block.timestamp > _execData.expiration) { diff --git a/src/interfaces/IMultiBridgeMessageReceiver.sol b/src/interfaces/IMultiBridgeMessageReceiver.sol index b7dfb1d..8df4b91 100644 --- a/src/interfaces/IMultiBridgeMessageReceiver.sol +++ b/src/interfaces/IMultiBridgeMessageReceiver.sol @@ -60,7 +60,7 @@ interface IMultiBridgeMessageReceiver { /// @notice Sends a message, that has achieved quorum and has not yet expired, to the governance timelock for eventual execution. /// @param _msgId is the unique identifier of the message - function scheduleMessageExecution(bytes32 _msgId) external; + function scheduleMessageExecution(bytes32 _msgId, ExecutionData calldata _execData) external; /// @notice adds or removes bridge receiver adapters. /// @param _receiverAdapters the list of receiver adapters to add or remove diff --git a/src/libraries/Error.sol b/src/libraries/Error.sol index dc73ba6..de5b436 100644 --- a/src/libraries/Error.sol +++ b/src/libraries/Error.sol @@ -66,6 +66,9 @@ library Error { /// @dev is thrown if refund address is zero (or) invalid error INVALID_REFUND_ADDRESS(); + /// @dev is thrown if execution data does not match the stored hash + error EXEC_DATA_HASH_MISMATCH(); + /*///////////////////////////////////////////////////////////////// ADAPTER ERRORS ////////////////////////////////////////////////////////////////*/ diff --git a/test/integration-tests/GracePeriodExpiry.t.sol b/test/integration-tests/GracePeriodExpiry.t.sol index a783ff9..4bbf38c 100644 --- a/test/integration-tests/GracePeriodExpiry.t.sol +++ b/test/integration-tests/GracePeriodExpiry.t.sol @@ -11,6 +11,7 @@ import "test/contracts-mock/MockUniswapReceiver.sol"; import {MultiBridgeMessageSender} from "src/MultiBridgeMessageSender.sol"; import {MultiBridgeMessageReceiver} from "src/MultiBridgeMessageReceiver.sol"; +import {IMultiBridgeMessageReceiver} from "src/interfaces/IMultiBridgeMessageReceiver.sol"; import {Error} from "src/libraries/Error.sol"; import {GovernanceTimelock} from "src/controllers/GovernanceTimelock.sol"; @@ -40,11 +41,17 @@ contract GracePeriodExpiryTest is Setup { 0.01 ether, wormholeFee ); - MultiBridgeMessageSender(contractAddress[SRC_CHAIN_ID][bytes("MMA_SENDER")]).remoteCall{value: 2 ether}( + + bytes memory callData = abi.encode(MockUniswapReceiver.setValue.selector, ""); + uint256 nativeValue = 0; + uint256 expiration = block.timestamp + EXPIRATION_CONSTANT; + MultiBridgeMessageSender sender = MultiBridgeMessageSender(contractAddress[SRC_CHAIN_ID][bytes("MMA_SENDER")]); + uint256 nonce = sender.nonce() + 1; + sender.remoteCall{value: 2 ether}( DST_CHAIN_ID, address(target), - abi.encode(MockUniswapReceiver.setValue.selector, ""), - 0, + callData, + nativeValue, EXPIRATION_CONSTANT, refundAddress, fees, @@ -63,7 +70,16 @@ contract GracePeriodExpiryTest is Setup { vm.selectFork(fork[DST_CHAIN_ID]); vm.recordLogs(); /// schedule the message for execution by moving it to governance timelock contract - MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).scheduleMessageExecution(msgId); + MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).scheduleMessageExecution( + msgId, + IMultiBridgeMessageReceiver.ExecutionData({ + target: address(target), + callData: callData, + value: nativeValue, + nonce: nonce, + expiration: expiration + }) + ); (uint256 txId, address finalTarget, uint256 value, bytes memory data, uint256 eta) = _getExecParams(vm.getRecordedLogs()); diff --git a/test/integration-tests/MultiMessageAggregation.t.sol b/test/integration-tests/MultiMessageAggregation.t.sol index f09b6da..e0339f5 100644 --- a/test/integration-tests/MultiMessageAggregation.t.sol +++ b/test/integration-tests/MultiMessageAggregation.t.sol @@ -11,6 +11,7 @@ import "test/contracts-mock/MockUniswapReceiver.sol"; import {MultiBridgeMessageSender} from "src/MultiBridgeMessageSender.sol"; import {MultiBridgeMessageReceiver} from "src/MultiBridgeMessageReceiver.sol"; +import {IMultiBridgeMessageReceiver} from "src/interfaces/IMultiBridgeMessageReceiver.sol"; import {Error} from "src/libraries/Error.sol"; import {GovernanceTimelock} from "src/controllers/GovernanceTimelock.sol"; @@ -40,11 +41,17 @@ contract MultiBridgeMessageAggregationTest is Setup { 0.01 ether, wormholeFee ); - MultiBridgeMessageSender(contractAddress[SRC_CHAIN_ID][bytes("MMA_SENDER")]).remoteCall{value: 2 ether}( + + bytes memory callData = abi.encode(MockUniswapReceiver.setValue.selector, ""); + uint256 nativeValue = 0; + uint256 expiration = block.timestamp + EXPIRATION_CONSTANT; + MultiBridgeMessageSender sender = MultiBridgeMessageSender(contractAddress[SRC_CHAIN_ID][bytes("MMA_SENDER")]); + uint256 nonce = sender.nonce() + 1; + sender.remoteCall{value: 2 ether}( DST_CHAIN_ID, address(target), - abi.encode(MockUniswapReceiver.setValue.selector, ""), - 0, + callData, + nativeValue, EXPIRATION_CONSTANT, refundAddress, fees, @@ -63,7 +70,16 @@ contract MultiBridgeMessageAggregationTest is Setup { vm.selectFork(fork[DST_CHAIN_ID]); vm.recordLogs(); /// schedule the message for execution by moving it to governance timelock contract - MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).scheduleMessageExecution(msgId); + MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).scheduleMessageExecution( + msgId, + IMultiBridgeMessageReceiver.ExecutionData({ + target: address(target), + callData: callData, + value: nativeValue, + nonce: nonce, + expiration: expiration + }) + ); (uint256 txId, address finalTarget, uint256 value, bytes memory data, uint256 eta) = _getExecParams(vm.getRecordedLogs()); diff --git a/test/integration-tests/RemoteAdapterAdd.t.sol b/test/integration-tests/RemoteAdapterAdd.t.sol index 573e16e..f2a49b7 100644 --- a/test/integration-tests/RemoteAdapterAdd.t.sol +++ b/test/integration-tests/RemoteAdapterAdd.t.sol @@ -10,6 +10,7 @@ import "test/Setup.t.sol"; import {MultiBridgeMessageSender} from "src/MultiBridgeMessageSender.sol"; import {MultiBridgeMessageReceiver} from "src/MultiBridgeMessageReceiver.sol"; +import {IMultiBridgeMessageReceiver} from "src/interfaces/IMultiBridgeMessageReceiver.sol"; import {Error} from "src/libraries/Error.sol"; import {GovernanceTimelock} from "src/controllers/GovernanceTimelock.sol"; @@ -51,7 +52,7 @@ contract RemoteAdapterAdd is Setup { _adapterAdd(adaptersToAdd, operation); } - function _adapterAdd(address[] memory adaptersToAdd, bool[] memory operation) internal { + function _adapterAdd(address[] memory adaptersToAdd, bool[] memory operation) private { vm.selectFork(fork[ETHEREUM_CHAIN_ID]); vm.startPrank(caller); @@ -65,10 +66,28 @@ contract RemoteAdapterAdd is Setup { 0.01 ether, wormholeFee ); - MultiBridgeMessageSender(contractAddress[SRC_CHAIN_ID][bytes("MMA_SENDER")]).remoteCall{value: 2 ether}( + + _sendAndExecuteMessage(adaptersToAdd, operation, fees); + + for (uint256 j; j < adaptersToAdd.length; ++j) { + bool isTrusted = MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]) + .isTrustedExecutor(adaptersToAdd[j]); + assert(isTrusted); + } + } + + function _sendAndExecuteMessage(address[] memory adaptersToAdd, bool[] memory operation, uint256[] memory fees) + private + { + MultiBridgeMessageSender sender = MultiBridgeMessageSender(contractAddress[SRC_CHAIN_ID][bytes("MMA_SENDER")]); + uint256 nonce = sender.nonce() + 1; + bytes memory callData = + abi.encodeWithSelector(MultiBridgeMessageReceiver.updateReceiverAdapters.selector, adaptersToAdd, operation); + uint256 expiration = block.timestamp + EXPIRATION_CONSTANT; + sender.remoteCall{value: 2 ether}( DST_CHAIN_ID, - address(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]), - abi.encodeWithSelector(MultiBridgeMessageReceiver.updateReceiverAdapters.selector, adaptersToAdd, operation), + contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")], + callData, 0, EXPIRATION_CONSTANT, refundAddress, @@ -88,7 +107,16 @@ contract RemoteAdapterAdd is Setup { vm.selectFork(fork[DST_CHAIN_ID]); vm.recordLogs(); /// schedule the message for execution by moving it to governance timelock contract - MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).scheduleMessageExecution(msgId); + MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).scheduleMessageExecution( + msgId, + IMultiBridgeMessageReceiver.ExecutionData({ + target: contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")], + callData: callData, + value: 0, + nonce: nonce, + expiration: expiration + }) + ); (uint256 txId, address finalTarget, uint256 value, bytes memory data, uint256 eta) = _getExecParams(vm.getRecordedLogs()); @@ -97,11 +125,5 @@ contract RemoteAdapterAdd is Setup { GovernanceTimelock(contractAddress[DST_CHAIN_ID][bytes("TIMELOCK")]).executeTransaction( txId, finalTarget, value, data, eta ); - - for (uint256 j; j < adaptersToAdd.length; ++j) { - bool isTrusted = MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]) - .isTrustedExecutor(adaptersToAdd[j]); - assert(isTrusted); - } } } diff --git a/test/integration-tests/RemoteAdapterRemove.t.sol b/test/integration-tests/RemoteAdapterRemove.t.sol index d5f2011..aa8a023 100644 --- a/test/integration-tests/RemoteAdapterRemove.t.sol +++ b/test/integration-tests/RemoteAdapterRemove.t.sol @@ -10,6 +10,7 @@ import "test/Setup.t.sol"; import {MultiBridgeMessageSender} from "src/MultiBridgeMessageSender.sol"; import {MultiBridgeMessageReceiver} from "src/MultiBridgeMessageReceiver.sol"; +import {IMultiBridgeMessageReceiver} from "src/interfaces/IMultiBridgeMessageReceiver.sol"; import {Error} from "src/libraries/Error.sol"; import {GovernanceTimelock} from "src/controllers/GovernanceTimelock.sol"; @@ -70,15 +71,52 @@ contract RemoteAdapterRemove is Setup { 0.01 ether, wormholeFee ); - MultiBridgeMessageSender(contractAddress[SRC_CHAIN_ID][bytes("MMA_SENDER")]).remoteCall{value: 2 ether}( + + _sendAndExecuteMessage(newQuorum, adaptersToRemove, operation, fees); + + /// @dev validates quorum post update + assertEq(MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).quorum(), newQuorum); + + /// @dev validates adapters post update + for (uint256 j; j < adaptersToRemove.length; ++j) { + bool isTrusted = MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]) + .isTrustedExecutor(adaptersToRemove[j]); + assert(!isTrusted); + } + } + + function _updateDummy() private { + address[] memory newDummyAdapter = new address[](1); + newDummyAdapter[0] = address(420); + + /// true = add + /// false = remove + bool[] memory operation = new bool[](1); + operation[0] = true; + + vm.startPrank(contractAddress[DST_CHAIN_ID]["TIMELOCK"]); + MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID]["MMA_RECEIVER"]).updateReceiverAdapters( + newDummyAdapter, operation + ); + vm.stopPrank(); + } + + function _sendAndExecuteMessage( + uint256 newQuorum, + address[] memory adaptersToRemove, + bool[] memory operation, + uint256[] memory fees + ) private { + bytes memory callData = abi.encodeWithSelector( + MultiBridgeMessageReceiver.updateReceiverAdaptersAndQuorum.selector, adaptersToRemove, operation, newQuorum + ); + uint256 expiration = block.timestamp + EXPIRATION_CONSTANT; + MultiBridgeMessageSender sender = MultiBridgeMessageSender(contractAddress[SRC_CHAIN_ID][bytes("MMA_SENDER")]); + uint256 nonce = sender.nonce() + 1; + sender.remoteCall{value: 2 ether}( DST_CHAIN_ID, - address(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]), - abi.encodeWithSelector( - MultiBridgeMessageReceiver.updateReceiverAdaptersAndQuorum.selector, - adaptersToRemove, - operation, - newQuorum - ), + contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")], + callData, 0, EXPIRATION_CONSTANT, refundAddress, @@ -98,7 +136,16 @@ contract RemoteAdapterRemove is Setup { vm.selectFork(fork[DST_CHAIN_ID]); vm.recordLogs(); /// schedule the message for execution by moving it to governance timelock contract - MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).scheduleMessageExecution(msgId); + MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).scheduleMessageExecution( + msgId, + IMultiBridgeMessageReceiver.ExecutionData({ + target: contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")], + callData: callData, + value: 0, + nonce: nonce, + expiration: expiration + }) + ); (uint256 txId, address finalTarget, uint256 value, bytes memory data, uint256 eta) = _getExecParams(vm.getRecordedLogs()); @@ -107,32 +154,5 @@ contract RemoteAdapterRemove is Setup { GovernanceTimelock(contractAddress[DST_CHAIN_ID][bytes("TIMELOCK")]).executeTransaction( txId, finalTarget, value, data, eta ); - - /// @dev validates quorum post update - uint256 currQuorum = MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).quorum(); - assertEq(currQuorum, newQuorum); - - /// @dev validates adapters post update - for (uint256 j; j < adaptersToRemove.length; ++j) { - bool isTrusted = MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]) - .isTrustedExecutor(adaptersToRemove[j]); - assert(!isTrusted); - } - } - - function _updateDummy() internal { - address[] memory newDummyAdapter = new address[](1); - newDummyAdapter[0] = address(420); - - /// true = add - /// false = remove - bool[] memory operation = new bool[](1); - operation[0] = true; - - vm.startPrank(contractAddress[DST_CHAIN_ID]["TIMELOCK"]); - MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID]["MMA_RECEIVER"]).updateReceiverAdapters( - newDummyAdapter, operation - ); - vm.stopPrank(); } } diff --git a/test/integration-tests/RemoteSetQuorum.t.sol b/test/integration-tests/RemoteSetQuorum.t.sol index ac10f04..191bd6b 100644 --- a/test/integration-tests/RemoteSetQuorum.t.sol +++ b/test/integration-tests/RemoteSetQuorum.t.sol @@ -10,6 +10,7 @@ import "test/Setup.t.sol"; import {MultiBridgeMessageSender} from "src/MultiBridgeMessageSender.sol"; import {MultiBridgeMessageReceiver} from "src/MultiBridgeMessageReceiver.sol"; +import {IMultiBridgeMessageReceiver} from "src/interfaces/IMultiBridgeMessageReceiver.sol"; import {Error} from "src/libraries/Error.sol"; import {GovernanceTimelock} from "src/controllers/GovernanceTimelock.sol"; @@ -37,10 +38,23 @@ contract RemoteQuorumUpdate is Setup { 0.01 ether, wormholeFee ); - MultiBridgeMessageSender(contractAddress[SRC_CHAIN_ID][bytes("MMA_SENDER")]).remoteCall{value: 2 ether}( + + _sendAndExecuteMessage(newQuorum, fees); + + uint256 currQuorum = MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).quorum(); + assertEq(currQuorum, newQuorum); + } + + function _sendAndExecuteMessage(uint256 newQuorum, uint256[] memory fees) private { + address receiverAddr = contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]; + bytes memory callData = abi.encodeWithSelector(MultiBridgeMessageReceiver.updateQuorum.selector, newQuorum); + uint256 expiration = block.timestamp + EXPIRATION_CONSTANT; + MultiBridgeMessageSender sender = MultiBridgeMessageSender(contractAddress[SRC_CHAIN_ID][bytes("MMA_SENDER")]); + uint256 nonce = sender.nonce() + 1; + sender.remoteCall{value: 2 ether}( DST_CHAIN_ID, - address(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]), - abi.encodeWithSelector(MultiBridgeMessageReceiver.updateQuorum.selector, newQuorum), + receiverAddr, + callData, 0, EXPIRATION_CONSTANT, refundAddress, @@ -61,7 +75,16 @@ contract RemoteQuorumUpdate is Setup { vm.selectFork(fork[DST_CHAIN_ID]); vm.recordLogs(); /// schedule the message for execution by moving it to governance timelock contract - MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).scheduleMessageExecution(msgId); + MultiBridgeMessageReceiver(receiverAddr).scheduleMessageExecution( + msgId, + IMultiBridgeMessageReceiver.ExecutionData({ + target: receiverAddr, + callData: callData, + value: 0, + nonce: nonce, + expiration: expiration + }) + ); (uint256 txId, address finalTarget, uint256 value, bytes memory data, uint256 eta) = _getExecParams(vm.getRecordedLogs()); @@ -73,8 +96,5 @@ contract RemoteQuorumUpdate is Setup { GovernanceTimelock(contractAddress[DST_CHAIN_ID][bytes("TIMELOCK")]).executeTransaction( txId, finalTarget, value, data, eta ); - - uint256 currQuorum = MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).quorum(); - assertEq(currQuorum, newQuorum); } } diff --git a/test/integration-tests/RemoteTimelockUpdate.t.sol b/test/integration-tests/RemoteTimelockUpdate.t.sol index 4c1bb28..c9f570e 100644 --- a/test/integration-tests/RemoteTimelockUpdate.t.sol +++ b/test/integration-tests/RemoteTimelockUpdate.t.sol @@ -10,6 +10,7 @@ import "test/Setup.t.sol"; import {MultiBridgeMessageSender} from "src/MultiBridgeMessageSender.sol"; import {MultiBridgeMessageReceiver} from "src/MultiBridgeMessageReceiver.sol"; +import {IMultiBridgeMessageReceiver} from "src/interfaces/IMultiBridgeMessageReceiver.sol"; import {Error} from "src/libraries/Error.sol"; import {GovernanceTimelock} from "src/controllers/GovernanceTimelock.sol"; @@ -37,11 +38,25 @@ contract RemoteTimelockUpdate is Setup { 0.01 ether, wormholeFee ); - MultiBridgeMessageSender(contractAddress[SRC_CHAIN_ID][bytes("MMA_SENDER")]).remoteCall{value: 2 ether}( + + _sendAndExecuteMessage(newDelay, fees); + + uint256 currDelay = GovernanceTimelock(contractAddress[POLYGON_CHAIN_ID][bytes("TIMELOCK")]).delay(); + assertEq(currDelay, newDelay); + } + + function _sendAndExecuteMessage(uint256 newDelay, uint256[] memory fees) private { + address timelockAddr = contractAddress[POLYGON_CHAIN_ID][bytes("TIMELOCK")]; + bytes memory callData = abi.encodeWithSelector(GovernanceTimelock.setDelay.selector, newDelay); + uint256 nativeValue = 0; + uint256 expiration = block.timestamp + EXPIRATION_CONSTANT; + MultiBridgeMessageSender sender = MultiBridgeMessageSender(contractAddress[SRC_CHAIN_ID][bytes("MMA_SENDER")]); + uint256 nonce = sender.nonce() + 1; + sender.remoteCall{value: 2 ether}( POLYGON_CHAIN_ID, - address(contractAddress[POLYGON_CHAIN_ID][bytes("TIMELOCK")]), - abi.encodeWithSelector(GovernanceTimelock.setDelay.selector, newDelay), - 0, + timelockAddr, + callData, + nativeValue, EXPIRATION_CONSTANT, refundAddress, fees, @@ -62,7 +77,14 @@ contract RemoteTimelockUpdate is Setup { vm.recordLogs(); /// schedule the message for execution by moving it to governance timelock contract MultiBridgeMessageReceiver(contractAddress[POLYGON_CHAIN_ID][bytes("MMA_RECEIVER")]).scheduleMessageExecution( - msgId + msgId, + IMultiBridgeMessageReceiver.ExecutionData({ + target: timelockAddr, + callData: callData, + value: nativeValue, + nonce: nonce, + expiration: expiration + }) ); (uint256 txId, address finalTarget, uint256 value, bytes memory data, uint256 eta) = _getExecParams(vm.getRecordedLogs()); @@ -75,8 +97,5 @@ contract RemoteTimelockUpdate is Setup { GovernanceTimelock(contractAddress[POLYGON_CHAIN_ID][bytes("TIMELOCK")]).executeTransaction( txId, finalTarget, value, data, eta ); - - uint256 currDelay = GovernanceTimelock(contractAddress[POLYGON_CHAIN_ID][bytes("TIMELOCK")]).delay(); - assertEq(currDelay, newDelay); } } diff --git a/test/integration-tests/TimelockCheck.t.sol b/test/integration-tests/TimelockCheck.t.sol index 1a4891d..9fd66fa 100644 --- a/test/integration-tests/TimelockCheck.t.sol +++ b/test/integration-tests/TimelockCheck.t.sol @@ -11,6 +11,7 @@ import "test/contracts-mock/MockUniswapReceiver.sol"; import {MultiBridgeMessageSender} from "src/MultiBridgeMessageSender.sol"; import {MultiBridgeMessageReceiver} from "src/MultiBridgeMessageReceiver.sol"; +import {IMultiBridgeMessageReceiver} from "src/interfaces/IMultiBridgeMessageReceiver.sol"; import {Error} from "src/libraries/Error.sol"; import {GovernanceTimelock} from "src/controllers/GovernanceTimelock.sol"; @@ -42,11 +43,17 @@ contract TimelockCheckTest is Setup { 0.01 ether, wormholeFee ); - MultiBridgeMessageSender(contractAddress[SRC_CHAIN_ID][bytes("MMA_SENDER")]).remoteCall{value: 2 ether}( + + bytes memory callData = abi.encode(MockUniswapReceiver.setValue.selector, ""); + uint256 nativeValue = 0; + uint256 expiration = block.timestamp + EXPIRATION_CONSTANT; + MultiBridgeMessageSender sender = MultiBridgeMessageSender(contractAddress[SRC_CHAIN_ID][bytes("MMA_SENDER")]); + uint256 nonce = sender.nonce() + 1; + sender.remoteCall{value: 2 ether}( DST_CHAIN_ID, address(target), - abi.encode(MockUniswapReceiver.setValue.selector, ""), - 0, + callData, + nativeValue, EXPIRATION_CONSTANT, refundAddress, fees, @@ -65,7 +72,16 @@ contract TimelockCheckTest is Setup { vm.selectFork(fork[DST_CHAIN_ID]); vm.recordLogs(); /// schedule the message for execution by moving it to governance timelock contract - MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).scheduleMessageExecution(msgId); + MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).scheduleMessageExecution( + msgId, + IMultiBridgeMessageReceiver.ExecutionData({ + target: address(target), + callData: callData, + value: nativeValue, + nonce: nonce, + expiration: expiration + }) + ); (uint256 txId, address finalTarget, uint256 value, bytes memory data, uint256 eta) = _getExecParams(vm.getRecordedLogs()); diff --git a/test/unit-tests/MultiBridgeMessageReceiver.t.sol b/test/unit-tests/MultiBridgeMessageReceiver.t.sol index 2572d85..4f30dc1 100644 --- a/test/unit-tests/MultiBridgeMessageReceiver.t.sol +++ b/test/unit-tests/MultiBridgeMessageReceiver.t.sol @@ -10,6 +10,7 @@ import "src/adapters/wormhole/WormholeReceiverAdapter.sol"; import "src/libraries/Error.sol"; import "src/libraries/Message.sol"; import {MultiBridgeMessageReceiver} from "src/MultiBridgeMessageReceiver.sol"; +import {IMultiBridgeMessageReceiver} from "src/interfaces/IMultiBridgeMessageReceiver.sol"; contract MultiBridgeMessageReceiverTest is Setup { event BridgeReceiverAdapterUpdated(address indexed receiverAdapter, bool add); @@ -62,7 +63,12 @@ contract MultiBridgeMessageReceiverTest is Setup { receiverAdapters[0] = address(43); vm.expectRevert(Error.ZERO_ADDRESS_INPUT.selector); - new MultiBridgeMessageReceiver(SRC_CHAIN_ID, address(0), receiverAdapters, 1); + new MultiBridgeMessageReceiver( + SRC_CHAIN_ID, + address(0), + receiverAdapters, + 1 + ); } /// @dev cannot be called with receiver adapters containing zero address @@ -71,7 +77,12 @@ contract MultiBridgeMessageReceiverTest is Setup { receiverAdapters[0] = address(0); vm.expectRevert(Error.ZERO_ADDRESS_INPUT.selector); - new MultiBridgeMessageReceiver(SRC_CHAIN_ID, address(42), receiverAdapters, 1); + new MultiBridgeMessageReceiver( + SRC_CHAIN_ID, + address(42), + receiverAdapters, + 1 + ); } /// @dev cannot be called with zero quorum @@ -80,7 +91,12 @@ contract MultiBridgeMessageReceiverTest is Setup { receiverAdapters[0] = address(42); vm.expectRevert(Error.INVALID_QUORUM_THRESHOLD.selector); - new MultiBridgeMessageReceiver(SRC_CHAIN_ID, address(43), receiverAdapters, 0); + new MultiBridgeMessageReceiver( + SRC_CHAIN_ID, + address(43), + receiverAdapters, + 0 + ); } /// @dev cannot be called with quorum too large @@ -89,7 +105,12 @@ contract MultiBridgeMessageReceiverTest is Setup { receiverAdapters[0] = address(42); vm.expectRevert(Error.INVALID_QUORUM_THRESHOLD.selector); - new MultiBridgeMessageReceiver(SRC_CHAIN_ID, address(43), receiverAdapters, 2); + new MultiBridgeMessageReceiver( + SRC_CHAIN_ID, + address(43), + receiverAdapters, + 2 + ); } /// @dev receives message from one adapter @@ -118,13 +139,14 @@ contract MultiBridgeMessageReceiverTest is Setup { assertEq(receiver.msgDeliveryCount(msgId), 1); - (address target, bytes memory callData, uint256 nativeValue, uint256 nonce, uint256 expiration) = - receiver.msgExecData(msgId); - assertEq(target, message.target); - assertEq(callData, message.callData); - assertEq(nativeValue, message.nativeValue); - assertEq(nonce, message.nonce); - assertEq(expiration, message.expiration); + assertEq( + receiver.msgExecDataHash(msgId), + keccak256( + abi.encodePacked( + message.target, message.callData, message.nativeValue, message.nonce, message.expiration + ) + ) + ); } /// @dev receives message from two adapters @@ -262,7 +284,16 @@ contract MultiBridgeMessageReceiverTest is Setup { vm.startPrank(wormholeAdapterAddr); receiver.receiveMessage(message); - receiver.scheduleMessageExecution(msgId); + receiver.scheduleMessageExecution( + msgId, + IMultiBridgeMessageReceiver.ExecutionData({ + target: message.target, + callData: message.callData, + value: message.nativeValue, + nonce: message.nonce, + expiration: message.expiration + }) + ); vm.startPrank(axelarAdapterAddr); vm.expectRevert(Error.MSG_ID_ALREADY_SCHEDULED.selector); @@ -292,7 +323,16 @@ contract MultiBridgeMessageReceiverTest is Setup { vm.expectEmit(true, true, true, true, address(receiver)); emit MessageExecutionScheduled(msgId, address(42), 0, 42, bytes("42")); - receiver.scheduleMessageExecution(msgId); + receiver.scheduleMessageExecution( + msgId, + IMultiBridgeMessageReceiver.ExecutionData({ + target: message.target, + callData: message.callData, + value: message.nativeValue, + nonce: message.nonce, + expiration: message.expiration + }) + ); assertTrue(receiver.isExecutionScheduled(msgId)); } @@ -315,7 +355,16 @@ contract MultiBridgeMessageReceiverTest is Setup { receiver.receiveMessage(message); vm.expectRevert(Error.MSG_EXECUTION_PASSED_DEADLINE.selector); - receiver.scheduleMessageExecution(msgId); + receiver.scheduleMessageExecution( + msgId, + IMultiBridgeMessageReceiver.ExecutionData({ + target: message.target, + callData: message.callData, + value: message.nativeValue, + nonce: message.nonce, + expiration: message.expiration + }) + ); } /// @dev cannot schedule execution of message that has already been scheduled @@ -338,10 +387,28 @@ contract MultiBridgeMessageReceiverTest is Setup { vm.startPrank(axelarAdapterAddr); receiver.receiveMessage(message); - receiver.scheduleMessageExecution(msgId); + receiver.scheduleMessageExecution( + msgId, + IMultiBridgeMessageReceiver.ExecutionData({ + target: message.target, + callData: message.callData, + value: message.nativeValue, + nonce: message.nonce, + expiration: message.expiration + }) + ); vm.expectRevert(Error.MSG_ID_ALREADY_SCHEDULED.selector); - receiver.scheduleMessageExecution(msgId); + receiver.scheduleMessageExecution( + msgId, + IMultiBridgeMessageReceiver.ExecutionData({ + target: message.target, + callData: message.callData, + value: message.nativeValue, + nonce: message.nonce, + expiration: message.expiration + }) + ); } /// @dev cannot schedule message execution without quorum @@ -362,7 +429,16 @@ contract MultiBridgeMessageReceiverTest is Setup { receiver.receiveMessage(message); vm.expectRevert(Error.QUORUM_NOT_ACHIEVED.selector); - receiver.scheduleMessageExecution(msgId); + receiver.scheduleMessageExecution( + msgId, + IMultiBridgeMessageReceiver.ExecutionData({ + target: message.target, + callData: message.callData, + value: message.nativeValue, + nonce: message.nonce, + expiration: message.expiration + }) + ); } /// @dev updates governance timelock