Simplified EsploraExt API#1380
Merged
evanlinjin merged 13 commits intobitcoindevkit:masterfrom Apr 22, 2024
Merged
Conversation
1bb6193 to
abddc33
Compare
abddc33 to
f04207d
Compare
Member
Author
|
@notmandatory ready for you to base #1194 on! |
LLFourn
reviewed
Mar 27, 2024
Collaborator
LLFourn
left a comment
There was a problem hiding this comment.
Left some comments so I can understand a few things better before giving a full review.
5 tasks
Member
I finished rebasing #1194 on this PR and everything is working fine. |
8 tasks
Member
Author
|
@notmandatory I forgot to mention I also need to rebase this PR on master 😬 |
e59a142 to
a9aa7c4
Compare
91e8c91 to
bf74f18
Compare
eaebdee to
e5c81a2
Compare
5 tasks
76e1d97 to
954b6b5
Compare
This creates a checkpoint linked list which contains all blocks.
Previously, we would update the `TxGraph` and `KeychainTxOutIndex` first, then create a second update for `LocalChain`. This required locking the receiving structures 3 times (instead of twice, which is optimal). This PR eliminates this requirement by making use of the new `query` method of `CheckPoint`. Examples are also updated to use the new API.
These methods are no longer needed as we can determine missing heights directly from the `CheckPoint` tip.
This gets the genesis hash of the env blockchain.
We ensure that calling `finalize_chain_update` does not result in a chain which removed previous heights and all anchor heights are included.
Since we want to keep these methods private.
Co-authored-by: LLFourn <lloyd.fourn@gmail.com>
64bc6ca to
7554b1a
Compare
Co-authored-by: LLFourn <lloyd.fourn@gmail.com>
7554b1a to
72fe65b
Compare
690cccb to
a504a1d
Compare
The intention is to remove the `Update::introduce_older_blocks` parameter and update the local chain directly with `CheckPoint`. This simplifies the API and there is a way to do this efficiently.
a504a1d to
77d3595
Compare
evanlinjin
commented
Apr 22, 2024
`merge_chains` now returns a tuple of the resultant checkpoint AND changeset. This is arguably a more readable/understandable setup. To do this, we had to create `CheckPoint::apply_changeset` which is kept as a private method. Thank you @ValuedMammal for the suggestion. Co-authored-by: valuedvalued mammal <valuedmammal@protonmail.com>
877f728 to
96a9aa6
Compare
LLFourn
approved these changes
Apr 22, 2024
Closed
28 tasks
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
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 #1354
Description
Built on top of both #1369 and #1373, we simplify the
EsploraExtAPI by removing theupdate_local_chainmethod and havingfull_scanandsyncupdate the local chain in the same call. Thefull_scanandsyncmethods now takes in an additional input (local_tip) which provides us with the view of theLocalChainbefore the update. These methods now return structsFullScanUpdateandSyncUpdate.The examples are updated to use this new API.
TxGraph::missing_heightsandtx_graph::ChangeSet::missing_heights_fromare no longer needed, therefore they are removed.Additionally, we used this opportunity to simplify the logic which updates
LocalChain. We got rid of thelocal_chain::Updatestruct (which contained the updateCheckPointtip and aboolwhich signaled whether we want to introduce blocks below point of agreement). It turns out we can use something likeCheckPoint::insertso the chain source can craft an update based on the old tip. This way, we can make better use ofmerge_chains' optimization that compares theArcpointers of the local and update chain (before we were crafting the update chain NOT based on top of the previous local chain). With this, we no longer need theUpdate::introduce_older_blockfield since the logic will naturally break when we reach a matchingArcpointer.Notes to the reviewers
LocalChain's update now happens withinEsploraExt::full_scanandEsploraExt::sync. Creating theLocalChainupdate is now split into two methods (fetch_latest_blocksandchain_update) that are called before and after fetching transactions and anchors.bdk_esplora. One for blocking and one for async.Changelog notice
EsploraExtAPI so that sync only requires one round of fetching data. Thelocal_chain_updatemethod is removed and thelocal_tipparameter is added to thefull_scanandsyncmethods.TxGraph::missing_heightsandtx_graph::ChangeSet::missing_heights_frommethods.CheckPoint::insertwhich allows convenient checkpoint-insertion. This is intended for use by chain-sources when crafting an update.merge_chainsto also return the resultantCheckPointtip.LocalChainlogic - use the updateCheckPointas the newCheckPointtip when possible.Checklists
All Submissions:
cargo fmtandcargo clippybefore committingNew Features: