OPSM: Deploy Superchain #11468
OPSM: Deploy Superchain #11468
Conversation
![semgrep-app[bot]](https://avatars.githubusercontent.com/in/60555?s=60&v=4){kind=small}
| _input; | ||
| _output; | ||
| outfile_; | ||
| require(false, "writeOutputFile not implemented"); |
There was a problem hiding this comment.
| function parseInputFile(string memory _infile) public pure returns (Input memory input_) { | ||
| _infile; | ||
| input_; | ||
| require(false, "parseInputFile is not implemented"); |
There was a problem hiding this comment.
| Input memory _input = parseInputFile(_infile); | ||
| output_ = runWithoutIO(_input); | ||
| outfile_ = writeOutputFile(_infile, _input, output_); | ||
| require(false, "run is not implemented"); |
There was a problem hiding this comment.
| } | ||
|
|
||
| /// @notice This entrypoint is useful for e2e testing purposes, and doesn't use any file I/O. | ||
| function runWithoutIO(Input memory _input) public returns (Output memory output_) { |
There was a problem hiding this comment.
Instead of passing input/output as memory, it would be nice to have it structured as:
- a
SuperchainConfigcontract that has the attributes ofInput(as public getters), and a method to load config from a file. - a
SuperchainDeploymentcontract with setters to write the results to, and a method that writes the final output as file to disk.
And then etch in each contract to some address that is a hash of the contract name, maybe combined with the sender address (so configs/deployments of different tests don't collide).
This would then work well with the Go forge execution, where I can insert precompiles to form the Go config + capture the outputs, without IO, by overriding these input/output contracts with Go.
Any unit-tests could read the addresses from the SuperchainDeployment contract, not that different from reading from a contract struct variable.
And, we can then Semver the input/output contracts; versioned deploy configuration would be very nice to have.
And the setters/getters of a contract would then all be "type safe" from an ABI standpoint, whereas a memory/calldata struct can have a layout with two swapped address fields and it would be hard to tell apart. The ABI signature here is: runWithoutIO(((address,address,address),bool,uint256,uint256)).
But instead it can be one ABI per config variable, and adding/removing configurables and deployment outputs would be much easier to keep compatible (not multiple struct definitions, but rather just additional getters/setters).
And, even better would be to standardize around something like Edit: Putting a check() on the config input contract and the deployment output contract, where we can do some invariant checks (ensure inputs are correct, and ensure outputs are complete/consistent).check() in the DeploySuperchain contract that uses the input/output would both be easier (no need to source the inputs to check outputs), and allow the check() to run even when the config/output are overridden.
There was a problem hiding this comment.
Took a shot at this alternate approach in #11480 (though it doesn't have check() yet), keeping both PRs in draft pending a decision
|
Closing in favor of #11480 |
Description
Adds
DeploySuperchain.s.solwhich deploys and configures the superchain contracts as described in Step 1 of ethereum-optimism/design-docs#60This PR is focused on the in-memory version of the deployment (i.e. no file-based IO required) which is useful for e2e testing in Go. The interfaces for where files are wrapped around this core are scaffolded, but not implemented, in this PR, and will be implemented in a future PR.
This script and it's tests exist independently of the existing
DeployConfig.s.sol,Deploy.s.sol, andArtifacts.s.solcontracts. This is intentional, so we can start with a clean slate here and avoid tech debt. We can integrate these deploy scripts into those ones as needed later, but I don't see a reason to pull them in yet.Tests
A simple fuzz test is added that ensures the provided input configuration is respected, along with another concrete test to validate that zero addresses are not allowed for the role values.
Additional context
Please review the resulting Superchain architecture and ensure it's consistent with the current production architecture