fix: harden system-contract bytecode and storage-layout verification#316
Open
RealiCZ wants to merge 3 commits into
Open
fix: harden system-contract bytecode and storage-layout verification#316RealiCZ wants to merge 3 commits into
RealiCZ wants to merge 3 commits into
Conversation
…gating doc comment
- Attest the latest-bytecode alias at build time: verify its own keccak256(bytecode)==code_hash and that it matches a versioned artifact on both version and code hash (the generated LATEST_CODE aliases V{version}_CODE, so matching on hash alone could point the alias at the wrong version).
- Track the generated constant files with cargo::rerun-if-changed so editing one re-runs build-time verification.
- Emit a build warning when source attestation cannot run (no Foundry/Solidity source, e.g. a published crate) instead of silently trusting the generated constants; add a test asserting keccak256(code)==code_hash for every generated constant, which runs in any context.
- Add a test asserting the SequencerRegistry Rust storage-slot constants match Foundry's reported storage layout; it skips gracefully when Foundry or the Solidity source/config are unavailable (forge-less env or packaged crate).
- Order the Cargo.toml dependency sections so the sort check passes.
- Correct an inverted doc comment on the spec version-gating check so it matches the code (a version at or below the current spec is enabled).
RealiCZ
added
spec:unchanged
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 433334e71a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
|
Review: LGTM with one minor note The PR is well-structured and closes a real gap: the Specific things done well:
One small note left inline. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
|
LGTM. Prior feedback addressed; no new issues in the latest push. |
Troublor
approved these changes
Jun 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hardens the build-time verification of the embedded system-contract bytecode and storage-layout constants, and corrects a misleading doc comment. No runtime or consensus behavior changes — the diff is confined to
build.rs, tests, a dev-dependency, and documentation.*-latest.jsonbytecode alias. Versioned artifacts are self-hash-validated when collected, but the*-latest.jsonalias was loaded and trusted without any check. It is now verified to (a) be internally consistent (keccak256(deployed_bytecode) == code_hash) and (b) match one of the attested versioned artifacts on both version and code hash. The generatedLATEST_CODEaliasesV{version}_CODE, so binding version+hash prevents a latest alias whose bytecode is one version but whoseversionfield says another from pointing at the wrong versioned constant.build.rsnow emitscargo::rerun-if-changedfor eachsrc/generated/*.rs, so editing a generated constant re-runs verification instead of compiling unverified.scripts/are unavailable (e.g. building from a published crate), the build emits acargo::warningmaking the skipped source-attestation explicit rather than silently trusting the checked-in constants.keccak256(CODE) == CODE_HASHfor every exported bytecode constant. Unlike the build-time source-attestation, it runs viacargo testin any context — including a published crate — catching an out-of-sync or tampered generated file.SequencerRegistry. The hand-maintained Rust storage-slot constants had no automated check against the Solidity layout (the existing Solidity test is tautological). A new test asserts each constant matchesforge inspect SequencerRegistry storageLayout, catching a field reorder or a missed constant update. It skips gracefully when Foundry or the Solidity source aren't present.is_enableddoc comment, which described the version-gating direction backwards (the current spec enables every version at or below it).Test plan
tests/generated_self_consistency.rs—keccak256(CODE) == CODE_HASHfor all exported constants across the 5 contracts.tests/sequencer_registry_storage_layout.rs— Rust slot constants vsforge inspectstorage layout, with a field-count guard.test_naming convention.cargo test -p mega-system-contractsand the fullmega-evmsuite pass;cargo sort --check,cargo fmt --check, andRUSTFLAGS="-D warnings" cargo clippyare clean.