bitcoindevkit / bitcoindevkit/bdk

Duplicate inputs inflate sent amounts and calculated fees

Open
#2,288 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Rust
Stars
1.1k
Forks
483
Avg merge
20d 3h
Merged PRs (30d)
3

Description

**Describe the bug**

A transaction repeating a prevout is accepted by `TxGraph`, and accounting counts its value once per input. `SpkTxOutIndex::sent_and_received` and `net_value` overstate the amount sent; `spent_txouts` repeats the output, and `calculate_fee` can return a positive fee even when outputs exceed the distinct input value.

This requires a consensus-invalid transaction; it affects accounting for supplied transaction data.

This issue was found by AI.

**To Reproduce**

Add `crates/chain/tests/test_duplicate_inputs.rs` and run `cargo test -p bdk_chain --test test_duplicate_inputs`. These assertions demonstrate the current double-counting:

```rust
use bdk_chain::{spk_txout::SpkTxOutIndex, BlockId, IndexedTxGraph};
use bitcoin::{absolute, hashes::Hash, transaction, Amount, OutPoint, ScriptBuf,
Transaction, TxIn, TxOut, Txid};

#[test]
fn duplicate_inputs_inflate_accounting() {
let script = ScriptBuf::from_bytes(vec![0x51]);
let outpoint = OutPoint::new(Txid::from_byte_array([1; 32]), 0);
let mut graph = IndexedTxGraph::>::default();
graph.index.insert_spk(0, script.clone());
let _ = graph.insert_txout(outpoint, TxOut {
value: Amount::from_sat(10_000), script_pubkey: script,
});
let tx = Transaction {
version: transaction::Version::TWO,
lock_time: absolute::LockTime::ZERO,
input: vec![TxIn { previous_output: outpoint, ..Default::default() }; 2],
output: vec![TxOut {
value: Amount::from_sat(19_000), script_pubkey: ScriptBuf::new(),
}],
};
let _ = graph.insert_tx(tx.clone());

// Current behavior: the same 10,000-sat outpoint is counted twice.
assert_eq!(graph.index.sent_and_received(&tx, ..),
(Amount::from_sat(20_000), Amount::ZERO));
assert_eq!(graph.index.net_value(&tx, ..).to_sat(), -20_000);
assert_eq!(graph.index.spent_txouts(&tx).count(), 2);
assert_eq!(graph.graph().calculate_fee(&tx), Ok(Amount::from_sat(1_000)));
}
```

**Expected behavior**

Malformed transactions with repeated prevouts should not silently produce inflated wallet accounting or a seemingly valid fee.

Contributor guide

Open the contributing guide

Research direction

Start with crates/chain/tests/test_duplicate_inputs.rs and run cargo test -p bdk_chain --test test_duplicate_inputs to reproduce the accounting results. Trace SpkTxOutIndex::sent_and_received, net_value, spent_txouts, and IndexedTxGraph::calculate_fee for repeated prevouts. Done means duplicate inputs no longer inflate accounting or produce a seemingly valid fee, with the regression test passing.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.