You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
dash-spv accepts QRInfo lastCommitmentPerIndex entries without checking them against a coinbase-committed list #1264
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
fix(dash-spv)!: refuse QRInfo active-set entries that are not in the …
fix(dash-spv)!: refuse `QRInfo` entries missing from the tip list