Skip to content

Commit 7300318

Browse files
committed
fix(dash): classify missing infrastructure errors as Skipped
Several `QuorumValidationError` variants represent missing caller-supplied infrastructure (unknown block, unloaded masternode list, missing snapshot, chain lock, rotation sig) rather than bad quorum data, but the call sites mapped them to `LLMQEntryVerificationStatus::Invalid`, pushing callers to reject quorums they should refetch. Add a `From<QuorumValidationError> for LLMQEntryVerificationStatus` impl as the single canonical classifier, route every construction site through `.into()`, and add the missing `Skipped` sub-variants needed to preserve context. Also guard the bulk-overwrite path so structurally-broken quorums keep `Invalid` when an infrastructure error fires.
1 parent fcaf66e commit 7300318

3 files changed

Lines changed: 348 additions & 45 deletions

File tree

dash/src/sml/llmq_entry_verification.rs

Lines changed: 183 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ use core::fmt::{Display, Formatter};
33
#[cfg(feature = "bincode")]
44
use bincode::{Decode, Encode};
55

6-
use crate::BlockHash;
76
use crate::prelude::CoreBlockHeight;
87
use crate::sml::quorum_validation_error::QuorumValidationError;
8+
use crate::{BlockHash, QuorumHash};
99

1010
#[derive(Clone, Ord, PartialOrd, PartialEq, Eq, Hash, Debug)]
1111
#[cfg_attr(feature = "bincode", derive(Encode, Decode))]
@@ -14,6 +14,29 @@ pub enum LLMQEntryVerificationSkipStatus {
1414
NotMarkedForVerification,
1515
MissedList(CoreBlockHeight),
1616
UnknownBlock(BlockHash),
17+
/// The snapshot required to validate this quorum entry was not provided
18+
/// by the caller. Distinct from `UnknownBlock` so retry/back-off logic
19+
/// can target snapshot fetches separately from block fetches.
20+
MissingSnapshot(BlockHash),
21+
/// The chain lock at the given height/block hash was not provided by the
22+
/// caller. The block itself may be known; the chain-lock signature for
23+
/// it just hasn't been fetched yet. Distinct from `UnknownBlock` so
24+
/// retry logic can dispatch to a chain-lock fetch instead of a block
25+
/// fetch.
26+
MissingChainLock(CoreBlockHeight, BlockHash),
27+
/// The quorum entry came through without an attached
28+
/// `VerifyingChainLockSignaturesType::Rotating`. Typically happens when
29+
/// a QRInfo's historical diff covers a block range in which no rotating
30+
/// DKG successfully committed, so `apply_diff` extracts no
31+
/// `rotation_sig` and `feed_qr_info` can't populate the 4-sig tuple for
32+
/// the quorums in `lastCommitmentPerIndex`.
33+
MissingRotationChainLockSigs(QuorumHash),
34+
/// A specific rotation chain-lock signature at offset `h - n` was not
35+
/// present for the masternode diff at the given block hash. The first
36+
/// field is the rotation offset, the second is the diff block hash.
37+
/// Distinct from `MissingRotationChainLockSigs`, which covers the case
38+
/// where the entire 4-sig tuple is absent.
39+
MissingRotationChainLockSig(u8, BlockHash),
1740
OtherContext(String),
1841
}
1942

@@ -30,6 +53,21 @@ impl Display for LLMQEntryVerificationSkipStatus {
3053
LLMQEntryVerificationSkipStatus::UnknownBlock(block_hash) => {
3154
format!("UnknownBlock({})", block_hash)
3255
}
56+
LLMQEntryVerificationSkipStatus::MissingSnapshot(block_hash) => {
57+
format!("MissingSnapshot({})", block_hash)
58+
}
59+
LLMQEntryVerificationSkipStatus::MissingChainLock(height, block_hash) => {
60+
format!("MissingChainLock({}, {})", height, block_hash)
61+
}
62+
LLMQEntryVerificationSkipStatus::MissingRotationChainLockSigs(quorum_hash) => {
63+
format!("MissingRotationChainLockSigs({})", quorum_hash)
64+
}
65+
LLMQEntryVerificationSkipStatus::MissingRotationChainLockSig(
66+
offset,
67+
block_hash,
68+
) => {
69+
format!("MissingRotationChainLockSig(h - {}, {})", offset, block_hash)
70+
}
3371
LLMQEntryVerificationSkipStatus::OtherContext(message) => {
3472
format!("OtherContext({message})")
3573
}
@@ -49,6 +87,46 @@ pub enum LLMQEntryVerificationStatus {
4987
Skipped(LLMQEntryVerificationSkipStatus),
5088
Invalid(QuorumValidationError),
5189
}
90+
impl From<QuorumValidationError> for LLMQEntryVerificationStatus {
91+
/// Classify a validation error as either `Skipped` (missing infrastructure
92+
/// data that the caller should have provided) or `Invalid` (the quorum
93+
/// data itself is genuinely bad).
94+
fn from(error: QuorumValidationError) -> Self {
95+
match error {
96+
QuorumValidationError::RequiredBlockNotPresent(block_hash, _) => {
97+
Self::Skipped(LLMQEntryVerificationSkipStatus::UnknownBlock(block_hash))
98+
}
99+
// `VerifyingMasternodeListNotPresent` is grouped here because the
100+
// verifying masternode list at the validation height is caller-
101+
// supplied infrastructure, not quorum data. Treating it as
102+
// `Skipped` mirrors the sibling `RequiredMasternodeListNotPresent`
103+
// case and lets the caller refetch instead of rejecting the quorum.
104+
QuorumValidationError::RequiredMasternodeListNotPresent(height)
105+
| QuorumValidationError::RequiredBlockHeightNotPresent(height)
106+
| QuorumValidationError::VerifyingMasternodeListNotPresent(height) => {
107+
Self::Skipped(LLMQEntryVerificationSkipStatus::MissedList(height))
108+
}
109+
QuorumValidationError::RequiredSnapshotNotPresent(hash) => {
110+
Self::Skipped(LLMQEntryVerificationSkipStatus::MissingSnapshot(hash))
111+
}
112+
QuorumValidationError::RequiredChainLockNotPresent(height, block_hash) => {
113+
Self::Skipped(LLMQEntryVerificationSkipStatus::MissingChainLock(height, block_hash))
114+
}
115+
QuorumValidationError::RequiredRotatedChainLockSigsNotPresent(quorum_hash) => {
116+
Self::Skipped(LLMQEntryVerificationSkipStatus::MissingRotationChainLockSigs(
117+
quorum_hash,
118+
))
119+
}
120+
QuorumValidationError::RequiredRotatedChainLockSigNotPresent(offset, block_hash) => {
121+
Self::Skipped(LLMQEntryVerificationSkipStatus::MissingRotationChainLockSig(
122+
offset, block_hash,
123+
))
124+
}
125+
other => Self::Invalid(other),
126+
}
127+
}
128+
}
129+
52130
impl Display for LLMQEntryVerificationStatus {
53131
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
54132
f.write_str(
@@ -62,3 +140,107 @@ impl Display for LLMQEntryVerificationStatus {
62140
)
63141
}
64142
}
143+
144+
#[cfg(test)]
145+
mod tests {
146+
use hashes::Hash;
147+
148+
use super::*;
149+
150+
fn dummy_hash(byte: u8) -> BlockHash {
151+
BlockHash::from_byte_array([byte; 32])
152+
}
153+
154+
#[test]
155+
fn from_quorum_validation_error_classifies_each_arm() {
156+
let h1 = dummy_hash(1);
157+
let h2 = dummy_hash(2);
158+
let h3 = dummy_hash(3);
159+
let h4 = dummy_hash(4);
160+
let h5 = dummy_hash(5);
161+
162+
let cases: Vec<(QuorumValidationError, LLMQEntryVerificationStatus)> = vec![
163+
(
164+
QuorumValidationError::RequiredBlockNotPresent(h1, "ctx".to_string()),
165+
LLMQEntryVerificationStatus::Skipped(
166+
LLMQEntryVerificationSkipStatus::UnknownBlock(h1),
167+
),
168+
),
169+
(
170+
QuorumValidationError::RequiredMasternodeListNotPresent(42),
171+
LLMQEntryVerificationStatus::Skipped(LLMQEntryVerificationSkipStatus::MissedList(
172+
42,
173+
)),
174+
),
175+
(
176+
QuorumValidationError::RequiredBlockHeightNotPresent(99),
177+
LLMQEntryVerificationStatus::Skipped(LLMQEntryVerificationSkipStatus::MissedList(
178+
99,
179+
)),
180+
),
181+
(
182+
QuorumValidationError::VerifyingMasternodeListNotPresent(123),
183+
LLMQEntryVerificationStatus::Skipped(LLMQEntryVerificationSkipStatus::MissedList(
184+
123,
185+
)),
186+
),
187+
(
188+
QuorumValidationError::RequiredSnapshotNotPresent(h2),
189+
LLMQEntryVerificationStatus::Skipped(
190+
LLMQEntryVerificationSkipStatus::MissingSnapshot(h2),
191+
),
192+
),
193+
(
194+
QuorumValidationError::RequiredChainLockNotPresent(7, h5),
195+
LLMQEntryVerificationStatus::Skipped(
196+
LLMQEntryVerificationSkipStatus::MissingChainLock(7, h5),
197+
),
198+
),
199+
(
200+
QuorumValidationError::RequiredRotatedChainLockSigsNotPresent(h3),
201+
LLMQEntryVerificationStatus::Skipped(
202+
LLMQEntryVerificationSkipStatus::MissingRotationChainLockSigs(h3),
203+
),
204+
),
205+
(
206+
QuorumValidationError::RequiredRotatedChainLockSigNotPresent(2, h4),
207+
LLMQEntryVerificationStatus::Skipped(
208+
LLMQEntryVerificationSkipStatus::MissingRotationChainLockSig(2, h4),
209+
),
210+
),
211+
(
212+
QuorumValidationError::InvalidQuorumPublicKey,
213+
LLMQEntryVerificationStatus::Invalid(QuorumValidationError::InvalidQuorumPublicKey),
214+
),
215+
];
216+
217+
for (error, expected) in cases {
218+
let actual: LLMQEntryVerificationStatus = error.clone().into();
219+
assert_eq!(actual, expected, "case: {error:?}");
220+
}
221+
}
222+
223+
#[test]
224+
fn skip_status_display_formats_new_variants() {
225+
let h = dummy_hash(1);
226+
let cases: Vec<(LLMQEntryVerificationSkipStatus, String)> = vec![
227+
(LLMQEntryVerificationSkipStatus::MissingSnapshot(h), format!("MissingSnapshot({h})")),
228+
(
229+
LLMQEntryVerificationSkipStatus::MissingChainLock(42, h),
230+
format!("MissingChainLock(42, {h})"),
231+
),
232+
(
233+
LLMQEntryVerificationSkipStatus::MissingRotationChainLockSigs(h),
234+
format!("MissingRotationChainLockSigs({h})"),
235+
),
236+
(
237+
LLMQEntryVerificationSkipStatus::MissingRotationChainLockSig(2, h),
238+
format!("MissingRotationChainLockSig(h - 2, {h})"),
239+
),
240+
];
241+
242+
for (status, expected) in cases {
243+
assert_eq!(status.to_string(), expected, "case: {status:?}");
244+
}
245+
}
246+
}

dash/src/sml/masternode_list_engine/validation.rs

Lines changed: 111 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -89,31 +89,31 @@ impl MasternodeListEngine {
8989
// first let's do basic structure validation
9090
for quorum in quorums {
9191
if let Err(e) = quorum.quorum_entry.validate_structure() {
92-
return_statuses.insert(
93-
quorum.quorum_entry.quorum_hash,
94-
LLMQEntryVerificationStatus::Invalid(e),
95-
);
92+
return_statuses.insert(quorum.quorum_entry.quorum_hash, e.into());
9693
} else if !quorum.quorum_entry.llmq_type.is_rotating_quorum_type() {
9794
return_statuses.insert(
9895
quorum.quorum_entry.quorum_hash,
99-
LLMQEntryVerificationStatus::Invalid(
100-
QuorumValidationError::ExpectedOnlyRotatedQuorums(
101-
quorum.quorum_entry.quorum_hash,
102-
quorum.quorum_entry.llmq_type,
103-
),
104-
),
96+
QuorumValidationError::ExpectedOnlyRotatedQuorums(
97+
quorum.quorum_entry.quorum_hash,
98+
quorum.quorum_entry.llmq_type,
99+
)
100+
.into(),
105101
);
106102
}
107103
}
108104

109105
let masternodes_by_quorum_hash = match self.find_rotated_masternodes_for_quorums(quorums) {
110106
Ok(masternodes_by_quorum_hash) => masternodes_by_quorum_hash,
111107
Err(e) => {
108+
let status: LLMQEntryVerificationStatus = e.into();
112109
for quorum in quorums {
113-
return_statuses.insert(
114-
quorum.quorum_entry.quorum_hash,
115-
LLMQEntryVerificationStatus::Invalid(e.clone()),
116-
);
110+
if matches!(
111+
return_statuses.get(&quorum.quorum_entry.quorum_hash),
112+
Some(LLMQEntryVerificationStatus::Invalid(_))
113+
) {
114+
continue;
115+
}
116+
return_statuses.insert(quorum.quorum_entry.quorum_hash, status.clone());
117117
}
118118
return return_statuses;
119119
}
@@ -156,13 +156,106 @@ impl MasternodeListEngine {
156156
);
157157
}
158158
Err(e) => {
159-
return_statuses.insert(
160-
quorum.quorum_entry.quorum_hash,
161-
LLMQEntryVerificationStatus::Invalid(e),
162-
);
159+
return_statuses.insert(quorum.quorum_entry.quorum_hash, e.into());
163160
}
164161
}
165162
}
166163
return_statuses
167164
}
168165
}
166+
167+
#[cfg(all(test, feature = "quorum_validation"))]
168+
mod tests {
169+
use hashes::Hash;
170+
171+
use super::*;
172+
use crate::bls_sig_utils::{BLSPublicKey, BLSSignature};
173+
use crate::hash_types::QuorumVVecHash;
174+
use crate::sml::llmq_entry_verification::LLMQEntryVerificationStatus;
175+
use crate::sml::llmq_type::LLMQType;
176+
use crate::sml::quorum_entry::qualified_quorum_entry::QualifiedQuorumEntry;
177+
use crate::transaction::special_transaction::quorum_commitment::QuorumEntry;
178+
179+
fn rotating_quorum(
180+
quorum_hash: QuorumHash,
181+
quorum_index: i16,
182+
valid_structure: bool,
183+
) -> QualifiedQuorumEntry {
184+
let (signers, valid_members, public_key, threshold_sig, agg_sig) = if valid_structure {
185+
(
186+
vec![true; 4],
187+
vec![true; 4],
188+
BLSPublicKey::from([1; 48]),
189+
BLSSignature::from([1; 96]),
190+
BLSSignature::from([1; 96]),
191+
)
192+
} else {
193+
(
194+
vec![false; 4],
195+
vec![false; 4],
196+
BLSPublicKey::from([0; 48]),
197+
BLSSignature::from([0; 96]),
198+
BLSSignature::from([0; 96]),
199+
)
200+
};
201+
QuorumEntry {
202+
version: 2,
203+
llmq_type: LLMQType::LlmqtypeTestDIP0024,
204+
quorum_hash,
205+
quorum_index: Some(quorum_index),
206+
signers,
207+
valid_members,
208+
quorum_public_key: public_key,
209+
quorum_vvec_hash: QuorumVVecHash::all_zeros(),
210+
threshold_sig,
211+
all_commitment_aggregated_signature: agg_sig,
212+
}
213+
.into()
214+
}
215+
216+
#[test]
217+
fn rotation_cycle_statuses_classify_infra_error_as_skipped_and_preserve_invalid() {
218+
let engine = MasternodeListEngine::default();
219+
220+
let broken_hash = QuorumHash::from_byte_array([1; 32]);
221+
let unknown_hash = QuorumHash::from_byte_array([2; 32]);
222+
let broken = rotating_quorum(broken_hash, 0, false);
223+
let unknown_block = rotating_quorum(unknown_hash, 1, true);
224+
225+
let statuses =
226+
engine.validate_rotation_cycle_quorums_validation_statuses(&[&broken, &unknown_block]);
227+
228+
assert!(
229+
matches!(statuses.get(&broken_hash), Some(LLMQEntryVerificationStatus::Invalid(_))),
230+
"structurally-broken quorum must keep an Invalid status, got {:?}",
231+
statuses.get(&broken_hash),
232+
);
233+
assert!(
234+
matches!(statuses.get(&unknown_hash), Some(LLMQEntryVerificationStatus::Skipped(_))),
235+
"infrastructure-error quorum must surface as Skipped, got {:?}",
236+
statuses.get(&unknown_hash),
237+
);
238+
}
239+
240+
#[test]
241+
fn rotation_cycle_statuses_classify_all_quorums_as_skipped_when_no_pre_existing_invalid() {
242+
let engine = MasternodeListEngine::default();
243+
244+
let hash_a = QuorumHash::from_byte_array([3; 32]);
245+
let hash_b = QuorumHash::from_byte_array([4; 32]);
246+
let quorum_a = rotating_quorum(hash_a, 0, true);
247+
let quorum_b = rotating_quorum(hash_b, 1, true);
248+
249+
let statuses =
250+
engine.validate_rotation_cycle_quorums_validation_statuses(&[&quorum_a, &quorum_b]);
251+
252+
for hash in [hash_a, hash_b] {
253+
assert!(
254+
matches!(statuses.get(&hash), Some(LLMQEntryVerificationStatus::Skipped(_))),
255+
"every quorum must be Skipped when find_rotated_masternodes_for_quorums errors and no entry was pre-marked Invalid, got {:?} for {:?}",
256+
statuses.get(&hash),
257+
hash,
258+
);
259+
}
260+
}
261+
}

0 commit comments

Comments
 (0)