From 7e3a8a0b99c284e2c71c724425aced6942f59ea7 Mon Sep 17 00:00:00 2001 From: Evaldas Latoskinas Date: Fri, 28 Jun 2024 14:18:29 +0200 Subject: [PATCH] feat: remove messageId equality check with message hash --- .../v0.8/ccip/offRamp/EVM2EVMMultiOffRamp.sol | 7 +--- .../test/offRamp/EVM2EVMMultiOffRamp.t.sol | 32 +++++++------------ 2 files changed, 12 insertions(+), 27 deletions(-) diff --git a/contracts/src/v0.8/ccip/offRamp/EVM2EVMMultiOffRamp.sol b/contracts/src/v0.8/ccip/offRamp/EVM2EVMMultiOffRamp.sol index aa869382a9..101bb3acf4 100644 --- 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 index 978ff5bb85..80ef95594d 100644 --- 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 {