-
Notifications
You must be signed in to change notification settings - Fork 45
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
A few concurrency-related issues Block has faced while using bdk-ffi #205
Comments
Just linking some uniffi-rs threads related to asynchronicity here: |
I did a little research on issue 1 and unfortunately in our current API access to Short term (Q4 2022) I think there's a couple things we can do, first we need to figure out why your electrum sync is taking so long, is the wallet you're syncing against very large in terms of addresses and/or transactions? Next it'd be good to know how you're syncing (normal IP or Tor?), and what kind of electrum server you're syncing against (default electrs?). And finally what does your Long term (Q1 2023) we should have the In the longer term there is an open issue on uniffi-rs to expose async functions directly in the target languages. But it's a tricky issue since they support multiple languages that all work a bit differently. But for now once we're able to reproduce this issue I think we'll be able to find a solution with all the threading happening in Kotlin (or Swift). |
Here's the iOS implementation right now:
@quad @0xarvin could you help answer some of the above questions WRT our electrum setup?
I haven't tried to reproduce in a while, but the wallets we were seeing issues on didn't have particularly large numbers of addresses or transactions. We are also polling for sync every 5 seconds, but only if there's not a current sync already in progress. |
No. They're newly created wallet with only a few (less than 10) transactions.
Normal IP.
Fulcrum. |
Has anyone compared performance of |
Here's some very interesting performance analysis of |
Thanks! Will circulate these back with the team. Somewhat related to these issues is the issue of the sync |
I have an open issue for the sync |
Thanks for following up @notmandatory! Do you know the timeline for the v1 release? |
Hi, the original goal for |
Got it, thanks for the context 👍 |
We've got a timeline we're going to try and stick to: https://github.com/LLFourn/bdk_core_staging#development-and-release-plan with the first release of
I'll leave to @notmandatory as to what gets FFI bindings and when :) |
I'm coming back to this and see that most issues here will be solved by using the 1.0 API, or require new/updated issues to discuss them further. Bitkey team feel free to reopen if you feel like you'd rather continue the discussion here! The 2 final things I think the bindings need before being ready for testing/prod is (a) simple sync/full_scan, and (b) SQLite store. But both of those are partially solved right now in the alpha releases (flat file storage and slightly more complex sync), so they're ready to start experimenting with! |
At Block, We're using the BDK via the bdk-ffi bindings in our iOS and Android Bitcoin Wallet. It's been incredibly helpful so far! We surfaced a few issues related to synchronicity in the BDK #mobile Discord channel and are creating this issue to track them. As discussed in the thread, I'm starting with a single issue with multiple related topics. But I'm happy to break this into three separate GitHub issues if the team prefers! Also, I'm acknowledging I'm not using the issue template until we've decided if this should be a single issue or three. But I'm happy to provide the additional detail requested in the template when helpful.
Issue 1: Concurrent Calls to Wallet in BDK
If we make calls concurrently to the BDK wallet, we sometimes experience the main thread hanging on a mutex lock. The calls involved in one example were
sync
andget_balance
. Below is a sample stack trace from an app hang when we were calling into the BDK from multiple threads:get_balance
on Thread 1 (main) andsync
on Thread 6 (background):We changed our implementation to ensure all calls made to the BDK wallet are serialized, which fixes the issue. But we worry about performance implications with this solution. We call
sync
often, and have also observed that call hanging and not returning (separate issue below). So, if our calls are forced to be serialized, we could end up in a situation where we have to wait on a call tosync
(that may never return) to finish before executing some other work.Issue 2: Long-running / Non-cancellable Calls
In our development, we have encountered calls to
sync
that never terminated or returned. Examining the long-running process usually shows work within the BDK related to Electrum. There is not a way to cancel an in-flight call in BDK, which causes issues when / if this happens. On Android we tried to wrap blocking BDK calls into runInterruptible to make them cancellable, which seems to do the trick. We additionally tried to wrap the calls with withTimeout to prevent them from hanging for too long but that doesn’t work. We also have concerns about resources/memory leaks when wrapping blocking calls into cancellable coroutines without being able to properly clean up those calls.Issue 3: Synchronous API
All methods in the BDK API are synchronous, but many are expensive and often take between hundreds of milliseconds to multiple seconds. We are currently manually wrapping the BDK API with our own asynchronous API. It would be nice if the BDK API could adopt an async or callback model to reduce the overhead of using it.
The text was updated successfully, but these errors were encountered: