Skip to content

Add mixed V17/V29 execution payload invalidation test#9089

Merged
mergify[bot] merged 2 commits intosigp:unstablefrom
dapplion:dapplion/fc-mixed-invalidation-test
Apr 16, 2026
Merged

Add mixed V17/V29 execution payload invalidation test#9089
mergify[bot] merged 2 commits intosigp:unstablefrom
dapplion:dapplion/fc-mixed-invalidation-test

Conversation

@dapplion
Copy link
Copy Markdown
Collaborator

@dapplion dapplion commented Apr 4, 2026

Follow-up from #9025. When a V17 block is invalidated, its V29 descendants were added to the invalidated set but their weight was never zeroed because apply_score_changes only checks execution_status on V17 nodes. Fixes by zeroing V29 node weight directly during invalidation propagation, and adds a test for the mixed V17/V29 case.

@dapplion dapplion added ready-for-review The code is ready for review gloas labels Apr 4, 2026
@dapplion dapplion force-pushed the dapplion/fc-mixed-invalidation-test branch from ae4049b to 41d8b9f Compare April 4, 2026 01:59
Err(_) => (),
// V29 nodes don't have execution_status. Zero their weight
// so invalidation propagates across the V17→V29 boundary.
Err(_) => *node.weight_mut() = 0,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? filter_block_tree would block the branch at the invalid v17 parent right?

would this become non-zero when apply_score_changes gets called again?

/// genesis(V17) -> block_1(V17, slot 31) -> block_2(V29, slot 32)
///
/// Invalidate block_1, verify that both block_1 and its V29 descendant block_2 are zeroed.
fn get_gloas_mixed_invalidation_test_definition() -> ForkChoiceTestDefinition {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be moved under the tests mod?

@jimmygchen jimmygchen added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Apr 9, 2026
…ests

filter_block_tree already excludes the invalid branch from head selection,
and the weight zeroing was not durable across apply_score_changes calls.
Keep the test focused on verifying head falls back to genesis.
@jimmygchen jimmygchen added the test improvement Improve tests label Apr 13, 2026
@jimmygchen jimmygchen changed the title Fix V29 weight leak on execution payload invalidation Add mixed V17/V29 execution payload invalidation test Apr 16, 2026
Copy link
Copy Markdown
Member

@jimmygchen jimmygchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Apr 16, 2026
@mergify mergify Bot added the queued label Apr 16, 2026
@mergify
Copy link
Copy Markdown

mergify Bot commented Apr 16, 2026

Merge Queue Status

This pull request spent 31 minutes 9 seconds in the queue, including 29 minutes 15 seconds running CI.

Required conditions to merge

mergify Bot added a commit that referenced this pull request Apr 16, 2026
@mergify mergify Bot merged commit a9f43f9 into sigp:unstable Apr 16, 2026
39 checks passed
@mergify mergify Bot removed the queued label Apr 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork-choice gloas ready-for-merge This PR is ready to merge. test improvement Improve tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants