Skip to content

Commit

Permalink
feat: add mmr check to reconstructed block and mempool validation for…
Browse files Browse the repository at this point in the history
… unique excess signature (#3930)

Description
---

- Adds a check to the reconstructed block to ensure MMR roots match, if not then requests the full block from source peer
- Adds mempool validation to check submitted kernel excess signature does not already exist in the chain db
  • Loading branch information
Byron Hambly authored Mar 21, 2022
1 parent 7145201 commit b8f9db5
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 3 deletions.
25 changes: 23 additions & 2 deletions base_layer/core/src/base_node/comms_interface/inbound_handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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(
Expand Down
13 changes: 13 additions & 0 deletions base_layer/core/src/base_node/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<IntCounterVec> = 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<IntCounter> = Lazy::new(|| {
tari_metrics::register_int_counter(
Expand Down
2 changes: 1 addition & 1 deletion base_layer/core/src/mempool/mempool_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
}
Expand Down
24 changes: 24 additions & 0 deletions base_layer/core/src/validation/transaction_validators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,28 @@ impl<B: BlockchainBackend> TxConsensusValidator<B> {

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<B: BlockchainBackend> MempoolTransactionValidation for TxConsensusValidator<B> {
Expand All @@ -235,6 +257,8 @@ impl<B: BlockchainBackend> 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)
Expand Down
30 changes: 30 additions & 0 deletions base_layer/core/tests/mempool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down

0 comments on commit b8f9db5

Please sign in to comment.