From 30e99d3aecfbbfd89af28ac43927abd64d9a77a4 Mon Sep 17 00:00:00 2001 From: Lucas Soriano del Pino Date: Tue, 19 Sep 2023 20:58:45 +0200 Subject: [PATCH 1/6] feat(app): Run wallet refresh asynchronously We don't need to wait for the on-chain sync + wallet refresh to complete when calling `refresh_wallet_info`, because the result will be sent asynchronously to the UI via an event anyway. --- mobile/native/src/ln_dlc/mod.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/mobile/native/src/ln_dlc/mod.rs b/mobile/native/src/ln_dlc/mod.rs index 79d927694..4309736b2 100644 --- a/mobile/native/src/ln_dlc/mod.rs +++ b/mobile/native/src/ln_dlc/mod.rs @@ -89,12 +89,27 @@ pub const FUNDING_TX_WEIGHT_ESTIMATE: u64 = 220; static NODE: Storage> = Storage::new(); static SEED: Storage = Storage::new(); +/// Trigger an on-chain sync followed by an update to the wallet balance and history. +/// +/// We do not wait for the triggered task to finish, because the effect will be reflected +/// asynchronously on the UI. pub async fn refresh_wallet_info() -> Result<()> { let node = NODE.try_get().context("failed to get ln dlc node")?; let wallet = node.inner.wallet(); - spawn_blocking(move || wallet.sync()).await??; - keep_wallet_balance_and_history_up_to_date(node)?; + // Spawn into the blocking thread pool of the dedicated backend runtime to avoid blocking the UI + // thread. + let runtime = get_or_create_tokio_runtime()?; + runtime.spawn_blocking(move || { + if let Err(e) = wallet.sync() { + tracing::error!("Manually triggered on-chain sync failed: {e:#}"); + } + if let Err(e) = keep_wallet_balance_and_history_up_to_date(node) { + tracing::error!("Failed to keep wallet history up to date: {e:#}"); + } + + anyhow::Ok(()) + }); Ok(()) } From 36eb2f3ab01edbdd5c8e77ba8d35b809e8651402 Mon Sep 17 00:00:00 2001 From: Lucas Soriano del Pino Date: Wed, 20 Sep 2023 10:17:20 +0200 Subject: [PATCH 2/6] feat(app): Delay first periodic on-chain sync by a minute To ensure that the wallet refresh can happen quickly before the first on-chain sync. This allows the wallet to appear ready before finishing the first on-chain sync. To achieve this we've had to remove the automatic on-chain sync from the "default" version of `ln_dlc_node::Node`. Now consumers have to set up the periodic task themselves, although the library provides convenience methods. --- coordinator/src/bin/coordinator.rs | 2 + crates/ln-dlc-node/src/ln_dlc_wallet.rs | 10 ++++- crates/ln-dlc-node/src/node/mod.rs | 56 ++++++++++++------------- maker/src/bin/maker.rs | 2 + mobile/native/src/ln_dlc/mod.rs | 23 ++++++++++ 5 files changed, 64 insertions(+), 29 deletions(-) diff --git a/coordinator/src/bin/coordinator.rs b/coordinator/src/bin/coordinator.rs index 12c1a5b30..706ad0436 100644 --- a/coordinator/src/bin/coordinator.rs +++ b/coordinator/src/bin/coordinator.rs @@ -139,6 +139,8 @@ async fn main() -> Result<()> { }); } + std::thread::spawn(node.inner.sync_on_chain_wallet_periodically()); + tokio::spawn({ let node = node.clone(); async move { diff --git a/crates/ln-dlc-node/src/ln_dlc_wallet.rs b/crates/ln-dlc-node/src/ln_dlc_wallet.rs index 87d0cb89f..936eed06c 100644 --- a/crates/ln-dlc-node/src/ln_dlc_wallet.rs +++ b/crates/ln-dlc-node/src/ln_dlc_wallet.rs @@ -118,7 +118,15 @@ impl LnDlcWallet { self.address_cache.read().clone() } - pub fn update_address_cache(&self) -> Result<()> { + pub fn sync_and_update_address_cache(&self) -> Result<()> { + self.inner().sync()?; + + self.update_address_cache()?; + + Ok(()) + } + + fn update_address_cache(&self) -> Result<()> { let address = self.inner().get_last_unused_address()?; *self.address_cache.write() = address; diff --git a/crates/ln-dlc-node/src/node/mod.rs b/crates/ln-dlc-node/src/node/mod.rs index 82a7f6ace..47b7a30aa 100644 --- a/crates/ln-dlc-node/src/node/mod.rs +++ b/crates/ln-dlc-node/src/node/mod.rs @@ -432,11 +432,6 @@ where self.listen_address, )]; - std::thread::spawn(sync_on_chain_wallet_periodically( - self.settings.clone(), - self.wallet.clone(), - )); - std::thread::spawn(shadow_sync_periodically( self.settings.clone(), self.storage.clone(), @@ -522,6 +517,34 @@ where ) .await } + + /// Returns a closure which triggers an on-chain sync and subsequently updates the address + /// cache, at an interval. + /// + /// The task will loop at an interval determined by the node's [`LnDlcNodeSettings`]. + /// + /// Suitable for daemons such as the coordinator and the maker. + pub fn sync_on_chain_wallet_periodically(&self) -> impl Fn() { + let handle = tokio::runtime::Handle::current(); + let settings = self.settings.clone(); + let ln_dlc_wallet = self.wallet.clone(); + move || loop { + if let Err(e) = ln_dlc_wallet.sync_and_update_address_cache() { + tracing::error!("Failed on-chain sync: {e:#}"); + } + + let interval = handle.block_on(async { + let guard = settings.read().await; + guard.on_chain_sync_interval + }); + + std::thread::sleep(interval); + } + } + + pub fn sync_on_chain_wallet(&self) -> Result<()> { + self.wallet.sync_and_update_address_cache() + } } async fn update_fee_rate_estimates( @@ -636,29 +659,6 @@ fn shadow_sync_periodically( } } -fn sync_on_chain_wallet_periodically( - settings: Arc>, - ln_dlc_wallet: Arc, -) -> impl Fn() { - let handle = tokio::runtime::Handle::current(); - move || loop { - if let Err(e) = ln_dlc_wallet.inner().sync() { - tracing::error!("Failed on-chain sync: {e:#}"); - } - - if let Err(e) = ln_dlc_wallet.update_address_cache() { - tracing::warn!("Failed to update address cache: {e:#}"); - } - - let interval = handle.block_on(async { - let guard = settings.read().await; - guard.on_chain_sync_interval - }); - - std::thread::sleep(interval); - } -} - fn spawn_connection_management( peer_manager: Arc, listen_address: SocketAddr, diff --git a/maker/src/bin/maker.rs b/maker/src/bin/maker.rs index 28b1e2d25..3d0959ccb 100644 --- a/maker/src/bin/maker.rs +++ b/maker/src/bin/maker.rs @@ -90,6 +90,8 @@ async fn main() -> Result<()> { let event_handler = EventHandler::new(node.clone()); let _running_node = node.start(event_handler)?; + std::thread::spawn(node.sync_on_chain_wallet_periodically()); + let (health, health_tx) = health::Health::new(); let node_pubkey = node.info.pubkey; diff --git a/mobile/native/src/ln_dlc/mod.rs b/mobile/native/src/ln_dlc/mod.rs index 4309736b2..1c7be7668 100644 --- a/mobile/native/src/ln_dlc/mod.rs +++ b/mobile/native/src/ln_dlc/mod.rs @@ -76,6 +76,7 @@ pub use channel_status::ChannelStatus; const PROCESS_INCOMING_DLC_MESSAGES_INTERVAL: Duration = Duration::from_millis(200); const UPDATE_WALLET_HISTORY_INTERVAL: Duration = Duration::from_secs(5); const CHECK_OPEN_ORDERS_INTERVAL: Duration = Duration::from_secs(60); +const ON_CHAIN_SYNC_INTERVAL: Duration = Duration::from_secs(300); /// The weight estimate of the funding transaction /// @@ -239,6 +240,28 @@ pub fn run(data_dir: String, seed_dir: String, runtime: &Runtime) -> Result<()> let _running = node.start(event_handler)?; let node = Arc::new(Node::new(node, _running)); + std::thread::spawn({ + let node = node.clone(); + move || { + // We wait a minute to not interfere with the app's startup. We want to be able to + // quickly refresh the wallet _before_ the first on-chain sync, as this one can take + // a long time and blocks the wallet refresh since they both need to acquire the + // same mutex. + // + // TODO: This should not be necessary as soon as we rewrite the on-chain wallet with + // bdk:1.0.0. + std::thread::sleep(Duration::from_secs(60)); + + loop { + if let Err(e) = node.inner.sync_on_chain_wallet() { + tracing::error!("Failed on-chain sync: {e:#}"); + } + + std::thread::sleep(ON_CHAIN_SYNC_INTERVAL); + } + } + }); + runtime.spawn({ let node = node.clone(); async move { node.listen_for_lightning_events(event_receiver).await } From 94ff866dcb0cc43ae1a38cb2986a61b183736198 Mon Sep 17 00:00:00 2001 From: Lucas Soriano del Pino Date: Wed, 20 Sep 2023 12:44:56 +0200 Subject: [PATCH 3/6] feat(app): Do not force on-chain sync on startup We already have: - A periodic on-chain sync starting one minute after the app has started. - A periodic wallet refresh which updates the wallet history and balances every 5 seconds (without syncing on-chain every time). To have an app which appears ready as soon as possible, what we actually want is to refresh the wallet balances and history as soon as possible and delay the on-chain sync until a bit later. This is already achieved by the periodic tasks described above, but we need to remove a manual call to `refreshWalletInfo`, because that one actually triggers the on-chain sync. We are obviously introducing a trade-off, as the user will no longer see the latest state of their wallet in terms of the blockchain as soon as the app appears ready. --- mobile/lib/main.dart | 3 --- 1 file changed, 3 deletions(-) diff --git a/mobile/lib/main.dart b/mobile/lib/main.dart index ae61dea76..911cbd771 100644 --- a/mobile/lib/main.dart +++ b/mobile/lib/main.dart @@ -259,7 +259,6 @@ class _TenTenOneAppState extends State { final orderChangeNotifier = context.read(); final positionChangeNotifier = context.read(); final candlestickChangeNotifier = context.read(); - final walletChangeNotifier = context.read(); try { setupRustLogging(); @@ -277,8 +276,6 @@ class _TenTenOneAppState extends State { final lastLogin = await rust.api.updateLastLogin(); FLog.debug(text: "Last login was at ${lastLogin.date}"); - - await walletChangeNotifier.refreshWalletInfo(); } on FfiException catch (error) { FLog.error(text: "Failed to initialise: Error: ${error.message}", exception: error); } catch (error) { From 0cd3ffa241db3a4db2aa4ec0f1ef3e7433e4eee9 Mon Sep 17 00:00:00 2001 From: Lucas Soriano del Pino Date: Wed, 20 Sep 2023 12:57:22 +0200 Subject: [PATCH 4/6] feat(app): Include Lightning wallet sync in refresh_wallet_info I do not think that the Lightning wallet (on-chain) sync should have an impact on the balances or history in most scenarios, but I think it is still useful if we are waiting for confirmations on Lightning-related on-chain transactions (fund, commitment, etc.). --- crates/ln-dlc-node/src/node/mod.rs | 49 +++++++++++++++++++++--------- mobile/native/src/ln_dlc/mod.rs | 5 +++ 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/crates/ln-dlc-node/src/node/mod.rs b/crates/ln-dlc-node/src/node/mod.rs index 47b7a30aa..dc3bfed81 100644 --- a/crates/ln-dlc-node/src/node/mod.rs +++ b/crates/ln-dlc-node/src/node/mod.rs @@ -439,7 +439,7 @@ where self.channel_manager.clone(), )); - tokio::spawn(lightning_wallet_sync( + tokio::spawn(periodic_lightning_wallet_sync( self.channel_manager.clone(), self.chain_monitor.clone(), self.settings.clone(), @@ -545,6 +545,14 @@ where pub fn sync_on_chain_wallet(&self) -> Result<()> { self.wallet.sync_and_update_address_cache() } + + pub fn sync_lightning_wallet(&self) -> Result<()> { + lightning_wallet_sync( + &self.channel_manager, + &self.chain_monitor, + &self.esplora_client, + ) + } } async fn update_fee_rate_estimates( @@ -603,26 +611,15 @@ fn spawn_background_processor( remote_handle } -async fn lightning_wallet_sync( +async fn periodic_lightning_wallet_sync( channel_manager: Arc, chain_monitor: Arc, settings: Arc>, esplora_client: Arc>>, ) { loop { - let now = Instant::now(); - let confirmables = vec![ - &*channel_manager as &(dyn Confirm + Sync + Send), - &*chain_monitor as &(dyn Confirm + Sync + Send), - ]; - match esplora_client.sync(confirmables) { - Ok(()) => tracing::info!( - "Background sync of Lightning wallet finished in {}ms.", - now.elapsed().as_millis() - ), - Err(e) => { - tracing::error!("Background sync of Lightning wallet failed: {e:#}") - } + if let Err(e) = lightning_wallet_sync(&channel_manager, &chain_monitor, &esplora_client) { + tracing::error!("Background sync of Lightning wallet failed: {e:#}") } let interval = { @@ -633,6 +630,28 @@ async fn lightning_wallet_sync( } } +fn lightning_wallet_sync( + channel_manager: &ChannelManager, + chain_monitor: &ChainMonitor, + esplora_client: &EsploraSyncClient>, +) -> Result<()> { + let now = Instant::now(); + let confirmables = vec![ + channel_manager as &(dyn Confirm + Sync + Send), + chain_monitor as &(dyn Confirm + Sync + Send), + ]; + esplora_client + .sync(confirmables) + .context("Lightning wallet sync failed")?; + + tracing::info!( + "Lightning wallet sync finished in {}ms.", + now.elapsed().as_millis() + ); + + Ok(()) +} + fn shadow_sync_periodically( settings: Arc>, node_storage: Arc, diff --git a/mobile/native/src/ln_dlc/mod.rs b/mobile/native/src/ln_dlc/mod.rs index 1c7be7668..8685492e7 100644 --- a/mobile/native/src/ln_dlc/mod.rs +++ b/mobile/native/src/ln_dlc/mod.rs @@ -105,6 +105,11 @@ pub async fn refresh_wallet_info() -> Result<()> { if let Err(e) = wallet.sync() { tracing::error!("Manually triggered on-chain sync failed: {e:#}"); } + + if let Err(e) = node.inner.sync_lightning_wallet() { + tracing::error!("Manually triggered Lightning wallet sync failed: {e:#}"); + } + if let Err(e) = keep_wallet_balance_and_history_up_to_date(node) { tracing::error!("Failed to keep wallet history up to date: {e:#}"); } From 76f386c4b8e1b8e620b14b358b4613efcebbb036 Mon Sep 17 00:00:00 2001 From: Lucas Soriano del Pino Date: Wed, 20 Sep 2023 14:40:42 +0200 Subject: [PATCH 5/6] feat(app): Remove arbitrary sleep before on-chain sync A considerably better solution is to ensure that we execute `keep_wallet_balance_and_history_up_to_date` to completion once before we spawn the periodic on-chain sync task. Co-authored-by: Richard Holzeis --- mobile/native/src/ln_dlc/mod.rs | 63 +++++++++++++++++---------------- 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/mobile/native/src/ln_dlc/mod.rs b/mobile/native/src/ln_dlc/mod.rs index 8685492e7..6e13fb948 100644 --- a/mobile/native/src/ln_dlc/mod.rs +++ b/mobile/native/src/ln_dlc/mod.rs @@ -245,25 +245,44 @@ pub fn run(data_dir: String, seed_dir: String, runtime: &Runtime) -> Result<()> let _running = node.start(event_handler)?; let node = Arc::new(Node::new(node, _running)); - std::thread::spawn({ + // Refresh the wallet balance and history eagerly so that it can complete before the + // triggering the first on-chain sync. This ensures that the UI appears ready as soon as + // possible. + // + // TODO: This might not be necessary once we rewrite the on-chain wallet with bdk:1.0.0. + spawn_blocking({ let node = node.clone(); - move || { - // We wait a minute to not interfere with the app's startup. We want to be able to - // quickly refresh the wallet _before_ the first on-chain sync, as this one can take - // a long time and blocks the wallet refresh since they both need to acquire the - // same mutex. - // - // TODO: This should not be necessary as soon as we rewrite the on-chain wallet with - // bdk:1.0.0. - std::thread::sleep(Duration::from_secs(60)); + move || keep_wallet_balance_and_history_up_to_date(&node) + }) + .await + .expect("task to complete")?; + runtime.spawn({ + let node = node.clone(); + async move { loop { - if let Err(e) = node.inner.sync_on_chain_wallet() { - tracing::error!("Failed on-chain sync: {e:#}"); + tokio::time::sleep(UPDATE_WALLET_HISTORY_INTERVAL).await; + + let node = node.clone(); + if let Err(e) = + spawn_blocking(move || keep_wallet_balance_and_history_up_to_date(&node)) + .await + .expect("To spawn blocking task") + { + tracing::error!("Failed to sync balance and wallet history: {e:#}"); } + } + } + }); - std::thread::sleep(ON_CHAIN_SYNC_INTERVAL); + std::thread::spawn({ + let node = node.clone(); + move || loop { + if let Err(e) = node.inner.sync_on_chain_wallet() { + tracing::error!("Failed on-chain sync: {e:#}"); } + + std::thread::sleep(ON_CHAIN_SYNC_INTERVAL); } }); @@ -290,24 +309,6 @@ pub fn run(data_dir: String, seed_dir: String, runtime: &Runtime) -> Result<()> } }); - runtime.spawn({ - let node = node.clone(); - async move { - loop { - let node = node.clone(); - if let Err(e) = - spawn_blocking(move || keep_wallet_balance_and_history_up_to_date(&node)) - .await - .expect("To spawn blocking task") - { - tracing::error!("Failed to sync balance and wallet history: {e:#}"); - } - - tokio::time::sleep(UPDATE_WALLET_HISTORY_INTERVAL).await; - } - } - }); - runtime.spawn(async move { loop { if let Err(e) = spawn_blocking(order::handler::check_open_orders) From 8fb0324135b49a18e43f0fc463d9f9937678d5ed Mon Sep 17 00:00:00 2001 From: Lucas Soriano del Pino Date: Wed, 20 Sep 2023 14:43:29 +0200 Subject: [PATCH 6/6] feat(app): Do not wait for all jobs to complete on startup If we don't add the `await` keyword, the future is executed in the background. This allows us to speed up app startup slightly. --- mobile/lib/main.dart | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/mobile/lib/main.dart b/mobile/lib/main.dart index 911cbd771..a6c8ec929 100644 --- a/mobile/lib/main.dart +++ b/mobile/lib/main.dart @@ -268,14 +268,15 @@ class _TenTenOneAppState extends State { await runBackend(config); FLog.info(text: "Backend started"); - await orderChangeNotifier.initialize(); - await positionChangeNotifier.initialize(); - await candlestickChangeNotifier.initialize(); + orderChangeNotifier.initialize(); + positionChangeNotifier.initialize(); + candlestickChangeNotifier.initialize(); - await logAppSettings(config); + logAppSettings(config); - final lastLogin = await rust.api.updateLastLogin(); - FLog.debug(text: "Last login was at ${lastLogin.date}"); + rust.api + .updateLastLogin() + .then((lastLogin) => FLog.debug(text: "Last login was at ${lastLogin.date}")); } on FfiException catch (error) { FLog.error(text: "Failed to initialise: Error: ${error.message}", exception: error); } catch (error) {