Skip to content

docs(wallet): add sync operation to bdk_wallet examples #274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 6, 2025

Conversation

tvpeter
Copy link
Contributor

@tvpeter tvpeter commented Jun 30, 2025

Description

This PR adds sync operation to bdk_wallet examples for electrum and esplora clients.

Fixes #253

Checklists

All Submissions:

@tvpeter tvpeter changed the title docs(wallet): add sync operation to bdk_wallet examples for electrum and esplora docs(wallet): add sync operation to bdk_wallet examples Jun 30, 2025
@coveralls
Copy link

coveralls commented Jun 30, 2025

Pull Request Test Coverage Report for Build 16763241238

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 84.888%

Totals Coverage Status
Change from base Build 16730629587: 0.0%
Covered Lines: 6651
Relevant Lines: 7835

💛 - Coveralls

@notmandatory notmandatory added the documentation Improvements or additions to documentation label Jul 1, 2025
@notmandatory notmandatory moved this to Needs Review in BDK Wallet Jul 1, 2025
@notmandatory notmandatory added this to the Wallet 2.1.0 milestone Jul 1, 2025
Copy link
Contributor

@oleonardolima oleonardolima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cACK 8e1a6ef

I left a few comments that I think are applicable. Also, I was unable to test the electrum example locally (I guess it's a local problem on my end).

I think it's a good opportunity to also add a forced eviction, and show that to the user. WDYT?

Any reason to not add the sync usage to blocking esplora example ?

@tvpeter
Copy link
Contributor Author

tvpeter commented Jul 2, 2025

I think it's a good opportunity to also add a forced eviction, and show that to the user. WDYT?

Alright, I will do that.

Any reason to not add the sync usage to blocking esplora example ?

No reason, I will also add that.

Thank you for the review. I will make updates.

@tvpeter tvpeter force-pushed the doc/wallet-sync-example branch 3 times, most recently from 98cb6b9 to 4130f46 Compare July 4, 2025 11:15
@tvpeter
Copy link
Contributor Author

tvpeter commented Jul 4, 2025

I think it's a good opportunity to also add a forced eviction, and show that to the user. WDYT?

Alright, I will do that.

I have added the eviction logic that identifies transactions that were unconfirmed before a partial sync but are no longer canonical after the sync.

Any reason to not add the sync usage to blocking esplora example ?

No reason, I will also add that.

I have also added the sync to the esplora blocking example.

Another thing I considered was reducing the output of the full scan by a multiple of say 5 or 10 as it is getting lengthy.

@tvpeter tvpeter requested a review from oleonardolima July 4, 2025 11:37
@tvpeter tvpeter force-pushed the doc/wallet-sync-example branch from 4130f46 to 50dacbf Compare July 9, 2025 14:54
@tvpeter tvpeter force-pushed the doc/wallet-sync-example branch 2 times, most recently from 5cf1d52 to cd29221 Compare July 29, 2025 10:17
@notmandatory
Copy link
Member

I pushed a new commit to fix a couple things I found while testing:

  1. for electrum, esplora examples they all now use sqlite+testnet and print mempool.space URLs for created txids
  2. for esplore examples I get the fee rate estimate from explora rather than using the default 1 sat/vb
  3. for the RPC example I added a justfile to help testing with regtest bitcoind

Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK ce0a17e

@oleonardolima
Copy link
Contributor

I'm not sure why, but now that it's using testnet instead of signet, I keep getting timeout errors and the RBF fails because the transaction is already mined in within that timeframe.

tvpeter added 3 commits August 1, 2025 17:43
- add partial syncing to electrum, esplora_async
and esplora_blocking examples
- add applying evicted txns to wallet in electrum,
esplora_async and esplora_blocking examples
@tvpeter tvpeter force-pushed the doc/wallet-sync-example branch 4 times, most recently from 75178be to ae7af35 Compare August 1, 2025 17:27
@notmandatory notmandatory force-pushed the doc/wallet-sync-example branch 2 times, most recently from f31ae60 to 10f56f5 Compare August 2, 2025 23:03
@notmandatory
Copy link
Member

notmandatory commented Aug 2, 2025

I'm not sure why, but now that it's using testnet instead of signet, I keep getting timeout errors and the RBF fails because the transaction is already mined in within that timeframe.

I switched it to use mempool.space and testnet4 which seem to be more reliable. It's also easier to get testnet4 coins from mempool.space since all you need to do is login with your GitHub account. And I cleaned up the just file for RPC to use the stop command, good suggestion @oleonardolima.

@tvpeter tvpeter force-pushed the doc/wallet-sync-example branch from 10f56f5 to ade903d Compare August 3, 2025 18:17
@tvpeter
Copy link
Contributor Author

tvpeter commented Aug 3, 2025

@ValuedMammal following your comments on the issue here, I have removed the step that call the Wallet::apply_evicted_txs to evict txs.

Thank you.

@notmandatory notmandatory force-pushed the doc/wallet-sync-example branch from ade903d to bb4d79b Compare August 4, 2025 17:54
Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK bb4d79b

Copy link
Contributor

@oleonardolima oleonardolima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

partial tACK bb4d79b

I've test for all examples, both esplora and electrum works just fine.

I found a problem in the wallet_rpc one, I've left it as a comment.

… mempool.space

For electrum, esplora examples print mempool.space tx URLs and add delay before RBF.
For rpc example also add justfile to help testing with regtest bitcoind.
@notmandatory notmandatory force-pushed the doc/wallet-sync-example branch from bb4d79b to 11bfc2f Compare August 5, 2025 23:02
Copy link
Contributor

@oleonardolima oleonardolima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK 11bfc2f

@notmandatory notmandatory merged commit d0a0abc into bitcoindevkit:master Aug 6, 2025
20 checks passed
@github-project-automation github-project-automation bot moved this from Needs Review to Done in BDK Wallet Aug 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Missing bdk_wallet examples with sync
4 participants