feat(starfish): evict pending commit votes via quorum commit index#11392
Merged
polinikita merged 3 commits intostarfish/feat/starfish-hardeningfrom May 7, 2026
Merged
Conversation
Base automatically changed from
starfish/feat/restrictions-quorum-guard
to
starfish/feat/starfish-hardening
April 30, 2026 09:38
polinikita
force-pushed
the
starfish/feat/restrictions-evict-commit-votes
branch
2 times, most recently
from
5c1023d to
ad1bc5f
Compare
polinikita
force-pushed
the
starfish/feat/restrictions-evict-commit-votes
branch
from
31ff1a1 to
279b13a
Compare
polinikita
force-pushed
the
starfish/feat/restrictions-evict-commit-votes
branch
from
279b13a to
58b3b6a
Compare
polinikita
force-pushed
the
starfish/feat/starfish-hardening
branch
from
7a2a964 to
47c1d00
Compare
polinikita
force-pushed
the
starfish/feat/restrictions-evict-commit-votes
branch
from
58b3b6a to
e1bf221
Compare
piotrm50
reviewed
May 5, 2026
VorobyevIlya
reviewed
May 7, 2026
Bounds the in-memory `pending_commit_votes` tracker in `DagState` by dropping votes that the network has already certified. - Switch `pending_commit_votes` from `VecDeque<CommitVote>` to `BTreeSet<CommitVote>` so eviction uses `split_off` and insertions dedup naturally. - Add `Option<Arc<CommitVoteMonitor>>` on `DagState`, wired in by `Core::new` after construction. - Add `evict_pending_commit_votes` that drops votes with `index <= quorum_commit_index - gc_depth`. Gated on `consensus_block_restrictions`. - Call the eviction from both `take_commit_votes` and `flush()`, mirroring the existing `pending_acknowledgments` pattern. Closes #11391
…tate writes - Introduce `SkipProposalReason` enum + `label()` for the `core_skipped_proposals` metric, mirroring `FastSyncPauseSource`. The variant data carries the per-reason context interpolated into the corresponding `debug!` line, so the label space and the message templates live in one place. - Extract `Core::skip_proposal` helper that emits the `debug!` line, bumps the metric and returns `false`. Each `should_propose` early return collapses from ~6 lines to 1–4. - Merge the two adjacent `dag_state.write()` acquisitions in `try_commit_inner` under one guard. - Reword the upstream-eligibility comment in `try_new_block`.
polinikita
force-pushed
the
starfish/feat/restrictions-evict-commit-votes
branch
from
e1bf221 to
957641c
Compare
polinikita
requested review from
DaughterOfMars,
alexsporn,
muXxer and
thibault-martinez
as code owners
github-actions
Bot
added
documentation
polinikita
force-pushed
the
starfish/feat/starfish-hardening
branch
from
47c1d00 to
b0db814
Compare
polinikita
requested review from
VorobyevIlya and
piotrm50
and removed request for
a team,
DaughterOfMars,
alexsporn,
muXxer and
thibault-martinez
polinikita
removed
documentation
VorobyevIlya
approved these changes
May 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of change
Bounds the in-memory
pending_commit_votestracker inDagStateand tightens the proposal skip paths inCore.Eviction (issue #11391)
pending_commit_votesfromVecDeque<CommitVote>toBTreeSet<CommitVote>so eviction usessplit_off. Insertion order is preserved (the linearizer produces commits monotonically); insertion dedup is a free side benefit.DagState:last_known_quorum_commit_index: CommitIndex(noArc<CommitVoteMonitor>reference).Coreis the sole holder of the monitor and pushes the value via a setter.evict_pending_commit_votes()drops votes withindex <= last_known_quorum_commit_index - gc_depth. Gated onconsensus_block_restrictions; no-op otherwise.take_commit_votes(proposal path) and fromflush()(mirrors the existingpending_acknowledgmentspattern).CorerefreshesDagState::last_known_quorum_commit_indexfromCommitVoteMonitor::quorum_commit_index()at three places:Core::try_commitafter every successfulcommit_observer.handle_committed_leaders(regular consensus path);Core::handle_committed_sub_dags_from_fast_sync(fast-sync path);Core::should_propose(right before we propose).should_proposecleanup (issue #11400)SkipProposalReasonenum +skip_proposalhelper, so thecore_skipped_proposalsmetric label space is disjoint and visible in one place.behind_quorum_commit_round— theconsensus_block_restrictionsgate (formerly insidetry_new_block) moves up toshould_propose.clock_round <= last_proposed_roundrecheck moves up too. It is silent (no metric, no debug log) since it fires on every accepted block in a round we have already proposed in — counting it would drown the genuinely interesting reasons.try_new_blocknow readsclock_roundonly; all gates are upstream.Links to any relevant issues
Fixes #11391
Fixes #11400
How the change has been tested
dag_state::test::test_evict_pending_commit_votescovering: quorum-driven eviction with the flag on; default-field no-op (no setter call); flag-off no-op even with a non-zero fieldcargo ci-clippyclean across the workspace