Simplified `EsploraExt` API #1380

evanlinjin · 2024-03-21T16:33:20Z

Description

Built on top of both #1369 and #1373, we simplify the EsploraExt API by removing the update_local_chain method and having full_scan and sync update the local chain in the same call. The full_scan and sync methods now takes in an additional input (local_tip) which provides us with the view of the LocalChain before the update. These methods now return structs FullScanUpdate and SyncUpdate.

The examples are updated to use this new API. TxGraph::missing_heights and tx_graph::ChangeSet::missing_heights_from are no longer needed, therefore they are removed.

Additionally, we used this opportunity to simplify the logic which updates LocalChain. We got rid of the local_chain::Update struct (which contained the update CheckPoint tip and a bool which signaled whether we want to introduce blocks below point of agreement). It turns out we can use something like CheckPoint::insert so the chain source can craft an update based on the old tip. This way, we can make better use of merge_chains' optimization that compares the Arc pointers 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 the Update::introduce_older_block field since the logic will naturally break when we reach a matching Arc pointer.

Notes to the reviewers

Obtaining the LocalChain's update now happens within EsploraExt::full_scan and EsploraExt::sync. Creating the LocalChain update is now split into two methods (fetch_latest_blocks and chain_update) that are called before and after fetching transactions and anchors.
We need to duplicate code for bdk_esplora. One for blocking and one for async.

Changelog notice

Changed EsploraExt API so that sync only requires one round of fetching data. The local_chain_update method is removed and the local_tip parameter is added to the full_scan and sync methods.
Removed TxGraph::missing_heights and tx_graph::ChangeSet::missing_heights_from methods.
Introduced CheckPoint::insert which allows convenient checkpoint-insertion. This is intended for use by chain-sources when crafting an update.
Refactored merge_chains to also return the resultant CheckPoint tip.
Optimized the update LocalChain logic - use the update CheckPoint as the new CheckPoint tip when possible.

Checklists

All Submissions:

I've signed all my commits
I followed the contribution guidelines
I ran cargo fmt and cargo clippy before committing

New Features:

I've added tests for the new feature
I've added docs for the new feature

evanlinjin · 2024-03-26T15:03:54Z

@notmandatory ready for you to base #1194 on!

LLFourn

Left some comments so I can understand a few things better before giving a full review.

crates/esplora/src/blocking_ext.rs

notmandatory · 2024-03-29T15:28:52Z

@notmandatory ready for you to base #1194 on!

I finished rebasing #1194 on this PR and everything is working fine.

evanlinjin · 2024-03-29T15:38:20Z

@notmandatory I forgot to mention I also need to rebase this PR on master 😬

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 <[email protected]>

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.

crates/chain/src/local_chain.rs

crates/esplora/src/blocking_ext.rs

@ValuedMammal

`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 <[email protected]>

LLFourn

ACK 96a9aa6

notmandatory added the module-blockchain label Mar 21, 2024

notmandatory assigned evanlinjin Mar 21, 2024

notmandatory added this to the 1.0.0-alpha milestone Mar 21, 2024

notmandatory added the api A breaking API change label Mar 21, 2024

evanlinjin force-pushed the clean_esplora branch 11 times, most recently from 1bb6193 to abddc33 Compare March 26, 2024 15:00

evanlinjin marked this pull request as ready for review March 26, 2024 15:01

evanlinjin requested review from LLFourn and notmandatory March 26, 2024 15:01

evanlinjin force-pushed the clean_esplora branch from abddc33 to f04207d Compare March 26, 2024 15:02

LLFourn reviewed Mar 27, 2024

View reviewed changes

LLFourn mentioned this pull request Mar 27, 2024

feat(chain): add get and range methods to CheckPoint #1369

Merged

5 tasks

notmandatory mentioned this pull request Mar 29, 2024

Add wallet sync_request and full_scan_request functions #1194

Closed

8 tasks

evanlinjin force-pushed the clean_esplora branch 5 times, most recently from e59a142 to a9aa7c4 Compare April 2, 2024 13:23

evanlinjin force-pushed the clean_esplora branch from 91e8c91 to bf74f18 Compare April 2, 2024 13:45

evanlinjin force-pushed the clean_esplora branch 2 times, most recently from eaebdee to e5c81a2 Compare April 12, 2024 04:07

LagginTimes mentioned this pull request Apr 12, 2024

Update bdk_electrum crate to use sync/full-scan structs #1403

Merged

5 tasks

evanlinjin force-pushed the clean_esplora branch from 76e1d97 to 954b6b5 Compare April 15, 2024 11:52

evanlinjin and others added 7 commits April 16, 2024 17:51

feat(testenv): add make_checkpoint_tip

1e99793

This creates a checkpoint linked list which contains all blocks.

chore(chain)!: rm missing_heights and missing_heights_from methods

886d72e

These methods are no longer needed as we can determine missing heights directly from the `CheckPoint` tip.

feat(testenv): add genesis_hash method

494d253

This gets the genesis hash of the env blockchain.

test(esplora): add test_finalize_chain_update

a6e613e

We ensure that calling `finalize_chain_update` does not result in a chain which removed previous heights and all anchor heights are included.

test(esplora): move esplora tests into src files

519cd75

Since we want to keep these methods private.

feat(chain): introduce CheckPoint::insert

eded1a7

Co-authored-by: LLFourn <[email protected]>

evanlinjin force-pushed the clean_esplora branch from 64bc6ca to 7554b1a Compare April 16, 2024 11:31

feat(esplora)!: simplify chain update logic

72fe65b

Co-authored-by: LLFourn <[email protected]>

evanlinjin force-pushed the clean_esplora branch from 7554b1a to 72fe65b Compare April 16, 2024 11:40

test(chain): fix incorrect test case

1269b06

evanlinjin force-pushed the clean_esplora branch 2 times, most recently from 690cccb to a504a1d Compare April 17, 2024 02:56

feat(chain)!: rm local_chain::Update

77d3595

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.

evanlinjin force-pushed the clean_esplora branch from a504a1d to 77d3595 Compare April 17, 2024 02:58

feat(chain): optimize merge_chains

daf588f

ValuedMammal reviewed Apr 17, 2024

View reviewed changes

crates/chain/src/local_chain.rs Outdated Show resolved Hide resolved

ValuedMammal reviewed Apr 17, 2024

View reviewed changes

crates/chain/src/local_chain.rs Outdated Show resolved Hide resolved

chore(chain): fix comment

2f22987

evanlinjin commented Apr 22, 2024

View reviewed changes

crates/esplora/src/blocking_ext.rs Show resolved Hide resolved

evanlinjin force-pushed the clean_esplora branch from 877f728 to 96a9aa6 Compare April 22, 2024 09:39

LLFourn approved these changes Apr 22, 2024

View reviewed changes

evanlinjin merged commit 8e73998 into bitcoindevkit:master Apr 22, 2024
12 checks passed

danielabrozzoni mentioned this pull request May 2, 2024

Release 1.0.0-alpha.10 #1418

Closed

28 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplified `EsploraExt` API #1380

Simplified `EsploraExt` API #1380

evanlinjin commented Mar 21, 2024 •

edited

Loading

evanlinjin commented Mar 26, 2024

LLFourn left a comment

notmandatory commented Mar 29, 2024

evanlinjin commented Mar 29, 2024

LLFourn left a comment

Simplified EsploraExt API #1380

Simplified EsploraExt API #1380

Conversation

evanlinjin commented Mar 21, 2024 • edited Loading

Description

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

New Features:

evanlinjin commented Mar 26, 2024

LLFourn left a comment

Choose a reason for hiding this comment

notmandatory commented Mar 29, 2024

evanlinjin commented Mar 29, 2024

LLFourn left a comment

Choose a reason for hiding this comment

Simplified `EsploraExt` API #1380

Simplified `EsploraExt` API #1380

evanlinjin commented Mar 21, 2024 •

edited

Loading