Skip to content

Conversation

@GrapeBaBa
Copy link
Contributor

πŸ—’οΈ Description

Fix finalization pruning to handle duplicate roots by tracking all slots per root.

πŸ”— Related Issues or PRs

βœ… Checklist

  • Ran tox checks to avoid unnecessary CI fails:
    uvx tox
  • Considered adding appropriate tests for the changes.
  • Considered updating the online docs in the ./docs/ directory.

Copilot AI review requested due to automatic review settings January 16, 2026 15:46
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug in finalization pruning where duplicate block roots in historical data (e.g., ZERO_HASH for missed slots) would cause incorrect pruning of pending justifications. The fix tracks all slots associated with each root rather than just one, ensuring that justifications are preserved if any associated slot remains after the finalized boundary.

Changes:

  • Modified process_attestations to track all slots per root using dict[Bytes32, list[Slot]] instead of dict[Bytes32, Slot]
  • Updated pruning logic to check if any slot for a root is beyond the finalized boundary
  • Added test coverage for the duplicate roots scenario

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/lean_spec/subspecs/containers/state/state.py Fixed root-to-slot mapping to handle duplicate roots by storing all slots per root, and updated pruning logic to check all slots
tests/lean_spec/subspecs/containers/test_state_justified_slots.py Added test case to verify duplicate roots are handled correctly during finalization, and updated imports to include required types and helpers

πŸ’‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

)


def test_pruning_with_duplicate_roots_keeps_pending_justification() -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've checked to run this test locally on the main branch without your fix and it works so I think it doesn't test the right thing regarding your fix in this PR.

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.

2 participants