Store hash of execution data

Reduces storage gas cost, but requires caller to pass in full execution
data.

Fixes #35.

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) {

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

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
  ////////////////////////////////////////////////////////////////*/

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());
 

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());
 

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);
- }
  }
 }

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();
  }
 }