From 46e7697d2ddc17eb44dff220df8082ff423e1462 Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Fri, 15 Jul 2022 12:24:54 +0200 Subject: [PATCH] Fixes a potential validation bug Ensures all validators check this --- .../block_validators/async_validator.rs | 20 +++++++++++++-- .../validation/block_validators/body_only.rs | 4 ++- base_layer/core/src/validation/helpers.rs | 20 +++++++++++++++ .../src/validation/transaction_validators.rs | 25 ++++++++----------- 4 files changed, 52 insertions(+), 17 deletions(-) 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 7ac03a2d5f..e7aaceb914 100644 --- a/base_layer/core/src/validation/block_validators/async_validator.rs +++ b/base_layer/core/src/validation/block_validators/async_validator.rs @@ -27,7 +27,7 @@ use log::*; use tari_common_types::types::{Commitment, HashOutput, PublicKey}; use tari_crypto::commitment::HomomorphicCommitmentFactory; use tari_script::ScriptContext; -use tari_utilities::Hashable; +use tari_utilities::{hex::Hex, Hashable}; use tokio::task; use super::LOG_TARGET; @@ -176,8 +176,9 @@ impl BlockValidator { .factories .commitment .commit_value(&total_kernel_offset, total_reward.as_u64()); - + let db = self.db.inner().clone(); task::spawn_blocking(move || { + let db = db.db_read_access()?; let timer = Instant::now(); let mut kernel_sum = KernelSum { sum: total_offset, @@ -204,6 +205,21 @@ impl BlockValidator { coinbase_index = Some(i); } + if let Some((db_kernel, header_hash)) = db.fetch_kernel_by_excess_sig(&kernel.excess_sig)? { + let msg = format!( + "Block contains kernel excess: {} which matches already existing excess signature in chain \ + database block hash: {}. Existing kernel excess: {}, excess sig nonce: {}, excess signature: \ + {}", + kernel.excess.to_hex(), + header_hash.to_hex(), + db_kernel.excess.to_hex(), + db_kernel.excess_sig.get_public_nonce().to_hex(), + db_kernel.excess_sig.get_signature().to_hex(), + ); + warn!(target: LOG_TARGET, "{}", msg); + return Err(ValidationError::ConsensusError(msg)); + }; + max_kernel_timelock = cmp::max(max_kernel_timelock, kernel.lock_height); kernel_sum.fees += kernel.fee; kernel_sum.sum = &kernel_sum.sum + &kernel.excess; diff --git a/base_layer/core/src/validation/block_validators/body_only.rs b/base_layer/core/src/validation/block_validators/body_only.rs index e538b177fb..5f957c826a 100644 --- a/base_layer/core/src/validation/block_validators/body_only.rs +++ b/base_layer/core/src/validation/block_validators/body_only.rs @@ -53,6 +53,7 @@ impl PostOrphanBodyValidation for BodyOnlyValidator { /// 1. Does the block satisfy the stateless checks? /// 1. Are all inputs currently in the UTXO set? /// 1. Are all inputs and outputs not in the STXO set? + /// 1. Are all kernels excesses unique? /// 1. Are the block header MMR roots valid? fn validate_body_for_valid_orphan( &self, @@ -80,9 +81,10 @@ impl PostOrphanBodyValidation for BodyOnlyValidator { self.rules.consensus_constants(block.height()), &block.block().body, )?; + helpers::check_unique_kernels(backend, &block.block().body)?; trace!( target: LOG_TARGET, - "Block validation: All inputs and outputs are valid for {}", + "Block validation: All inputs, outputs and kernels are valid for {}", block_id ); let mmr_roots = chain_storage::calculate_mmr_roots(backend, block.block())?; diff --git a/base_layer/core/src/validation/helpers.rs b/base_layer/core/src/validation/helpers.rs index baaaad5203..55a6d25626 100644 --- a/base_layer/core/src/validation/helpers.rs +++ b/base_layer/core/src/validation/helpers.rs @@ -671,6 +671,26 @@ pub fn check_not_bad_block(db: &B, hash: &[u8]) -> Result< Ok(()) } +/// This checks to ensure that every kernel included in the block is a unique kernel in the block chain. +pub fn check_unique_kernels(db: &B, block_body: &AggregateBody) -> Result<(), ValidationError> { + for kernel in block_body.kernels() { + if let Some((db_kernel, header_hash)) = db.fetch_kernel_by_excess_sig(&kernel.excess_sig)? { + let msg = format!( + "Block contains kernel excess: {} which matches already existing excess signature in chain database \ + block hash: {}. Existing kernel excess: {}, excess sig nonce: {}, excess signature: {}", + kernel.excess.to_hex(), + header_hash.to_hex(), + db_kernel.excess.to_hex(), + db_kernel.excess_sig.get_public_nonce().to_hex(), + db_kernel.excess_sig.get_signature().to_hex(), + ); + warn!(target: LOG_TARGET, "{}", msg); + return Err(ValidationError::ConsensusError(msg)); + }; + } + Ok(()) +} + pub fn validate_covenants(block: &Block) -> Result<(), ValidationError> { for input in block.body.inputs() { let output_set_size = input diff --git a/base_layer/core/src/validation/transaction_validators.rs b/base_layer/core/src/validation/transaction_validators.rs index b3a72e7e62..25a59bc8e6 100644 --- a/base_layer/core/src/validation/transaction_validators.rs +++ b/base_layer/core/src/validation/transaction_validators.rs @@ -229,25 +229,22 @@ impl TxConsensusValidator { } fn validate_excess_sig_not_in_db(&self, tx: &Transaction) -> Result<(), ValidationError> { - let excess_sig = tx - .first_kernel_excess_sig() - .ok_or_else(|| ValidationError::ConsensusError("Transaction has no kernel excess signature".into()))?; - - match self.db.fetch_kernel_by_excess_sig(excess_sig.to_owned())? { - Some((kernel, header_hash)) => { + for kernel in tx.body.kernels() { + if let Some((db_kernel, header_hash)) = self.db.fetch_kernel_by_excess_sig(kernel.excess_sig.to_owned())? { let msg = format!( - "Transaction kernel excess signature already exists in chain database block hash: {}. Existing \ - kernel excess: {}, excess sig nonce: {}, excess signature: {}", - header_hash.to_hex(), + "Block contains kernel excess: {} which matches already existing excess signature in chain \ + database block hash: {}. Existing kernel excess: {}, excess sig nonce: {}, excess signature: {}", kernel.excess.to_hex(), - kernel.excess_sig.get_public_nonce().to_hex(), - kernel.excess_sig.get_signature().to_hex(), + header_hash.to_hex(), + db_kernel.excess.to_hex(), + db_kernel.excess_sig.get_public_nonce().to_hex(), + db_kernel.excess_sig.get_signature().to_hex(), ); warn!(target: LOG_TARGET, "{}", msg); - Err(ValidationError::ConsensusError(msg)) - }, - None => Ok(()), + return Err(ValidationError::ConsensusError(msg)); + }; } + Ok(()) } }