bitcoindevkit / rust-esplora-client

Bitcoin Esplora API client library. Supports plaintext, TLS and Onion servers. Blocking or async.
MIT License
28 stars 44 forks source link

fix(wallet_esplora): Gracefully handle 429s errors #59

Closed realeinherjar closed 10 months ago

realeinherjar commented 11 months ago

Closes https://github.com/bitcoindevkit/bdk/issues/1120

Details

realeinherjar commented 11 months ago

Thanks for the feedback. It is helpful. I will try to modify the approach. 👍🏻

LLFourn commented 11 months ago

Aproach NACK

Thank you for taking a shot at this.

Why 500ms? What if the server expects us to wait for 600ms? I would prefer if we checked the Retry-After header. If that does not exist, then have an exponential back-off delay as a fallback.

Do we need the async recursion? Is there a way to do it with a loop?

I also think introducing tokio as an async dependency is a big ask. Would it be trivial to implement a simple Future using Instant?

Not really trivial and using std::time::Instant breaks WASM probably. We'd need to do some work to have the timer work on WASM: See: https://github.com/whizsid/wasmtimer-rs

I would avoid sleeping and just lower the number of batch requests in the next round by the number of 429s you get.

PS are we testing this crate on WASM? If not PR fixing that before this one would be good.

realeinherjar commented 10 months ago

I will try another approach in the bdk_esplora crate and not here.

EDIT: More specifically, 429s could be handled inside this loop that calls scripthash_txs:

https://github.com/bitcoindevkit/bdk/blob/3569acca0b3f3540e1f1a2278794eac4642a05e4/crates/esplora/src/blocking_ext.rs#L213-L239

https://github.com/bitcoindevkit/bdk/blob/3569acca0b3f3540e1f1a2278794eac4642a05e4/crates/esplora/src/async_ext.rs#L234-L242