More IndexedTxGraph tests#956
Closed
rajarshimaitra wants to merge 46 commits intobitcoindevkit:masterfrom
Closed
Conversation
* 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.
`tx_graph.rs` is rearranged as well.
Previously, I have misunderstood the definition of anchor. If a tx is anchored in a block, it does not necessarily mean it is confirmed in that block. The tx can be confirmed in an ancestor block of the anchor block. With this new definition, we need a new trait `ConfirmationHeight` that has one method `confirmation_height`. This trait can be used to extend `Anchor` for those implementations that can give us the exact conirmation height of a tx (which is useful in most cases). Another change is to add another variant to the `ObservedAs` enum; `ObservedAs::ConfirmedImplicit(A)`. If a tx does not have an anchor, but another tx that spends it has an anchor that in in the best chain, we can assume that tx is also in the best chain. The logic of `TxGraph::try_get_chain_position` is also changed to reflect this. Some methods from `IndexedTxGraph` have been moved to `TxGraph` as they do not require the `Indexer`. Some `TxGraph` methods have been renamed for clarity and consistency. Also more docs are added.
`ObservedAs::ConfirmedImplicit` is incomplete, remove for now. `local_chain::ChangeSet` does not need to be a single-element tuple struct.
This is mostly copying over the relevant tests from `SparseChain`. Changes are made to `local_chain::ChangeSet` to re-add the ability to remove blocks.
The `HashSet` was used for iterating without duplicate items. However, since `anchors` is a `BTreeSet`, heights are in order. So a single variable tracking last height will be sufficient.
* Remove `chain_oracle::CacheBackend` for now as it is not used. * `SparseChain` does not need to implement `ChainOracle`. * Remove filter predicate for `list..` methods of `TxGraph` and `IndexedTxGraph` as this is premature optimisation. * `Append` can be implemented for all `BTreeMap`s and `BTreeSet`s, instead of only `local_chain::ChangeSet`.
The `ConfirmationHeight` trait has been removed in favour of a second method on the `Anchor` trait: `confirmation_height_upper_bound()`. Methods `try_balance_at()` and `balance_at()` of `IndexedTxGraph` have been removed as they do not provide additional functionality. `IndexedTxGraph::insert_relevant_txs` now uses two loops, the first loop indexes all transactions first. This is done as some indexes require ancestor transactions to be indexed first and we cannot guarantee that the input transactions are in topological order.
Ensure `insert_relevant_txs` does not require transactions to be in topological order. Other changes: Rm `try_list_owned_txs` as it is useless
The `insert_relevant_txs` test has also been changed to used `KeychainTxOutIndex` so that index additions can be checked (`SpkTxOutIndex` has no additions). Additionally, generic bounds of some `IndexedTxGraph` list methods have been fixed.
…able` Docs are updated to explain why `is_mature` and `is_confirmed_and_spendable` may return false-negatives.
Instead of forcing all transactions inserted to use the same anchors, we change the API to have unique anchors per transaction. This allows for more flexibility in general. For example, use `Anchor` implementations that contain the position in a block of a transaction.
The try-list-chain_unspent logic should be reversed Include confirmed and spendable txs into balance calculation
Member
|
Thank you for the work @rajarshimaitra. Changes here have been incorporated into #926 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This covers the test for IndexedTxGraph with LocalChain and tests the
list_owned_txouts,list_owned_unspents, andbalancefunctions.Checks are performed at different heights along the local chain to assert behaviors.
Notes to the reviewers
The first two commits are bug fixes and the
OwnedIndexerimplementation for our existing indexers.Checklists
All Submissions:
cargo fmtandcargo clippybefore committing