Merge pull request #1511 from fluidvanadium/send_height

Use Local Tree Send height

zingolib/CHANGELOG.md

@@ -18,3 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - `LightClient::value_transfers::create_send_value_transfers` (in-function definition) -> `ValueTransfers::create_send_value_transfers`
+
+### Removed
+
+- `lightclient.do_list_notes` is deprecated

zingolib/src/commands.rs

@@ -1677,48 +1677,6 @@ impl Command for NewAddressCommand {
     }
 }
 
-struct NotesCommand {}
-impl Command for NotesCommand {
-    fn help(&self) -> &'static str {
-        indoc! {r#"
-            Show all shielded notes and transparent coins in this wallet
-            Usage:
-            notes [all]
-
-            If you supply the "all" parameter, all previously spent shielded notes and transparent coins are also included
-
-        "#}
-    }
-
-    fn short_help(&self) -> &'static str {
-        "Show all shielded notes and transparent coins in this wallet"
-    }
-
-    fn exec(&self, args: &[&str], lightclient: &LightClient) -> String {
-        // Parse the args.
-        if args.len() > 1 {
-            return self.short_help().to_string();
-        }
-
-        // Make sure we can parse the amount
-        let all_notes = if args.len() == 1 {
-            match args[0] {
-                "all" => true,
-                a => {
-                    return format!(
-                        "Invalid argument \"{}\". Specify 'all' to include unspent notes",
-                        a
-                    )
-                }
-            }
-        } else {
-            false
-        };
-
-        RT.block_on(async move { lightclient.do_list_notes(all_notes).await.pretty(2) })
-    }
-}
-
 struct QuitCommand {}
 impl Command for QuitCommand {
     fn help(&self) -> &'static str {
@@ -1838,7 +1796,6 @@ pub fn get_commands() -> HashMap<&'static str, Box<dyn Command>> {
         ("shield", Box::new(ShieldCommand {})),
         ("save", Box::new(DeprecatedNoCommand {})),
         ("quit", Box::new(QuitCommand {})),
-        ("notes", Box::new(NotesCommand {})),
         ("new", Box::new(NewAddressCommand {})),
         ("defaultfee", Box::new(DefaultFeeCommand {})),
         ("seed", Box::new(SeedCommand {})),

zingolib/src/error.rs

@@ -36,6 +36,8 @@ pub enum ZingoLibError {
     MissingOutputIndex(TxId),
     /// TODO: Add Doc Comment Here!
     CouldNotDecodeMemo(std::io::Error),
+    /// server error
+    Lightwalletd(String),
 }
 
 /// TODO: Add Doc Comment Here!
@@ -121,6 +123,10 @@ impl std::fmt::Display for ZingoLibError {
                 f,
                 "{txid} is missing output_index for note, cannot mark change"
             ),
+            Lightwalletd(string) => write!(
+                f,
+                "{string}"
+            ),
         }
     }
 }

zingolib/src/lightclient/describe.rs

@@ -6,26 +6,21 @@ use std::{cmp::Ordering, collections::HashMap};
 use tokio::runtime::Runtime;
 
 use zcash_client_backend::{encoding::encode_payment_address, PoolType, ShieldedProtocol};
-use zcash_primitives::consensus::{BlockHeight, NetworkConstants};
+use zcash_primitives::consensus::NetworkConstants;
 
 use crate::{
     config::margin_fee,
-    wallet::data::summaries::{
-        SelfSendValueTransfer, SentValueTransfer, TransactionSummaryInterface,
-    },
-};
-
-use super::{AccountBackupInfo, LightClient, PoolBalances, UserBalances};
-use crate::{
     error::ZingoLibError,
+    lightclient::{AccountBackupInfo, LightClient, PoolBalances, UserBalances},
     wallet::{
         data::{
             finsight,
             summaries::{
                 basic_transaction_summary_parts, DetailedTransactionSummaries,
-                DetailedTransactionSummaryBuilder, TransactionSummaries, TransactionSummary,
-                TransactionSummaryBuilder, ValueTransfer, ValueTransferBuilder, ValueTransferKind,
-                ValueTransfers,
+                DetailedTransactionSummaryBuilder, SelfSendValueTransfer, SentValueTransfer,
+                TransactionSummaries, TransactionSummary, TransactionSummaryBuilder,
+                TransactionSummaryInterface, ValueTransfer, ValueTransferBuilder,
+                ValueTransferKind, ValueTransfers,
             },
         },
         keys::address_from_pubkeyhash,
@@ -124,7 +119,10 @@ impl LightClient {
         };
 
         // anchor height is the highest block height that contains income that are considered spendable.
-        let anchor_height = self.wallet.get_anchor_height().await;
+        let current_height = self
+            .get_latest_block_height()
+            .await
+            .map_err(ZingoLibError::Lightwalletd)?;
 
         self.wallet
             .transactions()
@@ -133,9 +131,7 @@ impl LightClient {
             .transaction_records_by_id
             .iter()
             .for_each(|(_, tx)| {
-                let mature = tx
-                    .status
-                    .is_confirmed_before_or_at(&BlockHeight::from_u32(anchor_height));
+                let mature = tx.status.is_confirmed_before_or_at(&current_height);
                 let incoming = tx.is_incoming_transaction();
 
                 let mut change = 0;
@@ -756,7 +752,6 @@ impl LightClient {
     async fn list_sapling_notes(
         &self,
         all_notes: bool,
-        anchor_height: BlockHeight,
     ) -> (Vec<JsonValue>, Vec<JsonValue>, Vec<JsonValue>) {
         let mut unspent_sapling_notes: Vec<JsonValue> = vec![];
         let mut pending_spent_sapling_notes: Vec<JsonValue> = vec![];
@@ -769,7 +764,7 @@ impl LightClient {
                         None
                     } else {
                         let address = LightWallet::note_address::<sapling_crypto::note_encryption::SaplingDomain>(&self.config.chain, note_metadata, &self.wallet.wallet_capability());
-                        let spendable = transaction_metadata.status.is_confirmed_after_or_at(&anchor_height) && note_metadata.spending_tx_status().is_none();
+                        let spendable = transaction_metadata.status.is_confirmed() && note_metadata.spending_tx_status().is_none();
 
                         let created_block:u32 = transaction_metadata.status.get_height().into();
                         // this object should be created by the DomainOuput trait if this doesnt get deprecated
@@ -801,7 +796,6 @@ impl LightClient {
     async fn list_orchard_notes(
         &self,
         all_notes: bool,
-        anchor_height: BlockHeight,
     ) -> (Vec<JsonValue>, Vec<JsonValue>, Vec<JsonValue>) {
         let mut unspent_orchard_notes: Vec<JsonValue> = vec![];
         let mut pending_spent_orchard_notes: Vec<JsonValue> = vec![];
@@ -813,7 +807,7 @@ impl LightClient {
                         None
                     } else {
                         let address = LightWallet::note_address::<OrchardDomain>(&self.config.chain, note_metadata, &self.wallet.wallet_capability());
-                        let spendable = transaction_metadata.status.is_confirmed_after_or_at(&anchor_height) && note_metadata.spending_tx_status().is_none();
+                        let spendable = transaction_metadata.status.is_confirmed() && note_metadata.spending_tx_status().is_none();
 
                         let created_block:u32 = transaction_metadata.status.get_height().into();
                         Some(object!{
@@ -911,13 +905,12 @@ impl LightClient {
     ///  * TODO:   This fn must (on success) return an Ok(Vec\<Notes\>) where Notes is a 3 variant enum....
     ///  * TODO:   type-associated to the variants of the enum must impl From\<Type\> for JsonValue
     ///  * TODO:  DEPRECATE in favor of list_outputs
+    #[cfg(any(test, feature = "test-elevation"))]
     pub async fn do_list_notes(&self, all_notes: bool) -> JsonValue {
-        let anchor_height = BlockHeight::from_u32(self.wallet.get_anchor_height().await);
-
         let (mut unspent_sapling_notes, mut spent_sapling_notes, mut pending_spent_sapling_notes) =
-            self.list_sapling_notes(all_notes, anchor_height).await;
+            self.list_sapling_notes(all_notes).await;
         let (mut unspent_orchard_notes, mut spent_orchard_notes, mut pending_spent_orchard_notes) =
-            self.list_orchard_notes(all_notes, anchor_height).await;
+            self.list_orchard_notes(all_notes).await;
         let (
             mut unspent_transparent_notes,
             mut spent_transparent_notes,

zingolib/src/lightclient/send.rs

@@ -6,7 +6,7 @@ use super::LightClient;
 use super::LightWalletSendProgress;
 
 impl LightClient {
-    async fn get_latest_block(&self) -> Result<BlockHeight, String> {
+    pub(crate) async fn get_latest_block_height(&self) -> Result<BlockHeight, String> {
         Ok(BlockHeight::from_u32(
             crate::grpc_connector::get_latest_block(self.config.get_lightwalletd_uri())
                 .await?
@@ -138,7 +138,7 @@ pub mod send_with_proposal {
                 .write()
                 .await;
             let current_height = self
-                .get_latest_block()
+                .get_latest_block_height()
                 .await
                 .map_err(RecordCachedTransactionsError::Height)?;
             let mut transactions_to_record = vec![];
@@ -202,7 +202,7 @@ pub mod send_with_proposal {
                 .write()
                 .await;
             let current_height = self
-                .get_latest_block()
+                .get_latest_block_height()
                 .await
                 .map_err(BroadcastCachedTransactionsError::Height)?;
             let calculated_tx_cache = tx_map

zingolib/src/testutils/chain_generics/with_assertions.rs

@@ -1,7 +1,9 @@
 //! lightclient functions with added assertions. used for tests.
 
+use std::num::NonZeroU32;
+
 use crate::{lightclient::LightClient, testutils::lightclient::lookup_stati as lookup_statuses};
-use zcash_client_backend::PoolType;
+use zcash_client_backend::{data_api::WalletRead as _, PoolType};
 
 use crate::testutils::{
     assertions::{assert_recipient_total_lte_to_proposal_total, lookup_fees_with_proposal_check},
@@ -40,8 +42,12 @@ where
 
     let send_height = sender
         .wallet
-        .get_target_height_and_anchor_offset()
+        .transaction_context
+        .transaction_metadata_set
+        .read()
         .await
+        .get_target_and_anchor_heights(NonZeroU32::MIN)
+        .expect("sender has a target height")
         .expect("sender has a target height")
         .0;
 
@@ -60,7 +66,7 @@ where
         .expect("record is ok");
 
     lookup_statuses(sender, txids.clone()).await.map(|status| {
-        assert_eq!(status, ConfirmationStatus::Transmitted(send_height.into()));
+        assert_eq!(status, ConfirmationStatus::Transmitted(send_height));
     });
 
     let send_ua_id = sender.do_addresses().await[0]["address"].clone();
@@ -97,10 +103,7 @@ where
                     .transaction_records_by_id;
                 for txid in &txids {
                     let record = records.get(txid).expect("recipient must recognize txid");
-                    assert_eq!(
-                        record.status,
-                        ConfirmationStatus::Mempool(send_height.into()),
-                    )
+                    assert_eq!(record.status, ConfirmationStatus::Mempool(send_height),)
                 }
             }
         }
@@ -145,8 +148,12 @@ where
 
     let send_height = client
         .wallet
-        .get_target_height_and_anchor_offset()
+        .transaction_context
+        .transaction_metadata_set
+        .read()
         .await
+        .get_target_and_anchor_heights(NonZeroU32::MIN)
+        .expect("sender has a target height")
         .expect("sender has a target height")
         .0;
 
@@ -164,7 +171,7 @@ where
         .expect("record is ok");
 
     lookup_statuses(client, txids.clone()).await.map(|status| {
-        assert_eq!(status, ConfirmationStatus::Transmitted(send_height.into()));
+        assert_eq!(status, ConfirmationStatus::Transmitted(send_height));
     });
 
     if test_mempool {

zingolib/src/wallet/describe.rs

@@ -209,14 +209,6 @@ impl LightWallet {
         self.mnemonic.as_ref()
     }
 
-    /// Get the height of the anchor block
-    pub async fn get_anchor_height(&self) -> u32 {
-        match self.get_target_height_and_anchor_offset().await {
-            Some((height, anchor_offset)) => height - anchor_offset as u32 - 1, // what is the purpose of this -1 ?
-            None => 0,
-        }
-    }
-
     /// TODO: Add Doc Comment Here!
     pub async fn get_birthday(&self) -> u64 {
         let birthday = self.birthday.load(std::sync::atomic::Ordering::SeqCst);

zingolib/src/wallet/send.rs

@@ -5,7 +5,6 @@ use zcash_address::AddressKind;
 use zcash_client_backend::proposal::Proposal;
 use zcash_proofs::prover::LocalTxProver;
 
-use std::cmp;
 use std::ops::DerefMut as _;
 
 use zcash_client_backend::zip321::TransactionRequest;
@@ -46,34 +45,6 @@ impl SendProgress {
 }
 
 impl LightWallet {
-    /// Determines the target height for a transaction, and the offset from which to
-    /// select anchors, based on the current synchronised block chain.
-    pub(crate) async fn get_target_height_and_anchor_offset(&self) -> Option<(u32, usize)> {
-        let range = {
-            let blocks = self.last_100_blocks.read().await;
-            (
-                blocks.last().map(|block| block.height as u32),
-                blocks.first().map(|block| block.height as u32),
-            )
-        };
-        match range {
-            (Some(min_height), Some(max_height)) => {
-                let target_height = max_height + 1;
-
-                // Select an anchor ANCHOR_OFFSET back from the target block,
-                // unless that would be before the earliest block we have.
-                let anchor_height = cmp::max(
-                    target_height
-                        .saturating_sub(self.transaction_context.config.reorg_buffer_offset),
-                    min_height,
-                );
-
-                Some((target_height, (target_height - anchor_height) as usize))
-            }
-            _ => None,
-        }
-    }
-
     // Reset the send progress status to blank
     pub(crate) async fn reset_send_progress(&self) {
         let mut g = self.send_progress.write().await;

zingolib/src/wallet/tx_map/trait_walletread.rs

@@ -61,8 +61,11 @@ impl WalletRead for TxMap {
     /// knows about.
     ///
     /// This will return `Ok(None)` if no block data is present in the database.
-    /// IMPL: fully implemented. the target height is always the next block after the last block fetched from the server, and the anchor is a variable depth below.
-    /// IMPL: tested
+    ///
+    /// IMPL:
+    /// fully implemented. the target height is always the next block after the last block fetched from the server, and the anchor is a variable depth below.
+    /// tested
+    /// in a view-only wallet, will return Err(TxMapTraitError::NoSpendCapability)
     fn get_target_and_anchor_heights(
         &self,
         min_confirmations: std::num::NonZeroU32,