Implement linked-list LocalChain and add rpc-chain module/example
#1002
Conversation
This file contains 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
evanlinjin
changed the title
LocalChain
evanlinjin
changed the title
LocalChainLocalChain
evanlinjin
force-pushed
the
wallet_redesign_local_chain
branch
3 times, most recently
from
d2b2a99
to
c9b648e
Compare
19 tasks
LLFourn
reviewed
Jun 7, 2023
evanlinjin
force-pushed
the
wallet_redesign_local_chain
branch
6 times, most recently
from
7492685
to
9856d16
Compare
LLFourn
reviewed
Jun 15, 2023
LLFourn
reviewed
Jun 15, 2023
LLFourn
reviewed
Jun 15, 2023
This was referenced Jun 15, 2023
LLFourn
reviewed
Jun 19, 2023
evanlinjin
force-pushed
the
wallet_redesign_local_chain
branch
5 times, most recently
from
41ef9cd
to
26cc17f
Compare
evanlinjin
changed the title
LocalChainLocalChain and add rpc-chain module/example
16 tasks
This allows the data-source thread to hold a reference to checkpoints without a lock on `LocalChain` itself. Introduce `LocalChain::update` that replaces `determine_changeset` and `apply_update`. This returns a closure that actually updates `self` when called. This method allows for efficient and elegant updating while being able to "preview" the update before applying. The `LocalChain` update/determine_changeset tests have been updated to also check for the final state after applying the update (not just looking at the changeset). Update `keychain::LocalUpdate` struct to use `CheckPoint` Instead of containing a complete `LocalChain`, the update uses `CheckPoint`. This simplifies the API since updating a `LocalChain` only requires a `CheckPoint` now. The examples and chain source `..Ext` implementations have all been updated to use the new API. Additionally, `..Ext` implementations didn't 100% guarantee consistency of the updates, the logic has been changed to enforce better guarantees.
`prune_and_apply_update` first scans all txs contained in `update` through the index, then filters out txs using `I::is_tx_relevant` before applying the update. This is useful for block-by-block syncing. `Wallet::apply_update` now has a second input; `prune: bool`. If `prune` is set, irrelevant transactions of `update` will not be included.
Revert `get_or_insert` back as `insert_block`. Method `update` now mutates `LocalChain` directly, instead of mutating via a second call. `CheckPoint::new_with_prev` is replaced with `CheckPoint::extend`.
Within `update()`, it is not always necessary to call `fix_links()`. The logic to detect this was wrong previously. Add test that would fail with the previous logic.
For esplora: * Separate checkpoint-update logic into a separate method `construct_update_tip`. * We batch-get latest blocks to ensure consistency. * Reorg-mitigation logic is changed to only reconstruct checkpoints and anchors. For electrum: * Rename `prepare_chain_update` to `construct_update_tip`. Use `Client::block_headers` to get latest headers atomically (instead of fetching headers one by one). * `determine_tx_anchor` now uses the lowest anchor checkpoint possible. * Add comments for better documentation.
The old logic cannot guarantee consistency of the chain history (i.e. we may result in a `LocalChain` state which contain blocks that cannot belong in the same chain). We also had to do a rescan if the tip has changed since the start of the scan. To fix this, we anchor transactions at the confirmation height and hash (which is provided by the esplora API). We fetch the `LocalChain` update separately, and guarantee consistency by fetching most-recent blocks atomically via the `GET /blocks` endpoint. Because the anchors need to be reflected in `LocalChain`, `TxGraph::missing_blocks(&self, chain: &LocalChain)` is introduced to fetch missing blocks in a separate call.
This logic should carry over better when we change `LocalChain` to be monotone (bitcoindevkit#1005).
By iterating backwards over the two chains in tandem to find the difference between them.
Local chain is a linked list whose heights are indexed in a BTreeMap.
The lower bound height is the lowest checkpoint the update chain can introduce to the original chain. Also add optimisation when we have a perfect connection. Add `debug_assertions` to `LocalChain` methods that mutate. Add more `LocalChain` unit tests.
This replaces using a lower bound height (which is a more complex API).
The introduced method is `Wallet::prune_and_apply_update`.
So we don't confuse it with `Extend::extend` (trait) that resides in `core::iter::Extend`.
Remove the `info` field from the `BitcoindRpcUpdate::Block` variant. `BitcoindRpcUpdate::into_update` now returns the new `CheckPoint` tip + the `TxGraph` update (instead of `LocalUpdate`). Consequently, the `anchor` input is also simplified.
Thank you to @LLFourn for the suggestions. * `LocalChain::update` is now renamed back to `LocalChain::apply_update` and takes in a new struct `local_chain::Update` that combines a `CheckPoint` tip and a `introduce_older_blocks: bool`. This is because the boolean argument is determined by the chain source and not by the caller/user. * We've removed the premature optimization in `local_chain::merge_chains`. * Deriving `PartialEq`, `Eq`, `PartialOrd`, `Ord` for `CheckPoint` does not make much sense. They are removed for now. * Fix: bitcoind_rpc Cargo.toml: wrong bdk_chain version
evanlinjin
force-pushed
the
wallet_redesign_local_chain
branch
from
458e291
to
25a9879
Compare
5 tasks
|
Replaced by #1034 |
evanlinjin
added a commit
that referenced
this pull request
Aug 3, 2023
…crates/examples b206a98 fix: Even more refactoring to code and documentation (志宇) bea8e5a fix: `TxGraph::missing_blocks` logic (志宇) db15e03 fix: improve more docs and more refactoring (志宇) 95312d4 fix: docs and some minor refactoring (志宇) 8bf7a99 Refactor `debug_assertions` checks for `LocalChain` (志宇) 315e7e0 fix: rm duplicate `bdk_tmp_plan` module (志宇) af705da Add exclusion of example cli `*.db` files in `.gitignore` (志宇) eabeb6c Implement linked-list `LocalChain` and update chain-src crates/examples (志宇) Pull request description: Fixes #997 Replaces #1002 ### Description This PR changes the `LocalChain` implementation to have blocks stored as a linked-list. This allows the data-src thread to hold a shared ref to a single checkpoint and have access to the whole history of checkpoints without cloning or keeping a lock on `LocalChain`. The APIs of `bdk::Wallet`, `esplora` and `electrum` are also updated to reflect these changes. Note that the `esplora` crate is rewritten to anchor txs in the confirmation block (using the esplora API's tx status block_hash). This guarantees 100% consistency between anchor blocks and their transactions (instead of anchoring txs to the latest tip). `ExploraExt` now has separate methods for updating the `TxGraph` and `LocalChain`. A new method `TxGraph::missing_blocks` is introduced for finding "floating anchors" of a `TxGraph` update (given a chain). Additional changes: * `test_local_chain.rs` is refactored to make test cases easier to write. Additional tests are also added. * Examples are updated. * Exclude example-cli `*.db` files in `.gitignore`. * Rm duplicate `bdk_tmp_plan` module. ### Notes to the reviewers This is the smallest possible division of #1002 without resulting in PRs that do not compile. Since we have changed the API of `LocalChain`, we also need to change `esplora`, `electrum` crates and examples alongside `bdk::Wallet`. ### Changelog notice * Implement linked-list `LocalChain`. This allows the data-src thread to hold a shared ref to a single checkpoint and have access to the whole history of checkpoints without cloning or keeping a lock on `LocalChain`. * Rewrote `esplora` chain-src crate to anchor txs to their confirmation blocks (using esplora API's tx-status `block_hash`). ### 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: LLFourn: ACK b206a98 Tree-SHA512: a513eecb4f1aae6a5c06a69854e4492961424312a75a42d74377d363b364e3d52415bc81b4aa3fbc3f369ded19bddd07ab895130ebba288e8a43e9d6186e9fcc
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #997
Description
This PR changes the
LocalChainimplementation to have blocks stored as a linked-list. To verify the API of the linked-listLocalChain, thebitcoind_rpccrate and example is added (which is a block-by-block chain source). Additionally, the electrum/esplora crates and examples are also updated for the new API, but the logic is also updated to better-ensure consistency when obtaining a view of blocks.A linked-list
LocalChainallows the data-source thread to hold a shared reference to a single checkpoint and have access to the whole history of checkpoints without cloning or having a lock onLocalChain. This idea was originally proposed by @LLFourn's.Notes to the reviewers
Sorry this is a massive PR again. The changes can be "grouped" as follows:
LocalChainAPI to use linked-list.apply_updatemethod is renamed toupdateand the input is now theCheckPointtip. An additional input is added (introduce_older_blocks: bool) which allows the caller to specify whether they wish to introduce new checkpoints below the highest point of agreement. This is required optimization as the update tip's chain and the original chain may share a large portion of history._check_consistencymethod is added, whichassert!s whether the internal tip's history is consistent with the internal index.prune_and_apply_updatemethods for bothIndexedTxGraphandWallet. This prunes out irrelevant transactions from theTxGraphupdate before applying it. This will be used by block-by-block data sources as each block update is aTxGraph.electrumcrate to use the newLocalChainAPI. We cannot 100% guarantee consistency withelectrumupdates (yet), but this is an improvement.esplorato ensure 100% consistency.LocalChainandTxGraphupdates are now handled separately inExploraExt. We make use of theblock_hashfield of tx statuses from the esplora API.bitcoind_rpccrate and example.Changelog notice
LocalChainAPI to use a linked-list of block data.bitcoind_rpccrate and example.esploracrate to guarantee a consistent view of blocks when formulating an update.example_rpcCLI example forbitcoind_rpccrate.Checklists
All Submissions:
cargo fmtandcargo clippybefore committingNew Features:
Bugfixes:
* [ ] I've added tests to reproduce the issue which are now passing