chore: follow up on #9390 on EIP-7688 changes - #9689
Merged
Conversation
EIP-7688 replaces List with ProgressiveList for validators and inactivityScores at gloas, changing the merkle tree shape. loadState() reused the seed state's list nodes unconditionally, so loading state bytes from a seed state on the other side of the gloas fork produced a state with a silently wrong hashTreeRoot. Deserialize these fields from the state bytes instead when the seed state cannot be reused. Also make the empty inactivityScores fallback use the state's own field type instead of the altair List type. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Gloas progressive containers report a near-unbounded SSZ maxSize, and the ssz-snappy decoder validates the length prefix only against type.maxSize. Post-gloas this left beacon_blocks_by_*, execution_payload_envelopes_by_* and data_column_sidecars_by_* responses effectively unbounded. Clamp request and response size bounds to MAX_PAYLOAD_SIZE, and to MAX_DATA_COLUMN_SIDECAR_SIZE for data column sidecar responses post-gloas, per https://github.com/ethereum/consensus-specs/blob/v1.7.0-alpha.12/specs/phase0/p2p-interface.md#encoding-strategies https://github.com/ethereum/consensus-specs/blob/v1.7.0-alpha.12/specs/gloas/p2p-interface.md#type-specific-ssz-bounds Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- deduplicate the nodesPopulated double-casts behind a single isViewDUNodesPopulated() helper - link ChainSafe/ssz#542 in the progressive list utils TODO - un-export gindex helpers only used within the light client spec utils Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![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: 4f63eff6a3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+40
to
+41
| for (let i = seedState.validators.length; i < migratedState.validators.length; i++) { | ||
| modifiedValidators.push(i); |
There was a problem hiding this comment.
When this fallback is used by loadCachedBeaconState() with a seed state that is not the target's ancestor, an overlapping validator can have different bytes or pubkey in the target. This branch reports only appended indices, unlike the normal loadValidators() path, so packages/state-transition/src/cache/stateCache.ts:183-188 will not refresh epochCtx.pubkeyCache for that index and signature verification for the loaded state can use the seed pubkey. Consider computing modified validator indices from the serialized validator bytes even though the tree nodes cannot be reused.
Useful? React with 👍 / 👎.
Contributor
Performance Report🚀🚀 Significant benchmark improvement detected
Full benchmark results
|
The seed state may not be an ancestor of the loaded state, so an overlapping index can hold a different validator. Diff the serialized validator bytes in the cross-fork path as well so that loadCachedBeaconState() refreshes the pubkey cache for those indices, matching the loadValidators() behavior. Only the tree-node reuse is skipped across the fork boundary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
nflaig
reviewed
Jul 22, 2026
1 task
twoeths
reviewed
Jul 29, 2026
| * appended past the seed state's validator count. Unlike loadValidators() this only diffs | ||
| * bytes and does not share the seed state's tree. | ||
| */ | ||
| function findModifiedAndAppendedValidators( |
Member
There was a problem hiding this comment.
I notice the modified validators are only consumed by loadCachedBeaconState() and noone uses it anymore, this is a dead code
so it's worth to leave a TODO and follow up in another cleanup PR or just change the signature of loadState() not to return modified validators
Member
There was a problem hiding this comment.
seems like this comment was not addressed @ensi321 can you follow up on it please
nflaig
reviewed
Aug 4, 2026
Member
There was a problem hiding this comment.
seems fine to me, but probably something @wemeetagain should review
wemeetagain
approved these changes
Aug 4, 2026
22 tasks
Member
|
🎉 This PR is included in v1.46.0 🎉 |
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
5 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.
Follow up on unaddressed comments from #9390