From 3d429ff2ef0b29ca8a0baf659ead35b4d20a7ffb Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Fri, 13 Aug 2021 11:27:33 +0200 Subject: [PATCH 1/3] Change kernel ordering --- base_layer/core/src/transactions/transaction.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/base_layer/core/src/transactions/transaction.rs b/base_layer/core/src/transactions/transaction.rs index 5492381c49..05ee7b0e51 100644 --- a/base_layer/core/src/transactions/transaction.rs +++ b/base_layer/core/src/transactions/transaction.rs @@ -926,7 +926,7 @@ impl From> for FullRewindResult { /// [Mimblewimble TLU post](https://tlu.tarilabs.com/protocols/mimblewimble-1/sources/PITCHME.link.html?highlight=mimblewimble#mimblewimble). /// The kernel also tracks other transaction metadata, such as the lock height for the transaction (i.e. the earliest /// this transaction can be mined) and the transaction fee, in cleartext. -#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct TransactionKernel { /// Options for a kernel's structure or use pub features: KernelFeatures, @@ -1072,6 +1072,18 @@ impl Display for TransactionKernel { } } +impl PartialOrd for TransactionKernel { + fn partial_cmp(&self, other: &Self) -> Option { + self.excess_sig.partial_cmp(&other.excess_sig) + } +} + +impl Ord for TransactionKernel { + fn cmp(&self, other: &Self) -> Ordering { + self.excess_sig.cmp(&other.excess_sig) + } +} + /// This struct holds the result of calculating the sum of the kernels in a Transaction /// and returns the summed commitments and the total fees #[derive(Default)] From 885a39ccafc7fc5f417325cb9545c5d6018da39b Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Tue, 17 Aug 2021 08:52:49 +0200 Subject: [PATCH 2/3] Change input_mmr to be of type merklemountain range Co-authored-by: Stan Bondi --- base_layer/core/src/chain_storage/blockchain_database.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/base_layer/core/src/chain_storage/blockchain_database.rs b/base_layer/core/src/chain_storage/blockchain_database.rs index 97f83da7e0..22ebe57d1b 100644 --- a/base_layer/core/src/chain_storage/blockchain_database.rs +++ b/base_layer/core/src/chain_storage/blockchain_database.rs @@ -66,7 +66,7 @@ use std::{ collections::VecDeque, convert::TryFrom, mem, - ops::Bound, + ops::{Bound, RangeBounds}, sync::{Arc, RwLock, RwLockReadGuard, RwLockWriteGuard}, time::Instant, }; @@ -75,8 +75,7 @@ use tari_common_types::{ types::{BlockHash, Commitment, HashDigest, HashOutput, Signature}, }; use tari_crypto::tari_utilities::{hex::Hex, ByteArray, Hashable}; -use tari_mmr::{MerkleMountainRange, MutableMmr}; -use uint::static_assertions::_core::ops::RangeBounds; +use tari_mmr::{pruned_hashset::PrunedHashSet, MerkleMountainRange, MutableMmr}; const LOG_TARGET: &str = "c::cs::database"; @@ -995,7 +994,7 @@ pub fn calculate_mmr_roots(db: &T, block: &Block) -> Resul let mut kernel_mmr = MerkleMountainRange::::new(kernels); let mut output_mmr = MutableMmr::::new(outputs, deleted)?; let mut witness_mmr = MerkleMountainRange::::new(range_proofs); - let mut input_mmr = MutableMmr::::new(Vec::new(), Bitmap::create())?; + let mut input_mmr = MerkleMountainRange::::new(PrunedHashSet::default()); for kernel in body.kernels().iter() { kernel_mmr.push(kernel.hash())?; From ed13c22ad90028486df52923ed3115ea80d8e23a Mon Sep 17 00:00:00 2001 From: Stanimal Date: Mon, 20 Sep 2021 16:22:29 +0400 Subject: [PATCH 3/3] refactor: implement breaking consensus for block v2 Block v1 (compatible with weatherwax at all heights): - Kernel ordering using every kernel field - Input MR commits to empty serialization of bitmap Block v2 (Igor, breaking change) - Kernel ordering (#3193) - Input MR uses input hashes only without extaneous empty bitmap bytes (#3195) --- base_layer/core/src/blocks/block.rs | 6 +- base_layer/core/src/blocks/genesis_block.rs | 8 +- .../src/chain_storage/blockchain_database.rs | 12 +- .../core/src/consensus/consensus_constants.rs | 24 +-- base_layer/core/src/proto/transaction.rs | 3 +- .../core/src/transactions/aggregated_body.rs | 19 ++- .../core/src/transactions/transaction.rs | 142 +++++++++--------- .../transaction_protocol/sender.rs | 20 ++- .../block_validators/async_validator.rs | 20 ++- .../src/validation/block_validators/orphan.rs | 4 +- base_layer/core/src/validation/error.rs | 2 + base_layer/core/src/validation/helpers.rs | 49 +++++- base_layer/core/tests/block_validation.rs | 4 - .../core/tests/helpers/block_builders.rs | 2 +- base_layer/core/tests/node_service.rs | 8 +- 15 files changed, 202 insertions(+), 121 deletions(-) diff --git a/base_layer/core/src/blocks/block.rs b/base_layer/core/src/blocks/block.rs index abd0a316b0..15c4d9c8ef 100644 --- a/base_layer/core/src/blocks/block.rs +++ b/base_layer/core/src/blocks/block.rs @@ -81,6 +81,10 @@ impl Block { Self { header, body } } + pub fn version(&self) -> u16 { + self.header.version + } + /// This function will calculate the total fees contained in a block pub fn calculate_fees(&self) -> MicroTari { self.body.kernels().iter().fold(0.into(), |sum, x| sum + x.fee) @@ -239,7 +243,7 @@ impl BlockBuilder { header: self.header, body: AggregateBody::new(self.inputs, self.outputs, self.kernels), }; - block.body.sort(); + block.body.sort(block.header.version); block } } diff --git a/base_layer/core/src/blocks/genesis_block.rs b/base_layer/core/src/blocks/genesis_block.rs index 861b3e8650..9f03977a4b 100644 --- a/base_layer/core/src/blocks/genesis_block.rs +++ b/base_layer/core/src/blocks/genesis_block.rs @@ -159,7 +159,7 @@ pub fn get_stibbons_genesis_block_raw() -> Block { excess_sig: sig, }], ); - body.sort(); + body.sort(1); Block { header: BlockHeader { version: 0, @@ -226,7 +226,7 @@ pub fn get_weatherwax_genesis_block_raw() -> Block { excess_sig: sig, }], ); - body.sort(); + body.sort(1); // set genesis timestamp let genesis = DateTime::parse_from_rfc2822("07 Jul 2021 06:00:00 +0200").unwrap(); let timestamp = genesis.timestamp() as u64; @@ -336,7 +336,7 @@ pub fn get_ridcully_genesis_block_raw() -> Block { excess_sig: sig, }], ); - body.sort(); + body.sort(1); Block { header: BlockHeader { version: 0, @@ -419,7 +419,7 @@ pub fn get_igor_genesis_block_raw() -> Block { excess_sig: sig, }], ); - body.sort(); + body.sort(2); // set genesis timestamp let genesis = DateTime::parse_from_rfc2822("27 Aug 2021 06:00:00 +0200").unwrap(); let timestamp = genesis.timestamp() as u64; diff --git a/base_layer/core/src/chain_storage/blockchain_database.rs b/base_layer/core/src/chain_storage/blockchain_database.rs index 22ebe57d1b..9837720147 100644 --- a/base_layer/core/src/chain_storage/blockchain_database.rs +++ b/base_layer/core/src/chain_storage/blockchain_database.rs @@ -228,7 +228,6 @@ where B: BlockchainBackend /// Returns a reference to the consensus cosntants at the current height pub fn consensus_constants(&self) -> Result<&ConsensusConstants, ChainStorageError> { let height = self.get_height()?; - Ok(self.consensus_manager.consensus_constants(height)) } @@ -648,7 +647,7 @@ where B: BlockchainBackend pub fn prepare_new_block(&self, template: NewBlockTemplate) -> Result { let NewBlockTemplate { header, mut body, .. } = template; - body.sort(); + body.sort(header.version); let mut header = BlockHeader::from(header); // If someone advanced the median timestamp such that the local time is less than the median timestamp, we need // to increase the timestamp to be greater than the median timestamp @@ -1051,10 +1050,17 @@ pub fn calculate_mmr_roots(db: &T, block: &Block) -> Resul output_mmr.compress(); + // TODO: #testnetreset clean up this code + let input_mr = if header.version == 1 { + MutableMmr::::new(input_mmr.get_pruned_hash_set()?, Bitmap::create())?.get_merkle_root()? + } else { + input_mmr.get_merkle_root()? + }; + let mmr_roots = MmrRoots { kernel_mr: kernel_mmr.get_merkle_root()?, kernel_mmr_size: kernel_mmr.get_leaf_count()? as u64, - input_mr: input_mmr.get_merkle_root()?, + input_mr, output_mr: output_mmr.get_merkle_root()?, output_mmr_size: output_mmr.get_leaf_count() as u64, witness_mr: witness_mmr.get_merkle_root()?, diff --git a/base_layer/core/src/consensus/consensus_constants.rs b/base_layer/core/src/consensus/consensus_constants.rs index b8fb2f0aa5..e06c1f6ee2 100644 --- a/base_layer/core/src/consensus/consensus_constants.rs +++ b/base_layer/core/src/consensus/consensus_constants.rs @@ -209,7 +209,7 @@ impl ConsensusConstants { emission_initial: 5_538_846_115 * uT, emission_decay: &EMISSION_DECAY, emission_tail: 100.into(), - max_randomx_seed_height: std::u64::MAX, + max_randomx_seed_height: u64::MAX, proof_of_work: algos, faucet_value: (5000 * 4000) * T, }] @@ -218,7 +218,7 @@ impl ConsensusConstants { pub fn ridcully() -> Vec { let difficulty_block_window = 90; let mut algos = HashMap::new(); - // seting sha3/monero to 40/60 split + // setting sha3/monero to 40/60 split algos.insert(PowAlgorithm::Sha3, PowAlgorithmConstants { max_target_time: 1800, min_difficulty: 60_000_000.into(), @@ -242,7 +242,7 @@ impl ConsensusConstants { emission_initial: 5_538_846_115 * uT, emission_decay: &EMISSION_DECAY, emission_tail: 100.into(), - max_randomx_seed_height: std::u64::MAX, + max_randomx_seed_height: u64::MAX, proof_of_work: algos, faucet_value: (5000 * 4000) * T, }] @@ -277,7 +277,7 @@ impl ConsensusConstants { target_time: 200, }); let mut algos2 = HashMap::new(); - // seting sha3/monero to 40/60 split + // setting sha3/monero to 40/60 split algos2.insert(PowAlgorithm::Sha3, PowAlgorithmConstants { max_target_time: 1800, min_difficulty: 60_000_000.into(), @@ -302,7 +302,7 @@ impl ConsensusConstants { emission_initial: 5_538_846_115 * uT, emission_decay: &EMISSION_DECAY, emission_tail: 100.into(), - max_randomx_seed_height: std::u64::MAX, + max_randomx_seed_height: u64::MAX, proof_of_work: algos, faucet_value: (5000 * 4000) * T, }, @@ -317,7 +317,7 @@ impl ConsensusConstants { emission_initial: 5_538_846_115 * uT, emission_decay: &EMISSION_DECAY, emission_tail: 100.into(), - max_randomx_seed_height: std::u64::MAX, + max_randomx_seed_height: u64::MAX, proof_of_work: algos2, faucet_value: (5000 * 4000) * T, }, @@ -326,7 +326,7 @@ impl ConsensusConstants { pub fn weatherwax() -> Vec { let mut algos = HashMap::new(); - // seting sha3/monero to 40/60 split + // setting sha3/monero to 40/60 split algos.insert(PowAlgorithm::Sha3, PowAlgorithmConstants { max_target_time: 1800, min_difficulty: 60_000_000.into(), @@ -350,7 +350,7 @@ impl ConsensusConstants { emission_initial: 5_538_846_115 * uT, emission_decay: &EMISSION_DECAY, emission_tail: 100.into(), - max_randomx_seed_height: std::u64::MAX, + max_randomx_seed_height: u64::MAX, proof_of_work: algos, faucet_value: (5000 * 4000) * T, }] @@ -358,7 +358,7 @@ impl ConsensusConstants { pub fn igor() -> Vec { let mut algos = HashMap::new(); - // seting sha3/monero to 40/60 split + // setting sha3/monero to 40/60 split algos.insert(PowAlgorithm::Sha3, PowAlgorithmConstants { max_target_time: 1800, min_difficulty: 60_000_000.into(), @@ -374,7 +374,7 @@ impl ConsensusConstants { vec![ConsensusConstants { effective_from_height: 0, coinbase_lock_height: 6, - blockchain_version: 1, + blockchain_version: 2, future_time_limit: 540, difficulty_block_window: 90, max_block_transaction_weight: 19500, @@ -382,7 +382,7 @@ impl ConsensusConstants { emission_initial: 5_538_846_115 * uT, emission_decay: &EMISSION_DECAY, emission_tail: 100.into(), - max_randomx_seed_height: std::u64::MAX, + max_randomx_seed_height: u64::MAX, proof_of_work: algos, faucet_value: (5000 * 4000) * T, }] @@ -415,7 +415,7 @@ impl ConsensusConstants { emission_initial: 10_000_000.into(), emission_decay: &EMISSION_DECAY, emission_tail: 100.into(), - max_randomx_seed_height: std::u64::MAX, + max_randomx_seed_height: u64::MAX, proof_of_work: algos, faucet_value: MicroTari::from(0), }] diff --git a/base_layer/core/src/proto/transaction.rs b/base_layer/core/src/proto/transaction.rs index d500157360..93bbfbd46c 100644 --- a/base_layer/core/src/proto/transaction.rs +++ b/base_layer/core/src/proto/transaction.rs @@ -223,8 +223,7 @@ impl TryFrom for AggregateBody { let inputs = try_convert_all(body.inputs)?; let outputs = try_convert_all(body.outputs)?; let kernels = try_convert_all(body.kernels)?; - let mut body = AggregateBody::new(inputs, outputs, kernels); - body.sort(); + let body = AggregateBody::new(inputs, outputs, kernels); Ok(body) } } diff --git a/base_layer/core/src/transactions/aggregated_body.rs b/base_layer/core/src/transactions/aggregated_body.rs index 4a01acadcd..53b37fedfc 100644 --- a/base_layer/core/src/transactions/aggregated_body.rs +++ b/base_layer/core/src/transactions/aggregated_body.rs @@ -45,7 +45,7 @@ pub const LOG_TARGET: &str = "c::tx::aggregated_body"; /// The components of the block or transaction. The same struct can be used for either, since in Mimblewimble, /// cut-through means that blocks and transactions have the same structure. -#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct AggregateBody { sorted: bool, /// List of inputs spent by the transaction. @@ -198,13 +198,18 @@ impl AggregateBody { } /// Sort the component lists of the aggregate body - pub fn sort(&mut self) { + pub fn sort(&mut self, version: u16) { if self.sorted { return; } self.inputs.sort(); self.outputs.sort(); - self.kernels.sort(); + // TODO: #testnetreset clean up this code + if version <= 1 { + self.kernels.sort_by(|a, b| a.deprecated_cmp(b)); + } else { + self.kernels.sort(); + } self.sorted = true; } @@ -471,6 +476,14 @@ impl AggregateBody { } } +impl PartialEq for AggregateBody { + fn eq(&self, other: &Self) -> bool { + self.kernels == other.kernels && self.inputs == other.inputs && self.outputs == other.outputs + } +} + +impl Eq for AggregateBody {} + /// This will strip away the offset of the transaction returning a pure aggregate body impl From for AggregateBody { fn from(transaction: Transaction) -> Self { diff --git a/base_layer/core/src/transactions/transaction.rs b/base_layer/core/src/transactions/transaction.rs index 05ee7b0e51..34f2c4d8a9 100644 --- a/base_layer/core/src/transactions/transaction.rs +++ b/base_layer/core/src/transactions/transaction.rs @@ -947,6 +947,77 @@ impl TransactionKernel { pub fn is_coinbase(&self) -> bool { self.features.contains(KernelFeatures::COINBASE_KERNEL) } + + pub fn verify_signature(&self) -> Result<(), TransactionError> { + let excess = self.excess.as_public_key(); + let r = self.excess_sig.get_public_nonce(); + let m = TransactionMetadata { + lock_height: self.lock_height, + fee: self.fee, + }; + let c = build_challenge(r, &m); + if self.excess_sig.verify_challenge(excess, &c) { + Ok(()) + } else { + Err(TransactionError::InvalidSignatureError( + "Verifying kernel signature".to_string(), + )) + } + } + + /// This method was used to sort kernels. It has been replaced, and will be removed in future + pub fn deprecated_cmp(&self, other: &Self) -> Ordering { + self.features + .cmp(&other.features) + .then(self.fee.cmp(&other.fee)) + .then(self.lock_height.cmp(&other.lock_height)) + .then(self.excess.cmp(&other.excess)) + .then(self.excess_sig.cmp(&other.excess_sig)) + } +} + +impl Hashable for TransactionKernel { + /// Produce a canonical hash for a transaction kernel. The hash is given by + /// $$ H(feature_bits | fee | lock_height | P_excess | R_sum | s_sum) + fn hash(&self) -> Vec { + HashDigest::new() + .chain(&[self.features.bits]) + .chain(u64::from(self.fee).to_le_bytes()) + .chain(self.lock_height.to_le_bytes()) + .chain(self.excess.as_bytes()) + .chain(self.excess_sig.get_public_nonce().as_bytes()) + .chain(self.excess_sig.get_signature().as_bytes()) + .finalize() + .to_vec() + } +} + +impl Display for TransactionKernel { + fn fmt(&self, fmt: &mut Formatter<'_>) -> Result<(), std::fmt::Error> { + let msg = format!( + "Fee: {}\nLock height: {}\nFeatures: {:?}\nExcess: {}\nExcess signature: {}\n", + self.fee, + self.lock_height, + self.features, + self.excess.to_hex(), + self.excess_sig + .to_json() + .unwrap_or_else(|_| "Failed to serialize signature".into()), + ); + fmt.write_str(&msg) + } +} + +impl PartialOrd for TransactionKernel { + fn partial_cmp(&self, other: &Self) -> Option { + self.excess_sig.partial_cmp(&other.excess_sig) + } +} + +impl Ord for TransactionKernel { + fn cmp(&self, other: &Self) -> Ordering { + self.excess_sig.cmp(&other.excess_sig) + } } /// A version of Transaction kernel with optional fields. This struct is only used in constructing transaction kernels @@ -1021,69 +1092,6 @@ impl Default for KernelBuilder { } } -impl TransactionKernel { - pub fn verify_signature(&self) -> Result<(), TransactionError> { - let excess = self.excess.as_public_key(); - let r = self.excess_sig.get_public_nonce(); - let m = TransactionMetadata { - lock_height: self.lock_height, - fee: self.fee, - }; - let c = build_challenge(r, &m); - if self.excess_sig.verify_challenge(excess, &c) { - Ok(()) - } else { - Err(TransactionError::InvalidSignatureError( - "Verifying kernel signature".to_string(), - )) - } - } -} - -impl Hashable for TransactionKernel { - /// Produce a canonical hash for a transaction kernel. The hash is given by - /// $$ H(feature_bits | fee | lock_height | P_excess | R_sum | s_sum) - fn hash(&self) -> Vec { - HashDigest::new() - .chain(&[self.features.bits]) - .chain(u64::from(self.fee).to_le_bytes()) - .chain(self.lock_height.to_le_bytes()) - .chain(self.excess.as_bytes()) - .chain(self.excess_sig.get_public_nonce().as_bytes()) - .chain(self.excess_sig.get_signature().as_bytes()) - .finalize() - .to_vec() - } -} - -impl Display for TransactionKernel { - fn fmt(&self, fmt: &mut Formatter<'_>) -> Result<(), std::fmt::Error> { - let msg = format!( - "Fee: {}\nLock height: {}\nFeatures: {:?}\nExcess: {}\nExcess signature: {}\n", - self.fee, - self.lock_height, - self.features, - self.excess.to_hex(), - self.excess_sig - .to_json() - .unwrap_or_else(|_| "Failed to serialize signature".into()), - ); - fmt.write_str(&msg) - } -} - -impl PartialOrd for TransactionKernel { - fn partial_cmp(&self, other: &Self) -> Option { - self.excess_sig.partial_cmp(&other.excess_sig) - } -} - -impl Ord for TransactionKernel { - fn cmp(&self, other: &Self) -> Ordering { - self.excess_sig.cmp(&other.excess_sig) - } -} - /// This struct holds the result of calculating the sum of the kernels in a Transaction /// and returns the summed commitments and the total fees #[derive(Default)] @@ -1119,12 +1127,10 @@ impl Transaction { kernels: Vec, offset: BlindingFactor, script_offset: BlindingFactor, - ) -> Transaction { - let mut body = AggregateBody::new(inputs, outputs, kernels); - body.sort(); - Transaction { + ) -> Self { + Self { offset, - body, + body: AggregateBody::new(inputs, outputs, kernels), script_offset, } } diff --git a/base_layer/core/src/transactions/transaction_protocol/sender.rs b/base_layer/core/src/transactions/transaction_protocol/sender.rs index c91097d32a..c9a420678e 100644 --- a/base_layer/core/src/transactions/transaction_protocol/sender.rs +++ b/base_layer/core/src/transactions/transaction_protocol/sender.rs @@ -20,17 +20,6 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use std::fmt; - -use digest::Digest; -use serde::{Deserialize, Serialize}; -use tari_crypto::{ - keys::PublicKey as PublicKeyTrait, - ristretto::pedersen::{PedersenCommitment, PedersenCommitmentFactory}, - script::TariScript, - tari_utilities::ByteArray, -}; - use crate::transactions::{ crypto_factories::CryptoFactories, tari_amount::*, @@ -55,7 +44,16 @@ use crate::transactions::{ TransactionProtocolError as TPE, }, }; +use digest::Digest; +use serde::{Deserialize, Serialize}; +use std::fmt; use tari_common_types::types::{BlindingFactor, ComSignature, PrivateKey, PublicKey, RangeProofService, Signature}; +use tari_crypto::{ + keys::PublicKey as PublicKeyTrait, + ristretto::pedersen::{PedersenCommitment, PedersenCommitmentFactory}, + script::TariScript, + tari_utilities::ByteArray, +}; //---------------------------------------- Local Data types ----------------------------------------------------// diff --git a/base_layer/core/src/validation/block_validators/async_validator.rs b/base_layer/core/src/validation/block_validators/async_validator.rs index c563d12c15..1547027cf1 100644 --- a/base_layer/core/src/validation/block_validators/async_validator.rs +++ b/base_layer/core/src/validation/block_validators/async_validator.rs @@ -41,7 +41,7 @@ use crate::{ use async_trait::async_trait; use futures::{stream::FuturesUnordered, StreamExt}; use log::*; -use std::{cmp, thread, time::Instant}; +use std::{cmp, cmp::Ordering, thread, time::Instant}; use tari_common_types::types::{Commitment, HashOutput, PublicKey}; use tari_crypto::commitment::HomomorphicCommitmentFactory; use tokio::task; @@ -138,6 +138,15 @@ impl BlockValidator { kernels: Vec, ) -> AbortOnDropJoinHandle> { let height = header.height; + let block_version = header.version; + let kernel_comparer = move |a: &TransactionKernel, b: &TransactionKernel| -> Ordering { + if block_version == 1 { + a.deprecated_cmp(b) + } else { + a.cmp(b) + } + }; + let total_kernel_offset = header.total_kernel_offset.clone(); let total_reward = self.rules.calculate_coinbase_and_fees(height, &kernels); let total_offset = self @@ -155,6 +164,15 @@ impl BlockValidator { let mut coinbase_index = None; let mut max_kernel_timelock = 0; for (i, kernel) in kernels.iter().enumerate() { + if i > 0 && + matches!( + kernel_comparer(kernel, &kernels[i - 1]), + Ordering::Equal | Ordering::Less + ) + { + return Err(ValidationError::UnsortedOrDuplicateKernel); + } + kernel.verify_signature()?; if kernel.is_coinbase() { diff --git a/base_layer/core/src/validation/block_validators/orphan.rs b/base_layer/core/src/validation/block_validators/orphan.rs index 6d73d984fe..7c5ae09540 100644 --- a/base_layer/core/src/validation/block_validators/orphan.rs +++ b/base_layer/core/src/validation/block_validators/orphan.rs @@ -81,10 +81,10 @@ impl OrphanValidation for OrphanBlockValidator { "Checking duplicate inputs and outputs on {}", block_id ); - check_sorting_and_duplicates(&block.body)?; + check_sorting_and_duplicates(&block)?; trace!( target: LOG_TARGET, - "SV - No duplicate inputs or outputs for {} ", + "SV - No duplicate inputs / outputs for {} ", &block_id ); diff --git a/base_layer/core/src/validation/error.rs b/base_layer/core/src/validation/error.rs index 4202b3c0b5..10f5225d04 100644 --- a/base_layer/core/src/validation/error.rs +++ b/base_layer/core/src/validation/error.rs @@ -71,6 +71,8 @@ pub enum ValidationError { UnsortedOrDuplicateInput, #[error("Duplicate or unsorted output found in block body")] UnsortedOrDuplicateOutput, + #[error("Duplicate or unsorted kernel found in block body")] + UnsortedOrDuplicateKernel, #[error("Error in merge mine data:{0}")] MergeMineError(#[from] MergeMineError), #[error("Contains an input with an invalid mined-height in body")] diff --git a/base_layer/core/src/validation/helpers.rs b/base_layer/core/src/validation/helpers.rs index 1092ab4fca..5a64537135 100644 --- a/base_layer/core/src/validation/helpers.rs +++ b/base_layer/core/src/validation/helpers.rs @@ -50,6 +50,7 @@ use crate::{ }, validation::ValidationError, }; +use std::cmp::Ordering; use tari_common_types::types::{Commitment, CommitmentFactory, PublicKey}; use tari_crypto::keys::PublicKey as PublicKeyTrait; @@ -259,14 +260,14 @@ pub fn check_coinbase_output( .map_err(ValidationError::from) } -pub fn is_all_unique_and_sorted, T: PartialOrd>(items: I) -> bool { - let items = items.as_ref(); - if items.is_empty() { +pub fn is_all_unique_and_sorted<'a, I: IntoIterator, T: PartialOrd + 'a>(items: I) -> bool { + let mut items = items.into_iter(); + let prev_item = items.next(); + if prev_item.is_none() { return true; } - - let mut prev_item = &items[0]; - for item in items.iter().skip(1) { + let mut prev_item = prev_item.unwrap(); + for item in items { if item <= prev_item { return false; } @@ -277,7 +278,8 @@ pub fn is_all_unique_and_sorted, T: PartialOrd>(items: I) -> bool } // This function checks for duplicate inputs and outputs. There should be no duplicate inputs or outputs in a block -pub fn check_sorting_and_duplicates(body: &AggregateBody) -> Result<(), ValidationError> { +pub fn check_sorting_and_duplicates(block: &Block) -> Result<(), ValidationError> { + let body = &block.body; if !is_all_unique_and_sorted(body.inputs()) { return Err(ValidationError::UnsortedOrDuplicateInput); } @@ -285,9 +287,42 @@ pub fn check_sorting_and_duplicates(body: &AggregateBody) -> Result<(), Validati return Err(ValidationError::UnsortedOrDuplicateOutput); } + if block.version() == 1 { + let wrapped = body + .kernels() + .iter() + .map(KernelDeprecatedOrdWrapper::new) + .collect::>(); + if !is_all_unique_and_sorted(&wrapped) { + return Err(ValidationError::UnsortedOrDuplicateKernel); + } + } else if !is_all_unique_and_sorted(body.kernels()) { + return Err(ValidationError::UnsortedOrDuplicateKernel); + } + Ok(()) } +#[derive(PartialEq, Eq)] +struct KernelDeprecatedOrdWrapper<'a> { + kernel: &'a TransactionKernel, +} +impl<'a> KernelDeprecatedOrdWrapper<'a> { + pub fn new(kernel: &'a TransactionKernel) -> Self { + Self { kernel } + } +} +impl PartialOrd for KernelDeprecatedOrdWrapper<'_> { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.kernel.deprecated_cmp(&other.kernel)) + } +} +impl Ord for KernelDeprecatedOrdWrapper<'_> { + fn cmp(&self, other: &Self) -> Ordering { + self.kernel.deprecated_cmp(&other.kernel) + } +} + /// This function checks that all inputs in the blocks are valid UTXO's to be spent pub fn check_inputs_are_utxos(db: &B, body: &AggregateBody) -> Result<(), ValidationError> { let mut not_found_inputs = Vec::new(); diff --git a/base_layer/core/tests/block_validation.rs b/base_layer/core/tests/block_validation.rs index 673886c9c4..8b7303ed47 100644 --- a/base_layer/core/tests/block_validation.rs +++ b/base_layer/core/tests/block_validation.rs @@ -482,7 +482,6 @@ OutputFeatures::default()), unblinded_utxo2.as_transaction_input(&factories.commitment).unwrap(), ]; new_block.body = AggregateBody::new(inputs, template.body.outputs().clone(), template.body.kernels().clone()); - new_block.body.sort(); new_block.header.nonce = OsRng.next_u64(); find_header_with_achieved_difficulty(&mut new_block.header, 10.into()); @@ -513,7 +512,6 @@ OutputFeatures::default()), // signatures. let inputs = vec![new_block.body.inputs()[0].clone(), new_block.body.inputs()[0].clone()]; new_block.body = AggregateBody::new(inputs, template.body.outputs().clone(), template.body.kernels().clone()); - new_block.body.sort(); new_block.header.nonce = OsRng.next_u64(); find_header_with_achieved_difficulty(&mut new_block.header, 10.into()); @@ -769,7 +767,6 @@ async fn test_block_sync_body_validator() { unblinded_utxo2.as_transaction_input(&factories.commitment).unwrap(), ]; new_block.body = AggregateBody::new(inputs, template.body.outputs().clone(), template.body.kernels().clone()); - new_block.body.sort(); validator.validate_body(new_block).await.unwrap_err(); // lets check duplicate txos @@ -779,7 +776,6 @@ async fn test_block_sync_body_validator() { // signatures. let inputs = vec![new_block.body.inputs()[0].clone(), new_block.body.inputs()[0].clone()]; new_block.body = AggregateBody::new(inputs, template.body.outputs().clone(), template.body.kernels().clone()); - new_block.body.sort(); validator.validate_body(new_block).await.unwrap_err(); // let break coinbase value diff --git a/base_layer/core/tests/helpers/block_builders.rs b/base_layer/core/tests/helpers/block_builders.rs index a0203e9852..3c13ff5c82 100644 --- a/base_layer/core/tests/helpers/block_builders.rs +++ b/base_layer/core/tests/helpers/block_builders.rs @@ -154,7 +154,7 @@ pub fn create_genesis_block( fn update_genesis_block_mmr_roots(template: NewBlockTemplate) -> Result { let NewBlockTemplate { header, mut body, .. } = template; // Make sure the body components are sorted. If they already are, this is a very cheap call. - body.sort(); + body.sort(header.version); let kernel_hashes: Vec = body.kernels().iter().map(|k| k.hash()).collect(); let out_hashes: Vec = body.outputs().iter().map(|out| out.hash()).collect(); let rp_hashes: Vec = body.outputs().iter().map(|out| out.witness_hash()).collect(); diff --git a/base_layer/core/tests/node_service.rs b/base_layer/core/tests/node_service.rs index b03b2e291f..c064fe9518 100644 --- a/base_layer/core/tests/node_service.rs +++ b/base_layer/core/tests/node_service.rs @@ -716,7 +716,9 @@ async fn local_get_new_block_with_zero_conf() { ); block_template.body.add_kernel(kernel); block_template.body.add_output(output); - block_template.body.sort(); + block_template + .body + .sort(rules.consensus_constants(0).blockchain_version()); let block = node.local_nci.get_new_block(block_template.clone()).await.unwrap(); assert_eq!(block.header.height, 1); assert_eq!(block.body, block_template.body); @@ -788,7 +790,9 @@ async fn local_get_new_block_with_combined_transaction() { ); block_template.body.add_kernel(kernel); block_template.body.add_output(output); - block_template.body.sort(); + block_template + .body + .sort(rules.consensus_constants(0).blockchain_version()); let block = node.local_nci.get_new_block(block_template.clone()).await.unwrap(); assert_eq!(block.header.height, 1); assert_eq!(block.body, block_template.body);