Fix unguarded worker_index read and harden POS commit-reveal - #216
Fix unguarded worker_index read and harden POS commit-reveal#216Tore-tto wants to merge 1 commit into
Conversation
-master_node_voting: the signature-failure log read quorum.workers[vote.state_change.worker_index] for all vote types, but worker_index only exists on obligation votes -- for a checkpoint vote it aliases attacker-controlled bytes and indexes an empty vector. Guard it by vote type. Also re-enable the two success logs at MTRACE (level 3) instead of leaving them commented out. -pos: only accept a random value reveal from a quorum position that actually committed a hash, so the commit-reveal binding is enforced locally rather than relying on the stage timeout. -pos: drop the dead std::sample in signature selection and fix the comment -- the loop always used the first N.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes update vote-signature logging, reject random values without committed hashes, and replace random final-signature selection with deterministic selection of the lowest participating validator positions. ChangesVoting and PoS validation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The changes are merge-ready after normal checks and review; no actionable merge-blocking risk remains. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
master_node_voting: the signature-failure log read quorum.workers[vote.state_change.worker_index] for all vote types, but worker_index only exists on obligation votes -- for a checkpoint vote it aliases attacker-controlled bytes and indexes an empty vector. Guard it by vote type. Also re-enable the two success logs at MTRACE (level 3) instead of leaving them commented out.
pos: only accept a random value reveal from a quorum position that actually committed a hash, so the commit-reveal binding is enforced locally rather than relying on the stage timeout.
pos: drop the dead std::sample in signature selection and fix the comment -- the loop always used the first N.