fix: prevent SMT corruption during HA leadership churn#158
Conversation
Storage pressure could cause leadership churn while old child-round goroutines were still waiting for parent proofs. A stale finalizer could then write the current round's pending SMT data under an older block, causing blocks, block_records, aggregator_records, and smt_nodes to diverge. Fix by cancelling and waiting for in-flight round processing on Deactivate, refusing to overwrite a same-or-newer active round on reactivation, and validating round number, snapshot root, and duplicate-block contents before finalization continues.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the block finalization and round management logic in the RoundManager to prevent stale finalizations, handle duplicate blocks safely, and manage activation/deactivation lifecycles properly. The reviewer feedback highlights critical concurrency issues in Activate where re-activation or partial failure can lead to resource leaks and inconsistent states. Additionally, several performance and safety improvements were suggested, such as using Cmp for big integer comparisons to avoid nil pointer dereferences and avoiding unnecessary slice allocations during block finalization.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
MastaP
left a comment
MastaP
left a comment
There was a problem hiding this comment.
Two findings from a focused review of the child-sharding HA path. Context: in child mode the child RoundManager has no BFT client (it talks to the parent via the root client), and finalization is driven by processRound → pollForParentProof → FinalizeBlockWithRetry, which Deactivate's new roundWG.Wait() correctly covers — and cancel-during-poll is handled and tested. These two are additional to the existing automated-review comments: one High (a leadership stand-down landing in the finalize window is misclassified as fatal and crashes the node) and one Medium (the new duplicate-mismatch guard is untested because Test8 exits early at the snapshot-root guard).
2 participants
Storage pressure could cause HA leadership churn (the aggregator instance repeatedly lost and reacquired
leadership) while old child-round goroutines were still waiting for parent proofs. A stale finalizer could then write the current round's pending SMT data under an older block, causing blocks, block_records, aggregator_records, and smt_nodes to diverge.
Fix by cancelling and waiting for in-flight round processing on Deactivate, refusing to overwrite a same-or-newer active round on reactivation, and validating round number, snapshot root, and duplicate-block contents before finalization continues.