core: do not trigger rewind behaviour when modifying pre-genesis hardfork times / blocks

ethereum-optimism / op-geth

GNU Lesser General Public License v3.0

280 stars 735 forks source link

core: do not trigger rewind behaviour when modifying pre-genesis hardfork times / blocks #332

Closed geoknee closed 3 months ago

geoknee commented 3 months ago

Replaces https://github.com/ethereum-optimism/op-geth/pull/257

Currently, no Optimism hard forks are checked as a part of this function: https://github.com/ethereum-optimism/op-geth/blob/0d7fd67bfa00ea6dfc2e1371ea72db001731b43d/params/config.go#L807-L876

So modifying an optimism hard fork time will not cause a chain rewind, whereas modifying a non-optimism hardfork will cause a chain rewind.

Towards https://github.com/ethereum-optimism/client-pod/issues/918 Counterpart to https://github.com/ethereum-optimism/superchain-registry/pull/260

adds all of the optimism hard forks to the check so they have the same behaviour
modifies the check so that changing a hardfork from a pregenesis timestamp to another pregenesis timestamp is considered a compatible change.

geoknee commented 3 months ago

Next steps following sync with @sebastianst

Add some unit test for checkCompatible and patch it to include all Optimism hard forks
Patch it again so that pre-genesis hardfork times are always considered compatible
Test out the a "test release" of op-geth on a) a devnet replica and/or b) our prod metal replica (but protect that one first by asking devinfra to take a disk snapshot prior)

geoknee commented 3 months ago

Shanghai activates with Canyon The code that ties these together is outside of the function covered by the test on this PR. So a "live" test which say modifies a canyon time can also modify the shanghai time (which would cause a chain rewind currently).

geoknee commented 3 months ago

Next steps following sync with @sebastianst

Add some unit test for checkCompatible and patch it to include all Optimism hard forks

Done

Patch it again so that pre-genesis hardfork times are always considered compatible

Done

Test out the a "test release" of op-geth on a) a devnet replica and/or b) our prod metal replica (but protect that one first by asking devinfra to take a disk snapshot prior)

Done!

geoknee commented 3 months ago

We already pass a uint46 ptr to the isTimestampPreGenesis, so we might as well also pass it to CheckCompatible.

2a67982