search

ethereum-optimism / superchain-registry

Superchain-configuration data in a portable, extensible format.
MIT License
59 stars 50 forks source link

Regenerate rollupConfig via op-node and check against local rollup.json #316

Closed bitwiseguy closed 5 days ago

bitwiseguy commented 1 week ago

Addresses the rollup.json check described in this issue: https://github.com/ethereum-optimism/superchain-registry/issues/286

geoknee commented 1 week ago

When I tried to run this, I got 

 failed when using op-node to generate rollup config: unknown chain ID: 42069

which makes sense if we are erroring before we have written the files into the registry (the op-node can only load files which have been embedded). Also seeing this error on CI.

What are your thoughts about working around this? Probably the check needs to run after the files have been written by the add-chain tool, I think that would be sufficient.

bitwiseguy commented 1 week ago

When I tried to run this, I got

 failed when using op-node to generate rollup config: unknown chain ID: 42069

which makes sense if we are erroring before we have written the files into the registry (the op-node can only load files which have been embedded). Also seeing this error on CI.

What are your thoughts about working around this? Probably the check needs to run after the files have been written by the add-chain tool, I think that would be sufficient.

The issue is that the op-node code imports the superchain-registry/superchain package, which contains actual chain config .yaml files from that package. But the add-chain tests use the mock testdata directory structure as the destination to write .yaml files. So the rollup.LoadOPStackRollupConfig(rollupConfig.ChainID) call will not see the test .yaml files generated during testing.

What do you mean by "the check needs to run after the files have been written by the add-chain tool, I think that would be sufficient"? Maybe we move the rollup.json sanity check from the add-chain main flow to a add-chain subcommand that is only run by CI (instead of getting triggered by the current add-chain/e2e_test.go?