Repository navigation
feat(platform-wallet): return dash-spv's verification status with an SPV quorum key - #5366
Conversation
…SPV quorum key `SpvRuntime::get_quorum_public_key` returns only the 48-byte key, from any entry dash-spv holds that is not `Invalid`, so a context provider built on it cannot require a verified quorum. The new `get_quorum_public_key_with_status` returns the key with dash-spv's `LLMQEntryVerificationStatus`; the key-only lookup delegates to it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
✅ Final review complete — no blockers (commit 738f833) · triage: low |
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
The additive lookup returns the public key and verification status from the same upstream quorum entry while preserving the existing key-only API. Source inspection confirms the status handling, hash byte order, and test fixture are consistent with the pinned dependency; no actionable in-scope findings remain. Validation was static only: the supplied CI snapshot contains no completed Rust build, test, or clippy checks, so the author's reported results remain independently unconfirmed.
Review provenance
Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
lowbygpt-6.1-sol(effort low) — The diff is a small, additive wrapper that returns an existing quorum verification status alongside its key, preserves the old API's behavior, and adds focused tests without changing verification logic. - Phase 1 reviewers:
glm-5.3-flash— general (completed, effort high); agentphase1-reviewer,glm-5.3-flash— rust-quality (completed, effort high); agentphase1-reviewer - Phase 1 model:
glm-5.3-flash— zai quota: 5h 99% left, weekly 86% left; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 14% left, 5h 100% left) - Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort medium); agentphase2-reviewer
Out-of-scope follow-up suggestions (2)
These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.
- No consumer acts on the new status yet — proof verification still accepts unverified keys — Consumer migration is explicitly outside this additive PR's goal, which is to make the previously inaccessible status available while retaining existing behavior. The lack of a production caller of the new method does not establish an affected proof-verification path or a new vulnerability. The proposed follow-up is broader integration work, not an exceptional concrete issue to carry forward.
- Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.
- quorum_type u32 is silently truncated to u8 and unmapped values become LlmqtypeUnknown — The base implementation already performs the identical LLMQType::from(quorum_type as u8) conversion. Moving that unchanged conversion into the delegated method neither introduces nor worsens it. Parameter validation is routine pre-existing boundary cleanup outside this PR.
- Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.
7ee8f7b
into
dashpay:chore/rust-dashcore-1112-v5.1
…SPV quorum key (#5366) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Issue being fixed or feature implemented
SpvRuntime::get_quorum_public_keylooks the quorum up through dash-spv'sget_quorum_at_heightand returns only its 48-byte public key. That lookup returns any entry that is notInvalid, so the key may come from a quorum dash-spv holds but has not verified (Unknown, orSkipped(..)when it lacked the block, the work-block list or a ChainLock it needed).SpvRuntimekeeps its client private, so aContextProviderbuilt on it cannot tell a verified key from an unverified one, and cannot requireVerifiedbefore it trusts a proof.What was done?
SpvRuntime::get_quorum_public_key_with_statusreturnsSpvQuorumPublicKey { public_key, status }, wherestatusis dash-spv'sLLMQEntryVerificationStatusfor the quorum.get_quorum_public_keyis unchanged for its callers and now delegates to it.SpvQuorumPublicKeyis re-exported from the crate root next toSpvRuntime.The change is additive: no existing signature changes.
Stacked on #5307: the test reaches dash-spv's masternode list engine through
dash_spv::test_utils::masternode_list_engine, which needs #5307's rust-dashcore update. Merge after #5307.How Has This Been Tested?
spv::runtime::tests::should_return_the_quorum_status_with_its_keystarts an offline runtime, places a quorum in the client's masternode list engine (through dash-spv'stest-utilsfeature, now enabled for this crate's dev-dependencies), and checks that the key and status come back for anUnknown, aSkipped(MissedList)and aVerifiedentry, with Platform's reversed hash byte order, that the key-only lookup returns the same key for each, and that neither lookup returns anInvalidentry.cargo test -p platform-wallet --lib spv::runtime: 31 passed, 0 failed.cargo clippy -p platform-wallet --all-targets -- -D warningsis clean.Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code