-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
op-chain-ops/interopgen: experimental interop genesis-state generation #11589
Conversation
@@ -25,9 +26,107 @@ struct Deployment { | |||
/// @notice Useful for accessing deployment artifacts from within scripts. | |||
/// When a contract is deployed, call the `save` function to write its name and | |||
/// contract address to disk. Inspired by `forge-deploy`. | |||
/// @dev This uses the DeploymentRegistry contract, to load/store deployments, |
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.
Prefer @notice
over @dev
in natspec comments
Semgrep found 3 Named return arguments to functions must be appended with an underscore ( |
d347de9
to
12b253c
Compare
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.
Bunch of questions for my own benefit, and a couple of code cleanliness/ comment opportunities.
if c.ChainID == nil { | ||
return errors.New("missing L1 chain ID") | ||
} | ||
// nothing to check on c.DevL1DeployConfig |
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 assume nothing to check on Prefund
as well?
return nil | ||
} | ||
|
||
type SuperchainConfig struct { |
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 would love comments over these struct
s that show the different purpose of:
- L1Config
- SuperchainConfig
- L2Config
My intuition is that there's One L1, One Superchain, and N L2s, but I don't see the SuperchainConfig linked into the L1 config in any way. Rather they're linked to a genesis....Config
type WorldConfig struct { | ||
L1 *L1Config | ||
Superchain *SuperchainConfig | ||
L2s map[string]*L2Config | ||
} |
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.
Ah I think this answers one of my earlier questions about how these configs interplay.
// Insert an empty beaconchain deposit contract with valid empty-tree prestate. | ||
// This is part of dev-genesis, but not part of scripts yet. | ||
beaconDepositAddr := common.HexToAddress("0x1111111111111111111111111111111111111111") |
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.
what is the relevance of 0x1111...
here? Any value in making this configurable?
}, nil | ||
} | ||
|
||
func completeL2(l2Host *script.Host, cfg *L2Config, l1Block *types.Block, deployment *L2Deployment) (*L2Output, error) { |
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.
Could use function comments to highlight the difference between deploying an L2 and completing it.
// Padded chain ID, hex encoded, prefixed with 0xff like inboxes, then 0x02 to signify devnet. | ||
batchInboxAddress := common.HexToAddress(fmt.Sprintf("0xff02%016x", l2ChainID)) |
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.
Would love more explanation on the relationship between 0x02
and Devnet
}, | ||
FaultProofDeployConfig: genesis.FaultProofDeployConfig{ | ||
UseFaultProofs: true, | ||
FaultGameAbsolutePrestate: common.HexToHash("0x03c7ae758795765c6664a5d39bf63841c71ff191e9189522bad8ebff5d4eca98"), |
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.
What's this hex about? The other ones look obvious but I don't see any special value here.
require.NoError(t, err) | ||
enc := json.NewEncoder(os.Stdout) | ||
enc.SetIndent(" ", " ") | ||
require.NoError(t, enc.Encode(worldDeployment)) | ||
logger.Info("L1 output", "accounts", len(worldOutput.L1.Genesis.Alloc)) | ||
for id, l2Output := range worldOutput.L2s { | ||
logger.Info("L2 output", "chain", &id, "accounts", len(l2Output.Genesis.Alloc)) | ||
} |
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.
Mostly we're just looking for no errors when Deploying? is there any testing you'd want to do on these emitted logs?
12b253c
to
9e3beb7
Compare
Closing in favor of |
Description
Experimental draft of interop multi-L2 devnet genesis state generation code.
This can largely be cleaned up after the L2 chain deployment part of OPSM lands:
Artifacts
can be dropped, as we don't use the legacy deployment anymore at that pointBut for now, just need to get a working genesis deployment, so these temporary workarounds will have to do.
Tests
recipe_test.go
turns a basic interop recipe into an expandedWorldConfig
, which can then be deployed into aWorldDeployment
(address data) andWorldOutput
(genesis configs and state).Metadata
Fix #11493