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.

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

Make the blockchain interface async again on wasm32-unknown-unknown #28

Merged
merged 1 commit into from
Jul 21, 2020

Conversation

afilini
Copy link
Member

@afilini afilini commented Jul 20, 2020

The procedural macro #[maybe_async] makes a method or every method of a trait
"async" whenever the target_arch is wasm32, and leaves them untouched on
every other platform.

The macro maybe_await!($e:expr) can be used to call maybe_async methods on
multi-platform code: it expands to $e on non-wasm32 platforms and to
$e.await on wasm32.

The macro await_or_block!($e:expr) can be used to contain async code as much
as possible: it expands to tokio::runtime::Runtime::new().unwrap().block_on($e)
on non-wasm32 platforms, and to $e.await on wasm32.

@afilini afilini force-pushed the async-isolation branch 4 times, most recently from c39c453 to 177c515 Compare July 20, 2020 17:39
The procedural macro `#[maybe_async]` makes a method or every method of a trait
"async" whenever the target_arch is `wasm32`, and leaves them untouched on
every other platform.

The macro `maybe_await!($e:expr)` can be used to call `maybe_async` methods on
multi-platform code: it expands to `$e` on non-wasm32 platforms and to
`$e.await` on wasm32.

The macro `await_or_block!($e:expr)` can be used to contain async code as much
as possible: it expands to `tokio::runtime::Runtime::new().unwrap().block_on($e)`
on non-wasm32 platforms, and to `$e.await` on wasm32.
@LLFourn
Copy link
Contributor

LLFourn commented Jul 21, 2020

I've been poking around this library recently. Obviously it's early days but It looks like a really nice design so far.
One thing I'm wondering about is why everything is not just consistently async? (it seems related to this issue).

Thanks!

@afilini afilini merged commit 4fcf7ac into master Jul 21, 2020
@afilini
Copy link
Member Author

afilini commented Aug 3, 2020

I've been poking around this library recently. Obviously it's early days but It looks like a really nice design so far.
One thing I'm wondering about is why everything is not just consistently async? (it seems related to this issue).

Thanks!

Sorry it took a while to answer, I had totally missed your comment.

The main reason is that async stuff are in general more painful to use, they add stricter requirements on lifetimes of your structs and they require a runtime to even be used. So using them in projects that are not async would force you to potentially change quite a few parts of your code to have the right lifetimes, plus you would have to wrap every call with the runtime stuff.

It's also very hard to make functions that "look synchronous" but that behind the curtains hide the complexity of starting a runtime, because if you find yourself in the opposite situation where you have a project that uses async and you try to call one of these functions you'll get panics because you can't start a runtime from within a runtime. This was actually the issue that made me switch back to non-async: I was trying to use rust-electrum-client in another project of mine that is async and i was calling the "pseudo-sync" interface that would start the runtime automatically, and that was causing a lot of troubles. But if you are inside a function that is not defined as "async" (think about implementing a trait for instance) then you are in this awkward situation where you can't use the "explicitly async" interface because you can't use await, but you also can't use the "pseudo-sync" interface because that can't start a runtime.

I think, in general, you can summarize this to: you can call fully-sync code from everywhere (albeit with performance penalties if you do it in the wrong spot) but not the other way around. So I think if you are designing a library you should always try to make it easier for other projects to integrate your code, and using non-async is the best way to achieve that.

I also like to let the user of the library have full control over what's running and when, and with async runtimes that would be weakened because the runtime would spawn background threads that cannot be easily controlled from the outside. This could mean for instance random crashes in your app while you are not even doing anything related to this library just because one background thread did something wrong.

Considering that async is mostly useful in I/O heavy applications (because you can do something else while you wait for I/O) I don't think this project would particularly benefit from it, and it was starting to become more of a "liability" than a positive element.

Unfortunately because of the way browsers work they require you to use asynchronous operations, so this specific PR adds back some async parts, but tries to minimize them as much as possible and contain them only where strictly necessary (only the "blockchain" interface part). This ultimately means that if you use this project to make a desktop/mobile wallet you can completely forget about any async stuff. You'll only have to deal with async code if you try to use this library in a web environment, but at that point you are already forced to use async by the browser so it probably wouldn't make a big difference for you anyways.

Hope this clarifies what I was trying to do, and again, sorry it took so long to answer!

@LLFourn
Copy link
Contributor

LLFourn commented Aug 7, 2020

@afilini Thanks for the detailed response. I think I understand the problem. Async isn't needed in most contexts and so why force the complexities of dealing with it onto people who get none of the upside especially in a security critical context.

This was actually the issue that made me switch back to non-async: I was trying to use rust-electrum-client in another project of mine that is async and i was calling the "pseudo-sync" interface that would start the runtime automatically, and that was causing a lot of troubles. But if you are inside a function that is not defined as "async" (think about implementing a trait for instance) then you are in this awkward situation where you can't use the "explicitly async" interface because you can't use await, but you also can't use the "pseudo-sync" interface because that can't start a runtime.

My interpretation of your comment is that pseudo-sync is a bad idea. But it looks like your solution is pseudo-sync? Or is block_on safe to call in these contexts.

@afilini
Copy link
Member Author

afilini commented Aug 7, 2020

Yeah it currently is, but only in the esplora client because it's the only one that uses block_on(). I'm planning to update the maybe_async proc macro though, so that you can "ask for" the async interface if you are an async project. I'm probably gonna do it with an opt-in feature.

So yeah, it's technically not 100% clean yet, but the bad stuff are all isolated in a single place and I'll get rid of them as soon as I can.

@afilini afilini deleted the async-isolation branch August 7, 2020 07:52
afilini added a commit to afilini/bdk that referenced this pull request Aug 10, 2020
rajarshimaitra pushed a commit to rajarshimaitra/bdk that referenced this pull request May 8, 2021
nickfarrow pushed a commit to nickfarrow/bdk that referenced this pull request Feb 21, 2022
Added and_n and andor compiler rules
notmandatory pushed a commit to notmandatory/bdk that referenced this pull request Mar 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants