From e419784e9f2da76e8e86c03c8773f0dcb2a883da Mon Sep 17 00:00:00 2001 From: Michael Mallan Date: Fri, 1 Nov 2024 09:08:46 +0000 Subject: [PATCH 1/5] gui: move sync status function to wallet module This will make it easier to reuse elsewhere. --- gui/src/app/state/mod.rs | 50 ++++----------------------------------- gui/src/app/wallet.rs | 51 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 47 deletions(-) diff --git a/gui/src/app/state/mod.rs b/gui/src/app/state/mod.rs index 00b1489e..6694b248 100644 --- a/gui/src/app/state/mod.rs +++ b/gui/src/app/state/mod.rs @@ -25,17 +25,14 @@ use super::{ menu::Menu, message::Message, view, - wallet::{SyncStatus, Wallet}, + wallet::{sync_status, SyncStatus, Wallet}, }; pub const HISTORY_EVENT_PAGE_SIZE: u64 = 20; -use crate::{ - daemon::{ - model::{remaining_sequence, Coin, HistoryTransaction, Labelled}, - Daemon, DaemonBackend, - }, - node::NodeType, +use crate::daemon::{ + model::{remaining_sequence, Coin, HistoryTransaction, Labelled}, + Daemon, DaemonBackend, }; pub use coins::CoinsPanel; use label::LabelsEdited; @@ -76,45 +73,6 @@ 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 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 - // syncing status could be misleading as it could suggest the rescan is - // 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. - if daemon_backend == DaemonBackend::RemoteBackend - || daemon_backend == DaemonBackend::EmbeddedLianad(Some(NodeType::Electrum)) - { - return SyncStatus::WalletFullScan; - } - } - // 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, sync_status: SyncStatus, diff --git a/gui/src/app/wallet.rs b/gui/src/app/wallet.rs index 506e3984..d694cb5d 100644 --- a/gui/src/app/wallet.rs +++ b/gui/src/app/wallet.rs @@ -2,7 +2,9 @@ use std::collections::{HashMap, HashSet}; use std::path::Path; use std::sync::Arc; -use crate::{app::settings, hw::HardwareWalletConfig, signer::Signer}; +use crate::{ + app::settings, daemon::DaemonBackend, hw::HardwareWalletConfig, node::NodeType, signer::Signer, +}; use liana::{miniscript::bitcoin, signer::HotSigner}; @@ -206,3 +208,50 @@ impl SyncStatus { self == &SyncStatus::Synced } } + +/// Get the [`SyncStatus`]. +/// +/// The `last_poll_at_startup` is the timestamp of the last poll +/// of the blockchain when the application was first loaded, while +/// `last_poll` refers to the most recent poll. +/// +/// `sync_progress` is the blockchain synchronization progress as +/// a number between `0.0` and `1.0`. +pub fn sync_status( + daemon_backend: DaemonBackend, + blockheight: i32, + sync_progress: f64, + last_poll: Option, + last_poll_at_startup: Option, +) -> SyncStatus { + 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 + // syncing status could be misleading as it could suggest the rescan is + // 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. + if daemon_backend == DaemonBackend::RemoteBackend + || daemon_backend == DaemonBackend::EmbeddedLianad(Some(NodeType::Electrum)) + { + return SyncStatus::WalletFullScan; + } + } + // 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 +} From acf14c6d734fc238dc1f44a4b5bab1f3f6411c9b Mon Sep 17 00:00:00 2001 From: Michael Mallan Date: Thu, 31 Oct 2024 13:29:25 +0000 Subject: [PATCH 2/5] gui: refresh cache more often while syncing Refresh the cache more often while the wallet has a syncing status of some kind in order to detect sooner that this has finished. This does not cover syncing scenarios that require the first poll to be detected since we don't currently store the required info (the last poll at startup). This should be covered in a follow-up. --- gui/src/app/mod.rs | 43 ++++++++++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/gui/src/app/mod.rs b/gui/src/app/mod.rs index bf0c4bf7..a09a0a89 100644 --- a/gui/src/app/mod.rs +++ b/gui/src/app/mod.rs @@ -32,6 +32,7 @@ use state::{ CoinsPanel, CreateSpendPanel, Home, PsbtsPanel, ReceivePanel, RecoveryPanel, State, TransactionsPanel, }; +use wallet::{sync_status, SyncStatus}; use crate::{ app::{cache::Cache, error::Error, menu::Menu, wallet::Wallet}, @@ -227,19 +228,35 @@ impl App { pub fn subscription(&self) -> Subscription { Subscription::batch(vec![ time::every(Duration::from_secs( - // LianaLite has no rescan feature, the cache refresh loop is only - // to fetch the new block height tip, which for a synced wallet - // (height > 0) is only used to warn user about recovery availability. - if self.daemon.backend() == DaemonBackend::RemoteBackend - && self.cache.blockheight > 0 - { - 120 - // For the rescan feature, we set a higher frequency of cache refresh - // to give to user an up-to-date view of the rescan progress. - // For a remote backend, we refresh cache more often while height is 0 - // to detect sooner that syncing has finished. - } else { - 10 + // Note that for now we pass `None` for `last_poll_at_startup`, + // which means the `LatestWalletSync` status will not be returned + // unless the last poll is also `None`. + // TODO: Store last poll at startup and use it here. + match sync_status( + self.daemon.backend(), + self.cache.blockheight, + self.cache.sync_progress, + self.cache.last_poll_timestamp, + None, + ) { + SyncStatus::BlockchainSync(_) => 5, // Only applies to local backends + SyncStatus::WalletFullScan + if self.daemon.backend() == DaemonBackend::RemoteBackend => + { + 10 + } // If remote backend, don't ping too often + SyncStatus::WalletFullScan | SyncStatus::LatestWalletSync => 3, + SyncStatus::Synced => { + if self.daemon.backend() == DaemonBackend::RemoteBackend { + // Remote backend has no rescan feature. For a synced wallet, + // cache refresh is only used to warn user about recovery availability. + 120 + } else { + // For the rescan feature, we refresh more often in order + // to give user an up-to-date view of the rescan progress. + 10 + } + } }, )) .map(|_| Message::Tick), From 92a4a4f8dcaa75bc7181767b1958e39c94e8ad3f Mon Sep 17 00:00:00 2001 From: Michael Mallan Date: Fri, 1 Nov 2024 11:29:16 +0000 Subject: [PATCH 3/5] gui(cache): store last poll at startup --- gui/src/app/cache.rs | 4 ++++ gui/src/app/mod.rs | 2 ++ gui/src/loader.rs | 2 ++ gui/src/main.rs | 4 +++- 4 files changed, 11 insertions(+), 1 deletion(-) diff --git a/gui/src/app/cache.rs b/gui/src/app/cache.rs index b4c54e30..e33f3153 100644 --- a/gui/src/app/cache.rs +++ b/gui/src/app/cache.rs @@ -10,7 +10,10 @@ pub struct Cache { pub coins: Vec, pub rescan_progress: Option, pub sync_progress: f64, + /// The most recent `last_poll_timestamp`. pub last_poll_timestamp: Option, + /// The `last_poll_timestamp` when starting the application. + pub last_poll_at_startup: Option, } /// only used for tests. @@ -24,6 +27,7 @@ impl std::default::Default for Cache { rescan_progress: None, sync_progress: 1.0, last_poll_timestamp: None, + last_poll_at_startup: None, } } } diff --git a/gui/src/app/mod.rs b/gui/src/app/mod.rs index a09a0a89..9e5a4b60 100644 --- a/gui/src/app/mod.rs +++ b/gui/src/app/mod.rs @@ -284,6 +284,7 @@ impl App { let daemon = self.daemon.clone(); let datadir_path = self.cache.datadir_path.clone(); let network = self.cache.network; + let last_poll_at_startup = self.cache.last_poll_at_startup; Command::perform( async move { // we check every 10 second if the daemon poller is alive @@ -302,6 +303,7 @@ impl App { rescan_progress: info.rescan_progress, sync_progress: info.sync, last_poll_timestamp: info.last_poll_timestamp, + last_poll_at_startup, // doesn't change }) }, Message::UpdateCache, diff --git a/gui/src/loader.rs b/gui/src/loader.rs index 0f9b679a..28a6dd5b 100644 --- a/gui/src/loader.rs +++ b/gui/src/loader.rs @@ -407,7 +407,9 @@ pub async fn load_application( blockheight: info.block_height, coins, sync_progress: info.sync, + // Both last poll fields start with the same value. last_poll_timestamp: info.last_poll_timestamp, + last_poll_at_startup: info.last_poll_timestamp, ..Default::default() }; diff --git a/gui/src/main.rs b/gui/src/main.rs index 74b6cc85..57c0a120 100644 --- a/gui/src/main.rs +++ b/gui/src/main.rs @@ -462,7 +462,9 @@ pub fn create_app_with_remote_backend( 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. + // We ignore last poll fields for remote backend. + last_poll_timestamp: None, + last_poll_at_startup: None, }, Arc::new( Wallet::new(wallet.descriptor) From c915970c7b00adbd27c0d8a20116d1bda57a8d2c Mon Sep 17 00:00:00 2001 From: Michael Mallan Date: Fri, 1 Nov 2024 11:31:59 +0000 Subject: [PATCH 4/5] gui(home): pass sync status directly --- gui/src/app/mod.rs | 11 +++++++---- gui/src/app/state/mod.rs | 40 ++++------------------------------------ 2 files changed, 11 insertions(+), 40 deletions(-) diff --git a/gui/src/app/mod.rs b/gui/src/app/mod.rs index 9e5a4b60..55f86bac 100644 --- a/gui/src/app/mod.rs +++ b/gui/src/app/mod.rs @@ -67,10 +67,13 @@ impl Panels { home: Home::new( wallet.clone(), &cache.coins, - cache.blockheight, - cache.sync_progress, - cache.last_poll_timestamp, - daemon_backend.clone(), + sync_status( + daemon_backend.clone(), + cache.blockheight, + cache.sync_progress, + cache.last_poll_timestamp, + cache.last_poll_at_startup, + ), ), coins: CoinsPanel::new(&cache.coins, wallet.main_descriptor.first_timelock_value()), transactions: TransactionsPanel::new(wallet.clone()), diff --git a/gui/src/app/state/mod.rs b/gui/src/app/state/mod.rs index 6694b248..682864a0 100644 --- a/gui/src/app/state/mod.rs +++ b/gui/src/app/state/mod.rs @@ -32,7 +32,7 @@ pub const HISTORY_EVENT_PAGE_SIZE: u64 = 20; use crate::daemon::{ model::{remaining_sequence, Coin, HistoryTransaction, Labelled}, - Daemon, DaemonBackend, + Daemon, }; pub use coins::CoinsPanel; use label::LabelsEdited; @@ -76,7 +76,6 @@ pub fn redirect(menu: Menu) -> Command { pub struct Home { wallet: Arc, sync_status: SyncStatus, - last_poll_at_startup: Option, balance: Amount, unconfirmed_balance: Amount, remaining_sequence: Option, @@ -91,14 +90,7 @@ pub struct Home { } impl Home { - pub fn new( - wallet: Arc, - coins: &[Coin], - blockheight: i32, - sync_progress: f64, - last_poll: Option, - daemon_backend: DaemonBackend, - ) -> Self { + pub fn new(wallet: Arc, coins: &[Coin], sync_status: SyncStatus) -> Self { let (balance, unconfirmed_balance) = coins.iter().fold( (Amount::from_sat(0), Amount::from_sat(0)), |(balance, unconfirmed_balance), coin| { @@ -112,18 +104,9 @@ impl Home { }, ); - let sync_status = sync_status( - daemon_backend, - blockheight, - sync_progress, - last_poll, - last_poll, - ); - Self { wallet, sync_status, - last_poll_at_startup: last_poll, balance, unconfirmed_balance, remaining_sequence: None, @@ -137,22 +120,6 @@ impl Home { processing: false, } } - - fn sync_status( - &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, - ) - } } impl State for Home { @@ -263,11 +230,12 @@ impl State for Home { }, Message::UpdatePanelCache(is_current, Ok(cache)) => { let wallet_was_syncing = !self.sync_status.is_synced(); - self.sync_status = self.sync_status( + self.sync_status = sync_status( daemon.backend(), cache.blockheight, cache.sync_progress, cache.last_poll_timestamp, + cache.last_poll_at_startup, ); // If this is the current panel, reload it if wallet is no longer syncing. if is_current && wallet_was_syncing && self.sync_status.is_synced() { From 918909db235ff8a0886a9f74d7ac3b1caec83324 Mon Sep 17 00:00:00 2001 From: Michael Mallan Date: Fri, 1 Nov 2024 11:33:31 +0000 Subject: [PATCH 5/5] gui: include last poll for cache refresh interval --- gui/src/app/mod.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/gui/src/app/mod.rs b/gui/src/app/mod.rs index 55f86bac..ac9f69cc 100644 --- a/gui/src/app/mod.rs +++ b/gui/src/app/mod.rs @@ -231,16 +231,12 @@ impl App { pub fn subscription(&self) -> Subscription { Subscription::batch(vec![ time::every(Duration::from_secs( - // Note that for now we pass `None` for `last_poll_at_startup`, - // which means the `LatestWalletSync` status will not be returned - // unless the last poll is also `None`. - // TODO: Store last poll at startup and use it here. match sync_status( self.daemon.backend(), self.cache.blockheight, self.cache.sync_progress, self.cache.last_poll_timestamp, - None, + self.cache.last_poll_at_startup, ) { SyncStatus::BlockchainSync(_) => 5, // Only applies to local backends SyncStatus::WalletFullScan