mirror of
https://github.com/luxfi/consensus.git
synced 2026-09-24 00:26:58 +00:00
zeekayandHanzo Dev
4256fb283c
a749cfc6 closed a real window and its commentary said it closed THE window. It did not.
Review reproduced the identical symptom arriving through a second, far wider one, and
that one is the more likely production cause precisely because it is wider.
W2: from the cert freezing in assembleCertLocked to finalizedByCert being set — an
interval spanning ApplyCert AND the whole of VM.Accept, i.e. the EVM state write.
Throughout it the block is STILL PENDING, so handleVote never consults finalizedByCert
or acceptedPos at all. The vote lands in pending.certVotes, assembleCertLocked returns
the already-frozen cert, and dropPendingBlockLocked deletes the PendingBlock with the
vote inside it. Remembering the position early does not help: nothing on that path looks
at it. Measured in-window — stillPending=true certFrozen=true certVotes=3
finalizedByCert=false acceptedPosKnown=true — with 4 of 5 voting reading back
ResponsiveStake=0.60 Degraded=true QuasarHeight=0, and re-delivering the identical vote
exports it, so the vote is dropped rather than rejected.
This commit does NOT close W2. Draining pending.certVotes into the attestor at the drop
is its own change with its own review, because ingesting on the pending path naively
emits before the Nova ledger holds the height, and PromoteQuasar then discards the cert.
What it does:
- votesDroppedAtCertFreeze counts signed accept votes that reached certVotes after the
cert froze and are discarded with the PendingBlock, surfaced through Stats() as
votes_dropped_at_cert_freeze. This is the discriminator for a question no green test
suite can answer: 0/40 under 4x CPU contention bounds the flake rate at ~7.5% but
cannot say WHICH window produced it. Non-zero over a few thousand live heights means
W2 fires in production and the drain is required; zero means W1 was the whole story.
- The ordering test's anti-vacuity guard was DEAD — the Fatal above it already returned
on !posAtAccept, so its condition could never hold. Measurement shows the probe
stands at a point strictly EARLIER than the window (pending=true,
finalizedByCert=false), not "exactly the interval that used to be empty" as claimed.
The test remains sound because nothing removes the entry in between, so earlier
implies later — but that is an ordering argument, and it now says so. The guard
pins the probe's location instead, so the argument cannot rot.
- The FourOfFive gate exempted ids.Empty, which is exactly the empty canonical
SyncQuasarFrontier accepts — the one case the check cites as its reason to exist.
Exemption removed.
- Noted at the hoist: acceptedPos is now written unconditionally, where it previously
sat behind promoteQuasar's stake==nil / attestor==nil early returns, so non-attestor
nodes populate it. The ≤1024 sweep is not attestor-gated, so this costs a bounded map.
Co-authored-by: Hanzo Dev <[email protected]>