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.

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

Conversation

elatoskinas
Copy link
Collaborator

@elatoskinas elatoskinas commented Jun 12, 2024

Motivation

Due to the different encoding schemes on non-EVM chains, the conversion of the MultiOffRamp requires making the metadataHash not tied to the abi encoding scheme.

Solution

Makes the metadataHash configurable with a uniqueness check

@elatoskinas elatoskinas added the do not merge Do not merge at this time label Jun 12, 2024
Copy link
Contributor

I see you updated files related to contracts. Please run pnpm changeset in the contracts directory to add a changeset.

@elatoskinas elatoskinas marked this pull request as ready for review June 12, 2024 10:48
@elatoskinas elatoskinas requested review from makramkd and a team as code owners June 12, 2024 10:48
Copy link
Contributor

Go solidity wrappers are out-of-date, regenerate them via the make wrappers-all command

Copy link
Contributor

github-actions bot commented Jun 12, 2024

LCOV of commit ebbea12 during Solidity Foundry #5858

Summary coverage rate:
  lines......: 99.0% (1733 of 1751 lines)
  functions..: 96.7% (326 of 337 functions)
  branches...: 91.8% (731 of 796 branches)

Files changed coverage rate: n/a

@elatoskinas elatoskinas force-pushed the feat/merged-ramp-size-optimizations branch from ba45a06 to a9ebe66 Compare June 13, 2024 16:14
@elatoskinas elatoskinas requested review from RayXpub and a team as code owners June 13, 2024 16:14
Base automatically changed from feat/merged-ramp-size-optimizations to ccip-develop June 21, 2024 08:33
@elatoskinas elatoskinas force-pushed the feat/any2evm-configurable-metadata-hash branch from 1d84a86 to 9900b28 Compare June 21, 2024 08:53
@elatoskinas elatoskinas removed the do not merge Do not merge at this time label Jun 21, 2024
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

@elatoskinas elatoskinas marked this pull request as draft June 26, 2024 13:45
@elatoskinas
Copy link
Collaborator Author

Closing - reworking to use OnRamp address without a configurable metadataHash

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants