From e4acaa3f4c8692c74e1ab3965b3a28fbf0d1ee3c Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Mon, 28 Sep 2026 11:03:53 +0700 Subject: [PATCH 1/2] feat(dash-spv)!: check masternode list diffs against the block coinbase A masternode list diff is now checked against the coinbase commitments of its block before it is applied, as Dash Core does when it validates the block: the coinbase merkle proof has to lead to the merkle root of the stored block header and prove the coinbase as the block's first transaction, and the list the diff builds has to match `merkleRootMNList` and, from coinbase payload version 2 on, `merkleRootQuorums`. - dash: `MnListDiff::verify_coinbase_merkle_proof` checks the partial merkle tree. `MasternodeList::verify_coinbase_merkle_roots` checks the roots and runs inside `MasternodeList::apply_diff` and the full-diff conversion, so every diff the engine applies passes it, including every diff of a QRInfo. A list built from a diff stores the roots its coinbase commits to, computed once for the check and the list. - dash-spv: the mnlistdiff and qrinfo handlers and the start-up replay check each diff's proof against the stored header of its block. A diff that fails is handled like one the engine refuses. - tests: all 59 committed fixture diffs match their coinbase, fixture proofs hold against the real block headers, and edited entries, quorums, proofs and coinbases are refused. Five older fixture captures hold service addresses or platform node ids in a form Core does not send; test code restores those fields at load time. Co-Authored-By: Claude Opus 5.5 --- dash-spv/src/storage/masternode.rs | 268 ++++++++--- dash-spv/src/storage/mod.rs | 4 +- dash-spv/src/sync/masternodes/sync_manager.rs | 136 +++++- dash-spv/src/test_utils/header_storage.rs | 52 ++- dash/src/merkle_tree/block.rs | 29 +- dash/src/network/message_sml.rs | 234 +++++++++- dash/src/sml/error.rs | 49 +- dash/src/sml/masternode_list/apply_diff.rs | 49 +- dash/src/sml/masternode_list/builder.rs | 25 +- dash/src/sml/masternode_list/from_diff.rs | 23 +- dash/src/sml/masternode_list/merkle_roots.rs | 431 +++++++++++++++++- .../src/sml/masternode_list_engine/helpers.rs | 12 +- dash/src/sml/masternode_list_engine/mod.rs | 179 +++++++- dash/src/test_utils/sml.rs | 180 +++++++- 14 files changed, 1475 insertions(+), 196 deletions(-) diff --git a/dash-spv/src/storage/masternode.rs b/dash-spv/src/storage/masternode.rs index 10767b1ed..5c2dc6a1d 100644 --- a/dash-spv/src/storage/masternode.rs +++ b/dash-spv/src/storage/masternode.rs @@ -15,7 +15,7 @@ use dashcore::sml::masternode_list_engine::{qr_info_diffs, MasternodeListEngine, use dashcore::sml::quorum_entry::qualified_quorum_entry::QualifiedQuorumEntry; use dashcore::{BlockHash, Network, QuorumHash}; -use crate::error::{StorageError, StorageResult}; +use crate::error::{StorageError, StorageResult, SyncError, SyncResult}; use crate::storage::{io::atomic_write, BlockHeaderStorage}; type IndexMap = BTreeMap; @@ -196,6 +196,14 @@ impl MessageLog { ); continue; } + let proven = match &message { + Message::Diff(diff) => verify_diff_coinbase(&*headers, diff).await, + Message::QrInfo(qr_info) => verify_qr_info_coinbases(&*headers, qr_info).await, + }; + if let Err(e) = proven { + tracing::warn!("Skipping masternode message at {height}: {e}"); + continue; + } match &message { Message::QrInfo(qr_info) => { feed_qrinfo_heights_to_engine(&mut engine, qr_info, &*headers).await; @@ -341,6 +349,44 @@ impl MasternodeStorage for PersistentMasternodeStorage } } +/// Checks that `diff`'s coinbase is the first transaction of the stored block +/// the diff names, see [`MnListDiff::verify_coinbase_merkle_proof`]. +/// +/// The engine checks the list a diff builds against that coinbase whenever it +/// applies the diff, so a diff that passes both ties its list to the header +/// chain. A diff naming a block without a stored header cannot be checked and +/// fails with `SyncError::MissingDependency`, an unproven one with +/// `SyncError::Validation`. +pub(crate) async fn verify_diff_coinbase( + storage: &S, + diff: &MnListDiff, +) -> SyncResult<()> { + let header = match storage.get_header_height_by_hash(&diff.block_hash).await? { + Some(height) => storage.get_header(height).await?, + None => None, + } + .filter(|header| *header.hash() == diff.block_hash) + .ok_or_else(|| { + SyncError::MissingDependency(format!( + "no stored header for the masternode list diff block {}", + diff.block_hash + )) + })?; + diff.verify_coinbase_merkle_proof(header.header().merkle_root) + .map_err(|e| SyncError::Validation(e.to_string())) +} + +/// [`verify_diff_coinbase`] for every diff a QRInfo carries. +pub(crate) async fn verify_qr_info_coinbases( + storage: &S, + qr_info: &QRInfo, +) -> SyncResult<()> { + for diff in qr_info_diffs(qr_info) { + verify_diff_coinbase(storage, diff).await?; + } + Ok(()) +} + /// Feed QRInfo block heights to the engine from storage. /// /// Resolves heights for every hash enumerated by @@ -388,6 +434,10 @@ pub(crate) async fn feed_qrinfo_heights_to_engine( mod tests { use super::*; use crate::test_utils::MockHeaderStorage; + use dashcore::block::Header; + use dashcore::network::message::{NetworkMessage, RawNetworkMessage}; + use dashcore::sml::masternode_list_entry::MasternodeListEntry; + use dashcore::TxMerkleNode; use dashcore_hashes::Hash; use tempfile::TempDir; @@ -396,31 +446,53 @@ mod tests { BlockHash::from_slice(&[byte; 32]).unwrap() } + /// Knows the heights of `heights` and holds the header that proves the + /// coinbase of each of `diffs`. + fn headers(heights: &[(u8, u32)], diffs: &[&MnListDiff]) -> MockHeaderStorage { + let map = heights.iter().map(|(b, h)| (hash(*b), *h)).collect(); + diffs + .iter() + .fold(MockHeaderStorage::new(map), |headers, diff| headers.with_header_for(diff)) + } + async fn open_storage( dir: &TempDir, - heights: &[(u8, u32)], + headers: MockHeaderStorage, ) -> PersistentMasternodeStorage { - let map = heights.iter().map(|(b, h)| (hash(*b), *h)).collect(); PersistentMasternodeStorage::open( dir.path(), - Arc::new(RwLock::new(MockHeaderStorage(map))), + Arc::new(RwLock::new(headers)), Network::Regtest, ) .await .expect("open") } + /// [`MnListDiff::dummy`] from `base` to `tip` applied on the list the + /// dummy diff from genesis to `base` built. + fn dummy_on_dummy(base: u8, tip: u8) -> MnListDiff { + MnListDiff::dummy(base, tip).with_coinbase_committing_to( + &[MasternodeListEntry::dummy(base), MasternodeListEntry::dummy(tip)], + &[], + ) + } + #[tokio::test] async fn a_reopened_storage_replays_what_was_stored() { let dir = TempDir::new().unwrap(); let heights = [(0x00, 0), (0xAA, 100), (0xBB, 200)]; + let (first, second) = (MnListDiff::dummy(0x00, 0xAA), dummy_on_dummy(0xAA, 0xBB)); { - let mut storage = open_storage(&dir, &heights).await; - storage.store_diff(100, &MnListDiff::dummy(0x00, 0xAA)).await.unwrap(); - storage.store_diff(200, &MnListDiff::dummy(0xAA, 0xBB)).await.unwrap(); + let mut storage = open_storage(&dir, headers(&heights, &[])).await; + storage.store_diff(100, &first).await.unwrap(); + storage.store_diff(200, &second).await.unwrap(); } - let engine = open_storage(&dir, &heights).await.load_engine().await.unwrap(); + let engine = open_storage(&dir, headers(&heights, &[&first, &second])) + .await + .load_engine() + .await + .unwrap(); assert_eq!(engine.masternode_lists.keys().copied().collect::>(), vec![100, 200]); assert_eq!(engine.masternode_lists[&200].block_hash, hash(0xBB)); @@ -441,26 +513,28 @@ mod tests { let dir = TempDir::new().unwrap(); let quorum_hash = QuorumHash::from_byte_array([0xAB; 32]); + let quorum = QuorumEntry { + version: 1, + llmq_type: LLMQType::LlmqtypeTest, + quorum_hash, + quorum_index: None, + signers: vec![true; 3], + valid_members: vec![true; 3], + quorum_public_key: BLSPublicKey::from([7; 48]), + quorum_vvec_hash: QuorumVVecHash::all_zeros(), + threshold_sig: BLSSignature::from([1; 96]), + all_commitment_aggregated_signature: BLSSignature::from([1; 96]), + }; let diff = MnListDiff { - new_quorums: vec![QuorumEntry { - version: 1, - llmq_type: LLMQType::LlmqtypeTest, - quorum_hash, - quorum_index: None, - signers: vec![true; 3], - valid_members: vec![true; 3], - quorum_public_key: BLSPublicKey::from([7; 48]), - quorum_vvec_hash: QuorumVVecHash::all_zeros(), - threshold_sig: BLSSignature::from([1; 96]), - all_commitment_aggregated_signature: BLSSignature::from([1; 96]), - }], + new_quorums: vec![quorum.clone()], quorums_chainlock_signatures: vec![QuorumCLSigObject { signature: BLSSignature::from([1; 96]), index_set: vec![0], }], ..MnListDiff::dummy(0x00, 0xAA) - }; - let mut storage = open_storage(&dir, &[(0x00, 0), (0xAA, 100)]).await; + } + .with_coinbase_committing_to(&[MasternodeListEntry::dummy(0xAA)], &[quorum]); + let mut storage = open_storage(&dir, headers(&[(0x00, 0), (0xAA, 100)], &[&diff])).await; storage.store_diff(100, &diff).await.unwrap(); let engine = storage.load_engine().await.unwrap(); @@ -481,9 +555,10 @@ mod tests { #[tokio::test] async fn replay_skips_a_message_whose_block_left_the_chain() { let dir = TempDir::new().unwrap(); - let mut storage = open_storage(&dir, &[(0x00, 0), (0xAA, 100)]).await; - storage.store_diff(100, &MnListDiff::dummy(0x00, 0xAA)).await.unwrap(); - storage.store_diff(200, &MnListDiff::dummy(0xAA, 0xBB)).await.unwrap(); + let first = MnListDiff::dummy(0x00, 0xAA); + let mut storage = open_storage(&dir, headers(&[(0x00, 0), (0xAA, 100)], &[&first])).await; + storage.store_diff(100, &first).await.unwrap(); + storage.store_diff(200, &dummy_on_dummy(0xAA, 0xBB)).await.unwrap(); let engine = storage.load_engine().await.unwrap(); @@ -493,12 +568,17 @@ mod tests { #[tokio::test] async fn replay_skips_an_orphan_and_keeps_going() { let dir = TempDir::new().unwrap(); - let mut storage = - open_storage(&dir, &[(0x00, 0), (0xAA, 100), (0xBB, 150), (0xDD, 120)]).await; + let (first, orphan, second) = ( + MnListDiff::dummy(0x00, 0xAA), + MnListDiff::dummy(0xEE, 0xDD), + dummy_on_dummy(0xAA, 0xBB), + ); + let heights = [(0x00, 0), (0xAA, 100), (0xBB, 150), (0xDD, 120)]; + let mut storage = open_storage(&dir, headers(&heights, &[&first, &orphan, &second])).await; - storage.store_diff(100, &MnListDiff::dummy(0x00, 0xAA)).await.unwrap(); - storage.store_diff(120, &MnListDiff::dummy(0xEE, 0xDD)).await.unwrap(); - storage.store_diff(150, &MnListDiff::dummy(0xAA, 0xBB)).await.unwrap(); + storage.store_diff(100, &first).await.unwrap(); + storage.store_diff(120, &orphan).await.unwrap(); + storage.store_diff(150, &second).await.unwrap(); let engine = storage.load_engine().await.expect("replay must not fail on an orphan"); @@ -512,7 +592,7 @@ mod tests { #[tokio::test] async fn storing_the_same_height_twice_keeps_the_newer_message() { let dir = TempDir::new().unwrap(); - let mut storage = open_storage(&dir, &[]).await; + let mut storage = open_storage(&dir, MockHeaderStorage::default()).await; storage.store_diff(100, &MnListDiff::dummy(0x00, 0xAA)).await.unwrap(); storage.store_diff(100, &MnListDiff::dummy(0x00, 0xBB)).await.unwrap(); @@ -531,15 +611,17 @@ mod tests { let folder = dir.path().join(PersistentMasternodeStorage::::FOLDER_NAME); tokio::fs::create_dir_all(&folder).await.unwrap(); + let diff = MnListDiff::dummy(0x00, 0xAA); + let heights = [(0x00, 0), (0xAA, 100)]; { - let mut storage = open_storage(&dir, &[(0x00, 0), (0xAA, 100)]).await; - storage.store_diff(100, &MnListDiff::dummy(0x00, 0xAA)).await.unwrap(); + let mut storage = open_storage(&dir, headers(&heights, &[&diff])).await; + storage.store_diff(100, &diff).await.unwrap(); } tokio::fs::write(folder.join("diff_50.dat"), b"not a diff").await.unwrap(); tokio::fs::write(folder.join("diff_abc.dat"), b"x").await.unwrap(); - let storage = open_storage(&dir, &[(0x00, 0), (0xAA, 100)]).await; + let storage = open_storage(&dir, headers(&heights, &[&diff])).await; assert_eq!( storage.diffs.keys().copied().collect::>(), vec![50, 100], @@ -567,32 +649,40 @@ mod tests { let tip = 2_500; let llmq_type = Network::Regtest.platform_type(); let quorum_hash = QuorumHash::from_byte_array([0xAB; 32]); + let quorum = QuorumEntry { + version: 1, + llmq_type, + quorum_hash, + quorum_index: None, + signers: vec![true; 3], + valid_members: vec![true; 3], + quorum_public_key: BLSPublicKey::from([7; 48]), + quorum_vvec_hash: QuorumVVecHash::all_zeros(), + threshold_sig: BLSSignature::from([1; 96]), + all_commitment_aggregated_signature: BLSSignature::from([1; 96]), + }; + let masternodes = [MasternodeListEntry::dummy(0x01)]; let mined = MnListDiff { block_hash: BlockHash::dummy(1), - new_quorums: vec![QuorumEntry { - version: 1, - llmq_type, - quorum_hash, - quorum_index: None, - signers: vec![true; 3], - valid_members: vec![true; 3], - quorum_public_key: BLSPublicKey::from([7; 48]), - quorum_vvec_hash: QuorumVVecHash::all_zeros(), - threshold_sig: BLSSignature::from([1; 96]), - all_commitment_aggregated_signature: BLSSignature::from([1; 96]), - }], + new_quorums: vec![quorum.clone()], quorums_chainlock_signatures: vec![QuorumCLSigObject { signature: BLSSignature::from([1; 96]), index_set: vec![0], }], ..MnListDiff::dummy(0x00, 0x01) - }; + } + .with_coinbase_committing_to(&masternodes, &[quorum]); let retired = MnListDiff { deleted_quorums: vec![DeletedQuorum { llmq_type, quorum_hash, }], ..MnListDiff::dummy_between(1, 2) + } + .with_coinbase_committing_to(&masternodes, &[]); + let unchanged = |height| { + MnListDiff::dummy_between(height - 1, height) + .with_coinbase_committing_to(&masternodes, &[]) }; let dir = TempDir::new().unwrap(); @@ -602,19 +692,18 @@ mod tests { std::fs::write(folder.join(S::file_name(S::DIFF_PREFIX, height)), serialize(diff)) .unwrap() }; + let heights = (1..=tip).map(|height| (BlockHash::dummy(height), height)).collect(); + let mut headers = + MockHeaderStorage::new(heights).with_header_for(&mined).with_header_for(&retired); write(1, &mined); write(2, &retired); for height in 3..=tip { - write(height, &MnListDiff::dummy_between(height - 1, height)); + let diff = unchanged(height); + write(height, &diff); + headers = headers.with_header_for(&diff); } - let heights = (1..=tip).map(|height| (BlockHash::dummy(height), height)).collect(); - let storage = S::open( - dir.path(), - Arc::new(RwLock::new(MockHeaderStorage(heights))), - Network::Regtest, - ) - .await - .unwrap(); + let storage = + S::open(dir.path(), Arc::new(RwLock::new(headers)), Network::Regtest).await.unwrap(); let engine = storage.load_engine().await.unwrap(); assert!(engine @@ -628,6 +717,73 @@ mod tests { assert!(log.quorum_entry_at_or_before(llmq_type, unknown, 100).await.is_none()); } + #[tokio::test] + async fn replay_skips_a_message_its_block_header_does_not_prove() { + let dir = TempDir::new().unwrap(); + let diff = MnListDiff::dummy(0x00, 0xAA); + let heights = [(0x00, 0), (0xAA, 100)]; + let unproven = headers(&heights, &[]).with_header(hash(0xAA), TxMerkleNode::all_zeros()); + let mut storage = open_storage(&dir, unproven).await; + storage.store_diff(100, &diff).await.unwrap(); + + let engine = storage.load_engine().await.unwrap(); + assert!(engine.masternode_lists.is_empty(), "a coinbase the header does not prove"); + + let engine = + open_storage(&dir, headers(&heights, &[&diff])).await.load_engine().await.unwrap(); + assert!(engine.masternode_lists.contains_key(&100), "the header that proves it"); + } + + /// The mainnet QRInfo capture and a header storage holding the headers of + /// its blocks, as public block explorers serve them. Each header hashes to + /// the block hash of one of the diffs. + fn mainnet_qr_info_with_headers() -> (QRInfo, MockHeaderStorage) { + const HEADERS: [(u32, &str); 5] = [ + (2224359, "00000020552ffd09b040ec6aafd6a76fe586eed37dca3a88b653732027000000000000001a28433a3aaf9be619f3d9ae713e329a48c35a366afd488662a159df84d4484f08deb367ccfe2619980e0ae3"), + (2224216, "000000203d986f88b0a06213229d3ab8a3650bc86af8e04fb3d9be9b20000000000000000bbea6733e6180f312fbd891d5dbd72f3089d66af33e9292aa4e1be985652ffc8186b367773b2c196646dce8"), + (2223928, "000000200c2693bfeffebad43ea41aca74fb652f58e3264d65454ace1d0000000000000040a50328f3f6e173ce97b4d1fa9e0b3f36948e6888be0adf39d024540e4bb950a2d5b26799c12b19821af24e"), + (2223640, "00000020174c02dd39c2371274cf025eeb2b70b9a156ad1327ecded51200000000000000785ec2546f5347f4960171ee93b319094a34aa4cab8277f985434b6b4b4fa6e9fb21b267ba072f19b46b3938"), + (2223352, "000000207654d9fcd141892f9d47cf5c40d46d133d573e8bb9fbaf350400000000000000925d75e37bfd754cc73d6f54629ab6e1e38eb917a3452a73d1377be0d6c62876d36eb16781f3231902819b59"), + ]; + let hex = include_str!("../../../dash/tests/data/test_DML_diffs/QR_INFO_0_2224359.hex"); + let message: RawNetworkMessage = deserialize(&hex::decode(hex).unwrap()).unwrap(); + let NetworkMessage::QRInfo(qr_info) = message.payload else { + panic!("expected a qrinfo message"); + }; + + let headers: Vec<(u32, Header)> = HEADERS + .iter() + .map(|(height, header)| (*height, deserialize(&hex::decode(header).unwrap()).unwrap())) + .collect(); + let heights = + headers.iter().map(|(height, header)| (header.block_hash(), *height)).collect(); + let storage = + headers.iter().fold(MockHeaderStorage::new(heights), |storage, (_, header)| { + storage.with_header(header.block_hash(), header.merkle_root) + }); + (qr_info, storage) + } + + #[tokio::test] + async fn every_diff_of_a_qr_info_is_proven_by_its_stored_block_header() { + let (qr_info, storage) = mainnet_qr_info_with_headers(); + verify_qr_info_coinbases(&storage, &qr_info).await.expect("every coinbase is proven"); + + let mut swapped = qr_info.clone(); + swapped.mn_list_diff_h.coinbase_tx = qr_info.mn_list_diff_tip.coinbase_tx.clone(); + assert!(matches!( + verify_qr_info_coinbases(&storage, &swapped).await, + Err(SyncError::Validation(_)) + )); + + let mut unknown = qr_info; + unknown.mn_list_diff_h.block_hash = hash(0x09); + assert!(matches!( + verify_qr_info_coinbases(&storage, &unknown).await, + Err(SyncError::MissingDependency(_)) + )); + } + #[test] fn qr_info_bases_are_what_its_chain_starts_from() { let mut qr_info = QRInfo::dummy(0x00); diff --git a/dash-spv/src/storage/mod.rs b/dash-spv/src/storage/mod.rs index afb2e1361..7b67a62ad 100644 --- a/dash-spv/src/storage/mod.rs +++ b/dash-spv/src/storage/mod.rs @@ -33,7 +33,9 @@ pub use crate::storage::block_headers::{ pub use crate::storage::blocks::{BlockStorage, PersistentBlockStorage}; pub use crate::storage::filter_headers::{FilterHeaderStorage, PersistentFilterHeaderStorage}; pub use crate::storage::filters::{FilterStorage, PersistentFilterStorage}; -pub(crate) use crate::storage::masternode::feed_qrinfo_heights_to_engine; +pub(crate) use crate::storage::masternode::{ + feed_qrinfo_heights_to_engine, verify_diff_coinbase, verify_qr_info_coinbases, +}; pub use crate::storage::masternode::{MasternodeStorage, PersistentMasternodeStorage}; pub use crate::storage::metadata::{MetadataStorage, PersistentMetadataStorage}; pub use crate::storage::peers::{PeerStorage, PersistentPeerStorage}; diff --git a/dash-spv/src/sync/masternodes/sync_manager.rs b/dash-spv/src/sync/masternodes/sync_manager.rs index d32bb6980..c79064db1 100644 --- a/dash-spv/src/sync/masternodes/sync_manager.rs +++ b/dash-spv/src/sync/masternodes/sync_manager.rs @@ -1,7 +1,10 @@ use super::manager::PipelineMode; use crate::error::SyncResult; use crate::network::{Message, MessageType, RequestSender}; -use crate::storage::{feed_qrinfo_heights_to_engine, BlockHeaderStorage}; +use crate::storage::{ + feed_qrinfo_heights_to_engine, verify_diff_coinbase, verify_qr_info_coinbases, + BlockHeaderStorage, +}; use crate::sync::{ ManagerIdentifier, MasternodesManager, SyncEvent, SyncManager, SyncManagerProgress, SyncState, }; @@ -208,8 +211,18 @@ impl SyncManager for MasternodesManager { } tracing::info!("Processing QRInfo message"); - // Feed block heights to engine using internal storage + // Every diff's coinbase has to be the first transaction of its + // stored block before the engine checks the lists against it. let storage = self.header_storage.read().await; + if let Err(e) = verify_qr_info_coinbases(&*storage, qr_info).await { + drop(storage); + tracing::error!("QRInfo does not match the stored block headers: {}", e); + // Rejected like a response the engine refuses, see below. + self.sync_state.qrinfo_rejected(); + return Err(SyncError::MasternodeSyncFailed(e.to_string())); + } + + // Feed block heights to engine using internal storage let mut engine = self.engine.write().await; let fed = feed_qrinfo_heights_to_engine(&mut engine, qr_info, &*storage).await; drop(storage); @@ -368,29 +381,43 @@ impl SyncManager for MasternodesManager { return Ok(vec![]); } }; + + // The diff's coinbase has to be the first transaction of the + // stored block before the engine checks the list against it. + let proven = verify_diff_coinbase(&*storage, diff).await; drop(storage); - // Apply diff to engine - let mut engine = self.engine.write().await; - engine.feed_block_height(target_height, diff.block_hash); - - let apply_ok = - match engine.apply_diff(diff.clone(), Some(target_height), false, None) { - Ok(_) => { - self.sync_state.known_mn_list_heights.insert(target_height); - tracing::debug!("Applied MnListDiff at height {}", target_height); - true - } - Err(e) => { - tracing::warn!( - "Failed to apply MnListDiff at height {}: {}", - target_height, - e - ); - false + let apply_ok = match proven { + Ok(()) => { + // Apply diff to engine + let mut engine = self.engine.write().await; + engine.feed_block_height(target_height, diff.block_hash); + + match engine.apply_diff(diff.clone(), Some(target_height), false, None) { + Ok(_) => { + self.sync_state.known_mn_list_heights.insert(target_height); + tracing::debug!("Applied MnListDiff at height {}", target_height); + true + } + Err(e) => { + tracing::warn!( + "Failed to apply MnListDiff at height {}: {}", + target_height, + e + ); + false + } } - }; - drop(engine); + } + Err(e) => { + tracing::warn!( + "MnListDiff at height {} does not match its block header: {}", + target_height, + e + ); + false + } + }; if apply_ok { if let Err(e) = self.store_diff(target_height, diff).await { @@ -715,7 +742,7 @@ impl SyncManager for MasternodesManager { #[cfg(test)] mod tests { - use super::super::manager::{MasternodeSyncState, QRInfoInFlight}; + use super::super::manager::{MasternodeSyncState, PipelineMode, QRInfoInFlight}; use super::{ qrinfo_timeout_for, MAX_RETRY_ATTEMPTS, QRINFO_STALL_WATCHDOG, QRINFO_TIMEOUT_SCHEDULE_SECS, }; @@ -737,7 +764,7 @@ mod tests { use dashcore::sml::llmq_type::LLMQType; use dashcore::sml::masternode_list_engine::MasternodeListEngine; use dashcore::transaction::special_transaction::quorum_commitment::QuorumEntry; - use dashcore::{BlockHash, Network}; + use dashcore::{BlockHash, Network, TxMerkleNode}; use dashcore_hashes::Hash; use std::collections::HashMap; use std::sync::Arc; @@ -818,7 +845,8 @@ mod tests { network: Network::Testnet, ..Default::default() }; - feed_qrinfo_heights_to_engine(&mut engine, &qr_info, &MockHeaderStorage(height_map)).await; + feed_qrinfo_heights_to_engine(&mut engine, &qr_info, &MockHeaderStorage::new(height_map)) + .await; for &b in expected_hashes { let hash = BlockHash::from_slice(&[b; 32]).unwrap(); @@ -1073,6 +1101,64 @@ mod tests { ); } + /// A diff reaches the engine only once its coinbase proof leads to the + /// merkle root of the stored header of its block. A diff that fails is + /// dropped like one the engine refuses. + #[tokio::test] + async fn test_mnlistdiff_applies_only_when_its_block_header_proves_the_coinbase() { + let tip = 200u32; + let peer = "127.0.0.1:19999".parse().unwrap(); + for header_proves_it in [true, false] { + let template = MnListDiff::dummy(0x00, 0x01); + let mut headers = Header::dummy_batch(0..tip + 1); + headers[tip as usize].merkle_root = if header_proves_it { + template.dummy_block_merkle_root() + } else { + TxMerkleNode::all_zeros() + }; + let tip_hash = headers[tip as usize].block_hash(); + let storage = DiskStorageManager::with_temp_dir().await.unwrap(); + let block_headers = storage.block_headers(); + block_headers + .write() + .await + .store_headers(&headers.iter().map(HashedBlockHeader::from).collect::>()) + .await + .unwrap(); + let engine = + Arc::new(RwLock::new(MasternodeListEngine::default_for_network(Network::Regtest))); + let mut manager = + MasternodesManager::new(block_headers, Arc::clone(&engine), Network::Regtest, None) + .await; + manager.set_state(SyncState::Synced); + manager.sync_state.pipeline_mode = PipelineMode::Incremental; + let (tx, _rx) = mpsc::unbounded_channel(); + let requests = RequestSender::new(tx); + manager + .sync_state + .mnlistdiff_pipeline + .queue_requests(vec![(BlockHash::all_zeros(), tip_hash)]); + manager.sync_state.mnlistdiff_pipeline.send_pending(&requests).unwrap(); + + let diff = MnListDiff { + block_hash: tip_hash, + ..template + }; + manager + .handle_message(Message::new(peer, NetworkMessage::MnListDiff(diff)), &requests) + .await + .expect("an unusable diff is dropped, not an error"); + + assert_eq!( + engine.read().await.masternode_lists.contains_key(&tip), + header_proves_it, + "header proves the coinbase: {header_proves_it}" + ); + assert_eq!(manager.sync_state.known_mn_list_heights.contains(&tip), header_proves_it); + assert!(manager.sync_state.mnlistdiff_pipeline.is_complete()); + } + } + /// The stall watchdog is the backstop for every route into the stranded state /// that keeping the in-flight flag cannot cover - notably a /// `send_qrinfo_for_tip` that returns without dispatching *after* its caller diff --git a/dash-spv/src/test_utils/header_storage.rs b/dash-spv/src/test_utils/header_storage.rs index 50141ca33..05dbe781b 100644 --- a/dash-spv/src/test_utils/header_storage.rs +++ b/dash-spv/src/test_utils/header_storage.rs @@ -2,15 +2,51 @@ use std::collections::HashMap; use std::ops::Range; use async_trait::async_trait; -use dashcore::BlockHash; +use dashcore::block::Header; +use dashcore::network::message_sml::MnListDiff; +use dashcore::{BlockHash, TxMerkleNode}; use crate::error::StorageResult; use crate::storage::{BlockHeaderStorage, BlockHeaderTip}; use crate::types::HashedBlockHeader; -/// A [`BlockHeaderStorage`] that answers hash lookups from a map and nothing else. -/// Everything outside `get_header_height_by_hash` is inert. -pub struct MockHeaderStorage(pub HashMap); +/// A [`BlockHeaderStorage`] that answers hash lookups and header reads from +/// maps and nothing else. Everything outside `get_header_height_by_hash` and +/// `get_header` is inert. +#[derive(Default)] +pub struct MockHeaderStorage { + heights: HashMap, + headers: HashMap, +} + +impl MockHeaderStorage { + /// Knows the height of every block in `heights` and holds no header. + pub fn new(heights: HashMap) -> Self { + MockHeaderStorage { + heights, + headers: HashMap::new(), + } + } + + /// Holds a header for `block_hash`, whose height must be known, that + /// commits to `merkle_root`. The rest of the header is filler, and it is + /// stored under `block_hash` rather than its own hash. + pub fn with_header(mut self, block_hash: BlockHash, merkle_root: TxMerkleNode) -> Self { + let height = *self.heights.get(&block_hash).expect("the block's height is known"); + let header = Header { + merkle_root, + ..Header::dummy(height) + }; + self.headers.insert(height, HashedBlockHeader::with_trusted_hash(header, block_hash)); + self + } + + /// Holds the header of `diff`'s block that its coinbase proof leads to, as + /// [`MnListDiff::with_coinbase_committing_to`] sets it up. + pub fn with_header_for(self, diff: &MnListDiff) -> Self { + self.with_header(diff.block_hash, diff.dummy_block_merkle_root()) + } +} #[async_trait] impl BlockHeaderStorage for MockHeaderStorage { @@ -27,6 +63,9 @@ impl BlockHeaderStorage for MockHeaderStorage { async fn load_headers(&self, _: Range) -> StorageResult> { Ok(vec![]) } + async fn get_header(&self, height: u32) -> StorageResult> { + Ok(self.headers.get(&height).cloned()) + } async fn get_tip_height(&self) -> Option { None } @@ -40,10 +79,11 @@ impl BlockHeaderStorage for MockHeaderStorage { 0 } async fn get_header_height_by_hash(&self, hash: &BlockHash) -> StorageResult> { - Ok(self.0.get(hash).copied()) + Ok(self.heights.get(hash).copied()) } async fn truncate_above(&mut self, target_height: u32) -> StorageResult<()> { - self.0.retain(|_, h| *h <= target_height); + self.heights.retain(|_, h| *h <= target_height); + self.headers.retain(|h, _| *h <= target_height); Ok(()) } } diff --git a/dash/src/merkle_tree/block.rs b/dash/src/merkle_tree/block.rs index 264977984..2da21c350 100644 --- a/dash/src/merkle_tree/block.rs +++ b/dash/src/merkle_tree/block.rs @@ -225,6 +225,23 @@ impl PartialMerkleTree { &self.hashes } + /// Builds a partial merkle tree from the parts it is serialized as: the block's total + /// transaction count, the hashes in depth-first order, and the flag bits packed eight per + /// byte, least significant bit first. + /// + /// Messages that embed a partial merkle tree field by field, such as `mnlistdiff`, use this to + /// check their proof with [`Self::extract_matches`]. Nothing is validated here. + pub fn from_parts(num_transactions: u32, hashes: Vec, flag_bytes: &[u8]) -> Self { + let bits = (0..flag_bytes.len() * 8) + .map(|p| (flag_bytes[p / 8] & (1 << (p % 8) as u8)) != 0) + .collect(); + PartialMerkleTree { + num_transactions, + hashes, + bits, + } + } + /// Construct a partial merkle tree /// The `txids` are the transaction hashes of the block and the `matches` is the contains flags /// wherever a tx hash should be included in the proof. @@ -457,18 +474,8 @@ impl Decodable for PartialMerkleTree { fn consensus_decode(r: &mut R) -> Result { let num_transactions: u32 = Decodable::consensus_decode(r)?; let hashes: Vec = Decodable::consensus_decode(r)?; - let bytes: Vec = Decodable::consensus_decode(r)?; - let mut bits: Vec = vec![false; bytes.len() * 8]; - - for (p, bit) in bits.iter_mut().enumerate() { - *bit = (bytes[p / 8] & (1 << (p % 8) as u8)) != 0; - } - Ok(PartialMerkleTree { - num_transactions, - hashes, - bits, - }) + Ok(PartialMerkleTree::from_parts(num_transactions, hashes, &bytes)) } } diff --git a/dash/src/network/message_sml.rs b/dash/src/network/message_sml.rs index 9212811d8..a1b259769 100644 --- a/dash/src/network/message_sml.rs +++ b/dash/src/network/message_sml.rs @@ -4,10 +4,12 @@ use bincode::{Decode, Encode}; use crate::bls_sig_utils::BLSSignature; use crate::hash_types::MerkleRootMasternodeList; use crate::internal_macros::impl_consensus_encoding; +use crate::merkle_tree::PartialMerkleTree; +use crate::sml::error::SmlError; use crate::sml::llmq_type::LLMQType; use crate::sml::masternode_list_entry::MasternodeListEntry; use crate::transaction::special_transaction::quorum_commitment::QuorumEntry; -use crate::{BlockHash, ProTxHash, QuorumHash, Transaction}; +use crate::{BlockHash, ProTxHash, QuorumHash, Transaction, TxMerkleNode}; /// The `getmnlistd` message requests a `mnlistdiff` message that provides either: /// - A full masternode list (if `base_block_hash` is all-zero) @@ -76,6 +78,57 @@ impl_consensus_encoding!( quorums_chainlock_signatures ); +impl MnListDiff { + /// Checks that `coinbase_tx` is the first transaction of the block whose header commits to + /// `block_merkle_root`, the way Dash Core builds the proof in `BuildSimplifiedMNListDiff`: + /// the partial merkle tree (`total_transactions`, `merkle_hashes`, `merkle_flags`) must lead + /// to that root and match exactly one transaction, at position 0, whose txid is the + /// coinbase's. + /// + /// Together with [`MasternodeList::verify_coinbase_merkle_roots`], which runs whenever a diff + /// is applied, this ties the resulting masternode list to the block header. + /// + /// [`MasternodeList::verify_coinbase_merkle_roots`]: crate::sml::masternode_list::MasternodeList::verify_coinbase_merkle_roots + /// + /// # Errors + /// + /// - `SmlError::InvalidCoinbaseMerkleProof` if the partial merkle tree is malformed. + /// - `SmlError::CoinbaseMerkleRootMismatch` if it leads to a different merkle root. + /// - `SmlError::CoinbaseNotProven` if it does not match exactly the coinbase at position 0. + pub fn verify_coinbase_merkle_proof( + &self, + block_merkle_root: TxMerkleNode, + ) -> Result<(), SmlError> { + let tree = PartialMerkleTree::from_parts( + self.total_transactions, + self.merkle_hashes + .iter() + .map(|hash| TxMerkleNode::from_raw_hash(hash.to_raw_hash())) + .collect(), + &self.merkle_flags, + ); + let mut matches = Vec::new(); + let mut indexes = Vec::new(); + let computed = tree.extract_matches(&mut matches, &mut indexes).map_err(|error| { + SmlError::InvalidCoinbaseMerkleProof { + block_hash: self.block_hash, + reason: error.to_string(), + } + })?; + if computed != block_merkle_root { + return Err(SmlError::CoinbaseMerkleRootMismatch { + block_hash: self.block_hash, + expected: block_merkle_root, + computed, + }); + } + if indexes != [0] || matches != [self.coinbase_tx.txid()] { + return Err(SmlError::CoinbaseNotProven(self.block_hash)); + } + Ok(()) + } +} + #[derive(PartialEq, Eq, Clone, Debug)] #[cfg_attr(feature = "bincode", derive(Encode, Decode))] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] @@ -99,10 +152,189 @@ impl_consensus_encoding!(DeletedQuorum, llmq_type, quorum_hash); #[cfg(test)] mod tests { use assert_matches::assert_matches; + use hashes::Hash; + use crate::block::Header; use crate::consensus::{deserialize, serialize}; + use crate::hash_types::{MerkleRootMasternodeList, MerkleRootQuorums}; + use crate::merkle_tree::PartialMerkleTree; use crate::network::message::{NetworkMessage, RawNetworkMessage}; use crate::network::message_sml::MnListDiff; + use crate::sml::error::SmlError; + use crate::transaction::special_transaction::TransactionPayload; + use crate::transaction::special_transaction::coinbase::CoinbasePayload; + use crate::{TxMerkleNode, Txid}; + + fn mainnet_diff_0_2221605() -> MnListDiff { + let hex = include_str!("../../tests/data/test_DML_diffs/DML_0_2221605.hex"); + let message: RawNetworkMessage = + deserialize(&hex::decode(hex).expect("hex")).expect("raw message"); + let NetworkMessage::MnListDiff(diff) = message.payload else { + panic!("expected an mnlistdiff message"); + }; + diff + } + + /// The fixture diffs with the headers of their blocks, as public block + /// explorers serve them. Each header must hash to its diff's + /// `block_hash`, which vouches for the merkle root it carries. + fn fixtures_with_headers() -> Vec<(MnListDiff, Header)> { + [ + ( + mainnet_diff_0_2221605(), + "000000204ec775c3857587ed8ce3e6cf6eaf64bfb90db816033385f71d0000000000000068c9fe75363f12a93d2650265d79a2f7211a2cf8d05836e2f62c40a500cb379d3e3cad675c6a2619187b5901", + ), + ( + MnListDiff::mainnet_fixture_0_2227096(), + "00000020e78c642c9c5eb8c74fed6b669804cd137c3b0962e42ad9c5020000000000000031f22fe6c6efe79702be25811d2bb599393a896996e9e560d01a1181a78585294f78ba67247728195c403c3b", + ), + ( + MnListDiff::mainnet_fixture_2227096_2241332(), + "000000202996884499815938739e79ab0ddd74f3d29c8c0d14f552ab0d000000000000005880e9ed43f861deacd74ee245eb7e6c6c8e89c0cd67f792c4b35e08c4e471a9fdb6dc677e4621196c87ee9c", + ), + ( + MnListDiff::testnet_fixture_0_1296600(), + "000000200433318ebbd848278dc2f76559d4567e18a3d90de3a87374c55cfca237010000bbcb40f521223cac77feef6223ecb62beeccbf78fd918b7f8c98f29c661d1914d0bf86680176021e0f4f0400", + ), + ] + .into_iter() + .map(|(diff, header)| { + let header: Header = deserialize(&hex::decode(header).expect("hex")).expect("header"); + assert_eq!(header.block_hash(), diff.block_hash, "a header of another block"); + (diff, header) + }) + .collect() + } + + fn set_proof(diff: &mut MnListDiff, tree: &PartialMerkleTree) { + diff.total_transactions = tree.num_transactions(); + diff.merkle_hashes = tree + .hashes() + .iter() + .map(|hash| MerkleRootMasternodeList::from_raw_hash(hash.to_raw_hash())) + .collect(); + let mut flags = vec![0u8; tree.bits().len().div_ceil(8)]; + for (position, bit) in tree.bits().iter().enumerate() { + if *bit { + flags[position / 8] |= 1 << (position % 8); + } + } + diff.merkle_flags = flags; + } + + fn root_of(tree: &PartialMerkleTree) -> TxMerkleNode { + tree.extract_matches(&mut vec![], &mut vec![]).expect("a well-formed tree") + } + + #[test] + fn fixture_coinbases_are_proven_by_their_block_headers() { + for (diff, header) in fixtures_with_headers() { + diff.verify_coinbase_merkle_proof(header.merkle_root) + .unwrap_or_else(|e| panic!("{}: {e}", diff.block_hash)); + } + } + + #[test] + fn an_edited_merkle_proof_is_refused() { + let (diff, header) = fixtures_with_headers().swap_remove(0); + assert!(diff.total_transactions > 2, "the fixture proves a block of several transactions"); + + let mut edited = diff.clone(); + let last = edited.merkle_hashes.len() - 1; + edited.merkle_hashes[last] = MerkleRootMasternodeList::from_byte_array([7; 32]); + assert_matches!( + edited.verify_coinbase_merkle_proof(header.merkle_root), + Err(SmlError::CoinbaseMerkleRootMismatch { .. }) + ); + + for bit in 0..2 { + let mut edited = diff.clone(); + edited.merkle_flags[0] ^= 1 << bit; + assert!( + edited.verify_coinbase_merkle_proof(header.merkle_root).is_err(), + "flag bit {bit} flipped" + ); + } + + // A count that changes the height of the tree changes the path to the + // coinbase. One of the same height leaves the leftmost path, and so + // the proof, as it is. + let mut edited = diff.clone(); + edited.total_transactions *= 2; + assert!(edited.verify_coinbase_merkle_proof(header.merkle_root).is_err()); + } + + #[test] + fn a_foreign_coinbase_or_header_is_refused() { + let fixtures = fixtures_with_headers(); + let (diff, header) = &fixtures[0]; + let (other_diff, other_header) = &fixtures[1]; + + assert_matches!( + diff.verify_coinbase_merkle_proof(other_header.merkle_root), + Err(SmlError::CoinbaseMerkleRootMismatch { .. }) + ); + + let mut swapped = diff.clone(); + swapped.coinbase_tx = other_diff.coinbase_tx.clone(); + assert_matches!( + swapped.verify_coinbase_merkle_proof(header.merkle_root), + Err(SmlError::CoinbaseNotProven(_)) + ); + } + + /// Core's proof flags the coinbase and nothing else, so a proof of any + /// other selection does not prove the coinbase. + #[test] + fn a_proof_that_does_not_select_only_the_coinbase_is_refused() { + let diff = mainnet_diff_0_2221605(); + let txids = [ + diff.coinbase_tx.txid(), + Txid::from_byte_array([1; 32]), + Txid::from_byte_array([2; 32]), + ]; + + for selection in [[false, true, false], [true, true, false]] { + let tree = PartialMerkleTree::from_txids(&txids, &selection); + let mut proven = diff.clone(); + set_proof(&mut proven, &tree); + assert_matches!( + proven.verify_coinbase_merkle_proof(root_of(&tree)), + Err(SmlError::CoinbaseNotProven(_)), + "selection {selection:?}" + ); + } + } + + /// A version 1 coinbase payload carries only the height and + /// `merkleRootMNList`, and is proven like any other. + #[test] + fn a_version_1_coinbase_is_proven_by_its_block() { + let mut diff = mainnet_diff_0_2221605(); + let Some(TransactionPayload::CoinbasePayloadType(payload)) = + &mut diff.coinbase_tx.special_transaction_payload + else { + panic!("the fixture's coinbase carries a payload"); + }; + *payload = CoinbasePayload { + version: 1, + height: payload.height, + merkle_root_masternode_list: payload.merkle_root_masternode_list, + merkle_root_quorums: MerkleRootQuorums::all_zeros(), + best_cl_height: None, + best_cl_signature: None, + asset_locked_amount: None, + }; + let tree = PartialMerkleTree::from_txids( + &[diff.coinbase_tx.txid(), Txid::from_byte_array([1; 32])], + &[true, false], + ); + set_proof(&mut diff, &tree); + + let decoded: MnListDiff = deserialize(&serialize(&diff)).expect("round trip"); + assert_eq!(decoded, diff, "a version 1 payload survives the wire"); + decoded.verify_coinbase_merkle_proof(root_of(&tree)).expect("the proof holds"); + } #[test] fn deserialize_mn_list_diff() { diff --git a/dash/src/sml/error.rs b/dash/src/sml/error.rs index a8f71bfe0..d28cd4837 100644 --- a/dash/src/sml/error.rs +++ b/dash/src/sml/error.rs @@ -2,7 +2,8 @@ use bincode::{Decode, Encode}; use thiserror::Error; -use crate::BlockHash; +use crate::hash_types::{MerkleRootMasternodeList, MerkleRootQuorums}; +use crate::{BlockHash, TxMerkleNode}; #[derive(Debug, Error, Clone, PartialEq, Eq, Ord, PartialOrd, Hash)] #[cfg_attr(feature = "bincode", derive(Encode, Decode))] @@ -50,4 +51,50 @@ pub enum SmlError { /// Error indicating the quorum signature set is incomplete (some slots were not filled). #[error("Incomplete quorum signature set; not all slots were filled")] IncompleteSignatureSet, + + /// The diff's partial merkle tree is malformed. + #[error("Invalid coinbase merkle proof in the diff for block {block_hash}: {reason}")] + InvalidCoinbaseMerkleProof { + block_hash: BlockHash, + reason: String, + }, + + /// The diff's partial merkle tree does not lead to the merkle root of the block header. + #[error( + "Coinbase merkle proof for block {block_hash} leads to {computed}, but the block header commits to {expected}" + )] + CoinbaseMerkleRootMismatch { + block_hash: BlockHash, + expected: TxMerkleNode, + computed: TxMerkleNode, + }, + + /// The diff's partial merkle tree does not prove exactly its coinbase transaction as the + /// first transaction of the block. + #[error("The merkle proof in the diff for block {0} does not prove its coinbase transaction")] + CoinbaseNotProven(BlockHash), + + /// The diff's coinbase transaction carries no coinbase special transaction payload. + #[error("The coinbase transaction in the diff for block {0} carries no coinbase payload")] + MissingCoinbasePayload(BlockHash), + + /// The masternode list built from the diff does not match `merkleRootMNList` of the coinbase. + #[error( + "Masternode list merkle root at block {block_hash} is {computed}, but the coinbase commits to {expected}" + )] + MasternodeListMerkleRootMismatch { + block_hash: BlockHash, + expected: MerkleRootMasternodeList, + computed: MerkleRootMasternodeList, + }, + + /// The quorums built from the diff do not match `merkleRootQuorums` of the coinbase. + #[error( + "Quorum merkle root at block {block_hash} is {computed}, but the coinbase commits to {expected}" + )] + QuorumMerkleRootMismatch { + block_hash: BlockHash, + expected: MerkleRootQuorums, + computed: MerkleRootQuorums, + }, } diff --git a/dash/src/sml/masternode_list/apply_diff.rs b/dash/src/sml/masternode_list/apply_diff.rs index 83bd8a2a0..e1fad9d49 100644 --- a/dash/src/sml/masternode_list/apply_diff.rs +++ b/dash/src/sml/masternode_list/apply_diff.rs @@ -23,6 +23,11 @@ impl MasternodeList { /// - Adds or updates new masternodes. /// - Removes deleted quorums. /// - Adds or updates new quorums. + /// - Checks the resulting list against the diff's coinbase, see + /// [`MasternodeList::verify_coinbase_merkle_roots`]. + /// + /// The coinbase is taken as given. [`MnListDiff::verify_coinbase_merkle_proof`] ties it to + /// the block header, which only the caller holds. /// /// # Parameters /// @@ -32,12 +37,15 @@ impl MasternodeList { /// # Returns /// /// - `Ok(MasternodeList)`: A new `MasternodeList` reflecting the applied changes. - /// - `Err(SmlError)`: An error if the base block hash does not match the expected value. + /// - `Err(SmlError)`: An error if the diff does not apply to this list. /// /// # Errors /// /// - Returns `SmlError::BaseBlockHashMismatch` if the `base_block_hash` of the `diff` /// does not match the expected block hash of the current masternode list. + /// - Returns `SmlError::MissingCoinbasePayload`, `SmlError::MasternodeListMerkleRootMismatch` + /// or `SmlError::QuorumMerkleRootMismatch` if the resulting list does not match the + /// coinbase commitments. pub fn apply_diff( &self, diff: MnListDiff, @@ -53,6 +61,8 @@ impl MasternodeList { }); } + let coinbase_tx = diff.coinbase_tx; + let mut updated_masternodes = Arc::clone(&self.masternodes); if !diff.deleted_masternodes.is_empty() || !diff.new_masternodes.is_empty() { let masternodes = Arc::make_mut(&mut updated_masternodes); @@ -69,13 +79,14 @@ impl MasternodeList { && diff.new_quorums.is_empty() && diff.quorums_chainlock_signatures.is_empty() { - let builder = MasternodeList::build( + let masternode_list = MasternodeList::build( updated_masternodes, shared_quorums, diff.block_hash, diff_end_height, - ); - return Ok((builder.build(), None)); + ) + .build_matching_coinbase(&coinbase_tx)?; + return Ok((masternode_list, None)); } let updated_quorums = Arc::make_mut(&mut shared_quorums); @@ -165,42 +176,36 @@ impl MasternodeList { ); } - // Create and return the new MasternodeList - let builder = MasternodeList::build( + // Create the new MasternodeList and return it once the coinbase vouches for it + let masternode_list = MasternodeList::build( updated_masternodes, shared_quorums, diff.block_hash, diff_end_height, - ); + ) + .build_matching_coinbase(&coinbase_tx)?; - Ok((builder.build(), rotating_sig)) + Ok((masternode_list, rotating_sig)) } } #[cfg(test)] mod tests { use super::*; - use crate::consensus::deserialize; use crate::sml::masternode_list::from_diff::TryFromWithBlockHashLookup; #[test] fn apply_diff_post_v20_requires_chainlock_signatures() { // Create base list from first diff - let base_diff_bytes: &[u8] = - include_bytes!("../../../tests/data/test_DML_diffs/mn_list_diff_0_2227096.bin"); - let base_diff: MnListDiff = deserialize(base_diff_bytes).expect("expected to deserialize"); - let base_list = MasternodeList::try_from_with_block_hash_lookup( - base_diff, + MnListDiff::mainnet_fixture_0_2227096(), |_| Some(2_227_096), Network::Mainnet, ) .expect("expected to create base list"); // Load second diff and clear signatures - let diff_bytes: &[u8] = - include_bytes!("../../../tests/data/test_DML_diffs/mn_list_diff_2227096_2241332.bin"); - let mut diff: MnListDiff = deserialize(diff_bytes).expect("expected to deserialize"); + let mut diff = MnListDiff::mainnet_fixture_2227096_2241332(); diff.quorums_chainlock_signatures.clear(); // Height 2241332 is post-V20 on mainnet (1,987,776) @@ -218,22 +223,16 @@ mod tests { #[test] fn apply_diff_pre_v20_allows_missing_chainlock_signatures() { // Create base list from first diff at pre-V20 height - let base_diff_bytes: &[u8] = - include_bytes!("../../../tests/data/test_DML_diffs/mn_list_diff_0_2227096.bin"); - let base_diff: MnListDiff = deserialize(base_diff_bytes).expect("expected to deserialize"); - let base_height = 1_800_000u32; let base_list = MasternodeList::try_from_with_block_hash_lookup( - base_diff, + MnListDiff::mainnet_fixture_0_2227096(), |_| Some(base_height), Network::Mainnet, ) .expect("expected to create base list"); // Load second diff and clear signatures - let diff_bytes: &[u8] = - include_bytes!("../../../tests/data/test_DML_diffs/mn_list_diff_2227096_2241332.bin"); - let mut diff: MnListDiff = deserialize(diff_bytes).expect("expected to deserialize"); + let mut diff = MnListDiff::mainnet_fixture_2227096_2241332(); // Fix base_block_hash to match our base list diff.base_block_hash = base_list.block_hash; diff --git a/dash/src/sml/masternode_list/builder.rs b/dash/src/sml/masternode_list/builder.rs index ba44adf8d..719401309 100644 --- a/dash/src/sml/masternode_list/builder.rs +++ b/dash/src/sml/masternode_list/builder.rs @@ -1,8 +1,9 @@ use std::sync::Arc; -use crate::BlockHash; use crate::hash_types::{MerkleRootMasternodeList, MerkleRootQuorums}; +use crate::sml::error::SmlError; use crate::sml::masternode_list::{MasternodeList, MasternodeMap, QuorumMap}; +use crate::{BlockHash, Transaction}; pub struct MasternodeListBuilder { pub block_hash: BlockHash, @@ -40,6 +41,28 @@ impl MasternodeListBuilder { self } + /// Builds the list once it matches the commitments of `coinbase_transaction`, see + /// [`MasternodeList::verify_coinbase_merkle_roots`]. The roots are computed once, for the + /// check and for the list, which then holds the ones the coinbase commits to. + pub(crate) fn build_matching_coinbase( + self, + coinbase_transaction: &Transaction, + ) -> Result { + let mut list = MasternodeList { + block_hash: self.block_hash, + known_height: self.block_height, + masternode_merkle_root: None, + llmq_merkle_root: None, + masternodes: self.masternodes, + quorums: self.quorums, + }; + let (masternode_root, quorum_root) = list.coinbase_merkle_roots(); + list.check_coinbase_merkle_roots(coinbase_transaction, masternode_root, quorum_root)?; + list.masternode_merkle_root = Some(masternode_root); + list.llmq_merkle_root = Some(quorum_root); + Ok(list) + } + pub fn build(self) -> MasternodeList { let mut list = MasternodeList { block_hash: self.block_hash, diff --git a/dash/src/sml/masternode_list/from_diff.rs b/dash/src/sml/masternode_list/from_diff.rs index fb0acbc10..6ad6d2e98 100644 --- a/dash/src/sml/masternode_list/from_diff.rs +++ b/dash/src/sml/masternode_list/from_diff.rs @@ -144,29 +144,19 @@ impl TryFromWithBlockHashLookup for MasternodeList { }, ); - // Construct `MasternodeList` - Ok(MasternodeList { - block_hash: diff.block_hash, - known_height, - masternode_merkle_root: diff.merkle_hashes.first().cloned(), - llmq_merkle_root: None, // Adjust based on real data availability - masternodes: masternodes.into(), - quorums: quorums.into(), - }) + // Construct the `MasternodeList` and return it once the coinbase vouches for it + MasternodeList::build(masternodes, quorums, diff.block_hash, known_height) + .build_matching_coinbase(&diff.coinbase_tx) } } #[cfg(test)] mod tests { use super::*; - use crate::consensus::deserialize; #[test] fn post_v20_requires_chainlock_signatures() { - let mn_list_diff_bytes: &[u8] = - include_bytes!("../../../tests/data/test_DML_diffs/mn_list_diff_0_2227096.bin"); - let mut diff: MnListDiff = - deserialize(mn_list_diff_bytes).expect("expected to deserialize"); + let mut diff = MnListDiff::mainnet_fixture_0_2227096(); // Clear signatures to simulate missing data diff.quorums_chainlock_signatures.clear(); @@ -189,10 +179,7 @@ mod tests { #[test] fn pre_v20_allows_missing_chainlock_signatures() { - let mn_list_diff_bytes: &[u8] = - include_bytes!("../../../tests/data/test_DML_diffs/mn_list_diff_0_2227096.bin"); - let mut diff: MnListDiff = - deserialize(mn_list_diff_bytes).expect("expected to deserialize"); + let mut diff = MnListDiff::mainnet_fixture_0_2227096(); // Clear signatures to simulate pre-V20 data diff.quorums_chainlock_signatures.clear(); diff --git a/dash/src/sml/masternode_list/merkle_roots.rs b/dash/src/sml/masternode_list/merkle_roots.rs index ba12c7680..38157faed 100644 --- a/dash/src/sml/masternode_list/merkle_roots.rs +++ b/dash/src/sml/masternode_list/merkle_roots.rs @@ -2,9 +2,14 @@ use hashes::{Hash, sha256d}; use crate::Transaction; use crate::hash_types::{MerkleRootMasternodeList, MerkleRootQuorums}; +use crate::sml::error::SmlError; use crate::sml::masternode_list::MasternodeList; use crate::transaction::special_transaction::TransactionPayload; +/// First coinbase payload version that carries `merkleRootQuorums`, Dash Core's +/// `CCbTx::Version::MERKLE_ROOT_QUORUMS`. Every version carries `merkleRootMNList`. +const COINBASE_PAYLOAD_VERSION_MERKLE_ROOT_QUORUMS: u16 = 2; + /// Computes the Merkle root from a list of hashes. /// /// This function constructs a Merkle tree from the provided vector of 32-byte hashes. @@ -44,6 +49,77 @@ pub fn merkle_root_from_hashes(hashes: Vec) -> Option Result<(), SmlError> { + let (masternode_root, quorum_root) = self.coinbase_merkle_roots(); + self.check_coinbase_merkle_roots(coinbase_transaction, masternode_root, quorum_root) + } + + /// [`Self::verify_coinbase_merkle_roots`] with this list's roots already computed by + /// [`Self::coinbase_merkle_roots`]. + pub(crate) fn check_coinbase_merkle_roots( + &self, + coinbase_transaction: &Transaction, + masternode_root: MerkleRootMasternodeList, + quorum_root: MerkleRootQuorums, + ) -> Result<(), SmlError> { + let Some(TransactionPayload::CoinbasePayloadType(coinbase_payload)) = + &coinbase_transaction.special_transaction_payload + else { + return Err(SmlError::MissingCoinbasePayload(self.block_hash)); + }; + + if masternode_root != coinbase_payload.merkle_root_masternode_list { + return Err(SmlError::MasternodeListMerkleRootMismatch { + block_hash: self.block_hash, + expected: coinbase_payload.merkle_root_masternode_list, + computed: masternode_root, + }); + } + if coinbase_payload.version >= COINBASE_PAYLOAD_VERSION_MERKLE_ROOT_QUORUMS + && quorum_root != coinbase_payload.merkle_root_quorums + { + return Err(SmlError::QuorumMerkleRootMismatch { + block_hash: self.block_hash, + expected: coinbase_payload.merkle_root_quorums, + computed: quorum_root, + }); + } + + Ok(()) + } + + /// The `merkleRootMNList` and `merkleRootQuorums` a coinbase commits to for this list, with + /// the all-zero root for an empty set. See [`Self::verify_coinbase_merkle_roots`]. + pub(crate) fn coinbase_merkle_roots(&self) -> (MerkleRootMasternodeList, MerkleRootQuorums) { + let masternode_root = merkle_root_from_hashes(self.sorted_masternode_entry_hashes()) + .unwrap_or_else(sha256d::Hash::all_zeros); + let quorum_root = merkle_root_from_hashes(self.hashes_for_quorum_merkle_root()) + .unwrap_or_else(sha256d::Hash::all_zeros); + ( + MerkleRootMasternodeList::from_raw_hash(masternode_root), + MerkleRootQuorums::from_raw_hash(quorum_root), + ) + } + /// Validates whether the stored masternode list Merkle root matches the one in the coinbase transaction. /// /// This function compares the calculated masternode Merkle root with the one provided @@ -156,14 +232,15 @@ impl MasternodeList { /// - `Some(Vec)`: A sorted list of masternode entry hashes. /// - `None`: If the block height is invalid (`u32::MAX`). pub fn hashes_for_merkle_root(&self, block_height: u32) -> Option> { - (block_height != u32::MAX).then_some({ - let mut pro_tx_hashes = self.reversed_pro_reg_tx_hashes(); - pro_tx_hashes.sort_by_key(|&s| s.reverse()); - pro_tx_hashes - .into_iter() - .map(|hash| self.masternodes[hash].entry_hash) - .collect::>() - }) + (block_height != u32::MAX).then(|| self.sorted_masternode_entry_hashes()) + } + + /// The entry hashes ordered by ProRegTx hash, the leaves of `merkleRootMNList`. Core sorts + /// with `uint256::Compare`, a byte-wise comparison of the hash as serialized. + fn sorted_masternode_entry_hashes(&self) -> Vec { + let mut pro_tx_hashes = self.reversed_pro_reg_tx_hashes(); + pro_tx_hashes.sort_by_key(|&s| s.reverse()); + pro_tx_hashes.into_iter().map(|hash| self.masternodes[hash].entry_hash).collect() } /// Retrieves the list of hashes required to compute the quorum Merkle root. @@ -184,3 +261,341 @@ impl MasternodeList { llmq_commitment_hashes } } + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + use std::net::{IpAddr, Ipv4Addr, SocketAddr}; + + use super::*; + use crate::bls_sig_utils::BLSPublicKey; + use crate::consensus::deserialize; + use crate::network::constants::NetworkExt; + use crate::network::message::{NetworkMessage, RawNetworkMessage}; + use crate::network::message_qrinfo::QRInfo; + use crate::network::message_sml::MnListDiff; + use crate::sml::masternode_list::from_diff::TryFromWithBlockHashLookup; + use crate::sml::masternode_list_engine::qr_info_diffs; + use crate::sml::masternode_list_entry::MasternodeNetInfo; + use crate::transaction::special_transaction::coinbase::CoinbasePayload; + use crate::{BlockHash, Network}; + + fn mainnet_diff_0_2221605() -> MnListDiff { + let hex = include_str!("../../../tests/data/test_DML_diffs/DML_0_2221605.hex"); + let message: RawNetworkMessage = + deserialize(&hex::decode(hex).expect("hex")).expect("raw message"); + let NetworkMessage::MnListDiff(diff) = message.payload else { + panic!("expected an mnlistdiff message"); + }; + diff + } + + fn mainnet_qr_info_0_2224359() -> QRInfo { + let hex = include_str!("../../../tests/data/test_DML_diffs/QR_INFO_0_2224359.hex"); + let message: RawNetworkMessage = + deserialize(&hex::decode(hex).expect("hex")).expect("raw message"); + let NetworkMessage::QRInfo(qr_info) = message.payload else { + panic!("expected a qrinfo message"); + }; + qr_info + } + + fn list_from(diff: MnListDiff) -> Result { + MasternodeList::try_from_with_block_hash_lookup(diff, |_| Some(2_221_605), Network::Mainnet) + } + + fn coinbase_payload(diff: &mut MnListDiff) -> &mut CoinbasePayload { + match &mut diff.coinbase_tx.special_transaction_payload { + Some(TransactionPayload::CoinbasePayloadType(payload)) => payload, + _ => panic!("the fixture's coinbase carries a payload"), + } + } + + /// Applies `diffs` in whatever order their bases allow, from genesis or + /// from a list an earlier one built, and returns how many applied. Every + /// diff has to apply: the heights are left at 0, which only relaxes the + /// post-V20 signature requirement, and no height enters the roots. + fn apply_every_diff(label: &str, diffs: Vec, network: Network) -> usize { + let genesis = network.known_genesis_block_hash(); + let mut lists: HashMap = HashMap::new(); + let mut pending = diffs; + let mut applied = 0; + while !pending.is_empty() { + let before = pending.len(); + let mut waiting = Vec::new(); + for diff in pending { + let block_hash = diff.block_hash; + let result = if let Some(base) = lists.get(&diff.base_block_hash) { + base.apply_diff(diff, 0, None, network).map(|(list, _)| list) + } else if diff.base_block_hash == BlockHash::all_zeros() + || Some(diff.base_block_hash) == genesis + { + MasternodeList::try_from_with_block_hash_lookup(diff, |_| Some(0), network) + } else { + waiting.push(diff); + continue; + }; + let list = result.unwrap_or_else(|e| panic!("{label}: {block_hash}: {e}")); + lists.insert(block_hash, list); + applied += 1; + } + assert!(waiting.len() < before, "{label}: {} diffs have no base", waiting.len()); + pending = waiting; + } + applied + } + + /// Every masternode list diff committed to the repository builds a list + /// that matches the coinbase commitments of its block. A false refusal + /// would stall SPV sync, so this sweeps them all: mainnet, testnet and a + /// Core 23.1 devnet with ProTx v3 entries, raw captures and QRInfo + /// payloads alike. + /// + /// Three captures are brought back to Core's bytes first, see + /// [`MnListDiff::restore_core_service_addresses`] and + /// [`MnListDiff::reverse_platform_node_ids`]. Their roots match only after + /// that, which pins down those fields as the only difference. + #[test] + fn every_fixture_diff_matches_its_coinbase() { + let mut applied = 0; + + applied += + apply_every_diff("DML_0_2221605", vec![mainnet_diff_0_2221605()], Network::Mainnet); + + let qr_info = mainnet_qr_info_0_2224359(); + applied += apply_every_diff( + "QR_INFO_0_2224359", + qr_info_diffs(&qr_info).into_iter().cloned().collect(), + Network::Mainnet, + ); + + let paloma = include_str!("../../../tests/data/test_DML_diffs/qrinfo_core231_paloma.hex"); + let paloma: QRInfo = + deserialize(&hex::decode(paloma.trim()).expect("hex")).expect("qrinfo"); + applied += apply_every_diff( + "qrinfo_core231_paloma", + qr_info_diffs(&paloma).into_iter().cloned().collect(), + Network::Devnet, + ); + + applied += apply_every_diff( + "mn_list_diff_0_2227096 and _2227096_2241332", + vec![ + MnListDiff::mainnet_fixture_0_2227096(), + MnListDiff::mainnet_fixture_2227096_2241332(), + ], + Network::Mainnet, + ); + + applied += apply_every_diff( + "mn_list_diff_testnet_0_1296600", + vec![MnListDiff::testnet_fixture_0_1296600()], + Network::Testnet, + ); + + #[cfg(feature = "bincode")] + { + use std::collections::BTreeMap; + + fn decode>(bytes: &[u8]) -> T { + bincode::decode_from_slice(bytes, bincode::config::standard()) + .expect("fixture decodes") + .0 + } + + let qr_info: QRInfo = + decode(include_bytes!("../../../tests/data/test_DML_diffs/qrinfo_2518986.dat")); + applied += apply_every_diff( + "qrinfo_2518986", + qr_info_diffs(&qr_info).into_iter().cloned().collect(), + Network::Mainnet, + ); + + let diffs: BTreeMap<(u32, u32), MnListDiff> = decode(include_bytes!( + "../../../tests/data/test_DML_diffs/mnlistdiffs_2240504.dat" + )); + let qr_info: QRInfo = + decode(include_bytes!("../../../tests/data/test_DML_diffs/qrinfo_2240504.dat")); + let mut diffs: Vec = + diffs.into_values().chain(qr_info_diffs(&qr_info).into_iter().cloned()).collect(); + for diff in &mut diffs { + diff.restore_core_service_addresses(); + diff.reverse_platform_node_ids(); + } + applied += + apply_every_diff("mnlistdiffs_2240504 and qrinfo_2240504", diffs, Network::Mainnet); + } + + #[cfg(feature = "bincode")] + assert_eq!(applied, 59, "a fixture diff was skipped"); + #[cfg(not(feature = "bincode"))] + assert_eq!(applied, 15, "a fixture diff was skipped"); + } + + /// Without the restore, the captures that wrote service addresses in + /// another form are refused, which is what makes the restore necessary. + #[test] + fn a_capture_with_non_wire_service_addresses_is_refused() { + let raw: MnListDiff = deserialize(include_bytes!( + "../../../tests/data/test_DML_diffs/mn_list_diff_0_2227096.bin" + )) + .expect("fixture decodes"); + assert!(matches!( + MasternodeList::try_from_with_block_hash_lookup( + raw, + |_| Some(2_227_096), + Network::Mainnet + ), + Err(SmlError::MasternodeListMerkleRootMismatch { .. }) + )); + } + + #[test] + fn a_list_built_from_a_diff_holds_the_roots_its_coinbase_commits_to() { + let mut diff = mainnet_diff_0_2221605(); + let payload = coinbase_payload(&mut diff).clone(); + let list = list_from(diff).expect("the diff applies"); + assert_eq!(list.masternode_merkle_root, Some(payload.merkle_root_masternode_list)); + assert_eq!(list.llmq_merkle_root, Some(payload.merkle_root_quorums)); + } + + #[test] + fn a_full_diff_with_an_edited_masternode_entry_is_refused() { + fn first_ipv4(diff: &MnListDiff) -> usize { + diff.new_masternodes + .iter() + .position(|entry| { + matches!(entry.service_address, MasternodeNetInfo::Legacy(SocketAddr::V4(_))) + }) + .expect("the fixture has IPv4 entries") + } + type Edit = (&'static str, fn(&mut MnListDiff)); + let edits: [Edit; 4] = [ + ("service IP", |diff| { + let index = first_ipv4(diff); + let MasternodeNetInfo::Legacy(address) = + &mut diff.new_masternodes[index].service_address + else { + unreachable!("picked a legacy address"); + }; + address.set_ip(IpAddr::V4(Ipv4Addr::new(203, 0, 113, 7))); + }), + ("validity flag", |diff| { + diff.new_masternodes[0].is_valid = !diff.new_masternodes[0].is_valid; + }), + ("operator key", |diff| { + diff.new_masternodes[0].operator_public_key = BLSPublicKey::from([7; 48]); + }), + ("dropped entry", |diff| { + diff.new_masternodes.remove(0); + }), + ]; + + assert!(list_from(mainnet_diff_0_2221605()).is_ok(), "the untouched diff applies"); + for (edit, apply_edit) in edits { + let mut diff = mainnet_diff_0_2221605(); + apply_edit(&mut diff); + assert!( + matches!(list_from(diff), Err(SmlError::MasternodeListMerkleRootMismatch { .. })), + "an edited {edit} must be refused" + ); + } + } + + #[test] + fn a_full_diff_with_an_edited_quorum_is_refused() { + let mut diff = mainnet_diff_0_2221605(); + diff.new_quorums[0].quorum_vvec_hash = crate::hash_types::QuorumVVecHash::all_zeros(); + assert!(matches!(list_from(diff), Err(SmlError::QuorumMerkleRootMismatch { .. }))); + + // A dropped commitment also drops its slot from the signature groups, + // so the signature checks still pass and the root is what refuses it. + let mut diff = mainnet_diff_0_2221605(); + let last = (diff.new_quorums.len() - 1) as u16; + diff.new_quorums.pop(); + for group in &mut diff.quorums_chainlock_signatures { + group.index_set.retain(|index| *index != last); + } + assert!(matches!(list_from(diff), Err(SmlError::QuorumMerkleRootMismatch { .. }))); + } + + #[test] + fn a_diff_carrying_another_blocks_coinbase_is_refused() { + let mut diff = mainnet_diff_0_2221605(); + diff.coinbase_tx = mainnet_qr_info_0_2224359().mn_list_diff_tip.coinbase_tx; + assert!(matches!(list_from(diff), Err(SmlError::MasternodeListMerkleRootMismatch { .. }))); + + let mut diff = mainnet_diff_0_2221605(); + diff.coinbase_tx.special_transaction_payload = None; + assert!(matches!(list_from(diff), Err(SmlError::MissingCoinbasePayload(_)))); + } + + /// Edits to a diff applied on a base list: a deletion the diff leaves + /// out keeps an entry the coinbase does not count. + #[test] + fn an_applied_diff_that_leaves_out_a_change_is_refused() { + let base = MasternodeList::try_from_with_block_hash_lookup( + MnListDiff::mainnet_fixture_0_2227096(), + |_| Some(2_227_096), + Network::Mainnet, + ) + .expect("base list"); + let apply = |diff: MnListDiff| base.apply_diff(diff, 2_241_332, None, Network::Mainnet); + + assert!(apply(MnListDiff::mainnet_fixture_2227096_2241332()).is_ok()); + + let mut diff = MnListDiff::mainnet_fixture_2227096_2241332(); + diff.deleted_masternodes.pop().expect("the fixture deletes masternodes"); + assert!(matches!(apply(diff), Err(SmlError::MasternodeListMerkleRootMismatch { .. }))); + + let mut diff = MnListDiff::mainnet_fixture_2227096_2241332(); + diff.new_masternodes[0].is_valid = !diff.new_masternodes[0].is_valid; + assert!(matches!(apply(diff), Err(SmlError::MasternodeListMerkleRootMismatch { .. }))); + + let mut diff = MnListDiff::mainnet_fixture_2227096_2241332(); + diff.deleted_quorums.pop().expect("the fixture deletes quorums"); + assert!(matches!(apply(diff), Err(SmlError::QuorumMerkleRootMismatch { .. }))); + } + + /// Coinbase payload version 1 carries `merkleRootMNList` but no + /// `merkleRootQuorums`, so only the masternode root is compared. From + /// version 2 on both are. + #[test] + fn coinbase_payload_version_decides_which_roots_are_compared() { + let with_payload = |version: u16, masternode_root: Option| { + let mut diff = mainnet_diff_0_2221605(); + let payload = coinbase_payload(&mut diff); + *payload = CoinbasePayload { + version, + height: payload.height, + merkle_root_masternode_list: masternode_root + .unwrap_or(payload.merkle_root_masternode_list), + merkle_root_quorums: MerkleRootQuorums::all_zeros(), + best_cl_height: None, + best_cl_signature: None, + asset_locked_amount: None, + }; + list_from(diff) + }; + let wrong_root = Some(MerkleRootMasternodeList::from_byte_array([7; 32])); + + assert!(with_payload(1, None).is_ok(), "version 1 has no quorum root to compare"); + assert!(matches!( + with_payload(1, wrong_root), + Err(SmlError::MasternodeListMerkleRootMismatch { .. }) + )); + assert!(matches!(with_payload(2, None), Err(SmlError::QuorumMerkleRootMismatch { .. }))); + } + + /// Core's `ComputeMerkleRoot` gives an empty set the all-zero root. + #[test] + fn an_empty_list_matches_all_zero_roots() { + let list = MasternodeList::empty(BlockHash::all_zeros(), 1); + let empty = MnListDiff::dummy_between(0, 1); + list.verify_coinbase_merkle_roots(&empty.coinbase_tx).expect("zero roots match"); + assert_eq!( + list.coinbase_merkle_roots(), + (MerkleRootMasternodeList::all_zeros(), MerkleRootQuorums::all_zeros()) + ); + } +} diff --git a/dash/src/sml/masternode_list_engine/helpers.rs b/dash/src/sml/masternode_list_engine/helpers.rs index 25b20d453..0db5fcf42 100644 --- a/dash/src/sml/masternode_list_engine/helpers.rs +++ b/dash/src/sml/masternode_list_engine/helpers.rs @@ -394,6 +394,7 @@ mod shared_map_tests { use crate::sml::masternode_list_engine::MasternodeListEngine; use crate::{BlockHash, QuorumHash}; use hashes::Hash; + use std::slice; use std::sync::Arc; const TIP: CoreBlockHeight = 1_000_000; @@ -441,14 +442,17 @@ mod shared_map_tests { let mut engine = MasternodeListEngine::dummy_with_lists(&[TIP - 2]); let mut quorum = quorum_entry(quorum_hash, 1); quorum.verified = LLMQEntryVerificationStatus::Unknown; + let commitment = quorum.quorum_entry.clone(); Arc::make_mut(&mut engine.masternode_lists.get_mut(&(TIP - 2)).unwrap().quorums) .entry(PLATFORM_TYPE) .or_default() .insert(quorum_hash, Arc::new(quorum)); - engine - .apply_diff(MnListDiff::dummy_between(TIP - 2, TIP - 1), Some(TIP - 1), false, None) - .unwrap(); - engine.apply_diff(MnListDiff::dummy_between(TIP - 1, TIP), Some(TIP), false, None).unwrap(); + let unchanged = |base, tip| { + MnListDiff::dummy_between(base, tip) + .with_coinbase_committing_to(&[], slice::from_ref(&commitment)) + }; + engine.apply_diff(unchanged(TIP - 2, TIP - 1), Some(TIP - 1), false, None).unwrap(); + engine.apply_diff(unchanged(TIP - 1, TIP), Some(TIP), false, None).unwrap(); engine.set_quorum_status_in_lists( [TIP - 2, TIP - 1, TIP], diff --git a/dash/src/sml/masternode_list_engine/mod.rs b/dash/src/sml/masternode_list_engine/mod.rs index d20d8ecb6..091f108f4 100644 --- a/dash/src/sml/masternode_list_engine/mod.rs +++ b/dash/src/sml/masternode_list_engine/mod.rs @@ -1107,6 +1107,9 @@ impl MasternodeListEngine { /// surface as `QuorumValidationError::RequiredBlockNotPresent` once they are /// actually needed. /// + /// Every diff is applied through [`Self::apply_diff`] and so checked against + /// the coinbase commitments of its block. + /// /// # Parameters /// - `qr_info`: The QRInfo message containing quorum snapshots and diffs /// - `verify_tip_non_rotated_quorums`: Whether to verify non-rotating quorums at the tip @@ -1473,6 +1476,11 @@ impl MasternodeListEngine { /// Applies a masternode list diff to create or update a masternode list. /// + /// The list the diff builds has to match the coinbase commitments of the + /// diff's block, see [`MasternodeList::verify_coinbase_merkle_roots`]. Tying + /// that coinbase to the block header is up to the caller, who holds the + /// header: see [`MnListDiff::verify_coinbase_merkle_proof`]. + /// /// A diff built on the newest list moves the tip and prunes the lists that /// fall out of the retention window. Other diffs may share an older base, so /// they prune nothing. A diff requested from a list older than the window @@ -1757,13 +1765,13 @@ impl MasternodeListEngine { #[cfg(test)] mod tests { use crate::Network; - use crate::consensus::deserialize; use crate::hashes::Hash; use crate::network::message_qrinfo::QRInfo; use crate::network::message_sml::MnListDiff; #[cfg(feature = "quorum_validation")] use crate::network::message_sml::QuorumCLSigObject; use crate::prelude::CoreBlockHeight; + use crate::sml::error::SmlError; use crate::sml::llmq_entry_verification::{ LLMQEntryVerificationSkipStatus, LLMQEntryVerificationStatus, }; @@ -1798,6 +1806,8 @@ mod tests { crate::QuorumHash, crate::bls_sig_utils::{BLSPublicKey, BLSSignature}, crate::hash_types::QuorumVVecHash, + crate::sml::masternode_list_entry::MasternodeListEntry, + crate::transaction::special_transaction::TransactionPayload, crate::transaction::special_transaction::quorum_commitment::QuorumEntry, }; @@ -1837,9 +1847,7 @@ mod tests { new_quorums: Vec, groups: Vec<(BLSSignature, Vec)>, ) -> MnListDiff { - let diff_bytes: &[u8] = - include_bytes!("../../../tests/data/test_DML_diffs/mn_list_diff_0_2227096.bin"); - let mut diff: MnListDiff = deserialize(diff_bytes).expect("expected to deserialize"); + let mut diff = MnListDiff::mainnet_fixture_0_2227096(); diff.block_hash = end_hash; diff.new_quorums = new_quorums; diff.quorums_chainlock_signatures = groups @@ -2130,19 +2138,14 @@ mod tests { #[test] fn validate_from_mn_list_diff_chain_locks() { - let mn_list_diff_bytes: &[u8] = - include_bytes!("../../../tests/data/test_DML_diffs/mn_list_diff_0_2227096.bin"); - // This one is serialized not with bincode, but with core consensus - let diff: MnListDiff = deserialize(mn_list_diff_bytes).expect("expected to deserialize"); - let mut masternode_list_engine = - MasternodeListEngine::initialize_with_diff_to_height(diff, 2227096, Network::Mainnet) - .expect("expected to start engine"); - - let mn_list_diff_bytes_2: &[u8] = - include_bytes!("../../../tests/data/test_DML_diffs/mn_list_diff_2227096_2241332.bin"); - // This one is serialized not with bincode, but with core consensus - let diff_2: MnListDiff = - deserialize(mn_list_diff_bytes_2).expect("expected to deserialize"); + let mut masternode_list_engine = MasternodeListEngine::initialize_with_diff_to_height( + MnListDiff::mainnet_fixture_0_2227096(), + 2227096, + Network::Mainnet, + ) + .expect("expected to start engine"); + + let diff_2 = MnListDiff::mainnet_fixture_2227096_2241332(); masternode_list_engine .apply_diff(diff_2, Some(2241332), false, None) @@ -2220,12 +2223,12 @@ mod tests { #[cfg(feature = "quorum_validation")] fn load_qrinfo_2240504_fixture() -> (MasternodeListEngine, QRInfo) { - let mn_list_diff_bytes: &[u8] = - include_bytes!("../../../tests/data/test_DML_diffs/mn_list_diff_0_2227096.bin"); - let diff: MnListDiff = deserialize(mn_list_diff_bytes).expect("expected to deserialize"); - let mut engine = - MasternodeListEngine::initialize_with_diff_to_height(diff, 2227096, Network::Mainnet) - .expect("expected to start engine"); + let mut engine = MasternodeListEngine::initialize_with_diff_to_height( + MnListDiff::mainnet_fixture_0_2227096(), + 2227096, + Network::Mainnet, + ) + .expect("expected to start engine"); let block_container_bytes: &[u8] = include_bytes!("../../../tests/data/test_DML_diffs/block_container_2240504.dat"); @@ -2237,16 +2240,74 @@ mod tests { decode_fixture(mn_list_diffs_bytes); let qr_info_bytes: &[u8] = include_bytes!("../../../tests/data/test_DML_diffs/qrinfo_2240504.dat"); - let qr_info: QRInfo = decode_fixture(qr_info_bytes); + let mut qr_info: QRInfo = decode_fixture(qr_info_bytes); + for diff in qr_info_diffs_mut(&mut qr_info) { + restore_2240504_capture(diff); + } engine.block_container = block_container; - for ((_start_height, height), diff) in mn_list_diffs.into_iter() { + for ((_start_height, height), mut diff) in mn_list_diffs.into_iter() { + restore_2240504_capture(&mut diff); engine.apply_diff(diff, Some(height), false, None).expect("expected to apply diff"); } (engine, qr_info) } + /// The 2240504 fixtures are bincode captures from a build that decoded an + /// unset service address as `0.0.0.0` and held platform node ids in Core's + /// wire order. Both feed the entry hash, so the lists they build match + /// `merkleRootMNList` only once the diff is back to what the current + /// decoder makes of Core's bytes. + #[cfg(feature = "quorum_validation")] + fn restore_2240504_capture(diff: &mut MnListDiff) { + diff.restore_core_service_addresses(); + diff.reverse_platform_node_ids(); + } + + /// Points the coinbase `merkleRootQuorums` of every diff in `qr_info` at + /// the quorum set that diff builds on the engine's lists. Tests that edit + /// commitments to reach the rotation paths use it: the coinbase check + /// refuses an edited diff before those paths run. + #[cfg(feature = "quorum_validation")] + fn recommit_quorum_roots(engine: &MasternodeListEngine, qr_info: &mut QRInfo) { + for diff in qr_info_diffs_mut(qr_info) { + let base = engine + .masternode_list_for_block_hash(&diff.base_block_hash) + .expect("every diff of the fixture builds on a list the engine holds"); + let computed = match base.apply_diff(diff.clone(), 0, None, engine.network) { + Ok(_) => continue, + Err(SmlError::QuorumMerkleRootMismatch { + computed, + .. + }) => computed, + Err(e) => panic!("the edit must only move the quorum root: {e}"), + }; + let Some(TransactionPayload::CoinbasePayloadType(payload)) = + &mut diff.coinbase_tx.special_transaction_payload + else { + panic!("the fixture's coinbase carries a payload"); + }; + payload.merkle_root_quorums = computed; + } + } + + #[cfg(feature = "quorum_validation")] + fn qr_info_diffs_mut(qr_info: &mut QRInfo) -> Vec<&mut MnListDiff> { + let mut diffs: Vec<&mut MnListDiff> = vec![ + &mut qr_info.mn_list_diff_tip, + &mut qr_info.mn_list_diff_h, + &mut qr_info.mn_list_diff_at_h_minus_c, + &mut qr_info.mn_list_diff_at_h_minus_2c, + &mut qr_info.mn_list_diff_at_h_minus_3c, + ]; + if let Some((_, diff)) = &mut qr_info.quorum_snapshot_and_mn_list_diff_at_h_minus_4c { + diffs.push(diff); + } + diffs.extend(qr_info.mn_list_diff_list.iter_mut()); + diffs + } + /// Captured 2026-08-09 from a fresh mainnet sync at tip 2518986 (cycle /// boundary 2518848). The active rotation set at that state carried a /// quorum whose latest commitment came from an older cycle, so the @@ -2271,6 +2332,71 @@ mod tests { (engine, qr_info) } + /// A diff whose resulting list does not match its coinbase is refused + /// whether it starts the engine, extends a list or arrives in a QRInfo, + /// and no list is stored for its block. + #[test] + fn a_diff_that_does_not_match_its_coinbase_is_not_applied() { + let tampered = |mut diff: MnListDiff| { + diff.new_masternodes[0].is_valid = !diff.new_masternodes[0].is_valid; + diff + }; + fn refused(result: Result) -> bool { + matches!(result, Err(SmlError::MasternodeListMerkleRootMismatch { .. })) + } + + assert!(refused(MasternodeListEngine::initialize_with_diff_to_height( + tampered(MnListDiff::mainnet_fixture_0_2227096()), + 2227096, + Network::Mainnet, + ))); + + let mut engine = MasternodeListEngine::default_for_network(Network::Mainnet); + assert!(refused(engine.apply_diff( + tampered(MnListDiff::mainnet_fixture_0_2227096()), + Some(2227096), + false, + None, + ))); + assert!(engine.masternode_lists.is_empty()); + + engine + .apply_diff(MnListDiff::mainnet_fixture_0_2227096(), Some(2227096), false, None) + .expect("the untouched diff applies"); + assert!(refused(engine.apply_diff( + tampered(MnListDiff::mainnet_fixture_2227096_2241332()), + Some(2241332), + false, + None, + ))); + assert!(!engine.masternode_lists.contains_key(&2241332)); + } + + #[test] + #[cfg(feature = "quorum_validation")] + fn a_qr_info_diff_that_does_not_match_its_coinbase_fails_the_feed() { + let (_, qr_info) = load_qrinfo_2518986_fixture(); + let diff_count = qr_info_diffs(&qr_info).len(); + assert!(diff_count >= 5, "the fixture carries every QRInfo diff"); + + for position in 0..diff_count { + let (mut engine, mut qr_info) = load_qrinfo_2518986_fixture(); + qr_info_diffs_mut(&mut qr_info)[position] + .new_masternodes + .push(MasternodeListEntry::dummy(0x11)); + let result = engine.feed_qr_info(qr_info, true, true); + assert!( + matches!( + result, + Err(QuorumValidationError::SMLError( + SmlError::MasternodeListMerkleRootMismatch { .. } + )) + ), + "diff {position} of the QRInfo: {result:?}" + ); + } + } + #[test] #[cfg(feature = "quorum_validation")] fn validate_first_qr_info_on_fresh_engine_with_mixed_cycle_quorums() { @@ -2594,6 +2720,7 @@ mod tests { for diff in qr_info.mn_list_diff_list.iter_mut() { strip(diff); } + recommit_quorum_roots(&engine, &mut qr_info); let entries: Vec = qr_info.last_commitment_per_index.iter().cloned().map(Into::into).collect(); @@ -2820,6 +2947,7 @@ mod tests { .all_commitment_aggregated_signature; qr_info.mn_list_diff_h.new_quorums[corrupt_position].all_commitment_aggregated_signature = foreign_signature; + recommit_quorum_roots(&engine, &mut qr_info); let h_block_hash = qr_info.mn_list_diff_h.block_hash; // The captured container has no height for the commitments of the @@ -2894,6 +3022,7 @@ mod tests { let (mut engine, mut qr_info) = load_qrinfo_2240504_fixture(); qr_info.mn_list_diff_h.new_quorums[corrupt_position].all_commitment_aggregated_signature = foreign_signature; + recommit_quorum_roots(&engine, &mut qr_info); let feed_result = engine .feed_qr_info(qr_info, true, true) .expect("previous-cycle corruption must not abort the feed") diff --git a/dash/src/test_utils/sml.rs b/dash/src/test_utils/sml.rs index 957f50991..9506105c4 100644 --- a/dash/src/test_utils/sml.rs +++ b/dash/src/test_utils/sml.rs @@ -1,17 +1,26 @@ -use std::net::SocketAddr; +use std::collections::BTreeMap; +use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV6}; +use std::sync::Arc; use hashes::Hash; -use crate::bls_sig_utils::BLSPublicKey; +use crate::bls_sig_utils::{BLSPublicKey, BLSSignature}; +use crate::consensus::deserialize; use crate::hash_types::{MerkleRootMasternodeList, ProTxHash}; use crate::network::message_qrinfo::{MNSkipListMode, QRInfo, QuorumSnapshot}; use crate::network::message_sml::MnListDiff; -use crate::sml::masternode_list::MasternodeList; +use crate::sml::masternode_list::{MasternodeList, QuorumMap}; use crate::sml::masternode_list_engine::MasternodeListEngine; use crate::sml::masternode_list_entry::{ EntryMasternodeType, MasternodeListEntry, MasternodeNetInfo, }; -use crate::{BlockHash, Network, PubkeyHash, Transaction}; +use crate::transaction::special_transaction::TransactionPayload; +use crate::transaction::special_transaction::coinbase::CoinbasePayload; +use crate::transaction::special_transaction::quorum_commitment::QuorumEntry; +use crate::{ + BlockHash, Network, OutPoint, PlatformNodeId, PubkeyHash, ScriptBuf, Transaction, TxIn, + TxMerkleNode, Witness, +}; fn dummy_hash(byte: u8) -> BlockHash { BlockHash::from_slice(&[byte; 32]).unwrap() @@ -36,14 +45,17 @@ impl MnListDiff { /// Carries one masternode and one merkle hash, the minimum /// [`MasternodeList`] conversion accepts. Use this when the diff has to /// apply to an engine. + /// + /// The coinbase commits to a list holding only that masternode, which is + /// what the diff builds from genesis. A diff applied on top of another + /// list builds more, and needs [`Self::with_coinbase_committing_to`]. pub fn dummy(base_byte: u8, tip_byte: u8) -> Self { + let masternode = MasternodeListEntry::dummy(tip_byte); MnListDiff { - total_transactions: 1, - merkle_hashes: vec![MerkleRootMasternodeList::from([tip_byte; 32])], - merkle_flags: vec![1], - new_masternodes: vec![MasternodeListEntry::dummy(tip_byte)], + new_masternodes: vec![masternode.clone()], ..MnListDiff::dummy_empty(base_byte, tip_byte) } + .with_coinbase_committing_to(&[masternode], &[]) } /// Hashes only. An engine rejects this as an incomplete diff, which is what @@ -66,18 +78,158 @@ impl MnListDiff { } /// An empty diff from `BlockHash::dummy(base)` to `BlockHash::dummy(tip)`, - /// which an engine holding the list at `base` applies. + /// which an engine holding an empty list at `base` applies. On any other + /// list it needs [`Self::with_coinbase_committing_to`]. pub fn dummy_between(base: u32, tip: u32) -> Self { MnListDiff { base_block_hash: BlockHash::dummy(base), block_hash: BlockHash::dummy(tip), - total_transactions: 1, - coinbase_tx: Transaction { - version: 3, - ..Transaction::dummy_empty() - }, ..MnListDiff::dummy_empty(0x00, 0x00) } + .with_coinbase_committing_to(&[], &[]) + } + + /// Replaces the coinbase with one whose payload commits to a list of + /// exactly `masternodes` and `quorums`, and the merkle proof with that of a + /// block holding only this coinbase. The diff then passes the coinbase + /// checks wherever applying it builds that list, and its proof leads to + /// [`Self::dummy_block_merkle_root`]. + pub fn with_coinbase_committing_to( + mut self, + masternodes: &[MasternodeListEntry], + quorums: &[QuorumEntry], + ) -> Self { + let masternodes = masternodes + .iter() + .map(|entry| (entry.pro_reg_tx_hash.reverse(), Arc::new(entry.clone().into()))) + .collect::>(); + let mut quorum_map = QuorumMap::new(); + for quorum in quorums { + quorum_map + .entry(quorum.llmq_type) + .or_default() + .insert(quorum.quorum_hash, Arc::new(quorum.clone().into())); + } + let (masternode_root, quorum_root) = + MasternodeList::build(masternodes, quorum_map, self.block_hash, 0) + .build() + .coinbase_merkle_roots(); + + self.coinbase_tx = Transaction { + version: 3, + lock_time: 0, + input: vec![TxIn { + previous_output: OutPoint::null(), + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: Witness::new(), + }], + output: vec![], + special_transaction_payload: Some(TransactionPayload::CoinbasePayloadType( + CoinbasePayload::new( + 0, + masternode_root, + quorum_root, + Some(0), + Some(BLSSignature::from([0; 96])), + Some(0), + ), + )), + }; + self.total_transactions = 1; + self.merkle_hashes = + vec![MerkleRootMasternodeList::from_raw_hash(self.coinbase_tx.txid().to_raw_hash())]; + self.merkle_flags = vec![1]; + self + } + + /// The merkle root of a block holding only this diff's coinbase, which the + /// proof [`Self::with_coinbase_committing_to`] sets up leads to. + pub fn dummy_block_merkle_root(&self) -> TxMerkleNode { + TxMerkleNode::from_raw_hash(self.coinbase_tx.txid().to_raw_hash()) + } + + /// `tests/data/test_DML_diffs/mn_list_diff_0_2227096.bin`, the mainnet + /// diff from genesis to 2227096, with the service addresses restored by + /// [`Self::restore_core_service_addresses`]. + pub fn mainnet_fixture_0_2227096() -> Self { + Self::legacy_address_fixture(include_bytes!( + "../../tests/data/test_DML_diffs/mn_list_diff_0_2227096.bin" + )) + } + + /// `tests/data/test_DML_diffs/mn_list_diff_2227096_2241332.bin`, the + /// mainnet diff from 2227096 to 2241332, with the service addresses + /// restored by [`Self::restore_core_service_addresses`]. + pub fn mainnet_fixture_2227096_2241332() -> Self { + Self::legacy_address_fixture(include_bytes!( + "../../tests/data/test_DML_diffs/mn_list_diff_2227096_2241332.bin" + )) + } + + /// `artifacts/mn_list_diff_testnet_0_1296600.bin`, the testnet diff from + /// genesis to 1296600, with the service addresses restored by + /// [`Self::restore_core_service_addresses`]. + pub fn testnet_fixture_0_1296600() -> Self { + Self::legacy_address_fixture(include_bytes!( + "../../artifacts/mn_list_diff_testnet_0_1296600.bin" + )) + } + + fn legacy_address_fixture(bytes: &[u8]) -> Self { + let mut diff: MnListDiff = deserialize(bytes).expect("fixture decodes"); + diff.restore_core_service_addresses(); + diff + } + + /// Rewrites legacy service addresses into the bytes Dash Core sends: an + /// IPv4 address as IPv4-mapped IPv6 (`::ffff:a.b.c.d`) and an unset one as + /// all zeros (`::`). + /// + /// The `.bin` fixtures were written by an encoder that stored IPv4 + /// addresses as IPv4-compatible IPv6 (`::a.b.c.d`) or an unset address as + /// `::ffff:0.0.0.0`, and bincode captures made before the decoder kept + /// `::` apart from `0.0.0.0` hold unset addresses as `0.0.0.0`. The + /// address is part of the entry hash, so the lists those fixtures build + /// match `merkleRootMNList` of their coinbase only once this restores + /// Core's bytes. + pub fn restore_core_service_addresses(&mut self) { + for entry in &mut self.new_masternodes { + let MasternodeNetInfo::Legacy(address) = &mut entry.service_address else { + continue; + }; + match *address { + SocketAddr::V6(v6) => { + let octets = v6.ip().octets(); + if octets[..12] == [0; 12] && octets[12..] != [0; 4] { + let ip = Ipv4Addr::new(octets[12], octets[13], octets[14], octets[15]); + *address = SocketAddr::V4(SocketAddrV4::new(ip, v6.port())); + } + } + SocketAddr::V4(v4) if v4.ip().is_unspecified() => { + *address = + SocketAddr::V6(SocketAddrV6::new(Ipv6Addr::UNSPECIFIED, v4.port(), 0, 0)); + } + SocketAddr::V4(_) => {} + } + } + } + + /// Reverses every platform node id. Bincode captures made before + /// `PlatformNodeId` held its bytes in canonical order decode with the + /// Core wire order, which the entry hash then reverses once more. + pub fn reverse_platform_node_ids(&mut self) { + for entry in &mut self.new_masternodes { + if let EntryMasternodeType::HighPerformance { + platform_node_id, + .. + } = &mut entry.mn_type + { + let mut bytes = platform_node_id.to_byte_array(); + bytes.reverse(); + *platform_node_id = PlatformNodeId::from_byte_array(bytes); + } + } } } From daf431447960e32dae89cc3d6fcdb24f9f0c6619 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 29 Sep 2026 04:04:03 +0700 Subject: [PATCH 2/2] test(dash): a refused diff leaves the engine as it was Since #1076 the engine records every quorum of an applied list in `quorum_statuses` and verifies the newest list's quorums itself. The coinbase root check runs before either, so a diff whose list does not match its coinbase changes nothing. The test now compares the whole engine before and after the refused extension, which fails if its quorums are recorded or verified ahead of the check. Co-Authored-By: Claude Opus 5.5 --- dash/src/sml/masternode_list_engine/mod.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dash/src/sml/masternode_list_engine/mod.rs b/dash/src/sml/masternode_list_engine/mod.rs index 26e4b31f1..fbc51d9c3 100644 --- a/dash/src/sml/masternode_list_engine/mod.rs +++ b/dash/src/sml/masternode_list_engine/mod.rs @@ -2313,7 +2313,9 @@ mod tests { /// A diff whose resulting list does not match its coinbase is refused /// whether it starts the engine, extends a list or arrives in a QRInfo, - /// and no list is stored for its block. + /// and no list is stored for its block. A refused extension leaves the + /// engine as it was: its quorums are neither recorded in + /// `quorum_statuses` nor verified. #[test] fn a_diff_that_does_not_match_its_coinbase_is_not_applied() { let tampered = |mut diff: MnListDiff| { @@ -2341,12 +2343,14 @@ mod tests { engine .apply_diff(MnListDiff::mainnet_fixture_0_2227096(), Some(2227096), None) .expect("the untouched diff applies"); + let before = engine.clone(); assert!(refused(engine.apply_diff( tampered(MnListDiff::mainnet_fixture_2227096_2241332()), Some(2241332), None, ))); assert!(!engine.masternode_lists.contains_key(&2241332)); + assert!(engine == before, "a refused diff leaves the engine as it was"); } #[test]