From 03fe06fd1f1d5c32b4630b40f253d49e3e44b55d Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 18 Oct 2023 19:10:10 +0200 Subject: [PATCH 1/4] descriptors: remove trailing debug trace --- src/descriptors/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/descriptors/mod.rs b/src/descriptors/mod.rs index 574b153c..2426f259 100644 --- a/src/descriptors/mod.rs +++ b/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) From f687145c1e5865c2e9be9aa7643c64da6f1ee0ff Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 18 Oct 2023 19:04:23 +0200 Subject: [PATCH 2/4] bitcoin: make sure we are *completely* synced before starting up When not in IBD but catching up to the latest tip, our rounding up of verificationprogress makes us start while bitcoind is still kind of far from being caught up. Make sure it doesn't happen by not returning until bitcoind validated all the blocks we've fetched the headers for. --- src/bitcoin/d/mod.rs | 33 +++++++++++++++++++++++++++++---- src/bitcoin/mod.rs | 10 ++++++---- src/bitcoin/poller/looper.rs | 12 ++++++++---- src/commands/mod.rs | 2 +- src/lib.rs | 2 +- src/testutils.rs | 10 +++++++--- 6 files changed, 52 insertions(+), 17 deletions(-) diff --git a/src/bitcoin/d/mod.rs b/src/bitcoin/d/mod.rs index 74f3ee56..d8a8af51 100644 --- a/src/bitcoin/d/mod.rs +++ b/src/bitcoin/d/mod.rs @@ -748,14 +748,28 @@ 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() + let chain_info = self.block_chain_info(); + let percentage = roundup_progress( + 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 +1136,17 @@ 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. + pub percentage: f64, + /// Headers count for the best known tip. + pub headers: u64, + /// Number of blocks validated toward the best known tip. + pub blocks: u64, +} + /// An entry in the 'listdescriptors' result. #[derive(Debug, Clone)] pub struct ListDescEntry { diff --git a/src/bitcoin/mod.rs b/src/bitcoin/mod.rs index b1d32fae..86bf5ccd 100644 --- a/src/bitcoin/mod.rs +++ b/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> self.lock().unwrap().genesis_block() } - fn sync_progress(&self) -> f64 { + fn sync_progress(&self) -> SyncProgress { self.lock().unwrap().sync_progress() } diff --git a/src/bitcoin/poller/looper.rs b/src/bitcoin/poller/looper.rs index b15684b3..e01bffde 100644 --- a/src/bitcoin/poller/looper.rs +++ b/src/bitcoin/poller/looper.rs @@ -1,5 +1,5 @@ use crate::{ - bitcoin::{BitcoinInterface, BlockChainTip, UTxO}, + bitcoin::{BitcoinInterface, BlockChainTip, SyncProgress, UTxO}, database::{Coin, DatabaseConnection, DatabaseInterface}, descriptors, }; @@ -356,12 +356,16 @@ pub fn looper( // Don't poll until the Bitcoin backend is fully synced. if !synced { - let sync_progress = bit.sync_progress(); + let SyncProgress { + percentage, + headers, + blocks, + } = bit.sync_progress(); log::info!( "Block chain synchronization progress: {:.2}%", - sync_progress * 100.0 + percentage * 100.0 ); - synced = sync_progress == 1.0; + synced = headers == blocks; if !synced { continue; } diff --git a/src/commands/mod.rs b/src/commands/mod.rs index fb2c2b22..6f48c350 100644 --- a/src/commands/mod.rs +++ b/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().percentage, descriptors: GetInfoDescriptors { main: self.config.main_descriptor.clone(), }, diff --git a/src/lib.rs b/src/lib.rs index 37a68284..5f58e04f 100644 --- a/src/lib.rs +++ b/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(); diff --git a/src/testutils.rs b/src/testutils.rs index 3c7e3ea6..0d795637 100644 --- a/src/testutils.rs +++ b/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,12 @@ impl BitcoinInterface for DummyBitcoind { BlockChainTip { hash, height: 0 } } - fn sync_progress(&self) -> f64 { - 1.0 + fn sync_progress(&self) -> SyncProgress { + SyncProgress { + percentage: 1.0, + headers: 1_000, + blocks: 1_000, + } } fn chain_tip(&self) -> BlockChainTip { From b84dbc668c5aeb2560ee2b595c46e42ee89b7f40 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 19 Oct 2023 10:09:35 +0200 Subject: [PATCH 3/4] commands: don't return 100% sync progress until we're done syncing --- src/bitcoin/d/mod.rs | 38 +++++++++++++++++++++++++++++------- src/bitcoin/poller/looper.rs | 12 ++++-------- src/commands/mod.rs | 2 +- src/testutils.rs | 6 +----- 4 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/bitcoin/d/mod.rs b/src/bitcoin/d/mod.rs index d8a8af51..539ff308 100644 --- a/src/bitcoin/d/mod.rs +++ b/src/bitcoin/d/mod.rs @@ -751,12 +751,10 @@ impl BitcoinD { pub fn sync_progress(&self) -> SyncProgress { // TODO: don't harass lianad, be smarter like in revaultd. let chain_info = self.block_chain_info(); - let percentage = roundup_progress( - chain_info - .get("verificationprogress") - .and_then(Json::as_f64) - .expect("No valid 'verificationprogress' in getblockchaininfo response?"), - ); + 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) @@ -1140,13 +1138,39 @@ impl BitcoinD { #[derive(Debug, Clone, Copy)] pub struct SyncProgress { /// Chain verification progress as a percentage between 0 and 1. - pub percentage: f64, + 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 { diff --git a/src/bitcoin/poller/looper.rs b/src/bitcoin/poller/looper.rs index e01bffde..9f3fe4b2 100644 --- a/src/bitcoin/poller/looper.rs +++ b/src/bitcoin/poller/looper.rs @@ -1,5 +1,5 @@ use crate::{ - bitcoin::{BitcoinInterface, BlockChainTip, SyncProgress, UTxO}, + bitcoin::{BitcoinInterface, BlockChainTip, UTxO}, database::{Coin, DatabaseConnection, DatabaseInterface}, descriptors, }; @@ -356,16 +356,12 @@ pub fn looper( // Don't poll until the Bitcoin backend is fully synced. if !synced { - let SyncProgress { - percentage, - headers, - blocks, - } = bit.sync_progress(); + let progress = bit.sync_progress(); log::info!( "Block chain synchronization progress: {:.2}%", - percentage * 100.0 + progress.rounded_up_progress() * 100.0 ); - synced = headers == blocks; + synced = progress.is_complete(); if !synced { continue; } diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 6f48c350..0bdf09e8 100644 --- a/src/commands/mod.rs +++ b/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().percentage, + sync: self.bitcoin.sync_progress().rounded_up_progress(), descriptors: GetInfoDescriptors { main: self.config.main_descriptor.clone(), }, diff --git a/src/testutils.rs b/src/testutils.rs index 0d795637..c9b44f13 100644 --- a/src/testutils.rs +++ b/src/testutils.rs @@ -43,11 +43,7 @@ impl BitcoinInterface for DummyBitcoind { } fn sync_progress(&self) -> SyncProgress { - SyncProgress { - percentage: 1.0, - headers: 1_000, - blocks: 1_000, - } + SyncProgress::new(1.0, 1_000, 1_000) } fn chain_tip(&self) -> BlockChainTip { From 6dfa04f6c7f6e5649f44ccf1503d9ce6b75d7568 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 19 Oct 2023 10:11:12 +0200 Subject: [PATCH 4/4] bitcoin: also log the number of verified blocks in the poller --- src/bitcoin/poller/looper.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/bitcoin/poller/looper.rs b/src/bitcoin/poller/looper.rs index 9f3fe4b2..0d64eabe 100644 --- a/src/bitcoin/poller/looper.rs +++ b/src/bitcoin/poller/looper.rs @@ -358,8 +358,10 @@ pub fn looper( if !synced { let progress = bit.sync_progress(); log::info!( - "Block chain synchronization progress: {:.2}%", - progress.rounded_up_progress() * 100.0 + "Block chain synchronization progress: {:.2}% ({} blocks / {} headers)", + progress.rounded_up_progress() * 100.0, + progress.blocks, + progress.headers ); synced = progress.is_complete(); if !synced {