bitcoindevkit / bitcoindevkit/bdk

Smaller invalid witnesses replace usable witnesses and cannot be restored

Open
#2,292 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**

`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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.