Skip to content

Commit

Permalink
Standardize message hashing functions (#1424)
Browse files Browse the repository at this point in the history
## Motivation
The goal of this PR is to standardize the message hashing functions.
Currently we have equivalent but inconsistent message hashing functions:
```solidity
function _hash(Any2EVMRampMessage memory original, bytes memory onRamp) internal pure returns (bytes32)
```
and 
```solidity
function _hash(EVM2AnyRampMessage memory original, bytes32 metadataHash) internal pure returns (bytes32)
```

## Solution
Standardize to `function _hash(message, metadataHash)` and consistently
encode dynamic fields.

---------

Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
  • Loading branch information
1 parent a847f78 commit 65f31a8
Show file tree
Hide file tree
Showing 12 changed files with 250 additions and 221 deletions.
218 changes: 109 additions & 109 deletions contracts/gas-snapshots/ccip.gas-snapshot

Large diffs are not rendered by default.

15 changes: 5 additions & 10 deletions contracts/src/v0.8/ccip/libraries/Internal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -187,30 +187,25 @@ library Internal {
/// OnRamp hash(EVM2AnyMessage) != Any2EVMRampMessage.messageId
/// OnRamp hash(EVM2AnyMessage) != OffRamp hash(Any2EVMRampMessage)
/// @param original OffRamp message to hash
/// @param onRamp OnRamp to hash the message with - used to compute the metadataHash
/// @param metadataHash Hash preimage to ensure global uniqueness
/// @return hashedMessage hashed message as a keccak256
function _hash(Any2EVMRampMessage memory original, bytes memory onRamp) internal pure returns (bytes32) {
function _hash(Any2EVMRampMessage memory original, bytes32 metadataHash) 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(
abi.encode(
MerkleMultiProof.LEAF_DOMAIN_SEPARATOR,
// Implicit metadata hash
keccak256(
abi.encode(
ANY_2_EVM_MESSAGE_HASH, original.header.sourceChainSelector, original.header.destChainSelector, onRamp
)
),
metadataHash,
keccak256(
abi.encode(
original.header.messageId,
original.sender,
original.receiver,
original.header.sequenceNumber,
original.gasLimit,
original.header.nonce
)
),
keccak256(original.sender),
keccak256(original.data),
keccak256(abi.encode(original.tokenAmounts))
)
Expand All @@ -227,13 +222,13 @@ library Internal {
keccak256(
abi.encode(
original.sender,
original.receiver,
original.header.sequenceNumber,
original.header.nonce,
original.feeToken,
original.feeTokenAmount
)
),
keccak256(original.receiver),
keccak256(original.data),
keccak256(abi.encode(original.tokenAmounts)),
keccak256(original.extraArgs)
Expand Down
12 changes: 11 additions & 1 deletion contracts/src/v0.8/ccip/offRamp/OffRamp.sol
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,17 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
// over the same data, which increases gas cost.
// Hashing all of the message fields ensures that the message being executed is correct and not tampered with.
// Including the known OnRamp ensures that the message originates from the correct on ramp version
hashedLeaves[i] = Internal._hash(message, onRamp);
hashedLeaves[i] = Internal._hash(
message,
keccak256(
abi.encode(
Internal.ANY_2_EVM_MESSAGE_HASH,
message.header.sourceChainSelector,
message.header.destChainSelector,
keccak256(onRamp)
)
)
);
}

// SECURITY CRITICAL CHECK
Expand Down
22 changes: 11 additions & 11 deletions contracts/src/v0.8/ccip/test/NonceManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ contract NonceManager_OffRampUpgrade is OffRampSetup {
SOURCE_CHAIN_SELECTOR_1,
messages[0].header.sequenceNumber,
messages[0].header.messageId,
Internal._hash(messages[0], ON_RAMP_ADDRESS_1),
_hashMessage(messages[0], ON_RAMP_ADDRESS_1),
Internal.MessageExecutionState.SUCCESS,
""
);
Expand Down Expand Up @@ -429,7 +429,7 @@ contract NonceManager_OffRampUpgrade is OffRampSetup {
SOURCE_CHAIN_SELECTOR_3,
messagesChain3[0].header.sequenceNumber,
messagesChain3[0].header.messageId,
Internal._hash(messagesChain3[0], ON_RAMP_ADDRESS_3),
_hashMessage(messagesChain3[0], ON_RAMP_ADDRESS_3),
Internal.MessageExecutionState.SUCCESS,
""
);
Expand Down Expand Up @@ -499,7 +499,7 @@ contract NonceManager_OffRampUpgrade is OffRampSetup {
_generateSingleBasicMessage(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1);

messagesMultiRamp[0].header.nonce++;
messagesMultiRamp[0].header.messageId = Internal._hash(messagesMultiRamp[0], ON_RAMP_ADDRESS_1);
messagesMultiRamp[0].header.messageId = _hashMessage(messagesMultiRamp[0], ON_RAMP_ADDRESS_1);

vm.recordLogs();

Expand All @@ -511,7 +511,7 @@ contract NonceManager_OffRampUpgrade is OffRampSetup {
SOURCE_CHAIN_SELECTOR_1,
messagesMultiRamp[0].header.sequenceNumber,
messagesMultiRamp[0].header.messageId,
Internal._hash(messagesMultiRamp[0], ON_RAMP_ADDRESS_1),
_hashMessage(messagesMultiRamp[0], ON_RAMP_ADDRESS_1),
Internal.MessageExecutionState.SUCCESS,
""
);
Expand All @@ -522,7 +522,7 @@ contract NonceManager_OffRampUpgrade is OffRampSetup {

messagesMultiRamp[0].header.nonce++;
messagesMultiRamp[0].header.sequenceNumber++;
messagesMultiRamp[0].header.messageId = Internal._hash(messagesMultiRamp[0], ON_RAMP_ADDRESS_1);
messagesMultiRamp[0].header.messageId = _hashMessage(messagesMultiRamp[0], ON_RAMP_ADDRESS_1);

vm.recordLogs();
s_offRamp.executeSingleReport(
Expand All @@ -532,7 +532,7 @@ contract NonceManager_OffRampUpgrade is OffRampSetup {
SOURCE_CHAIN_SELECTOR_1,
messagesMultiRamp[0].header.sequenceNumber,
messagesMultiRamp[0].header.messageId,
Internal._hash(messagesMultiRamp[0], ON_RAMP_ADDRESS_1),
_hashMessage(messagesMultiRamp[0], ON_RAMP_ADDRESS_1),
Internal.MessageExecutionState.SUCCESS,
""
);
Expand All @@ -555,7 +555,7 @@ contract NonceManager_OffRampUpgrade is OffRampSetup {

bytes memory newSender = abi.encode(address(1234567));
messagesMultiRamp[0].sender = newSender;
messagesMultiRamp[0].header.messageId = Internal._hash(messagesMultiRamp[0], ON_RAMP_ADDRESS_1);
messagesMultiRamp[0].header.messageId = _hashMessage(messagesMultiRamp[0], ON_RAMP_ADDRESS_1);

// new sender nonce in new offramp should go from 0 -> 1
assertEq(s_inboundNonceManager.getInboundNonce(SOURCE_CHAIN_SELECTOR_1, newSender), 0);
Expand All @@ -567,7 +567,7 @@ contract NonceManager_OffRampUpgrade is OffRampSetup {
SOURCE_CHAIN_SELECTOR_1,
messagesMultiRamp[0].header.sequenceNumber,
messagesMultiRamp[0].header.messageId,
Internal._hash(messagesMultiRamp[0], ON_RAMP_ADDRESS_1),
_hashMessage(messagesMultiRamp[0], ON_RAMP_ADDRESS_1),
Internal.MessageExecutionState.SUCCESS,
""
);
Expand All @@ -581,7 +581,7 @@ contract NonceManager_OffRampUpgrade is OffRampSetup {
address newSender = address(1234567);
messages[0].sender = abi.encode(newSender);
messages[0].header.nonce = 2;
messages[0].header.messageId = Internal._hash(messages[0], ON_RAMP_ADDRESS_1);
messages[0].header.messageId = _hashMessage(messages[0], ON_RAMP_ADDRESS_1);

uint64 startNonce = s_inboundNonceManager.getInboundNonce(SOURCE_CHAIN_SELECTOR_1, messages[0].sender);

Expand Down Expand Up @@ -611,7 +611,7 @@ contract NonceManager_OffRampUpgrade is OffRampSetup {
);

messages[0].header.nonce = 2;
messages[0].header.messageId = Internal._hash(messages[0], ON_RAMP_ADDRESS_1);
messages[0].header.messageId = _hashMessage(messages[0], ON_RAMP_ADDRESS_1);

// new offramp is able to execute
vm.recordLogs();
Expand All @@ -622,7 +622,7 @@ contract NonceManager_OffRampUpgrade is OffRampSetup {
SOURCE_CHAIN_SELECTOR_1,
messages[0].header.sequenceNumber,
messages[0].header.messageId,
Internal._hash(messages[0], ON_RAMP_ADDRESS_1),
_hashMessage(messages[0], ON_RAMP_ADDRESS_1),
Internal.MessageExecutionState.SUCCESS,
""
);
Expand Down
12 changes: 6 additions & 6 deletions contracts/src/v0.8/ccip/test/e2e/MultiRampsEnd2End.sol
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,10 @@ contract MultiRampsE2E is OnRampSetup, OffRampSetup {
// Scoped to commit to reduce stack pressure
{
bytes32[] memory hashedMessages1 = new bytes32[](2);
hashedMessages1[0] = messages1[0]._hash(abi.encode(address(s_onRamp)));
hashedMessages1[1] = messages1[1]._hash(abi.encode(address(s_onRamp)));
hashedMessages1[0] = _hashMessage(messages1[0], abi.encode(address(s_onRamp)));
hashedMessages1[1] = _hashMessage(messages1[1], abi.encode(address(s_onRamp)));
bytes32[] memory hashedMessages2 = new bytes32[](1);
hashedMessages2[0] = messages2[0]._hash(abi.encode(address(s_onRamp2)));
hashedMessages2[0] = _hashMessage(messages2[0], abi.encode(address(s_onRamp2)));

merkleRoots[0] = MerkleHelper.getMerkleRoot(hashedMessages1);
merkleRoots[1] = MerkleHelper.getMerkleRoot(hashedMessages2);
Expand Down Expand Up @@ -211,7 +211,7 @@ contract MultiRampsE2E is OnRampSetup, OffRampSetup {
SOURCE_CHAIN_SELECTOR,
messages1[0].header.sequenceNumber,
messages1[0].header.messageId,
messages1[0]._hash(abi.encode(address(s_onRamp))),
_hashMessage(messages1[0], abi.encode(address(s_onRamp))),
Internal.MessageExecutionState.SUCCESS,
""
);
Expand All @@ -220,7 +220,7 @@ contract MultiRampsE2E is OnRampSetup, OffRampSetup {
SOURCE_CHAIN_SELECTOR,
messages1[1].header.sequenceNumber,
messages1[1].header.messageId,
messages1[1]._hash(abi.encode(address(s_onRamp))),
_hashMessage(messages1[1], abi.encode(address(s_onRamp))),
Internal.MessageExecutionState.SUCCESS,
""
);
Expand All @@ -229,7 +229,7 @@ contract MultiRampsE2E is OnRampSetup, OffRampSetup {
SOURCE_CHAIN_SELECTOR + 1,
messages2[0].header.sequenceNumber,
messages2[0].header.messageId,
messages2[0]._hash(abi.encode(address(s_onRamp2))),
_hashMessage(messages2[0], abi.encode(address(s_onRamp2))),
Internal.MessageExecutionState.SUCCESS,
""
);
Expand Down
9 changes: 8 additions & 1 deletion contracts/src/v0.8/ccip/test/helpers/MessageHasher.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,14 @@ import {Internal} from "../../libraries/Internal.sol";
/// @dev This is only deployed in tests and is not part of the production contracts.
contract MessageHasher {
function hash(Internal.Any2EVMRampMessage memory message, bytes memory onRamp) public pure returns (bytes32) {
return Internal._hash(message, onRamp);
return Internal._hash(
message,
keccak256(
abi.encode(
Internal.ANY_2_EVM_MESSAGE_HASH, message.header.sourceChainSelector, message.header.destChainSelector, onRamp
)
)
);
}

function encodeTokenAmountsHashPreimage(
Expand Down
Loading

0 comments on commit 65f31a8

Please sign in to comment.