`wallet_esplora_async` returns error 429 while scanning

[danielabrozzoni]

Describe the bug
Running wallet_esplora_async we panic due to blockstream.info replying with error code 429 to us (too many requests)

To Reproduce
cargo run in bdk/example-crates/wallet_esplora_async produces the following:

Scanning keychain [External] 1   2   3   4   5   6   7   8   9   10  11  12  13  14  15  16  17  18  19  20  21  22  23  24 Error: Reqwest(reqwest::Error { kind: Status(429), url: Url { scheme: "https", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("blockstream.info")), port: None, path: "/testnet/api/scripthash/aaaf78825de8d6cce8fc0865688238e8169f64967f102c10db8112a806ff9713/txs", query: None, fragment: None } })

Build environment

• BDK tag/commit: fda8c75

[realeinherjar]

Last month, Aug 2023, I was playing with BDK alpha-1 and I had this problem with blockstream, see realeinherjar/sweepr#11.

A safe solution is to add a 250ms delay between as suggested in Blockstream/esplora#449.

Additionally, a candidate solution for the async clients could be tower::Service

From: seanmonstar/reqwest#491 (comment)

You can use tower::ServiceBuilder to add rate-limiting to any request-like functionality and returns a Future.

[LLFourn]

Why do we panic? I'd expect it to return an error.

@realeinherjar I guess this solution won't work for the blocking client which can also trigger this. How long does it take after getting 429 before you can make another request? It seems like not long from testing so I think we can safely just handle the 429 by slowing down when we receive it. Note it seems like that the async esplora code is somehow much faster than the blocking one so it hits the limit with a lower number of parallel requests. I'm not sure why, maybe reqwest is better at maintaining idle connections. I've checked that it isn't because of http2 using tracing-subscriber (it isn't using it). I wonder why we aren't using http2 here does esplora not support it?

But in conclusion I think we should just slow down on 429. Sleep for a little and decrement the number of parallel requests we're doing. This should be done on both the blocking and non-blocking versions.

[realeinherjar]

But in conclusion I think we should just slow down on 429

I don't like to hardcode wait time, if I have a personal Esplora server that I can DoS with API calls I should be able to do so.

Hence, we should expose the API with an extra argument delay that takes a usize in miliseconds to wait between [async] API calls.
We could even add a "default value" of 250ms with Option<usize> and unwrap_or(250).

Here's where it happens in blocking:

bdk/crates/esplora/src/blocking_ext.rs

Lines 213 to 239 in 59fc1b3

	loop {
	let handles = spks
	.by_ref()
	.take(parallel_requests)
	.map(|(spk_index, spk)| {
	std::thread::spawn({
	let client = self.clone();
	move || -> Result<TxsOfSpkIndex, Error> {
	let mut last_seen = None;
	let mut spk_txs = Vec::new();
	loop {
	let txs = client.scripthash_txs(&spk, last_seen)?;
	let tx_count = txs.len();
	last_seen = txs.last().map(|tx| tx.txid);
	spk_txs.extend(txs);
	if tx_count < 25 {
	break Ok((spk_index, spk_txs));
	}
	}
	}
	})
	})
	.collect::<Vec<JoinHandle<Result<TxsOfSpkIndex, Error>>>>();
	if handles.is_empty() {
	break;
	}

and async

bdk/crates/esplora/src/async_ext.rs

Lines 225 to 249 in 59fc1b3

	loop {
	let handles = spks
	.by_ref()
	.take(parallel_requests)
	.map(|(spk_index, spk)| {
	let client = self.clone();
	async move {
	let mut last_seen = None;
	let mut spk_txs = Vec::new();
	loop {
	let txs = client.scripthash_txs(&spk, last_seen).await?;
	let tx_count = txs.len();
	last_seen = txs.last().map(|tx| tx.txid);
	spk_txs.extend(txs);
	if tx_count < 25 {
	break Result::<_, Error>::Ok((spk_index, spk_txs));
	}
	}
	}
	})
	.collect::<FuturesOrdered<_>>();
	if handles.is_empty() {
	break;
	}

[evanlinjin]

The specs for 429 HTTP code suggests including a Retry-After header. If esplora has this, we can use it, otherwise we can have a decaying delay.

[ismyhc]

Im seeing a 429 as well using the swift-bdk library.

Esplora(message: "Esplora client error: HttpResponse(429)")

I assume this is the same issue as above?

Im simply doing this:

        let esploraConfig = EsploraConfig(baseUrl: "https://blockstream.info/testnet/api", proxy: nil, concurrency: 1, stopGap: 10, timeout: nil)
        
        let blockchainConfig = BlockchainConfig.esplora(config: esploraConfig)
        
        do {
            
            let blockchain = try Blockchain(config: blockchainConfig)
            let desc = try Descriptor(descriptor: descriptor, network: Network.testnet)
            let wallet = try Wallet(descriptor: desc, changeDescriptor: nil, network: Network.testnet, databaseConfig: db)
            
            self.syncWallet = SyncWallet(db: db, blockchain: blockchain, wallet: wallet)

[realeinherjar]

Yes, especially when using blockstream....

[notmandatory]

Will bitcoindevkit/rust-esplora-client#58 make it easier to handle these in a more graceful way or do we need full parsing of the returned messages as proposed in bitcoindevkit/rust-esplora-client#47?

[realeinherjar]

I plan to work on this as well.
I like @evanlinjin's idea of:

Retry-After header <...> otherwise we can have a decaying delay.

[realeinherjar]

The specs for 429 HTTP code suggests including a Retry-After header. If esplora has this, we can use it, otherwise we can have a decaying delay.

No Retry-After header:

Error: HttpResponse { status: 429, message: "Test", headers: ["server", "date", "content-type", "content-length", "access-control-allow-origin", "via", "alt-svc"] }

Ignore the "Test".
It was a hack