feat(op-deployer): add prepare command to predict L1 addresses#21560
feat(op-deployer): add prepare command to predict L1 addresses#215600xiamflux wants to merge 19 commits into
Conversation
…PChain to check the Deployed flag
prepare command and predict L1 addresses| // predictChains predicts the L1 addresses for each undeployed chain in the intent | ||
| // and records them as not deployed. Chains that have been deployed are skipped so a |
There was a problem hiding this comment.
can we have more than 1 chain defined in the intent?
There was a problem hiding this comment.
Yes, that's possible.
| }, | ||
| { | ||
| Name: "prepare", | ||
| Usage: "prepares a chain deployment by generating the genesis artifacts", |
There was a problem hiding this comment.
should we also mention that prepare should be called after init and that it should not be called with apply?
There was a problem hiding this comment.
Yes, passing a prepared state/intent through the apply pipeline can create unsupported scenarios so we check for those to error out clearly on apply. So I think it's worth to callout this in the Usage.
| } | ||
| } | ||
|
|
||
| func Prepare(ctx context.Context, cfg PrepareConfig) error { |
There was a problem hiding this comment.
what happens if someone runs init -> prepare -> apply ? would our state.json be overwritten? maybe we should have some way of preventing apply if we have already called prepare or do you think this is not an issue?
There was a problem hiding this comment.
same question but what happens if init -> prepare -> init?
There was a problem hiding this comment.
Addressed here by checking for Prepared flag in apply pipeline, we error out if it's there, it's backward compatible as any state missing the flag is assumed to be produced by an older pipeline and defaults to false.
| // pinned during the prepare dry-run, keeping the predicted L1 addresses valid | ||
| // across the relevant stages of the permissionless pipeline. A nil L1PredictSenderAddress | ||
| // means no sender was pinned, so any deployer is accepted (older pipeline). | ||
| func (s *State) CheckL1PredictSender(deployer common.Address) error { |
There was a problem hiding this comment.
i think we should have something similar like we check the sender but for the opcm aswell, basically making sure that we use the same opcm for both prepare to calculate the addresses but also we use the same opcm for the actual deployment (continue)
There was a problem hiding this comment.
Addressed here. Similar shape as the deployer's address check.
| intent, err := pipeline.ReadIntent(cfg.Workdir) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to read intent: %w", err) | ||
| } | ||
|
|
||
| st, err := pipeline.ReadState(cfg.Workdir) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to read state: %w", err) | ||
| } | ||
|
|
||
| if len(intent.Chains) == 0 { | ||
| return fmt.Errorf("intent has no chains to prepare") | ||
| } |
There was a problem hiding this comment.
noticed the apply pipeline enforces a few more checks like if the intent is valid, or the state version and chain id. If we end up using similar checks later in continue then maybe we should enforce them from prepare to make sure we can later actually call continue without issues, but wdyt?
Co-authored-by: 0xOneTony <112496816+0xOneTony@users.noreply.github.com>
|
|
||
| // L1PredictSenderAddress is the address that performed the L1 deploy dry-run. | ||
| // It is used to verify that the same deployer address is used for the relevant stages of the permissionless pipeline. | ||
| L1PredictSenderAddress *common.Address `json:"deployerAddress,omitempty"` |
There was a problem hiding this comment.
This json key is inconsistent. Should we change it to predictSenderAddress to align with predictOpcmAddress below?
| // The OPCM, superchain config and salt mixer are taken from the committed intent | ||
| // and state so the prediction matches the eventual broadcast. Role addresses are set to | ||
| // placeholders since they are not relevant for the prediction. | ||
| func makePredictionInput(intent *state.Intent, st *state.State, chain *state.ChainIntent) (opcm.DeployOPChainInput, error) { |
There was a problem hiding this comment.
This function duplicates much of makeDCI, and the two have already drifted: gas-limit defaulting here vs. raw thisIntent.GasLimit there, and Opcm/SuperchainConfig sourced from the intent here vs. from state there.
Could you modify the approach by introducing a helper buildDeployOPChainInput(proofParams, roles, opcm, superchainCfg, l2ChainID, saltMixer, gasLimit, chain) that makeDCI, makePredictionInput, and the future continue code all call, with each caller supplying its own roles/OPCM/superchain provenance? I think that would help ensure the prediction and actual deployment stay in-sync
| GasLimit: gasLimit, | ||
|
|
||
| DisputeGameType: proofParams.DisputeGameType, | ||
| DisputeAbsolutePrestate: proofParams.DisputeAbsolutePrestate, |
There was a problem hiding this comment.
Once #21588 lands, DeployOPChainInput will have a required StartingAnchorRoot field, and DeployOPChain.checkInput reverts with "startingAnchorRoot not set" when it's zero. So this code will need to be updated to include that new field.
Summary
Adds the
preparecommand to op-deployer.preparepredicts a chain's L1 contract addresses without broadcasting anything.What it does
For each chain in the intent,
prepare:intent.tomlandstate.jsonfrom the workdir.--l1-rpc-url) and dry-runsOPCM.deploy()through theDeployOPChainscript, using aNoopBroadcasterso nothing is written to L1.state.json.The dry-run runs as the address derived from
--private-key. Predicted addresses are deterministic in the sender and salt mixer, so the prediction matches the real deployment.State changes
ChainStategains aDeployedbool to distinguish predicted addresses (prepare) from broadcast addresses (apply/continue).shouldDeployOPChainand contract verification now skip predicted-only chains, so runningapplyover a prepared state deploys the chain rather than treating the prediction as finished.Test Plan
preparefails when the intent is missing the OPCM address or superchain config proxy.Deployed=false.Closes #20908