Skip to content

Conversation

@dapplion
Copy link
Collaborator

@dapplion dapplion commented Oct 29, 2025

Issue Addressed

Part of a fork-choice tech debt clean-up #8325

#7089 (non-finalized checkpoint sync) changes the meaning of the checkpoints inside fork-choice. It turns out that we persist the justified and finalized checkpoints twice in fork-choice

  1. Inside the fork-choice store
  2. Inside the proto-array

There's no reason for 2. except for making the function signature of some methods smallers. It's not consistent with the rest of the crate, because in some functions we pass the external variable of time (current_slot) via args, but then read the finalized checkpoint from the internal state. Passing both variables as args makes fork-choice easier to reason about at the cost of a few extra lines.

Proposed Changes

Remove the unnecessary state (justified_checkpoint, finalized_checkpoint) inside ProtoArray, to make it easier to reason about.

@dapplion dapplion added the ready-for-review The code is ready for review label Oct 29, 2025
Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Needs a schema migration, see my comment.

@michaelsproul michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. backwards-incompat Backwards-incompatible API change and removed ready-for-review The code is ready for review labels Nov 5, 2025
@michaelsproul michaelsproul added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. backwards-incompat Backwards-incompatible API change labels Nov 11, 2025
@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Nov 12, 2025
@mergify mergify bot added the queued label Nov 12, 2025
mergify bot added a commit that referenced this pull request Nov 12, 2025
@mergify mergify bot merged commit 53e73fa into sigp:unstable Nov 12, 2025
36 checks passed
@mergify mergify bot removed the queued label Nov 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants