diff --git a/contracts/src/v0.8/ccip/libraries/Internal.sol b/contracts/src/v0.8/ccip/libraries/Internal.sol index af1fc5e583..8b0dc2a6ef 100644 --- a/contracts/src/v0.8/ccip/libraries/Internal.sol +++ b/contracts/src/v0.8/ccip/libraries/Internal.sol @@ -173,7 +173,9 @@ library Internal { ); } - function _hash(Any2EVMRampMessage memory original) internal pure returns (bytes32) { + bytes32 internal constant ANY_2_EVM_MESSAGE_HASH = keccak256("Any2EVMMessageHashV1"); + + function _hash(Any2EVMRampMessage memory original, bytes memory onRamp) internal pure returns (bytes32) { // Fixed-size message fields are included in nested hash to reduce stack pressure. // This hashing scheme is also used by RMN. If changing it, please notify the RMN maintainers. return keccak256( @@ -181,9 +183,9 @@ library Internal { MerkleMultiProof.LEAF_DOMAIN_SEPARATOR, // Implicit metadata hash keccak256( - // TODO: include prefix message hash - // TODO: include OnRamp address - abi.encode(original.header.sourceChainSelector, original.header.destChainSelector) + abi.encode( + ANY_2_EVM_MESSAGE_HASH, onRamp, original.header.sourceChainSelector, original.header.destChainSelector + ) ), keccak256( abi.encode( @@ -240,6 +242,7 @@ library Internal { } /// @notice Family-agnostic header for OnRamp & OffRamp messages + // TODO: revisit if destChainSelector is required (likely sufficient to have it implicitly in the commit roots) struct RampMessageHeader { bytes32 messageId; // Unique identifier for the message, with family-agnostic encoding (i.e. not necessarily abi.encoded) uint64 sourceChainSelector; // ───────╮ the chain selector of the source chain, note: not chainId diff --git a/contracts/src/v0.8/ccip/offRamp/EVM2EVMMultiOffRamp.sol b/contracts/src/v0.8/ccip/offRamp/EVM2EVMMultiOffRamp.sol index 3a88528f61..aa869382a9 100644 --- a/contracts/src/v0.8/ccip/offRamp/EVM2EVMMultiOffRamp.sol +++ b/contracts/src/v0.8/ccip/offRamp/EVM2EVMMultiOffRamp.sol @@ -362,9 +362,7 @@ contract EVM2EVMMultiOffRamp is ITypeAndVersion, MultiOCR3Base { uint64 sourceChainSelector = report.sourceChainSelector; _whenNotCursed(sourceChainSelector); - // TODO: currently unused - but should not be removed - it will be used once the message signature changes - // SourceChainConfig storage sourceChainConfig = _getEnabledSourceChainConfig(sourceChainSelector); - _getEnabledSourceChainConfig(sourceChainSelector); + SourceChainConfig storage sourceChainConfig = _getEnabledSourceChainConfig(sourceChainSelector); uint256 numMsgs = report.messages.length; if (numMsgs == 0) revert EmptyReport(); @@ -376,9 +374,8 @@ contract EVM2EVMMultiOffRamp is ITypeAndVersion, MultiOCR3Base { Internal.Any2EVMRampMessage memory message = report.messages[i]; // 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.onRamp == config.onRamp // TODO: verify message.destChainSelector == config.destChainSelector - hashedLeaves[i] = Internal._hash(message); + 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 @@ -788,6 +785,7 @@ contract EVM2EVMMultiOffRamp is ITypeAndVersion, MultiOCR3Base { // OnRamp can never be zero - if it is, then the source chain has been added for the first time if (currentOnRamp.length == 0) { + // TODO: is this check sufficient / is an all-0 check needed? if (newOnRamp.length == 0) { revert ZeroAddressNotAllowed(); } @@ -844,9 +842,11 @@ contract EVM2EVMMultiOffRamp is ITypeAndVersion, MultiOCR3Base { /// @param sourceAmount The amount of tokens to be released/minted. /// @param originalSender The message sender on the source chain. /// @param receiver The address that will receive the tokens. + /// @param sourceChainSelector The remote source chain selector /// @param sourceTokenData A struct containing the local token address, the source pool address and optional data /// returned from the source pool. /// @param offchainTokenData Data fetched offchain by the DON. + /// @return destTokenAmount local token address with amount function _releaseOrMintSingleToken( uint256 sourceAmount, bytes memory originalSender, @@ -917,10 +917,14 @@ contract EVM2EVMMultiOffRamp is ITypeAndVersion, MultiOCR3Base { return Client.EVMTokenAmount({token: localToken, amount: localAmount}); } - // TODO: add missing param comments /// @notice Uses pools to release or mint a number of different tokens to a receiver address. /// @param sourceTokenAmounts List of tokens and amount values to be released/minted. + /// @param originalSender The message sender on the source chain. + /// @param receiver The address that will receive the tokens. + /// @param sourceChainSelector The remote source chain selector + /// @param encodedSourceTokenData Encoded source token data, decoding to Internal.SourceTokenData /// @param offchainTokenData Array of token data fetched offchain by the DON. + /// @return destTokenAmounts local token addresses with amounts /// @dev This function wrappes the token pool call in a try catch block to gracefully handle /// any non-rate limiting errors that may occur. If we encounter a rate limiting related error /// we bubble it up. If we encounter a non-rate limiting error we wrap it in a TokenHandlingError. diff --git a/contracts/src/v0.8/ccip/test/e2e/MultiRampsEnd2End.sol b/contracts/src/v0.8/ccip/test/e2e/MultiRampsEnd2End.sol index f1c29077ba..f686221361 100644 --- a/contracts/src/v0.8/ccip/test/e2e/MultiRampsEnd2End.sol +++ b/contracts/src/v0.8/ccip/test/e2e/MultiRampsEnd2End.sol @@ -127,12 +127,12 @@ contract MultiRampsE2E is EVM2EVMMultiOnRampSetup, EVM2EVMMultiOffRampSetup { // Commit bytes32[] memory hashedMessages1 = new bytes32[](2); - hashedMessages1[0] = messages1[0]._hash(); + hashedMessages1[0] = messages1[0]._hash(abi.encode(address(s_onRamp))); messages1[0].header.messageId = hashedMessages1[0]; - hashedMessages1[1] = messages1[1]._hash(); + hashedMessages1[1] = messages1[1]._hash(abi.encode(address(s_onRamp))); messages1[1].header.messageId = hashedMessages1[1]; bytes32[] memory hashedMessages2 = new bytes32[](1); - hashedMessages2[0] = messages2[0]._hash(); + hashedMessages2[0] = messages2[0]._hash(abi.encode(address(s_onRamp2))); messages2[0].header.messageId = hashedMessages2[0]; bytes32[] memory merkleRoots = new bytes32[](2); 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 379d112086..978ff5bb85 100644 --- a/contracts/src/v0.8/ccip/test/offRamp/EVM2EVMMultiOffRamp.t.sol +++ b/contracts/src/v0.8/ccip/test/offRamp/EVM2EVMMultiOffRamp.t.sol @@ -342,7 +342,7 @@ contract EVM2EVMMultiOffRamp_executeSingleReport is EVM2EVMMultiOffRampSetup { messages[0].header.nonce++; messages[0].header.sequenceNumber++; - messages[0].header.messageId = Internal._hash(messages[0]); + messages[0].header.messageId = Internal._hash(messages[0], ON_RAMP_ADDRESS_1); vm.expectEmit(); emit EVM2EVMMultiOffRamp.ExecutionStateChanged( @@ -362,7 +362,7 @@ contract EVM2EVMMultiOffRamp_executeSingleReport is EVM2EVMMultiOffRampSetup { Internal.Any2EVMRampMessage[] memory messages = _generateSingleBasicMessage(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1); messages[0].header.nonce = 0; - messages[0].header.messageId = Internal._hash(messages[0]); + messages[0].header.messageId = Internal._hash(messages[0], ON_RAMP_ADDRESS_1); vm.expectEmit(); emit EVM2EVMMultiOffRamp.ExecutionStateChanged( @@ -383,7 +383,7 @@ contract EVM2EVMMultiOffRamp_executeSingleReport is EVM2EVMMultiOffRampSetup { ); messages[0].header.sequenceNumber++; - messages[0].header.messageId = Internal._hash(messages[0]); + messages[0].header.messageId = Internal._hash(messages[0], ON_RAMP_ADDRESS_1); vm.expectEmit(); emit EVM2EVMMultiOffRamp.ExecutionStateChanged( @@ -437,7 +437,7 @@ contract EVM2EVMMultiOffRamp_executeSingleReport is EVM2EVMMultiOffRampSetup { s_reverting_receiver.setErr(realError1); messages[0].receiver = address(s_reverting_receiver); - messages[0].header.messageId = Internal._hash(messages[0]); + messages[0].header.messageId = Internal._hash(messages[0], ON_RAMP_ADDRESS_1); vm.expectEmit(); emit EVM2EVMMultiOffRamp.ExecutionStateChanged( @@ -461,7 +461,7 @@ contract EVM2EVMMultiOffRamp_executeSingleReport is EVM2EVMMultiOffRampSetup { _generateSingleBasicMessage(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1); messages[0].header.nonce++; - messages[0].header.messageId = Internal._hash(messages[0]); + messages[0].header.messageId = Internal._hash(messages[0], ON_RAMP_ADDRESS_1); vm.expectEmit(); emit EVM2EVMMultiOffRamp.SkippedIncorrectNonce( @@ -476,7 +476,7 @@ contract EVM2EVMMultiOffRamp_executeSingleReport is EVM2EVMMultiOffRampSetup { _generateMessagesWithTokens(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1); messages[1].header.nonce++; - messages[1].header.messageId = Internal._hash(messages[1]); + messages[1].header.messageId = Internal._hash(messages[1], ON_RAMP_ADDRESS_1); vm.expectEmit(); emit EVM2EVMMultiOffRamp.ExecutionStateChanged( @@ -520,7 +520,7 @@ contract EVM2EVMMultiOffRamp_executeSingleReport is EVM2EVMMultiOffRampSetup { Internal.Any2EVMRampMessage[] memory messages = _generateSingleBasicMessage(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1); messages[0].header.nonce = 0; - messages[0].header.messageId = Internal._hash(messages[0]); + messages[0].header.messageId = Internal._hash(messages[0], ON_RAMP_ADDRESS_1); vm.expectEmit(); emit EVM2EVMMultiOffRamp.ExecutionStateChanged( @@ -546,7 +546,7 @@ contract EVM2EVMMultiOffRamp_executeSingleReport is EVM2EVMMultiOffRampSetup { _generateSingleBasicMessage(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1); MaybeRevertMessageReceiverNo165 newReceiver = new MaybeRevertMessageReceiverNo165(true); messages[0].receiver = address(newReceiver); - messages[0].header.messageId = Internal._hash(messages[0]); + messages[0].header.messageId = Internal._hash(messages[0], ON_RAMP_ADDRESS_1); vm.expectEmit(); emit EVM2EVMMultiOffRamp.ExecutionStateChanged( @@ -586,7 +586,7 @@ contract EVM2EVMMultiOffRamp_executeSingleReport is EVM2EVMMultiOffRampSetup { _generateMessagesWithTokens(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1); // Set message 1 to use another receiver to simulate more fair gas costs messages[1].receiver = address(s_secondary_receiver); - messages[1].header.messageId = Internal._hash(messages[1]); + messages[1].header.messageId = Internal._hash(messages[1], ON_RAMP_ADDRESS_1); vm.expectEmit(); emit EVM2EVMMultiOffRamp.ExecutionStateChanged( @@ -617,7 +617,7 @@ contract EVM2EVMMultiOffRamp_executeSingleReport is EVM2EVMMultiOffRampSetup { _generateMessagesWithTokens(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1); // Set message 1 to use another receiver to simulate more fair gas costs messages[1].receiver = address(s_secondary_receiver); - messages[1].header.messageId = Internal._hash(messages[1]); + messages[1].header.messageId = Internal._hash(messages[1], ON_RAMP_ADDRESS_1); vm.expectEmit(); emit EVM2EVMMultiOffRamp.ExecutionStateChanged( @@ -659,7 +659,7 @@ contract EVM2EVMMultiOffRamp_executeSingleReport is EVM2EVMMultiOffRampSetup { if (orderings[i]) { messages[i].header.nonce = ++expectedNonce; } - messages[i].header.messageId = Internal._hash(messages[i]); + messages[i].header.messageId = Internal._hash(messages[i], ON_RAMP_ADDRESS_1); vm.expectEmit(); emit EVM2EVMMultiOffRamp.ExecutionStateChanged( @@ -699,8 +699,8 @@ contract EVM2EVMMultiOffRamp_executeSingleReport is EVM2EVMMultiOffRampSetup { }) ); - messages[0].header.messageId = Internal._hash(messages[0]); - messages[1].header.messageId = Internal._hash(messages[1]); + messages[0].header.messageId = Internal._hash(messages[0], ON_RAMP_ADDRESS_1); + messages[1].header.messageId = Internal._hash(messages[1], ON_RAMP_ADDRESS_1); vm.expectEmit(); emit EVM2EVMMultiOffRamp.ExecutionStateChanged( @@ -897,7 +897,7 @@ contract EVM2EVMMultiOffRamp_executeSingleReport is EVM2EVMMultiOffRampSetup { // gas limit too high, Router's external call should revert messages[0].gasLimit = 1e36; messages[0].receiver = address(new ConformingReceiver(address(s_destRouter), s_destFeeToken)); - messages[0].header.messageId = Internal._hash(messages[0]); + messages[0].header.messageId = Internal._hash(messages[0], ON_RAMP_ADDRESS_1); Internal.ExecutionReportSingleChain memory executionReport = _generateReportFromMessages(SOURCE_CHAIN_SELECTOR_1, messages); @@ -922,7 +922,7 @@ contract EVM2EVMMultiOffRamp_executeSingleReport is EVM2EVMMultiOffRampSetup { s_reverting_receiver.setErr(realError1); messages[0].receiver = address(s_reverting_receiver); - messages[0].header.messageId = Internal._hash(messages[0]); + messages[0].header.messageId = Internal._hash(messages[0], ON_RAMP_ADDRESS_1); vm.expectEmit(); emit EVM2EVMMultiOffRamp.ExecutionStateChanged( @@ -1353,7 +1353,7 @@ contract EVM2EVMMultiOffRamp_executeSingleMessage is EVM2EVMMultiOffRampSetup { // Setup the receiver to a non-CCIP Receiver, which will skip the Router call (but should still perform the validation) MaybeRevertMessageReceiverNo165 newReceiver = new MaybeRevertMessageReceiverNo165(true); message.receiver = address(newReceiver); - message.header.messageId = Internal._hash(message); + message.header.messageId = Internal._hash(message, ON_RAMP_ADDRESS_1); s_inboundMessageValidator.setMessageIdValidationState(message.header.messageId, true); vm.expectRevert( @@ -1565,7 +1565,7 @@ contract EVM2EVMMultiOffRamp_manuallyExecute is EVM2EVMMultiOffRampSetup { Internal.Any2EVMRampMessage[] memory messages = _generateSingleBasicMessage(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1); messages[0].receiver = address(s_reverting_receiver); - messages[0].header.messageId = Internal._hash(messages[0]); + messages[0].header.messageId = Internal._hash(messages[0], ON_RAMP_ADDRESS_1); s_offRamp.batchExecute(_generateBatchReportFromMessages(SOURCE_CHAIN_SELECTOR_1, messages), new uint256[][](1)); s_reverting_receiver.setRevert(false); @@ -1588,7 +1588,7 @@ contract EVM2EVMMultiOffRamp_manuallyExecute is EVM2EVMMultiOffRampSetup { Internal.Any2EVMRampMessage[] memory messages = _generateSingleBasicMessage(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1); messages[0].receiver = address(s_reverting_receiver); - messages[0].header.messageId = Internal._hash(messages[0]); + messages[0].header.messageId = Internal._hash(messages[0], ON_RAMP_ADDRESS_1); s_offRamp.batchExecute(_generateBatchReportFromMessages(SOURCE_CHAIN_SELECTOR_1, messages), new uint256[][](1)); s_reverting_receiver.setRevert(false); @@ -1613,7 +1613,7 @@ contract EVM2EVMMultiOffRamp_manuallyExecute is EVM2EVMMultiOffRampSetup { Internal.Any2EVMRampMessage[] memory messages = _generateSingleBasicMessage(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1); messages[0].receiver = address(s_reverting_receiver); - messages[0].header.messageId = Internal._hash(messages[0]); + messages[0].header.messageId = Internal._hash(messages[0], ON_RAMP_ADDRESS_1); assertEq(messages[0].header.nonce - 1, s_offRamp.getSenderNonce(SOURCE_CHAIN_SELECTOR_1, messages[0].sender)); @@ -1646,13 +1646,13 @@ contract EVM2EVMMultiOffRamp_manuallyExecute is EVM2EVMMultiOffRampSetup { for (uint64 i = 0; i < 3; ++i) { messages1[i] = _generateAny2EVMMessageNoTokens(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1, i + 1); messages1[i].receiver = address(s_reverting_receiver); - messages1[i].header.messageId = Internal._hash(messages1[i]); + messages1[i].header.messageId = Internal._hash(messages1[i], ON_RAMP_ADDRESS_1); } for (uint64 i = 0; i < 2; ++i) { messages2[i] = _generateAny2EVMMessageNoTokens(SOURCE_CHAIN_SELECTOR_3, ON_RAMP_ADDRESS_3, i + 1); messages2[i].receiver = address(s_reverting_receiver); - messages2[i].header.messageId = Internal._hash(messages2[i]); + messages2[i].header.messageId = Internal._hash(messages2[i], ON_RAMP_ADDRESS_3); } Internal.ExecutionReportSingleChain[] memory reports = new Internal.ExecutionReportSingleChain[](2); @@ -1703,7 +1703,7 @@ contract EVM2EVMMultiOffRamp_manuallyExecute is EVM2EVMMultiOffRampSetup { messages[i] = _generateAny2EVMMessageNoTokens(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1, i + 1); } messages[1].receiver = address(s_reverting_receiver); - messages[1].header.messageId = Internal._hash(messages[1]); + messages[1].header.messageId = Internal._hash(messages[1], ON_RAMP_ADDRESS_1); vm.expectEmit(); emit EVM2EVMMultiOffRamp.ExecutionStateChanged( @@ -1764,7 +1764,7 @@ contract EVM2EVMMultiOffRamp_manuallyExecute is EVM2EVMMultiOffRampSetup { _generateSingleBasicMessage(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1); messages[0].gasLimit = 1; messages[0].receiver = address(new ConformingReceiver(address(s_destRouter), s_destFeeToken)); - messages[0].header.messageId = Internal._hash(messages[0]); + messages[0].header.messageId = Internal._hash(messages[0], ON_RAMP_ADDRESS_1); vm.expectEmit(); emit EVM2EVMMultiOffRamp.ExecutionStateChanged( @@ -1908,7 +1908,7 @@ contract EVM2EVMMultiOffRamp_manuallyExecute is EVM2EVMMultiOffRampSetup { _generateSingleBasicMessage(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1); messages[0].receiver = address(s_reverting_receiver); - messages[0].header.messageId = Internal._hash(messages[0]); + messages[0].header.messageId = Internal._hash(messages[0], ON_RAMP_ADDRESS_1); s_offRamp.batchExecute(_generateBatchReportFromMessages(SOURCE_CHAIN_SELECTOR_1, messages), new uint256[][](1)); @@ -1957,7 +1957,7 @@ contract EVM2EVMMultiOffRamp_manuallyExecute is EVM2EVMMultiOffRampSetup { messages[0].receiver = address(receiver); - messages[0].header.messageId = Internal._hash(messages[0]); + messages[0].header.messageId = Internal._hash(messages[0], ON_RAMP_ADDRESS_1); Internal.ExecutionReportSingleChain memory report = _generateReportFromMessages(SOURCE_CHAIN_SELECTOR_1, messages); @@ -2516,7 +2516,7 @@ contract EVM2EVMMultiOffRamp_trialExecute is EVM2EVMMultiOffRampSetup { }) ); - message.header.messageId = Internal._hash(message); + message.header.messageId = Internal._hash(message, ON_RAMP_ADDRESS_1); // Unhappy path, no revert but marked as failed. (newState, err) = s_offRamp.trialExecute(message, new bytes[](message.tokenAmounts.length)); @@ -2534,7 +2534,7 @@ contract EVM2EVMMultiOffRamp_trialExecute is EVM2EVMMultiOffRampSetup { }) ); - message.header.messageId = Internal._hash(message); + message.header.messageId = Internal._hash(message, ON_RAMP_ADDRESS_1); (newState, err) = s_offRamp.trialExecute(message, new bytes[](message.tokenAmounts.length)); diff --git a/contracts/src/v0.8/ccip/test/offRamp/EVM2EVMMultiOffRampSetup.t.sol b/contracts/src/v0.8/ccip/test/offRamp/EVM2EVMMultiOffRampSetup.t.sol index 68be3a7f2b..42293cb8e0 100644 --- a/contracts/src/v0.8/ccip/test/offRamp/EVM2EVMMultiOffRampSetup.t.sol +++ b/contracts/src/v0.8/ccip/test/offRamp/EVM2EVMMultiOffRampSetup.t.sol @@ -284,8 +284,7 @@ contract EVM2EVMMultiOffRampSetup is TokenSetup, PriceRegistrySetup, MultiOCR3Ba function _generateAny2EVMMessage( uint64 sourceChainSelector, - // TODO: currently unused - but it will be required after the message struct is updated - bytes memory _onRamp, + bytes memory onRamp, uint64 sequenceNumber, Client.EVMTokenAmount[] memory tokenAmounts, bool allowOutOfOrderExecution @@ -318,7 +317,7 @@ contract EVM2EVMMultiOffRampSetup is TokenSetup, PriceRegistrySetup, MultiOCR3Ba ); } - message.header.messageId = Internal._hash(message); + message.header.messageId = Internal._hash(message, onRamp); return message; }