diff --git a/base_layer/core/src/base_node/comms_interface/inbound_handlers.rs b/base_layer/core/src/base_node/comms_interface/inbound_handlers.rs index c800e6385a..8c6788ff11 100644 --- a/base_layer/core/src/base_node/comms_interface/inbound_handlers.rs +++ b/base_layer/core/src/base_node/comms_interface/inbound_handlers.rs @@ -51,6 +51,7 @@ use crate::{ consensus::{ConsensusConstants, ConsensusManager}, mempool::Mempool, proof_of_work::{Difficulty, PowAlgorithm}, + validation::helpers, }; const LOG_TARGET: &str = "c::bn::comms_interface::inbound_handler"; @@ -617,9 +618,29 @@ where B: BlockchainBackend + 'static // NB: Add the header last because `with_transactions` etc updates the current header, but we have the final one // already - builder = builder.with_header(header); + builder = builder.with_header(header.clone()); + let block = builder.build(); + + // Perform a sanity check on the reconstructed block, if the MMR roots don't match then it's possible one or + // more transactions in our mempool had the same excess/signature for a *different* transaction. + // This is extremely unlikely, but still possible. In case of a mismatch, request the full block from the peer. + let (block, mmr_roots) = self.blockchain_db.calculate_mmr_roots(block).await?; + if let Err(e) = helpers::check_mmr_roots(&header, &mmr_roots) { + let block_hash = block.hash(); + warn!( + target: LOG_TARGET, + "Reconstructed block #{} ({}) failed MMR check validation!. Requesting full block. Error: {}", + header.height, + block_hash.to_hex(), + e, + ); + + metrics::compact_block_mmr_mismatch(header.height).inc(); + let block = self.request_full_block_from_peer(source_peer, block_hash).await?; + return Ok(block); + } - Ok(Arc::new(builder.build())) + Ok(Arc::new(block)) } async fn request_full_block_from_peer( diff --git a/base_layer/core/src/base_node/metrics.rs b/base_layer/core/src/base_node/metrics.rs index b78b5b8dc7..9bb6a49e86 100644 --- a/base_layer/core/src/base_node/metrics.rs +++ b/base_layer/core/src/base_node/metrics.rs @@ -80,6 +80,19 @@ pub fn compact_block_full_misses(height: u64) -> IntCounter { METER.with_label_values(&[&height.to_string()]) } +pub fn compact_block_mmr_mismatch(height: u64) -> IntCounter { + static METER: Lazy = Lazy::new(|| { + tari_metrics::register_int_counter_vec( + "base_node::blockchain::compact_block_mmr_mismatch", + "Number of full blocks that had to be requested because of MMR mismatch", + &["height"], + ) + .unwrap() + }); + + METER.with_label_values(&[&height.to_string()]) +} + pub fn orphaned_blocks() -> IntCounter { static METER: Lazy = Lazy::new(|| { tari_metrics::register_int_counter( diff --git a/base_layer/core/src/mempool/mempool_storage.rs b/base_layer/core/src/mempool/mempool_storage.rs index 52c97dfd86..a97a0dd62e 100644 --- a/base_layer/core/src/mempool/mempool_storage.rs +++ b/base_layer/core/src/mempool/mempool_storage.rs @@ -112,7 +112,7 @@ impl MempoolStorage { Ok(TxStorageResponse::NotStoredConsensus) }, Err(e) => { - warn!(target: LOG_TARGET, "Validation failed due to error:{}", e); + warn!(target: LOG_TARGET, "Validation failed due to error: {}", e); Ok(TxStorageResponse::NotStored) }, } diff --git a/base_layer/core/src/validation/transaction_validators.rs b/base_layer/core/src/validation/transaction_validators.rs index 358ad54219..65455bfe39 100644 --- a/base_layer/core/src/validation/transaction_validators.rs +++ b/base_layer/core/src/validation/transaction_validators.rs @@ -223,6 +223,28 @@ impl TxConsensusValidator { Ok(()) } + + 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)) => { + 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(), + kernel.excess.to_hex(), + kernel.excess_sig.get_public_nonce().to_hex(), + kernel.excess_sig.get_signature().to_hex(), + ); + warn!(target: LOG_TARGET, "{}", msg); + Err(ValidationError::ConsensusError(msg)) + }, + None => Ok(()), + } + } } impl MempoolTransactionValidation for TxConsensusValidator { @@ -235,6 +257,8 @@ impl MempoolTransactionValidation for TxConsensusValidator return Err(ValidationError::MaxTransactionWeightExceeded); } + self.validate_excess_sig_not_in_db(tx)?; + self.validate_versions(tx, consensus_constants)?; self.validate_unique_asset_rules(tx) diff --git a/base_layer/core/tests/mempool.rs b/base_layer/core/tests/mempool.rs index 330abf7fc2..0bdf4e5c38 100644 --- a/base_layer/core/tests/mempool.rs +++ b/base_layer/core/tests/mempool.rs @@ -1167,6 +1167,36 @@ async fn consensus_validation_versions() { assert!(matches!(response, TxStorageResponse::NotStoredConsensus)); } +#[tokio::test] +async fn consensus_validation_unique_excess_sig() { + let network = Network::LocalNet; + let (mut store, mut blocks, mut outputs, consensus_manager) = create_new_blockchain(network); + + let mempool_validator = TxConsensusValidator::new(store.clone()); + + let mempool = Mempool::new( + MempoolConfig::default(), + consensus_manager.clone(), + Box::new(mempool_validator), + ); + + // Create a block with 5 outputs + let txs = vec![txn_schema!( + from: vec![outputs[0][0].clone()], + to: vec![2 * T, 2 * T, 2 * T, 2 * T, 2 * T], fee: 25.into(), lock: 0, features: OutputFeatures::default() + )]; + generate_new_block(&mut store, &mut blocks, &mut outputs, txs, &consensus_manager).unwrap(); + + let schema = txn_schema!(from: vec![outputs[1][0].clone()], to: vec![1_500_000 * uT]); + let (tx1, _) = spend_utxos(schema.clone()); + generate_block(&store, &mut blocks, vec![tx1.clone()], &consensus_manager).unwrap(); + + // trying to submit a transaction with an existing excess signature already in the chain is an error + let tx = Arc::new(tx1); + let response = mempool.insert(tx).await.unwrap(); + assert!(matches!(response, TxStorageResponse::NotStoredConsensus)); +} + #[tokio::test] #[allow(clippy::erasing_op)] #[allow(clippy::identity_op)]