diff --git a/dash-spv/src/storage/masternode.rs b/dash-spv/src/storage/masternode.rs index bce06660a..2abf04396 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; @@ -331,6 +339,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 @@ -378,6 +424,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; @@ -386,31 +436,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)); @@ -431,26 +503,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(); @@ -471,9 +545,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(); @@ -483,12 +558,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"); @@ -502,7 +582,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(); @@ -521,15 +601,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], @@ -557,32 +639,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(); @@ -592,19 +682,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 @@ -618,6 +707,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 8d745db1e..46f6eb664 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, }; @@ -209,8 +212,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); @@ -378,28 +391,43 @@ impl SyncManager for MasternodesManager { return Ok(vec![]); } }; - drop(storage); - // Apply diff to engine - let mut engine = self.engine.write().await; - engine.feed_block_height(target_height, diff.block_hash); + // 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); - let apply_ok = match engine.apply_diff(diff.clone(), Some(target_height), None) { - Ok(_) => { - self.sync_state.known_mn_list_heights.insert(target_height); - tracing::debug!("Applied MnListDiff at height {}", target_height); - true + 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), 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 + } + } } Err(e) => { tracing::warn!( - "Failed to apply MnListDiff at height {}: {}", + "MnListDiff at height {} does not match its block header: {}", target_height, e ); false } }; - drop(engine); if apply_ok { if let Err(e) = self.store_diff(target_height, diff).await { @@ -724,7 +752,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, }; @@ -746,7 +774,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; @@ -828,7 +856,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(); @@ -1016,7 +1045,10 @@ mod tests { DiskStorageManager::new(&ClientConfig::testnet().with_storage_path(dir.path())) .await .unwrap(); - let headers = Header::dummy_batch(0..target + 1); + let template = MnListDiff::dummy_between(0, 0); + let mut headers = Header::dummy_batch(0..target + 1); + // The stored header of the diff's block proves its coinbase. + headers[target as usize].merkle_root = template.dummy_block_merkle_root(); let block_headers = storage.block_headers(); block_headers .write() @@ -1047,7 +1079,7 @@ mod tests { let diff = MnListDiff { base_block_hash: tip_hash, block_hash: target_hash, - ..MnListDiff::dummy_between(0, 0) + ..template }; let peer = "127.0.0.1:19999".parse().unwrap(); manager @@ -1142,6 +1174,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 5b16a5231..81c6af9bf 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; @@ -455,8 +456,10 @@ mod shared_map_tests { quorum_hash, ([TIP - 1].into(), quorum.quorum_entry.quorum_public_key, not_marked.clone()), ); + let unchanged = MnListDiff::dummy_between(TIP - 1, TIP) + .with_coinbase_committing_to(&[], slice::from_ref(&quorum.quorum_entry)); - engine.apply_diff(MnListDiff::dummy_between(TIP - 1, TIP), Some(TIP), None).unwrap(); + engine.apply_diff(unchanged, Some(TIP), None).unwrap(); assert_eq!( engine.masternode_lists[&TIP].quorums[&PLATFORM_TYPE][&quorum_hash].verified, @@ -474,6 +477,7 @@ 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() @@ -486,10 +490,12 @@ mod shared_map_tests { LLMQEntryVerificationStatus::Unknown, ), ); - engine - .apply_diff(MnListDiff::dummy_between(TIP - 2, TIP - 1), Some(TIP - 1), None) - .unwrap(); - engine.apply_diff(MnListDiff::dummy_between(TIP - 1, TIP), Some(TIP), 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), None).unwrap(); + engine.apply_diff(unchanged(TIP - 1, TIP), Some(TIP), 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 f1e64748d..fbc51d9c3 100644 --- a/dash/src/sml/masternode_list_engine/mod.rs +++ b/dash/src/sml/masternode_list_engine/mod.rs @@ -1104,6 +1104,11 @@ impl MasternodeListEngine { /// surface as `QuorumValidationError::RequiredBlockNotPresent` once they are /// actually needed. /// + /// Every diff is checked against the coinbase commitments of its block, as + /// [`Self::apply_diff`] checks one, before its list is stored. The + /// non-rotating quorums of the newest list are verified once, after the + /// last diff. + /// /// # Parameters /// - `qr_info`: The QRInfo message containing quorum snapshots and diffs /// @@ -1449,6 +1454,14 @@ 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`]. The + /// check runs before the list is stored or its quorums are recorded in + /// `quorum_statuses`, so a refused diff leaves the engine as it was and + /// none of its quorums is verified. 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 @@ -1731,13 +1744,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, }; @@ -1772,6 +1785,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, }; @@ -1811,9 +1826,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 @@ -2104,19 +2117,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), None) @@ -2194,12 +2202,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"); @@ -2211,16 +2219,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), 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 @@ -2245,6 +2311,73 @@ 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. 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| { + 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), + None, + ))); + assert!(engine.masternode_lists.is_empty()); + + 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] + #[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); + 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() { @@ -2568,6 +2701,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(); @@ -2794,6 +2928,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 @@ -2868,6 +3003,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) .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..d6f472817 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 persists the id in canonical + /// order, but captures made before it did hold Core's wire order, which + /// the decoder then takes as canonical and the entry hash reverses once + /// more. The bytes such a capture holds are the wire order. + 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 + { + *platform_node_id = + PlatformNodeId::from_bytes(platform_node_id.to_canonical_bytes()); + } + } } }