bitcoindevkit / bitcoindevkit/bdk
Smaller invalid witnesses replace usable witnesses and cannot be restored
- Dominant language
- Rust
- Stars
- 1.1k
- Forks
- 483
- Avg merge
- 20d 3h
- Merged PRs (30d)
- 3
Description
**Describe the bug**
`TxGraph::insert_tx` prefers smaller nonempty witnesses when merging copies of the same transaction. A witness with one empty element counts as nonempty, so it can replace a usable witness. Reinserting the original transaction does not restore it, and changeset restoration retains the smaller copy.
For a recorded P2WSH prevout, this can replace the matching witness script with an empty script. The stored transaction then has an unusable witness and reduced weight, affecting rebroadcast and weight-based fee-rate calculations.
This issue was found by AI.
**To Reproduce**
Add `crates/chain/tests/test_witness_merge.rs` and run `cargo test -p bdk_chain --test test_witness_merge`:
```rust
use bdk_chain::{BlockId, TxGraph};
use bitcoin::{absolute, hashes::Hash, transaction, Amount, OutPoint, ScriptBuf,
Transaction, TxIn, TxOut, Txid, Witness};
#[test]
fn usable_witness_survives_a_smaller_invalid_copy() {
let witness_script = ScriptBuf::from_bytes(vec![0x51]); // OP_TRUE
let outpoint = OutPoint::new(Txid::from_byte_array([1; 32]), 0);
let mut graph = TxGraph::::default();
let _ = graph.insert_txout(outpoint, TxOut {
value: Amount::from_sat(10_000), script_pubkey: witness_script.to_p2wsh(),
});
let original = Transaction {
version: transaction::Version::TWO,
lock_time: absolute::LockTime::ZERO,
input: vec![TxIn {
previous_output: outpoint,
witness: Witness::from_slice(&[witness_script.as_bytes()]),
..Default::default()
}],
output: vec![TxOut {
value: Amount::from_sat(9_000), script_pubkey: ScriptBuf::new(),
}],
};
let txid = original.compute_txid();
let _ = graph.insert_tx(original.clone());
let mut smaller = original.clone();
smaller.input[0].witness = Witness::from_slice(&[[]]);
assert!(!smaller.input[0].witness.is_empty());
assert_eq!(smaller.compute_txid(), txid);
let _ = graph.insert_tx(smaller);
let _ = graph.insert_tx(original.clone());
let restored = TxGraph::::from_changeset(graph.initial_changeset());
assert_eq!(restored.get_tx(txid).unwrap().input[0].witness, original.input[0].witness);
}
```
The final assertion fails: the restored witness contains an empty element instead of `OP_TRUE`.
**Expected behavior**
A malformed smaller witness should not irreversibly replace a usable stored witness for the same transaction.
Contributor guide
Research direction
Start with TxGraph::insert_tx and the changeset restoration path, then add crates/chain/tests/test_witness_merge.rs from the reproduction. Run cargo test -p bdk_chain --test test_witness_merge and verify that reinserting a usable witness and restoring the changeset preserves it instead of retaining the smaller invalid copy.
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
- Clearly specified
- Newbie friendliness
- 78/100