Merge bitcoindevkit#1594: Replace trait AnchorFromBlockPosition wit…

…h new struct

ab8068b refactor(chain)!: Replace trait `AnchorFromBlockPosition` with new struct (Jiri Jakes)

Pull request description:

  ### Description

  This change replaces a way of creating new generic anchor from block and its height. Previous way was using conversion trait, newly it is a struct and `From`.

  Closes bitcoindevkit#1266.

  ### Notes to the reviewers

  - Removed `tx_pos`: I did not see its relevance (`Anchor` does not give access to it). Was it a relic from the past or something I overlooked?
  - I put `BlockPosition` into `tx_data_traits.rs` because I believe it should be next to `Anchor`. But then it's not a module just for traits. What would be better place?

  ### 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

ACKs for top commit:
  LagginTimes:
    ACK ab8068b
  LLFourn:
    ACK ab8068b

Tree-SHA512: 9546ff177247796a36aeec6e015e564fb093cae0fe3b2962520843e8371bf3060f6d62f676a3989a49a6c25da6b7adbcde47c5fffe2f772f871980eb77cd1aea

crates/chain/src/indexed_tx_graph.rs

@@ -7,7 +7,7 @@ use bitcoin::{Block, OutPoint, Transaction, TxOut, Txid};
 
 use crate::{
     tx_graph::{self, TxGraph},
-    Anchor, AnchorFromBlockPosition, BlockId, Indexer, Merge,
+    Anchor, BlockId, Indexer, Merge, TxPosInBlock,
 };
 
 /// The [`IndexedTxGraph`] combines a [`TxGraph`] and an [`Indexer`] implementation.
@@ -252,17 +252,17 @@ where
     }
 }
 
-/// Methods are available if the anchor (`A`) implements [`AnchorFromBlockPosition`].
-impl<A: Anchor, I: Indexer> IndexedTxGraph<A, I>
+/// Methods are available if the anchor (`A`) can be created from [`TxPosInBlock`].
+impl<A, I> IndexedTxGraph<A, I>
 where
     I::ChangeSet: Default + Merge,
-    A: AnchorFromBlockPosition,
+    for<'b> A: Anchor + From<TxPosInBlock<'b>>,
+    I: Indexer,
 {
     /// Batch insert all transactions of the given `block` of `height`, filtering out those that are
     /// irrelevant.
     ///
-    /// Each inserted transaction's anchor will be constructed from
-    /// [`AnchorFromBlockPosition::from_block_position`].
+    /// Each inserted transaction's anchor will be constructed using [`TxPosInBlock`].
     ///
     /// Relevancy is determined by the internal [`Indexer::is_tx_relevant`] implementation of `I`.
     /// Irrelevant transactions in `txs` will be ignored.
@@ -280,7 +280,12 @@ where
             changeset.indexer.merge(self.index.index_tx(tx));
             if self.index.is_tx_relevant(tx) {
                 let txid = tx.compute_txid();
-                let anchor = A::from_block_position(block, block_id, tx_pos);
+                let anchor = TxPosInBlock {
+                    block,
+                    block_id,
+                    tx_pos,
+                }
+                .into();
                 changeset.tx_graph.merge(self.graph.insert_tx(tx.clone()));
                 changeset
                     .tx_graph
@@ -292,8 +297,7 @@ where
 
     /// Batch insert all transactions of the given `block` of `height`.
     ///
-    /// Each inserted transaction's anchor will be constructed from
-    /// [`AnchorFromBlockPosition::from_block_position`].
+    /// Each inserted transaction's anchor will be constructed using [`TxPosInBlock`].
     ///
     /// To only insert relevant transactions, use [`apply_block_relevant`] instead.
     ///
@@ -305,7 +309,12 @@ where
         };
         let mut graph = tx_graph::ChangeSet::default();
         for (tx_pos, tx) in block.txdata.iter().enumerate() {
-            let anchor = A::from_block_position(&block, block_id, tx_pos);
+            let anchor = TxPosInBlock {
+                block: &block,
+                block_id,
+                tx_pos,
+            }
+            .into();
             graph.merge(self.graph.insert_anchor(tx.compute_txid(), anchor));
             graph.merge(self.graph.insert_tx(tx.clone()));
         }

crates/chain/src/tx_data_traits.rs

@@ -98,24 +98,31 @@ impl Anchor for ConfirmationBlockTime {
     }
 }
 
-/// An [`Anchor`] that can be constructed from a given block, block height and transaction position
-/// within the block.
-pub trait AnchorFromBlockPosition: Anchor {
-    /// Construct the anchor from a given `block`, block height and `tx_pos` within the block.
-    fn from_block_position(block: &bitcoin::Block, block_id: BlockId, tx_pos: usize) -> Self;
+/// Set of parameters sufficient to construct an [`Anchor`].
+///
+/// Typically used as an additional constraint on anchor:
+/// `for<'b> A: Anchor + From<TxPosInBlock<'b>>`.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct TxPosInBlock<'b> {
+    /// Block in which the transaction appeared.
+    pub block: &'b bitcoin::Block,
+    /// Block's [`BlockId`].
+    pub block_id: BlockId,
+    /// Position in the block on which the transaction appeared.
+    pub tx_pos: usize,
 }
 
-impl AnchorFromBlockPosition for BlockId {
-    fn from_block_position(_block: &bitcoin::Block, block_id: BlockId, _tx_pos: usize) -> Self {
-        block_id
+impl<'b> From<TxPosInBlock<'b>> for BlockId {
+    fn from(pos: TxPosInBlock) -> Self {
+        pos.block_id
     }
 }
 
-impl AnchorFromBlockPosition for ConfirmationBlockTime {
-    fn from_block_position(block: &bitcoin::Block, block_id: BlockId, _tx_pos: usize) -> Self {
+impl<'b> From<TxPosInBlock<'b>> for ConfirmationBlockTime {
+    fn from(pos: TxPosInBlock) -> Self {
         Self {
-            block_id,
-            confirmation_time: block.header.time as _,
+            block_id: pos.block_id,
+            confirmation_time: pos.block.header.time as _,
         }
     }
 }