Skip to content

Commit

Permalink
TN11 bug fix: activate mass hashing when modifying a cached block tem…
Browse files Browse the repository at this point in the history
…plate (kaspanet#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
  • Loading branch information
michaelsutton authored Aug 22, 2024
1 parent 261a750 commit 8e93437
Show file tree
Hide file tree
Showing 18 changed files with 118 additions and 55 deletions.
4 changes: 4 additions & 0 deletions consensus/core/src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!()
}
Expand Down
15 changes: 15 additions & 0 deletions consensus/core/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -66,6 +67,20 @@ impl Block {
pub fn from_precomputed_hash(hash: Hash, parents: Vec<Hash>) -> 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>()
+ self.header.estimate_mem_bytes()
+ size_of::<Vec<Transaction>>()
+ self.transactions.iter().map(Transaction::estimate_mem_bytes).sum::<usize>()
}
}

/// An abstraction for a recallable transaction selector with persistent state
Expand Down
2 changes: 1 addition & 1 deletion consensus/core/src/config/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
Expand Down
4 changes: 4 additions & 0 deletions consensus/core/src/errors/tx.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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")]
Expand Down
7 changes: 7 additions & 0 deletions consensus/core/src/header.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -92,6 +93,12 @@ impl Header {
}
}

impl MemSizeEstimator for Header {
fn estimate_mem_bytes(&self) -> usize {
size_of::<Self>() + self.parents_by_level.iter().map(|l| l.len()).sum::<usize>() * size_of::<Hash>()
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
8 changes: 2 additions & 6 deletions consensus/core/src/merkle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Item = &'a Transaction>, include_mass_field: bool) -> Hash {
pub fn calc_hash_merkle_root<'a>(txs: impl ExactSizeIterator<Item = &'a Transaction>, 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<Item = &'a Transaction>) -> Hash {
calc_merkle_root(txs.map(|tx| hashing::tx::hash(tx, false)))
}

#[cfg(test)]
mod tests {
use crate::merkle::calc_hash_merkle_root;
Expand Down Expand Up @@ -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,
Expand Down
55 changes: 23 additions & 32 deletions consensus/core/src/subnets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Self, Self::Error> {
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))
}
}

Expand All @@ -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<Self, Self::Err> {
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<Self, Self::Error> {
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))
}
}

Expand Down
17 changes: 17 additions & 0 deletions consensus/core/src/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>()
+ self.payload.len()
+ self
.inputs
.iter()
.map(|i| i.signature_script.len() + size_of::<TransactionInput>())
.chain(self.outputs.iter().map(|o| {
// size_of::<TransactionOutput>() 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::<TransactionOutput>()
}))
.sum::<usize>()
}
}

/// Represents any kind of transaction which has populated UTXO entry data and can be verified/signed etc
pub trait VerifiableTransaction {
fn tx(&self) -> &Transaction;
Expand Down
6 changes: 6 additions & 0 deletions consensus/src/consensus/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion consensus/src/consensus/test_consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
4 changes: 1 addition & 3 deletions consensus/src/model/stores/headers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,7 @@ pub struct HeaderWithBlockLevel {

impl MemSizeEstimator for HeaderWithBlockLevel {
fn estimate_mem_bytes(&self) -> usize {
size_of::<Header>()
+ self.header.parents_by_level.iter().map(|l| l.len()).sum::<usize>() * size_of::<Hash>()
+ size_of::<Self>()
self.header.as_ref().estimate_mem_bytes() + size_of::<Self>()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Item = &'a Transaction>) -> Hash {
calc_hash_merkle_root_with_options(txs, false)
}

#[tokio::test]
async fn validate_body_in_context_test() {
let config = ConfigBuilder::new(DEVNET_PARAMS)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Self>, block: &Block) -> BlockProcessResult<u64> {
Expand All @@ -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));
}
Expand Down Expand Up @@ -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<Item = &'a Transaction>) -> Hash {
calc_hash_merkle_root_with_options(txs, false)
}

#[test]
fn validate_body_in_isolation_test() {
let consensus = TestConsensus::new(&Config::new(MAINNET_PARAMS));
Expand Down
4 changes: 2 additions & 2 deletions consensus/src/pipeline/virtual_processor/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand Down
Loading

0 comments on commit 8e93437

Please sign in to comment.