Skip to content

refactor: derive rotation cycle key from the cycle boundary block#641

Closed
xdustinface wants to merge 1 commit intofix/rotated-quorums-indexfrom
fix/rotated-quorums-key
Closed

refactor: derive rotation cycle key from the cycle boundary block#641
xdustinface wants to merge 1 commit intofix/rotated-quorums-indexfrom
fix/rotated-quorums-key

Conversation

@xdustinface
Copy link
Copy Markdown
Collaborator

@xdustinface xdustinface commented Apr 13, 2026

Previously the per-cycle map was keyed by the quorum hash of whichever entry happened to be first in the incoming commitment list. In practice this produces the cycle boundary block hash because the wire format is ordered by quorum_index and the quorum at index 0 has its DKG start block at the cycle boundary itself, so the old and new keys should match in theory.

Derive the key directly from the work block in mn_list_diff_h (at cycle_boundary - WORK_DIFF_DEPTH) and look up the cycle boundary block hash in the block container. This expresses the intent at the call site, removes the implicit dependency on wire ordering, and fails loudly when the block container does not yet have the cycle boundary block.

Based on:

Summary by CodeRabbit

  • Refactor

    • Improved quorum cycle validation and indexing mechanisms for enhanced system consistency.
  • Tests

    • Expanded test coverage to verify cycle boundary validation processes.

Previously the per-cycle map was keyed by the quorum hash of whichever entry happened to be first in the incoming commitment list. In practice this produces the cycle boundary block hash because the wire format is ordered by `quorum_index` and the quorum at index 0 has its DKG start block at the cycle boundary itself, so the old and new keys should match in theory.

Derive the key directly from the work block in `mn_list_diff_h` (at `cycle_boundary - WORK_DIFF_DEPTH`) and look up the cycle boundary block hash in the block container. This expresses the intent at the call site, removes the implicit dependency on wire ordering, and fails loudly when the block container does not yet have the cycle boundary block.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f043a131-6710-41ea-904b-2527f5fde715

📥 Commits

Reviewing files that changed from the base of the PR and between aa3ec78 and 3d86fd7.

📒 Files selected for processing (1)
  • dash/src/sml/masternode_list_engine/mod.rs

📝 Walkthrough

Walkthrough

Updated rotated-cycle indexing to use cycle_boundary_hash (computed from work block height plus WORK_DIFF_DEPTH) as the key for storing quorum maps, replacing the previous quorum-hash-derived approach. Simplified the validation fallback branch and updated tests to verify the new indexing scheme.

Changes

Cohort / File(s) Summary
Rotated-Cycle Indexing Refactor
dash/src/sml/masternode_list_engine/mod.rs
Changed quorum map storage key from quorum-derived value to cycle_boundary_hash (computed via block container lookup at h_height + WORK_DIFF_DEPTH). Updated quorum_validation to compute and error-check the boundary hash, simplified fallback logic, and updated test assertions to verify correct key usage.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 Through cycles we hop, with hashes so bright,
No more quorum keys—the boundary's right!
At work depth we leap, the correct block we find,
Indexed and stored with precision in mind,
Our rotating quorums now perfectly aligned! 🌟

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: refactoring to derive the rotation cycle key from the cycle boundary block instead of from a quorum hash.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/rotated-quorums-key

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@xdustinface
Copy link
Copy Markdown
Collaborator Author

@CodeRabbit review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 13, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 13, 2026

Codecov Report

❌ Patch coverage is 88.88889% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.88%. Comparing base (aa3ec78) to head (3d86fd7).

Files with missing lines Patch % Lines
dash/src/sml/masternode_list_engine/mod.rs 88.88% 3 Missing ⚠️
Additional details and impacted files
@@                      Coverage Diff                      @@
##           fix/rotated-quorums-index     #641      +/-   ##
=============================================================
- Coverage                      67.90%   67.88%   -0.03%     
=============================================================
  Files                            318      318              
  Lines                          67832    67846      +14     
=============================================================
- Hits                           46063    46056       -7     
- Misses                         21769    21790      +21     
Flag Coverage Δ
core 75.29% <88.88%> (+0.02%) ⬆️
ffi 38.45% <ø> (-0.18%) ⬇️
rpc 20.00% <ø> (ø)
spv 85.50% <ø> (-0.05%) ⬇️
wallet 67.56% <ø> (ø)
Files with missing lines Coverage Δ
dash/src/sml/masternode_list_engine/mod.rs 75.55% <88.88%> (+0.65%) ⬆️

... and 3 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant