fix!: fix tx and txo validation callback bug in wallet FFI

base_layer/wallet/src/output_manager_service/handle.rs

@@ -232,32 +232,20 @@ pub type OutputManagerEventReceiver = broadcast::Receiver<Arc<OutputManagerEvent
 /// Events that can be published on the Output Manager Service Event Stream
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum OutputManagerEvent {
-    TxoValidationTimedOut(u64),
     TxoValidationSuccess(u64),
     TxoValidationFailure(u64),
-    TxoValidationAborted(u64),
-    TxoValidationDelayed(u64),
     Error(String),
 }
 
 impl fmt::Display for OutputManagerEvent {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         match self {
-            OutputManagerEvent::TxoValidationTimedOut(tx) => {
-                write!(f, "TxoValidationTimedOut for {}", tx)
-            },
             OutputManagerEvent::TxoValidationSuccess(tx) => {
                 write!(f, "TxoValidationSuccess for {}", tx)
             },
             OutputManagerEvent::TxoValidationFailure(tx) => {
                 write!(f, "TxoValidationFailure for {}", tx)
             },
-            OutputManagerEvent::TxoValidationAborted(tx) => {
-                write!(f, "TxoValidationAborted for {}", tx)
-            },
-            OutputManagerEvent::TxoValidationDelayed(tx) => {
-                write!(f, "TxoValidationDelayed for {}", tx)
-            },
             OutputManagerEvent::Error(error) => {
                 write!(f, "Error {}", error)
             },

base_layer/wallet/src/output_manager_service/service.rs

@@ -71,7 +71,7 @@ use crate::{
     output_manager_service::{
         config::OutputManagerServiceConfig,
         error::{OutputManagerError, OutputManagerProtocolError, OutputManagerStorageError},
-        handle::{OutputManagerEventSender, OutputManagerRequest, OutputManagerResponse},
+        handle::{OutputManagerEvent, OutputManagerEventSender, OutputManagerRequest, OutputManagerResponse},
         recovery::StandardUtxoRecoverer,
         resources::OutputManagerResources,
         storage::{
@@ -474,7 +474,7 @@ where
         );
 
         let shutdown = self.resources.shutdown_signal.clone();
-
+        let event_publisher = self.resources.event_publisher.clone();
         tokio::spawn(async move {
             match utxo_validation.execute(shutdown).await {
                 Ok(id) => {
@@ -488,6 +488,12 @@ where
                         target: LOG_TARGET,
                         "Error completing UTXO Validation Protocol (Id: {}): {:?}", id, error
                     );
+                    if let Err(e) = event_publisher.send(Arc::new(OutputManagerEvent::TxoValidationFailure(id))) {
+                        debug!(
+                            target: LOG_TARGET,
+                            "Error sending event because there are no subscribers: {:?}", e
+                        );
+                    }
                 },
             }
         });

base_layer/wallet/src/transaction_service/handle.rs

@@ -201,6 +201,8 @@ pub enum TransactionEvent {
         is_valid: bool,
     },
     TransactionValidationStateChanged(OperationId),
+    TransactionValidationCompleted(OperationId),
+    TransactionValidationFailed(OperationId),
     Error(String),
 }
 
@@ -263,6 +265,12 @@ impl fmt::Display for TransactionEvent {
             TransactionEvent::TransactionValidationStateChanged(operation_id) => {
                 write!(f, "Transaction validation state changed: {}", operation_id)
             },
+            TransactionEvent::TransactionValidationCompleted(operation_id) => {
+                write!(f, "Transaction validation completed: {}", operation_id)
+            },
+            TransactionEvent::TransactionValidationFailed(operation_id) => {
+                write!(f, "Transaction validation failed: {}", operation_id)
+            },
         }
     }
 }

base_layer/wallet/src/transaction_service/protocols/transaction_validation_protocol.rs

@@ -191,6 +191,7 @@ where
         if state_changed {
             self.publish_event(TransactionEvent::TransactionValidationStateChanged(self.operation_id));
         }
+        self.publish_event(TransactionEvent::TransactionValidationCompleted(self.operation_id));
         Ok(self.operation_id)
     }
 

base_layer/wallet/src/transaction_service/service.rs

@@ -1860,7 +1860,9 @@ where
                 );
                 let _ = self
                     .event_publisher
-                    .send(Arc::new(TransactionEvent::Error(format!("{:?}", error))));
+                    .send(Arc::new(TransactionEvent::TransactionValidationFailed(
+                        OperationId::from(id),
+                    )));
             },
         }
     }

base_layer/wallet_ffi/src/callback_handler.rs

@@ -69,14 +69,6 @@ use tari_wallet::{
 
 const LOG_TARGET: &str = "wallet::transaction_service::callback_handler";
 
-#[derive(Clone, Copy)]
-enum CallbackValidationResults {
-    Success,           // 0
-    Aborted,           // 1
-    Failure,           // 2
-    BaseNodeNotInSync, // 3
-}
-
 pub struct CallbackHandler<TBackend>
 where TBackend: TransactionBackend + 'static
 {
@@ -89,9 +81,9 @@ where TBackend: TransactionBackend + 'static
     callback_direct_send_result: unsafe extern "C" fn(u64, bool),
     callback_store_and_forward_send_result: unsafe extern "C" fn(u64, bool),
     callback_transaction_cancellation: unsafe extern "C" fn(*mut CompletedTransaction, u64),
-    callback_txo_validation_complete: unsafe extern "C" fn(u64, u8),
+    callback_txo_validation_complete: unsafe extern "C" fn(u64, bool),
     callback_balance_updated: unsafe extern "C" fn(*mut Balance),
-    callback_transaction_validation_complete: unsafe extern "C" fn(u64, u8),
+    callback_transaction_validation_complete: unsafe extern "C" fn(u64, bool),
     callback_saf_messages_received: unsafe extern "C" fn(),
     db: TransactionDatabase<TBackend>,
     transaction_service_event_stream: TransactionEventReceiver,
@@ -124,9 +116,9 @@ where TBackend: TransactionBackend + 'static
         callback_direct_send_result: unsafe extern "C" fn(u64, bool),
         callback_store_and_forward_send_result: unsafe extern "C" fn(u64, bool),
         callback_transaction_cancellation: unsafe extern "C" fn(*mut CompletedTransaction, u64),
-        callback_txo_validation_complete: unsafe extern "C" fn(u64, u8),
+        callback_txo_validation_complete: unsafe extern "C" fn(u64, bool),
         callback_balance_updated: unsafe extern "C" fn(*mut Balance),
-        callback_transaction_validation_complete: unsafe extern "C" fn(u64, u8),
+        callback_transaction_validation_complete: unsafe extern "C" fn(u64, bool),
         callback_saf_messages_received: unsafe extern "C" fn(),
     ) -> Self {
         info!(
@@ -258,10 +250,15 @@ where TBackend: TransactionBackend + 'static
                                     self.receive_transaction_mined_unconfirmed_event(tx_id, num_confirmations).await;
                                     self.trigger_balance_refresh().await;
                                 },
-                                TransactionEvent::TransactionValidationStateChanged(request_key)  => {
-                                    self.transaction_validation_complete_event(request_key.as_u64());
+                                TransactionEvent::TransactionValidationStateChanged(_request_key)  => {
                                     self.trigger_balance_refresh().await;
                                 },
+                                TransactionEvent::TransactionValidationCompleted(request_key)  => {
+                                    self.transaction_validation_complete_event(request_key.as_u64(), true);
+                                },
+                                TransactionEvent::TransactionValidationFailed(request_key)  => {
+                                    self.transaction_validation_complete_event(request_key.as_u64(), false);
+                                },
                                 TransactionEvent::TransactionMinedRequestTimedOut(_tx_id) |
                                 TransactionEvent::TransactionImported(_tx_id) |
                                 TransactionEvent::TransactionCompletedImmediately(_tx_id)
@@ -281,17 +278,11 @@ where TBackend: TransactionBackend + 'static
                             trace!(target: LOG_TARGET, "Output Manager Service Callback Handler event {:?}", msg);
                             match (*msg).clone() {
                                 OutputManagerEvent::TxoValidationSuccess(request_key) => {
-                                    self.output_validation_complete_event(request_key,  CallbackValidationResults::Success);
+                                    self.output_validation_complete_event(request_key,  true);
                                     self.trigger_balance_refresh().await;
                                 },
                                 OutputManagerEvent::TxoValidationFailure(request_key) => {
-                                    self.output_validation_complete_event(request_key,  CallbackValidationResults::Failure);
-                                },
-                                OutputManagerEvent::TxoValidationAborted(request_key) => {
-                                    self.output_validation_complete_event(request_key,  CallbackValidationResults::Aborted);
-                                },
-                                OutputManagerEvent::TxoValidationDelayed(request_key) => {
-                                    self.output_validation_complete_event(request_key,  CallbackValidationResults::BaseNodeNotInSync);
+                                    self.output_validation_complete_event(request_key,  false);
                                 },
                                 // Only the above variants are mapped to callbacks
                                 _ => (),
@@ -496,40 +487,26 @@ where TBackend: TransactionBackend + 'static
         }
     }
 
-    fn transaction_validation_complete_event(&mut self, request_key: u64) {
+    fn transaction_validation_complete_event(&mut self, request_key: u64, success: bool) {
         debug!(
             target: LOG_TARGET,
             "Calling Transaction Validation Complete callback function for Request Key: {}", request_key,
         );
         unsafe {
-            (self.callback_transaction_validation_complete)(request_key, CallbackValidationResults::Success as u8);
+            (self.callback_transaction_validation_complete)(request_key, success);
         }
     }
 
-    fn output_validation_complete_event(&mut self, request_key: u64, result: CallbackValidationResults) {
+    fn output_validation_complete_event(&mut self, request_key: u64, success: bool) {
         debug!(
             target: LOG_TARGET,
-            "Calling Output Validation Complete callback function for Request Key: {} with result {:?}",
+            "Calling Output Validation Complete callback function for Request Key: {} with success = {:?}",
             request_key,
-            result as u8,
+            success,
         );
 
-        match result {
-            CallbackValidationResults::Success => unsafe {
-                (self.callback_txo_validation_complete)(request_key, CallbackValidationResults::Success as u8);
-            },
-            CallbackValidationResults::Aborted => unsafe {
-                (self.callback_txo_validation_complete)(request_key, CallbackValidationResults::Aborted as u8);
-            },
-            CallbackValidationResults::Failure => unsafe {
-                (self.callback_txo_validation_complete)(request_key, CallbackValidationResults::Failure as u8);
-            },
-            CallbackValidationResults::BaseNodeNotInSync => unsafe {
-                (self.callback_txo_validation_complete)(
-                    request_key,
-                    CallbackValidationResults::BaseNodeNotInSync as u8,
-                );
-            },
+        unsafe {
+            (self.callback_txo_validation_complete)(request_key, success);
         }
     }
 

base_layer/wallet_ffi/src/callback_handler_tests.rs

@@ -181,7 +181,7 @@ mod test {
         Box::from_raw(tx);
     }
 
-    unsafe extern "C" fn txo_validation_complete_callback(_tx_id: u64, result: u8) {
+    unsafe extern "C" fn txo_validation_complete_callback(_tx_id: u64, result: bool) {
         let mut lock = CALLBACK_STATE.lock().unwrap();
         lock.callback_txo_validation_complete += result as u32;
         drop(lock);
@@ -194,7 +194,7 @@ mod test {
         Box::from_raw(balance);
     }
 
-    unsafe extern "C" fn transaction_validation_complete_callback(request_key: u64, _result: u8) {
+    unsafe extern "C" fn transaction_validation_complete_callback(request_key: u64, _result: bool) {
         let mut lock = CALLBACK_STATE.lock().unwrap();
         lock.callback_transaction_validation_complete += request_key as u32;
         drop(lock);
@@ -485,58 +485,22 @@ mod test {
             )))
             .unwrap();
 
-        oms_event_sender
-            .send(Arc::new(OutputManagerEvent::TxoValidationFailure(1u64)))
-            .unwrap();
-
-        oms_event_sender
-            .send(Arc::new(OutputManagerEvent::TxoValidationFailure(1u64)))
-            .unwrap();
-
-        oms_event_sender
-            .send(Arc::new(OutputManagerEvent::TxoValidationFailure(1u64)))
-            .unwrap();
-
         transaction_event_sender
             .send(Arc::new(TransactionEvent::TransactionValidationStateChanged(
                 2u64.into(),
             )))
             .unwrap();
 
         oms_event_sender
-            .send(Arc::new(OutputManagerEvent::TxoValidationAborted(1u64)))
-            .unwrap();
-
-        oms_event_sender
-            .send(Arc::new(OutputManagerEvent::TxoValidationAborted(1u64)))
-            .unwrap();
-
-        oms_event_sender
-            .send(Arc::new(OutputManagerEvent::TxoValidationAborted(1u64)))
+            .send(Arc::new(OutputManagerEvent::TxoValidationFailure(1u64)))
             .unwrap();
 
         transaction_event_sender
-            .send(Arc::new(TransactionEvent::TransactionValidationStateChanged(
-                3u64.into(),
-            )))
-            .unwrap();
-
-        oms_event_sender
-            .send(Arc::new(OutputManagerEvent::TxoValidationDelayed(1u64)))
-            .unwrap();
-
-        oms_event_sender
-            .send(Arc::new(OutputManagerEvent::TxoValidationDelayed(1u64)))
-            .unwrap();
-
-        oms_event_sender
-            .send(Arc::new(OutputManagerEvent::TxoValidationDelayed(1u64)))
+            .send(Arc::new(TransactionEvent::TransactionValidationCompleted(3u64.into())))
             .unwrap();
 
         transaction_event_sender
-            .send(Arc::new(TransactionEvent::TransactionValidationStateChanged(
-                4u64.into(),
-            )))
+            .send(Arc::new(TransactionEvent::TransactionValidationCompleted(4u64.into())))
             .unwrap();
 
         dht_event_sender
@@ -558,9 +522,9 @@ mod test {
         assert!(lock.tx_cancellation_callback_called_completed);
         assert!(lock.tx_cancellation_callback_called_outbound);
         assert!(lock.saf_messages_received);
-        assert_eq!(lock.callback_txo_validation_complete, 18);
+        assert_eq!(lock.callback_txo_validation_complete, 3);
         assert_eq!(lock.callback_balance_updated, 5);
-        assert_eq!(lock.callback_transaction_validation_complete, 10);
+        assert_eq!(lock.callback_transaction_validation_complete, 7);
 
         drop(lock);
     }

base_layer/wallet_ffi/src/lib.rs

@@ -3144,12 +3144,12 @@ unsafe fn init_logging(
 /// }
 /// `callback_txo_validation_complete` - The callback function pointer matching the function signature. This is called
 /// when a TXO validation process is completed. The request_key is used to identify which request this
-/// callback references and the second parameter is a u8 that represent the ClassbackValidationResults enum.
+/// callback references and the second parameter is a is a bool that returns if the validation was successful or not.
 /// `callback_balance_updated` - The callback function pointer matching the function signature. This is called whenever
 /// the balance changes.
 /// `callback_transaction_validation_complete` - The callback function pointer matching the function signature. This is
 /// called when a Transaction validation process is completed. The request_key is used to identify which request this
-/// callback references and the second parameter is a u8 that represent the ClassbackValidationResults enum.
+/// callback references and the second parameter is a bool that returns if the validation was successful or not.
 /// `callback_saf_message_received` - The callback function pointer that will be called when the Dht has determined that
 /// is has connected to enough of its neighbours to be confident that it has received any SAF messages that were waiting
 /// for it.
@@ -3181,9 +3181,9 @@ pub unsafe extern "C" fn wallet_create(
     callback_direct_send_result: unsafe extern "C" fn(c_ulonglong, bool),
     callback_store_and_forward_send_result: unsafe extern "C" fn(c_ulonglong, bool),
     callback_transaction_cancellation: unsafe extern "C" fn(*mut TariCompletedTransaction, u64),
-    callback_txo_validation_complete: unsafe extern "C" fn(u64, u8),
+    callback_txo_validation_complete: unsafe extern "C" fn(u64, bool),
     callback_balance_updated: unsafe extern "C" fn(*mut TariBalance),
-    callback_transaction_validation_complete: unsafe extern "C" fn(u64, u8),
+    callback_transaction_validation_complete: unsafe extern "C" fn(u64, bool),
     callback_saf_messages_received: unsafe extern "C" fn(),
     recovery_in_progress: *mut bool,
     error_out: *mut c_int,
@@ -6044,15 +6044,15 @@ mod test {
         completed_transaction_destroy(tx);
     }
 
-    unsafe extern "C" fn txo_validation_complete_callback(_tx_id: c_ulonglong, _result: u8) {
+    unsafe extern "C" fn txo_validation_complete_callback(_tx_id: c_ulonglong, _result: bool) {
         // assert!(true); //optimized out by compiler
     }
 
     unsafe extern "C" fn balance_updated_callback(_balance: *mut TariBalance) {
         // assert!(true); //optimized out by compiler
     }
 
-    unsafe extern "C" fn transaction_validation_complete_callback(_tx_id: c_ulonglong, _result: u8) {
+    unsafe extern "C" fn transaction_validation_complete_callback(_tx_id: c_ulonglong, _result: bool) {
         // assert!(true); //optimized out by compiler
     }