mirror of
https://github.com/luxfi/consensus.git
synced 2026-09-24 00:26:58 +00:00
zeekayandHanzo Dev
bdc1190f58
Two defects, both found by comparing against upstream's single decided-predicate. The held-copy preference lived inside the >finalized+1 arm of the height switch, so the one height that MOVES the ledger — exactly finalized+1 — kept the unconditional Verify. A node behind on certs holds its whole gap, so that Verify is a re-insert of an old canonical block; past the commit interval it is ErrPrunedAncestor and the VM answers "side chain insertion is not supported". The refusal returned before the block was tracked, and pathFromTip then deferred every cert above the resulting one-height hole. Measured: at finalized+1 Verify calls=1 tracked=false; at finalized+2 Verify calls=0 tracked=true. The arms differ in what they do with a block, not in whether it needs verifying, so the rule now sits above the switch. Second, the VM read-back was wired only in NewWithConfig, guarded on the VM being present at construction. The node builds through NewWithParams with no VM and attaches one afterwards with SetVM, which never wired it — so the walk fell back to this process's own tree and the recovery it exists for never ran outside tests, where New(WithVM(vm)) supplies the VM up front. wireRecall is now called from both. Contiguity is unchanged and is deliberately kept strict: a cert riding with a block above our next height is no longer offered to HandleIncomingCert. With the walk able to resolve intermediate blocks, offering it would let one cert carry finality across a gap the per-height guard exists to refuse — TestCatchup_OutOfOrderRefusedThenInOrderConverges caught exactly that. Finality still only advances in order, one verified cert at a time. Tests: finalized+1 with a held block is not re-verified; recall resolves after SetVM on the construction path the node uses. Negative controls executed for both — each fails only its own assertion. Co-authored-by: Hanzo Dev <[email protected]>