bdk_electrum: Handle negative heights properly

crates/electrum/src/bdk_electrum_client.rs

@@ -276,7 +276,9 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
 
                 for tx_res in spk_history {
                     tx_update.txs.push(self.fetch_tx(tx_res.tx_hash)?);
-                    self.validate_merkle_for_anchor(tx_update, tx_res.tx_hash, tx_res.height)?;
+                    if let Ok(height) = tx_res.height.try_into() {
+                        self.validate_merkle_for_anchor(tx_update, tx_res.tx_hash, height)?;
+                    }
                 }
             }
         }
@@ -312,7 +314,9 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
                 if !has_residing && res.tx_hash == op_txid {
                     has_residing = true;
                     tx_update.txs.push(Arc::clone(&op_tx));
-                    self.validate_merkle_for_anchor(tx_update, res.tx_hash, res.height)?;
+                    if let Ok(height) = res.height.try_into() {
+                        self.validate_merkle_for_anchor(tx_update, res.tx_hash, height)?;
+                    }
                 }
 
                 if !has_spending && res.tx_hash != op_txid {
@@ -326,7 +330,9 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
                         continue;
                     }
                     tx_update.txs.push(Arc::clone(&res_tx));
-                    self.validate_merkle_for_anchor(tx_update, res.tx_hash, res.height)?;
+                    if let Ok(height) = res.height.try_into() {
+                        self.validate_merkle_for_anchor(tx_update, res.tx_hash, height)?;
+                    }
                 }
             }
         }
@@ -360,7 +366,9 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
                 .into_iter()
                 .find(|r| r.tx_hash == txid)
             {
-                self.validate_merkle_for_anchor(tx_update, txid, r.height)?;
+                if let Ok(height) = r.height.try_into() {
+                    self.validate_merkle_for_anchor(tx_update, txid, height)?;
+                }
             }
 
             tx_update.txs.push(tx);
@@ -374,11 +382,11 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
         &self,
         tx_update: &mut TxUpdate<ConfirmationBlockTime>,
         txid: Txid,
-        confirmation_height: i32,
+        confirmation_height: usize,
     ) -> Result<(), Error> {
         if let Ok(merkle_res) = self
             .inner
-            .transaction_get_merkle(&txid, confirmation_height as usize)
+            .transaction_get_merkle(&txid, confirmation_height)
         {
             let mut header = self.fetch_header(merkle_res.block_height as u32)?;
             let mut is_confirmed_tx = electrum_client::utils::validate_merkle_proof(

crates/electrum/tests/test_electrum.rs

@@ -5,9 +5,15 @@ use bdk_chain::{
     spk_txout::SpkTxOutIndex,
     Balance, ConfirmationBlockTime, IndexedTxGraph, Indexer, Merge, TxGraph,
 };
+use bdk_core::bitcoin::Network;
 use bdk_electrum::BdkElectrumClient;
-use bdk_testenv::{anyhow, bitcoincore_rpc::RpcApi, TestEnv};
+use bdk_testenv::{
+    anyhow,
+    bitcoincore_rpc::{json::CreateRawTransactionInput, RawTx, RpcApi},
+    TestEnv,
+};
 use core::time::Duration;
+use electrum_client::ElectrumApi;
 use std::collections::{BTreeSet, HashSet};
 use std::str::FromStr;
 
@@ -54,6 +60,63 @@ where
     Ok(update)
 }
 
+/// If an spk history contains a tx that spends another unconfirmed tx (chained mempool history),
+/// the Electrum API will return the tx with a negative height. This should succeed and not panic.
+#[test]
+pub fn chained_mempool_tx_sync() -> anyhow::Result<()> {
+    let env = TestEnv::new()?;
+    let rpc_client = env.rpc_client();
+    let electrum_client = electrum_client::Client::new(env.electrsd.electrum_url.as_str())?;
+
+    let tracked_addr = rpc_client
+        .get_new_address(None, None)?
+        .require_network(Network::Regtest)?;
+
+    env.mine_blocks(100, None)?;
+
+    // First unconfirmed tx.
+    env.send(&tracked_addr, Amount::from_btc(1.0)?)?;
+
+    // Create second unconfirmed tx that spends the first.
+    let utxo = rpc_client
+        .list_unspent(None, Some(0), None, Some(true), None)?
+        .into_iter()
+        .find(|utxo| utxo.script_pub_key == tracked_addr.script_pubkey())
+        .expect("must find the newly created utxo");
+    let tx_that_spends_unconfirmed = rpc_client.create_raw_transaction(
+        &[CreateRawTransactionInput {
+            txid: utxo.txid,
+            vout: utxo.vout,
+            sequence: None,
+        }],
+        &[(
+            tracked_addr.to_string(),
+            utxo.amount - Amount::from_sat(1000),
+        )]
+        .into(),
+        None,
+        None,
+    )?;
+    let signed_tx = rpc_client
+        .sign_raw_transaction_with_wallet(tx_that_spends_unconfirmed.raw_hex(), None, None)?
+        .transaction()?;
+    rpc_client.send_raw_transaction(signed_tx.raw_hex())?;
+
+    env.wait_until_electrum_sees_txid(signed_tx.compute_txid(), Duration::from_secs(5))?;
+
+    let spk_history = electrum_client.script_get_history(&tracked_addr.script_pubkey())?;
+    assert!(
+        spk_history.into_iter().any(|tx_res| tx_res.height < 0),
+        "must find tx with negative height"
+    );
+
+    let client = BdkElectrumClient::new(electrum_client);
+    let request = SyncRequest::builder().spks(core::iter::once(tracked_addr.script_pubkey()));
+    let _response = client.sync(request, 1, false)?;
+
+    Ok(())
+}
+
 #[test]
 pub fn test_update_tx_graph_without_keychain() -> anyhow::Result<()> {
     let env = TestEnv::new()?;