Skip to content

Commit 8d9df97

Browse files
committed
Merge #2008: IndexedTxGraph: Transactions that conflict with relevant txs are also relevant.
7ca58b3 chore(chain): fix typo in method doc comment (Wei Chen) c846ad8 test(chain): `relevant_conflicts` (志宇) 7003ad4 feat(chain): Txs that conflict with relevant txs are also relevant (志宇) Pull request description: ## Description This PR changes the behavior of `IndexedTxGraph` insert-if-relevant methods to consider relevant-tx-conflicts as relevant. Affected methods: * `.apply_block_relevant` * `.batch_insert_relevant` * `.batch_insert_relevant_unconfirmed` #### Rationale It is useful to be able to determine: * Why something is no longer part of the best history. * Whether it is possible that something can reappear in the best history. In order to do this, we need to track conflicts of spk-relevant transactions. For example, an incoming transaction may be evicted from the mempool due to insufficient fees or cancelled (a conflicting transaction is confirmed). The caller may want to handle these two possibilities differently: * **Transaction has insufficient fees** - the caller may want to CPFP the transaction. * **Transaction is cancelled/replaced** - The user may want to forget about this transaction once the conflict reaches x confirmations. The `IntentTracker` will make use of these relevant-conflicts to help determine the course of action. #### Side note about chain sources For some chain sources, obtaining relevant-conflicts is extremely costly or downright impossible (i.e. Electrum, BIP-158 filters). `bdk_bitcoind_rpc::Emitter` is still the most robust chain source to use. ### Changelog notice ```md Changed: - Behavior of `IndexedTxGraph` methods (`apply_block_relevant`, `batch_insert_relevant` and `batch_insert_relevant_unconfirmed`) to also consider conflicts of spk-relevant transactions as relevant. ``` ### Checklists #### All Submissions: * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md) #### New Features: * [x] I've added tests for the new feature * [x] I've added docs for the new feature ACKs for top commit: LagginTimes: ACK 7ca58b3 nymius: ACK 7ca58b3 Tree-SHA512: c5e8d45681ad149d9c963c11b8db7787d03ac1f17aa9cdaca7e1044537127c35bfdb3f6f56169a7a35f64bc9cc7d5a9867a01cdb2f2db8613b1d41ed62917e3b
2 parents c43154d + 7ca58b3 commit 8d9df97

File tree

3 files changed

+203
-11
lines changed

3 files changed

+203
-11
lines changed

crates/chain/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ rusqlite = { version = "0.31.0", features = ["bundled"], optional = true }
2727
[dev-dependencies]
2828
rand = "0.8"
2929
proptest = "1.2.0"
30-
bdk_testenv = { path = "../testenv", default-features = false }
30+
bdk_testenv = { path = "../testenv" }
3131
criterion = { version = "0.2" }
3232

3333
[features]

crates/chain/src/indexed_tx_graph.rs

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,16 @@ impl<A: Anchor, I: Indexer> IndexedTxGraph<A, I> {
6767
indexer,
6868
}
6969
}
70+
71+
// If `tx` replaces a relevant tx, it should also be considered relevant.
72+
fn is_tx_or_conflict_relevant(&self, tx: &Transaction) -> bool {
73+
self.index.is_tx_relevant(tx)
74+
|| self
75+
.graph
76+
.direct_conflicts(tx)
77+
.filter_map(|(_, txid)| self.graph.get_tx(txid))
78+
.any(|tx| self.index.is_tx_relevant(&tx))
79+
}
7080
}
7181

7282
impl<A: Anchor, I: Indexer> IndexedTxGraph<A, I>
@@ -239,8 +249,11 @@ where
239249

240250
/// Batch insert transactions, filtering out those that are irrelevant.
241251
///
242-
/// Relevancy is determined by the [`Indexer::is_tx_relevant`] implementation of `I`. Irrelevant
243-
/// transactions in `txs` will be ignored. `txs` do not need to be in topological order.
252+
/// `txs` do not need to be in topological order.
253+
///
254+
/// Relevancy is determined by the internal [`Indexer::is_tx_relevant`] implementation of `I`.
255+
/// A transaction that conflicts with a relevant transaction is also considered relevant.
256+
/// Irrelevant transactions in `txs` will be ignored.
244257
pub fn batch_insert_relevant<T: Into<Arc<Transaction>>>(
245258
&mut self,
246259
txs: impl IntoIterator<Item = (T, impl IntoIterator<Item = A>)>,
@@ -263,7 +276,7 @@ where
263276

264277
let mut tx_graph = tx_graph::ChangeSet::default();
265278
for (tx, anchors) in txs {
266-
if self.index.is_tx_relevant(&tx) {
279+
if self.is_tx_or_conflict_relevant(&tx) {
267280
let txid = tx.compute_txid();
268281
tx_graph.merge(self.graph.insert_tx(tx.clone()));
269282
for anchor in anchors {
@@ -278,7 +291,8 @@ where
278291
/// Batch insert unconfirmed transactions, filtering out those that are irrelevant.
279292
///
280293
/// Relevancy is determined by the internal [`Indexer::is_tx_relevant`] implementation of `I`.
281-
/// Irrelevant transactions in `txs` will be ignored.
294+
/// A transaction that conflicts with a relevant transaction is also considered relevant.
295+
/// Irrelevant transactions in `unconfirmed_txs` will be ignored.
282296
///
283297
/// Items of `txs` are tuples containing the transaction and a *last seen* timestamp. The
284298
/// *last seen* communicates when the transaction is last seen in the mempool which is used for
@@ -305,8 +319,9 @@ where
305319

306320
let graph = self.graph.batch_insert_unconfirmed(
307321
txs.into_iter()
308-
.filter(|(tx, _)| self.index.is_tx_relevant(tx))
309-
.map(|(tx, seen_at)| (tx.clone(), seen_at)),
322+
.filter(|(tx, _)| self.is_tx_or_conflict_relevant(tx))
323+
.map(|(tx, seen_at)| (tx.clone(), seen_at))
324+
.collect::<Vec<_>>(),
310325
);
311326

312327
ChangeSet {
@@ -350,7 +365,8 @@ where
350365
/// Each inserted transaction's anchor will be constructed using [`TxPosInBlock`].
351366
///
352367
/// Relevancy is determined by the internal [`Indexer::is_tx_relevant`] implementation of `I`.
353-
/// Irrelevant transactions in `txs` will be ignored.
368+
/// A transaction that conflicts with a relevant transaction is also considered relevant.
369+
/// Irrelevant transactions in `block` will be ignored.
354370
pub fn apply_block_relevant(
355371
&mut self,
356372
block: &Block,
@@ -363,7 +379,7 @@ where
363379
let mut changeset = ChangeSet::<A, I::ChangeSet>::default();
364380
for (tx_pos, tx) in block.txdata.iter().enumerate() {
365381
changeset.indexer.merge(self.index.index_tx(tx));
366-
if self.index.is_tx_relevant(tx) {
382+
if self.is_tx_or_conflict_relevant(tx) {
367383
let txid = tx.compute_txid();
368384
let anchor = TxPosInBlock {
369385
block,

crates/chain/tests/test_indexed_tx_graph.rs

Lines changed: 178 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,198 @@
33
#[macro_use]
44
mod common;
55

6-
use std::{collections::BTreeSet, sync::Arc};
6+
use std::{
7+
collections::{BTreeSet, HashMap},
8+
sync::Arc,
9+
};
710

811
use bdk_chain::{
912
indexed_tx_graph::{self, IndexedTxGraph},
1013
indexer::keychain_txout::KeychainTxOutIndex,
1114
local_chain::LocalChain,
15+
spk_txout::SpkTxOutIndex,
1216
tx_graph, Balance, CanonicalizationParams, ChainPosition, ConfirmationBlockTime, DescriptorExt,
1317
SpkIterator,
1418
};
1519
use bdk_testenv::{
20+
anyhow::{self},
21+
bitcoincore_rpc::{json::CreateRawTransactionInput, RpcApi},
1622
block_id, hash,
1723
utils::{new_tx, DESCRIPTORS},
24+
TestEnv,
25+
};
26+
use bitcoin::{
27+
secp256k1::Secp256k1, Address, Amount, Network, OutPoint, ScriptBuf, Transaction, TxIn, TxOut,
28+
Txid,
1829
};
19-
use bitcoin::{secp256k1::Secp256k1, Amount, OutPoint, ScriptBuf, Transaction, TxIn, TxOut};
2030
use miniscript::Descriptor;
2131

32+
fn gen_spk() -> ScriptBuf {
33+
use bitcoin::secp256k1::{Secp256k1, SecretKey};
34+
35+
let secp = Secp256k1::new();
36+
let (x_only_pk, _) = SecretKey::new(&mut rand::thread_rng())
37+
.public_key(&secp)
38+
.x_only_public_key();
39+
ScriptBuf::new_p2tr(&secp, x_only_pk, None)
40+
}
41+
42+
/// Conflicts of relevant transactions must also be considered relevant.
43+
///
44+
/// This allows the receiving structures to determine the reason why a given transaction is not part
45+
/// of the best history. I.e. Is this transaction evicted from the mempool because of insufficient
46+
/// fee, or because a conflict is confirmed?
47+
///
48+
/// This tests the behavior of the "relevant-conflicts" logic.
49+
#[test]
50+
fn relevant_conflicts() -> anyhow::Result<()> {
51+
type SpkTxGraph = IndexedTxGraph<ConfirmationBlockTime, SpkTxOutIndex<()>>;
52+
53+
/// This environment contains a sender and receiver.
54+
///
55+
/// The sender sends a transaction to the receiver and attempts to cancel it later.
56+
struct ScenarioEnv {
57+
env: TestEnv,
58+
graph: SpkTxGraph,
59+
tx_send: Transaction,
60+
tx_cancel: Transaction,
61+
}
62+
63+
impl ScenarioEnv {
64+
fn new() -> anyhow::Result<Self> {
65+
let env = TestEnv::new()?;
66+
let client = env.rpc_client();
67+
68+
let sender_addr = client
69+
.get_new_address(None, None)?
70+
.require_network(Network::Regtest)?;
71+
72+
let recv_spk = gen_spk();
73+
let recv_addr = Address::from_script(&recv_spk, &bitcoin::params::REGTEST)?;
74+
75+
let mut graph = SpkTxGraph::default();
76+
assert!(graph.index.insert_spk((), recv_spk));
77+
78+
env.mine_blocks(1, Some(sender_addr.clone()))?;
79+
env.mine_blocks(101, None)?;
80+
81+
let tx_input = client
82+
.list_unspent(None, None, None, None, None)?
83+
.into_iter()
84+
.take(1)
85+
.map(|r| CreateRawTransactionInput {
86+
txid: r.txid,
87+
vout: r.vout,
88+
sequence: None,
89+
})
90+
.collect::<Vec<_>>();
91+
let tx_send = {
92+
let outputs =
93+
HashMap::from([(recv_addr.to_string(), Amount::from_btc(49.999_99)?)]);
94+
let tx = client.create_raw_transaction(&tx_input, &outputs, None, Some(true))?;
95+
client
96+
.sign_raw_transaction_with_wallet(&tx, None, None)?
97+
.transaction()?
98+
};
99+
let tx_cancel = {
100+
let outputs =
101+
HashMap::from([(sender_addr.to_string(), Amount::from_btc(49.999_98)?)]);
102+
let tx = client.create_raw_transaction(&tx_input, &outputs, None, Some(true))?;
103+
client
104+
.sign_raw_transaction_with_wallet(&tx, None, None)?
105+
.transaction()?
106+
};
107+
108+
Ok(Self {
109+
env,
110+
graph,
111+
tx_send,
112+
tx_cancel,
113+
})
114+
}
115+
116+
/// Rudimentary sync implementation.
117+
///
118+
/// Scans through all transactions in the blockchain + mempool.
119+
fn sync(&mut self) -> anyhow::Result<()> {
120+
let client = self.env.rpc_client();
121+
for height in 0..=client.get_block_count()? {
122+
let hash = client.get_block_hash(height)?;
123+
let block = client.get_block(&hash)?;
124+
let _ = self.graph.apply_block_relevant(&block, height as _);
125+
}
126+
let _ = self.graph.batch_insert_relevant_unconfirmed(
127+
client
128+
.get_raw_mempool()?
129+
.into_iter()
130+
.map(|txid| client.get_raw_transaction(&txid, None).map(|tx| (tx, 0)))
131+
.collect::<Result<Vec<_>, _>>()?,
132+
);
133+
Ok(())
134+
}
135+
136+
/// Broadcast the original sending transaction.
137+
fn broadcast_send(&self) -> anyhow::Result<Txid> {
138+
let client = self.env.rpc_client();
139+
Ok(client.send_raw_transaction(&self.tx_send)?)
140+
}
141+
142+
/// Broadcast the cancellation transaction.
143+
fn broadcast_cancel(&self) -> anyhow::Result<Txid> {
144+
let client = self.env.rpc_client();
145+
Ok(client.send_raw_transaction(&self.tx_cancel)?)
146+
}
147+
}
148+
149+
// Broadcast `tx_send`.
150+
// Sync.
151+
// Broadcast `tx_cancel`.
152+
// `tx_cancel` gets confirmed.
153+
// Sync.
154+
// Expect: Both `tx_send` and `tx_cancel` appears in `recv_graph`.
155+
{
156+
let mut env = ScenarioEnv::new()?;
157+
let send_txid = env.broadcast_send()?;
158+
env.sync()?;
159+
let cancel_txid = env.broadcast_cancel()?;
160+
env.env.mine_blocks(6, None)?;
161+
env.sync()?;
162+
163+
assert_eq!(env.graph.graph().full_txs().count(), 2);
164+
assert!(env.graph.graph().get_tx(send_txid).is_some());
165+
assert!(env.graph.graph().get_tx(cancel_txid).is_some());
166+
}
167+
168+
// Broadcast `tx_send`.
169+
// Sync.
170+
// Broadcast `tx_cancel`.
171+
// Sync.
172+
// Expect: Both `tx_send` and `tx_cancel` appears in `recv_graph`.
173+
{
174+
let mut env = ScenarioEnv::new()?;
175+
let send_txid = env.broadcast_send()?;
176+
env.sync()?;
177+
let cancel_txid = env.broadcast_cancel()?;
178+
env.sync()?;
179+
180+
assert_eq!(env.graph.graph().full_txs().count(), 2);
181+
assert!(env.graph.graph().get_tx(send_txid).is_some());
182+
assert!(env.graph.graph().get_tx(cancel_txid).is_some());
183+
}
184+
185+
// If we don't see `tx_send` in the first place, `tx_cancel` should not be relevant.
186+
{
187+
let mut env = ScenarioEnv::new()?;
188+
let _ = env.broadcast_send()?;
189+
let _ = env.broadcast_cancel()?;
190+
env.sync()?;
191+
192+
assert_eq!(env.graph.graph().full_txs().count(), 0);
193+
}
194+
195+
Ok(())
196+
}
197+
22198
/// Ensure [`IndexedTxGraph::insert_relevant_txs`] can successfully index transactions NOT presented
23199
/// in topological order.
24200
///

0 commit comments

Comments
 (0)