Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion crates/floresta-chain/benches/chain_state_bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Transaction> = 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,
Expand All @@ -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);
50 changes: 48 additions & 2 deletions crates/floresta-chain/src/pruned_utreexo/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -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();
Expand Down
4 changes: 4 additions & 0 deletions crates/floresta-chain/src/pruned_utreexo/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ pub enum BlockValidationErrors {
CoinbaseNotMatured,
UnspendableUTXO,
BIP94TimeWarp,
DuplicateInput,
}

// Helpful macro for generating a TransactionError
Expand Down Expand Up @@ -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")
}
}
}
}
Expand Down
14 changes: 12 additions & 2 deletions crates/floresta-mempool/src/mempool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
Comment on lines +457 to +459

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t understand the reason for this here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The used_in_this_tx guard is necessary since check_transaction_context_free() now rejects any transaction containing duplicate inputs.

In the original build_transactions(..., true) code, transactions with duplicate inputs were inadvertently being created because the same outpoint could be reused multiple times inside the inner loop. Once the consensus validation was added, test_gbt_with_conflict started failing with DuplicateInput before the mempool conflict logic even kicked in.

So i added guard which makes sure that each outpoint appears only once within a single transaction.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the mempool this does not make sense, because in the mempool there can be RBF.

@R27-pixel R27-pixel May 21, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the mempool this does not make sense, because in the mempool there can be RBF.

To clarify, used_in_this_tx is in build_transactions(), which is a test helper function that creates fake transactions. This is not related to the mempool logic at all and does not affect RBF.

RBF requires two valid transactions competing against one another in the mempool, and that behavior is untouched by this PR.

This is just a guard that stops build_transactions() from creating an invalid transaction that uses the same input twice in a transaction.

However, I can see where you're coming from and am happy to follow your recommendation on closing this and waiting until the discussion in the issue is over.


let input = bitcoin::TxIn {
previous_output,
script_sig: bitcoin::Script::new().into(),
Expand Down Expand Up @@ -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) => {
Expand Down
1 change: 1 addition & 0 deletions crates/floresta-wire/src/p2p_wire/node/blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ where
BlockValidationErrors::InvalidCoinbase(_)
| BlockValidationErrors::ScriptValidationError(_)
| BlockValidationErrors::NullPrevOut
| BlockValidationErrors::DuplicateInput
| BlockValidationErrors::EmptyInputs
| BlockValidationErrors::EmptyOutputs
| BlockValidationErrors::ScriptError
Expand Down