Merge #1386: [GUI] Skip loading screen for existing wallets

0a674d591affd6190d5b4979718fb09d3d8f79f3 gui: load app directly if wallet was previously synced (Michael Mallan)
1408f66963cf6ae5f2fe2ac29d2cd91f747477bc gui(home): indicate if blockchain is syncing (Michael Mallan)
62788d105c2fcdb1e6c400ca6f5f19138378ae0b gui(cache): include sync progress (Michael Mallan)
f3a136c30be0e443cebae85785f70999dfaf8485 gui(home): refactor sync status logic (Michael Mallan)

Pull request description:

  This is to resolve #1384 by skipping the loading screen for an existing wallet even if the blockchain is still syncing.

  This change will only make a difference for local backends given that ~~the blockchain for a Liana Connect wallet is always considered to be synced and so the loading screen was already loading the application directly~~ Liana Connect doesn't use the loading screen.

  It builds on #1377, to additionally check the blockchain sync progress, and show a message with the sync progress as a percentage in case the blockchain is not fully synced. ~~using the logic added there to indicate on the home page that syncing is in progress if the poller has not yet run. Polling does not start until the blockchain has fully synced and so the same logic can be applied here.~~

  ~~For now, no distinction is made on the home page between the wallet syncing and the blockchain syncing, but this may change in future.~~

ACKs for top commit:
  edouardparis:
    ACK 0a674d591affd6190d5b4979718fb09d3d8f79f3

Tree-SHA512: 802e31b7f4be7ed9687e32a5921f19e4560f5f4b4f975fff452be2907954fc9c4c577add713bc999f6d8efa4c6502825361553c9cd5bcdb3be0d5f800f0aa213

gui/src/app/cache.rs

@@ -9,6 +9,7 @@ pub struct Cache {
     pub blockheight: i32,
     pub coins: Vec<Coin>,
     pub rescan_progress: Option<f64>,
+    pub sync_progress: f64,
     pub last_poll_timestamp: Option<u32>,
 }
 
@@ -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,
         }
     }

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(),
             ),
@@ -282,6 +283,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,
                         })
                     },

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,16 @@ pub fn redirect(menu: Menu) -> Command<Message> {
     })
 }
 
-fn wallet_is_syncing(
+fn sync_status(
     daemon_backend: DaemonBackend,
     blockheight: i32,
+    sync_progress: f64,
     last_poll: Option<u32>,
     last_poll_at_startup: Option<u32>,
-) -> bool {
-    match daemon_backend {
-        // If remote, the wallet is always synced except before the first scan
-        // after creation.
-        DaemonBackend::RemoteBackend => blockheight <= 0,
+) -> 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
@@ -86,30 +93,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>,
-    wallet_is_syncing: bool,
-    blockheight: i32,
+    sync_status: SyncStatus,
     last_poll_at_startup: Option<u32>,
     balance: Amount,
     unconfirmed_balance: Amount,
@@ -129,6 +137,7 @@ impl Home {
         wallet: Arc<Wallet>,
         coins: &[Coin],
         blockheight: i32,
+        sync_progress: f64,
         last_poll: Option<u32>,
         daemon_backend: DaemonBackend,
     ) -> Self {
@@ -145,14 +154,18 @@ impl Home {
             },
         );
 
-        let wallet_is_syncing =
-            wallet_is_syncing(daemon_backend, blockheight, last_poll, last_poll);
+        let sync_status = sync_status(
+            daemon_backend,
+            blockheight,
+            sync_progress,
+            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 +180,17 @@ impl Home {
         }
     }
 
-    fn wallet_is_syncing(&self, daemon_backend: DaemonBackend, last_poll: Option<u32>) -> bool {
-        wallet_is_syncing(
+    fn sync_status(
+        &self,
+        daemon_backend: DaemonBackend,
+        blockheight: i32,
+        sync_progress: f64,
+        last_poll: Option<u32>,
+    ) -> SyncStatus {
+        sync_status(
             daemon_backend,
-            self.blockheight,
+            blockheight,
+            sync_progress,
             last_poll,
             self.last_poll_at_startup,
         )
@@ -206,8 +226,7 @@ impl State for Home {
                     &self.events,
                     self.is_last_page,
                     self.processing,
-                    self.wallet_is_syncing,
-                    self.blockheight,
+                    &self.sync_status,
                 ),
             )
         }
@@ -285,12 +304,15 @@ 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.sync_progress,
+                    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 +393,7 @@ impl State for Home {
         wallet: Arc<Wallet>,
     ) -> Command<Message> {
         // 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;

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,15 +58,18 @@ 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 {
-                                    "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(
@@ -79,17 +82,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| {

gui/src/app/wallet.rs

@@ -187,3 +187,22 @@ impl From<settings::SettingsError> 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,
+    /// Blockchain is syncing with given progress between 0.0 and 1.0.
+    BlockchainSync(f64),
+}
+
+impl SyncStatus {
+    pub fn is_synced(&self) -> bool {
+        self == &SyncStatus::Synced
+    }
+}

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<client::jsonrpc::JsonRPCClient>;
+type StartedResult = Result<
+    (
+        Arc<dyn Daemon + Sync + Send>,
+        Option<Bitcoind>,
+        GetInfoResult,
+    ),
+    Error,
+>;
 
 pub struct Loader {
     pub datadir_path: PathBuf,
@@ -78,8 +86,8 @@ pub enum Message {
             Error,
         >,
     ),
-    Started(Result<(Arc<dyn Daemon + Sync + Send>, Option<Bitcoind>), Error>),
-    Loaded(Result<Arc<dyn Daemon + Sync + Send>, Error>),
+    Started(StartedResult),
+    Loaded(Result<(Arc<dyn Daemon + Sync + Send>, GetInfoResult), Error>),
     BitcoindLog(Option<String>),
     Failure(DaemonError),
     None,
@@ -110,18 +118,44 @@ impl Loader {
         )
     }
 
-    fn on_load(&mut self, res: Result<Arc<dyn Daemon + Sync + Send>, Error>) -> Command<Message> {
+    fn maybe_skip_syncing(
+        &mut self,
+        daemon: Arc<dyn Daemon + Sync + Send>,
+        info: GetInfoResult,
+    ) -> Command<Message> {
+        // 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<dyn Daemon + Sync + Send>, GetInfoResult), Error>,
+    ) -> Command<Message> {
         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<dyn Daemon + Sync + Send>, Option<Bitcoind>), Error>,
-    ) -> Command<Message> {
+    fn on_start(&mut self, res: StartedResult) -> Command<Message> {
         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));
@@ -380,6 +406,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()
     };
@@ -480,15 +507,17 @@ pub fn cover<'a, T: 'a + Clone, C: Into<Element<'a, T>>>(
         .into()
 }
 
-async fn connect(socket_path: PathBuf) -> Result<Arc<dyn Daemon + Sync + Send>, Error> {
+async fn connect(
+    socket_path: PathBuf,
+) -> Result<(Arc<dyn Daemon + Sync + Send>, 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.
@@ -496,7 +525,7 @@ pub async fn start_bitcoind_and_daemon(
     config_path: PathBuf,
     liana_datadir_path: PathBuf,
     start_internal_bitcoind: bool,
-) -> Result<(Arc<dyn Daemon + Sync + Send>, Option<Bitcoind>), Error> {
+) -> StartedResult {
     let config = Config::from_file(Some(config_path)).map_err(Error::Config)?;
     let mut bitcoind: Option<Bitcoind> = None;
     if start_internal_bitcoind {
@@ -522,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(

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.