Update persistance With new IndexedTxGraph#937
Update persistance With new IndexedTxGraph#937rajarshimaitra wants to merge 30 commits intobitcoindevkit:masterfrom
IndexedTxGraph#937Conversation
* Introduce `GraphedTx` struct to access transaction data of graphed transactions. * Ability to insert/access anchors and "seen at" values for graphed transactions. * `Additions` now records changes to anchors and last_seen_at.
The chain oracle keeps track of the best chain, while the transaction index indexes transaction data in relation to script pubkeys. This commit also includes initial work on `IndexedTxGraph`.
Add methods to `TxGraph` and `IndexedTxGraph` that gets in-best-chain data (such as transactions, txouts, unspent txouts).
Methods that list chain data have try and non-try versions. Both of these versions now return an `Iterator`. * Try versions return `Iterator<Item = Result>`. * Non-try versions require the `ChainOracle` implementation to be `ChainOracle<Error = Infallible>`.
* Get mutable index from `IndexedChainGraph`. * Also add `apply_additions` method to `TxIndex` trait.
…height This is important as a `ChainOracle` implementation is updated separately to an `IndexedTxGraph`.
This allows us to use the old API with minimal changes. `TxGraph` methods have also been rearranged to allow for it.
Methods of old structures that return transaction(s) no longer return `TxNode`, but `Transaction` as done previously. `TxInGraph` is renamed to `TxNode`, while the internal `TxNode` is renamed to `TxNodeInternal`.
* Rename `TxNode::last_seen` to `last_seen_unconfirmed` and improve docs * Improve `try_get_chain_position` logic and tweak comments
Also made the `IndexedTxGraph::index` field public (`index()` and `index_mut()` methods are no longer needed).
Introduce `chain_oracle::Cache` which is a cache for requests to the chain oracle. `ChainOracle` has also been moved to the `chain_oracle` module. Introduce `get_tip_in_best_chain` method to the `ChainOracle` trait. This allows for guaranteeing that chain state can be consistent across operations with `IndexedTxGraph`.
* Instead of implementing `ChainPosition` for `ObservedIn<BlockId>` to use `FullTxOut` methods (`is_spendable_at` and `is_mature`), we create alternative versions of those methods that require bounds with `Anchor`. This removes all `ObservedIn<A>: ChainPosition` bounds for methods of `IndexedTxGraph`. * Various improvements to comments and names.
Before, we were using `core::ops::AddAsign` but it was not the most appropriate.
This makes the API of `TxIndex` more consistent between scanning in data and checking whether certain data is relevant.
The problem with the previous `ChainOracle` interface is that it had no guarantee for consistency. For example, a block deemed to be part of the "best chain" can be reorged out. So when `ChainOracle` is called multiple times for an operation (such as getting the UTXO set), the returned result may be inconsistent. This PR changes `ChainOracle::is_block_in_chain` to take in another input `static_block`, ensuring `block` is an ancestor of `static_block`. Thus, if `static_block` is consistent across the operation, the result will be consistent also. `is_block_in_chain` now returns `Option<bool>`. The `None` case means that the oracle implementation cannot determine whether block is an ancestor of static block. `IndexedTxGraph::list_chain_txouts` handles this case by checking child spends that are in chain, and if so, the parent tx must be in chain too.
Remove the requirement that evicted blocks should have in-best-chain counterparts in the update.
It is better to have this external to this structure.
evanlinjin
left a comment
There was a problem hiding this comment.
Thank you for starting work on this!
There was a problem hiding this comment.
Instead of modifying this structure (which is specifically for persisting the KeychainTracker), I think it's best to create a new file:
chain/src/persist.rs
Also, because we have the Append trait, the Persist::stage field only needs to implement Append imo.
pub struct Persist<A, B, T> {
backend: B,
stage: A,
phantom: PhantomData<T>,
}
impl<A: Append + Default, B: PersistBackend> Persist<A, B> { /* TODO */ }PersistBackend will look as follows:
pub trait PersistBackend<A, T> {
type WriteError: core::fmt::Debug;
type LoadError: core::fmt::Debug;
fn append(&mut self, additions: A) -> Result<(), Self::WriteError>;
fn load(&mut self, tracker: &mut T) -> Result<(), Self::LoadError>;
}The intention here is that we can make tracker T anything we want (tracker is the in-memory representation of all our data). I would expect that IndexedTxGraph would be nested somewhere inside the T implementation. Because we will support different tracker implementation, the corresponding additions A implementation also needs to be a generic. A only needs to implement Append and Default.
145330b to
0ff20d4
Compare
|
closing in favor of #943 |
b799a57 [bdk_chain_redesign] Add tests for `IndexedTxGraph` with `LocalChain` (rajarshimaitra) 8cd0328 [bdk_chain_redesign] Implement `OwnedIndexer` for indexers (rajarshimaitra) 911af34 [bdk_chain_redesign] Fix calculation bugs. (rajarshimaitra) e536307 [bdk_chain_redesign] Fix `tx_graph::Additions::append` logic (志宇) f101dde [bdk_chain_redesign] Fix `tx_graph::Additions::append` logic (志宇) 1b15264 [bdk_chain_redesign] Change `insert_relevant_txs` method (志宇) ecc74ce [bdk_chain_redesign] Docs for `is_mature` and `is_confirmed_and_spendable` (志宇) ac336aa [bdk_chain_redesign] Make `insert_relevant_txs` topologically-agnostic (志宇) 165b874 [bdk_chain_redesign] Add test for `insert_relevant_txs` (志宇) f3e7b67 [bdk_chain_redesign] Various tweaks and fixes (志宇) 03c1283 [bdk_chain_redesign] Revert changes to `SparseChain` (志宇) 34a7bf5 [bdk_chain_redesign] Rm unnecessary code and premature optimisation (志宇) 6c49570 [bdk_chain_redesign] Rm `HashSet` from `TxGraph::relevant_heights` (志宇) 1003fe2 [bdk_chain_redesign] Test `LocalChain` (志宇) 7175a82 [bdk_chain_redesign] Add tests for `TxGraph::relevant_heights` (志宇) 8e36a2e [bdk_chain_redesign] Remove incomplete logic (志宇) 81436fc [bdk_chain_redesign] Fix `Anchor` definition + docs (志宇) 001efdd Include tests for new updates of TxGraph (rajarshimaitra) 10ab77c [bdk_chain_redesign] MOVE `TxIndex` into `indexed_chain_graph.rs` (志宇) 7d92337 [bdk_chain_redesign] Remove `IndexedTxGraph::last_height` (志宇) a7fbe0a [bdk_chain_redesign] Documentation improvements (志宇) ee1060f [bdk_chain_redesign] Simplify `LocalChain` (志宇) 611d2e3 [bdk_chain_redesign] Consistent `ChainOracle` (志宇) bff80ec [bdk_chain_redesign] Improve `BlockAnchor` docs (志宇) 24cd8c5 [bdk_chain_redesign] More tweaks and renamings (志宇) ddd5e95 [bdk_chain_redesign] Modify signature of `TxIndex` (志宇) da4cef0 [bdk_chain_redesign] Introduce `Append` trait for additions (志宇) 89cfa4d [bdk_chain_redesign] Better names, comments and generic bounds (志宇) 6e59dce [bdk_chain_redesign] `chain_oracle::Cache` (志宇) a7eaebb [bdk_chain_redesign] Add serde support for `IndexedAdditions` (志宇) c09cd2a [bdk_chain_redesign] Added methods to `LocalChain` (志宇) 7810059 [bdk_chain_redesign] `TxGraph` tweaks (志宇) a63ffe9 [bdk_chain_redesign] Simplify `TxIndex` (志宇) a1172de [bdk_chain_redesign] Revert some API changes (志宇) 8c90617 [bdk_chain_redesign] Make default anchor for `TxGraph` as `()` (志宇) 468701a [bdk_chain_redesign] Initial work on `LocalChain`. (志宇) 34d0277 [bdk_chain_redesign] Rm anchor type param for structs that don't use it (志宇) 3440a05 [bdk_chain_redesign] Add docs (志宇) 236c50f [bdk_chain_redesign] `IndexedTxGraph` keeps track of the last synced height (志宇) e902c10 [bdk_chain_redesign] Fix `apply_additions` logic for `IndexedTxGraph`. (志宇) 313965d [bdk_chain_redesign] `mut_index` should be `index_mut` (志宇) db7883d [bdk_chain_redesign] Add balance methods to `IndexedTxGraph` (志宇) d0a2aa8 [bdk_chain_redesign] Add `apply_additions` to `IndexedTxGraph` (志宇) 6cbb18d [bdk_chain_redesign] MOVE: `IndexedTxGraph` into submodule (志宇) 784cd34 [bdk_chain_redesign] List chain data methods can be try/non-try (志宇) 43b648f [bdk_chain_redesign] Add `..in_chain` methods (志宇) 61a8606 [bdk_chain_redesign] Introduce `ChainOracle` and `TxIndex` traits (志宇) 5ae5fe3 [bdk_chain_redesign] Introduce `BlockAnchor` trait (志宇) Pull request description: ### Description This is part of #895 The initial `bdk_chain` structures allowed updating to be done without blocking IO (alongside many other benefits). However, the requirement to have transactions "perfectly positioned" in our `SparseChain` increased complexity of the syncing API. Updates needed to be meticulously crafted to properly connect with the original `SparseChain`. Additionally, it forced us to keep a local copy of the "best chain" data (which may not always be needed depending on the chain source). The redesigned structures, as introduced by this PR, fixes these shortcomings. 1. Instead of `SparseChain`, we introduce the ability to `Anchor` a transaction to multiple blocks that may or may not be in the same chain history. We expand `TxGraph` to records these anchors (while still maintaining the *monotone* nature of `TxGraph`). When updating our new `TxGraph`, we don't need to complicated *are-we-connected* checks that we need for `SparseChain`. 2. The chain source, in combination with our new`TxGraph` is all that we need to determine the "chain position" of a transaction. The chain source only needs to implement a single trait `ChainOracle`. This typically does not need to be updated (as it is remote), although we can have a special `ChainOracle` implementation that is stored locally. This only needs block height and hash information, reducing the scope of *non-monotine* structures that need to be updated. **What is done:** * `TxGraph` includes anchors (ids of blocks that the tx is seen in) and last-seem unix timestamp (for determining which non-confirmed tx we should consider as part of "best chain" if there are conflicts). This structure continues to be fully "monotone"; we can introduce data in any manner and not risk resulting in an inconsistent state. * `LocalChain` includes the "checkpoint" logic of `SparseChain` but removes the `txid` data. `LocalChain` implements the `ChainOracle` trait. Any blockchain-source can also implement the `ChainOracle` trait. * `IndexedTxGraph` is introduced and contains two fields; an internal `TxGraph` struct and a `TxIndex` implementation. These two fields will be updated atomically and can replace the functionality of `keychain::Tracker`. **What is in-progress:** * ~@evanlinjin: The `TxIndex` trait should not have `is_{}_relevant` methods as we cannot guarantee this across all transaction indexes. We should introduce extension traits for these (#926 (comment) * ~@evanlinjin: `BlockAnchor` should be defined as "if this block is in chain, then this tx must be in chain". Therefore, the anchor does not provide us with the exact confirmation height of the tx. We need to introduce an extension trait `ExactConfirmationHeightAnchor` for certain operations (#926 (comment) **What will be done external to this PR:** * @rajarshimaitra: Persisting `indexed_tx_graph::Additions` (#937). * @notmandatory: Update examples to use redesigned structures. * Update `bdk::Wallet` to use the redesigned structures. ### Changelog notice * Initial implementation of the `bdk_chain` redesign (as mentioned in #895). Currently, only the `bdk_chain` core structures are implemented. Persistence and updates to the current examples and `bdk::Wallet` will be done in separate PRs. ### 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 #### New Features: * [x] I've added tests for the new feature * [x] I've added docs for the new feature ACKs for top commit: rajarshimaitra: ACK b799a57 danielabrozzoni: Partial ACK b799a57 - good job! I haven't looked at the tests or at the methods implementations in depth, but I have reviewed the architecture and it looks good. I have a few non-blocking questions. Tree-SHA512: 8c386354cbd02f0701b5134991b65d206a54d19a2e78ab204e6ff1fa78a18f16299051bc0bf4ff4d2f5a0adab9b15658fa53cd0de2ca16969f4bf6a485225082
Description
This builds on top of #926 and updates the persistence module to include the new IndexedTxGraph.
Notes to the reviewers
The examples and wallet module isn't updated as I will wait for some reviews and concept ACKs on the overall change.
Had to add one extra trait
Emptyto make the generics fit together.Changelog notice
Introduce
IndexedTxGraphStorewhich persists anIndexedTxGraphand replaceKeychainStore.Checklists
All Submissions:
cargo fmtandcargo clippybefore committing