fix: allow resynchronizing while synchronizing

wallet/src/actors/worker/handlers/sync.rs

@@ -17,6 +17,6 @@ impl Handler<SyncRequest> for worker::Worker {
     type Result = <SyncRequest as Message>::Result;
 
     fn handle(&mut self, msg: SyncRequest, _ctx: &mut Self::Context) -> Self::Result {
-        self.sync(&msg.wallet_id, msg.wallet, msg.sink, false)
+        self.sync(&msg.wallet_id, &msg.wallet, msg.sink, false)
     }
 }

wallet/src/actors/worker/methods.rs

@@ -604,7 +604,7 @@ impl Worker {
     pub fn sync(
         &self,
         wallet_id: &str,
-        wallet: types::SessionWallet,
+        wallet: &types::SessionWallet,
         sink: types::DynamicSink,
         resynchronizing: bool,
     ) -> Result<()> {
@@ -964,7 +964,7 @@ impl Worker {
                 )
                 .ok();
 
-                self.sync(&wallet.id, wallet.clone(), sink, false)?
+                self.sync(&wallet.id, &wallet, sink, false)?
             }
         }
 
@@ -1061,10 +1061,17 @@ impl Worker {
         wallet: types::SessionWallet,
         sink: DynamicSink,
     ) -> Result<bool> {
-        // Only trigger sync of chain if data has been cleared
-        match wallet.clear_chain_data()? {
-            true => self.sync(wallet_id, wallet, sink, true).map(|_| true),
-            false => Ok(false),
+        // Do not try to clear chain data and resync if a resynchronization is already in progress
+        if !wallet.is_resyncing()? {
+            wallet.clear_chain_data()?;
+
+            wallet.lock_and_update_state(|mut state| state.synchronization.set_resyncing(true))?;
+            let sync_result = self.sync(wallet_id, &wallet, sink, true).map(|_| true);
+            wallet.lock_and_update_state(|mut state| state.synchronization.set_resyncing(false))?;
+
+            sync_result
+        } else {
+            Ok(false)
         }
     }
 }

wallet/src/repository/wallet/mod.rs

@@ -1,12 +1,13 @@
 use std::{
+    cmp::min,
     collections::HashMap,
     convert::TryFrom,
+    ops::Range,
     str::FromStr,
-    sync::{Arc, RwLock},
+    sync::{Arc, RwLock, RwLockWriteGuard},
 };
 
 use state::State;
-
 use witnet_crypto::hash::calculate_sha256;
 use witnet_data_structures::{
     chain::{CheckpointBeacon, Environment, Epoch, EpochConstants, PublicKeyHash},
@@ -23,8 +24,6 @@ use crate::{
 };
 
 use super::*;
-use std::cmp::min;
-use std::ops::Range;
 
 mod state;
 #[cfg(test)]
@@ -241,6 +240,7 @@ where
             db_movements_to_update: Default::default(),
             transient_external_addresses: Default::default(),
             transient_internal_addresses: Default::default(),
+            synchronization: Default::default(),
         });
 
         Ok(Self {
@@ -1660,21 +1660,24 @@ where
     /// - Balances
     /// - Movements
     /// - Addresses and their metadata
-    ///
-    /// In order to prevent data race conditions, resyncing is not allowed while a sync or resync
-    /// process is already in progress. Accordingly, this function returns whether chain data has
-    /// been cleared or not.
-    pub fn clear_chain_data(&self) -> Result<bool> {
+    pub fn clear_chain_data(&self) -> Result<()> {
         let mut state = self.state.write()?;
+        state.clear_chain_data(&self.params.genesis_prev_hash);
+
+        Ok(())
+    }
 
-        // Prevent chain data from being cleared if a sync or resync process is in progress
-        if !state.is_syncing() {
-            state.clear_chain_data(&self.params.genesis_prev_hash);
+    /// Run a predicate on the state of a wallet in a thread safe manner, thanks to a write lock.
+    pub fn lock_and_update_state<P, O>(&self, predicate: P) -> Result<O>
+    where
+        P: FnOnce(RwLockWriteGuard<'_, State>) -> O,
+    {
+        Ok(predicate(self.state.write()?))
+    }
 
-            Ok(true)
-        } else {
-            Ok(false)
-        }
+    /// Tell whether a wallet is resynchronizing.
+    pub fn is_resyncing(&self) -> Result<bool> {
+        Ok(self.state.read()?.synchronization.is_resyncing())
     }
 }
 

wallet/src/repository/wallet/state.rs

@@ -18,6 +18,10 @@ pub struct StateSnapshot {
 /// A single wallet state. It includes:
 ///  - fields required to operate wallet accounts (e.g. derive addresses)
 ///  - on-memory state after indexing pending block transactions
+///
+/// TODO: refactor all synchronization-related fields (e.g. transient addresses) from `State` into
+///  `SynchronizationState` structure so that the number of fields in `State` does not keep
+///  growing.
 #[derive(Debug)]
 pub struct State {
     /// Current account index
@@ -70,6 +74,8 @@ pub struct State {
     pub transient_internal_addresses: HashMap<types::PublicKeyHash, model::Address>,
     /// Transient external addresses
     pub transient_external_addresses: HashMap<types::PublicKeyHash, model::Address>,
+    /// Synchronization info and status
+    pub synchronization: SynchronizationState,
 }
 
 impl State {
@@ -80,6 +86,7 @@ impl State {
     /// - Balances
     /// - Movements
     /// - Addresses and their metadata
+    ///
     pub fn clear_chain_data(&mut self, genesis_prev_hash: &Hash) {
         self.balance = Default::default();
         self.last_confirmed = CheckpointBeacon {
@@ -103,17 +110,37 @@ impl State {
         self.transient_internal_addresses.clear();
         self.transient_external_addresses.clear();
     }
+}
 
-    /// Tell whether the wallet is undergoing a synchronization.
-    ///
-    /// Currently, this function derives the information from the presence or absence of transient
-    /// addresses, i.e. addresses that are temporarily generated during the synchronization process.
-    ///
-    /// TODO: refactor all synchronization-related fields into a `SynchronizationState` structure
-    ///  so that the number of fields in `State` does not keep growing, and take that as a chance
-    ///  to add a private `is_syncing` field for which this would act as a getter.
-    pub fn is_syncing(&self) -> bool {
-        !(self.transient_internal_addresses.is_empty()
-            && self.transient_external_addresses.is_empty())
+/// The synchronization state and information for a wallet.
+///
+/// TODO: refactor all synchronization-related fields (e.g. transient addresses) from `State` into
+///  `SynchronizationState` structure so that the number of fields in `State` does not keep
+///  growing.
+#[derive(Debug)]
+pub struct SynchronizationState {
+    is_resyncing: bool,
+}
+
+impl SynchronizationState {
+    /// Tell whether a wallet is resyncing.
+    pub fn is_resyncing(&self) -> bool {
+        self.is_resyncing
+    }
+
+    /// Set the resynchronization status. Returns the old status.
+    pub fn set_resyncing(&mut self, new_status: bool) -> bool {
+        let old_status = self.is_resyncing;
+        self.is_resyncing = new_status;
+
+        old_status
+    }
+}
+
+impl Default for SynchronizationState {
+    fn default() -> Self {
+        Self {
+            is_resyncing: false,
+        }
     }
 }