feat(wallet): add extra feedback to recovery monitoring callback in W…

…allet FFI (#3128)

## Description
This PR adds an extra stage of callback feedback to the recovery process in the Wallet FFI. The new stage shows when connection’s are being attempted and not just when they are made.

Edit: Added even more detail to the callback, this changes the FFI interface to support extra argument in the callback

## How Has This Been Tested?
For testing in the mobile client

## Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
* [x] I'm merging against the `development` branch.
* [x] I have squashed my commits into a single commit.

applications/tari_console_wallet/src/recovery.rs

@@ -168,6 +168,9 @@ pub async fn wallet_recovery(wallet: &WalletSqlite, base_node_config: &PeerConfi
                 debug!(target: LOG_TARGET, "Error receiving Wallet recovery events: {}", e);
                 continue;
             },
+            Ok(UtxoScannerEvent::ScanningFailed) => {
+                error!(target: LOG_TARGET, "Wallet Recovery process failed and is exiting");
+            },
         }
     }
 

base_layer/wallet/src/utxo_scanner_service/handle.rs

@@ -62,6 +62,8 @@ pub enum UtxoScannerEvent {
         value_received: MicroTari,
         time_taken: Duration,
     },
+    /// Scanning process has failed and scanning process has exited
+    ScanningFailed,
 }
 
 #[derive(Clone)]

base_layer/wallet/src/utxo_scanner_service/utxo_scanning.rs

@@ -588,7 +588,10 @@ where TBackend: WalletBackend + 'static
                             target: LOG_TARGET,
                             "Failed to scan UTXO's from base node {}: {}", peer, e
                         );
-
+                        self.publish_event(UtxoScannerEvent::ScanningRoundFailed {
+                            num_retries: self.num_retries,
+                            retry_limit: self.retry_limit,
+                        });
                         continue;
                     },
                 },
@@ -599,6 +602,7 @@ where TBackend: WalletBackend + 'static
                     });
 
                     if self.num_retries >= self.retry_limit {
+                        self.publish_event(UtxoScannerEvent::ScanningFailed);
                         return Err(UtxoScannerError::UtxoScanningError(format!(
                             "Failed to scan UTXO's after {} attempt(s) using all {} sync peer(s). Aborting...",
                             self.num_retries,

base_layer/wallet_ffi/src/lib.rs

@@ -5266,12 +5266,42 @@ pub unsafe extern "C" fn wallet_is_recovery_in_progress(wallet: *mut TariWallet,
 /// `wallet` - The TariWallet pointer.
 /// `base_node_public_key` - The TariPublicKey pointer of the Base Node the recovery process will use
 /// `recovery_progress_callback` - The callback function pointer that will be used to asynchronously communicate
-/// progress to the client. The first argument is the current block the process has completed and the second argument is
-/// the total chain height. When the current block reaches the total chain height the process is complete.
-///     - The first callback with arguments (0,1) indicate a successful base node connection and process has started
-///     - In progress callbacks will be of the form (n, m) where n < m
-///     - If the process completed successfully then the final callback will have arguments (x, x) where x == x
-///     - If there is an error in the process then the final callback will be called with zero arguments i.e. (0, 0)
+/// progress to the client. The first argument of the callback is an event enum encoded as a u8 as follows:
+/// ```
+/// enum RecoveryEvent {
+///     ConnectingToBaseNode,       // 0
+///     ConnectedToBaseNode,        // 1
+///     ConnectionToBaseNodeFailed, // 2
+///     Progress,                   // 3
+///     Completed,                  // 4
+///     ScanningRoundFailed,        // 5
+///     RecoveryFailed,             // 6
+/// }
+/// ```
+/// The second and third arguments are u64 values that will contain different information depending on the event
+/// that triggered the callback. The meaning of the second and third argument for each event are as follows:
+///     - ConnectingToBaseNode, 0, 0
+///     - ConnectedToBaseNode, 0, 1
+///     - ConnectionToBaseNodeFailed, number of retries, retry limit
+///     - Progress, current block, total number of blocks
+///     - Completed, total number of UTXO's scanned, MicroTari recovered,
+///     - ScanningRoundFailed, number of retries, retry limit
+///     - RecoveryFailed, 0, 0
+///
+/// If connection to a base node is successful the flow of callbacks should be:
+///     - The process will start with a callback with `ConnectingToBaseNode` showing a connection is being attempted
+///       this could be repeated multiple times until a connection is made.
+///     - The next a callback with `ConnectedToBaseNode` indicate a successful base node connection and process has
+///       started
+///     - In Progress callbacks will be of the form (n, m) where n < m
+///     - If the process completed successfully then the final `Completed` callback will return how many UTXO's were
+///       scanned and how much MicroTari was recovered
+///     - If there is an error in the connection process then the `ConnectionToBaseNodeFailed` will be returned
+///     - If there is a minor error in scanning then `ScanningRoundFailed` will be returned and another connection/sync
+///       attempt will be made
+///     - If a unrecoverable error occurs the `RecoveryFailed` event will be returned and the client will need to start
+///       a new process.
+///
 /// `error_out` - Pointer to an int which will be modified to an error code should one occur, may not be null. Functions
 /// as an out parameter.
 ///
@@ -5286,7 +5316,7 @@ pub unsafe extern "C" fn wallet_is_recovery_in_progress(wallet: *mut TariWallet,
 pub unsafe extern "C" fn wallet_start_recovery(
     wallet: *mut TariWallet,
     base_node_public_key: *mut TariPublicKey,
-    recovery_progress_callback: unsafe extern "C" fn(u64, u64),
+    recovery_progress_callback: unsafe extern "C" fn(u8, u64, u64),
     error_out: *mut c_int,
 ) -> bool {
     let mut error = 0;

base_layer/wallet_ffi/src/tasks.rs

@@ -28,16 +28,37 @@ use tokio::{sync::broadcast, task::JoinHandle};
 
 const LOG_TARGET: &str = "wallet_ffi";
 
+/// Events that the recovery process will report via the callback
+enum RecoveryEvent {
+    ConnectingToBaseNode,       // 0
+    ConnectedToBaseNode,        // 1
+    ConnectionToBaseNodeFailed, // 2
+    Progress,                   // 3
+    Completed,                  // 4
+    ScanningRoundFailed,        // 5
+    RecoveryFailed,             // 6
+}
+
 pub async fn recovery_event_monitoring(
     mut event_stream: broadcast::Receiver<UtxoScannerEvent>,
     recovery_join_handle: JoinHandle<Result<(), WalletError>>,
-    recovery_progress_callback: unsafe extern "C" fn(u64, u64),
+    recovery_progress_callback: unsafe extern "C" fn(u8, u64, u64),
 ) {
     while let Some(event) = event_stream.next().await {
         match event {
+            Ok(UtxoScannerEvent::ConnectingToBaseNode(peer)) => {
+                unsafe {
+                    (recovery_progress_callback)(RecoveryEvent::ConnectingToBaseNode as u8, 0u64, 0u64);
+                }
+                info!(
+                    target: LOG_TARGET,
+                    "Attempting connection to base node {}",
+                    peer.to_hex(),
+                );
+            },
             Ok(UtxoScannerEvent::ConnectedToBaseNode(pk, elapsed)) => {
                 unsafe {
-                    (recovery_progress_callback)(0u64, 1u64);
+                    (recovery_progress_callback)(RecoveryEvent::ConnectedToBaseNode as u8, 0u64, 1u64);
                 }
                 info!(
                     target: LOG_TARGET,
@@ -46,12 +67,32 @@ pub async fn recovery_event_monitoring(
                     elapsed
                 );
             },
+            Ok(UtxoScannerEvent::ConnectionFailedToBaseNode {
+                peer,
+                num_retries,
+                retry_limit,
+                error,
+            }) => {
+                unsafe {
+                    (recovery_progress_callback)(
+                        RecoveryEvent::ConnectionToBaseNodeFailed as u8,
+                        num_retries as u64,
+                        retry_limit as u64,
+                    );
+                }
+                warn!(
+                    target: LOG_TARGET,
+                    "Failed to connect to base node {} with error {}",
+                    peer.to_hex(),
+                    error
+                );
+            },
             Ok(UtxoScannerEvent::Progress {
                 current_block: current,
                 current_chain_height: total,
             }) => {
                 unsafe {
-                    (recovery_progress_callback)(current, total);
+                    (recovery_progress_callback)(RecoveryEvent::Progress as u8, current, total);
                 }
                 info!(target: LOG_TARGET, "Recovery progress: {}/{}", current, total);
                 if current == total {
@@ -74,9 +115,31 @@ pub async fn recovery_event_monitoring(
                     num_utxos,
                     total_amount
                 );
+                unsafe {
+                    (recovery_progress_callback)(RecoveryEvent::Completed as u8, num_scanned, u64::from(total_amount));
+                }
             },
-            Ok(event) => {
-                debug!(target: LOG_TARGET, "Recovery event {:?}", event);
+            Ok(UtxoScannerEvent::ScanningRoundFailed {
+                num_retries,
+                retry_limit,
+            }) => {
+                unsafe {
+                    (recovery_progress_callback)(
+                        RecoveryEvent::ScanningRoundFailed as u8,
+                        num_retries as u64,
+                        retry_limit as u64,
+                    );
+                }
+                info!(
+                    target: LOG_TARGET,
+                    "UTXO Scanning round failed on retry {} of {}", num_retries, retry_limit
+                );
+            },
+            Ok(UtxoScannerEvent::ScanningFailed) => {
+                unsafe {
+                    (recovery_progress_callback)(RecoveryEvent::RecoveryFailed as u8, 0u64, 0u64);
+                }
+                warn!(target: LOG_TARGET, "UTXO Scanner failed and exited",);
             },
             Err(e) => {
                 // Event lagging
@@ -90,13 +153,13 @@ pub async fn recovery_event_monitoring(
         Ok(Ok(_)) => {},
         Ok(Err(e)) => {
             unsafe {
-                (recovery_progress_callback)(0u64, 0u64);
+                (recovery_progress_callback)(RecoveryEvent::RecoveryFailed as u8, 0u64, 1u64);
             }
-            error!(target: LOG_TARGET, "Recovery error: {}", e);
+            error!(target: LOG_TARGET, "Recovery error: {:?}", e);
         },
         Err(e) => {
             unsafe {
-                (recovery_progress_callback)(0u64, 0u64);
+                (recovery_progress_callback)(RecoveryEvent::RecoveryFailed as u8, 1u64, 0u64);
             }
             error!(target: LOG_TARGET, "Recovery error: {}", e);
         },

base_layer/wallet_ffi/wallet.h

@@ -671,18 +671,48 @@ bool wallet_clear_value(struct TariWallet *wallet, const char* key, int* error_o
 /// None
 bool wallet_is_recovery_in_progress(struct TariWallet *wallet, int* error_out);
 
-/// Check if a Wallet has the data of an In Progress Recovery in its database.
+/// Starts the Wallet recovery process.
 ///
 /// ## Arguments
 /// `wallet` - The TariWallet pointer.
 /// `base_node_public_key` - The TariPublicKey pointer of the Base Node the recovery process will use
 /// `recovery_progress_callback` - The callback function pointer that will be used to asynchronously communicate
-/// progress to the client. The first argument is the current block the process has completed and the second argument is
-/// the total chain height. When the current block reaches the total chain height the process is complete.
-///     - The first callback with arguments (0,1) indicate a successful base node connection and process has started
-///     - In progress callbacks will be of the form (n, m) where n < m
-///     - If the process completed successfully then the final callback will have arguments (x, x) where x == x
-///     - If there is an error in the process then the final callback will be called with zero arguments i.e. (0, 0)
+/// progress to the client. The first argument of the callback is an event enum encoded as a u8 as follows:
+/// ```
+/// enum RecoveryEvent {
+///     ConnectingToBaseNode,       // 0
+///     ConnectedToBaseNode,        // 1
+///     ConnectionToBaseNodeFailed, // 2
+///     Progress,                   // 3
+///     Completed,                  // 4
+///     ScanningRoundFailed,        // 5
+///     RecoveryFailed,             // 6
+/// }
+/// ```
+/// The second and third arguments are u64 values that will contain different information depending on the event
+/// that triggered the callback. The meaning of the second and third argument for each event are as follows:
+///     - ConnectingToBaseNode, 0, 0
+///     - ConnectedToBaseNode, 0, 1
+///     - ConnectionToBaseNodeFailed, number of retries, retry limit
+///     - Progress, current block, total number of blocks
+///     - Completed, total number of UTXO's scanned, MicroTari recovered,
+///     - ScanningRoundFailed, number of retries, retry limit
+///     - RecoveryFailed, 0, 0
+///
+/// If connection to a base node is successful the flow of callbacks should be:
+///     - The process will start with a callback with `ConnectingToBaseNode` showing a connection is being attempted
+///       this could be repeated multiple times until a connection is made.
+///     - The next a callback with `ConnectedToBaseNode` indicate a successful base node connection and process has
+///       started
+///     - In Progress callbacks will be of the form (n, m) where n < m
+///     - If the process completed successfully then the final `Completed` callback will return how many UTXO's were
+///       scanned and how much MicroTari was recovered
+///     - If there is an error in the connection process then the `ConnectionToBaseNodeFailed` will be returned
+///     - If there is a minor error in scanning then `ScanningRoundFailed` will be returned and another connection/sync
+///       attempt will be made
+///     - If a unrecoverable error occurs the `RecoveryFailed` event will be returned and the client will need to start
+///       a new process.
+///
 /// `error_out` - Pointer to an int which will be modified to an error code should one occur, may not be null. Functions
 /// as an out parameter.
 ///
@@ -693,7 +723,7 @@ bool wallet_is_recovery_in_progress(struct TariWallet *wallet, int* error_out);
 ///
 /// # Safety
 /// None
-bool wallet_start_recovery(struct TariWallet *wallet, struct TariPublicKey *base_node_public_key, void (*recovery_progress_callback)(unsigned long long, unsigned long long), int* error_out);
+bool wallet_start_recovery(struct TariWallet *wallet, struct TariPublicKey *base_node_public_key, void (*recovery_progress_callback)(unsigned char, unsigned long long, unsigned long long), int* error_out);
 
 // Frees memory for a TariWallet
 void wallet_destroy(struct TariWallet *wallet);