fix: repair Gloas block replay crash with IncorrectStateVariant#8876
fix: repair Gloas block replay crash with IncorrectStateVariant#8876bit2swaz wants to merge 1 commit intosigp:stablefrom
Conversation
…igp#8869) Gloas (EIP-7732) decouples the execution payload from the beacon block: the block body carries only a signed_execution_payload_bid, while the actual payload arrives as a separate SignedExecutionPayloadEnvelope. Two code paths in per_block_processing incorrectly called pre-Gloas accessors that are not defined on Gloas block bodies: 1. process_execution_payload / process_withdrawals: is_execution_enabled() returns true for Gloas (Capella is always enabled), so body.execution_payload()? was called and returned Err(IncorrectStateVariant) for Gloas block bodies. 2. process_operations (Electra path): electra_enabled() is also true for Gloas, causing block_body.execution_requests()? to be called. That field only exists on Electra/Fulu bodies, so Gloas returned Err(IncorrectStateVariant) again. Fixes applied: so they are skipped for Gloas blocks. - Add process_execution_payload_envelope() which applies a SignedExecutionPayloadEnvelope to a Gloas state (updates latest_block_hash; TODO: withdrawals, execution_requests, etc.). - Add a look-ahead loop in BlockReplayer: after each Gloas block N, compare block N+1's bid.parent_block_hash against state.latest_execution_payload_bid.block_hash; if they match, apply the envelope for block N before continuing. - Add DBColumn::ExecPayloadEnvelope and put/get_execution_payload_envelope methods in hot_cold_store so envelopes can be stored and retrieved by ExecutionBlockHash. - Wire replay_blocks to load envelopes for all Gloas blocks in the replay range and pass them to BlockReplayer. A regression test gloas_block_replay_no_incorrect_state_variant confirms the fix: it builds a minimal Gloas genesis and replays one Gloas block, asserting per_block_processing succeeds.
|
|
There was a problem hiding this comment.
Looks like this PR duplicates some DB functionality that already exists from this PR
https://github.com/sigp/lighthouse/pull/8717/changes#top
We should just use the existing db operations. Also note that the payload envelopes are keyed by beacon block root, not execution block hash
| /// - Key: 32-byte execution `block_hash` from the block's `signed_execution_payload_bid`. | ||
| /// - Value: SSZ-encoded `SignedExecutionPayloadEnvelope`. | ||
| #[strum(serialize = "ppe")] | ||
| ExecPayloadEnvelope, |
There was a problem hiding this comment.
We already have a PayloadEnvelope column defined
| /// The key is the execution `block_hash` committed to by the builder in the corresponding | ||
| /// `signed_execution_payload_bid`. This allows efficient lookup during block replay using | ||
| /// only the information present in each block's bid header. | ||
| pub fn put_execution_payload_envelope( |
There was a problem hiding this comment.
we already have a function payload_envelope_as_kv_store_ops that does this
| /// Returns `None` when no envelope has been stored for the given hash (e.g. the builder did | ||
| /// not deliver the payload for that slot, or envelope storage is not yet implemented for the | ||
| /// current database schema). | ||
| pub fn get_execution_payload_envelope( |
There was a problem hiding this comment.
we already have a function get_payload_envelope that does this. Note that we fetch execution payload envelopes by block root, not by execution block hash
eserilev
added
waiting-on-author
|
I'm sorry, I'm going to go ahead and close this PR. This reads as fully LLM-generated which isn't something were interested in accepting. I'm not anti-LLM but we could produce this same output ourselves in a matter of minutes. Contributing should be a learning experience and I'm happy to support external contributors who want to dig in and genuinely engage with the code. |
ah, fair point. i leaned way too heavily on an llm to try and blast through the boilerplate this weekend and completely missed the broader context of #8717 and the correct db keys. lesson learned. i genuinely want to dig into the codebase and learn the EIP-7732 architecture properly. if youre still open to it, id love to scrap this branch, review the db operations you linked in #8717, and take a manual, ground up pass at wiring the look ahead loop the right way. if not, i completely understand keeping it closed. sorry for any inconveniences i may have caused |
2 participants
Issue Addressed
Fixes #8869
Proposed Changes
Gloas (EIP-7732) decouples the execution payload from the beacon block, the block body carries only a
signed_execution_payload_bid, while the actual payload arrives separately as aSignedExecutionPayloadEnvelope. Two code paths inper_block_processingwere calling pre-Gloas accessors that don't exist on Gloas block bodies, causing block replay to crash withIncorrectStateVariantand the HTTP API to return500 UNHANDLED_ERRORfor any finalizedGloas state.
Bug 1: execution payload path (
per_block_processing.rs)is_execution_enabled()returnstruefor Gloas (Capella is always enabled, makingis_merge_transition_completereturn true). This causedbody.execution_payload()?to be called on a Gloas body, which returnsErr(IncorrectStateVariant).Fix: guard the
process_withdrawals/process_execution_payloadblock with&& !gloas_enabled().Bug 2: Electra operations path (
process_operations.rs)electra_enabled()is alsotruefor Gloas, causingblock_body.execution_requests()?to be called. That field only exists on Electra/Fulu bodies, so Gloas returnedErr(IncorrectStateVariant).Fix: guard the execution-requests block with
&& !gloas_enabled().Envelope processing architecture
Following the EIP-7732 spec design, envelopes are applied between blocks using a look-ahead:
after applying block N, if block N+1's
bid.parent_block_hashmatchesstate.latest_execution_payload_bid.block_hash, the envelope for block N is applied before continuing. This is wired through:process_execution_payload_envelope(): new function that validates the hash chain and updatesstate.latest_block_hash.BlockReplayer: newpayload_envelopesmap and look-ahead loop inapply_blocks.HotColdDB: newDBColumn::ExecPayloadEnvelope,put/get_execution_payload_envelopemethods, andreplay_blockswired to load and pass envelopes for any Gloas blocks in the replay range.Regression test
gloas_block_replay_no_incorrect_state_variant: builds a minimal Gloas genesis, constructs avalid blinded Gloas block at slot 1, and asserts
BlockReplayer::apply_blockssucceeds.Additional Info
process_execution_payload_envelopeis intentionally minimal for now (updates latest_block_hash`, validates parent hash chain). Withdrawals, execution requests, and other envelope contents are left as TODOs, consistent with the broader in-progress EIP-7732 implementation in this codebase.put_execution_payload_envelopeis added but not yet called at block import time; that wiring belongs in the beacon chain's block processing pipeline and can be done as a follow-up once the full Gloas block import path is built out.state_processingunit tests pass.cargo clippy -D warningsandcargo fmtare clean on the changed packages.