fix: disable P2P transaction negotiation while recovery is in progress

Some weird behaviour was observed when a wallet would be busy with recovery and then receive transaction negotiation messages, either directly or via SAF.

The Recovery process is updating the Key Manager Indices and looking for commitments on the blockchain so to allow transaction negotiation during this time is dangerous as it might put duplicate commitments into the db and reuse spending keys.

This PR checks for the db key/value used to indicate Recovery progress before handling a transaction negotiation p2p message and if it is there the message is ignored with a log.

base_layer/wallet/src/test_utils.rs

@@ -20,12 +20,7 @@
 // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
 // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-use crate::{
-    contacts_service::storage::sqlite_db::ContactsServiceSqliteDatabase,
-    output_manager_service::storage::sqlite_db::OutputManagerSqliteDatabase,
-    storage::{sqlite_db::WalletSqliteDatabase, sqlite_utilities::run_migration_and_create_sqlite_connection},
-    transaction_service::storage::sqlite_db::TransactionServiceSqliteDatabase,
-};
+use crate::storage::sqlite_utilities::{run_migration_and_create_sqlite_connection, WalletDbConnection};
 use core::iter;
 use rand::{distributions::Alphanumeric, rngs::OsRng, Rng};
 use std::path::Path;
@@ -39,15 +34,7 @@ pub fn random_string(len: usize) -> String {
 }
 
 /// A test helper to create a temporary wallet service databases
-pub fn make_wallet_databases(
-    path: Option<String>,
-) -> (
-    WalletSqliteDatabase,
-    TransactionServiceSqliteDatabase,
-    OutputManagerSqliteDatabase,
-    ContactsServiceSqliteDatabase,
-    Option<TempDir>,
-) {
+pub fn make_wallet_database_connection(path: Option<String>) -> (WalletDbConnection, Option<TempDir>) {
     let (path_string, temp_dir): (String, Option<TempDir>) = if let Some(p) = path {
         (p, None)
     } else {
@@ -61,11 +48,5 @@ pub fn make_wallet_databases(
 
     let connection =
         run_migration_and_create_sqlite_connection(&db_path.to_str().expect("Should be able to make path")).unwrap();
-    (
-        WalletSqliteDatabase::new(connection.clone(), None).expect("Should be able to create wallet database"),
-        TransactionServiceSqliteDatabase::new(connection.clone(), None),
-        OutputManagerSqliteDatabase::new(connection.clone(), None),
-        ContactsServiceSqliteDatabase::new(connection),
-        temp_dir,
-    )
+    (connection, temp_dir)
 }

base_layer/wallet/src/transaction_service/error.rs

@@ -21,6 +21,7 @@
 // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 use crate::{
+    error::WalletStorageError,
     output_manager_service::{error::OutputManagerError, TxId},
     transaction_service::storage::database::DbKey,
 };
@@ -100,6 +101,8 @@ pub enum TransactionServiceError {
     TransportChannelError(#[from] TransportChannelError),
     #[error("Transaction storage error: `{0}`")]
     TransactionStorageError(#[from] TransactionStorageError),
+    #[error("Wallet storage error: `{0}`")]
+    WalletStorageError(#[from] WalletStorageError),
     #[error("Invalid message error: `{0}`")]
     InvalidMessageError(String),
     #[error("Transaction error: `{0}`")]
@@ -140,6 +143,8 @@ pub enum TransactionServiceError {
     ByteArrayError(#[from] tari_crypto::tari_utilities::ByteArrayError),
     #[error("Transaction Service Error: `{0}`")]
     ServiceError(String),
+    #[error("Wallet Recovery in progress so Transaction Service Messaging Requests ignored")]
+    WalletRecoveryInProgress,
 }
 
 #[derive(Debug, Error)]

base_layer/wallet/src/transaction_service/mod.rs

@@ -22,6 +22,16 @@
 
 use std::sync::Arc;
 
+use crate::{
+    output_manager_service::handle::OutputManagerHandle,
+    storage::database::{WalletBackend, WalletDatabase},
+    transaction_service::{
+        config::TransactionServiceConfig,
+        handle::TransactionServiceHandle,
+        service::TransactionService,
+        storage::database::{TransactionBackend, TransactionDatabase},
+    },
+};
 use futures::{Stream, StreamExt};
 use log::*;
 use tokio::sync::broadcast;
@@ -46,16 +56,6 @@ use tari_service_framework::{
     ServiceInitializerContext,
 };
 
-use crate::{
-    output_manager_service::handle::OutputManagerHandle,
-    transaction_service::{
-        config::TransactionServiceConfig,
-        handle::TransactionServiceHandle,
-        service::TransactionService,
-        storage::database::{TransactionBackend, TransactionDatabase},
-    },
-};
-
 pub mod config;
 pub mod error;
 pub mod handle;
@@ -67,32 +67,39 @@ pub mod tasks;
 const LOG_TARGET: &str = "wallet::transaction_service";
 const SUBSCRIPTION_LABEL: &str = "Transaction Service";
 
-pub struct TransactionServiceInitializer<T>
-where T: TransactionBackend
+pub struct TransactionServiceInitializer<T, W>
+where
+    T: TransactionBackend,
+    W: WalletBackend,
 {
     config: TransactionServiceConfig,
     subscription_factory: Arc<SubscriptionFactory>,
-    backend: Option<T>,
+    tx_backend: Option<T>,
     node_identity: Arc<NodeIdentity>,
     factories: CryptoFactories,
+    wallet_database: Option<WalletDatabase<W>>,
 }
 
-impl<T> TransactionServiceInitializer<T>
-where T: TransactionBackend
+impl<T, W> TransactionServiceInitializer<T, W>
+where
+    T: TransactionBackend,
+    W: WalletBackend,
 {
     pub fn new(
         config: TransactionServiceConfig,
         subscription_factory: Arc<SubscriptionFactory>,
         backend: T,
         node_identity: Arc<NodeIdentity>,
         factories: CryptoFactories,
+        wallet_database: WalletDatabase<W>,
     ) -> Self {
         Self {
             config,
             subscription_factory,
-            backend: Some(backend),
+            tx_backend: Some(backend),
             node_identity,
             factories,
+            wallet_database: Some(wallet_database),
         }
     }
 
@@ -164,8 +171,10 @@ where T: TransactionBackend
 }
 
 #[async_trait]
-impl<T> ServiceInitializer for TransactionServiceInitializer<T>
-where T: TransactionBackend + 'static
+impl<T, W> ServiceInitializer for TransactionServiceInitializer<T, W>
+where
+    T: TransactionBackend + 'static,
+    W: WalletBackend + 'static,
 {
     async fn initialize(&mut self, context: ServiceInitializerContext) -> Result<(), ServiceInitializationError> {
         let (sender, receiver) = reply_channel::unbounded();
@@ -182,11 +191,16 @@ where T: TransactionBackend + 'static
         // Register handle before waiting for handles to be ready
         context.register_handle(transaction_handle);
 
-        let backend = self
-            .backend
+        let tx_backend = self
+            .tx_backend
             .take()
             .expect("Cannot start Transaction Service without providing a backend");
 
+        let wallet_database = self
+            .wallet_database
+            .take()
+            .expect("Cannot start Transaction Service without providing a wallet database");
+
         let node_identity = self.node_identity.clone();
         let factories = self.factories.clone();
         let config = self.config.clone();
@@ -198,7 +212,8 @@ where T: TransactionBackend + 'static
 
             let result = TransactionService::new(
                 config,
-                TransactionDatabase::new(backend),
+                TransactionDatabase::new(tx_backend),
+                wallet_database,
                 receiver,
                 transaction_stream,
                 transaction_reply_stream,