diff --git a/crates/chain/src/chain_data.rs b/crates/chain/src/chain_data.rs index 33381ab7..e585d82d 100644 --- a/crates/chain/src/chain_data.rs +++ b/crates/chain/src/chain_data.rs @@ -2,7 +2,7 @@ use bitcoin::{hashes::Hash, BlockHash, OutPoint, TxOut, Txid}; use crate::{ sparse_chain::{self, ChainPosition}, - Anchor, ConfirmationHeight, COINBASE_MATURITY, + Anchor, COINBASE_MATURITY, }; /// Represents an observation of some chain data. @@ -241,20 +241,20 @@ impl FullTxOut

{ } } -impl FullTxOut> { +impl FullTxOut> { /// Whether the `txout` is considered mature. /// /// This is the alternative version of [`is_mature`] which depends on `chain_position` being a /// [`ObservedAs`] where `A` implements [`Anchor`]. /// /// [`is_mature`]: Self::is_mature - pub fn is_observed_as_mature(&self, tip: u32) -> bool { + pub fn is_mature(&self, tip: u32) -> bool { if !self.is_on_coinbase { - return false; + return true; } let tx_height = match &self.chain_position { - ObservedAs::Confirmed(anchor) => anchor.confirmation_height(), + ObservedAs::Confirmed(anchor) => anchor.confirmation_height_upper_bound(), ObservedAs::Unconfirmed(_) => { debug_assert!(false, "coinbase tx can never be unconfirmed"); return false; @@ -271,19 +271,19 @@ impl FullTxOut> { /// Whether the utxo is/was/will be spendable with chain `tip`. /// - /// Currently this method does not take into account the locktime. + /// This method does not take into account the locktime. /// /// This is the alternative version of [`is_spendable_at`] which depends on `chain_position` /// being a [`ObservedAs`] where `A` implements [`Anchor`]. /// /// [`is_spendable_at`]: Self::is_spendable_at - pub fn is_observed_as_confirmed_and_spendable(&self, tip: u32) -> bool { - if !self.is_observed_as_mature(tip) { + pub fn is_confirmed_and_spendable(&self, tip: u32) -> bool { + if !self.is_mature(tip) { return false; } let confirmation_height = match &self.chain_position { - ObservedAs::Confirmed(anchor) => anchor.confirmation_height(), + ObservedAs::Confirmed(anchor) => anchor.confirmation_height_upper_bound(), ObservedAs::Unconfirmed(_) => return false, }; if confirmation_height > tip { diff --git a/crates/chain/src/chain_graph.rs b/crates/chain/src/chain_graph.rs index 0e3e3439..acf104e7 100644 --- a/crates/chain/src/chain_graph.rs +++ b/crates/chain/src/chain_graph.rs @@ -465,7 +465,7 @@ where #[must_use] pub struct ChangeSet

{ pub chain: sparse_chain::ChangeSet

, - pub graph: tx_graph::Additions<()>, + pub graph: tx_graph::Additions, } impl

ChangeSet

{ diff --git a/crates/chain/src/indexed_tx_graph.rs b/crates/chain/src/indexed_tx_graph.rs index 5ce53111..f44b7847 100644 --- a/crates/chain/src/indexed_tx_graph.rs +++ b/crates/chain/src/indexed_tx_graph.rs @@ -1,11 +1,12 @@ use core::convert::Infallible; +use alloc::vec::Vec; use bitcoin::{OutPoint, Script, Transaction, TxOut, Txid}; use crate::{ keychain::Balance, tx_graph::{Additions, CanonicalTx, TxGraph}, - Anchor, Append, BlockId, ChainOracle, ConfirmationHeight, FullTxOut, ObservedAs, + Anchor, Append, BlockId, ChainOracle, FullTxOut, ObservedAs, }; /// A struct that combines [`TxGraph`] and an [`Indexer`] implementation. @@ -120,20 +121,34 @@ where /// /// `anchors` can be provided to anchor the transactions to blocks. `seen_at` is a unix /// timestamp of when the transactions are last seen. - pub fn insert_relevant_txs<'t, T: Iterator>( + pub fn insert_relevant_txs<'t>( &mut self, - txs: T, + txs: impl IntoIterator, anchors: impl IntoIterator + Clone, seen_at: Option, ) -> IndexedAdditions { - txs.filter_map(|tx| match self.index.is_tx_relevant(tx) { - true => Some(self.insert_tx(tx, anchors.clone(), seen_at)), - false => None, - }) - .fold(Default::default(), |mut acc, other| { - acc.append(other); - acc - }) + // As mentioned by @LLFourn: This algorithm requires the transactions to be topologically + // sorted because most indexers cannot decide whether something is relevant unless you have + // first inserted its ancestors in the index. We can fix this if we instead do this: + // 1. insert all txs into the index. If they are irrelevant then that's fine it will just + // not store anything about them. + // 2. decide whether to insert them into the graph depending on whether `is_tx_relevant` + // returns true or not. (in a second loop). + let txs = txs + .into_iter() + .inspect(|tx| { + let _ = self.index.index_tx(tx); + }) + .collect::>(); + txs.into_iter() + .filter_map(|tx| match self.index.is_tx_relevant(tx) { + true => Some(self.insert_tx(tx, anchors.clone(), seen_at)), + false => None, + }) + .fold(Default::default(), |mut acc, other| { + acc.append(other); + acc + }) } } @@ -222,32 +237,31 @@ impl IndexedTxGraph { self.try_list_owned_unspents(chain, chain_tip) .map(|r| r.expect("oracle is infallible")) } -} -impl IndexedTxGraph { pub fn try_balance( &self, chain: &C, chain_tip: BlockId, - tip: u32, mut should_trust: F, ) -> Result where C: ChainOracle, F: FnMut(&Script) -> bool, { + let tip_height = chain_tip.anchor_block().height; + let mut immature = 0; let mut trusted_pending = 0; let mut untrusted_pending = 0; let mut confirmed = 0; - for res in self.try_list_owned_txouts(chain, chain_tip) { + for res in self.try_list_owned_unspents(chain, chain_tip) { let txout = res?; match &txout.chain_position { ObservedAs::Confirmed(_) => { if txout.is_on_coinbase { - if txout.is_observed_as_mature(tip) { + if txout.is_mature(tip_height) { confirmed += txout.txout.value; } else { immature += txout.txout.value; @@ -272,39 +286,12 @@ impl IndexedTxGraph { }) } - pub fn balance(&self, chain: &C, chain_tip: BlockId, tip: u32, should_trust: F) -> Balance + pub fn balance(&self, chain: &C, chain_tip: BlockId, should_trust: F) -> Balance where C: ChainOracle, F: FnMut(&Script) -> bool, { - self.try_balance(chain, chain_tip, tip, should_trust) - .expect("error is infallible") - } - - pub fn try_balance_at( - &self, - chain: &C, - chain_tip: BlockId, - height: u32, - ) -> Result - where - C: ChainOracle, - { - let mut sum = 0; - for txo_res in self.try_list_owned_unspents(chain, chain_tip) { - let txo = txo_res?; - if txo.is_observed_as_confirmed_and_spendable(height) { - sum += txo.txout.value; - } - } - Ok(sum) - } - - pub fn balance_at(&self, chain: &C, chain_tip: BlockId, height: u32) -> u64 - where - C: ChainOracle, - { - self.try_balance_at(chain, chain_tip, height) + self.try_balance(chain, chain_tip, should_trust) .expect("error is infallible") } } diff --git a/crates/chain/src/tx_data_traits.rs b/crates/chain/src/tx_data_traits.rs index bd7e66ac..8ec695ad 100644 --- a/crates/chain/src/tx_data_traits.rs +++ b/crates/chain/src/tx_data_traits.rs @@ -1,7 +1,7 @@ use crate::collections::BTreeMap; use crate::collections::BTreeSet; use crate::BlockId; -use bitcoin::{Block, BlockHash, OutPoint, Transaction, TxOut}; +use bitcoin::{Block, OutPoint, Transaction, TxOut}; /// Trait to do something with every txout contained in a structure. /// @@ -41,11 +41,17 @@ impl ForEachTxOut for Transaction { /// assume that transaction A is also confirmed in the best chain. This does not necessarily mean /// that transaction A is confirmed in block B. It could also mean transaction A is confirmed in a /// parent block of B. -pub trait Anchor: - core::fmt::Debug + Clone + Eq + PartialOrd + Ord + core::hash::Hash + Send + Sync + 'static -{ +pub trait Anchor: core::fmt::Debug + Clone + Eq + PartialOrd + Ord + core::hash::Hash { /// Returns the [`BlockId`] that the associated blockchain data is "anchored" in. fn anchor_block(&self) -> BlockId; + + /// Get the upper bound of the chain data's confirmation height. + /// + /// The default definition gives a pessimistic answer. This can be overridden by the `Anchor` + /// implementation for a more accurate value. + fn confirmation_height_upper_bound(&self) -> u32 { + self.anchor_block().height + } } impl Anchor for &'static A { @@ -54,21 +60,6 @@ impl Anchor for &'static A { } } -impl Anchor for (u32, BlockHash) { - fn anchor_block(&self) -> BlockId { - (*self).into() - } -} - -/// A trait that returns a confirmation height. -/// -/// This is typically used to provide an [`Anchor`] implementation the exact confirmation height of -/// the data being anchored. -pub trait ConfirmationHeight { - /// Returns the confirmation height. - fn confirmation_height(&self) -> u32; -} - /// Trait that makes an object appendable. pub trait Append { /// Append another object of the same type onto `self`. diff --git a/crates/chain/src/tx_graph.rs b/crates/chain/src/tx_graph.rs index c7c496ef..04c9fa27 100644 --- a/crates/chain/src/tx_graph.rs +++ b/crates/chain/src/tx_graph.rs @@ -908,7 +908,7 @@ impl TxGraph { ) )] #[must_use] -pub struct Additions { +pub struct Additions { pub tx: BTreeSet, pub txout: BTreeMap, pub anchors: BTreeSet<(A, Txid)>,