Merge #729: Make sure we're actuall caught up to tip before starting up

6dfa04f6c7f6e5649f44ccf1503d9ce6b75d7568 bitcoin: also log the number of verified blocks in the poller (Antoine Poinsot)
b84dbc668c5aeb2560ee2b595c46e42ee89b7f40 commands: don't return 100% sync progress until we're done syncing (Antoine Poinsot)
f687145c1e5865c2e9be9aa7643c64da6f1ee0ff bitcoin: make sure we are *completely* synced before starting up (Antoine Poinsot)
03fe06fd1f1d5c32b4630b40f253d49e3e44b55d descriptors: remove trailing debug trace (Antoine Poinsot)

Pull request description:

  Fixes #726. We actually do update our internal state before we are synced but we weren't waiting for the chain to be entirely synced.

ACKs for top commit:
  edouardparis:
    ACK 6dfa04f6c7f6e5649f44ccf1503d9ce6b75d7568

Tree-SHA512: 0c3a2494b1522d94cb2731e951bca9623fa02824426ab0a526e2a99b0c5433bb11059717ad1873eda778f175bafbbf636dedfaccfae88f217de0d5e677458fc1

src/bitcoin/d/mod.rs

@@ -748,14 +748,26 @@ impl BitcoinD {
         self.make_node_request("getblockchaininfo", &[])
     }
 
-    pub fn sync_progress(&self) -> f64 {
+    pub fn sync_progress(&self) -> SyncProgress {
         // TODO: don't harass lianad, be smarter like in revaultd.
-        roundup_progress(
-            self.block_chain_info()
-                .get("verificationprogress")
-                .and_then(Json::as_f64)
-                .expect("No valid 'verificationprogress' in getblockchaininfo response?"),
-        )
+        let chain_info = self.block_chain_info();
+        let percentage = chain_info
+            .get("verificationprogress")
+            .and_then(Json::as_f64)
+            .expect("No valid 'verificationprogress' in getblockchaininfo response?");
+        let headers = chain_info
+            .get("headers")
+            .and_then(Json::as_u64)
+            .expect("No valid 'verificationprogress' in getblockchaininfo response?");
+        let blocks = chain_info
+            .get("blocks")
+            .and_then(Json::as_u64)
+            .expect("No valid 'blocks' in getblockchaininfo response?");
+        SyncProgress {
+            percentage,
+            headers,
+            blocks,
+        }
     }
 
     pub fn chain_tip(&self) -> BlockChainTip {
@@ -1122,6 +1134,43 @@ impl BitcoinD {
     }
 }
 
+/// Information about the block chain verification progress.
+#[derive(Debug, Clone, Copy)]
+pub struct SyncProgress {
+    /// Chain verification progress as a percentage between 0 and 1.
+    percentage: f64,
+    /// Headers count for the best known tip.
+    pub headers: u64,
+    /// Number of blocks validated toward the best known tip.
+    pub blocks: u64,
+}
+
+impl SyncProgress {
+    pub fn new(percentage: f64, headers: u64, blocks: u64) -> Self {
+        Self {
+            percentage,
+            headers,
+            blocks,
+        }
+    }
+
+    /// Get the verification progress, roundup up to to three decimal places. This will not return
+    /// 1.0 (ie 100% verification progress) until the verification is complete.
+    pub fn rounded_up_progress(&self) -> f64 {
+        let progress = roundup_progress(self.percentage);
+        if progress == 1.0 && self.blocks != self.headers {
+            // Don't return a 100% progress until we are actually done syncing.
+            0.999
+        } else {
+            progress
+        }
+    }
+
+    pub fn is_complete(&self) -> bool {
+        self.rounded_up_progress() == 1.0
+    }
+}
+
 /// An entry in the 'listdescriptors' result.
 #[derive(Debug, Clone)]
 pub struct ListDescEntry {

src/bitcoin/mod.rs

@@ -9,6 +9,7 @@ use crate::{
     bitcoin::d::{BitcoindError, CachedTxGetter, LSBlockEntry},
     descriptors,
 };
+pub use d::SyncProgress;
 
 use std::{fmt, sync};
 
@@ -42,8 +43,9 @@ pub trait BitcoinInterface: Send {
     fn genesis_block(&self) -> BlockChainTip;
 
     /// Get the progress of the block chain synchronization.
-    /// Returns a percentage between 0 and 1.
-    fn sync_progress(&self) -> f64;
+    /// Returns a rounded up percentage between 0 and 1. Use the `is_synced` method to be sure the
+    /// backend is completely synced to the best known tip.
+    fn sync_progress(&self) -> SyncProgress;
 
     /// Get the best block info.
     fn chain_tip(&self) -> BlockChainTip;
@@ -117,7 +119,7 @@ impl BitcoinInterface for d::BitcoinD {
         BlockChainTip { hash, height }
     }
 
-    fn sync_progress(&self) -> f64 {
+    fn sync_progress(&self) -> SyncProgress {
         self.sync_progress()
     }
 
@@ -333,7 +335,7 @@ impl BitcoinInterface for sync::Arc<sync::Mutex<dyn BitcoinInterface + 'static>>
         self.lock().unwrap().genesis_block()
     }
 
-    fn sync_progress(&self) -> f64 {
+    fn sync_progress(&self) -> SyncProgress {
         self.lock().unwrap().sync_progress()
     }
 

src/bitcoin/poller/looper.rs

@@ -356,12 +356,14 @@ pub fn looper(
 
         // Don't poll until the Bitcoin backend is fully synced.
         if !synced {
-            let sync_progress = bit.sync_progress();
+            let progress = bit.sync_progress();
             log::info!(
-                "Block chain synchronization progress: {:.2}%",
-                sync_progress * 100.0
+                "Block chain synchronization progress: {:.2}% ({} blocks / {} headers)",
+                progress.rounded_up_progress() * 100.0,
+                progress.blocks,
+                progress.headers
             );
-            synced = sync_progress == 1.0;
+            synced = progress.is_complete();
             if !synced {
                 continue;
             }

src/commands/mod.rs

@@ -263,7 +263,7 @@ impl DaemonControl {
             version: VERSION.to_string(),
             network: self.config.bitcoin_config.network,
             block_height,
-            sync: self.bitcoin.sync_progress(),
+            sync: self.bitcoin.sync_progress().rounded_up_progress(),
             descriptors: GetInfoDescriptors {
                 main: self.config.main_descriptor.clone(),
             },

src/descriptors/mod.rs

@@ -830,7 +830,6 @@ mod tests {
         assert_eq!(signed_single_psbt.inputs[0].partial_sigs.len(), 1);
         assert_eq!(info.primary_path.threshold, 1);
         assert_eq!(info.primary_path.sigs_count, 1);
-        dbg!(&info.primary_path.signed_pubkeys);
         assert!(
             info.primary_path.signed_pubkeys.len() == 1
                 && info.primary_path.signed_pubkeys.contains_key(&prim_key_fg)

src/lib.rs

@@ -671,7 +671,7 @@ mod tests {
     // Send them a response to 'getblockchaininfo' saying we are far from being synced
     fn complete_sync_check(server: &net::TcpListener) {
         let net_resp = [
-            "HTTP/1.1 200\n\r\n{\"jsonrpc\":\"2.0\",\"id\":1,\"result\":{\"verificationprogress\":0.1}}\n".as_bytes(),
+            "HTTP/1.1 200\n\r\n{\"jsonrpc\":\"2.0\",\"id\":1,\"result\":{\"verificationprogress\":0.1,\"headers\":1000,\"blocks\":100}}\n".as_bytes(),
         ]
         .concat();
         let (mut stream, _) = server.accept().unwrap();

src/testutils.rs

@@ -1,5 +1,5 @@
 use crate::{
-    bitcoin::{BitcoinInterface, Block, BlockChainTip, UTxO},
+    bitcoin::{BitcoinInterface, Block, BlockChainTip, SyncProgress, UTxO},
     config::{BitcoinConfig, Config},
     database::{BlockInfo, Coin, CoinStatus, DatabaseConnection, DatabaseInterface, LabelItem},
     descriptors, DaemonHandle,
@@ -42,8 +42,8 @@ impl BitcoinInterface for DummyBitcoind {
         BlockChainTip { hash, height: 0 }
     }
 
-    fn sync_progress(&self) -> f64 {
-        1.0
+    fn sync_progress(&self) -> SyncProgress {
+        SyncProgress::new(1.0, 1_000, 1_000)
     }
 
     fn chain_tip(&self) -> BlockChainTip {