From 8e93437566853df08a10bedeafc9f1872db2550c Mon Sep 17 00:00:00 2001 From: Michael Sutton Date: Thu, 22 Aug 2024 18:56:55 +0300 Subject: [PATCH] TN11 bug fix: activate mass hashing when modifying a cached block template (#476) * fix transaction hash merkle root calculation in modify block template to consider storage mass activation * avoid similar future errors: expose only a single calc_hash_merkle_root function with `include_mass_field` arg and update all test usages explicitly * move subnet checks to inner location * reorganize cache mem estimators --- consensus/core/src/api/mod.rs | 4 ++ consensus/core/src/block.rs | 15 +++++ consensus/core/src/config/genesis.rs | 2 +- consensus/core/src/errors/tx.rs | 4 ++ consensus/core/src/header.rs | 7 +++ consensus/core/src/merkle.rs | 8 +-- consensus/core/src/subnets.rs | 55 ++++++++----------- consensus/core/src/tx.rs | 17 ++++++ consensus/src/consensus/mod.rs | 6 ++ consensus/src/consensus/test_consensus.rs | 2 +- consensus/src/model/stores/headers.rs | 4 +- .../body_validation_in_context.rs | 6 +- .../body_validation_in_isolation.rs | 10 +++- .../pipeline/virtual_processor/processor.rs | 4 +- .../tx_validation_in_isolation.rs | 15 ++++- mining/src/block_template/builder.rs | 4 +- mining/src/testutils/consensus_mock.rs | 8 ++- protocol/flows/src/flowcontext/orphans.rs | 2 +- 18 files changed, 118 insertions(+), 55 deletions(-) diff --git a/consensus/core/src/api/mod.rs b/consensus/core/src/api/mod.rs index 4d8fbc3b4..6df33579c 100644 --- a/consensus/core/src/api/mod.rs +++ b/consensus/core/src/api/mod.rs @@ -190,6 +190,10 @@ pub trait ConsensusApi: Send + Sync { unimplemented!() } + fn calc_transaction_hash_merkle_root(&self, txs: &[Transaction], pov_daa_score: u64) -> Hash { + unimplemented!() + } + fn validate_pruning_proof(&self, proof: &PruningPointProof) -> PruningImportResult<()> { unimplemented!() } diff --git a/consensus/core/src/block.rs b/consensus/core/src/block.rs index 0c2367028..cbd76b42d 100644 --- a/consensus/core/src/block.rs +++ b/consensus/core/src/block.rs @@ -5,6 +5,7 @@ use crate::{ BlueWorkType, }; use kaspa_hashes::Hash; +use kaspa_utils::mem_size::MemSizeEstimator; use std::sync::Arc; /// A mutable block structure where header and transactions within can still be mutated. @@ -66,6 +67,20 @@ impl Block { pub fn from_precomputed_hash(hash: Hash, parents: Vec) -> Block { Block::from_header(Header::from_precomputed_hash(hash, parents)) } + + pub fn asses_for_cache(&self) -> Option<()> { + (self.estimate_mem_bytes() < 1_000_000).then_some(()) + } +} + +impl MemSizeEstimator for Block { + fn estimate_mem_bytes(&self) -> usize { + // Calculates mem bytes of the block (for cache tracking purposes) + size_of::() + + self.header.estimate_mem_bytes() + + size_of::>() + + self.transactions.iter().map(Transaction::estimate_mem_bytes).sum::() + } } /// An abstraction for a recallable transaction selector with persistent state diff --git a/consensus/core/src/config/genesis.rs b/consensus/core/src/config/genesis.rs index 204098ad2..9f9ea21e5 100644 --- a/consensus/core/src/config/genesis.rs +++ b/consensus/core/src/config/genesis.rs @@ -231,7 +231,7 @@ mod tests { fn test_genesis_hashes() { [GENESIS, TESTNET_GENESIS, TESTNET11_GENESIS, SIMNET_GENESIS, DEVNET_GENESIS].into_iter().for_each(|genesis| { let block: Block = (&genesis).into(); - assert_hashes_eq(calc_hash_merkle_root(block.transactions.iter()), block.header.hash_merkle_root); + assert_hashes_eq(calc_hash_merkle_root(block.transactions.iter(), false), block.header.hash_merkle_root); assert_hashes_eq(block.hash(), genesis.hash); }); } diff --git a/consensus/core/src/errors/tx.rs b/consensus/core/src/errors/tx.rs index 1aeb5220f..f21409857 100644 --- a/consensus/core/src/errors/tx.rs +++ b/consensus/core/src/errors/tx.rs @@ -1,4 +1,5 @@ use crate::constants::MAX_SOMPI; +use crate::subnets::SubnetworkId; use crate::tx::TransactionOutpoint; use kaspa_txscript_errors::TxScriptError; use thiserror::Error; @@ -92,6 +93,9 @@ pub enum TxRuleError { #[error("calculated contextual mass (including storage mass) {0} is not equal to the committed mass field {1}")] WrongMass(u64, u64), + #[error("transaction subnetwork id {0} is neither native nor coinbase")] + SubnetworksDisabled(SubnetworkId), + /// [`TxRuleError::FeerateTooLow`] is not a consensus error but a mempool error triggered by the /// fee/mass RBF validation rule #[error("fee rate per contextual mass gram is not greater than the fee rate of the replaced transaction")] diff --git a/consensus/core/src/header.rs b/consensus/core/src/header.rs index b6c2b9bc7..b57337afc 100644 --- a/consensus/core/src/header.rs +++ b/consensus/core/src/header.rs @@ -1,6 +1,7 @@ use crate::{hashing, BlueWorkType}; use borsh::{BorshDeserialize, BorshSerialize}; use kaspa_hashes::Hash; +use kaspa_utils::mem_size::MemSizeEstimator; use serde::{Deserialize, Serialize}; /// @category Consensus @@ -92,6 +93,12 @@ impl Header { } } +impl MemSizeEstimator for Header { + fn estimate_mem_bytes(&self) -> usize { + size_of::() + self.parents_by_level.iter().map(|l| l.len()).sum::() * size_of::() + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/consensus/core/src/merkle.rs b/consensus/core/src/merkle.rs index cfd5c9045..59c6ca7c4 100644 --- a/consensus/core/src/merkle.rs +++ b/consensus/core/src/merkle.rs @@ -2,14 +2,10 @@ use crate::{hashing, tx::Transaction}; use kaspa_hashes::Hash; use kaspa_merkle::calc_merkle_root; -pub fn calc_hash_merkle_root_with_options<'a>(txs: impl ExactSizeIterator, include_mass_field: bool) -> Hash { +pub fn calc_hash_merkle_root<'a>(txs: impl ExactSizeIterator, include_mass_field: bool) -> Hash { calc_merkle_root(txs.map(|tx| hashing::tx::hash(tx, include_mass_field))) } -pub fn calc_hash_merkle_root<'a>(txs: impl ExactSizeIterator) -> Hash { - calc_merkle_root(txs.map(|tx| hashing::tx::hash(tx, false))) -} - #[cfg(test)] mod tests { use crate::merkle::calc_hash_merkle_root; @@ -242,7 +238,7 @@ mod tests { ), ]; assert_eq!( - calc_hash_merkle_root(txs.iter()), + calc_hash_merkle_root(txs.iter(), false), Hash::from_slice(&[ 0x46, 0xec, 0xf4, 0x5b, 0xe3, 0xba, 0xca, 0x34, 0x9d, 0xfe, 0x8a, 0x78, 0xde, 0xaf, 0x05, 0x3b, 0x0a, 0xa6, 0xd5, 0x38, 0x97, 0x4d, 0xa5, 0x0f, 0xd6, 0xef, 0xb4, 0xd2, 0x66, 0xbc, 0x8d, 0x21, diff --git a/consensus/core/src/subnets.rs b/consensus/core/src/subnets.rs index 2456f8444..756c4d40a 100644 --- a/consensus/core/src/subnets.rs +++ b/consensus/core/src/subnets.rs @@ -4,7 +4,7 @@ use std::str::{self, FromStr}; use borsh::{BorshDeserialize, BorshSerialize}; use kaspa_utils::hex::{FromHex, ToHex}; use kaspa_utils::{serde_impl_deser_fixed_bytes_ref, serde_impl_ser_fixed_bytes_ref}; -use thiserror::Error; +use thiserror::Error; /// The size of the array used to store subnetwork IDs. pub const SUBNETWORK_ID_SIZE: usize = 20; @@ -59,35 +59,34 @@ impl SubnetworkId { *self == SUBNETWORK_ID_COINBASE || *self == SUBNETWORK_ID_REGISTRY } + /// Returns true if the subnetwork is the native subnetwork + #[inline] + pub fn is_native(&self) -> bool { + *self == SUBNETWORK_ID_NATIVE + } + /// Returns true if the subnetwork is the native or a built-in subnetwork #[inline] pub fn is_builtin_or_native(&self) -> bool { - *self == SUBNETWORK_ID_NATIVE || self.is_builtin() + self.is_native() || self.is_builtin() } } -#[derive(Error, Debug, Clone)] -pub enum SubnetworkConversionError { - #[error("Invalid bytes")] - InvalidBytes, - - #[error(transparent)] - SliceError(#[from] std::array::TryFromSliceError), - - #[error(transparent)] - HexError(#[from] faster_hex::Error), -} - +#[derive(Error, Debug, Clone)] +pub enum SubnetworkConversionError { + #[error(transparent)] + SliceError(#[from] std::array::TryFromSliceError), + + #[error(transparent)] + HexError(#[from] faster_hex::Error), +} + impl TryFrom<&[u8]> for SubnetworkId { - type Error = SubnetworkConversionError; + type Error = SubnetworkConversionError; fn try_from(value: &[u8]) -> Result { let bytes = <[u8; SUBNETWORK_ID_SIZE]>::try_from(value)?; - if bytes != Self::from_byte(0).0 && bytes != Self::from_byte(1).0 { - Err(Self::Error::InvalidBytes) - } else { - Ok(Self(bytes)) - } + Ok(Self(bytes)) } } @@ -109,30 +108,22 @@ impl ToHex for SubnetworkId { } impl FromStr for SubnetworkId { - type Err = SubnetworkConversionError; + type Err = SubnetworkConversionError; #[inline] fn from_str(hex_str: &str) -> Result { let mut bytes = [0u8; SUBNETWORK_ID_SIZE]; faster_hex::hex_decode(hex_str.as_bytes(), &mut bytes)?; - if bytes != Self::from_byte(0).0 && bytes != Self::from_byte(1).0 { - Err(Self::Err::InvalidBytes) - } else { - Ok(Self(bytes)) - } + Ok(Self(bytes)) } } impl FromHex for SubnetworkId { - type Error = SubnetworkConversionError; + type Error = SubnetworkConversionError; fn from_hex(hex_str: &str) -> Result { let mut bytes = [0u8; SUBNETWORK_ID_SIZE]; faster_hex::hex_decode(hex_str.as_bytes(), &mut bytes)?; - if bytes != Self::from_byte(0).0 && bytes != Self::from_byte(1).0 { - Err(Self::Error::InvalidBytes) - } else { - Ok(Self(bytes)) - } + Ok(Self(bytes)) } } diff --git a/consensus/core/src/tx.rs b/consensus/core/src/tx.rs index 67e09df73..3595dcb8b 100644 --- a/consensus/core/src/tx.rs +++ b/consensus/core/src/tx.rs @@ -230,6 +230,23 @@ impl Transaction { } } +impl MemSizeEstimator for Transaction { + fn estimate_mem_bytes(&self) -> usize { + // Calculates mem bytes of the transaction (for cache tracking purposes) + size_of::() + + self.payload.len() + + self + .inputs + .iter() + .map(|i| i.signature_script.len() + size_of::()) + .chain(self.outputs.iter().map(|o| { + // size_of::() already counts SCRIPT_VECTOR_SIZE bytes within, so we only add the delta + o.script_public_key.script().len().saturating_sub(SCRIPT_VECTOR_SIZE) + size_of::() + })) + .sum::() + } +} + /// Represents any kind of transaction which has populated UTXO entry data and can be verified/signed etc pub trait VerifiableTransaction { fn tx(&self) -> &Transaction; diff --git a/consensus/src/consensus/mod.rs b/consensus/src/consensus/mod.rs index fb86d0dab..919929a8c 100644 --- a/consensus/src/consensus/mod.rs +++ b/consensus/src/consensus/mod.rs @@ -58,6 +58,7 @@ use kaspa_consensus_core::{ tx::TxResult, }, header::Header, + merkle::calc_hash_merkle_root, muhash::MuHashExtensions, network::NetworkType, pruning::{PruningPointProof, PruningPointTrustedData, PruningPointsList}, @@ -675,6 +676,11 @@ impl ConsensusApi for Consensus { self.services.coinbase_manager.modify_coinbase_payload(payload, miner_data) } + fn calc_transaction_hash_merkle_root(&self, txs: &[Transaction], pov_daa_score: u64) -> Hash { + let storage_mass_activated = pov_daa_score > self.config.storage_mass_activation_daa_score; + calc_hash_merkle_root(txs.iter(), storage_mass_activated) + } + fn validate_pruning_proof(&self, proof: &PruningPointProof) -> Result<(), PruningImportError> { self.services.pruning_proof_manager.validate_pruning_point_proof(proof) } diff --git a/consensus/src/consensus/test_consensus.rs b/consensus/src/consensus/test_consensus.rs index c626e00ff..a705d9ecc 100644 --- a/consensus/src/consensus/test_consensus.rs +++ b/consensus/src/consensus/test_consensus.rs @@ -176,7 +176,7 @@ impl TestConsensus { let cb = Transaction::new(TX_VERSION, vec![], vec![], 0, SUBNETWORK_ID_COINBASE, 0, cb_payload); txs.insert(0, cb); - header.hash_merkle_root = calc_hash_merkle_root(txs.iter()); + header.hash_merkle_root = calc_hash_merkle_root(txs.iter(), false); MutableBlock::new(header, txs) } diff --git a/consensus/src/model/stores/headers.rs b/consensus/src/model/stores/headers.rs index b0c25b596..64e10a90b 100644 --- a/consensus/src/model/stores/headers.rs +++ b/consensus/src/model/stores/headers.rs @@ -29,9 +29,7 @@ pub struct HeaderWithBlockLevel { impl MemSizeEstimator for HeaderWithBlockLevel { fn estimate_mem_bytes(&self) -> usize { - size_of::
() - + self.header.parents_by_level.iter().map(|l| l.len()).sum::() * size_of::() - + size_of::() + self.header.as_ref().estimate_mem_bytes() + size_of::() } } diff --git a/consensus/src/pipeline/body_processor/body_validation_in_context.rs b/consensus/src/pipeline/body_processor/body_validation_in_context.rs index 2425556d0..042410fa8 100644 --- a/consensus/src/pipeline/body_processor/body_validation_in_context.rs +++ b/consensus/src/pipeline/body_processor/body_validation_in_context.rs @@ -94,13 +94,17 @@ mod tests { }; use kaspa_consensus_core::{ api::ConsensusApi, - merkle::calc_hash_merkle_root, + merkle::calc_hash_merkle_root as calc_hash_merkle_root_with_options, subnets::SUBNETWORK_ID_NATIVE, tx::{Transaction, TransactionInput, TransactionOutpoint}, }; use kaspa_core::assert_match; use kaspa_hashes::Hash; + fn calc_hash_merkle_root<'a>(txs: impl ExactSizeIterator) -> Hash { + calc_hash_merkle_root_with_options(txs, false) + } + #[tokio::test] async fn validate_body_in_context_test() { let config = ConfigBuilder::new(DEVNET_PARAMS) diff --git a/consensus/src/pipeline/body_processor/body_validation_in_isolation.rs b/consensus/src/pipeline/body_processor/body_validation_in_isolation.rs index e5a51d815..c413552b9 100644 --- a/consensus/src/pipeline/body_processor/body_validation_in_isolation.rs +++ b/consensus/src/pipeline/body_processor/body_validation_in_isolation.rs @@ -2,7 +2,7 @@ use std::{collections::HashSet, sync::Arc}; use super::BlockBodyProcessor; use crate::errors::{BlockProcessResult, RuleError}; -use kaspa_consensus_core::{block::Block, merkle::calc_hash_merkle_root_with_options, tx::TransactionOutpoint}; +use kaspa_consensus_core::{block::Block, merkle::calc_hash_merkle_root, tx::TransactionOutpoint}; impl BlockBodyProcessor { pub fn validate_body_in_isolation(self: &Arc, block: &Block) -> BlockProcessResult { @@ -29,7 +29,7 @@ impl BlockBodyProcessor { } fn check_hash_merkle_root(block: &Block, storage_mass_activated: bool) -> BlockProcessResult<()> { - let calculated = calc_hash_merkle_root_with_options(block.transactions.iter(), storage_mass_activated); + let calculated = calc_hash_merkle_root(block.transactions.iter(), storage_mass_activated); if calculated != block.header.hash_merkle_root { return Err(RuleError::BadMerkleRoot(block.header.hash_merkle_root, calculated)); } @@ -137,13 +137,17 @@ mod tests { api::{BlockValidationFutures, ConsensusApi}, block::MutableBlock, header::Header, - merkle::calc_hash_merkle_root, + merkle::calc_hash_merkle_root as calc_hash_merkle_root_with_options, subnets::{SUBNETWORK_ID_COINBASE, SUBNETWORK_ID_NATIVE}, tx::{scriptvec, ScriptPublicKey, Transaction, TransactionId, TransactionInput, TransactionOutpoint, TransactionOutput}, }; use kaspa_core::assert_match; use kaspa_hashes::Hash; + fn calc_hash_merkle_root<'a>(txs: impl ExactSizeIterator) -> Hash { + calc_hash_merkle_root_with_options(txs, false) + } + #[test] fn validate_body_in_isolation_test() { let consensus = TestConsensus::new(&Config::new(MAINNET_PARAMS)); diff --git a/consensus/src/pipeline/virtual_processor/processor.rs b/consensus/src/pipeline/virtual_processor/processor.rs index e2c18ac2c..163e88893 100644 --- a/consensus/src/pipeline/virtual_processor/processor.rs +++ b/consensus/src/pipeline/virtual_processor/processor.rs @@ -54,7 +54,7 @@ use kaspa_consensus_core::{ coinbase::MinerData, config::genesis::GenesisBlock, header::Header, - merkle::calc_hash_merkle_root_with_options, + merkle::calc_hash_merkle_root, pruning::PruningPointsList, tx::{MutableTransaction, Transaction}, utxo::{ @@ -975,7 +975,7 @@ impl VirtualStateProcessor { // Hash according to hardfork activation let storage_mass_activated = virtual_state.daa_score > self.storage_mass_activation_daa_score; - let hash_merkle_root = calc_hash_merkle_root_with_options(txs.iter(), storage_mass_activated); + let hash_merkle_root = calc_hash_merkle_root(txs.iter(), storage_mass_activated); let accepted_id_merkle_root = kaspa_merkle::calc_merkle_root(virtual_state.accepted_tx_ids.iter().copied()); let utxo_commitment = virtual_state.multiset.clone().finalize(); diff --git a/consensus/src/processes/transaction_validator/tx_validation_in_isolation.rs b/consensus/src/processes/transaction_validator/tx_validation_in_isolation.rs index 67901612d..914624f94 100644 --- a/consensus/src/processes/transaction_validator/tx_validation_in_isolation.rs +++ b/consensus/src/processes/transaction_validator/tx_validation_in_isolation.rs @@ -17,6 +17,7 @@ impl TransactionValidator { check_duplicate_transaction_inputs(tx)?; check_gas(tx)?; check_transaction_payload(tx)?; + check_transaction_subnetwork(tx)?; check_transaction_version(tx) } @@ -146,10 +147,18 @@ fn check_transaction_output_value_ranges(tx: &Transaction) -> TxResult<()> { Ok(()) } +fn check_transaction_subnetwork(tx: &Transaction) -> TxResult<()> { + if tx.is_coinbase() || tx.subnetwork_id.is_native() { + Ok(()) + } else { + Err(TxRuleError::SubnetworksDisabled(tx.subnetwork_id.clone())) + } +} + #[cfg(test)] mod tests { use kaspa_consensus_core::{ - subnets::{SUBNETWORK_ID_COINBASE, SUBNETWORK_ID_NATIVE}, + subnets::{SubnetworkId, SUBNETWORK_ID_COINBASE, SUBNETWORK_ID_NATIVE}, tx::{scriptvec, ScriptPublicKey, Transaction, TransactionId, TransactionInput, TransactionOutpoint, TransactionOutput}, }; use kaspa_core::assert_match; @@ -261,6 +270,10 @@ mod tests { tv.validate_tx_in_isolation(&valid_tx).unwrap(); + let mut tx: Transaction = valid_tx.clone(); + tx.subnetwork_id = SubnetworkId::from_byte(3); + assert_match!(tv.validate_tx_in_isolation(&tx), Err(TxRuleError::SubnetworksDisabled(_))); + let mut tx = valid_tx.clone(); tx.inputs = vec![]; assert_match!(tv.validate_tx_in_isolation(&tx), Err(TxRuleError::NoTxInputs)); diff --git a/mining/src/block_template/builder.rs b/mining/src/block_template/builder.rs index e111b2562..6f0dbe674 100644 --- a/mining/src/block_template/builder.rs +++ b/mining/src/block_template/builder.rs @@ -3,7 +3,6 @@ use kaspa_consensus_core::{ api::ConsensusApi, block::{BlockTemplate, TemplateBuildMode, TemplateTransactionSelector}, coinbase::MinerData, - merkle::calc_hash_merkle_root, tx::COINBASE_TRANSACTION_INDEX, }; use kaspa_core::time::{unix_now, Stopwatch}; @@ -106,7 +105,8 @@ impl BlockTemplateBuilder { coinbase_tx.outputs.last_mut().unwrap().script_public_key = new_miner_data.script_public_key.clone(); } // Update the hash merkle root according to the modified transactions - block_template.block.header.hash_merkle_root = calc_hash_merkle_root(block_template.block.transactions.iter()); + block_template.block.header.hash_merkle_root = + consensus.calc_transaction_hash_merkle_root(&block_template.block.transactions, block_template.block.header.daa_score); let new_timestamp = unix_now(); if new_timestamp > block_template.block.header.timestamp { // Only if new time stamp is later than current, update the header. Otherwise, diff --git a/mining/src/testutils/consensus_mock.rs b/mining/src/testutils/consensus_mock.rs index c38c04a02..d68e062a0 100644 --- a/mining/src/testutils/consensus_mock.rs +++ b/mining/src/testutils/consensus_mock.rs @@ -19,7 +19,7 @@ use kaspa_consensus_core::{ utxo::utxo_collection::UtxoCollection, }; use kaspa_core::time::unix_now; -use kaspa_hashes::ZERO_HASH; +use kaspa_hashes::{Hash, ZERO_HASH}; use parking_lot::RwLock; use std::{collections::HashMap, sync::Arc}; @@ -86,7 +86,7 @@ impl ConsensusApi for ConsensusMock { let coinbase = coinbase_manager.expected_coinbase_transaction(miner_data.clone()); txs.insert(0, coinbase.tx); let now = unix_now(); - let hash_merkle_root = calc_hash_merkle_root(txs.iter()); + let hash_merkle_root = self.calc_transaction_hash_merkle_root(&txs, 0); let header = Header::new_finalized( BLOCK_VERSION, vec![], @@ -177,4 +177,8 @@ impl ConsensusApi for ConsensusMock { let coinbase_manager = CoinbaseManagerMock::new(); Ok(coinbase_manager.modify_coinbase_payload(payload, miner_data)) } + + fn calc_transaction_hash_merkle_root(&self, txs: &[Transaction], _pov_daa_score: u64) -> Hash { + calc_hash_merkle_root(txs.iter(), false) + } } diff --git a/protocol/flows/src/flowcontext/orphans.rs b/protocol/flows/src/flowcontext/orphans.rs index 75c223caa..d1122b0e0 100644 --- a/protocol/flows/src/flowcontext/orphans.rs +++ b/protocol/flows/src/flowcontext/orphans.rs @@ -78,7 +78,7 @@ impl OrphanBlocksPool { if self.orphans.contains_key(&orphan_hash) { return None; } - + orphan_block.asses_for_cache()?; let (roots, orphan_ancestors) = match self.get_orphan_roots(consensus, orphan_block.header.direct_parents().iter().copied().collect()).await { FindRootsOutput::Roots(roots, orphan_ancestors) => (roots, orphan_ancestors),