diff --git a/crates/floresta-chain/benches/chain_state_bench.rs b/crates/floresta-chain/benches/chain_state_bench.rs index 8499e8585..861625adb 100644 --- a/crates/floresta-chain/benches/chain_state_bench.rs +++ b/crates/floresta-chain/benches/chain_state_bench.rs @@ -8,6 +8,7 @@ use std::io::Cursor; use bitcoin::Block; use bitcoin::Network; use bitcoin::OutPoint; +use bitcoin::Transaction; use bitcoin::block::Header as BlockHeader; use bitcoin::consensus::Decodable; use bitcoin::consensus::deserialize; @@ -276,6 +277,23 @@ fn chainstore_checksum_benchmark(c: &mut Criterion) { }); } +fn check_transaction_context_free_benchmark(c: &mut Criterion) { + let block_file = File::open("./testdata/block_866342/raw.zst").unwrap(); + let block_bytes = zstd::decode_all(block_file).unwrap(); + let block: Block = deserialize(&block_bytes).unwrap(); + + // Collect all non-coinbase transactions from the block + let transactions: Vec = block.txdata.into_iter().skip(1).collect(); + + c.bench_function("check_transaction_context_free_block_866342", |b| { + b.iter(|| { + for tx in &transactions { + black_box(Consensus::check_transaction_context_free(tx)).ok(); + } + }) + }); +} + criterion_group!( benches, initialize_chainstore_benchmark, @@ -285,6 +303,7 @@ criterion_group!( connect_blocks_benchmark, validate_full_block_benchmark, validate_many_inputs_block_benchmark, - chainstore_checksum_benchmark + chainstore_checksum_benchmark, + check_transaction_context_free_benchmark ); criterion_main!(benches); diff --git a/crates/floresta-chain/src/pruned_utreexo/consensus.rs b/crates/floresta-chain/src/pruned_utreexo/consensus.rs index ac25939fc..f108123d7 100644 --- a/crates/floresta-chain/src/pruned_utreexo/consensus.rs +++ b/crates/floresta-chain/src/pruned_utreexo/consensus.rs @@ -449,6 +449,22 @@ impl Consensus { Ok(()) } + fn has_duplicate_inputs(inputs: &[TxIn]) -> bool { + match inputs.len() { + 1 => false, + 2 => inputs[0].previous_output == inputs[1].previous_output, + _ => { + let mut seen = HashSet::with_capacity(inputs.len()); + for input in inputs { + if !seen.insert(&input.previous_output) { + return true; + } + } + false + } + } + } + /// Performs consensus checks that are independent of the spent outputs (non-coinbase only). /// Returns the total output value as an [`Amount`]. pub fn check_transaction_context_free( @@ -469,11 +485,23 @@ impl Consensus { Err(tx_err!(txid, NullPrevOut))?; } - // Check script sizes (current tx scriptsig and TODO witness if present) + // Witness size is intentionally not checked here — witness-specific + // limits are enforced during script execution, not in context-free checks. + // This matches Bitcoin Core's CheckTransaction() which explicitly skips + // witness in context-free checks because witness data has not been + // checked for malleability at this point. + // See: https://github.com/bitcoin/bitcoin/blob/master/src/consensus/tx_check.cpp Self::validate_script_size(&input.script_sig, txid)?; - // TODO check also witness script size } + // Check for duplicate inputs (CVE-2018-17144). + // UpdateCoins does not detect duplicates — a duplicate prevout causes either + // a crash or an inflation bug depending on the coins database implementation. + // Bitcoin Core catches this explicitly in CheckTransaction() for the same reason. + // after: + if Self::has_duplicate_inputs(&transaction.input) { + return Err(tx_err!(txid, DuplicateInput))?; + } let out_value = Self::total_out_value(transaction)?; // Sanity check @@ -1374,6 +1402,24 @@ mod tests { } } + #[test] + fn test_duplicate_inputs_rejected() { + let outpoint = dummy_outpoint(); + + // Same prevout used in both inputs + let tx = build_tx( + vec![txin!(outpoint), txin!(outpoint)], + vec![txout!(0, ScriptBuf::new())], + ); + + match Consensus::check_transaction_context_free(&tx) { + Err(BlockchainError::TransactionError(tx_err)) => { + assert_eq!(tx_err.error, BlockValidationErrors::DuplicateInput); + } + other => panic!("Expected DuplicateInput, got: {other:?}"), + } + } + #[test] fn test_input_value_above_max_money() { let outpoint = dummy_outpoint(); diff --git a/crates/floresta-chain/src/pruned_utreexo/error.rs b/crates/floresta-chain/src/pruned_utreexo/error.rs index 0e20fac95..4554e1e84 100644 --- a/crates/floresta-chain/src/pruned_utreexo/error.rs +++ b/crates/floresta-chain/src/pruned_utreexo/error.rs @@ -145,6 +145,7 @@ pub enum BlockValidationErrors { CoinbaseNotMatured, UnspendableUTXO, BIP94TimeWarp, + DuplicateInput, } // Helpful macro for generating a TransactionError @@ -238,6 +239,9 @@ impl Display for BlockValidationErrors { BlockValidationErrors::BIP94TimeWarp => { write!(f, "BIP94 time warp detected") } + BlockValidationErrors::DuplicateInput => { + write!(f, "This transaction has duplicate inputs") + } } } } diff --git a/crates/floresta-mempool/src/mempool.rs b/crates/floresta-mempool/src/mempool.rs index 4ae05f331..881f7c8fd 100644 --- a/crates/floresta-mempool/src/mempool.rs +++ b/crates/floresta-mempool/src/mempool.rs @@ -436,7 +436,12 @@ mod tests { output: Vec::new(), }; - let inputs = rng.random_range(1..10); + let inputs = rng.gen_range(1..10); + + // Track outpoints already used in this transaction to avoid + // duplicate inputs within a single tx, which is never valid. + // Conflicts are only meaningful across separate transactions. + let mut used_in_this_tx = HashSet::new(); for _ in 0..inputs { if outputs.is_empty() { break; @@ -448,6 +453,11 @@ mod tests { true => *outputs.get(index).unwrap(), }; + // Skip if this outpoint is already an input in this tx + if !used_in_this_tx.insert(previous_output) { + continue; + } + let input = bitcoin::TxIn { previous_output, script_sig: bitcoin::Script::new().into(), @@ -521,7 +531,7 @@ mod tests { for tx in transactions { match mempool.accept_to_mempool(tx) { Ok(_) => {} - Err(MempoolError::ConflictingTransaction) | Err(MempoolError::DuplicatedInputs) => { + Err(MempoolError::ConflictingTransaction) => { did_conflict = true; } Err(e) => { diff --git a/crates/floresta-wire/src/p2p_wire/node/blocks.rs b/crates/floresta-wire/src/p2p_wire/node/blocks.rs index a6fab9285..f39005c46 100644 --- a/crates/floresta-wire/src/p2p_wire/node/blocks.rs +++ b/crates/floresta-wire/src/p2p_wire/node/blocks.rs @@ -357,6 +357,7 @@ where BlockValidationErrors::InvalidCoinbase(_) | BlockValidationErrors::ScriptValidationError(_) | BlockValidationErrors::NullPrevOut + | BlockValidationErrors::DuplicateInput | BlockValidationErrors::EmptyInputs | BlockValidationErrors::EmptyOutputs | BlockValidationErrors::ScriptError