Skip to content

Commit

Permalink
fix: transaction validation excess signature check (#4314)
Browse files Browse the repository at this point in the history
Description
---
Currently, only the transaction validator checks the rules that a transaction kernel must have a unique excess signature. But it only checks the first signature in the transaction. Transactions can have more than one kernel. 
This check is also not currently checked in block validation checks. 

This means that if a block with a duplicate excess signature is submitted, it will fail with an LMDB::error(duplicate key) and not with a validation error, although this is a validation constraint as per RFC 120. 

Motivation and Context
---
We need to check the kernel excess signature for uniqueness because this ensures that every kernel in the blockchain is unique.  By enforcing this, we ensure that replay attacks are impossible as the same kernel cannot be added to the blockchain again.

The excess signature is also used as a key index for the lookup of the kernels.
  • Loading branch information
SWvheerden committed Jul 20, 2022
1 parent 15f41b3 commit f6342a5
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 17 deletions.
20 changes: 18 additions & 2 deletions base_layer/core/src/validation/block_validators/async_validator.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -176,8 +176,9 @@ impl<B: BlockchainBackend + 'static> BlockValidator<B> {
.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,
Expand All @@ -204,6 +205,21 @@ impl<B: BlockchainBackend + 'static> BlockValidator<B> {
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;
Expand Down
4 changes: 3 additions & 1 deletion base_layer/core/src/validation/block_validators/body_only.rs
Expand Up @@ -53,6 +53,7 @@ impl<B: BlockchainBackend> PostOrphanBodyValidation<B> 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,
Expand Down Expand Up @@ -80,9 +81,10 @@ impl<B: BlockchainBackend> PostOrphanBodyValidation<B> 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())?;
Expand Down
20 changes: 20 additions & 0 deletions base_layer/core/src/validation/helpers.rs
Expand Up @@ -671,6 +671,26 @@ pub fn check_not_bad_block<B: BlockchainBackend>(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<B: BlockchainBackend>(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
Expand Down
25 changes: 11 additions & 14 deletions base_layer/core/src/validation/transaction_validators.rs
Expand Up @@ -229,25 +229,22 @@ impl<B: BlockchainBackend> TxConsensusValidator<B> {
}

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(())
}
}

Expand Down

0 comments on commit f6342a5

Please sign in to comment.