Questions/Things to improve

[sebastiendan]

Hey @tnobayashi, I'll gather here my feedback from trying to deploy a new op-fep network!

1. We take care of the allocs before creating later new wallets like the batcher account. Shouldn't we prepare this account first so that we can prefund it?

2. When deploying L1 op contracts, I got errors until I changed the configType = "standard" to configType = "standard-overrides" in intent.toml

3. In that same file, I see you manually(?) add/change a few things like:

fundDevAccounts = true
[globalDeployOverrides]
  l2BlockTime = 1

Should we document it?

4. In step 4 (rollup initialization), we say

export l2_node_url="https://<your-l2-node>"
export l2_rpc_url="http://<your-l2-rpc>"

Maybe we can make it more explicit that "node" means op-node and "rpc" means op-geth? I think it's fine to imply that "rpc"<->"op-geth" is known to the reader, but the term "node" is maybe a bit too vague (if we abstract the op stack here, a "node" could be many things).

5. Same step, we say

L1_RPC="${l1_rpc_url}"
L1_BEACON_RPC="${l1_rpc_url}"

This is only valid if the L1 RPC URL is also a beacon node. We could add a node perhaps for this.

6. Same step, we say

This output.json file can be used to help construct aggchainParams.

but it looks to me that it's opsuccinctl2ooconfig.json that contains the data needed not for aggchainParams but more specifically for aggchainParams.initParams

7. In step 5, we use again fetch-l2oo-config like in step 4, is that expected? (I see these fields have different values in the output the 2nd time we use the tool: startingBlockNumber, startingOutputRoot, startingTimestamp)

8. Same step, when adding the OpSuccintConfig on chain, my cast command to send the tx worked but I got this which led me to think it didn't

Error: server returned a null response when a non-null response was expected

[tnobayashi]

1. We take care of the allocs before creating later new wallets like the batcher account. Shouldn't we prepare this account first so that we can prefund it?

I will need to carefully review this one and open a PR.

2. When deploying L1 op contracts, I got errors until I changed the configType = "standard" to configType = "standard-overrides" in intent.toml

That's actually the expected op-deployer behavior. To deploy a custom network, it's required to set configType = "standard-overrides" in the intent.toml.
https://docs.optimism.io/chain-operators/tools/op-deployer/reference/custom-deployments#deploying

3. [...] Should we document it?

I don't think we should document it as these configs aren't required.

4. [...] Maybe we can make it more explicit that "node" means op-node and "rpc" means op-geth? I think it's fine to imply that "rpc"<->"op-geth" is known to the reader, but the term "node" is maybe a bit too vague (if we abstract the op stack here, a "node" could be many things).
5. [...] This is only valid if the L1 RPC URL is also a beacon node. We could add a node perhaps for this.

That makes sense. I opened #9 to address these points.

6. [...] but it looks to me that it's opsuccinctl2ooconfig.json that contains the data needed not for aggchainParams but more specifically for aggchainParams.initParams

I reviewed the document and I think the output.json isn't used anymore. I completely removed this step.
#10

7. [...] In step 5, we use again fetch-l2oo-config like in step 4, is that expected? (I see these fields have different values in the output the 2nd time we use the tool: startingBlockNumber, startingOutputRoot, startingTimestamp)

Step 4 (Rollup Initialization) and Step 5 (OP Succinct Configuration) are needed in different parts of the network life cycle. If they are executed one after another, indeed it's not needed to run fetch-l2oo-config twice.

For every new op-succinct-proposer version, we need a new OP Succinct Configuration. Which means important values like rollupConfigHash might change in between. So, before creating a new OP Succinct Configuration, it's a good idea to fetch-l2oo-config.

8. Same step, when adding the OpSuccintConfig on chain, my cast command to send the tx worked but I got this which led me to think it didn't

hmmm I'm not familiar with this error 🤔

[sebastiendan]

I will need to carefully review this one and open a PR.

I don't think I had to fund any accounts post-genesis, so maybe it's not needed 🤔 (but I used the same batcher address than you used for spec-13-op)

Step 4 (Rollup Initialization) and Step 5 (OP Succinct Configuration) are needed in different parts of the network life cycle. If they are executed one after another, indeed it's not needed to run fetch-l2oo-config twice.

For every new op-succinct-proposer version, we need a new OP Succinct Configuration. Which means important values like rollupConfigHash might change in between. So, before creating a new OP Succinct Configuration, it's a good idea to fetch-l2oo-config.

Maybe we can document this? (we can mention at the second time we use that tool that it should be done for each X event)

[tnobayashi]

I don't think I had to fund any accounts post-genesis, so maybe it's not needed 🤔 (but I used the same batcher address than you used for spec-13-op)

hmmmm, I was thinking of creating a table to show funding requirements on L1 and L2. Btw, for new networks, it's a good idea to use different batcher addresses. Using the same address for multiple networks cause nonce issue.

Maybe we can document this? (we can mention at the second time we use that tool that it should be done for each X event)

Sure, we could add it to an operations guide.

[tnobayashi]

I will need to carefully review this one and open a PR.

I think it looks good like this #11

Maybe we can document this? (we can mention at the second time we use that tool that it should be done for each X event)

What do you think about a note for now?
https://github.com/0xPolygon/cdk-devnets/pull/11/changes#diff-f1f969dcfdf8944ae7e56cb9182e9e1f9eb312c48cb9ec76ae2846fa2155cb1aR5