-
Notifications
You must be signed in to change notification settings - Fork 169
Add Superchain interop message passing #595
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
base: master
Are you sure you want to change the base?
Conversation
Also manually tested as follows. Note these are testcases only and do not represent best practices for actual deployments.
e. Call the crosschain function using script: |
@SocketSecurity ignore-all |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!. Tested in supersim and succeeded. Just added one non-blocking comment, great work Eric!.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious, why are we using Soldeer for Optimism contracts but Foundry git submodules for the OpenZeppelin dependencies on the Foundry zip downloads?
Wouldn't it be easier and cleaner for developers to get a single package managing method instead of two?, or there is a hard requirement that forces us to use both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For Optimism contracts, we have 4 options:
- Not import them, and just hardcode the messenger contract's predeploy address and interface's function calls.
- Use git submodules with forge install
ethereum-optimism/optimism
- Use NPM package
@eth-optimism/contracts-bedrock
- Use Soldeer package https://soldeer.xyz/project/@eth-optimism-contracts-bedrock which is a Soldeer-maintained mirror of the above NPM package.
For Foundry with Optimism contracts, here are the drawbacks of each option:
- This works, but it seems better practice to import the address and interface since they are available.
- This requires downloading the entire repo which is over 500 MB and seems excessive for what we need.
- This requires NPM to be installed, and is not Foundry native.
- Depends on Soldeer and its repository, but this is natively supported in Foundry.
Option 4 seems the most appropriate in this case.
For OpenZeppelin Contracts dependencies, current recommendations in https://github.com/OpenZeppelin/openzeppelin-contracts are to use forge install. We can consider being consistent in Wizard and use Soldeer for this as well, but we should review the recommended patterns when doing so, and this should occur in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work 🔥
Great test coverage
I am wondering in the Custom contract it seem Optimism option add both emitter and receiver functions is it standar or does contract usually implement on or the other?
@@ -176,6 +177,11 @@ const script = (c: Contract, opts?: GenericOptions) => { | |||
} | |||
}; | |||
|
|||
const OPTIMISM_NPM_PACKAGE = '@eth-optimism/contracts-bedrock'; | |||
const OPTIMISM_SOLDEER_PACKAGE = '@eth-optimism-contracts-bedrock'; | |||
const importsOptimism = (c: Contract) => c.imports.some(i => i.path.startsWith(OPTIMISM_NPM_PACKAGE)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super nit: maybe "shouldImportOptimism" as it result in a boolean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"imports" can be interpreted as a boolean in this case, which seems clear enough and is shorter
In this example, we include both functions. If only one function is included, the emitter and/or receiver would need to take or track other addresses, which would be a more complex example. That can be considered as an enhancement if there is a need for it. |
In Solidity's Custom tab, this PR adds a Cross-Chain Messaging option that adds an example of Superchain interop message passing when enabled. This includes a function that can be called from a different network on the Superchain.
For example, when enabled, this adds the following functions by default:
callMyFunction
which takes a chain ID argument, and sends a message tomyFunction
at a contract at the same address on the given chain IDmyFunction
which receives messages from other chains. The name of this function is customizable (which affects the name of the above function as well).Implementation checklist: