add options to sync state method returns to fix bugs with API use cases and add saving to new address generation on the condtion sync is not running

Author: Oscar-Pepper

libtonode-tests/tests/concrete.rs

@@ -1006,7 +1006,7 @@ mod fast {
         let seed_phrase = Mnemonic::<bip0039::English>::from_entropy([1; 32])
             .unwrap()
             .to_string();
-        let recipient1 = client_builder
+        let mut recipient1 = client_builder
             .build_client(seed_phrase, 0, false, regtest_network)
             .await;
         let base_transparent_receiver = "tmS9nbexug7uT8x1cMTLP1ABEyKXpMjR5F1";
@@ -2232,11 +2232,14 @@ mod slow {
         let recipient_to_faucet_amount = 5_000u64;
         // check start state
         faucet.sync_and_await().await.unwrap();
-        let wallet_height = faucet.do_wallet_last_scanned_height().await;
-        assert_eq!(
-            wallet_height.as_fixed_point_u64(0).unwrap(),
-            BASE_HEIGHT as u64
-        );
+        let wallet_fully_scanned_height = faucet
+            .wallet
+            .lock()
+            .await
+            .sync_state
+            .fully_scanned_height()
+            .unwrap();
+        assert_eq!(wallet_fully_scanned_height, BASE_HEIGHT.into());
         let three_blocks_reward = block_rewards::CANOPY
             .checked_mul(BASE_HEIGHT as u64)
             .unwrap();
@@ -2629,7 +2632,8 @@ mod slow {
                 .lock()
                 .await
                 .sync_state
-                .fully_scanned_height(),
+                .fully_scanned_height()
+                .unwrap(),
             4.into()
         );
 

libtonode-tests/tests/wallet.rs

@@ -191,10 +191,16 @@ mod load_wallet {
 
         // Without sync push server forward 2 blocks
         zingolib::testutils::increase_server_height(&regtest_manager, 2).await;
-        let client_wallet_height = faucet.do_wallet_last_scanned_height().await;
+        let client_fully_scanned_height = faucet
+            .wallet
+            .lock()
+            .await
+            .sync_state
+            .fully_scanned_height()
+            .unwrap();
 
         // Verify that wallet is still back at 6.
-        assert_eq!(client_wallet_height.as_fixed_point_u64(0).unwrap(), 8);
+        assert_eq!(client_fully_scanned_height, 8.into());
 
         // Interrupt generating send
         from_inputs::quick_send(

pepper-sync/src/sync.rs

@@ -174,6 +174,7 @@ where
         .get_sync_state()
         .unwrap()
         .highest_scanned_height()
+        .expect("scan ranges must be non-empty")
         + 1;
     let mut interval = tokio::time::interval(Duration::from_millis(50));
     interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Delay);
@@ -607,8 +608,12 @@ where
     W: SyncWallet + SyncBlocks + SyncNullifiers + SyncTransactions,
 {
     let sync_state = wallet.get_sync_state().unwrap();
-    let fully_scanned_height = sync_state.fully_scanned_height();
-    let highest_scanned_height = sync_state.highest_scanned_height();
+    let fully_scanned_height = sync_state
+        .fully_scanned_height()
+        .expect("scan ranges must be non-empty");
+    let highest_scanned_height = sync_state
+        .highest_scanned_height()
+        .expect("scan ranges must be non-empty");
     let sync_start_height = sync_state.initial_sync_state.sync_start_height;
 
     let scanned_block_range_bounds = sync_state

pepper-sync/src/sync/state.rs

@@ -94,7 +94,10 @@ pub(super) async fn update_scan_ranges(
     )?;
     set_chain_tip_scan_range(consensus_parameters, sync_state, chain_height)?;
 
-    let verification_height = sync_state.highest_scanned_height() + 1;
+    let verification_height = sync_state
+        .highest_scanned_height()
+        .expect("scan ranges must be non-empty")
+        + 1;
     if verification_height <= chain_height {
         set_verify_scan_range(sync_state, verification_height, VerifyEnd::VerifyLowest);
     }
@@ -111,7 +114,10 @@ fn merge_scanned_ranges(sync_state: &mut SyncState) {
         .enumerate()
         .filter(|(_, scan_range)| {
             scan_range.priority() == ScanPriority::Scanned
-                && scan_range.block_range().end - 1 <= sync_state.fully_scanned_height()
+                && scan_range.block_range().end - 1
+                    <= sync_state
+                        .fully_scanned_height()
+                        .expect("scan ranges must be non-empty")
         })
         .collect::<Vec<_>>();
 
@@ -649,7 +655,11 @@ pub(super) async fn set_initial_state<W>(
 ) where
     W: SyncWallet + SyncBlocks,
 {
-    let fully_scanned_height = wallet.get_sync_state().unwrap().fully_scanned_height();
+    let fully_scanned_height = wallet
+        .get_sync_state()
+        .unwrap()
+        .fully_scanned_height()
+        .expect("scan ranges must be non-empty");
     let (sync_start_sapling_tree_size, sync_start_orchard_tree_size) = final_tree_sizes(
         consensus_parameters,
         fetch_request_sender.clone(),

pepper-sync/src/wallet.rs

@@ -137,41 +137,34 @@ impl SyncState {
     }
 
     /// Returns the block height at which all blocks equal to and below this height are scanned.
-    ///
-    /// Will panic if called before scan ranges are updated for the first time.
-    pub fn fully_scanned_height(&self) -> BlockHeight {
+    /// Returns `None` if `self.scan_ranges` is empty.
+    pub fn fully_scanned_height(&self) -> Option<BlockHeight> {
         if let Some(scan_range) = self
             .scan_ranges
             .iter()
             .find(|scan_range| scan_range.priority() != ScanPriority::Scanned)
         {
-            scan_range.block_range().start - 1
+            Some(scan_range.block_range().start - 1)
         } else {
             self.scan_ranges
                 .last()
-                .expect("scan ranges always non-empty")
-                .block_range()
-                .end
-                - 1
+                .map(|range| range.block_range().end - 1)
         }
     }
 
     /// Returns the highest block height that has been scanned.
-    ///
     /// If no scan ranges have been scanned, returns the block below the wallet birthday.
-    /// Will panic if called before scan ranges are updated for the first time.
-    pub fn highest_scanned_height(&self) -> BlockHeight {
+    /// Returns `None` if `self.scan_ranges` is empty.
+    pub fn highest_scanned_height(&self) -> Option<BlockHeight> {
         if let Some(last_scanned_range) = self
             .scan_ranges
             .iter()
             .filter(|scan_range| scan_range.priority() == ScanPriority::Scanned)
             .last()
         {
-            last_scanned_range.block_range().end - 1
+            Some(last_scanned_range.block_range().end - 1)
         } else {
-            self.wallet_birthday()
-                .expect("scan ranges always non-empty")
-                - 1
+            self.wallet_birthday().map(|birthday| birthday - 1)
         }
     }
 

pepper-sync/src/wallet/serialization.rs

@@ -192,7 +192,6 @@ impl NullifierMap {
     /// Deserialize into `reader`
     pub fn read<R: Read>(mut reader: R) -> std::io::Result<Self> {
         let _version = reader.read_u8()?;
-
         let sapling = Vector::read(&mut reader, |mut r| {
             let mut nullifier_bytes = [0u8; 32];
             r.read_exact(&mut nullifier_bytes)?;

zingolib/src/commands.rs

@@ -1586,22 +1586,21 @@ struct HeightCommand {}
 impl Command for HeightCommand {
     fn help(&self) -> &'static str {
         indoc! {r#"
-            Get the latest block height that the wallet is at.
+            Returns the blockchain height at the time the wallet last requested the latest block height from the server.
+
             Usage:
             height
 
-            Pass 'true' (default) to sync to the server to get the latest block height. Pass 'false' to get the latest height in the wallet without checking with the server.
-
         "#}
     }
 
     fn short_help(&self) -> &'static str {
-        "Get the latest block height that the wallet is at"
+        "Returns the blockchain height at the time the wallet last requested the latest block height from the server."
     }
 
     fn exec(&self, _args: &[&str], lightclient: &mut LightClient) -> String {
         RT.block_on(async move {
-            object! { "height" => lightclient.do_wallet_last_scanned_height().await}.pretty(2)
+            object! { "height" => json::JsonValue::from(lightclient.wallet.lock().await.sync_state.wallet_height().map(u32::from).unwrap_or(0))}.pretty(2)
         })
     }
 }

zingolib/src/lightclient.rs

@@ -2,7 +2,10 @@
 
 use json::{array, object, JsonValue};
 use log::error;
-use pepper_sync::{error::SyncError, wallet::SyncResult};
+use pepper_sync::{
+    error::SyncError,
+    wallet::{SyncMode, SyncResult},
+};
 use serde::Serialize;
 use std::sync::{atomic::AtomicU8, Arc};
 use tokio::{
@@ -465,7 +468,7 @@ impl LightClient {
     }
 
     /// Generates a new unified address from the given `addr_type`.
-    pub async fn do_new_address(&self, addr_type: &str) -> Result<JsonValue, String> {
+    pub async fn do_new_address(&mut self, addr_type: &str) -> Result<JsonValue, String> {
         //TODO: Placeholder interface
         let desired_receivers = ReceiverSelection {
             sapling: addr_type.contains('z'),
@@ -480,7 +483,9 @@ impl LightClient {
             .generate_unified_address(desired_receivers)
             .map_err(|e| e.to_string())?;
 
-        // self.save_internal_rust().await?;
+        if SyncMode::from_atomic_u8(self.sync_mode.clone()) == SyncMode::NotRunning {
+            self.save_internal_rust().await?;
+        }
 
         Ok(array![new_address.encode(&self.config.chain)])
     }

zingolib/src/lightclient/describe.rs

@@ -216,14 +216,6 @@ impl LightClient {
         finsight::TotalValueToAddress(by_address_total)
     }
 
-    /// TODO: Add Doc Comment Here!
-    // TODO: remove
-    pub async fn do_wallet_last_scanned_height(&self) -> JsonValue {
-        json::JsonValue::from(u32::from(
-            self.wallet.lock().await.sync_state.fully_scanned_height(),
-        ))
-    }
-
     /// TODO: Add Doc Comment Here!
     pub fn get_server(&self) -> std::sync::RwLockReadGuard<http::Uri> {
         self.config.lightwalletd_uri.read().unwrap()

zingolib/src/testutils.rs

@@ -277,8 +277,15 @@ pub async fn sync_to_target_height(
 ) -> Result<(), LightClientError> {
     // sync first so ranges exist for the `fully_scanned_height` call
     client.sync_and_await().await?;
-    while u32::from(client.wallet.lock().await.sync_state.fully_scanned_height())
-        < target_block_height
+    while u32::from(
+        client
+            .wallet
+            .lock()
+            .await
+            .sync_state
+            .fully_scanned_height()
+            .unwrap(),
+    ) < target_block_height
     {
         tokio::time::sleep(Duration::from_millis(500)).await;
         client.sync_and_await().await?;