bitcoindevkit / bdk

A modern, lightweight, descriptor-based wallet library written in Rust!
Other
855 stars 307 forks source link

Reqwest to Esplora panics in WASM #778

Open cryptoquick opened 2 years ago

cryptoquick commented 2 years ago

Describe the bug

I've been having a number of strange errors with async reqwest. Initially I thought I was being throttled or something, but I'm actually starting to think it might be reqwest that's flakey. For example, this one I saw was with mempool.space:

console.log div contained:
    panicked at 'called `Result::unwrap()` on an `Err` value: JsValue("Esplora(Reqwest(reqwest::Error { kind: Status(400), url: Url { scheme: "https", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("mempool.space")), port: None, path: "/testnet/api/tx", query: None, fragment: None } }))")', /home/hunter/Projects/diba/bitmask-core-3/src/web.rs:18:35

So I tried pointing it to my local Esplora server, and I get this much more verbose error, for running the exact same test:

    panicked at 'called `Result::unwrap()` on an `Err` value: JsValue("Esplora(Reqwest(reqwest::Error { kind: Request, source: "JsValue(TypeError: Failed to fetch\nTypeError: Failed to fetch\n    at http://127.0.0.1:39661/wasm-bindgen-test:776:21\n    at logError (http://127.0.0.1:39661/wasm-bindgen-test:220:18)\n    at imports.wbg.__wbg_fetch_386f87a3ebf5003c (http://127.0.0.1:39661/wasm-bindgen-test:775:68)\n    at reqwest::wasm::client::js_fetch::hf71c9a8da6721729 (http://127.0.0.1:39661/wasm-bindgen-test_bg.wasm:wasm-function[2752]:0x2dbad8)\n    at <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::h2605fac818d246d7 (http://127.0.0.1:39661/wasm-bindgen-test_bg.wasm:wasm-function[158]:0x69d99)\n    at <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::hb7d56330de2d7fb5 (http://127.0.0.1:39661/wasm-bindgen-test_bg.wasm:wasm-function[701]:0x1db3be)\n    at <S as futures_core::stream::TryStream>::try_poll_next::h80f00c85ef3755fb (http://127.0.0.1:39661/wasm-bindgen-test_bg.wasm:wasm-function[504]:0x18d4ce)\n    at <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::hedb89f65bb27d02e (http://127.0.0.1:39661/wasm-bindgen-test_bg.wasm:wasm-function[161]:0x6e417)\n    at <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::hfff785a967324fc6 (http://127.0.0.1:39661/wasm-bindgen-test_bg.wasm:wasm-function[359]:0x13d739)\n    at <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::hd9db6d42863d2823 (http://127.0.0.1:39661/wasm-bindgen-test_bg.wasm:wasm-function[182]:0x8cd2c))" }))")', /home/hunter/Projects/diba/bitmask-core-3/src/web.rs:18:35

So, that's quite strange... I looked for something similar, and I found this issue, but that's actually a very different issue, since I'm trying this with servers that don't use HTTPS and I'm getting 400 Bad Request errors, which are different than the one provided by Reqwest in that issue.

To Reproduce
Not sure if this is unhelpful (TMI?), but the code I'm testing is in this commit: https://github.com/diba-io/bitmask-core/tree/da7b8a9b433e8a3e1beb334239a5d9c37b865193

And this is what I'm using to run the tests (feel free to use your own wallet with testnet sats, or just use this one): TEST_WALLET_SEED="upgrade pink beef crisp fatal kick rough guess together talent address payment" wasm-pack test --release --headless --chrome

Expected behavior
Making a request to Esplora should behave the same either from WASM or native desktop.

Build environment

Additional context
I noticed when pointing it to my local Esplora server, the transaction log looked like this:

Screenshot from 2022-10-06 20-33-01

It makes about 8 good requests and then just peters out.

However, during more nominal modes of operation, such as running a test that compiles to native desktop and not wasm:

Screenshot from 2022-10-06 20-33-22

It will make many, many requests.

This kind of test can be run like this:

TEST_WALLET_SEED="upgrade pink beef crisp fatal kick rough guess together talent address payment" BITCOIN_ELECTRUM_API_TESTNET=127.0.0.1:60001 BITCOIN_EXPLORER_API_TESTNET=http://127.0.0.1:3000 cargo test -- --nocapture

If you don't have your own local Esplora instance, the env vars can be omitted and it should default to using Mempool's. There's also an IP address to one of our own servers in the .env file.

I know you're probably tired of hearing from me, but this is one of those things where I'm really wondering if I'm the only one experiencing this issue? I can't be, right? Honestly I could just use a sanity-check, I'd be super grateful.

And thinking about it a little further, for some reason we're actually not using Reqwest on our end, we use gloo_net. I'd be happy to provide an implementation similar to this one if that's something y'all would be open to: https://github.com/diba-io/bitmask-core/blob/6f2ca7c946685b1f0c25702d0fc60ed6fbee368b/src/util.rs#L62-L138

afilini commented 2 years ago

I think you should try reporting this to reqwest instead, it doesn't sound like a BDK bug. If the exact same code works on x86 but not on wasm I guess the issue is in the library itself.

Considering the bdk_core effort I don't think it's worth spending time on a gloo_net Blockchain implementation right now

cryptoquick commented 2 years ago

I think you're right. I actually tried experimenting with what it'd take to add wasm-only gloo_net support to rust_esplora_client, and it looks like it'd be a pain.

cryptoquick commented 2 years ago

I think I figured out what the issue is, @afilini. BDK isn't providing the RPC message upon reaching an error. What it returns is this: Esplora(Reqwest(reqwest::Error { kind: Status(400), url: Url { scheme: "https", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("mempool.space")), port: None, path: "/testnet/api/tx", query: None, fragment: None } })) But that's super unhelpful, compared to the actual RPC error response, found in the network tab: sendrawtransaction RPC error: {"code":-26,"message":"min relay fee not met, 211 < 212"} Also, I'm not sure why I see this error so often. Is there an error in what I set my fees to? Maybe the default FeeRate shouldn't be 1.0, but something closer to 1.1? So, really, there's two issues here, the default fee rate is always just a smidge too low, and proper RPC error messages aren't communicated back to the client, making it very difficult for applications to handle specific errors. Plus the extra debugging I at first didn't realize I needed to do. I'm kinda lucky I was able to do this in a browser, this error was completely opaque in my backend unit test.

notmandatory commented 1 year ago

I think undershooting the fee rate is related to #774.

cryptoquick commented 1 year ago

You might be right, @notmandatory. Once that's merged, feel free to close this. It might be nice to have another issue for passing RPC node errors through into Rust callers, but I think that might be an issue for a separate project.