diff --git a/dash/src/sml/llmq_entry_verification.rs b/dash/src/sml/llmq_entry_verification.rs index 69568bbff..58d85cc3a 100644 --- a/dash/src/sml/llmq_entry_verification.rs +++ b/dash/src/sml/llmq_entry_verification.rs @@ -3,9 +3,9 @@ use core::fmt::{Display, Formatter}; #[cfg(feature = "bincode")] use bincode::{Decode, Encode}; -use crate::BlockHash; use crate::prelude::CoreBlockHeight; use crate::sml::quorum_validation_error::QuorumValidationError; +use crate::{BlockHash, QuorumHash}; #[derive(Clone, Ord, PartialOrd, PartialEq, Eq, Hash, Debug)] #[cfg_attr(feature = "bincode", derive(Encode, Decode))] @@ -14,6 +14,29 @@ pub enum LLMQEntryVerificationSkipStatus { NotMarkedForVerification, MissedList(CoreBlockHeight), UnknownBlock(BlockHash), + /// The snapshot required to validate this quorum entry was not provided + /// by the caller. Distinct from `UnknownBlock` so retry/back-off logic + /// can target snapshot fetches separately from block fetches. + MissingSnapshot(BlockHash), + /// The chain lock at the given height/block hash was not provided by the + /// caller. The block itself may be known; the chain-lock signature for + /// it just hasn't been fetched yet. Distinct from `UnknownBlock` so + /// retry logic can dispatch to a chain-lock fetch instead of a block + /// fetch. + MissingChainLock(CoreBlockHeight, BlockHash), + /// The quorum entry came through without an attached + /// `VerifyingChainLockSignaturesType::Rotating`. Typically happens when + /// a QRInfo's historical diff covers a block range in which no rotating + /// DKG successfully committed, so `apply_diff` extracts no + /// `rotation_sig` and `feed_qr_info` can't populate the 4-sig tuple for + /// the quorums in `lastCommitmentPerIndex`. + MissingRotationChainLockSigs(QuorumHash), + /// A specific rotation chain-lock signature at offset `h - n` was not + /// present for the masternode diff at the given block hash. The first + /// field is the rotation offset, the second is the diff block hash. + /// Distinct from `MissingRotationChainLockSigs`, which covers the case + /// where the entire 4-sig tuple is absent. + MissingRotationChainLockSig(u8, BlockHash), OtherContext(String), } @@ -30,6 +53,21 @@ impl Display for LLMQEntryVerificationSkipStatus { LLMQEntryVerificationSkipStatus::UnknownBlock(block_hash) => { format!("UnknownBlock({})", block_hash) } + LLMQEntryVerificationSkipStatus::MissingSnapshot(block_hash) => { + format!("MissingSnapshot({})", block_hash) + } + LLMQEntryVerificationSkipStatus::MissingChainLock(height, block_hash) => { + format!("MissingChainLock({}, {})", height, block_hash) + } + LLMQEntryVerificationSkipStatus::MissingRotationChainLockSigs(quorum_hash) => { + format!("MissingRotationChainLockSigs({})", quorum_hash) + } + LLMQEntryVerificationSkipStatus::MissingRotationChainLockSig( + offset, + block_hash, + ) => { + format!("MissingRotationChainLockSig(h - {}, {})", offset, block_hash) + } LLMQEntryVerificationSkipStatus::OtherContext(message) => { format!("OtherContext({message})") } @@ -49,6 +87,46 @@ pub enum LLMQEntryVerificationStatus { Skipped(LLMQEntryVerificationSkipStatus), Invalid(QuorumValidationError), } +impl From for LLMQEntryVerificationStatus { + /// Classify a validation error as either `Skipped` (missing infrastructure + /// data that the caller should have provided) or `Invalid` (the quorum + /// data itself is genuinely bad). + fn from(error: QuorumValidationError) -> Self { + match error { + QuorumValidationError::RequiredBlockNotPresent(block_hash, _) => { + Self::Skipped(LLMQEntryVerificationSkipStatus::UnknownBlock(block_hash)) + } + // `VerifyingMasternodeListNotPresent` is grouped here because the + // verifying masternode list at the validation height is caller- + // supplied infrastructure, not quorum data. Treating it as + // `Skipped` mirrors the sibling `RequiredMasternodeListNotPresent` + // case and lets the caller refetch instead of rejecting the quorum. + QuorumValidationError::RequiredMasternodeListNotPresent(height) + | QuorumValidationError::RequiredBlockHeightNotPresent(height) + | QuorumValidationError::VerifyingMasternodeListNotPresent(height) => { + Self::Skipped(LLMQEntryVerificationSkipStatus::MissedList(height)) + } + QuorumValidationError::RequiredSnapshotNotPresent(hash) => { + Self::Skipped(LLMQEntryVerificationSkipStatus::MissingSnapshot(hash)) + } + QuorumValidationError::RequiredChainLockNotPresent(height, block_hash) => { + Self::Skipped(LLMQEntryVerificationSkipStatus::MissingChainLock(height, block_hash)) + } + QuorumValidationError::RequiredRotatedChainLockSigsNotPresent(quorum_hash) => { + Self::Skipped(LLMQEntryVerificationSkipStatus::MissingRotationChainLockSigs( + quorum_hash, + )) + } + QuorumValidationError::RequiredRotatedChainLockSigNotPresent(offset, block_hash) => { + Self::Skipped(LLMQEntryVerificationSkipStatus::MissingRotationChainLockSig( + offset, block_hash, + )) + } + other => Self::Invalid(other), + } + } +} + impl Display for LLMQEntryVerificationStatus { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { f.write_str( @@ -62,3 +140,107 @@ impl Display for LLMQEntryVerificationStatus { ) } } + +#[cfg(test)] +mod tests { + use hashes::Hash; + + use super::*; + + fn dummy_hash(byte: u8) -> BlockHash { + BlockHash::from_byte_array([byte; 32]) + } + + #[test] + fn from_quorum_validation_error_classifies_each_arm() { + let h1 = dummy_hash(1); + let h2 = dummy_hash(2); + let h3 = dummy_hash(3); + let h4 = dummy_hash(4); + let h5 = dummy_hash(5); + + let cases: Vec<(QuorumValidationError, LLMQEntryVerificationStatus)> = vec![ + ( + QuorumValidationError::RequiredBlockNotPresent(h1, "ctx".to_string()), + LLMQEntryVerificationStatus::Skipped( + LLMQEntryVerificationSkipStatus::UnknownBlock(h1), + ), + ), + ( + QuorumValidationError::RequiredMasternodeListNotPresent(42), + LLMQEntryVerificationStatus::Skipped(LLMQEntryVerificationSkipStatus::MissedList( + 42, + )), + ), + ( + QuorumValidationError::RequiredBlockHeightNotPresent(99), + LLMQEntryVerificationStatus::Skipped(LLMQEntryVerificationSkipStatus::MissedList( + 99, + )), + ), + ( + QuorumValidationError::VerifyingMasternodeListNotPresent(123), + LLMQEntryVerificationStatus::Skipped(LLMQEntryVerificationSkipStatus::MissedList( + 123, + )), + ), + ( + QuorumValidationError::RequiredSnapshotNotPresent(h2), + LLMQEntryVerificationStatus::Skipped( + LLMQEntryVerificationSkipStatus::MissingSnapshot(h2), + ), + ), + ( + QuorumValidationError::RequiredChainLockNotPresent(7, h5), + LLMQEntryVerificationStatus::Skipped( + LLMQEntryVerificationSkipStatus::MissingChainLock(7, h5), + ), + ), + ( + QuorumValidationError::RequiredRotatedChainLockSigsNotPresent(h3), + LLMQEntryVerificationStatus::Skipped( + LLMQEntryVerificationSkipStatus::MissingRotationChainLockSigs(h3), + ), + ), + ( + QuorumValidationError::RequiredRotatedChainLockSigNotPresent(2, h4), + LLMQEntryVerificationStatus::Skipped( + LLMQEntryVerificationSkipStatus::MissingRotationChainLockSig(2, h4), + ), + ), + ( + QuorumValidationError::InvalidQuorumPublicKey, + LLMQEntryVerificationStatus::Invalid(QuorumValidationError::InvalidQuorumPublicKey), + ), + ]; + + for (error, expected) in cases { + let actual: LLMQEntryVerificationStatus = error.clone().into(); + assert_eq!(actual, expected, "case: {error:?}"); + } + } + + #[test] + fn skip_status_display_formats_new_variants() { + let h = dummy_hash(1); + let cases: Vec<(LLMQEntryVerificationSkipStatus, String)> = vec![ + (LLMQEntryVerificationSkipStatus::MissingSnapshot(h), format!("MissingSnapshot({h})")), + ( + LLMQEntryVerificationSkipStatus::MissingChainLock(42, h), + format!("MissingChainLock(42, {h})"), + ), + ( + LLMQEntryVerificationSkipStatus::MissingRotationChainLockSigs(h), + format!("MissingRotationChainLockSigs({h})"), + ), + ( + LLMQEntryVerificationSkipStatus::MissingRotationChainLockSig(2, h), + format!("MissingRotationChainLockSig(h - 2, {h})"), + ), + ]; + + for (status, expected) in cases { + assert_eq!(status.to_string(), expected, "case: {status:?}"); + } + } +} diff --git a/dash/src/sml/masternode_list_engine/validation.rs b/dash/src/sml/masternode_list_engine/validation.rs index 011989c7b..bc9316064 100644 --- a/dash/src/sml/masternode_list_engine/validation.rs +++ b/dash/src/sml/masternode_list_engine/validation.rs @@ -89,19 +89,15 @@ impl MasternodeListEngine { // first let's do basic structure validation for quorum in quorums { if let Err(e) = quorum.quorum_entry.validate_structure() { - return_statuses.insert( - quorum.quorum_entry.quorum_hash, - LLMQEntryVerificationStatus::Invalid(e), - ); + return_statuses.insert(quorum.quorum_entry.quorum_hash, e.into()); } else if !quorum.quorum_entry.llmq_type.is_rotating_quorum_type() { return_statuses.insert( quorum.quorum_entry.quorum_hash, - LLMQEntryVerificationStatus::Invalid( - QuorumValidationError::ExpectedOnlyRotatedQuorums( - quorum.quorum_entry.quorum_hash, - quorum.quorum_entry.llmq_type, - ), - ), + QuorumValidationError::ExpectedOnlyRotatedQuorums( + quorum.quorum_entry.quorum_hash, + quorum.quorum_entry.llmq_type, + ) + .into(), ); } } @@ -109,11 +105,15 @@ impl MasternodeListEngine { let masternodes_by_quorum_hash = match self.find_rotated_masternodes_for_quorums(quorums) { Ok(masternodes_by_quorum_hash) => masternodes_by_quorum_hash, Err(e) => { + let status: LLMQEntryVerificationStatus = e.into(); for quorum in quorums { - return_statuses.insert( - quorum.quorum_entry.quorum_hash, - LLMQEntryVerificationStatus::Invalid(e.clone()), - ); + if matches!( + return_statuses.get(&quorum.quorum_entry.quorum_hash), + Some(LLMQEntryVerificationStatus::Invalid(_)) + ) { + continue; + } + return_statuses.insert(quorum.quorum_entry.quorum_hash, status.clone()); } return return_statuses; } @@ -156,13 +156,106 @@ impl MasternodeListEngine { ); } Err(e) => { - return_statuses.insert( - quorum.quorum_entry.quorum_hash, - LLMQEntryVerificationStatus::Invalid(e), - ); + return_statuses.insert(quorum.quorum_entry.quorum_hash, e.into()); } } } return_statuses } } + +#[cfg(all(test, feature = "quorum_validation"))] +mod tests { + use hashes::Hash; + + use super::*; + use crate::bls_sig_utils::{BLSPublicKey, BLSSignature}; + use crate::hash_types::QuorumVVecHash; + use crate::sml::llmq_entry_verification::LLMQEntryVerificationStatus; + use crate::sml::llmq_type::LLMQType; + use crate::sml::quorum_entry::qualified_quorum_entry::QualifiedQuorumEntry; + use crate::transaction::special_transaction::quorum_commitment::QuorumEntry; + + fn rotating_quorum( + quorum_hash: QuorumHash, + quorum_index: i16, + valid_structure: bool, + ) -> QualifiedQuorumEntry { + let (signers, valid_members, public_key, threshold_sig, agg_sig) = if valid_structure { + ( + vec![true; 4], + vec![true; 4], + BLSPublicKey::from([1; 48]), + BLSSignature::from([1; 96]), + BLSSignature::from([1; 96]), + ) + } else { + ( + vec![false; 4], + vec![false; 4], + BLSPublicKey::from([0; 48]), + BLSSignature::from([0; 96]), + BLSSignature::from([0; 96]), + ) + }; + QuorumEntry { + version: 2, + llmq_type: LLMQType::LlmqtypeTestDIP0024, + quorum_hash, + quorum_index: Some(quorum_index), + signers, + valid_members, + quorum_public_key: public_key, + quorum_vvec_hash: QuorumVVecHash::all_zeros(), + threshold_sig, + all_commitment_aggregated_signature: agg_sig, + } + .into() + } + + #[test] + fn rotation_cycle_statuses_classify_infra_error_as_skipped_and_preserve_invalid() { + let engine = MasternodeListEngine::default(); + + let broken_hash = QuorumHash::from_byte_array([1; 32]); + let unknown_hash = QuorumHash::from_byte_array([2; 32]); + let broken = rotating_quorum(broken_hash, 0, false); + let unknown_block = rotating_quorum(unknown_hash, 1, true); + + let statuses = + engine.validate_rotation_cycle_quorums_validation_statuses(&[&broken, &unknown_block]); + + assert!( + matches!(statuses.get(&broken_hash), Some(LLMQEntryVerificationStatus::Invalid(_))), + "structurally-broken quorum must keep an Invalid status, got {:?}", + statuses.get(&broken_hash), + ); + assert!( + matches!(statuses.get(&unknown_hash), Some(LLMQEntryVerificationStatus::Skipped(_))), + "infrastructure-error quorum must surface as Skipped, got {:?}", + statuses.get(&unknown_hash), + ); + } + + #[test] + fn rotation_cycle_statuses_classify_all_quorums_as_skipped_when_no_pre_existing_invalid() { + let engine = MasternodeListEngine::default(); + + let hash_a = QuorumHash::from_byte_array([3; 32]); + let hash_b = QuorumHash::from_byte_array([4; 32]); + let quorum_a = rotating_quorum(hash_a, 0, true); + let quorum_b = rotating_quorum(hash_b, 1, true); + + let statuses = + engine.validate_rotation_cycle_quorums_validation_statuses(&[&quorum_a, &quorum_b]); + + for hash in [hash_a, hash_b] { + assert!( + matches!(statuses.get(&hash), Some(LLMQEntryVerificationStatus::Skipped(_))), + "every quorum must be Skipped when find_rotated_masternodes_for_quorums errors and no entry was pre-marked Invalid, got {:?} for {:?}", + statuses.get(&hash), + hash, + ); + } + } +} diff --git a/dash/src/sml/quorum_entry/qualified_quorum_entry.rs b/dash/src/sml/quorum_entry/qualified_quorum_entry.rs index f9d29250f..acbd9a461 100644 --- a/dash/src/sml/quorum_entry/qualified_quorum_entry.rs +++ b/dash/src/sml/quorum_entry/qualified_quorum_entry.rs @@ -56,33 +56,61 @@ impl From for QualifiedQuorumEntry { impl QualifiedQuorumEntry { /// Updates the verification status of the quorum based on a validation result. /// - /// This method processes the result of a quorum validation and updates the `verified` field accordingly: - /// - If validation succeeds (`Ok(_)`), the status is set to `Verified`. - /// - If validation fails due to a missing block, it is marked as `Skipped` with `UnknownBlock`. - /// - If validation fails due to a missing masternode list, it is marked as `Skipped` with `MissedList`. - /// - Other errors result in the quorum being marked as `Invalid`. - /// - /// # Arguments - /// - /// * `result` - A `Result` containing either success (`Ok`) or a `QuorumValidationError`. + /// On `Ok`, sets `verified` to `Verified`. On `Err`, classifies the error + /// via `From for LLMQEntryVerificationStatus`, + /// which decides whether the failure is `Skipped` (missing infrastructure + /// the caller should have provided) or `Invalid` (genuinely bad quorum + /// data). pub fn update_quorum_status(&mut self, result: Result<(), QuorumValidationError>) { - match result { - Err(QuorumValidationError::RequiredBlockNotPresent(block_hash, _)) => { - self.verified = LLMQEntryVerificationStatus::Skipped( - LLMQEntryVerificationSkipStatus::UnknownBlock(block_hash), - ); - } - Err(QuorumValidationError::RequiredMasternodeListNotPresent(block_height)) => { - self.verified = LLMQEntryVerificationStatus::Skipped( - LLMQEntryVerificationSkipStatus::MissedList(block_height), - ); - } - Err(e) => { - self.verified = LLMQEntryVerificationStatus::Invalid(e); - } - Ok(_) => { - self.verified = LLMQEntryVerificationStatus::Verified; - } + self.verified = match result { + Ok(_) => LLMQEntryVerificationStatus::Verified, + Err(e) => e.into(), + }; + } +} + +#[cfg(test)] +mod tests { + use hashes::Hash; + + use super::*; + use crate::QuorumHash; + use crate::bls_sig_utils::{BLSPublicKey, BLSSignature}; + use crate::hash_types::QuorumVVecHash; + use crate::sml::llmq_type::LLMQType; + + fn dummy_qualified_quorum_entry() -> QualifiedQuorumEntry { + QuorumEntry { + version: 2, + llmq_type: LLMQType::LlmqtypeTestDIP0024, + quorum_hash: QuorumHash::all_zeros(), + quorum_index: Some(0), + signers: vec![true; 4], + valid_members: vec![true; 4], + quorum_public_key: BLSPublicKey::from([1; 48]), + quorum_vvec_hash: QuorumVVecHash::all_zeros(), + threshold_sig: BLSSignature::from([1; 96]), + all_commitment_aggregated_signature: BLSSignature::from([1; 96]), } + .into() + } + + #[test] + fn update_quorum_status_delegates_to_classifier() { + let mut entry = dummy_qualified_quorum_entry(); + entry.update_quorum_status(Ok(())); + assert_eq!(entry.verified, LLMQEntryVerificationStatus::Verified); + + let snapshot_hash = QuorumHash::from_byte_array([7; 32]); + let mut entry = dummy_qualified_quorum_entry(); + entry.update_quorum_status(Err(QuorumValidationError::RequiredSnapshotNotPresent( + snapshot_hash, + ))); + assert_eq!( + entry.verified, + LLMQEntryVerificationStatus::Skipped(LLMQEntryVerificationSkipStatus::MissingSnapshot( + snapshot_hash, + )), + ); } }