feat: remove messageId equality check with message hash

smartcontractkit · Jun 28, 2024 · 7e3a8a0 · 7e3a8a0
1 parent 33de478
commit 7e3a8a0
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 27 deletions.
diff --git a/contracts/src/v0.8/ccip/offRamp/EVM2EVMMultiOffRamp.sol b/contracts/src/v0.8/ccip/offRamp/EVM2EVMMultiOffRamp.sol
@@ -51,7 +51,6 @@ contract EVM2EVMMultiOffRamp is ITypeAndVersion, MultiOCR3Base {
   error TokenHandlingError(bytes error);
   error EmptyReport();
   error CursedByRMN(uint64 sourceChainSelector);
-  error InvalidMessageId(bytes32 messageId);
   error NotACompatiblePool(address notPool);
   error InvalidDataLength(uint256 expected, uint256 got);
   error InvalidNewState(uint64 sourceChainSelector, uint64 sequenceNumber, Internal.MessageExecutionState newState);
@@ -375,12 +374,8 @@ contract EVM2EVMMultiOffRamp is ITypeAndVersion, MultiOCR3Base {
       // We do this hash here instead of in _verifyMessages to avoid two separate loops
       // over the same data, which increases gas cost
       // TODO: verify message.destChainSelector == config.destChainSelector
+      // (alternatively, validate this in the commit() flow)
       hashedLeaves[i] = Internal._hash(message, sourceChainConfig.onRamp);
-      // TODO: revisit this - is messageID independent of the leaf hash?
-      // For EVM2EVM offramps, the messageID is the leaf hash.
-      // Asserting that this is true ensures we don't accidentally commit and then execute
-      // a message with an unexpected hash.
-      if (hashedLeaves[i] != message.header.messageId) revert InvalidMessageId(message.header.messageId);
     }
 
     // SECURITY CRITICAL CHECK

diff --git a/contracts/src/v0.8/ccip/test/offRamp/EVM2EVMMultiOffRamp.t.sol b/contracts/src/v0.8/ccip/test/offRamp/EVM2EVMMultiOffRamp.t.sol
@@ -731,27 +731,17 @@ contract EVM2EVMMultiOffRamp_executeSingleReport is EVM2EVMMultiOffRampSetup {
 
   // Reverts
 
-  function test_InvalidMessageId_Revert() public {
-    Internal.Any2EVMRampMessage[] memory messages =
-      _generateSingleBasicMessage(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1);
-    messages[0].header.nonce++;
-    // MessageID no longer matches hash.
-    Internal.ExecutionReportSingleChain memory executionReport =
-      _generateReportFromMessages(SOURCE_CHAIN_SELECTOR_1, messages);
-    vm.expectRevert(abi.encodeWithSelector(EVM2EVMMultiOffRamp.InvalidMessageId.selector, messages[0].header.messageId));
-    s_offRamp.executeSingleReport(executionReport, new uint256[](0));
-  }
-
-  function test_MismatchingSourceChainSelector_Revert() public {
-    Internal.Any2EVMRampMessage[] memory messages =
-      _generateSingleBasicMessage(SOURCE_CHAIN_SELECTOR_3, ON_RAMP_ADDRESS_3);
-    messages[0].header.sourceChainSelector = SOURCE_CHAIN_SELECTOR_1;
-    // MessageID no longer matches hash.
-    Internal.ExecutionReportSingleChain memory executionReport =
-      _generateReportFromMessages(SOURCE_CHAIN_SELECTOR_1, messages);
-    vm.expectRevert(abi.encodeWithSelector(EVM2EVMMultiOffRamp.InvalidMessageId.selector, messages[0].header.messageId));
-    s_offRamp.executeSingleReport(executionReport, new uint256[](0));
-  }
+  // TODO: re-implement for mismatching dest chain selector
+  // function test_MismatchingSourceChainSelector_Revert() public {
+  //   Internal.Any2EVMRampMessage[] memory messages =
+  //     _generateSingleBasicMessage(SOURCE_CHAIN_SELECTOR_3, ON_RAMP_ADDRESS_3);
+  //   messages[0].header.sourceChainSelector = SOURCE_CHAIN_SELECTOR_1;
+  //   // MessageID no longer matches hash.
+  //   Internal.ExecutionReportSingleChain memory executionReport =
+  //     _generateReportFromMessages(SOURCE_CHAIN_SELECTOR_1, messages);
+  //   vm.expectRevert(abi.encodeWithSelector(EVM2EVMMultiOffRamp.InvalidMessageId.selector, messages[0].header.messageId));
+  //   s_offRamp.executeSingleReport(executionReport, new uint256[](0));
+  // }
 
   // TODO: re-implement for mismatching OnRamp
   // function test_MismatchingMetadataHash_Revert() public {