feat: add one-liner wallet sync API with ElectrumSync#2094
feat: add one-liner wallet sync API with ElectrumSync#2094ShigrafS wants to merge 4 commits intobitcoindevkit:masterfrom
Conversation
Signed-off-by: ShigrafS <shigrafsalik@proton.me>
Signed-off-by: ShigrafS <shigrafsalik@proton.me>
|
The PR is ready for review. |
|
Where exactly is the "small, focused helper abstraction"? You changed nothing of relevance in the There is a whole lot of unrelated changes - I'm assuming this is an attempt at making thing run/compile on windows? This does not belong in this PR - feel free to create a new PR for these changes. |
|
Fixed it. |
|
@ShigrafS the "one liner helper" is currently an example and not part of the library. |
|
@evanlinjin I was thinking this fits right within the scope of the issue with respect to the last PR. |
|
This PR doesn't add a 1-liner API to the bdk_electrum crate, nor does it make sense to add one IMO. I think the core issue is that we kept #1974 open when it should not be. Worse, we added a My suggestion is to either:
|
|
@evanlinjin @thunderbiscuit @luisschwab Thanks for the feedback so far. Based on @evanlinjin’s comment [here]) and the discussion in this thread, I’ll be reducing the scope of this PR to an example-only implementation demonstrating a one-liner Electrum wallet sync. This keeps it safe, minimal, and in line with current BDK library constraints. Given that this example fully demonstrates the intended functionality from #1974, I’d suggest we close the original issue, as adding it to the library API is beyond the scope of the repo at present. I’ll update the PR accordingly to reflect the example-only version. Thanks! |
Reverts library API changes and consolidates one-liner sync logic into a single self-contained example in examples/example_electrum. Changes: - Revert public API additions in bdk_electrum - Remove test_api_compatibility - Remove separate example_one_liner crate - Add ElectrumSync helper to examples/example_electrum/src/bin/one_liner_sync.rs Signed-off-by: ShigrafS <shigrafsalik@proton.me>
… example This commit also reverts accidental changes to crates/electrum/src/lib.rs and crates/electrum/Cargo.toml to ensure no library API changes are introduced. Signed-off-by: ShigrafS <shigrafsalik@proton.me>
|
NACK Thanks for trying to contribute, but I still don't see the value in this. |
|
@evanlinjin Given the consensus that this doesn’t belong in the BDK repo, I’ll close the PR. |
3 participants
Closes #1974
Description
This PR introduces a small, focused helper abstraction to reduce the boilerplate required to synchronize a wallet index with an Electrum server, addressing the usability concerns raised in #1974.
The main addition is the
ElectrumSynchelper andSyncOptions, which wrap existingSyncRequestandFullScanRequestflows into a single, ergonomic call while preserving the existingBdkElectrumClientcache and behavior.Concretely, this PR:
ElectrumSync, a lightweight helper for performing Electrum syncs using a sharedBdkElectrumClientSyncOptionsto configure fast sync vs full scan, stop gap, batch size, and prevout fetchingbdk_electrumfor reuseThe helper is intentionally minimal and builds directly on top of existing BDK primitives without introducing a new builder API or altering wallet abstractions.
Notes to the reviewers
This PR is informed by the feedback and architectural concerns raised in #2059.
In particular:
BdkElectrumClientand its cache across sync calls, avoiding the performance regressions discussed previously.The goal of this PR is to provide a convenience layer that demonstrates a simplified sync flow while remaining close to existing abstractions.
Feedback on whether this belongs as a public helper or should instead live purely as an example is very welcome.
Changelog notice
ElectrumSyncandSyncOptionshelpers to simplify Electrum wallet synchronizationChecklists
All Submissions:
New Features:
Bugfixes: