diff --git a/dash-spv/src/sml_engine.rs b/dash-spv/src/sml_engine.rs index 4983356f8..b5183fe4b 100644 --- a/dash-spv/src/sml_engine.rs +++ b/dash-spv/src/sml_engine.rs @@ -25,8 +25,8 @@ use dashcore::sml::masternode_list::MasternodeList; use dashcore::sml::quorum_entry::qualified_quorum_entry::QualifiedQuorumEntry; use dashcore::sml::quorum_entry::qualified_quorum_entry::VerifyingChainLockSignaturesType; use dashcore::sml::quorum_validation_error::QuorumValidationError; +use dashcore::transaction::special_transaction::coinbase::CoinbasePayload; use dashcore::transaction::special_transaction::quorum_commitment::QuorumEntry; -use dashcore::transaction::special_transaction::TransactionPayload; use dashcore::{BlockHash, QuorumHash}; use dashcore::{Network, Transaction}; use tokio::sync::RwLock; @@ -173,16 +173,14 @@ fn build_cycle_quorum_map( /// The ChainLock `coinbase_tx`, the coinbase of the block at `height`, carries /// for member selection, as Dash Core's `GetNonNullCoinbaseChainlock` reads it -/// (`src/evo/cbtx.cpp:200-211`). +/// (`src/evo/cbtx.cpp:200-211`). A payload of a version newer than this +/// library decodes is read as far as it knows the layout, see +/// [`CoinbasePayload::from_coinbase_tx`]. fn valid_coinbase_chain_lock( coinbase_tx: &Transaction, height: CoreBlockHeight, ) -> Option { - let Some(TransactionPayload::CoinbasePayloadType(payload)) = - &coinbase_tx.special_transaction_payload - else { - return None; - }; + let payload = CoinbasePayload::from_coinbase_tx(coinbase_tx)?; let signature = payload.chain_lock_candidate(height)?; signature.as_scheme(BlsScheme::Modern).is_valid().then_some(signature) } @@ -1483,7 +1481,7 @@ pub(crate) mod test_support { #[cfg(test)] mod tests { use super::test_support::{fixture_heights, quorum_entry, TestEngine}; - use super::{build_cycle_quorum_map, qr_info_diffs}; + use super::{build_cycle_quorum_map, qr_info_diffs, valid_coinbase_chain_lock}; use super::{FeedError, MasternodeListEngine, QRInfoFeedResult, WORK_DIFF_DEPTH}; use crate::error::{StorageError, StorageResult}; use crate::storage::{BlockHeaderStorage, BlockHeaderTip}; @@ -3306,6 +3304,25 @@ mod tests { ); } + /// A coinbase payload of a version this library does not decode still + /// carries the ChainLock member selection reads: Dash Core only appends + /// fields in a new version, so it holds the version 3 fields where they + /// were. + #[test] + fn a_newer_coinbase_payload_carries_its_chain_lock() { + const HEIGHT: CoreBlockHeight = 2_227_096; + let diff: MnListDiff = deserialize(include_bytes!( + "../../dash/tests/data/test_DML_diffs/mn_list_diff_0_2227096.bin" + )) + .expect("diff"); + let chain_lock = valid_coinbase_chain_lock(&diff.coinbase_tx, HEIGHT); + assert!(chain_lock.is_some(), "the capture's coinbase carries a ChainLock"); + + let newer = diff.coinbase_tx.with_newer_coinbase_payload(); + + assert_eq!(valid_coinbase_chain_lock(&newer, HEIGHT), chain_lock); + } + /// On an engine that checks coinbases, a restarted engine records the /// newest list's coinbase ChainLock from the diff from that block to /// itself, and records nothing from one whose coinbase proof or roots fail. diff --git a/dash/src/blockdata/transaction/special_transaction/coinbase.rs b/dash/src/blockdata/transaction/special_transaction/coinbase.rs index 44e4ba87c..a7862c0f5 100644 --- a/dash/src/blockdata/transaction/special_transaction/coinbase.rs +++ b/dash/src/blockdata/transaction/special_transaction/coinbase.rs @@ -20,6 +20,8 @@ use hashes::Hash; +use super::{TransactionPayload, TransactionType}; +use crate::Transaction; use crate::bls_sig_utils::BLSSignature; use crate::consensus::encode::{compact_size_len, read_compact_size, write_compact_size}; use crate::consensus::{Decodable, Encodable, encode}; @@ -27,6 +29,10 @@ use crate::hash_types::{MerkleRootAssetUnlocks, MerkleRootMasternodeList, Merkle use crate::io; use crate::io::{Error, ErrorKind}; +/// Newest payload version whose layout this crate decodes: the last version +/// `CoinbasePayload::consensus_decode` reads fields for. +const NEWEST_DECODED_VERSION: u16 = 4; + /// A Coinbase payload. This is contained as the payload of a coinbase special transaction. /// The Coinbase payload is described in DIP4. /// @@ -204,6 +210,32 @@ impl CoinbasePayload { } } + /// The payload of `coinbase`, read as far as this crate knows its layout. + /// + /// The transaction decoder keeps a payload raw when bytes follow the fields this crate + /// knows, as they do in a newer version that appends fields. Dash Core only appends fields in + /// a new version, so a raw payload of a version newer than this crate decodes is read as the + /// newest version it decodes, leaving the newer fields out. Should a later version change + /// the meaning of a field this crate knows, this has to change with it. The result keeps the + /// payload's `version`, so it does not encode to the payload's bytes. + /// + /// `None` for a transaction without a coinbase payload, a raw payload of a version this + /// crate decodes, which Dash Core never produces, or one too short for the fields this crate + /// knows. + pub fn from_coinbase_tx(coinbase: &Transaction) -> Option { + match &coinbase.special_transaction_payload { + Some(TransactionPayload::CoinbasePayloadType(payload)) => Some(payload.clone()), + Some(TransactionPayload::RawPayloadType(tx_type, bytes)) + if *tx_type == TransactionType::Coinbase.to_u16() => + { + CoinbasePayload::consensus_decode(&mut &bytes[..]) + .ok() + .filter(|payload| payload.version > NEWEST_DECODED_VERSION) + } + _ => None, + } + } + /// The ChainLock signature this coinbase carries for member selection at /// `block_height`, as Dash Core's `GetNonNullCoinbaseChainlock` reads it /// (`src/evo/cbtx.cpp:200-211`) short of `IsValid()`, which the caller @@ -383,9 +415,11 @@ impl Decodable for CoinbasePayload { mod tests { use hashes::Hash; + use crate::Transaction; use crate::bls_sig_utils::BLSSignature; use crate::consensus::{Decodable, Encodable}; use crate::hash_types::{MerkleRootAssetUnlocks, MerkleRootMasternodeList, MerkleRootQuorums}; + use crate::transaction::special_transaction::TransactionPayload; use crate::transaction::special_transaction::coinbase::CoinbasePayload; use test_case::test_case; @@ -680,4 +714,69 @@ mod tests { json["version"] = 3.into(); assert!(serde_json::from_value::(json).is_err()); } + + /// A raw coinbase payload of a version newer than this crate decodes is + /// read as far as it knows the layout: the fields up to version 4, under + /// the payload's own version. A raw payload of a version it decodes, or + /// one cut short anywhere inside those fields, gives nothing. + #[test] + fn from_coinbase_tx_reads_the_known_fields_of_a_newer_payload() { + let typed = mainnet_v20_payload(); + let coinbase = Transaction { + version: 3, + lock_time: 0, + input: vec![], + output: vec![], + special_transaction_payload: Some(TransactionPayload::CoinbasePayloadType( + typed.clone(), + )), + }; + assert_eq!(CoinbasePayload::from_coinbase_tx(&coinbase), Some(typed.clone())); + let newer = coinbase.with_newer_coinbase_payload(); + let Some(TransactionPayload::RawPayloadType(tx_type, bytes)) = + &newer.special_transaction_payload + else { + panic!("a raw payload"); + }; + let raw = |bytes: Vec| Transaction { + special_transaction_payload: Some(TransactionPayload::RawPayloadType(*tx_type, bytes)), + ..newer.clone() + }; + let with_version = |version: u16| { + let mut bytes = bytes.clone(); + bytes[..2].copy_from_slice(&version.to_le_bytes()); + raw(bytes) + }; + + let known_fields = CoinbasePayload { + version: 4, + merkle_root_asset_unlocks: Some(MerkleRootAssetUnlocks::all_zeros()), + ..typed + }; + for version in [5, u16::MAX] { + assert_eq!( + CoinbasePayload::from_coinbase_tx(&with_version(version)), + Some(CoinbasePayload { + version, + ..known_fields.clone() + }), + "version {version}" + ); + } + for version in 0..=4 { + assert_eq!( + CoinbasePayload::from_coinbase_tx(&with_version(version)), + None, + "version {version}" + ); + } + for len in 0..known_fields.size() { + assert_eq!( + CoinbasePayload::from_coinbase_tx(&raw(bytes[..len].to_vec())), + None, + "cut to {len} bytes" + ); + } + assert_eq!(CoinbasePayload::from_coinbase_tx(&Transaction::dummy_empty()), None); + } } diff --git a/dash/src/sml/masternode_list/merkle_roots.rs b/dash/src/sml/masternode_list/merkle_roots.rs index 4ba79c53b..d8a1ba9b2 100644 --- a/dash/src/sml/masternode_list/merkle_roots.rs +++ b/dash/src/sml/masternode_list/merkle_roots.rs @@ -4,7 +4,7 @@ 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; +use crate::transaction::special_transaction::coinbase::CoinbasePayload; /// Computes the Merkle root from a list of hashes. /// @@ -46,14 +46,14 @@ pub fn merkle_root_from_hashes(hashes: Vec) -> Option Result<(), SmlError> { - let Some(TransactionPayload::CoinbasePayloadType(coinbase_payload)) = - &coinbase_transaction.special_transaction_payload - else { + let Some(coinbase_payload) = CoinbasePayload::from_coinbase_tx(coinbase_transaction) else { return Err(SmlError::CoinbaseCommitmentMismatch(self.block_hash)); }; @@ -159,14 +159,15 @@ mod tests { use hashes::Hash; use test_case::test_case; - use crate::consensus::deserialize; + use crate::consensus::{deserialize, serialize}; use crate::hash_types::{MerkleRootMasternodeList, MerkleRootQuorums}; use crate::network::message_sml::MnListDiff; use crate::sml::error::SmlError; use crate::sml::masternode_list::MasternodeList; + use crate::test_utils::UNASSIGNED_SPECIAL_TX_TYPE; use crate::transaction::special_transaction::TransactionPayload; use crate::transaction::special_transaction::coinbase::CoinbasePayload; - use crate::{BlockHash, Network}; + use crate::{BlockHash, Network, Transaction}; #[test_case(|_| {}, None; "matching")] #[test_case( @@ -191,10 +192,27 @@ mod tests { tamper: fn(&mut CoinbasePayload), error: Option SmlError>, ) { - let diff: MnListDiff = deserialize(include_bytes!( + let (list, mut coinbase) = fixture_list(|_| {}); + let Some(TransactionPayload::CoinbasePayloadType(payload)) = + &mut coinbase.special_transaction_payload + else { + panic!("coinbase payload"); + }; + tamper(payload); + + let result = list.verify_coinbase_roots(&coinbase); + + assert_eq!(result, error.map_or(Ok(()), |error| Err(error(list.block_hash)))); + } + + /// The list of the 2227096 capture, built after `edit`, and the capture's + /// coinbase committing to its masternodes. + fn fixture_list(edit: fn(&mut MnListDiff)) -> (MasternodeList, Transaction) { + let mut diff: MnListDiff = deserialize(include_bytes!( "../../../tests/data/test_DML_diffs/mn_list_diff_0_2227096.bin" )) .expect("expected to deserialize"); + edit(&mut diff); let mut coinbase = diff.coinbase_tx.clone(); let list = MasternodeList::from_diff(diff, 2_227_096, Network::Mainnet).expect("builds"); let Some(TransactionPayload::CoinbasePayloadType(payload)) = @@ -204,10 +222,74 @@ mod tests { }; // The capture's masternodes do not hash to its coinbase's root, its quorums do. payload.merkle_root_masternode_list = list.masternode_merkle_root.expect("masternodes"); - tamper(payload); + (list, coinbase) + } + + /// `coinbase` with its payload written as a version this crate does not + /// decode, its type and bytes edited, and decoded back from the wire. + fn newer_coinbase( + coinbase: &Transaction, + edit: impl FnOnce(&mut u16, &mut Vec), + ) -> Transaction { + let mut newer = coinbase.with_newer_coinbase_payload(); + let Some(TransactionPayload::RawPayloadType(tx_type, bytes)) = + &mut newer.special_transaction_payload + else { + panic!("a raw payload"); + }; + edit(tx_type, bytes); + let decoded: Transaction = deserialize(&serialize(&newer)).expect("decodes"); + assert_eq!(decoded, newer, "the decoder keeps the payload raw"); + decoded + } + + /// A coinbase payload of a version this crate does not decode stays raw. + /// Dash Core only appends fields in a new version, so the roots are read + /// from the fields this crate knows, at bytes 6..38 and 38..70, and still + /// have to match. A raw payload of a version this crate decodes, which + /// Dash Core never produces, or of another transaction type commits to + /// nothing. + #[test_case(|_, _| {}, true; "of a newer version")] + #[test_case(|_, bytes| bytes[6] ^= 1, false; "of a newer version off the masternode list root")] + #[test_case(|_, bytes| bytes[38] ^= 1, false; "of a newer version off the quorum list root")] + #[test_case( + |_, bytes| bytes[..2].copy_from_slice(&4u16.to_le_bytes()), + false; + "of a known version with bytes left over" + )] + #[test_case( + |tx_type, _| *tx_type = UNASSIGNED_SPECIAL_TX_TYPE, + false; + "of another transaction type" + )] + fn verify_coinbase_roots_of_a_raw_payload(edit: fn(&mut u16, &mut Vec), matches: bool) { + let (list, coinbase) = fixture_list(|_| {}); + let coinbase = newer_coinbase(&coinbase, edit); let result = list.verify_coinbase_roots(&coinbase); - assert_eq!(result, error.map_or(Ok(()), |error| Err(error(list.block_hash)))); + let expected = if matches { + Ok(()) + } else { + Err(SmlError::CoinbaseCommitmentMismatch(list.block_hash)) + }; + assert_eq!(result, expected); + } + + /// A newer coinbase payload cut short before its quorum list root commits + /// to nothing, even for a list without quorums, whose root is all zeros. + #[test] + fn a_newer_coinbase_payload_cut_short_commits_to_nothing() { + let (list, coinbase) = fixture_list(|diff| { + diff.new_quorums.clear(); + diff.quorums_chainlock_signatures.clear(); + }); + assert!(list.quorums.is_empty()); + let coinbase = newer_coinbase(&coinbase, |_, bytes| bytes.truncate(38)); + assert_eq!(CoinbasePayload::from_coinbase_tx(&coinbase), None); + + let result = list.verify_coinbase_roots(&coinbase); + + assert_eq!(result, Err(SmlError::CoinbaseCommitmentMismatch(list.block_hash))); } } diff --git a/dash/src/test_utils/transaction.rs b/dash/src/test_utils/transaction.rs index 53acd33f5..e538f9f68 100644 --- a/dash/src/test_utils/transaction.rs +++ b/dash/src/test_utils/transaction.rs @@ -1,5 +1,11 @@ use std::ops::Range; +use hashes::Hash; + +use crate::consensus::serialize; +use crate::hash_types::MerkleRootAssetUnlocks; +use crate::transaction::special_transaction::coinbase::CoinbasePayload; +use crate::transaction::special_transaction::{TransactionPayload, TransactionType}; use crate::{Address, OutPoint, ScriptBuf, Transaction, TxIn, TxOut, Txid, Witness}; /// A special transaction type no Dash Core version assigns @@ -75,4 +81,31 @@ impl Transaction { special_transaction_payload: None, } } + + /// This coinbase with its version 3 payload written as a version this crate does not decode, + /// the way a later Dash Core release that appends fields would write it: the version 4 + /// fields under version 5, then 8 bytes of a field of its own. The transaction decoder keeps + /// such a payload raw. + pub fn with_newer_coinbase_payload(&self) -> Transaction { + let Some(TransactionPayload::CoinbasePayloadType(payload)) = + &self.special_transaction_payload + else { + panic!("a coinbase payload"); + }; + assert_eq!(payload.version, 3, "a version 3 coinbase payload"); + let mut bytes = serialize(&CoinbasePayload { + version: 4, + merkle_root_asset_unlocks: Some(MerkleRootAssetUnlocks::all_zeros()), + ..payload.clone() + }); + bytes[..2].copy_from_slice(&5u16.to_le_bytes()); + bytes.extend([0xA5; 8]); + Transaction { + special_transaction_payload: Some(TransactionPayload::RawPayloadType( + TransactionType::Coinbase.to_u16(), + bytes, + )), + ..self.clone() + } + } }