diff --git a/crates/electrum/Cargo.toml b/crates/electrum/Cargo.toml index bbada83477..1ea92dd15d 100644 --- a/crates/electrum/Cargo.toml +++ b/crates/electrum/Cargo.toml @@ -21,6 +21,7 @@ electrum-client = { version = "0.24.0", features = [ "proxy" ], default-features bdk_testenv = { path = "../testenv" } bdk_chain = { path = "../chain" } criterion = { version = "0.7" } +serde_json = "1" [features] default = ["use-rustls"] diff --git a/crates/electrum/src/bdk_electrum_client.rs b/crates/electrum/src/bdk_electrum_client.rs index a7c943150a..fba2b0229b 100644 --- a/crates/electrum/src/bdk_electrum_client.rs +++ b/crates/electrum/src/bdk_electrum_client.rs @@ -305,6 +305,7 @@ impl BdkElectrumClient { let spk_histories = self .inner .batch_script_get_history(spks.iter().map(|(_, s)| s.spk.as_script()))?; + check_batch_len("batch_script_get_history", spks.len(), spk_histories.len())?; for ((spk_index, spk), spk_history) in spks.into_iter().zip(spk_histories) { let beyond_revealed = last_revealed.is_none_or(|lr| spk_index > lr); @@ -377,6 +378,11 @@ impl BdkElectrumClient { let histories = self .inner .batch_script_get_history(unique_spks.iter().map(|spk| spk.as_script()))?; + check_batch_len( + "batch_script_get_history", + unique_spks.len(), + histories.len(), + )?; let mut spk_map = HashMap::new(); for (spk, history) in unique_spks.into_iter().zip(histories) { spk_map.insert(spk, history); @@ -495,6 +501,11 @@ impl BdkElectrumClient { let spk_histories = self .inner .batch_script_get_history(scripts.iter().map(|spk| spk.as_script()))?; + check_batch_len( + "batch_script_get_history", + scripts.len(), + spk_histories.len(), + )?; for (tx, spk_history) in txs.into_iter().zip(spk_histories) { if let Some(res) = spk_history.into_iter().find(|res| res.tx_hash == tx.0) { @@ -546,6 +557,7 @@ impl BdkElectrumClient { if !missing_heights.is_empty() { let headers = self.inner.batch_block_header(missing_heights.clone())?; + check_batch_len("batch_block_header", missing_heights.len(), headers.len())?; for (height, header) in missing_heights.into_iter().zip(headers) { height_to_hash.insert(height, header.block_hash()); cache.insert(height, header); @@ -569,6 +581,7 @@ impl BdkElectrumClient { // Fetch merkle proofs. let proofs = self.inner.batch_transaction_get_merkle(to_fetch.iter())?; + check_batch_len("batch_transaction_get_merkle", to_fetch.len(), proofs.len())?; // Validate each proof, retrying once for each stale header. for ((txid, height), proof) in to_fetch.into_iter().zip(proofs) { @@ -647,6 +660,17 @@ impl BdkElectrumClient { } } +/// Ensure that a batch response has exactly one entry per request, so that pairing them with +/// `zip` or indexing can never silently drop entries or panic. +fn check_batch_len(method: &str, expected: usize, actual: usize) -> Result<(), Error> { + if expected != actual { + return Err(Error::Message(format!( + "electrum server returned {actual} responses for {expected} {method} requests" + ))); + } + Ok(()) +} + /// Return a [`CheckPoint`] of the latest tip, that connects with `prev_tip`. The latest blocks are /// fetched to construct checkpoint updates with the proper [`BlockHash`] in case of re-org. fn fetch_tip_and_latest_blocks( diff --git a/crates/electrum/tests/test_short_batch.rs b/crates/electrum/tests/test_short_batch.rs new file mode 100644 index 0000000000..75948f9a0f --- /dev/null +++ b/crates/electrum/tests/test_short_batch.rs @@ -0,0 +1,254 @@ +use std::borrow::Borrow; + +use bdk_chain::bitcoin::{ + absolute, consensus, hashes::Hash, transaction, Amount, Script, ScriptBuf, Transaction, TxIn, + TxOut, Txid, WPubkeyHash, +}; +use bdk_chain::spk_client::SyncRequest; +use bdk_electrum::electrum_client::{ + Batch, ElectrumApi, Error, GetBalanceRes, GetHeadersRes, GetHistoryRes, GetMerkleRes, + ListUnspentRes, Param, RawHeaderNotification, ScriptStatus, ServerFeaturesRes, TxidFromPosRes, +}; +use bdk_electrum::BdkElectrumClient; + +/// Transport that answers every batch request, but returns one entry fewer than requested for +/// each batch method whose flag is set. +#[derive(Default)] +struct ShortBatchTransport { + txs: Vec<(Transaction, usize)>, + short_headers: bool, + short_history: bool, + short_merkle: bool, +} + +impl ElectrumApi for ShortBatchTransport { + fn batch_script_get_history<'s, I>(&self, scripts: I) -> Result>, Error> + where + I: IntoIterator + Clone, + I::Item: Borrow<&'s Script>, + { + let history = self + .txs + .iter() + .map(|(tx, height)| GetHistoryRes { + height: *height as i32, + tx_hash: tx.compute_txid(), + fee: None, + }) + .collect::>(); + let mut out: Vec<_> = scripts.into_iter().map(|_| history.clone()).collect(); + if self.short_history { + out.pop(); + } + Ok(out) + } + + fn transaction_get_raw(&self, txid: &Txid) -> Result, Error> { + let (tx, _) = self + .txs + .iter() + .find(|(tx, _)| tx.compute_txid() == *txid) + .unwrap(); + Ok(consensus::serialize(tx)) + } + + fn batch_block_header_raw(&self, heights: I) -> Result>, Error> + where + I: IntoIterator + Clone, + I::Item: Borrow, + { + let requested = heights.into_iter().count(); + let len = requested - usize::from(self.short_headers); + Ok(vec![vec![0u8; 80]; len]) + } + + fn block_header_raw(&self, _: usize) -> Result, Error> { + Ok(vec![0u8; 80]) + } + + fn batch_transaction_get_merkle( + &self, + txids_and_heights: I, + ) -> Result, Error> + where + I: IntoIterator + Clone, + I::Item: Borrow<(Txid, usize)>, + { + let mut out = txids_and_heights + .into_iter() + .map(|item| GetMerkleRes { + block_height: item.borrow().1, + pos: 0, + merkle: vec![], + }) + .collect::>(); + if self.short_merkle { + out.pop(); + } + Ok(out) + } + + // Not exercised by `sync`. + fn raw_call( + &self, + _: &str, + _: impl IntoIterator, + ) -> Result { + unimplemented!() + } + fn batch_call(&self, _: &Batch) -> Result, Error> { + unimplemented!() + } + fn block_headers_subscribe_raw(&self) -> Result { + unimplemented!() + } + fn block_headers_pop_raw(&self) -> Result, Error> { + unimplemented!() + } + fn block_headers(&self, _: usize, _: usize) -> Result { + unimplemented!() + } + fn estimate_fee(&self, _: usize) -> Result { + unimplemented!() + } + fn relay_fee(&self) -> Result { + unimplemented!() + } + fn script_subscribe(&self, _: &Script) -> Result, Error> { + unimplemented!() + } + fn batch_script_subscribe<'s, I>(&self, _: I) -> Result>, Error> + where + I: IntoIterator + Clone, + I::Item: Borrow<&'s Script>, + { + unimplemented!() + } + fn script_unsubscribe(&self, _: &Script) -> Result { + unimplemented!() + } + fn script_pop(&self, _: &Script) -> Result, Error> { + unimplemented!() + } + fn script_get_balance(&self, _: &Script) -> Result { + unimplemented!() + } + fn batch_script_get_balance<'s, I>(&self, _: I) -> Result, Error> + where + I: IntoIterator + Clone, + I::Item: Borrow<&'s Script>, + { + unimplemented!() + } + fn script_get_history(&self, _: &Script) -> Result, Error> { + unimplemented!() + } + fn script_list_unspent(&self, _: &Script) -> Result, Error> { + unimplemented!() + } + fn batch_script_list_unspent<'s, I>(&self, _: I) -> Result>, Error> + where + I: IntoIterator + Clone, + I::Item: Borrow<&'s Script>, + { + unimplemented!() + } + fn batch_transaction_get_raw<'t, I>(&self, _: I) -> Result>, Error> + where + I: IntoIterator + Clone, + I::Item: Borrow<&'t Txid>, + { + unimplemented!() + } + fn batch_estimate_fee(&self, _: I) -> Result, Error> + where + I: IntoIterator + Clone, + I::Item: Borrow, + { + unimplemented!() + } + fn transaction_broadcast_raw(&self, _: &[u8]) -> Result { + unimplemented!() + } + fn transaction_get_merkle(&self, _: &Txid, _: usize) -> Result { + unimplemented!() + } + fn txid_from_pos(&self, _: usize, _: usize) -> Result { + unimplemented!() + } + fn txid_from_pos_with_merkle(&self, _: usize, _: usize) -> Result { + unimplemented!() + } + fn server_features(&self) -> Result { + unimplemented!() + } + fn ping(&self) -> Result<(), Error> { + unimplemented!() + } +} + +fn test_spk(byte: u8) -> ScriptBuf { + ScriptBuf::new_p2wpkh(&WPubkeyHash::from_byte_array([byte; 20])) +} + +fn tx_paying(spk: &ScriptBuf, lock_time: u32) -> Transaction { + Transaction { + version: transaction::Version::TWO, + lock_time: absolute::LockTime::from_consensus(lock_time), + input: vec![TxIn::default()], + output: vec![TxOut { + value: Amount::from_sat(1_000), + script_pubkey: spk.clone(), + }], + } +} + +/// Two transactions confirmed at different heights, so that two headers and two merkle proofs +/// are requested in a single batch. +fn two_confirmed_txs(spk: &ScriptBuf) -> Vec<(Transaction, usize)> { + vec![(tx_paying(spk, 1), 10), (tx_paying(spk, 2), 20)] +} + +#[test] +fn short_header_batch_is_reported_as_an_error() { + let spk = test_spk(1); + let client = BdkElectrumClient::new(ShortBatchTransport { + txs: two_confirmed_txs(&spk), + short_headers: true, + ..Default::default() + }); + + let result = client.sync(SyncRequest::builder().spks([spk]), 10, false); + assert!(result.is_err(), "sync accepted a short header batch"); +} + +#[test] +fn short_merkle_batch_is_reported_as_an_error() { + let spk = test_spk(1); + let client = BdkElectrumClient::new(ShortBatchTransport { + txs: two_confirmed_txs(&spk), + short_merkle: true, + ..Default::default() + }); + + let result = client.sync(SyncRequest::builder().spks([spk]), 10, false); + assert!(result.is_err(), "sync accepted a short merkle proof batch"); +} + +#[test] +fn short_history_batch_is_reported_as_an_error() { + let spk_a = test_spk(1); + let spk_b = test_spk(2); + let expected_txid = Txid::from_byte_array([9; 32]); + let client = BdkElectrumClient::new(ShortBatchTransport { + txs: vec![(tx_paying(&spk_a, 1), 0)], + short_history: true, + ..Default::default() + }); + + let request = SyncRequest::builder() + .spks([spk_a, spk_b.clone()]) + .expected_spk_txids([(spk_b, expected_txid)]); + let result = client.sync(request, 10, false); + assert!(result.is_err(), "sync accepted a short history batch"); +}