Skip to content

Commit

Permalink
feat(chain)!: IndexedTxGraph::expected_unconfirmed_spk_txids
Browse files Browse the repository at this point in the history
Make this method work when the indexer is `KeychainTxOutIndex`. We
reintroduce the ability to get the internal `SpkTxOutIndex` from
`KeychainTxOutIndex` so that `SpkTxOutIndex::relevant_spks_of_tx` is
callable from `KeychainTxOutIndex`.

This commit renames `iter_spks_with_expected_txids` to
`expected_unconfirmed_spk_txids` for `TxGraph`, `IndexedTxGraph` and
`SyncRequestBuilder`. Docs are also improved to explain how these
methods are useful.

Remove unused `SyncRequestBuilder` methods.
  • Loading branch information
evanlinjin committed Feb 6, 2025
1 parent 682d5c0 commit 7f3b6d4
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 83 deletions.
2 changes: 1 addition & 1 deletion crates/bitcoind_rpc/tests/test_emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -808,7 +808,7 @@ fn test_expect_tx_missing() -> anyhow::Result<()> {

// We evict the expected txs that are missing from mempool
let exp_txids = graph
.iter_spks_with_expected_txids(&chain, ..)
.expected_unconfirmed_spk_txids(&chain, ..)
.collect::<Vec<_>>();
assert_eq!(exp_txids, vec![(txid_1, spk)]);
let mempool = emitter
Expand Down
41 changes: 32 additions & 9 deletions crates/chain/src/indexed_tx_graph.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! Contains the [`IndexedTxGraph`] and associated types. Refer to the
//! [`IndexedTxGraph`] documentation for more.
use core::fmt;
use core::ops::RangeBounds;

use alloc::{sync::Arc, vec::Vec};
Expand Down Expand Up @@ -345,15 +344,15 @@ where
impl<A, I> IndexedTxGraph<A, SpkTxOutIndex<I>>
where
A: Anchor,
I: fmt::Debug + Clone + Ord,
I: core::fmt::Debug + Clone + Ord,
{
/// Returns an iterator over unconfirmed transactions and their associated script pubkeys,
/// filtered within the specified `range`.
/// Iterate over unconfirmed txids that we expect to exist in a chain source's spk history
/// response.
///
/// This function delegates the transaction filtering to [`TxGraph::iter_spks_with_expected_txids`],
/// using the [`SpkTxOutIndex`] stored in [`IndexedTxGraph`]. The [`TxGraph`] internally scans
/// for unconfirmed transactions relevant to the indexed outputs.
pub fn iter_spks_with_expected_txids<'a, O>(
/// This is used to fill [`SyncRequestBuilder::expected_unconfirmed_spk_txids`](bdk_core::spk_client::SyncRequestBuilder::expected_unconfirmed_spk_txids).
///
/// The spk range can be contrained with `range`.
pub fn expected_unconfirmed_spk_txids<'a, O>(
&'a self,
chain: &'a O,
range: impl RangeBounds<I> + 'a,
Expand All @@ -362,7 +361,31 @@ where
O: ChainOracle<Error = core::convert::Infallible>,
{
self.graph
.iter_spks_with_expected_txids(chain, &self.index, range)
.expected_unconfirmed_spk_txids(chain, &self.index, range)
}
}

impl<A, K> IndexedTxGraph<A, crate::keychain_txout::KeychainTxOutIndex<K>>
where
A: Anchor,
K: core::fmt::Debug + Clone + Ord,
{
/// Iterate over unconfirmed txids that we expect to exist in a chain source's spk history
/// response.
///
/// This is used to fill [`SyncRequestBuilder::expected_unconfirmed_spk_txids`](bdk_core::spk_client::SyncRequestBuilder::expected_unconfirmed_spk_txids).
///
/// The spk range can be contrained with `range`.
pub fn expected_unconfirmed_spk_txids<'a, O>(
&'a self,
chain: &'a O,
range: impl RangeBounds<(K, u32)> + 'a,
) -> impl Iterator<Item = (Txid, ScriptBuf)> + 'a
where
O: ChainOracle<Error = core::convert::Infallible>,
{
self.graph
.expected_unconfirmed_spk_txids(chain, &self.index, range)
}
}

Expand Down
52 changes: 12 additions & 40 deletions crates/chain/src/indexer/keychain_txout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,11 @@ use crate::{
spk_client::{FullScanRequestBuilder, SyncRequestBuilder},
spk_iter::BIP32_MAX_INDEX,
spk_txout::SpkTxOutIndex,
Anchor, CanonicalIter, CanonicalReason, ChainOracle, DescriptorExt, DescriptorId, Indexed,
Indexer, KeychainIndexed, SpkIterator,
DescriptorExt, DescriptorId, Indexed, Indexer, KeychainIndexed, SpkIterator,
};
use alloc::{borrow::ToOwned, vec::Vec};
use bitcoin::{Amount, OutPoint, ScriptBuf, SignedAmount, Transaction, TxOut, Txid};
use core::{
convert::Infallible,
fmt::Debug,
ops::{Bound, RangeBounds},
};
Expand Down Expand Up @@ -138,6 +136,12 @@ impl<K> Default for KeychainTxOutIndex<K> {
}
}

impl<K> AsRef<SpkTxOutIndex<(K, u32)>> for KeychainTxOutIndex<K> {
fn as_ref(&self) -> &SpkTxOutIndex<(K, u32)> {
self.inner()
}
}

impl<K: Clone + Ord + Debug> Indexer for KeychainTxOutIndex<K> {
type ChangeSet = ChangeSet;

Expand Down Expand Up @@ -202,6 +206,11 @@ impl<K> KeychainTxOutIndex<K> {
lookahead,
}
}

/// Get a reference to the internal [`SpkTxOutIndex`].
pub fn inner(&self) -> &SpkTxOutIndex<(K, u32)> {
&self.inner
}
}

/// Methods that are *re-exposed* from the internal [`SpkTxOutIndex`].
Expand Down Expand Up @@ -881,20 +890,6 @@ pub trait SyncRequestBuilderExt<K> {

/// Add [`Script`](bitcoin::Script)s that are revealed by the `indexer` but currently unused.
fn unused_spks_from_indexer(self, indexer: &KeychainTxOutIndex<K>) -> Self;

/// Add unconfirmed txids and their associated spks.
///
/// We expect that the chain source should include these txids in their spk histories. If not,
/// the transaction has been evicted for some reason and we will inform the receiving
/// structures in the response.
fn check_unconfirmed_statuses<A, C>(
self,
indexer: &KeychainTxOutIndex<K>,
canonical_iter: CanonicalIter<A, C>,
) -> Self
where
A: Anchor,
C: ChainOracle<Error = Infallible>;
}

impl<K: Clone + Ord + core::fmt::Debug> SyncRequestBuilderExt<K> for SyncRequestBuilder<(K, u32)> {
Expand All @@ -908,29 +903,6 @@ impl<K: Clone + Ord + core::fmt::Debug> SyncRequestBuilderExt<K> for SyncRequest
fn unused_spks_from_indexer(self, indexer: &KeychainTxOutIndex<K>) -> Self {
self.spks_with_indexes(indexer.unused_spks())
}

fn check_unconfirmed_statuses<A, C>(
self,
indexer: &KeychainTxOutIndex<K>,
canonical_iter: CanonicalIter<A, C>,
) -> Self
where
A: Anchor,
C: ChainOracle<Error = Infallible>,
{
self.expected_txids_of_spk(
canonical_iter
.map(|res| res.expect("infallible"))
.filter(|(_, _, reason)| matches!(reason, CanonicalReason::ObservedIn { .. }))
.flat_map(|(txid, tx, _)| {
indexer
.inner
.relevant_spks_of_tx(tx.as_ref())
.into_iter()
.map(move |spk| (txid, spk))
}),
)
}
}

/// Trait to extend [`FullScanRequestBuilder`].
Expand Down
6 changes: 6 additions & 0 deletions crates/chain/src/indexer/spk_txout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ pub struct SpkTxOutIndex<I> {
spk_txouts: BTreeSet<(I, OutPoint)>,
}

impl<I> AsRef<SpkTxOutIndex<I>> for SpkTxOutIndex<I> {
fn as_ref(&self) -> &SpkTxOutIndex<I> {
self
}
}

impl<I> Default for SpkTxOutIndex<I> {
fn default() -> Self {
Self {
Expand Down
17 changes: 9 additions & 8 deletions crates/chain/src/tx_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1160,23 +1160,24 @@ impl<A: Anchor> TxGraph<A> {
.expect("oracle is infallible")
}

/// Returns an iterator over unconfirmed transactions and their associated script pubkeys,
/// filtered within the specified `range`.
/// Iterate over unconfirmed txids that we expect to exist in a chain source's spk history
/// response.
///
/// This function scans the transaction graph for unconfirmed transactions relevant to the
/// provided [`SpkTxOutIndex`], determining which transactions should be considered based on
/// indexed outputs.
pub fn iter_spks_with_expected_txids<'a, C, I>(
/// This is used to fill [`SyncRequestBuilder::expected_unconfirmed_spk_txids`](bdk_core::spk_client::SyncRequestBuilder::expected_unconfirmed_spk_txids).
///
/// The spk range can be contrained with `range`.
pub fn expected_unconfirmed_spk_txids<'a, C, I>(
&'a self,
chain: &'a C,
indexer: &'a SpkTxOutIndex<I>,
indexer: &'a impl AsRef<SpkTxOutIndex<I>>,
range: impl RangeBounds<I> + 'a,
) -> impl Iterator<Item = (Txid, ScriptBuf)> + 'a
where
C: ChainOracle<Error = core::convert::Infallible>,
I: fmt::Debug + Clone + Ord,
I: fmt::Debug + Clone + Ord + 'a,
{
let chain_tip = chain.get_chain_tip().unwrap();
let indexer = indexer.as_ref();

self.list_canonical_txs(chain, chain_tip)
.filter(|c| !c.chain_position.is_confirmed() && indexer.is_tx_relevant(&c.tx_node))
Expand Down
2 changes: 1 addition & 1 deletion crates/core/src/spk_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ impl<I> SyncRequestBuilder<I> {
/// Add transactions that are expected to exist under a given spk.
///
/// This is useful for detecting a malicious replacement of an incoming transaction.
pub fn expected_txids_of_spk(
pub fn expected_unconfirmed_spk_txids(
mut self,
txs: impl IntoIterator<Item = (Txid, ScriptBuf)>,
) -> Self {
Expand Down
10 changes: 2 additions & 8 deletions crates/electrum/tests/test_electrum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,7 @@ pub fn detect_receive_tx_cancel() -> anyhow::Result<()> {
let sync_request = SyncRequest::builder()
.chain_tip(chain.tip())
.revealed_spks_from_indexer(&graph.index, ..)
.check_unconfirmed_statuses(
&graph.index,
graph.graph().canonical_iter(&chain, chain.tip().block_id()),
);
.expected_unconfirmed_spk_txids(graph.expected_unconfirmed_spk_txids(&chain, ..));
let sync_response = client.sync(sync_request, BATCH_SIZE, true)?;
assert!(
sync_response
Expand All @@ -163,10 +160,7 @@ pub fn detect_receive_tx_cancel() -> anyhow::Result<()> {
let sync_request = SyncRequest::builder()
.chain_tip(chain.tip())
.revealed_spks_from_indexer(&graph.index, ..)
.check_unconfirmed_statuses(
&graph.index,
graph.graph().canonical_iter(&chain, chain.tip().block_id()),
);
.expected_unconfirmed_spk_txids(graph.expected_unconfirmed_spk_txids(&chain, ..));
let sync_response = client.sync(sync_request, BATCH_SIZE, true)?;
assert!(
sync_response.tx_update.missing.contains(&send_txid),
Expand Down
10 changes: 2 additions & 8 deletions crates/esplora/tests/async_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,7 @@ pub async fn detect_receive_tx_cancel() -> anyhow::Result<()> {
let sync_request = SyncRequest::builder()
.chain_tip(chain.tip())
.revealed_spks_from_indexer(&graph.index, ..)
.check_unconfirmed_statuses(
&graph.index,
graph.graph().canonical_iter(&chain, chain.tip().block_id()),
);
.expected_unconfirmed_spk_txids(graph.expected_unconfirmed_spk_txids(&chain, ..));
let sync_response = client.sync(sync_request, 1).await?;
assert!(
sync_response
Expand All @@ -122,10 +119,7 @@ pub async fn detect_receive_tx_cancel() -> anyhow::Result<()> {
let sync_request = SyncRequest::builder()
.chain_tip(chain.tip())
.revealed_spks_from_indexer(&graph.index, ..)
.check_unconfirmed_statuses(
&graph.index,
graph.graph().canonical_iter(&chain, chain.tip().block_id()),
);
.expected_unconfirmed_spk_txids(graph.expected_unconfirmed_spk_txids(&chain, ..));
let sync_response = client.sync(sync_request, 1).await?;
assert!(
sync_response.tx_update.missing.contains(&send_txid),
Expand Down
10 changes: 2 additions & 8 deletions crates/esplora/tests/blocking_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,7 @@ pub fn detect_receive_tx_cancel() -> anyhow::Result<()> {
let sync_request = SyncRequest::builder()
.chain_tip(chain.tip())
.revealed_spks_from_indexer(&graph.index, ..)
.check_unconfirmed_statuses(
&graph.index,
graph.graph().canonical_iter(&chain, chain.tip().block_id()),
);
.expected_unconfirmed_spk_txids(graph.expected_unconfirmed_spk_txids(&chain, ..));
let sync_response = client.sync(sync_request, 1)?;
assert!(
sync_response
Expand All @@ -122,10 +119,7 @@ pub fn detect_receive_tx_cancel() -> anyhow::Result<()> {
let sync_request = SyncRequest::builder()
.chain_tip(chain.tip())
.revealed_spks_from_indexer(&graph.index, ..)
.check_unconfirmed_statuses(
&graph.index,
graph.graph().canonical_iter(&chain, chain.tip().block_id()),
);
.expected_unconfirmed_spk_txids(graph.expected_unconfirmed_spk_txids(&chain, ..));
let sync_response = client.sync(sync_request, 1)?;
assert!(
sync_response.tx_update.missing.contains(&send_txid),
Expand Down

0 comments on commit 7f3b6d4

Please sign in to comment.