fix: masternode list around height

[PastaPastaPasta]

Summary

Align quorum lookup semantics to use the masternode list at or before the requested height.
Apply the same rule in both Rust API (dash-spv) and FFI (dash-spv-ffi) so platform integration behaves consistently.

Why

Masternode lists are stored at diff heights (sparse), so exact-height lookup will likely fail after sync. For “active at height” logic, the list at or before the requested height is authoritative, and must be used. It was not previously.

What changed

queries.rs: get_quorum_at_height now uses the nearest list at or below height only (no fallback to newer list).
platform_integration.rs: ffi_dash_spv_get_quorum_public_key uses the same “at or before” rule.

Behavioral notes

If there is no masternode list at/before the requested height, return a clear error.
If the list exists but the quorum is missing, return a quorum-not-found error tied to that list height.

This should not be a breaking change, makes usage more flexible.

Testing

cargo fmt
cargo check

I intend to test this in DET soon.

Summary by CodeRabbit

• Bug Fixes
  • Quorum lookup now uses nearby masternode lists instead of a stale global index, improving accuracy.
  • Added clearer early-error handling when required list or quorum data is missing.
  • Error messages are more contextual (list height, requested height, quorum counts) to aid troubleshooting.

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactors quorum lookup to use masternode lists returned by engine.masternode_lists_around_height, derive an effective list_height from ml.known_height, search that list's quorums for the requested llmq_type/quorum_hash, and return contextual errors when lists or quorums are missing.

Changes

Cohort / File(s)	Summary
FFI Quorum Lookup dash-spv-ffi/src/platform_integration.rs	Replaced global quorum_statuses lookup with engine.masternode_lists_around_height. Use ml.known_height as list_height, search ml.quorums for llmq_type/quorum_hash, copy 48-byte quorum_public_key on match, and return contextual ValidationError messages when missing.
Client Query Refactor dash-spv/src/client/queries.rs	Rewrote get_quorum_at_height to query around-height masternode lists, use found list's known_height as effective list height, return quorum on match, and construct detailed QuorumLookupError warnings when lists or quorums are absent.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant Engine as MasternodeListEngine
    participant ML as MasternodeList
    participant FFI as FFI / Client Logic
    participant Output as ResultBuffer

    Caller->>Engine: masternode_lists_around_height(requested_height)
    Engine-->>Caller: ml or None

    alt ml found
        Caller->>FFI: ml (with known_height)
        FFI->>FFI: list_height = ml.known_height
        FFI->>FFI: lookup ml.quorums for (llmq_type, quorum_hash)
        alt quorum found
            FFI->>Output: write 48-byte quorum_public_key
            Output-->>Caller: success
        else quorum not found
            FFI-->>Caller: ValidationError (list_height, requested_height, quorum_hash, quorum_count)
        end
    else ml not found
        FFI-->>Caller: QuorumLookupError (no list at/before requested_height)
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hopped through lists by known-height light,
Found quorums hiding just out of sight,
I copied keys with careful paw,
And shouted errors that showed what I saw —
A tiny hop, a tidy cryptic bite.

🚥 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 'fix: masternode list around height' is directly related to the main change—both files now use masternode_lists_around_height instead of global quorum status indices, which is the core modification.
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

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch codex/masternode-list-around-height

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@dash-spv/src/client/queries.rs`:
- Around line 117-122: The error string passed to SpvError::QuorumLookupError is
inconsistent with the tracing::warn message semantics; update the
QuorumLookupError message (the Err(...) returned where
SpvError::QuorumLookupError is constructed) to say "No masternode list found at
or before height {height}" (or otherwise match the tracing::warn text) so both
the log and the returned error consistently reference "at or before height" and
include the height variable.

🧹 Nitpick comments (2)

dash-spv/src/client/queries.rs (1)

90-101: Minor: Consider consistent hex encoding approach.

The error message construction and logging are informative. One minor note: line 96 uses hex::encode(quorum_hash) which produces lowercase hex. This is fine, but note that the FFI counterpart uses {:x} format specifier. Keeping consistent formatting across both modules would aid debugging when comparing error messages.

dash-spv-ffi/src/platform_integration.rs (1)

139-158: Consider extracting shared quorum lookup logic.

The quorum lookup and error handling logic here closely mirrors get_quorum_at_height in queries.rs. While some duplication in FFI code is acceptable for explicitness, the error message formatting differs slightly ({:x} here vs hex::encode in queries.rs, line 146 vs queries.rs line 96).

If these modules evolve independently, maintaining consistent error messages may become difficult. Consider whether a shared helper for error message formatting would be beneficial.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

dash-spv/src/client/queries.rs (1)

63-64: ⚠️ Potential issue | 🟡 Minor

Documentation inconsistency with function signature.

The doc comment states "Returns None if the quorum is not found" but the function actually returns Result<QualifiedQuorumEntry>, returning an Err variant on failure.

📝 Proposed fix

 /// Get a quorum entry by type and hash at a specific block height.
-/// Returns None if the quorum is not found.
+/// Returns an error if the quorum is not found or no masternode list exists at or before the height.
 pub fn get_quorum_at_height(

🧹 Nitpick comments (1)

dash-spv/src/client/queries.rs (1)

99-99: Consider using format string for tracing macro.

Passing a String directly to tracing::warn! works but using a format string is more idiomatic and avoids potential clippy warnings about format string security.

♻️ Suggested change

-                        tracing::warn!(message);
+                        tracing::warn!("{}", message);

[github-actions]

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.

[github-actions]

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.