From 73d862fb4380f606af27101374f94bad53c65dc5 Mon Sep 17 00:00:00 2001 From: Hansie Odendaal <39146854+hansieodendaal@users.noreply.github.com> Date: Mon, 29 Nov 2021 13:58:04 +0200 Subject: [PATCH] feat: improve wallet responsiveness (#3625) Description --- Improved wallet responsiveness by cutting down on unnecessary wallet database calls: - `TransactionEvent::TransactionValidationSuccess` was published regardless of any state change in the wallet database, which resulted in many unnecessary `fetch 'All Complete Transactions'` calls to the wallet database. This issue was optimized to only send the event when a state change was affected. - A missing event was added when state changed due to reorgs. - This should have a slight positive impact on mobile wallet battery usage. Motivation and Context --- Unnecessary wallet database calls to fetch all completed transactions were made each time the transaction validation protocol was run, especially when nothing changed in the database. This effect grew linearly with the amount of transactions in the database, How Has This Been Tested? --- - Unit tests - Cucumber tests (`npm test -- --tags "not @long-running and not @broken"`) - System level tests --- .../src/ui/state/wallet_event_monitor.rs | 2 +- base_layer/wallet/src/transaction_service/handle.rs | 2 +- .../protocols/transaction_validation_protocol.rs | 11 ++++++++++- base_layer/wallet_ffi/src/callback_handler.rs | 2 +- base_layer/wallet_ffi/src/callback_handler_tests.rs | 2 +- 5 files changed, 14 insertions(+), 5 deletions(-) diff --git a/applications/tari_console_wallet/src/ui/state/wallet_event_monitor.rs b/applications/tari_console_wallet/src/ui/state/wallet_event_monitor.rs index b47b306e9c..1b2d2abce4 100644 --- a/applications/tari_console_wallet/src/ui/state/wallet_event_monitor.rs +++ b/applications/tari_console_wallet/src/ui/state/wallet_event_monitor.rs @@ -119,7 +119,7 @@ impl WalletEventMonitor { self.trigger_balance_refresh(); notifier.transaction_sent(tx_id); }, - TransactionEvent::TransactionValidationSuccess(_) => { + TransactionEvent::TransactionValidationStateChanged(_) => { self.trigger_full_tx_state_refresh().await; self.trigger_balance_refresh(); }, diff --git a/base_layer/wallet/src/transaction_service/handle.rs b/base_layer/wallet/src/transaction_service/handle.rs index 6e914aebde..8311773404 100644 --- a/base_layer/wallet/src/transaction_service/handle.rs +++ b/base_layer/wallet/src/transaction_service/handle.rs @@ -166,7 +166,7 @@ pub enum TransactionEvent { is_valid: bool, }, TransactionValidationTimedOut(u64), - TransactionValidationSuccess(u64), + TransactionValidationStateChanged(u64), TransactionValidationFailure(u64), TransactionValidationAborted(u64), TransactionValidationDelayed(u64), diff --git a/base_layer/wallet/src/transaction_service/protocols/transaction_validation_protocol.rs b/base_layer/wallet/src/transaction_service/protocols/transaction_validation_protocol.rs index bf34834081..c582c45b73 100644 --- a/base_layer/wallet/src/transaction_service/protocols/transaction_validation_protocol.rs +++ b/base_layer/wallet/src/transaction_service/protocols/transaction_validation_protocol.rs @@ -112,6 +112,7 @@ where .for_protocol(self.operation_id) .unwrap(); + let mut state_changed = false; for batch in unconfirmed_transactions.chunks(self.config.max_tx_query_batch_size) { let (mined, unmined, tip_info) = self .query_base_node_for_transactions(batch, &mut *base_node_wallet_client) @@ -133,6 +134,7 @@ where *num_confirmations, ) .await?; + state_changed = true; } if let Some((tip_height, tip_block)) = tip_info { for unmined_tx in &unmined { @@ -147,6 +149,7 @@ where tip_height.saturating_sub(unmined_tx.coinbase_block_height.unwrap_or_default()), ) .await?; + state_changed = true; } else { info!( target: LOG_TARGET, @@ -163,11 +166,14 @@ where ); self.update_transaction_as_unmined(unmined_tx.tx_id, &unmined_tx.status) .await?; + state_changed = true; } } } } - self.publish_event(TransactionEvent::TransactionValidationSuccess(self.operation_id)); + if state_changed { + self.publish_event(TransactionEvent::TransactionValidationStateChanged(self.operation_id)); + } Ok(self.operation_id) } @@ -234,6 +240,9 @@ where ); self.update_transaction_as_unmined(last_mined_transaction.tx_id, &last_mined_transaction.status) .await?; + self.publish_event(TransactionEvent::TransactionValidationStateChanged( + last_mined_transaction.tx_id, + )); } else { info!( target: LOG_TARGET, diff --git a/base_layer/wallet_ffi/src/callback_handler.rs b/base_layer/wallet_ffi/src/callback_handler.rs index 9418b38e1b..4f7b57eb64 100644 --- a/base_layer/wallet_ffi/src/callback_handler.rs +++ b/base_layer/wallet_ffi/src/callback_handler.rs @@ -258,7 +258,7 @@ where TBackend: TransactionBackend + 'static self.receive_transaction_mined_unconfirmed_event(tx_id, num_confirmations).await; self.trigger_balance_refresh().await; }, - TransactionEvent::TransactionValidationSuccess(tx_id) => { + TransactionEvent::TransactionValidationStateChanged(tx_id) => { self.transaction_validation_complete_event(tx_id, CallbackValidationResults::Success); self.trigger_balance_refresh().await; }, diff --git a/base_layer/wallet_ffi/src/callback_handler_tests.rs b/base_layer/wallet_ffi/src/callback_handler_tests.rs index 4d90f2b3ee..75e233d158 100644 --- a/base_layer/wallet_ffi/src/callback_handler_tests.rs +++ b/base_layer/wallet_ffi/src/callback_handler_tests.rs @@ -476,7 +476,7 @@ mod test { assert_eq!(callback_balance_updated, 5); transaction_event_sender - .send(Arc::new(TransactionEvent::TransactionValidationSuccess(1u64))) + .send(Arc::new(TransactionEvent::TransactionValidationStateChanged(1u64))) .unwrap(); oms_event_sender