Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add BdkActor #750

Closed
wants to merge 2 commits into from
Closed

Add BdkActor #750

wants to merge 2 commits into from

Commits on Jun 8, 2023

  1. chore: Remove unneeded OnChainWallet abstraction 

    A factory function is good enough, we were not using the struct for anything
other than calling new()
    @klochowicz @luckysori
    klochowicz authored and luckysori committed Jun 8, 2023
    Configuration menu
    Copy the full SHA
    8e98449 View commit details
    Browse the repository at this point in the history

  2. feat: Add wallet actor 

    An actor that manages the `bdk::Wallet` resource. It allows us to use
the wallet whilst the inevitably expensive on-chain sync is happening
in the background.

We would like to use the async version of `EsploraBlockchain`, but
bitcoindevkit/bdk#165 is still an issue.

The only hacky bit stems from the fact that we still have to implement
certain non-async foreign traits and to access any `bdk::Wallet`
resource we now have to go via async methods, since the actor is
async.

To do so, at some point we have to call an async function "from a sync
context". The natural way to do so (for me) would be to use
`runtime.block_on`, which schedules an async task and waits for it to
resolve, blocking the thread. *But*, these non-async foreign trait
methods are actually called elswhere in _async_ contexts. This leads
to `runtime.block_on` panicking because `tokio` is trying to prevent
us from blocking a thread in an async context. This is analogous to us
misusing async and doing an expensive CPU-bound computation in an
async context, but here `tokio` is able to "aid" us since `block_on`
is provided by `tokio`.

The solution is to use the following pattern:

```rust
tokio::task::block_in_place(|| {
    tokio::runtime::Handle::current().block_on(async {
        // async code
    })
});
```

From the documentation of `block_in_place`, we are able to run "the
provided blocking function on the current thread without blocking the
executor". We therefore avoid the panic, as we no longer block the
executor when calling `block_on`.

This has one final side-effect, which is that all `ln-dlc-node` async
tests now need to go back to using the `multi_thread` flavour.
`block_in_place` works by moving all scheduled tasks to a different
worker thread and without `multi_thread` there is only one worker
thread, so it just panics.

Co-authored-by: Mariusz Klochowicz <[email protected]>
    @luckysori @klochowicz
    luckysori and klochowicz committed Jun 8, 2023
    Configuration menu
    Copy the full SHA
    df57f34 View commit details
    Browse the repository at this point in the history