Skip to content

Commit

Permalink
feat: improve wallet responsiveness (#3625)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
hansieodendaal authored Nov 29, 2021
1 parent 5790a9d commit 73d862f
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
},
Expand Down
2 changes: 1 addition & 1 deletion base_layer/wallet/src/transaction_service/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ pub enum TransactionEvent {
is_valid: bool,
},
TransactionValidationTimedOut(u64),
TransactionValidationSuccess(u64),
TransactionValidationStateChanged(u64),
TransactionValidationFailure(u64),
TransactionValidationAborted(u64),
TransactionValidationDelayed(u64),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand All @@ -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,
Expand All @@ -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)
}

Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion base_layer/wallet_ffi/src/callback_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
},
Expand Down
2 changes: 1 addition & 1 deletion base_layer/wallet_ffi/src/callback_handler_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 73d862f

Please sign in to comment.