Allow opting out of getting LocalChain updates with FullScanRequest/SyncRequest structures
#1478
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
force-pushed
the
modular_esplora
branch
6 times, most recently
from
95dfb37
to
1924240
Compare
3 tasks
storopoli
reviewed
Jun 19, 2024
evanlinjin
force-pushed
the
modular_esplora
branch
from
1924240
to
8da3fab
Compare
|
Since this is a refactor and won't break the |
LagginTimes
reviewed
Jun 21, 2024
That is fine for me. Because some users are waiting on this, I would appreciate it if we have it in soon. It doesn't break any API since it only adds new exposed methods. |
|
Concept ACK |
LLFourn
reviewed
Jun 24, 2024
evanlinjin
force-pushed
the
modular_esplora
branch
4 times, most recently
from
e6ec3f1
to
2599f1d
Compare
LLFourn
requested changes
Jul 3, 2024
|
Hmm I wonder what would happen if we just made updating the chain optional as part of |
evanlinjin
force-pushed
the
modular_esplora
branch
3 times, most recently
from
954b3c8
to
f981105
Compare
Some users would like to use esplora updates with custom `ChainOracle`s (not `LocalChain`). We introduce "low-level" extension methods that populate an update `TxGraph` with associated data. Additionally, much of the logic has been made more efficient. We make use of the `/tx/:txid` endpoint (`get_tx_info` method) to do a single call to get both the tx and tx_status. If the tx already exists, we only fetch the tx_status. The logic for fetching data based on outpoints has been reworked to be more efficient and it now supports parallelism Additionally, the logic for fetching data based on outpoints has been reworked to be more efficient and it now supports parallelism. Documentation has also been reworked. Note that this is NOT a breaking change because the API of `full_scan` and `sync` are not changed.
Prevouts are needed to calculate fees for transactions. They are introduced as floating txouts in the update `TxGraph`. A helper method `insert_prevouts` is added to insert the floating txouts using the `Vin`s returned from Esplora. Also replaced `anchor_from_status` with `insert_anchor_from_status` as we always insert the anchor into the update `TxGraph` after getting it. Also removed `bitcoin` dependency as `bdk_chain` already depends on `bitcoin` (and it's re-exported).
evanlinjin
force-pushed
the
modular_esplora
branch
7 times, most recently
from
3364efb
to
8310cf5
Compare
Change `FullScanRequest` and `SyncRequest` take in a `chain_tip` as an option. In turn, `FullScanResult` and `SyncResult` are also changed to return the update `chain_tip` as an option. This allows the caller to opt-out of getting a `LocalChain` update. Rework `FullScanRequest` and `SyncRequest` to have better ergonomics when inspecting the progress of items of a sync request. Richer progress data is provided to the inspect closure. Introduce `FullScanRequestBuilder` and `SyncRequestBuilder`. Separating out request-construction and request-consumption in different structs simplifies the API and method names. Simplify `EsploraExt` and `EsploraAsyncExt` back to having two methods (`full_scan` and `sync`). The caller can still opt out of fetching a `LocalChain` update with the new `FullScanRequest` and `SyncRequest`.
evanlinjin
force-pushed
the
modular_esplora
branch
from
8310cf5
to
b1a6640
Compare
evanlinjin
changed the title
bdk_esplora more modularLocalChain updates with FullScanRequest/SyncRequest structures
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.
Description
Some users use
bdk_esplorato updatebdk_chainstructures without aLocalChain.This PR introduces "low-level" methods toEsploraExtandEsploraAsyncExtwhich populates aTxGraphupdate with associated data.We change
FullScanRequest/SyncRequestto take in thechain_tipparameter as an option. Spk-based chain sources (bdk_electrumandbdk_esplora) will not fetch a chain-update ifchain_tipisNone, allowing callers to opt-out of receiving updates forLocalChain.We change
FullScanRequest/SyncRequestto have better ergonomics when inspecting the progress of syncing (refer to #1528).We change
FullScanRequest/SyncRequestto be constructed with a builder pattern. This is a better API since we separate request-construction and request-consumption.Additionally, much of the
bdk_esploralogic has been made more efficient (less calls to Esplora) by utilizing the/tx/:txidendpoint (get_tx_infomethod). This method returns the tx and tx_status in one call. The logic for fetching updates for outpoints has been reworked to support parallelism.Documentation has also been updated.
Notes to reviewers
This PR has evolved somewhat. Initially, we were adding more methods on
EsploraExt/EsploraAsyncExtto make syncing/scanning more modular. However, it turns out we can just make thechain_tipparameter of the request structures optional. Since we are changing the request structures, we might as well go further and improve the ergonomics of the whole structures (refer to #1528). This is where we are at with this PR.Unfortunately, the changes are now breaking. I think this is an okay tradeoff for the API improvements (before we get to stable).
Changelog notice
bdk_chain::spk_clientto be constructed via a builder pattern, make providing achain_tipoptional, and have better ergonomics for inspecting progress while syncing.bdk_esplorato be more efficient by reducing the number of calls via the/tx/:txidendpoint. The logic for fetching outpoint updates has been reworked to support parallelism.Checklists
All Submissions:
cargo fmtandcargo clippybefore committingNew Features: