diff --git a/base_layer/core/src/consensus/consensus_constants.rs b/base_layer/core/src/consensus/consensus_constants.rs index 5b0955001a..f6b48112f5 100644 --- a/base_layer/core/src/consensus/consensus_constants.rs +++ b/base_layer/core/src/consensus/consensus_constants.rs @@ -38,7 +38,6 @@ use crate::{ transactions::{ tari_amount::{uT, MicroMinotari}, transaction_components::{ - CoinBaseExtra, OutputFeatures, OutputFeaturesVersion, OutputType, @@ -110,6 +109,8 @@ pub struct ConsensusConstants { permitted_output_types: &'static [OutputType], /// The allowlist of range proof types permitted_range_proof_types: [(OutputType, &'static [RangeProofType]); 5], + /// Coinbase outputs are allowed to have metadata, but it has the following length limit + coinbase_output_features_extra_max_length: u32, /// Maximum number of token elements permitted in covenants max_covenant_length: u32, /// Epoch duration in blocks @@ -238,15 +239,18 @@ impl ConsensusConstants { fn calculate_1_output_kernel_weight(&self) -> std::io::Result { let output_features = OutputFeatures { ..Default::default() }; + let max_extra_size = self.coinbase_output_features_extra_max_length() as usize; let features_and_scripts_size = self.transaction_weight.round_up_features_and_scripts_size( - output_features.get_serialized_size()? + - CoinBaseExtra::default().max_size() + - script![Nop].get_serialized_size()?, + output_features.get_serialized_size()? + max_extra_size + script![Nop].get_serialized_size()?, ); Ok(self.transaction_weight.calculate(1, 0, 1, features_and_scripts_size)) } + pub fn coinbase_output_features_extra_max_length(&self) -> u32 { + self.coinbase_output_features_extra_max_length + } + /// The amount of PoW algorithms used by the Tari chain. pub fn pow_algo_count(&self) -> u64 { self.proof_of_work.len() as u64 @@ -411,6 +415,7 @@ impl ConsensusConstants { vn_registration_min_deposit_amount: MicroMinotari(0), vn_registration_lock_height: 0, vn_registration_shuffle_interval: VnEpoch(100), + coinbase_output_features_extra_max_length: 64, }]; #[cfg(any(test, debug_assertions))] assert_hybrid_pow_constants(&consensus_constants, &[120], &[50], &[50]); @@ -476,6 +481,7 @@ impl ConsensusConstants { vn_registration_min_deposit_amount: MicroMinotari(0), vn_registration_lock_height: 0, vn_registration_shuffle_interval: VnEpoch(100), + coinbase_output_features_extra_max_length: 64, }]; #[cfg(any(test, debug_assertions))] assert_hybrid_pow_constants(&consensus_constants, &[target_time], &[randomx_split], &[sha3x_split]); @@ -532,6 +538,7 @@ impl ConsensusConstants { vn_registration_min_deposit_amount: MicroMinotari(0), vn_registration_lock_height: 0, vn_registration_shuffle_interval: VnEpoch(100), + coinbase_output_features_extra_max_length: 64, }]; #[cfg(any(test, debug_assertions))] assert_hybrid_pow_constants(&consensus_constants, &[120], &[50], &[50]); @@ -588,6 +595,7 @@ impl ConsensusConstants { vn_registration_min_deposit_amount: MicroMinotari(0), vn_registration_lock_height: 0, vn_registration_shuffle_interval: VnEpoch(100), + coinbase_output_features_extra_max_length: 64, }]; #[cfg(any(test, debug_assertions))] assert_hybrid_pow_constants(&consensus_constants, &[120], &[50], &[50]); @@ -638,6 +646,7 @@ impl ConsensusConstants { vn_registration_min_deposit_amount: MicroMinotari(0), vn_registration_lock_height: 0, vn_registration_shuffle_interval: VnEpoch(100), + coinbase_output_features_extra_max_length: 64, }]; #[cfg(any(test, debug_assertions))] assert_hybrid_pow_constants(&consensus_constants, &[120], &[50], &[50]); @@ -690,6 +699,7 @@ impl ConsensusConstants { vn_registration_min_deposit_amount: MicroMinotari(0), vn_registration_lock_height: 0, vn_registration_shuffle_interval: VnEpoch(100), + coinbase_output_features_extra_max_length: 64, }]; #[cfg(any(test, debug_assertions))] assert_hybrid_pow_constants(&consensus_constants, &[120], &[50], &[50]); diff --git a/base_layer/core/src/proof_of_work/monero_rx/pow_data.rs b/base_layer/core/src/proof_of_work/monero_rx/pow_data.rs index b313f72ab6..14ebf802e0 100644 --- a/base_layer/core/src/proof_of_work/monero_rx/pow_data.rs +++ b/base_layer/core/src/proof_of_work/monero_rx/pow_data.rs @@ -203,12 +203,22 @@ impl Display for MoneroPowData { #[cfg(test)] mod test { use borsh::{BorshDeserialize, BorshSerialize}; - use monero::{consensus::Encodable, BlockHeader, Hash, VarInt}; + use chacha20poly1305::aead::OsRng; + use monero::{blockdata::transaction::RawExtraField, consensus::Encodable, BlockHeader, Hash, VarInt}; + use tari_common::configuration::Network; + use tari_common_types::types::PrivateKey; + use tari_crypto::keys::SecretKey; use tari_utilities::ByteArray; use tiny_keccak::{Hasher, Keccak}; use super::MoneroPowData; - use crate::proof_of_work::monero_rx::{merkle_tree::MerkleProof, FixedByteArray}; + use crate::{ + consensus::NetworkConsensus, + proof_of_work::{ + monero_rx::{merkle_tree::MerkleProof, FixedByteArray}, + PowData, + }, + }; #[test] fn test_borsh_de_serialization() { @@ -249,4 +259,51 @@ mod test { MoneroPowData::deserialize(buf).unwrap(); assert_eq!(buf, &[1, 2, 3]); } + + #[test] + fn max_monero_pow_data_bytes_fits_inside_proof_of_work_pow_data() { + let coinbase: monero::Transaction = Default::default(); + let mut keccak = Keccak::v256(); + let mut encoder_prefix = Vec::new(); + coinbase.prefix.version.consensus_encode(&mut encoder_prefix).unwrap(); + coinbase + .prefix + .unlock_time + .consensus_encode(&mut encoder_prefix) + .unwrap(); + coinbase.prefix.inputs.consensus_encode(&mut encoder_prefix).unwrap(); + coinbase.prefix.outputs.consensus_encode(&mut encoder_prefix).unwrap(); + keccak.update(&encoder_prefix); + + for network in [ + Network::MainNet, + Network::StageNet, + Network::LocalNet, + Network::NextNet, + Network::Igor, + Network::Esmeralda, + ] { + for consensus_constants in NetworkConsensus::from(network).create_consensus_constants() { + let monero_pow_data = MoneroPowData { + header: BlockHeader { + major_version: VarInt(u64::MAX), + minor_version: VarInt(u64::MAX), + timestamp: VarInt(u64::MAX), + prev_id: Hash::new(PrivateKey::random(&mut OsRng).to_vec()), + nonce: u32::MAX, + }, + randomx_key: FixedByteArray::default(), + transaction_count: u16::MAX, + merkle_root: Hash::new(PrivateKey::random(&mut OsRng).to_vec()), + coinbase_merkle_proof: MerkleProof::default(), + coinbase_tx_extra: RawExtraField(vec![1u8; consensus_constants.max_extra_field_size()]), + coinbase_tx_hasher: keccak.clone(), + aux_chain_merkle_proof: MerkleProof::default(), + }; + let mut buf = Vec::new(); + monero_pow_data.serialize(&mut buf).unwrap(); + assert!(buf.len() <= PowData::default().max_size()); + } + } + } } diff --git a/base_layer/core/src/transactions/aggregated_body.rs b/base_layer/core/src/transactions/aggregated_body.rs index fb5da1aff3..9d4f798285 100644 --- a/base_layer/core/src/transactions/aggregated_body.rs +++ b/base_layer/core/src/transactions/aggregated_body.rs @@ -327,11 +327,18 @@ impl AggregateBody { Ok(()) } - pub fn verify_non_coinbase_has_coinbase_extra_empty(&self) -> Result<(), TransactionError> { + pub fn check_output_features(&self, max_coinbase_metadata_size: u32) -> Result<(), TransactionError> { for output in self.outputs() { if !output.is_coinbase() && !output.features.coinbase_extra.is_empty() { return Err(TransactionError::NonCoinbaseHasOutputFeaturesCoinbaseExtra); } + + if output.is_coinbase() && output.features.coinbase_extra.len() > max_coinbase_metadata_size as usize { + return Err(TransactionError::InvalidOutputFeaturesCoinbaseExtraSize { + len: output.features.coinbase_extra.len(), + max: max_coinbase_metadata_size, + }); + } } Ok(()) diff --git a/base_layer/core/src/validation/block_body/block_body_internal_validator.rs b/base_layer/core/src/validation/block_body/block_body_internal_validator.rs index 04d585bdec..ad75e45498 100644 --- a/base_layer/core/src/validation/block_body/block_body_internal_validator.rs +++ b/base_layer/core/src/validation/block_body/block_body_internal_validator.rs @@ -25,7 +25,7 @@ use tari_utilities::hex::Hex; use crate::{ blocks::Block, - consensus::ConsensusManager, + consensus::{ConsensusConstants, ConsensusManager}, transactions::{aggregated_body::AggregateBody, CryptoFactories}, validation::{ aggregate_body::AggregateBodyInternalConsistencyValidator, @@ -80,18 +80,23 @@ fn validate_block_specific_checks( consensus_manager: &ConsensusManager, factories: &CryptoFactories, ) -> Result<(), ValidationError> { + let constants = consensus_manager.consensus_constants(block.header.height); if block.header.height == 0 { warn!(target: LOG_TARGET, "Attempt to validate genesis block"); return Err(ValidationError::ValidatingGenesis); } check_coinbase_output(block, consensus_manager, factories)?; - check_coinbase_output_features(&block.body)?; + check_coinbase_output_features(&block.body, constants)?; Ok(()) } -fn check_coinbase_output_features(body: &AggregateBody) -> Result<(), ValidationError> { - body.verify_non_coinbase_has_coinbase_extra_empty() +fn check_coinbase_output_features( + body: &AggregateBody, + consensus_constants: &ConsensusConstants, +) -> Result<(), ValidationError> { + let max_coinbase_metadata_size = consensus_constants.coinbase_output_features_extra_max_length(); + body.check_output_features(max_coinbase_metadata_size) .map_err(ValidationError::from) } diff --git a/base_layer/core/src/validation/transaction/transaction_internal_validator.rs b/base_layer/core/src/validation/transaction/transaction_internal_validator.rs index 2953dcd602..7991352d06 100644 --- a/base_layer/core/src/validation/transaction/transaction_internal_validator.rs +++ b/base_layer/core/src/validation/transaction/transaction_internal_validator.rs @@ -77,7 +77,9 @@ impl TransactionInternalConsistencyValidator { return Err(ValidationError::OutputTypeNotPermitted { output_type: Coinbase }); } - tx.body.verify_non_coinbase_has_coinbase_extra_empty()?; + // We can call this function with a constant value, because we've just shown that this is NOT a coinbase, and + // only coinbases may have the extra field set (the only field that the fn argument affects). + tx.body.check_output_features(1)?; self.aggregate_body_validator.validate( &tx.body,