Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MultiOffRamp - configurable metadataHash #1004

Closed
wants to merge 6 commits into from
Closed

MultiOffRamp - configurable metadataHash #1004

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9900b28
feat: make offramp metadataHash configurable with uniqueness check
elatoskinas Jun 11, 2024
4044b3b
Merge branch 'ccip-develop' into feat/any2evm-configurable-metadata-hash
elatoskinas Jun 26, 2024
14e7676
fix: metadataHash-based tests
elatoskinas Jun 26, 2024
358ceb7
feat: use onRamp instead of metadataHash in offramp
elatoskinas Jun 26, 2024
8e30d7b
Merge 358ceb7db0d0762a538f209a3d9a5dda86739b25 into 05a1d3ffee626e211…
elatoskinas Jun 26, 2024
ebbea12
Update gethwrappers
app-token-issuer-infra-releng[bot] Jun 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions contracts/.changeset/popular-poets-reflect.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@chainlink/contracts-ccip': minor
---

#changed Configurable metadataHash for MultiOffRamp
elatoskinas marked this conversation as resolved.
Show resolved Hide resolved
272 changes: 135 additions & 137 deletions contracts/gas-snapshots/ccip.gas-snapshot
View file
Open in desktop

Large diffs are not rendered by default.

48 changes: 25 additions & 23 deletions contracts/src/v0.8/ccip/offRamp/EVM2EVMMultiOffRamp.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import {ERC165Checker} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts

/// @notice EVM2EVMOffRamp enables OCR networks to execute multiple messages
/// in an OffRamp in a single transaction.
/// @dev The EVM2EVMOnRamp, CommitStore and EVM2EVMOffRamp form an xchain upgradeable unit. Any change to one of them
/// results an onchain upgrade of all 3.
/// @dev The EVM2EVMMultiOnRamp and EVM2EVMMultiOffRamp form an xchain upgradeable unit. Any change to one of them
/// results an onchain upgrade of both contracts.
/// @dev MultiOCR3Base is used to store multiple OCR configs for both the OffRamp and the CommitStore.
/// The execution plugin type has to be configured without signature verification, and the commit
/// plugin type with verification.
Expand Down Expand Up @@ -60,6 +60,8 @@ contract EVM2EVMMultiOffRamp is IAny2EVMMultiOffRamp, ITypeAndVersion, MultiOCR3
error StaleCommitReport();
error InvalidInterval(uint64 sourceChainSelector, Interval interval);
error ZeroAddressNotAllowed();
error DuplicateMetadataHash(bytes32 metadataHash);
error ZeroMetadataHashNotAllowed();

/// @dev Atlas depends on this event, if changing, please notify Atlas.
event StaticConfigSet(StaticConfig staticConfig);
Expand Down Expand Up @@ -94,20 +96,18 @@ contract EVM2EVMMultiOffRamp is IAny2EVMMultiOffRamp, ITypeAndVersion, MultiOCR3
struct SourceChainConfig {
bool isEnabled; // ─────────╮ Flag whether the source chain is enabled or not
uint64 minSeqNr; // | The min sequence number expected for future messages
address prevOffRamp; // ────╯ Address of previous-version per-lane OffRamp. Used to be able to provide sequencing continuity during a zero downtime upgrade.
address onRamp; // OnRamp address on the source chain
elatoskinas marked this conversation as resolved.
Show resolved Hide resolved
address prevOffRamp; // ────╯ Address of previous-version per-lane OffRamp. Used to be able to provide seequencing continuity during a zero downtime upgrade.
/// @dev Ensures that 2 identical messages sent to 2 different lanes will have a distinct hash.
/// Must match the metadataHash used in computing leaf hashes offchain for the root committed in
/// the commitStore and i_metadataHash in the onRamp.
bytes32 metadataHash; // Source-chain specific message hash preimage to ensure global uniqueness
/// Must match the meatdataHash on the onRamp.
bytes32 metadataHash; // Source-chain specific message hash preimage to ensure global uniqueness
}

/// @notice SourceChainConfig update args scoped to one source chain
struct SourceChainConfigArgs {
uint64 sourceChainSelector; // ───╮ Source chain selector of the config to update
bool isEnabled; // │ Flag whether the source chain is enabled or not
address prevOffRamp; // ───────────╯ Address of previous-version per-lane OffRamp. Used to be able to provide sequencing continuity during a zero downtime upgrade.
address onRamp; // OnRamp address on the source chain
bytes32 metadataHash; // Source-chain specific message hash preimage to ensure global uniqueness
}

/// @notice Dynamic offRamp config
Expand Down Expand Up @@ -163,6 +163,8 @@ contract EVM2EVMMultiOffRamp is IAny2EVMMultiOffRamp, ITypeAndVersion, MultiOCR3
/// @notice SourceConfig per chain
/// (forms lane configurations from sourceChainSelector => StaticConfig.chainSelector)
mapping(uint64 sourceChainSelector => SourceChainConfig sourceChainConfig) internal s_sourceChainConfigs;
/// @notice Set of used metadata hashes - used to prevent configuring the same metadataHash more than once
mapping(bytes32 metadataHash => bool isActive) internal s_usedMetadataHashSet;

// STATE
/// @dev The expected nonce for a given sender per source chain.
Expand Down Expand Up @@ -591,11 +593,6 @@ contract EVM2EVMMultiOffRamp is IAny2EVMMultiOffRamp, ITypeAndVersion, MultiOCR3
if (!success) revert ReceiverError(returnData);
}

/// @notice creates a unique hash to be used in message hashing.
function _metadataHash(uint64 sourceChainSelector, address onRamp, bytes32 prefix) internal view returns (bytes32) {
return keccak256(abi.encode(prefix, sourceChainSelector, i_chainSelector, onRamp));
}

// ================================================================
// │ Commit │
// ================================================================
Expand Down Expand Up @@ -790,23 +787,28 @@ contract EVM2EVMMultiOffRamp is IAny2EVMMultiOffRamp, ITypeAndVersion, MultiOCR3
revert ZeroChainSelectorNotAllowed();
}

if (sourceConfigUpdate.onRamp == address(0)) {
revert ZeroAddressNotAllowed();
}

SourceChainConfig storage currentConfig = s_sourceChainConfigs[sourceChainSelector];

// OnRamp can never be zero - if it is, then the source chain has been added for the first time
if (currentConfig.onRamp == address(0)) {
currentConfig.metadataHash =
_metadataHash(sourceChainSelector, sourceConfigUpdate.onRamp, Internal.EVM_2_EVM_MESSAGE_HASH);
currentConfig.onRamp = sourceConfigUpdate.onRamp;
if (currentConfig.metadataHash == bytes32("")) {
bytes32 newMetadataHash = sourceConfigUpdate.metadataHash;

if (newMetadataHash == bytes32("")) {
revert ZeroMetadataHashNotAllowed();
}

if (s_usedMetadataHashSet[newMetadataHash]) {
revert DuplicateMetadataHash(newMetadataHash);
}
s_usedMetadataHashSet[newMetadataHash] = true;

currentConfig.metadataHash = newMetadataHash;
currentConfig.prevOffRamp = sourceConfigUpdate.prevOffRamp;
currentConfig.minSeqNr = 1;

emit SourceChainSelectorAdded(sourceChainSelector);
} else if (
currentConfig.onRamp != sourceConfigUpdate.onRamp || currentConfig.prevOffRamp != sourceConfigUpdate.prevOffRamp
currentConfig.metadataHash != sourceConfigUpdate.metadataHash
|| currentConfig.prevOffRamp != sourceConfigUpdate.prevOffRamp
) {
revert InvalidStaticConfig(sourceChainSelector);
}
Expand Down
9 changes: 7 additions & 2 deletions contracts/src/v0.8/ccip/test/e2e/MultiRampsEnd2End.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,18 @@ contract MultiRampsE2E is EVM2EVMMultiOnRampSetup, EVM2EVMMultiOffRampSetup {
sourceChainSelector: SOURCE_CHAIN_SELECTOR,
isEnabled: true,
prevOffRamp: address(0),
onRamp: address(s_onRamp)
// Must match OnRamp's metadataHash
metadataHash: keccak256(
abi.encode(Internal.EVM_2_EVM_MESSAGE_HASH, SOURCE_CHAIN_SELECTOR, DEST_CHAIN_SELECTOR, address(s_onRamp))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the metadataHash logic from the OnRamp. Would it be simpler if we also make this configurable on the MultiOnRamp? Then we could re-use the same hash compute logic off-chain - it would be less error-prone that way.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in practice we can just read directly the metadata hash onramp when we configure on the offramp

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's less error prone, true. We'll have to change this prefix to EVM_2_ANY_MESSAGE_HASH though

)
});
sourceChainConfigs[1] = EVM2EVMMultiOffRamp.SourceChainConfigArgs({
sourceChainSelector: SOURCE_CHAIN_SELECTOR + 1,
isEnabled: true,
prevOffRamp: address(0),
onRamp: address(s_onRamp2)
metadataHash: keccak256(
abi.encode(Internal.EVM_2_EVM_MESSAGE_HASH, SOURCE_CHAIN_SELECTOR + 1, DEST_CHAIN_SELECTOR, address(s_onRamp2))
)
});

_setupMultipleOffRampsFromConfigs(sourceChainConfigs);
Expand Down
4 changes: 0 additions & 4 deletions contracts/src/v0.8/ccip/test/helpers/EVM2EVMMultiOffRampHelper.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ contract EVM2EVMMultiOffRampHelper is EVM2EVMMultiOffRamp, IgnoreContractSize {
return s_executionStates[sourceChainSelector][bitmapIndex];
}

function metadataHash(uint64 sourceChainSelector, address onRamp) external view returns (bytes32) {
return _metadataHash(sourceChainSelector, onRamp, Internal.EVM_2_EVM_MESSAGE_HASH);
}

function releaseOrMintSingleToken(
uint256 sourceTokenAmount,
bytes calldata originalSender,
Expand Down
Loading
Loading