Skip to content

Conversation

@Andrurachi
Copy link

Summary

This PR ensures that the split block’s execution payload is never pruned during weak subjectivity checkpoint sync.
A guard is added to try_prune_execution_payloads so that pruning is skipped when block_root == split_block_root.

Two new regression tests verify this behavior across aligned and unaligned checkpoint scenarios.

Changes

Code

  • Add condition in try_prune_execution_payloads to skip pruning for the split block root.

Tests

  • Add helper verify_pruning_preserves_payload to set up aligned/unaligned checkpoint cases, enable prune_payloads = true, run checkpoint sync, and assert the split block payload is preserved.

  • Add:

    • pruning_preserves_payload_aligned
    • pruning_preserves_payload_unaligned

Relation to Other Work

The new test pruning_preserves_payload_unaligned covers the failure mode tested by this open PR #8458 and goes further. It verifies both that:

  • the BeaconChain initializes successfully (same as the older test)
  • the split block’s execution payload is actually present in the store after sync

Because of this, the earlier reproduction test becomes partially redundant, but it can still be useful because it explicitly demonstrates the original bug fixed in #8426

Issue

Fixes: #8431

Add a condition to skip pruning when the block is the split block root.
Prevents `try_prune_execution_payloads` from deleting the split block's
execution payload.

Add two tests `pruning_preserves_payload_aligned` and
`pruning_preserves_payload_unaligned` using the helper
`verify_pruning_preserves_payload` to ensure correct behavior.
@Andrurachi Andrurachi force-pushed the chore-dont-prune-split-payload-8431 branch from 0121f88 to e9311d8 Compare December 16, 2025 00:36
@Andrurachi Andrurachi marked this pull request as draft December 16, 2025 00:38
@Andrurachi
Copy link
Author

Hey @michaelsproul, I have a quick question on test coverage.

This PR covers the same regression as the recently merged reproduction_unaligned_checkpoint_sync_pruned_payload test (#8458), but with a stronger and more explicit assertion: it directly checks that the split block’s execution payload is preserved (rather than inferring correctness from successful startup and anchor payload availability).

Would you prefer removing the earlier reproduction test to avoid overlapping coverage, or keeping both? Happy to update the PR either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant