informalsystems / tendermint-rs

Client libraries for Tendermint/CometBFT in Rust!
Apache License 2.0
615 stars 224 forks source link

Make rpc::Client and SubscriptionClient traits usable in wasm #1385

Closed jstuczyn closed 11 months ago

jstuczyn commented 11 months ago

Description

Currently it's impossible to implement those traits in wasm as by default async_trait imposes the Send bound. This, however, makes it unusable in wasm. Therefore I propose introducing conditional compilation based on target:

#[cfg_attr(target_arch = "wasm32", async_trait(?Send))]
#[cfg_attr(not(target_arch = "wasm32"), async_trait)]
pub trait Client { ... }

Definition of "done"

romac commented 11 months ago

Thanks for the issue and associated PR, looks good!

For my own knowledge, would mind going into a bit more detail as to why Send bounds on result types of desugared async fns prevents implemented them when targeting WASM? I am asking because that's the first time I hear about that and I am genuinely interested in understanding why.

jstuczyn commented 11 months ago

Hi, I'm extremely glad you asked because turns out there was a bit of misunderstanding on my end. It's actually not a wasm32 target-wide issue, but is related to the futures created by wasm-bindgen that are then run in the browser: https://github.com/rustwasm/wasm-bindgen/issues/2833 I've been dealing with those for so long that I forgot other targets exist 😅 So I assume that, for example, under wasm32-wasi this isn't a problem at all. So perhaps, rather than making the trait boundary be target locked, a feature-lock solution would be more appropriate. What do you reckon?

tony-iqlusion commented 11 months ago

AFIT is shipping in Rust 1.75, FWIW. That's a couple weeks away and will obsolete async-trait.

romac commented 11 months ago

I'd rather avoid adding one more feature flag for the moment, so let's merge this as-is for now and we can revisit once Rust 1.75 is out and we decide to bump the MSRV.