From 47b1065db901e9e423e3b0cdb35fc94d9bd4b24c Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Mon, 21 Oct 2024 14:56:44 -0600 Subject: [PATCH] zcash_client_backend: Require wallet metadata for balance calculation --- .../src/data_api/wallet/input_selection.rs | 118 +++++++++--------- zcash_client_backend/src/fees.rs | 2 +- zcash_client_backend/src/fees/fixed.rs | 6 +- zcash_client_backend/src/fees/standard.rs | 2 +- zcash_client_backend/src/fees/zip317.rs | 30 ++--- 5 files changed, 80 insertions(+), 78 deletions(-) diff --git a/zcash_client_backend/src/data_api/wallet/input_selection.rs b/zcash_client_backend/src/data_api/wallet/input_selection.rs index 5125059bcb..09cbf7e99d 100644 --- a/zcash_client_backend/src/data_api/wallet/input_selection.rs +++ b/zcash_client_backend/src/data_api/wallet/input_selection.rs @@ -482,61 +482,6 @@ impl InputSelector for GreedyInputSelector { } } - #[cfg(not(feature = "transparent-inputs"))] - let ephemeral_balance = None; - - #[cfg(feature = "transparent-inputs")] - let (ephemeral_balance, tr1_balance_opt) = { - if tr1_transparent_outputs.is_empty() { - (None, None) - } else { - // The ephemeral input going into transaction 1 must be able to pay that - // transaction's fee, as well as the TEX address payments. - - // First compute the required total with an additional zero input, - // catching the `InsufficientFunds` error to obtain the required amount - // given the provided change strategy. Ignore the change memo in order - // to avoid adding a change output. - let tr1_required_input_value = match change_strategy - .compute_balance::<_, DbT::NoteRef>( - params, - target_height, - &[] as &[WalletTransparentOutput], - &tr1_transparent_outputs, - &sapling::EmptyBundleView, - #[cfg(feature = "orchard")] - &orchard_fees::EmptyBundleView, - Some(&EphemeralBalance::Input(NonNegativeAmount::ZERO)), - None, - ) { - Err(ChangeError::InsufficientFunds { required, .. }) => required, - Err(ChangeError::DustInputs { .. }) => unreachable!("no inputs were supplied"), - Err(other) => return Err(InputSelectorError::Change(other)), - Ok(_) => NonNegativeAmount::ZERO, // shouldn't happen - }; - - // Now recompute to obtain the `TransactionBalance` and verify that it - // fully accounts for the required fees. - let tr1_balance = change_strategy.compute_balance::<_, DbT::NoteRef>( - params, - target_height, - &[] as &[WalletTransparentOutput], - &tr1_transparent_outputs, - &sapling::EmptyBundleView, - #[cfg(feature = "orchard")] - &orchard_fees::EmptyBundleView, - Some(&EphemeralBalance::Input(tr1_required_input_value)), - None, - )?; - assert_eq!(tr1_balance.total(), tr1_balance.fee_required()); - - ( - Some(EphemeralBalance::Output(tr1_required_input_value)), - Some(tr1_balance), - ) - } - }; - let mut shielded_inputs = SpendableNotes::empty(); let mut prior_available = NonNegativeAmount::ZERO; let mut amount_required = NonNegativeAmount::ZERO; @@ -598,6 +543,63 @@ impl InputSelector for GreedyInputSelector { .fetch_wallet_meta(wallet_db, account, &selected_input_ids) .map_err(InputSelectorError::DataSource)?; + #[cfg(not(feature = "transparent-inputs"))] + let ephemeral_balance = None; + + #[cfg(feature = "transparent-inputs")] + let (ephemeral_balance, tr1_balance_opt) = { + if tr1_transparent_outputs.is_empty() { + (None, None) + } else { + // The ephemeral input going into transaction 1 must be able to pay that + // transaction's fee, as well as the TEX address payments. + + // First compute the required total with an additional zero input, + // catching the `InsufficientFunds` error to obtain the required amount + // given the provided change strategy. Ignore the change memo in order + // to avoid adding a change output. + let tr1_required_input_value = match change_strategy + .compute_balance::<_, DbT::NoteRef>( + params, + target_height, + &[] as &[WalletTransparentOutput], + &tr1_transparent_outputs, + &sapling::EmptyBundleView, + #[cfg(feature = "orchard")] + &orchard_fees::EmptyBundleView, + Some(&EphemeralBalance::Input(NonNegativeAmount::ZERO)), + &wallet_meta, + ) { + Err(ChangeError::InsufficientFunds { required, .. }) => required, + Err(ChangeError::DustInputs { .. }) => { + unreachable!("no inputs were supplied") + } + Err(other) => return Err(InputSelectorError::Change(other)), + Ok(_) => NonNegativeAmount::ZERO, // shouldn't happen + }; + + // Now recompute to obtain the `TransactionBalance` and verify that it + // fully accounts for the required fees. + let tr1_balance = change_strategy.compute_balance::<_, DbT::NoteRef>( + params, + target_height, + &[] as &[WalletTransparentOutput], + &tr1_transparent_outputs, + &sapling::EmptyBundleView, + #[cfg(feature = "orchard")] + &orchard_fees::EmptyBundleView, + Some(&EphemeralBalance::Input(tr1_required_input_value)), + &wallet_meta, + )?; + assert_eq!(tr1_balance.total(), tr1_balance.fee_required()); + + ( + Some(EphemeralBalance::Output(tr1_required_input_value)), + Some(tr1_balance), + ) + } + }; + // In the ZIP 320 case, this is the balance for transaction 0, taking into account // the ephemeral output. let balance = change_strategy.compute_balance( @@ -617,7 +619,7 @@ impl InputSelector for GreedyInputSelector { &orchard_outputs[..], ), ephemeral_balance.as_ref(), - Some(&wallet_meta), + &wallet_meta, ); match balance { @@ -819,7 +821,7 @@ impl ShieldingSelector for GreedyInputSelector { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, None, - Some(&wallet_meta), + &wallet_meta, ); let balance = match trial_balance { @@ -837,7 +839,7 @@ impl ShieldingSelector for GreedyInputSelector { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, None, - Some(&wallet_meta), + &wallet_meta, )? } Err(other) => return Err(InputSelectorError::Change(other)), diff --git a/zcash_client_backend/src/fees.rs b/zcash_client_backend/src/fees.rs index e109d58478..e855a0462a 100644 --- a/zcash_client_backend/src/fees.rs +++ b/zcash_client_backend/src/fees.rs @@ -505,7 +505,7 @@ pub trait ChangeStrategy { sapling: &impl sapling::BundleView, #[cfg(feature = "orchard")] orchard: &impl orchard::BundleView, ephemeral_balance: Option<&EphemeralBalance>, - wallet_meta: Option<&Self::WalletMetaT>, + wallet_meta: &Self::WalletMetaT, ) -> Result>; } diff --git a/zcash_client_backend/src/fees/fixed.rs b/zcash_client_backend/src/fees/fixed.rs index 529e9cdfcd..452135fbba 100644 --- a/zcash_client_backend/src/fees/fixed.rs +++ b/zcash_client_backend/src/fees/fixed.rs @@ -84,7 +84,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy { sapling: &impl sapling_fees::BundleView, #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView, ephemeral_balance: Option<&EphemeralBalance>, - _wallet_meta: Option<&Self::WalletMetaT>, + _wallet_meta: &Self::WalletMetaT, ) -> Result> { let split_policy = SplitPolicy::single_output(); let cfg = SinglePoolBalanceConfig::new( @@ -168,7 +168,7 @@ mod tests { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, None, - None, + &(), ); assert_matches!( @@ -218,7 +218,7 @@ mod tests { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, None, - None, + &(), ); assert_matches!( diff --git a/zcash_client_backend/src/fees/standard.rs b/zcash_client_backend/src/fees/standard.rs index 330ec6de36..c3e46d3875 100644 --- a/zcash_client_backend/src/fees/standard.rs +++ b/zcash_client_backend/src/fees/standard.rs @@ -88,7 +88,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy { sapling: &impl sapling_fees::BundleView, #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView, ephemeral_balance: Option<&EphemeralBalance>, - wallet_meta: Option<&Self::WalletMetaT>, + wallet_meta: &Self::WalletMetaT, ) -> Result> { #[allow(deprecated)] match self.fee_rule() { diff --git a/zcash_client_backend/src/fees/zip317.rs b/zcash_client_backend/src/fees/zip317.rs index c342c46657..0706bfaf81 100644 --- a/zcash_client_backend/src/fees/zip317.rs +++ b/zcash_client_backend/src/fees/zip317.rs @@ -91,7 +91,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy { sapling: &impl sapling_fees::BundleView, #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView, ephemeral_balance: Option<&EphemeralBalance>, - _wallet_meta: Option<&Self::WalletMetaT>, + _wallet_meta: &Self::WalletMetaT, ) -> Result> { let split_policy = SplitPolicy::single_output(); let cfg = SinglePoolBalanceConfig::new( @@ -189,7 +189,7 @@ impl ChangeStrategy for MultiOutputChangeStrategy { sapling: &impl sapling_fees::BundleView, #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView, ephemeral_balance: Option<&EphemeralBalance>, - wallet_meta: Option<&Self::WalletMetaT>, + wallet_meta: &Self::WalletMetaT, ) -> Result> { let cfg = SinglePoolBalanceConfig::new( params, @@ -204,7 +204,7 @@ impl ChangeStrategy for MultiOutputChangeStrategy { single_pool_output_balance( cfg, - wallet_meta, + Some(wallet_meta), target_height, transparent_inputs, transparent_outputs, @@ -277,7 +277,7 @@ mod tests { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, None, - None, + &(), ); assert_matches!( @@ -324,11 +324,11 @@ mod tests { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, None, - Some(&WalletMeta::new( + &WalletMeta::new( existing_notes, #[cfg(feature = "orchard")] 0, - )), + ), ) }; @@ -380,11 +380,11 @@ mod tests { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, None, - Some(&WalletMeta::new( + &WalletMeta::new( 0, #[cfg(feature = "orchard")] 0, - )), + ), ); assert_matches!( @@ -435,7 +435,7 @@ mod tests { ))][..], ), None, - None, + &(), ); assert_matches!( @@ -489,7 +489,7 @@ mod tests { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, None, - None, + &(), ); assert_matches!( @@ -534,7 +534,7 @@ mod tests { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, None, - None, + &(), ); assert_matches!( @@ -579,7 +579,7 @@ mod tests { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, None, - None, + &(), ); assert_matches!( @@ -630,7 +630,7 @@ mod tests { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, None, - None, + &(), ); assert_matches!( @@ -692,7 +692,7 @@ mod tests { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, None, - None, + &(), ); assert_matches!( @@ -744,7 +744,7 @@ mod tests { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, None, - None, + &(), ); // We will get an error here, because the dust input isn't free to add