Skip to content

dash-spv accepts QRInfo lastCommitmentPerIndex entries without checking them against a coinbase-committed list #1264

Open

Description

@PastaPastaPasta

A QRInfo carries the latest mined rotated commitment for each quorum index in lastCommitmentPerIndex. Dash Core builds it with GetLastMinedCommitmentsPerQuorumIndexUntilBlock at the requested block (src/llmq/snapshot.cpp). For a rotating type, that is also the set of commitments in that block's masternode list: GetMinedAndActiveCommitmentsUntilBlock (src/llmq/blockprocessor.cpp) calls the same function. So these commitments are the ones the block's coinbase merkleRootQuorums covers.

dash-spv takes the entries as served:

  • apply_qr_info turns each last_commitment_per_index entry into a QualifiedQuorumEntry, unless a stored cycle already has it, and attaches its quarter ChainLock signatures (dash-spv/src/sml_engine.rs:940-952).
  • settle_rotated_quorums then checks each entry's signatures against the members selected from the QRInfo's quorum snapshots and quarter ChainLock signatures (sml_engine.rs:964-976, dash-spv/src/sml_engine/validation.rs:57-85).
  • A fully verified set is stored as the cycle (sml_engine.rs:989-999). InstantSend locks are verified against these stored cycles (dash-spv/src/sync/instantsend/manager.rs:160, dash-spv/src/validation/instantlock.rs:177).

Nothing compares the entries with the quorums of a list whose coinbase was verified. Since #1200, every list the engine applies must hash to its block's merkleRootQuorums, and those lists include the rotated quorums. The previous-cycle path already reads its quorums from such a list, the list at the h work block (sml_engine.rs:556-575). The current-cycle path doesn't. As a result, the quorums InstantSend locks are checked against rest only on the signature check. That check selects members from data the same QRInfo supplies: the snapshots and the quorumsCLSigs (see #1262).

Proposed fix: before validating, require each last_commitment_per_index entry to match the commitment of the same type and quorum hash in the list at the requested block. This is the list the QRInfo's diffs build, and its coinbase is verified. Compare by entry hash, which merkleRootQuorums commits to. Refuse the QRInfo when an entry isn't there or differs.

Open design point: getqrinfo is sent with the client's tip as blockRequestHash (dash-spv/src/sync/masternodes/manager.rs:545), while mnListDiffTip ends at the serving peer's tip. If the two differ, the engine may need the list at the requested block in addition to the tip list.

Test: a QRInfo fixture with one lastCommitmentPerIndex entry altered, or replaced by a commitment that isn't in the list at the requested block. It should be refused. On dev today, an altered entry is refused only if its signature check fails, and the refusal doesn't say that the entry isn't on chain.

Activity

  1. added 2 commits that reference this issue on Nov 5, 2026
    0f534b8
    6c4fcf6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions