refactor: use consistent names for state after payload is applied - #9175
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the codebase to rename 'envelope state' references to 'payload state' for improved clarity and consistency. Key changes include renaming variables like envelopeStateRoot to payloadStateRoot and updating the function computeEnvelopeStateRoot to computePayloadEnvelopeStateRoot. The review feedback suggests further simplifying the function name to computePayloadStateRoot and ensuring that associated metric sources and enum members are updated accordingly to maintain a uniform naming convention.
Contributor
Performance Report🚀🚀 Significant benchmark improvement detected
Full benchmark results
|
Member
Author
|
@lodekeeper can you review this PR, is the naming aligned with the consensus spec naming? |
Contributor
|
@nflaig Reviewed — the naming aligns with the consensus spec. The spec's fork-choice store uses The function name |
matthewkeil
reviewed
Apr 6, 2026
matthewkeil
left a comment
matthewkeil
left a comment
Member
There was a problem hiding this comment.
I tend to lean towards keeping the Payload vs PayloadEnvelope naming convention consistent for all places. I tend to go back and forth on which I prefer but in honestly they both feel fine to me. Mostly just think we should pick one and stick with it everywhere. Up to you which you prefer.
| * | ||
| */ | ||
| export function computeEnvelopeStateRoot( | ||
| export function computePayloadEnvelopeStateRoot( |
Member
There was a problem hiding this comment.
Why did you choose to use the full PayloadEnvelope notation here. Would computePayloadStateRoot be more uniform?
Member
Author
There was a problem hiding this comment.
because this function computes the state root of the envelope for the ExecutionPayloadEnvelope container
lodekeeper pointed this out in this comment too
The function name computePayloadEnvelopeStateRoot (keeping "Envelope") is also correct — it computes the state_root field for the ExecutionPayloadEnvelope container, which is a different concept from the post-payload state itself.
I had it as computePayloadStateRoot initially but then realized it's semantically less accurate
Contributor
There was a problem hiding this comment.
Thanks for the feedback. Agreed consistency matters — we went with specifically because it computes the state_root field for the ExecutionPayloadEnvelope container (per spec: assert envelope.state_root == hash_tree_root(state)). Including "Envelope" clarifies the semantic: it's the root that lives in the envelope, not just the post-payload state root. Happy to revisit if there's a strong preference, but this felt like the more precise name.
|
|
||
| const hashTreeRootTimer = metrics?.stateHashTreeRootTime.startTimer({ | ||
| source: StateHashTreeRootSource.computeEnvelopeStateRoot, | ||
| source: StateHashTreeRootSource.computePayloadEnvelopeStateRoot, |
Member
There was a problem hiding this comment.
Same comment about PayloadEnvelope as the function name
Member
Author
there is a nuance to this, the envelope is the object used to transport the payload (a wrapper) that contains extra metadata so just picking one and using it everywhere is less accurate, this nuance also exists in the spec, there is |
matthewkeil
approved these changes
Apr 6, 2026
matthewkeil
left a comment
matthewkeil
left a comment
Member
There was a problem hiding this comment.
Ok. I'm fine now that you explained the nuance
ensi321
approved these changes
Apr 7, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #9175 +/- ##
=========================================
Coverage 52.53% 52.53%
=========================================
Files 848 848
Lines 61429 61427 -2
Branches 4528 4528
=========================================
Hits 32270 32270
+ Misses 29094 29092 -2
Partials 65 65 🚀 New features to boost your workflow:
|
2 tasks
ensi321
added a commit
that referenced
this pull request
Apr 17, 2026
…#9218) ## Summary Depends on #9215 - Remove `payloadPresent` parameter from regen interfaces (`addCheckpointState`, `updatePreComputedCheckpoint`, `getCheckpointStateOrBytes`, `getCheckpointStateSync`) - Remove `processPayloadState`, `upgradeForGloas` from `IStateRegenerator` - Remove `PayloadStatus` → `payloadPresent` conversion logic from regen (queued.ts, regen.ts) - Remove `UNEXPECTED_PAYLOAD_STATUS` and `INTERNAL_ERROR` regen error codes - Remove `CheckpointHexPayload` type and `fcCheckpointToHexPayload` helper — all callers now use `CheckpointHex` - Delete `computePayloadEnvelopeStateRoot()` and remove `payloadEnvelopeStateRoot` from `ProduceFullGloas` type - Remove `computePayloadEnvelopeStateRoot` from `StateHashTreeRootSource` enum - Rename `processBlockState` → `processState` and `postBlockState` → `postState` to remove dual-state naming Reverts #9175 ## Test plan - [x] `pnpm check-types` passes - [x] `pnpm lint` passes 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Member
|
🎉 This PR is included in v1.43.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.
Rename
postEnvelopeStatetopostPayloadStatefor consistency with the naming used inimportExecutionPayloadand regen caching methods.Closes #9126