Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

validation: check genesis predeploys against standard config #372

Closed
geoknee opened this issue Jul 10, 2024 · 10 comments · Fixed by #489
Closed

validation: check genesis predeploys against standard config #372

geoknee opened this issue Jul 10, 2024 · 10 comments · Fixed by #489
Assignees
Labels
BHIC? This ticket _might_ be covered by BHIC (Block History Integrity Checks) and therefore not necessary.

Comments

@geoknee
Copy link
Collaborator

geoknee commented Jul 10, 2024

Predeploy spec: https://specs.optimism.io/protocol/predeploys.html

Standard config spec states

[Genesis state should have] Only standard predeploys and preinstalls, no additional state.

@geoknee geoknee self-assigned this Jul 10, 2024
@geoknee geoknee changed the title validtaion: check genesis predeploys against standard config validation: check genesis predeploys against standard config Jul 10, 2024
@geoknee geoknee added the BHIC? This ticket _might_ be covered by BHIC (Block History Integrity Checks) and therefore not necessary. label Jul 10, 2024
@geoknee
Copy link
Collaborator Author

geoknee commented Jul 10, 2024

#373 is a sketch at a solution for this.

@geoknee geoknee added BHIC? This ticket _might_ be covered by BHIC (Block History Integrity Checks) and therefore not necessary. and removed BHIC? This ticket _might_ be covered by BHIC (Block History Integrity Checks) and therefore not necessary. labels Jul 10, 2024
@geoknee
Copy link
Collaborator Author

geoknee commented Jul 11, 2024

Q: with interoperable OP is it problematic if a chain enables the governance token on their chain?
A1: This OP token thing — there should be nothing deployed I think
A2: (Possibly) EAS predeploys — idk if chains other than OP Mainnet and Base have them. if they do, great, if not, I don’t think we need to require,

@geoknee
Copy link
Collaborator Author

geoknee commented Jul 16, 2024

From chat today with Seb:

Some predeploys (at least GasPriceOracle for Ecotone and Fjord, we can check for any others) have been upgraded during hardforks. So the test should switch on which fork is activated, and make the appropriate assertion pulling bytecode from the monorepo, or copying the codehash.

@geoknee
Copy link
Collaborator Author

geoknee commented Jul 16, 2024

@geoknee
Copy link
Collaborator Author

geoknee commented Jul 17, 2024

Here are some results investigating the distribution of codehashes across the superchain registry. (For each predeploy implementation address, it shows a number of codehashes, each with a list of chains which use that implementation).

https://github.com/ethereum-optimism/superchain-registry/blob/3ff4a2d6e69d248c36a9a1fee8a9b7df461d7710/validation/results.toml

@BlocksOnAChain
Copy link

We should check with @blmalone about his approach for this check.

@bitwiseguy
Copy link
Collaborator

We are planning to leverage @blmalone script. The work for this ticket will be to make sure the script is fully automated and integrate it into the superchain-registry CI

@geoknee
Copy link
Collaborator Author

geoknee commented Aug 13, 2024

We should take a look at the genesis tooling, and “save” any important metadata into a file which (along with the deploy-config.json) gets tracked in the SCR. The principle is we want to be able to run the tool in the future to produce the identical result.

@BlocksOnAChain
Copy link

just to increase visibility, I'm sharing a document with the current document for this implementation: https://www.notion.so/oplabs/Genesis-validation-update-f453b1b676d440efb0ac43adc05cab2f - we are thinking that approach 2 is what we will do to get this check done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BHIC? This ticket _might_ be covered by BHIC (Block History Integrity Checks) and therefore not necessary.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants