From f3a136c30be0e443cebae85785f70999dfaf8485 Mon Sep 17 00:00:00 2001 From: Michael Mallan Date: Mon, 28 Oct 2024 12:05:44 +0000 Subject: [PATCH 1/4] gui(home): refactor sync status logic Use an enum to clearly enumerate the different cases we are considering and avoid the need to repeat logic relating to blockheight in the view. --- gui/src/app/state/mod.rs | 87 ++++++++++++++++++++++------------------ gui/src/app/view/home.rs | 34 ++++++++-------- gui/src/app/wallet.rs | 17 ++++++++ 3 files changed, 83 insertions(+), 55 deletions(-) diff --git a/gui/src/app/state/mod.rs b/gui/src/app/state/mod.rs index 0a78d24b..57841da1 100644 --- a/gui/src/app/state/mod.rs +++ b/gui/src/app/state/mod.rs @@ -19,7 +19,14 @@ use liana::{ }; use liana_ui::widget::*; -use super::{cache::Cache, error::Error, menu::Menu, message::Message, view, wallet::Wallet}; +use super::{ + cache::Cache, + error::Error, + menu::Menu, + message::Message, + view, + wallet::{SyncStatus, Wallet}, +}; pub const HISTORY_EVENT_PAGE_SIZE: u64 = 20; @@ -69,16 +76,13 @@ pub fn redirect(menu: Menu) -> Command { }) } -fn wallet_is_syncing( +fn sync_status( daemon_backend: DaemonBackend, blockheight: i32, last_poll: Option, last_poll_at_startup: Option, -) -> bool { - match daemon_backend { - // If remote, the wallet is always synced except before the first scan - // after creation. - DaemonBackend::RemoteBackend => blockheight <= 0, +) -> SyncStatus { + if blockheight <= 0 { // If blockheight <= 0, then this is a newly created wallet. // If user imported descriptor and is using a local bitcoind, a rescan // will need to be performed in order to see past transactions and so the @@ -86,30 +90,31 @@ fn wallet_is_syncing( // being performed. // For external daemon or if we otherwise don't know the node type, // treat it the same as bitcoind to be sure we don't mislead the user. - DaemonBackend::EmbeddedLianad(Some(NodeType::Bitcoind)) - | DaemonBackend::EmbeddedLianad(None) - | DaemonBackend::ExternalLianad - if blockheight <= 0 => + if daemon_backend == DaemonBackend::RemoteBackend + || daemon_backend == DaemonBackend::EmbeddedLianad(Some(NodeType::Electrum)) { - false + return SyncStatus::WalletFullScan; } - // If external daemon, we cannot be sure it will return last poll - // as it depends on the version, so assume it won't unless the - // last poll at startup is set. - // TODO: should we check the daemon version at GUI startup? - DaemonBackend::ExternalLianad if last_poll_at_startup.is_none() => false, - // For an existing wallet with any local node type, the first poll - // completing means the wallet has caught up with the tip. - // For a new wallet with a non-bitcoind local node, the first poll - // completing also means that the initial rescan has completed. - _ => last_poll <= last_poll_at_startup, } + // For an existing wallet with any local node type, if the first poll has + // not completed, then the wallet has not yet caught up with the tip. + // An existing wallet with remote backend remains synced so we can ignore it. + // If external daemon, we cannot be sure it will return last poll as it + // depends on the version, so assume it won't unless the last poll at + // startup is set. + // TODO: should we check the daemon version at GUI startup? + else if last_poll <= last_poll_at_startup + && (daemon_backend.is_embedded() + || (daemon_backend == DaemonBackend::ExternalLianad && last_poll_at_startup.is_some())) + { + return SyncStatus::LatestWalletSync; + } + SyncStatus::Synced } pub struct Home { wallet: Arc, - wallet_is_syncing: bool, - blockheight: i32, + sync_status: SyncStatus, last_poll_at_startup: Option, balance: Amount, unconfirmed_balance: Amount, @@ -145,14 +150,12 @@ impl Home { }, ); - let wallet_is_syncing = - wallet_is_syncing(daemon_backend, blockheight, last_poll, last_poll); + let sync_status = sync_status(daemon_backend, blockheight, last_poll, last_poll); Self { wallet, - wallet_is_syncing, + sync_status, last_poll_at_startup: last_poll, - blockheight, balance, unconfirmed_balance, remaining_sequence: None, @@ -167,10 +170,15 @@ impl Home { } } - fn wallet_is_syncing(&self, daemon_backend: DaemonBackend, last_poll: Option) -> bool { - wallet_is_syncing( + fn sync_status( + &self, + daemon_backend: DaemonBackend, + blockheight: i32, + last_poll: Option, + ) -> SyncStatus { + sync_status( daemon_backend, - self.blockheight, + blockheight, last_poll, self.last_poll_at_startup, ) @@ -206,8 +214,7 @@ impl State for Home { &self.events, self.is_last_page, self.processing, - self.wallet_is_syncing, - self.blockheight, + &self.sync_status, ), ) } @@ -285,12 +292,14 @@ impl State for Home { } }, Message::UpdatePanelCache(is_current, Ok(cache)) => { - let wallet_was_syncing = self.wallet_is_syncing; - self.blockheight = cache.blockheight; - self.wallet_is_syncing = - self.wallet_is_syncing(daemon.backend(), cache.last_poll_timestamp); + let wallet_was_syncing = !self.sync_status.is_synced(); + self.sync_status = self.sync_status( + daemon.backend(), + cache.blockheight, + cache.last_poll_timestamp, + ); // If this is the current panel, reload it if wallet is no longer syncing. - if is_current && wallet_was_syncing && !self.wallet_is_syncing { + if is_current && wallet_was_syncing && self.sync_status.is_synced() { return self.reload(daemon, self.wallet.clone()); } } @@ -371,7 +380,7 @@ impl State for Home { wallet: Arc, ) -> Command { // Wait for wallet to finish syncing before reloading data. - if self.wallet_is_syncing { + if !self.sync_status.is_synced() { return Command::none(); } self.selected_event = None; diff --git a/gui/src/app/view/home.rs b/gui/src/app/view/home.rs index 731d04d2..6da3bce2 100644 --- a/gui/src/app/view/home.rs +++ b/gui/src/app/view/home.rs @@ -21,6 +21,7 @@ use crate::{ error::Error, menu::Menu, view::{coins, dashboard, label, message::Message}, + wallet::SyncStatus, }, daemon::model::{HistoryTransaction, TransactionKind}, }; @@ -35,14 +36,13 @@ pub fn home_view<'a>( events: &'a [HistoryTransaction], is_last_page: bool, processing: bool, - wallet_is_syncing: bool, - blockheight: i32, + sync_status: &SyncStatus, ) -> Element<'a, Message> { Column::new() .push(h3("Balance")) .push( Column::new() - .push(if !wallet_is_syncing { + .push(if sync_status.is_synced() { amount_with_size(balance, H1_SIZE) } else { Row::new().push(spinner::Carousel::new( @@ -58,11 +58,11 @@ pub fn home_view<'a>( ], )) }) - .push_maybe(if wallet_is_syncing { + .push_maybe(if !sync_status.is_synced() { Some( Row::new() .push( - text(if blockheight <= 0 { + text(if *sync_status == SyncStatus::WalletFullScan { "Syncing" } else { "Checking for new transactions" @@ -79,17 +79,19 @@ pub fn home_view<'a>( } else { None }) - .push_maybe(if unconfirmed_balance.to_sat() != 0 && !wallet_is_syncing { - Some( - Row::new() - .spacing(10) - .push(text("+").size(H3_SIZE).style(color::GREY_3)) - .push(unconfirmed_amount_with_size(unconfirmed_balance, H3_SIZE)) - .push(text("unconfirmed").size(H3_SIZE).style(color::GREY_3)), - ) - } else { - None - }), + .push_maybe( + if unconfirmed_balance.to_sat() != 0 && sync_status.is_synced() { + Some( + Row::new() + .spacing(10) + .push(text("+").size(H3_SIZE).style(color::GREY_3)) + .push(unconfirmed_amount_with_size(unconfirmed_balance, H3_SIZE)) + .push(text("unconfirmed").size(H3_SIZE).style(color::GREY_3)), + ) + } else { + None + }, + ), ) .push_maybe(if expiring_coins.is_empty() { remaining_sequence.map(|sequence| { diff --git a/gui/src/app/wallet.rs b/gui/src/app/wallet.rs index bea37a16..9dfade00 100644 --- a/gui/src/app/wallet.rs +++ b/gui/src/app/wallet.rs @@ -187,3 +187,20 @@ impl From for WalletError { WalletError::Settings(error) } } + +/// The sync status of a wallet with respect to the blockchain. +#[derive(Debug, Clone, PartialEq)] +pub enum SyncStatus { + /// Wallet and blockchain are fully synced. + Synced, + /// Wallet is performing a full scan of the blockchain. + WalletFullScan, + /// Wallet is syncing with latest transactions. + LatestWalletSync, +} + +impl SyncStatus { + pub fn is_synced(&self) -> bool { + self == &SyncStatus::Synced + } +} From 62788d105c2fcdb1e6c400ca6f5f19138378ae0b Mon Sep 17 00:00:00 2001 From: Michael Mallan Date: Mon, 28 Oct 2024 10:19:11 +0000 Subject: [PATCH 2/4] gui(cache): include sync progress --- gui/src/app/cache.rs | 2 ++ gui/src/app/mod.rs | 1 + gui/src/loader.rs | 1 + gui/src/main.rs | 1 + 4 files changed, 5 insertions(+) diff --git a/gui/src/app/cache.rs b/gui/src/app/cache.rs index 1ab235a4..b4c54e30 100644 --- a/gui/src/app/cache.rs +++ b/gui/src/app/cache.rs @@ -9,6 +9,7 @@ pub struct Cache { pub blockheight: i32, pub coins: Vec, pub rescan_progress: Option, + pub sync_progress: f64, pub last_poll_timestamp: Option, } @@ -21,6 +22,7 @@ impl std::default::Default for Cache { blockheight: 0, coins: Vec::new(), rescan_progress: None, + sync_progress: 1.0, last_poll_timestamp: None, } } diff --git a/gui/src/app/mod.rs b/gui/src/app/mod.rs index 92386340..aaf5e8f1 100644 --- a/gui/src/app/mod.rs +++ b/gui/src/app/mod.rs @@ -282,6 +282,7 @@ impl App { network: info.network, blockheight: info.block_height, rescan_progress: info.rescan_progress, + sync_progress: info.sync, last_poll_timestamp: info.last_poll_timestamp, }) }, diff --git a/gui/src/loader.rs b/gui/src/loader.rs index a7de1c19..db667176 100644 --- a/gui/src/loader.rs +++ b/gui/src/loader.rs @@ -380,6 +380,7 @@ pub async fn load_application( network: info.network, blockheight: info.block_height, coins, + sync_progress: info.sync, last_poll_timestamp: info.last_poll_timestamp, ..Default::default() }; diff --git a/gui/src/main.rs b/gui/src/main.rs index 7aac5a0a..74b6cc85 100644 --- a/gui/src/main.rs +++ b/gui/src/main.rs @@ -459,6 +459,7 @@ pub fn create_app_with_remote_backend( network, coins: Vec::new(), rescan_progress: None, + sync_progress: 1.0, // Remote backend is always synced datadir_path: datadir.clone(), blockheight: wallet.tip_height.unwrap_or(0), last_poll_timestamp: None, // We ignore this field for remote backend. From 1408f66963cf6ae5f2fe2ac29d2cd91f747477bc Mon Sep 17 00:00:00 2001 From: Michael Mallan Date: Mon, 28 Oct 2024 12:21:00 +0000 Subject: [PATCH 3/4] gui(home): indicate if blockchain is syncing --- gui/src/app/mod.rs | 1 + gui/src/app/state/mod.rs | 17 +++++++++++++++-- gui/src/app/view/home.rs | 13 ++++++++----- gui/src/app/wallet.rs | 2 ++ 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/gui/src/app/mod.rs b/gui/src/app/mod.rs index aaf5e8f1..bf0c4bf7 100644 --- a/gui/src/app/mod.rs +++ b/gui/src/app/mod.rs @@ -67,6 +67,7 @@ impl Panels { wallet.clone(), &cache.coins, cache.blockheight, + cache.sync_progress, cache.last_poll_timestamp, daemon_backend.clone(), ), diff --git a/gui/src/app/state/mod.rs b/gui/src/app/state/mod.rs index 57841da1..00b1489e 100644 --- a/gui/src/app/state/mod.rs +++ b/gui/src/app/state/mod.rs @@ -79,10 +79,13 @@ pub fn redirect(menu: Menu) -> Command { fn sync_status( daemon_backend: DaemonBackend, blockheight: i32, + sync_progress: f64, last_poll: Option, last_poll_at_startup: Option, ) -> SyncStatus { - if blockheight <= 0 { + if sync_progress < 1.0 { + return SyncStatus::BlockchainSync(sync_progress); + } else if blockheight <= 0 { // If blockheight <= 0, then this is a newly created wallet. // If user imported descriptor and is using a local bitcoind, a rescan // will need to be performed in order to see past transactions and so the @@ -134,6 +137,7 @@ impl Home { wallet: Arc, coins: &[Coin], blockheight: i32, + sync_progress: f64, last_poll: Option, daemon_backend: DaemonBackend, ) -> Self { @@ -150,7 +154,13 @@ impl Home { }, ); - let sync_status = sync_status(daemon_backend, blockheight, last_poll, last_poll); + let sync_status = sync_status( + daemon_backend, + blockheight, + sync_progress, + last_poll, + last_poll, + ); Self { wallet, @@ -174,11 +184,13 @@ impl Home { &self, daemon_backend: DaemonBackend, blockheight: i32, + sync_progress: f64, last_poll: Option, ) -> SyncStatus { sync_status( daemon_backend, blockheight, + sync_progress, last_poll, self.last_poll_at_startup, ) @@ -296,6 +308,7 @@ impl State for Home { self.sync_status = self.sync_status( daemon.backend(), cache.blockheight, + cache.sync_progress, cache.last_poll_timestamp, ); // If this is the current panel, reload it if wallet is no longer syncing. diff --git a/gui/src/app/view/home.rs b/gui/src/app/view/home.rs index 6da3bce2..c4bcdead 100644 --- a/gui/src/app/view/home.rs +++ b/gui/src/app/view/home.rs @@ -62,11 +62,14 @@ pub fn home_view<'a>( Some( Row::new() .push( - text(if *sync_status == SyncStatus::WalletFullScan { - "Syncing" - } else { - "Checking for new transactions" - }) + match sync_status { + SyncStatus::BlockchainSync(progress) => text(format!( + "Syncing blockchain ({:.1}%)", + 100.0 * *progress + )), + SyncStatus::WalletFullScan => text("Syncing"), + _ => text("Checking for new transactions"), + } .style(color::GREY_2), ) .push(spinner::typing_text_carousel( diff --git a/gui/src/app/wallet.rs b/gui/src/app/wallet.rs index 9dfade00..506e3984 100644 --- a/gui/src/app/wallet.rs +++ b/gui/src/app/wallet.rs @@ -197,6 +197,8 @@ pub enum SyncStatus { WalletFullScan, /// Wallet is syncing with latest transactions. LatestWalletSync, + /// Blockchain is syncing with given progress between 0.0 and 1.0. + BlockchainSync(f64), } impl SyncStatus { From 0a674d591affd6190d5b4979718fb09d3d8f79f3 Mon Sep 17 00:00:00 2001 From: Michael Mallan Date: Thu, 17 Oct 2024 11:29:07 +0100 Subject: [PATCH 4/4] gui: load app directly if wallet was previously synced --- gui/src/loader.rs | 81 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 55 insertions(+), 26 deletions(-) diff --git a/gui/src/loader.rs b/gui/src/loader.rs index db667176..0f9b679a 100644 --- a/gui/src/loader.rs +++ b/gui/src/loader.rs @@ -39,6 +39,14 @@ const SYNCING_PROGRESS_2: &str = "Bitcoin Core is synchronising the blockchain. const SYNCING_PROGRESS_3: &str = "Bitcoin Core is synchronising the blockchain. This may take a few minutes, depending on the last time it was done, your internet connection, and your computer performance."; type Lianad = client::Lianad; +type StartedResult = Result< + ( + Arc, + Option, + GetInfoResult, + ), + Error, +>; pub struct Loader { pub datadir_path: PathBuf, @@ -78,8 +86,8 @@ pub enum Message { Error, >, ), - Started(Result<(Arc, Option), Error>), - Loaded(Result, Error>), + Started(StartedResult), + Loaded(Result<(Arc, GetInfoResult), Error>), BitcoindLog(Option), Failure(DaemonError), None, @@ -110,18 +118,44 @@ impl Loader { ) } - fn on_load(&mut self, res: Result, Error>) -> Command { + fn maybe_skip_syncing( + &mut self, + daemon: Arc, + info: GetInfoResult, + ) -> Command { + // If the wallet was previously synced (blockheight > 0), load the + // application directly. + if info.block_height > 0 { + return Command::perform( + load_application( + daemon, + info, + self.datadir_path.clone(), + self.network, + self.internal_bitcoind.clone(), + ), + Message::Synced, + ); + } + // Otherwise, show the sync progress on the loading screen. + self.step = Step::Syncing { + daemon: daemon.clone(), + progress: 0.0, + bitcoind_logs: String::new(), + }; + Command::perform(sync(daemon, false), Message::Syncing) + } + + fn on_load( + &mut self, + res: Result<(Arc, GetInfoResult), Error>, + ) -> Command { match res { - Ok(daemon) => { - self.step = Step::Syncing { - daemon: daemon.clone(), - progress: 0.0, - bitcoind_logs: String::new(), - }; + Ok((daemon, info)) => { if self.gui_config.start_internal_bitcoind { warn!("Lianad is external, gui will not start internal bitcoind"); } - return Command::perform(sync(daemon, false), Message::Syncing); + return self.maybe_skip_syncing(daemon, info); } Err(e) => match e { Error::Config(_) => { @@ -164,24 +198,16 @@ impl Loader { Command::none() } - fn on_start( - &mut self, - res: Result<(Arc, Option), Error>, - ) -> Command { + fn on_start(&mut self, res: StartedResult) -> Command { match res { - Ok((daemon, bitcoind)) => { + Ok((daemon, bitcoind, info)) => { // bitcoind may have been already started and given to the loader // We should not override with None the loader bitcoind field if let Some(bitcoind) = bitcoind { self.internal_bitcoind = Some(bitcoind); } self.waiting_daemon_bitcoind = false; - self.step = Step::Syncing { - daemon: daemon.clone(), - progress: 0.0, - bitcoind_logs: String::new(), - }; - Command::perform(sync(daemon, false), Message::Syncing) + self.maybe_skip_syncing(daemon, info) } Err(e) => { self.step = Step::Error(Box::new(e)); @@ -481,15 +507,17 @@ pub fn cover<'a, T: 'a + Clone, C: Into>>( .into() } -async fn connect(socket_path: PathBuf) -> Result, Error> { +async fn connect( + socket_path: PathBuf, +) -> Result<(Arc, GetInfoResult), Error> { let client = client::jsonrpc::JsonRPCClient::new(socket_path); let daemon = Lianad::new(client); debug!("Searching for external daemon"); - daemon.get_info().await?; + let info = daemon.get_info().await?; info!("Connected to external daemon"); - Ok(Arc::new(daemon)) + Ok((Arc::new(daemon), info)) } // Daemon can start only if a config path is given. @@ -497,7 +525,7 @@ pub async fn start_bitcoind_and_daemon( config_path: PathBuf, liana_datadir_path: PathBuf, start_internal_bitcoind: bool, -) -> Result<(Arc, Option), Error> { +) -> StartedResult { let config = Config::from_file(Some(config_path)).map_err(Error::Config)?; let mut bitcoind: Option = None; if start_internal_bitcoind { @@ -523,8 +551,9 @@ pub async fn start_bitcoind_and_daemon( debug!("starting liana daemon"); let daemon = EmbeddedDaemon::start(config)?; + let info = daemon.get_info().await?; - Ok((Arc::new(daemon), bitcoind)) + Ok((Arc::new(daemon), bitcoind, info)) } async fn sync(