Skip to content

Commit

Permalink
Merge #1202: fix(chain): filter coinbase tx not in best chain
Browse files Browse the repository at this point in the history
991cb77 fix(chain): filter coinbase tx not in best chain (Wei Chen)

Pull request description:

  ### Description

  Fixes #1144.
  Coinbase transactions cannot exist in the mempool and be unconfirmed. `TxGraph::try_get_chain_position` should always return `None` for coinbase transactions not anchored in best chain.

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### Bugfixes:

  * [ ] This pull request breaks the existing API
  * [x] I've added tests to reproduce the issue which are now passing
  * [x] I'm linking the issue being fixed by this PR

ACKs for top commit:
  notmandatory:
    ACK 991cb77
  danielabrozzoni:
    ACK 991cb77

Tree-SHA512: 9e06d8404708eee050c96807a876a470303f4983666c82c56c17d97c2d4b72784e75271279fd393c53a6a967a352aea1ef2762da71ac4bb58f7a0c2f05354948
  • Loading branch information
danielabrozzoni committed Nov 14, 2023
2 parents e71770f + 991cb77 commit f382fa9
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 1 deletion.
9 changes: 8 additions & 1 deletion crates/chain/src/tx_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,14 @@ impl<A: Anchor> TxGraph<A> {
// might be in mempool, or it might have been dropped already.
// Let's check conflicts to find out!
let tx = match tx_node {
TxNodeInternal::Whole(tx) => tx,
TxNodeInternal::Whole(tx) => {
// A coinbase tx that is not anchored in the best chain cannot be unconfirmed and
// should always be filtered out.
if tx.is_coin_base() {
return Ok(None);
}
tx
}
TxNodeInternal::Partial(_) => {
// Partial transactions (outputs only) cannot have conflicts.
return Ok(None);
Expand Down
43 changes: 43 additions & 0 deletions crates/chain/tests/test_tx_graph_conflicts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,49 @@ fn test_tx_conflict_handling() {
.unwrap_or_default();

let scenarios = [
Scenario {
name: "coinbase tx cannot be in mempool and be unconfirmed",
tx_templates: &[
TxTemplate {
tx_name: "unconfirmed_coinbase",
inputs: &[TxInTemplate::Coinbase],
outputs: &[TxOutTemplate::new(5000, Some(0))],
..Default::default()
},
TxTemplate {
tx_name: "confirmed_genesis",
inputs: &[TxInTemplate::Bogus],
outputs: &[TxOutTemplate::new(10000, Some(1))],
anchors: &[block_id!(1, "B")],
last_seen: None,
},
TxTemplate {
tx_name: "unconfirmed_conflict",
inputs: &[
TxInTemplate::PrevTx("confirmed_genesis", 0),
TxInTemplate::PrevTx("unconfirmed_coinbase", 0)
],
outputs: &[TxOutTemplate::new(20000, Some(2))],
..Default::default()
},
TxTemplate {
tx_name: "confirmed_conflict",
inputs: &[TxInTemplate::PrevTx("confirmed_genesis", 0)],
outputs: &[TxOutTemplate::new(20000, Some(3))],
anchors: &[block_id!(4, "E")],
..Default::default()
},
],
exp_chain_txs: HashSet::from(["confirmed_genesis", "confirmed_conflict"]),
exp_chain_txouts: HashSet::from([("confirmed_genesis", 0), ("confirmed_conflict", 0)]),
exp_unspents: HashSet::from([("confirmed_conflict", 0)]),
exp_balance: Balance {
immature: 0,
trusted_pending: 0,
untrusted_pending: 0,
confirmed: 20000,
},
},
Scenario {
name: "2 unconfirmed txs with same last_seens conflict",
tx_templates: &[
Expand Down

0 comments on commit f382fa9

Please sign in to comment.