Always require change branch

zcash_client_backend/CHANGELOG.md

@@ -5,7 +5,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this library adheres to Rust's notion of
 [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [Unreleased]
+## [Zingo]
+- replace create_proposed_transaction with `calculate_proposed_transaction`
+  - dont try to write the calculated transaction to wallet, that will be handled later
+  - ignore collecting any transaction metadata
+  - handle usk differently
+- modify change algorithm
+- added display for NoteId
 
 ## [0.12.1] - 2024-03-27
 

zcash_client_backend/build.rs

@@ -43,7 +43,7 @@ fn build() -> io::Result<()> {
 
     // Build the gRPC types and client.
     tonic_build::configure()
-        .build_server(false)
+        .build_server(true)
         .client_mod_attribute(
             "cash.z.wallet.sdk.rpc",
             r#"#[cfg(feature = "lightwalletd-tonic")]"#,

zcash_client_backend/src/data_api/wallet.rs

@@ -36,7 +36,7 @@
 use nonempty::NonEmpty;
 use rand_core::OsRng;
 use sapling::{
     note_encryption::{try_sapling_note_decryption, PreparedIncomingViewingKey},
     prover::{OutputProver, SpendProver},
 };
 use std::num::NonZeroU32;
@@ -45,20 +45,20 @@
 use crate::{
     address::Address,
     data_api::{
         error::Error, Account, SentTransaction, SentTransactionOutput, WalletCommitmentTrees,
         WalletRead, WalletWrite,
     },
     decrypt_transaction,
     fees::{self, DustOutputPolicy},
     keys::UnifiedSpendingKey,
     proposal::{self, Proposal, ProposalError},
-    wallet::{Note, OvkPolicy, Recipient},
+    wallet::{Note, OvkPolicy, Recipient, TransparentAddressMetadata},
     zip321::{self, Payment},
     PoolType, ShieldedProtocol,
 };
 use zcash_primitives::transaction::{
     builder::{BuildConfig, BuildResult, Builder},
     components::{amount::NonNegativeAmount, sapling::zip212_enforcement},
     fees::{zip317::FeeError as Zip317FeeError, FeeRule, StandardFeeRule},
     Transaction, TxId,
 };
@@ -584,13 +584,13 @@
 #[allow(clippy::too_many_arguments)]
 #[allow(clippy::type_complexity)]
 pub fn create_proposed_transactions<DbT, ParamsT, InputsErrT, FeeRuleT, N>(
     wallet_db: &mut DbT,
     params: &ParamsT,
     spend_prover: &impl SpendProver,
     output_prover: &impl OutputProver,
     usk: &UnifiedSpendingKey,
     ovk_policy: OvkPolicy,
     proposal: &Proposal<FeeRuleT, N>,
 ) -> Result<
     NonEmpty<TxId>,
     Error<
@@ -605,9 +605,10 @@
     ParamsT: consensus::Parameters + Clone,
     FeeRuleT: FeeRule,
 {
+    unimplemented!();
     let mut step_results = Vec::with_capacity(proposal.steps().len());
     for step in proposal.steps() {
-        let step_result = create_proposed_transaction(
+        let step_result = calculate_proposed_transaction(
             wallet_db,
             params,
             spend_prover,
@@ -618,6 +619,8 @@
             proposal.min_target_height(),
             &step_results,
             step,
+            None,
+            None,
         )?;
         step_results.push((step, step_result));
     }
@@ -631,9 +634,10 @@
     .expect("proposal.steps is NonEmpty"))
 }
 
+/// Zingo uses calculate_proposed_transaction to create the transaction, and then stores it ASYNCRONOUSLY
 #[allow(clippy::too_many_arguments)]
 #[allow(clippy::type_complexity)]
-fn create_proposed_transaction<DbT, ParamsT, InputsErrT, FeeRuleT, N>(
+pub fn calculate_proposed_transaction<DbT, ParamsT, InputsErrT, FeeRuleT, N>(
     wallet_db: &mut DbT,
     params: &ParamsT,
     spend_prover: &impl SpendProver,
@@ -644,6 +648,10 @@
     min_target_height: BlockHeight,
     prior_step_results: &[(&proposal::Step<N>, BuildResult)],
     proposal_step: &proposal::Step<N>,
+    usk_to_tkey: Option<
+        fn(&UnifiedSpendingKey, &TransparentAddressMetadata) -> hdwallet::secp256k1::SecretKey,
+    >,
+    override_sapling_change_address: Option<sapling::PaymentAddress>,
 ) -> Result<
     BuildResult,
     Error<
@@ -654,7 +662,7 @@
     >,
 >
 where
-    DbT: WalletWrite + WalletCommitmentTrees,
+    DbT: WalletRead + WalletCommitmentTrees,
     ParamsT: consensus::Parameters + Clone,
     FeeRuleT: FeeRule,
 {
@@ -798,7 +806,7 @@
     }
 
     #[cfg(feature = "transparent-inputs")]
     let utxos_spent = {
         let known_addrs = wallet_db
             .get_transparent_receivers(account)
             .map_err(Error::DataSource)?;
@@ -822,10 +830,16 @@
                 .clone()
                 .ok_or_else(|| Error::NoSpendingKey(addr.encode(params)))?;
 
-            let secret_key = usk
-                .transparent()
-                .derive_secret_key(address_metadata.scope(), address_metadata.address_index())
-                .unwrap();
+            let secret_key = usk_to_tkey
+                .map(|f| f(usk, &address_metadata))
+                .unwrap_or_else(|| {
+                    usk.transparent()
+                        .derive_secret_key(
+                            address_metadata.scope(),
+                            address_metadata.address_index(),
+                        )
+                        .unwrap()
+                });
 
             utxos_spent.push(outpoint.clone());
             builder.add_transparent_input(secret_key, outpoint, utxo)?;
@@ -935,10 +949,6 @@
         Some(sapling_dfvk.to_ovk(Scope::Internal))
     };
 
-    #[cfg(feature = "orchard")]
-    let mut orchard_output_meta = vec![];
-    let mut sapling_output_meta = vec![];
-    let mut transparent_output_meta = vec![];
     for (payment, output_pool) in proposal_step
         .payment_pools()
         .iter()
@@ -973,14 +983,6 @@
                             payment.amount.into(),
                             memo.clone(),
                         )?;
-                        orchard_output_meta.push((
-                            Recipient::Unified(
-                                ua.clone(),
-                                PoolType::Shielded(ShieldedProtocol::Orchard),
-                            ),
-                            payment.amount,
-                            Some(memo),
-                        ));
                     }
 
                     PoolType::Shielded(ShieldedProtocol::Sapling) => {
@@ -990,14 +992,6 @@
                             payment.amount,
                             memo.clone(),
                         )?;
-                        sapling_output_meta.push((
-                            Recipient::Unified(
-                                ua.clone(),
-                                PoolType::Shielded(ShieldedProtocol::Sapling),
-                            ),
-                            payment.amount,
-                            Some(memo),
-                        ));
                     }
 
                     PoolType::Transparent => {
@@ -1023,15 +1017,13 @@
                     payment.amount,
                     memo.clone(),
                 )?;
-                sapling_output_meta.push((Recipient::Sapling(*addr), payment.amount, Some(memo)));
             }
             Address::Transparent(to) => {
                 if payment.memo.is_some() {
                     return Err(Error::MemoForbidden);
                 } else {
                     builder.add_transparent_output(to, payment.amount)?;
                 }
-                transparent_output_meta.push((to, payment.amount));
             }
         }
     }
@@ -1044,19 +1036,10 @@
             ShieldedProtocol::Sapling => {
                 builder.add_sapling_output(
                     sapling_internal_ovk(),
-                    sapling_dfvk.change_address().1,
+                    override_sapling_change_address.unwrap_or(sapling_dfvk.change_address().1),
                     change_value.value(),
                     memo.clone(),
                 )?;
-                sapling_output_meta.push((
-                    Recipient::InternalAccount {
-                        receiving_account: account,
-                        external_address: None,
-                        note: PoolType::Shielded(ShieldedProtocol::Sapling),
-                    },
-                    change_value.value(),
-                    Some(memo),
-                ))
             }
             ShieldedProtocol::Orchard => {
                 #[cfg(not(feature = "orchard"))]
@@ -1068,19 +1051,10 @@
                 {
                     builder.add_orchard_output(
                         orchard_internal_ovk(),
-                        orchard_fvk.address_at(0u32, orchard::keys::Scope::Internal),
+                        orchard_fvk.address_at(0u32, orchard::keys::Scope::External),
                         change_value.value().into(),
                         memo.clone(),
                     )?;
-                    orchard_output_meta.push((
-                        Recipient::InternalAccount {
-                            receiving_account: account,
-                            external_address: None,
-                            note: PoolType::Shielded(ShieldedProtocol::Orchard),
-                        },
-                        change_value.value(),
-                        Some(memo),
-                    ))
                 }
             }
         }
@@ -1089,105 +1063,6 @@
     // Build the transaction with the specified fee rule
     let build_result = builder.build(OsRng, spend_prover, output_prover, fee_rule)?;
 
-    #[cfg(feature = "orchard")]
-    let orchard_internal_ivk = orchard_fvk.to_ivk(orchard::keys::Scope::Internal);
-    #[cfg(feature = "orchard")]
-    let orchard_outputs =
-        orchard_output_meta
-            .into_iter()
-            .enumerate()
-            .map(|(i, (recipient, value, memo))| {
-                let output_index = build_result
-                    .orchard_meta()
-                    .output_action_index(i)
-                    .expect("An action should exist in the transaction for each Orchard output.");
-
-                let recipient = recipient
-                    .map_internal_account_note(|pool| {
-                        assert!(pool == PoolType::Shielded(ShieldedProtocol::Orchard));
-                        build_result
-                            .transaction()
-                            .orchard_bundle()
-                            .and_then(|bundle| {
-                                bundle
-                                    .decrypt_output_with_key(output_index, &orchard_internal_ivk)
-                                    .map(|(note, _, _)| Note::Orchard(note))
-                            })
-                    })
-                    .internal_account_note_transpose_option()
-                    .expect("Wallet-internal outputs must be decryptable with the wallet's IVK");
-
-                SentTransactionOutput::from_parts(output_index, recipient, value, memo)
-            });
-
-    let sapling_internal_ivk =
-        PreparedIncomingViewingKey::new(&sapling_dfvk.to_ivk(Scope::Internal));
-    let sapling_outputs =
-        sapling_output_meta
-            .into_iter()
-            .enumerate()
-            .map(|(i, (recipient, value, memo))| {
-                let output_index = build_result
-                    .sapling_meta()
-                    .output_index(i)
-                    .expect("An output should exist in the transaction for each Sapling payment.");
-
-                let recipient = recipient
-                    .map_internal_account_note(|pool| {
-                        assert!(pool == PoolType::Shielded(ShieldedProtocol::Sapling));
-                        build_result
-                            .transaction()
-                            .sapling_bundle()
-                            .and_then(|bundle| {
-                                try_sapling_note_decryption(
-                                    &sapling_internal_ivk,
-                                    &bundle.shielded_outputs()[output_index],
-                                    zip212_enforcement(params, min_target_height),
-                                )
-                                .map(|(note, _, _)| Note::Sapling(note))
-                            })
-                    })
-                    .internal_account_note_transpose_option()
-                    .expect("Wallet-internal outputs must be decryptable with the wallet's IVK");
-
-                SentTransactionOutput::from_parts(output_index, recipient, value, memo)
-            });
-
-    let transparent_outputs = transparent_output_meta.into_iter().map(|(addr, value)| {
-        let script = addr.script();
-        let output_index = build_result
-            .transaction()
-            .transparent_bundle()
-            .and_then(|b| {
-                b.vout
-                    .iter()
-                    .enumerate()
-                    .find(|(_, tx_out)| tx_out.script_pubkey == script)
-            })
-            .map(|(index, _)| index)
-            .expect("An output should exist in the transaction for each transparent payment.");
-
-        SentTransactionOutput::from_parts(output_index, Recipient::Transparent(*addr), value, None)
-    });
-
-    let mut outputs = vec![];
-    #[cfg(feature = "orchard")]
-    outputs.extend(orchard_outputs);
-    outputs.extend(sapling_outputs);
-    outputs.extend(transparent_outputs);
-
-    wallet_db
-        .store_sent_tx(&SentTransaction {
-            tx: build_result.transaction(),
-            created: time::OffsetDateTime::now_utc(),
-            account,
-            outputs,
-            fee_amount: proposal_step.balance().fee_required(),
-            #[cfg(feature = "transparent-inputs")]
-            utxos_spent,
-        })
-        .map_err(Error::DataSource)?;
-
     Ok(build_result)
 }
 

zcash_client_backend/src/fees/common.rs

@@ -202,40 +202,36 @@ where
         required: total_out,
     })?;
 
-    if proposed_change.is_zero() {
-        TransactionBalance::new(vec![], fee_amount).map_err(|_| overflow())
-    } else {
-        let dust_threshold = dust_output_policy
-            .dust_threshold()
-            .unwrap_or(default_dust_threshold);
-
-        if proposed_change < dust_threshold {
-            match dust_output_policy.action() {
-                DustAction::Reject => {
-                    let shortfall = (dust_threshold - proposed_change).ok_or_else(underflow)?;
-
-                    Err(ChangeError::InsufficientFunds {
-                        available: total_in,
-                        required: (total_in + shortfall).ok_or_else(overflow)?,
-                    })
-                }
-                DustAction::AllowDustChange => TransactionBalance::new(
-                    vec![ChangeValue::new(change_pool, proposed_change, change_memo)],
-                    fee_amount,
-                )
-                .map_err(|_| overflow()),
-                DustAction::AddDustToFee => TransactionBalance::new(
-                    vec![],
-                    (fee_amount + proposed_change).ok_or_else(overflow)?,
-                )
-                .map_err(|_| overflow()),
+    let dust_threshold = dust_output_policy
+        .dust_threshold()
+        .unwrap_or(default_dust_threshold);
+
+    if proposed_change < dust_threshold {
+        match dust_output_policy.action() {
+            DustAction::Reject => {
+                let shortfall = (dust_threshold - proposed_change).ok_or_else(underflow)?;
+
+                Err(ChangeError::InsufficientFunds {
+                    available: total_in,
+                    required: (total_in + shortfall).ok_or_else(overflow)?,
+                })
             }
-        } else {
-            TransactionBalance::new(
+            DustAction::AllowDustChange => TransactionBalance::new(
                 vec![ChangeValue::new(change_pool, proposed_change, change_memo)],
                 fee_amount,
             )
-            .map_err(|_| overflow())
+            .map_err(|_| overflow()),
+            DustAction::AddDustToFee => TransactionBalance::new(
+                vec![],
+                (fee_amount + proposed_change).ok_or_else(overflow)?,
+            )
+            .map_err(|_| overflow()),
         }
+    } else {
+        TransactionBalance::new(
+            vec![ChangeValue::new(change_pool, proposed_change, change_memo)],
+            fee_amount,
+        )
+        .map_err(|_| overflow())
     }
 }