ethereum-optimism / superchain-registry

An index of chains which serves as the source of truth for who’s in the Superchain Ecosystem
MIT License
74 stars 80 forks source link

fix: avoid skipping checks for conduit chains #178

Closed mds1 closed 5 months ago

mds1 commented 6 months ago

From https://github.com/ethereum-optimism/superchain-registry/pull/107#pullrequestreview-1998488407

Approving and merging this PR, but we'll later need to investigate the Genesis test failures that are currently being skipped for Metal Sepolia, Metal Mainnet, and Superlumio Mainnet

We also are skipping the L2OO tests for those chains + Mode Sepolia, and should confirm that's ok.

geoknee commented 5 months ago

https://www.notion.so/oplabs/Superchain-Genesis-Hash-checks-60e173e9fb0c43e88a81d0666cb3d582

geoknee commented 5 months ago

Helpfully, we have the raw JSON config files for these chains now https://github.com/ethereum-optimism/superchain-registry/pull/226 .

From a slack thread:

For both chains (Superlumio and Metal Sepolia) which fail the check at the moment, we are hardcoding "eip1559Denominator": 50 which doesn't match the value of 0 used in the supplied json files. I don't think this makes a difference to the genesis block hash validation check.

For Superlumio, it looks like you have hardfork times "missing". In the registry we allow chains to override superchain-wide hardfork times, but not with nil values. So by not specifying overrides for e.g. ecotone_time, the chain is inheriting the superchain-wide hardfork times (and a bunch of other values that are pinned to that hardfork time) when we reconstruct the genesis, and this affects the genesis block hash which we compute in the check.

On a local branch I hacked around with this by deactivating the hardfork times for the entire supechain, then the checks for these chains pass.

I can see two possible resolutions:

geoknee commented 5 months ago

The key thing is if either canyon or ecotone is activated at or before genesis. this affects genesis block because of these lines https://github.com/ethereum-optimism/op-geth/blob/9617f2a7dd0ab80c468d6e474e3cc26be9572c97/core/genesis.go#L538-L552

Canyon implies Shanghai Ecotone implies Cancun

geoknee commented 5 months ago

Here's one way of fixing this https://github.com/ethereum-optimism/superchain-registry/compare/gk/skip-default-hf-times?expand=1.

The idea is to not specify hardfork times in a superchain wide way any more. Every chain has to specify their own hard fork times.

Pros:

Cons:

Here is the PR where the behaviour was added https://github.com/ethereum-optimism/superchain-registry/pull/101