Repository: wizardsardine/liana
Pull request #1282 (2024-09-06 14:30:39)

electrum: ping server before returning client
Ping server as an extra connectivity check.

jp1ac4: I haven't been able to reproduce the test failures on my local machine. Still investigating.

Patch:
diff --git a/gui/Cargo.lock b/gui/Cargo.lock
index 1d661828..48d3bfb0 100644
--- a/gui/Cargo.lock
+++ b/gui/Cargo.lock
@@ -199,9 +199,9 @@ dependencies = [
 
 [[package]]
 name = "async-hwi"
-version = "0.0.23"
+version = "0.0.22"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2f5e1703fc8cf217444fde8f3fdcf44382a1dec3afe5430620aa1b421c692b58"
+checksum = "5b3f27c4ed452a71a415e1198b2ef0b586ef270f3921b6ebbd558311d46806fd"
 dependencies = [
  "async-trait",
  "bitbox-api",
@@ -212,6 +212,8 @@ dependencies = [
  "ledger-apdu",
  "ledger-transport-hidapi",
  "ledger_bitcoin_client",
+ "prost 0.12.2",
+ "prost-derive 0.12.2",
  "regex",
  "reqwest",
  "serde",
@@ -220,6 +222,7 @@ dependencies = [
  "serialport",
  "tokio",
  "tokio-serial",
+ "zeroize",
 ]
 
 [[package]]
diff --git a/gui/Cargo.toml b/gui/Cargo.toml
index 588922ed..70e089d3 100644
--- a/gui/Cargo.toml
+++ b/gui/Cargo.toml
@@ -15,7 +15,7 @@ path = "src/main.rs"
 
 [dependencies]
 async-trait = "0.1"
-async-hwi = { version = "0.0.23" }
+async-hwi = { version = "0.0.22" }
 liana = { git = "https://github.com/wizardsardine/liana", branch = "master", default-features = false, features = ["nonblocking_shutdown"] }
 liana_ui = { path = "ui" }
 backtrace = "0.3"
diff --git a/gui/src/hw.rs b/gui/src/hw.rs
index 74bd3a25..0b3d32b6 100644
--- a/gui/src/hw.rs
+++ b/gui/src/hw.rs
@@ -198,10 +198,6 @@ impl HardwareWallets {
         self.aliases.insert(fg, new_alias);
     }
 
-    pub fn reset_watch_list(&mut self) {
-        self.list = Vec::new();
-    }
-
     pub fn load_aliases(&mut self, aliases: HashMap<Fingerprint, String>) {
         self.aliases = aliases;
     }
diff --git a/gui/src/installer/mod.rs b/gui/src/installer/mod.rs
index 43be9df3..6464bae7 100644
--- a/gui/src/installer/mod.rs
+++ b/gui/src/installer/mod.rs
@@ -66,7 +66,6 @@ pub struct Installer {
 
 impl Installer {
     fn previous(&mut self) -> Command<Message> {
-        self.hws.reset_watch_list();
         let network = self.network;
         if self.current > 0 {
             self.current -= 1;
@@ -197,7 +196,6 @@ impl Installer {
     }
 
     fn next(&mut self) -> Command<Message> {
-        self.hws.reset_watch_list();
         let current_step = self
             .steps
             .get_mut(self.current)
diff --git a/src/bitcoin/electrum/client.rs b/src/bitcoin/electrum/client.rs
index 1fd5a3d0..eec542f1 100644
--- a/src/bitcoin/electrum/client.rs
+++ b/src/bitcoin/electrum/client.rs
@@ -57,7 +57,9 @@ impl Client {
     pub fn new(electrum_config: &config::ElectrumConfig) -> Result<Self, Error> {
         // First use a dummy config to check connectivity (no retries, short timeout).
         let dummy_config = Config::builder().retry(0).timeout(Some(3)).build();
+        // Try to ping the server.
         bdk_electrum::electrum_client::Client::from_config(&electrum_config.addr, dummy_config)
+            .and_then(|dummy_client| dummy_client.ping())
             .map_err(Error::Server)?;
 
         // Now connection has been checked, create client with required retries and timeout.
diff --git a/src/bitcoin/electrum/mod.rs b/src/bitcoin/electrum/mod.rs
index 10c30193..8b9745c7 100644
--- a/src/bitcoin/electrum/mod.rs
+++ b/src/bitcoin/electrum/mod.rs
@@ -55,13 +55,12 @@ impl Electrum {
     pub fn new(
         client: client::Client,
         bdk_wallet: wallet::BdkWallet,
-        full_scan: bool,
     ) -> Result<Self, ElectrumError> {
         Ok(Self {
             client,
             bdk_wallet,
             sync_count: 0,
-            full_scan,
+            full_scan: false, // by default, only perform full scan if wallet's local chain has height 0
         })
     }
 
@@ -201,26 +200,23 @@ impl Electrum {
         let changeset = self.bdk_wallet.apply_connected_chain_update(chain_update);
 
         let mut changes_iter = changeset.into_iter();
-        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"),
-                )
+        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.");
+                        Some(self.bdk_wallet.find_block_at_or_before_height(height))
+                    };
+                }
+                Some((_, None)) => continue,
+                None => break None,
             }
-        } else {
-            None
         };
 
         // Unconfirmed transactions have their last seen as 0, so we override to the `sync_count`
diff --git a/src/bitcoin/electrum/wallet.rs b/src/bitcoin/electrum/wallet.rs
index cea08c9c..2e467f21 100644
--- a/src/bitcoin/electrum/wallet.rs
+++ b/src/bitcoin/electrum/wallet.rs
@@ -295,20 +295,17 @@ impl BdkWallet {
         })
     }
 
-    /// 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> {
+    /// 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 {
         for cp in self.local_chain.iter_checkpoints() {
-            if cp.height() < height {
-                return Some(BlockChainTip {
+            if cp.height() <= height {
+                return BlockChainTip {
                     height: height_i32_from_u32(cp.height()),
                     hash: cp.hash(),
-                });
+                };
             }
         }
-        None
+        unreachable!("There must be at least the genesis block.")
     }
 
     /// Apply an update to the local chain.
diff --git a/src/lib.rs b/src/lib.rs
index 484cd6cd..b43ca31b 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -361,8 +361,7 @@ fn setup_electrum(
         receive_index,
         change_index,
     );
-    let full_scan = db_conn.rescan_timestamp().is_some();
-    let electrum = Electrum::new(client, bdk_wallet, full_scan).map_err(StartupError::Electrum)?;
+    let electrum = Electrum::new(client, bdk_wallet).map_err(StartupError::Electrum)?;
     electrum
         .sanity_checks(&genesis_hash)
         .map_err(StartupError::Electrum)?;


Test patch:
diff --git a/tests/test_framework/electrs.py b/tests/test_framework/electrs.py
index ee8c6a51..2c32b88f 100644
--- a/tests/test_framework/electrs.py
+++ b/tests/test_framework/electrs.py
@@ -2,7 +2,7 @@ import logging
 import os
 
 from ephemeral_port_reserve import reserve
-from test_framework.utils import BitcoinBackend, TailableProc, ELECTRS_PATH
+from test_framework.utils import BitcoinBackend, TailableProc, ELECTRS_PATH, TIMEOUT
 
 
 class Electrs(BitcoinBackend):
@@ -54,6 +54,7 @@ class Electrs(BitcoinBackend):
 
     def start(self):
         TailableProc.start(self)
+        self.wait_for_log("auto-compactions enabled", timeout=TIMEOUT)
         logging.info("Electrs started")
 
     def startup(self):