fix: store historical rotating quorums by cycle boundary hash

[xdustinface]

This fixes issues in InstantSend lock verification where we miss the selected quorums. It adds infrastructure for storing historical rotating quorums keyed by cycle boundary hash instead of quorum hash. This enables InstantLock verification for previous cycles by:

• Adding WORK_DIFF_DEPTH constant (8 blocks offset to cycle boundary)
• Adding store_rotating_quorums_for_cycle() to extract and store quorums
• Collecting work block hashes from QRInfo diffs (h-4c, h-3c, h-2c, h-c)
• Using cycle boundary hash as the key for rotated_quorums_per_cycle

[coderabbitai]

Warning

Rate limit exceeded

@PastaPastaPasta has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 15 minutes and 19 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/store-historical-cycles

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[PastaPastaPasta]

@coderabbitai review

Ensure you take into consideration this DIP https://github.com/dashpay/dips/blob/master/dip-0022.md and it's prior work, to understand system design.

[coderabbitai]

🧩 Analysis chain

@PastaPastaPasta, I'll review the PR with DIP-0022 in mind. Let me first fetch the DIP specification and examine the changes.

---

🏁 Script executed:

#!/bin/bash

# Fetch DIP-0022 content
echo "=== Fetching DIP-0022 ==="
curl -s https://raw.githubusercontent.com/dashpay/dips/master/dip-0022.md

echo -e "\n\n=== Fetching DIP-0006 (Long-Living Masternode Quorums) ==="
curl -s https://raw.githubusercontent.com/dashpay/dips/master/dip-0006.md

echo -e "\n\n=== Checking for DIP-0024 (mentioned in learnings) ==="
curl -s https://raw.githubusercontent.com/dashpay/dips/master/dip-0024.md

Length of output: 84079

---

🏁 Script executed:

#!/bin/bash

# Get the list of changed files in the PR
echo "=== Changed files in PR ==="
gh pr view 409 --json files --jq '.files[].path'

echo -e "\n\n=== PR diff ==="
gh pr diff 409

Length of output: 8821

---

🏁 Script executed:

#!/bin/bash

# Find relevant files related to rotating quorums and InstantSend
echo "=== Finding rotating quorum related files ==="
fd -e rs | rg -i "quorum|instant"

Length of output: 812

---

@PastaPastaPasta, I'll review this PR with DIP-0022 and DIP-0024 specifications in mind. Let me examine the implementation details.

---

🏁 Script executed:

#!/bin/bash

# Examine the rotated_quorums_per_cycle usage and structure
echo "=== Examining rotated_quorums_per_cycle structure ==="
rg -n -A5 -B5 "rotated_quorums_per_cycle" dash/src/sml/masternode_list_engine/mod.rs | head -100

echo -e "\n\n=== Check InstantLock verification code ==="
cat dash-spv/src/validation/instantlock.rs

Length of output: 12201

---

🏁 Script executed:

#!/bin/bash

# Look at QRInfo structure to understand the diff components
echo "=== Finding QRInfo structure definition ==="
rg -n "struct.*QRInfo" --type rust -A20

echo -e "\n\n=== Check cycle boundary and work block usage ==="
rg -n "cycle.*boundary|WORK_DIFF_DEPTH" --type rust -C3

Length of output: 10580

---

🏁 Script executed:

#!/bin/bash

# Examine how qualified quorum entries are used
echo "=== Check QualifiedQuorumEntry structure ==="
rg -n "struct.*QualifiedQuorumEntry" --type rust -A10

echo -e "\n\n=== Check masternode list quorums structure ==="
rg -n "pub quorums:" --type rust -C5

Length of output: 3144

---

🏁 Script executed:

#!/bin/bash

# Check how verify_is_lock uses the stored quorums
echo "=== Find verify_is_lock implementation ==="
rg -n "fn verify_is_lock" --type rust -A30

echo -e "\n\n=== Check InstantLock structure and cyclehash field ==="
rg -n "struct InstantLock|pub cyclehash" --type rust -C3

Length of output: 5282

---

🏁 Script executed:

#!/bin/bash

# Check how quorums are retrieved by cycle hash for verification
echo "=== Check quorum retrieval by cycle hash ==="
rg -n "rotated_quorums_per_cycle.*get\(|get.*cyclehash|cycle.*hash.*get" --type rust -C5

echo -e "\n\n=== Check if there are any tests for this functionality ==="
rg -n "#\[test\]|#\[cfg\(test\)\]" dash/src/sml/masternode_list_engine/mod.rs -A3 | head -50

Length of output: 4839

---

🏁 Script executed:

#!/bin/bash

# Look for any documentation or comments about the cycle boundary vs work block distinction
echo "=== Check comments about cycle boundary and work block ==="
rg -n "cycle.boundary|work.block|h-[0-9]c" --type rust -C2 dash/src/sml/masternode_list_engine/mod.rs

echo -e "\n\n=== Check the old implementation (what was it keyed by before?) ==="
git diff HEAD~1 dash/src/sml/masternode_list_engine/mod.rs | grep -A5 -B5 "rotated_quorums_per_cycle"

Length of output: 8707

---

@PastaPastaPasta, I've completed my review of PR #409 with DIP-0022 and DIP-0024 in mind. This is an excellent fix that properly implements cycle-based quorum storage for historical InstantLock verification.

✅ Correct Implementation of DIP-0022 & DIP-0024

The PR correctly addresses the requirement that InstantLock messages must be verified using cycleHash rather than quorumHash. Here's what I found:

Key Strengths

1. WORK_DIFF_DEPTH Constant - Correctly implements the 8-block offset between cycle boundary and work block as specified in DIP-0024

2. Cycle Boundary Hash as Key - The critical change from using quorum_entry.quorum_hash to cycle_boundary_hash aligns perfectly with DIP-0022's requirement that "any node can follow the steps required to determine the appropriate quorumHash" using the cycleHash

3. Historical Quorum Storage - Storing quorums from h-c, h-2c, h-3c, and h-4c enables verification of InstantLocks from previous cycles, which is essential for Platform State Transition validation as described in DIP-0022

4. Dual Storage Strategy:

   • Historical cycles: Extracted from masternode lists at work block heights
   • Current cycle: Populated from lastCommitmentPerIndex with validation status

   This distinction is important because lastCommitmentPerIndex contains "the most recent commitment for each quorumIndex" which may include older quorums if DKG sessions failed

Architecture Observations

Cycle Boundary Calculation

let cycle_boundary_height = work_block_height + WORK_DIFF_DEPTH;

This correctly maps the work block (where masternode lists are selected) to the cycle boundary (where the cycle begins), matching DIP-0024's requirement that "the masternode list at this height is used 8 blocks prior to the current height"

Duplicate Prevention

• contains_key() check prevents re-storing historical cycles
• clear() call allows current cycle updates on repeated feed_qr_info() calls
• This prevents accumulation while allowing state updates

Minor Suggestions

1. Enhanced Documentation: Consider adding a comment explaining why historical and current quorums use different storage paths (masternode list extraction vs lastCommitmentPerIndex validation)

2. Test Coverage: Verify tests exist for:

   • InstantLock verification using historical cycle hashes
   • Correct cycle boundary calculation from work blocks
   • Quorum retrieval by cycle hash for different DKG failure scenarios

3. Feature Flag Clarity: The removal of #[cfg(feature = "quorum_validation")] from the NetworkLLMQExt import is correct since isd_llmq_type() is now used in the non-feature-gated store_rotating_quorums_for_cycle() method

Questions

1. Edge Case: What happens if feed_qr_info() is called multiple times for the same height h? The clear() prevents accumulation for the current cycle, but are there any performance implications?

2. Masternode List Availability: The code gracefully handles missing masternode lists with early returns. Are there scenarios where a work block's masternode list might legitimately be absent after applying its diff?

Summary

This PR correctly implements the critical infrastructure needed for deterministic InstantSend lock verification per DIP-0022 and DIP-0024. The shift from quorum hash to cycle hash as the storage key is essential because "any quorum could sign any islock even if it were not responsible to do so for that quorum cycle" under the old approach. The new implementation ensures that "nodes can follow the steps required to determine the appropriate quorumHash and verify the signature" for InstantLocks from any cycle.

---

🧠 Learnings used

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Maintain backward compatibility for serialized wallets and monitor for new DIPs affecting wallet structure when updating derivation paths

---

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T14:54:24.111Z
Learning: This codebase is not intended for consensus-critical validation; do not rely on exact Dash Core consensus behavior

---

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Prepare support for future Dash upgrades including Schnorr/Taproot support, descriptor wallets, multi-signature account types, and Lightning Network payment channels

---

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T14:53:56.708Z
Learning: This library should NOT be used for consensus code and cannot replicate exact consensus behavior of Dash Core

---

Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 56
File: dash/src/sml/masternode_list_engine/message_request_verification.rs:98-99
Timestamp: 2025-02-27T05:44:42.338Z
Learning: In the Dash codebase, quorum selection for InstantLock verification uses a bit-shifting operation with (64 - n - 1) to extract n bits starting from the second-highest bit of a 64-bit selection hash. The exact reasoning for the extra "-1" isn't documented, but it matches the original C++ implementation for DIP-24 quorum selection.

✅ 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.