From 86965c188e29191b60da51ce7d998157e1d50f77 Mon Sep 17 00:00:00 2001
From: Michael Mallan <michaelmallan@proton.me>
Date: Sun, 8 Sep 2024 17:12:36 +0100
Subject: [PATCH 1/2] electrum: common ancestor is before first changed block

---
 src/bitcoin/electrum/mod.rs    |  7 ++++++-
 src/bitcoin/electrum/wallet.rs | 15 +++++++++------
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/src/bitcoin/electrum/mod.rs b/src/bitcoin/electrum/mod.rs
index af14c586..5f72ceb8 100644
--- a/src/bitcoin/electrum/mod.rs
+++ b/src/bitcoin/electrum/mod.rs
@@ -212,7 +212,12 @@ impl Electrum {
                         None
                     } else {
                         log::info!("Block chain reorganization detected.");
-                        Some(self.bdk_wallet.find_block_at_or_before_height(height))
+                        // We can assume height is positive as genesis block will not have changed.
+                        Some(
+                            self.bdk_wallet
+                                .find_block_before_height(height)
+                                .expect("height of first change is greater than 0"),
+                        )
                     };
                 }
                 Some((_, None)) => continue,
diff --git a/src/bitcoin/electrum/wallet.rs b/src/bitcoin/electrum/wallet.rs
index 2e467f21..cea08c9c 100644
--- a/src/bitcoin/electrum/wallet.rs
+++ b/src/bitcoin/electrum/wallet.rs
@@ -295,17 +295,20 @@ impl BdkWallet {
         })
     }
 
-    /// Find the first block in the local chain whose height is less than or equal to this.
-    pub fn find_block_at_or_before_height(&self, height: u32) -> BlockChainTip {
+    /// Find the highest block in the local chain whose height is below `height`.
+    ///
+    /// As the local chain will always contain the genesis block, this returns
+    /// `None` only if `height` is 0.
+    pub fn find_block_before_height(&self, height: u32) -> Option<BlockChainTip> {
         for cp in self.local_chain.iter_checkpoints() {
-            if cp.height() <= height {
-                return BlockChainTip {
+            if cp.height() < height {
+                return Some(BlockChainTip {
                     height: height_i32_from_u32(cp.height()),
                     hash: cp.hash(),
-                };
+                });
             }
         }
-        unreachable!("There must be at least the genesis block.")
+        None
     }
 
     /// Apply an update to the local chain.

From d551aeeeacf5356a0ba25f4929230dd099b01c32 Mon Sep 17 00:00:00 2001
From: Michael Mallan <michaelmallan@proton.me>
Date: Sun, 8 Sep 2024 17:43:30 +0100
Subject: [PATCH 2/2] electrum: consider invalidated blocks for reorgs

---
 src/bitcoin/electrum/mod.rs | 40 ++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/src/bitcoin/electrum/mod.rs b/src/bitcoin/electrum/mod.rs
index 5f72ceb8..10c30193 100644
--- a/src/bitcoin/electrum/mod.rs
+++ b/src/bitcoin/electrum/mod.rs
@@ -201,28 +201,26 @@ impl Electrum {
         let changeset = self.bdk_wallet.apply_connected_chain_update(chain_update);
 
         let mut changes_iter = changeset.into_iter();
-        let reorg_common_ancestor = loop {
-            match changes_iter.next() {
-                Some((height, Some(_))) => {
-                    // `BlockHash` being `Some(_)` means a checkpoint at this height was added to the chain.
-                    // Since we iterate in ascending height order, we'll see the lowest block height first.
-                    // If the lowest height it adds is higher than our height before syncing, we're good.
-                    // Else if it's adding a block at height before syncing or lower, it's a reorg.
-                    break if height > local_chain_tip.height() {
-                        None
-                    } else {
-                        log::info!("Block chain reorganization detected.");
-                        // We can assume height is positive as genesis block will not have changed.
-                        Some(
-                            self.bdk_wallet
-                                .find_block_before_height(height)
-                                .expect("height of first change is greater than 0"),
-                        )
-                    };
-                }
-                Some((_, None)) => continue,
-                None => break None,
+        let reorg_common_ancestor = if let Some((height, _)) = changes_iter.next() {
+            // Either a new block has been added at this height or an existing block in our local
+            // chain has been invalidated.
+            // Since we iterate in ascending height order, we'll see the lowest block height first.
+            // If the lowest height is higher than our height before syncing, we're good.
+            // Else if it's adding/invalidating a block at height before syncing or lower,
+            // it's a reorg.
+            if height > local_chain_tip.height() {
+                None
+            } else {
+                log::info!("Block chain reorganization detected.");
+                // We can assume height is positive as genesis block will not have changed.
+                Some(
+                    self.bdk_wallet
+                        .find_block_before_height(height)
+                        .expect("height of first change is greater than 0"),
+                )
             }
+        } else {
+            None
         };
 
         // Unconfirmed transactions have their last seen as 0, so we override to the `sync_count`