rework save so it does not double memory size and also can be updated from within pepper sync

Author: Oscar-Pepper

zingolib/src/commands.rs

@@ -23,7 +23,7 @@ mod error;
 mod utils;
 
 lazy_static! {
-    static ref RT: Runtime = tokio::runtime::Runtime::new().unwrap();
+    pub static ref RT: Runtime = tokio::runtime::Runtime::new().unwrap();
 }
 
 /// This command interface is used both by cli and also consumers.

zingolib/src/lightclient.rs

@@ -2,10 +2,7 @@
 
 use json::{array, object, JsonValue};
 use log::error;
-use pepper_sync::{
-    error::SyncError,
-    wallet::{SyncMode, SyncResult},
-};
+use pepper_sync::{error::SyncError, wallet::SyncResult};
 use serde::Serialize;
 use std::sync::{atomic::AtomicU8, Arc};
 use tokio::{
@@ -159,19 +156,6 @@ pub struct AccountBackupInfo {
     pub account_index: u32,
 }
 
-#[derive(Default)]
-struct ZingoSaveBuffer {
-    pub buffer: Arc<RwLock<Vec<u8>>>,
-}
-
-impl ZingoSaveBuffer {
-    fn new(buffer: Vec<u8>) -> Self {
-        ZingoSaveBuffer {
-            buffer: Arc::new(RwLock::new(buffer)),
-        }
-    }
-}
-
 /// Balances that may be presented to a user in a wallet app.
 /// The goal is to present a user-friendly and useful view of what the user has or can soon expect
 /// *without* requiring the user to understand the details of the Zcash protocol.
@@ -247,7 +231,6 @@ pub struct LightClient {
     sync_mode: Arc<AtomicU8>,
     sync_handle: Option<JoinHandle<Result<SyncResult, SyncError>>>,
     latest_proposal: Arc<RwLock<Option<ZingoProposal>>>, // TODO: move to wallet
-    save_buffer: ZingoSaveBuffer,                        // TODO: move save buffer to wallet itself?
 }
 
 /// all the wonderfully intertwined ways to conjure a LightClient
@@ -266,7 +249,7 @@ pub mod instantiation {
 
     use crate::config::ZingoConfig;
 
-    use super::{LightClient, ZingoSaveBuffer};
+    use super::LightClient;
     use crate::wallet::{LightWallet, WalletBase};
 
     impl LightClient {
@@ -285,7 +268,6 @@ pub mod instantiation {
                 sync_mode: Arc::new(AtomicU8::new(SyncMode::NotRunning as u8)),
                 sync_handle: None,
                 latest_proposal: Arc::new(RwLock::new(None)),
-                save_buffer: ZingoSaveBuffer::new(buffer),
             })
         }
 
@@ -324,7 +306,7 @@ pub mod instantiation {
                 ));
                 }
             }
-            let mut lightclient = LightClient::create_from_wallet_async(
+            let lightclient = LightClient::create_from_wallet_async(
                 config.clone(),
                 LightWallet::new(
                     config.chain,
@@ -337,10 +319,7 @@ pub mod instantiation {
             )
             .await?;
 
-            lightclient
-                .save_internal_rust()
-                .await
-                .map_err(|e| Error::new(ErrorKind::InvalidData, e))?;
+            lightclient.wallet.lock().await.save_required = true;
 
             debug!("Created new wallet!");
 
@@ -370,19 +349,16 @@ pub mod instantiation {
 
         fn create_with_new_wallet(config: &ZingoConfig, height: u64) -> io::Result<Self> {
             Runtime::new().unwrap().block_on(async move {
-                let mut l =
+                let lightclient =
                     LightClient::create_unconnected(config, WalletBase::FreshEntropy, height)
                         .await?;
 
                 debug!("Created new wallet with a new seed!");
                 debug!("Created LightClient to {}", &config.get_lightwalletd_uri());
 
-                // Save
-                l.save_internal_rust()
-                    .await
-                    .map_err(|s| io::Error::new(ErrorKind::PermissionDenied, s))?;
+                lightclient.wallet.lock().await.save_required = true;
 
-                Ok(l)
+                Ok(lightclient)
             })
         }
 
@@ -476,17 +452,13 @@ impl LightClient {
             transparent: addr_type.contains('t'),
         };
 
-        let new_address = self
-            .wallet
-            .lock()
-            .await
-            .generate_unified_address(desired_receivers)
-            .map_err(|e| e.to_string())?;
-
-        // FIXME: zingo2, rework wallet save to save while the wallet guard is acquired
-        if SyncMode::from_atomic_u8(self.sync_mode.clone()) == SyncMode::NotRunning {
-            self.save_internal_rust().await?;
-        }
+        let new_address = {
+            let mut wallet = self.wallet.lock().await;
+            wallet.save_required = true;
+            wallet
+                .generate_unified_address(desired_receivers)
+                .map_err(|e| e.to_string())?
+        };
 
         Ok(array![new_address.encode(&self.config.chain)])
     }

zingolib/src/lightclient/save.rs

@@ -5,90 +5,51 @@ use log::error;
 use std::{
     fs::{remove_file, File},
     io::Write,
-    path::{Path, PathBuf},
+    path::PathBuf,
 };
-use tokio::runtime::Runtime;
 
 use super::LightClient;
-use crate::error::{ZingoLibError, ZingoLibResult};
+use crate::{error::ZingoLibError, wallet::LightWallet};
 
 impl LightClient {
-    //        SAVE METHODS
-
-    /// Called internally at sync checkpoints to save state. Should not be called midway through sync.
-    pub(super) async fn save_internal_rust(&mut self) -> ZingoLibResult<bool> {
-        match self.save_internal_buffer().await {
-            Ok(_vu8) => {
-                // Save_internal_buffer ran without error. At this point, we assume that the save buffer is good to go. Depending on operating system, we may be able to write it to disk. (Otherwise, we wait for the FFI to offer save export.
-
-                #[cfg(not(any(target_os = "ios", target_os = "android")))]
-                {
-                    self.rust_write_save_buffer_to_file().await?;
-                    Ok(true)
-                }
-                #[cfg(any(target_os = "ios", target_os = "android"))]
-                {
-                    Ok(false)
-                }
-            }
-            Err(err) => {
-                error!("{}", err);
-                Err(err)
-            }
-        }
-    }
-
-    /// write down the state of the lightclient as a `Vec<u8>`
-    pub async fn save_internal_buffer(&mut self) -> ZingoLibResult<Vec<u8>> {
+    /// If the wallet state has changed since last save, serializes the wallet and returns the wallet bytes.
+    /// For OSs that are not iOS and Android, also persists the wallet bytes to file.
+    /// Returns `Ok(None)` if the wallet state has not changed and save is not required.
+    /// Returns error if serialization or persistance fails.
+    ///
+    /// Intended to be called from a save task which calls `save` repeatedly.
+    // FIXME: zingo-cli needs a save task
+    pub async fn save(&self, wallet: &mut LightWallet) -> std::io::Result<Option<Vec<u8>>> {
         let mut buffer: Vec<u8> = vec![];
-        let mut wallet = self.wallet.lock().await;
-        let network = wallet.network;
-        wallet
-            .write(&mut buffer, &network)
-            .await
-            .map_err(ZingoLibError::InternalWriteBufferError)?;
-        (self.save_buffer.buffer.write().await).clone_from(&buffer);
-        Ok(buffer)
-    }
-
-    #[cfg(not(any(target_os = "ios", target_os = "android")))]
-    /// If possible, write to disk.
-    async fn rust_write_save_buffer_to_file(&self) -> ZingoLibResult<()> {
-        {
-            let read_buffer = self.save_buffer.buffer.read().await;
-            if !read_buffer.is_empty() {
-                LightClient::write_to_file(self.config.get_wallet_path(), &read_buffer)
-                    .map_err(ZingoLibError::WriteFileError)?;
-                Ok(())
-            } else {
-                ZingoLibError::EmptySaveBuffer.handle()
-            }
+        if wallet.save_required {
+            let network = wallet.network;
+            wallet.write(&mut buffer, &network).await?;
+            wallet.save_required = false;
         }
-    }
 
-    #[cfg(not(any(target_os = "ios", target_os = "android")))]
-    fn write_to_file(path: Box<Path>, buffer: &[u8]) -> std::io::Result<()> {
-        let mut file = File::create(path)?;
-        file.write_all(buffer)?;
-        Ok(())
-    }
+        #[cfg(not(any(target_os = "ios", target_os = "android")))]
+        if !buffer.is_empty() {
+            let mut file = File::create(self.config.get_wallet_path())?;
+            file.write_all(&buffer)?;
+        }
 
-    /// TODO: Add Doc Comment Here!
-    pub async fn export_save_buffer_async(&self) -> ZingoLibResult<Vec<u8>> {
-        let read_buffer = self.save_buffer.buffer.read().await;
-        if !read_buffer.is_empty() {
-            Ok(read_buffer.clone())
+        if buffer.is_empty() {
+            Ok(None)
         } else {
-            ZingoLibError::EmptySaveBuffer.handle()
+            Ok(Some(buffer))
         }
     }
 
-    /// This function is the sole correct way to ask LightClient to save.
-    pub fn export_save_buffer_runtime(&self) -> Result<Vec<u8>, String> {
-        Runtime::new()
-            .unwrap()
-            .block_on(async move { self.export_save_buffer_async().await })
-            .map_err(String::from)
+    /// Calls `save` in a runtime and returns an empty buffer in the case save was not required.
+    // FIXME: zingo2, this is kept in to make zingomobile integration easier but should be moved into zingo-mobile
+    pub fn export_save_buffer_runtime(&mut self) -> Result<Vec<u8>, String> {
+        crate::commands::RT.block_on(async move {
+            match self.save(&mut *self.wallet.lock().await).await {
+                Ok(Some(wallet_bytes)) => Ok(wallet_bytes),
+                Ok(None) => Ok(vec![]),
+                Err(e) => Err(e.to_string()),
+            }
+        })
     }
 
     /// Only relevant in non-mobile, this function removes the save file.

zingolib/src/lightclient/send.rs

@@ -101,11 +101,14 @@ pub mod send_with_proposal {
             &self,
             proposal: &Proposal<zip317::FeeRule, NoteRef>,
         ) -> Result<NonEmpty<TxId>, CompleteAndBroadcastError> {
-            let mut wallet = self.wallet.lock().await;
-            let calculated_txids = wallet.create_transactions(proposal).await?;
-
-            // FIXME: zingo2, save wallet here in case send fails and tx is lost
+            let calculated_txids = {
+                let mut wallet = self.wallet.lock().await;
+                wallet.save_required = true;
+                wallet.create_transactions(proposal).await?
+            };
+            // drop wallet lock to allow save task to acquire it.
 
+            let mut wallet = self.wallet.lock().await;
             let broadcast_result = wallet
                 .broadcast_calculated_transactions(self.get_server_uri(), calculated_txids)
                 .await;

zingolib/src/testutils/lightclient.rs

@@ -1,21 +1,21 @@
 //! This mod is mostly to take inputs, raw data amd convert it into lightclient actions
 //! (obviously) in a test environment.
-use crate::{
-    error::ZingoLibError,
-    lightclient::{describe::UAReceivers, LightClient},
-};
+use crate::lightclient::{describe::UAReceivers, LightClient};
 use zcash_client_backend::{PoolType, ShieldedProtocol};
 use zcash_primitives::transaction::TxId;
 
 /// Create a lightclient from the buffer of another
 pub async fn new_client_from_save_buffer(
     template_client: &mut LightClient,
-) -> Result<LightClient, ZingoLibError> {
-    let buffer = template_client.save_internal_buffer().await?;
+) -> std::io::Result<LightClient> {
+    let mut wallet = template_client.wallet.lock().await;
+    wallet.save_required = true;
+    let buffer = template_client
+        .save(&mut wallet)
+        .await?
+        .expect("forced save_required true");
 
-    LightClient::read_wallet_from_buffer_async(template_client.config(), buffer.as_slice())
-        .await
-        .map_err(ZingoLibError::CantReadWallet)
+    LightClient::read_wallet_from_buffer_async(template_client.config(), buffer.as_slice()).await
 }
 /// gets the first address that will allow a sender to send to a specific pool, as a string
 /// calling \[0] on json may panic? not sure -fv

zingolib/src/wallet.rs

@@ -216,6 +216,12 @@ pub struct LightWallet {
     pub price: Arc<RwLock<WalletZecPriceInfo>>,
     /// Progress of an outgoing transaction
     send_progress: Arc<RwLock<SendProgress>>,
+    /// Boolean for tracking whether the wallet state has changed since last save.
+    ///
+    /// When wallet state is changed due to sync, send or creating addresses, this will be set to `true` automatically.
+    /// Calling [`crate::lightclient::LightClient::save`] will serialize (and in some cases persist) the wallet and
+    /// reset `save_required` to false.
+    pub save_required: bool,
 }
 
 impl LightWallet {
@@ -363,6 +369,7 @@ impl LightWallet {
             transparent_addresses,
             unified_addresses,
             network,
+            save_required: false,
         })
     }
 

zingolib/src/wallet/disk.rs

@@ -300,6 +300,7 @@ impl LightWallet {
             transparent_addresses: BTreeMap::new(),
             unified_addresses: BTreeMap::new(),
             network,
+            save_required: false,
         };
 
         Ok(lw)
@@ -413,6 +414,7 @@ impl LightWallet {
             wallet_options: Arc::new(RwLock::new(wallet_options)),
             price: Arc::new(RwLock::new(price)),
             send_progress: Arc::new(RwLock::new(SendProgress::new(0))),
+            save_required: false,
         })
     }
 }

zingolib/src/wallet/disk/testing/tests.rs

@@ -234,7 +234,15 @@ async fn reload_wallet_from_buffer() {
             .await;
     let mid_client_network = mid_client.wallet.lock().await.network;
 
-    let mid_buffer = mid_client.export_save_buffer_async().await.unwrap();
+    let mid_buffer = {
+        let mut wallet = mid_client.wallet.lock().await;
+        wallet.save_required = true;
+        mid_client
+            .save(&mut *wallet)
+            .await
+            .unwrap()
+            .expect("forced save_required true")
+    };
 
     let client =
         LightClient::read_wallet_from_buffer_async(&ZingoConfig::create_testnet(), &mid_buffer[..])