From 4e3d99f1d0c01ab63543b0d259bf5c0a3e1a3358 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 7 Feb 2024 18:19:02 -0700 Subject: [PATCH] zcash_client_backend: Allow proposer to specify fallback change pool. In the event that the pool to which change should be sent cannot automatically be determined based upon the inputs and outputs of a transaction, it is up to the caller to specify where change should be sent. --- zcash_client_backend/CHANGELOG.md | 12 +- zcash_client_backend/src/data_api/wallet.rs | 147 ++++++++++-------- .../src/data_api/wallet/input_selection.rs | 4 +- zcash_client_backend/src/fees/common.rs | 10 +- zcash_client_backend/src/fees/fixed.rs | 18 ++- zcash_client_backend/src/fees/standard.rs | 13 +- zcash_client_backend/src/fees/zip317.rs | 36 ++++- zcash_client_backend/src/lib.rs | 3 - zcash_client_backend/src/proposal.rs | 54 +++++++ zcash_client_backend/src/proto.rs | 2 - zcash_client_sqlite/src/chain.rs | 3 +- zcash_client_sqlite/src/testing.rs | 10 +- zcash_client_sqlite/src/wallet.rs | 5 +- zcash_client_sqlite/src/wallet/sapling.rs | 13 +- zcash_keys/CHANGELOG.md | 2 +- zcash_keys/src/address.rs | 20 +++ 16 files changed, 252 insertions(+), 100 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 5e03f531f..d536d7c23 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -154,8 +154,9 @@ and this library adheres to Rust's notion of the database identifiers for its contained notes by universally quantifying the `NoteRef` type parameter. - It returns a `NonEmpty` instead of a single `TxId` value. - - `wallet::create_spend_to_address` now takes an additional `change_memo` - argument. It also returns its result as a `NonEmpty` instead of a + - `wallet::create_spend_to_address` now takes additional `change_memo` and + `fallback_change_pool` arguments. It also returns its result as a + `NonEmpty` instead of a single `TxId`. - `wallet::spend` returns its result as a `NonEmpty` instead of a single `TxId`. @@ -226,9 +227,10 @@ and this library adheres to Rust's notion of now also provides the output pool to which change should be sent and an optional memo to be associated with the change output. - `ChangeError` has a new `BundleError` variant. - - `fixed::SingleOutputChangeStrategy::new` and - `zip317::SingleOutputChangeStrategy::new` each now accept an additional - `change_memo` argument. + - `fixed::SingleOutputChangeStrategy::new`, + `zip317::SingleOutputChangeStrategy::new`, and + `standard::SingleOutputChangeStrategy::new` each now accept additional + `change_memo` and `fallback_change_pool` arguments. - `zcash_client_backend::wallet`: - The fields of `ReceivedSaplingNote` are now private. Use `ReceivedSaplingNote::from_parts` for construction instead. Accessor methods diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index a8a931258..ab32f2683 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -27,8 +27,7 @@ use crate::{ decrypt_transaction, fees::{self, DustOutputPolicy}, keys::UnifiedSpendingKey, - proposal::ProposalError, - proposal::{self, Proposal}, + proposal::{self, Proposal, ProposalError}, wallet::{Note, OvkPolicy, Recipient}, zip321::{self, Payment}, PoolType, ShieldedProtocol, @@ -210,6 +209,7 @@ pub fn create_spend_to_address( ovk_policy: OvkPolicy, min_confirmations: NonZeroU32, change_memo: Option, + fallback_change_pool: ShieldedProtocol, ) -> Result< NonEmpty, Error< @@ -240,6 +240,7 @@ where amount, memo, change_memo, + fallback_change_pool, )?; create_proposed_transactions( @@ -423,6 +424,8 @@ where /// * `amount`: The amount to send. /// * `memo`: A memo to be included in the output to the recipient. /// * `change_memo`: A memo to be included in any change output that is created. +/// * `fallback_change_pool`: The shielded pool to which change should be sent if +/// automatic change pool determination fails. #[allow(clippy::too_many_arguments)] #[allow(clippy::type_complexity)] pub fn propose_standard_transfer_to_address( @@ -435,6 +438,7 @@ pub fn propose_standard_transfer_to_address( amount: NonNegativeAmount, memo: Option, change_memo: Option, + fallback_change_pool: ShieldedProtocol, ) -> Result< Proposal, Error< @@ -461,7 +465,11 @@ where "It should not be possible for this to violate ZIP 321 request construction invariants.", ); - let change_strategy = fees::standard::SingleOutputChangeStrategy::new(fee_rule, change_memo); + let change_strategy = fees::standard::SingleOutputChangeStrategy::new( + fee_rule, + change_memo, + fallback_change_pool, + ); let input_selector = GreedyInputSelector::::new(change_strategy, DustOutputPolicy::default()); @@ -644,73 +652,83 @@ where .map_err(Error::DataSource)? .ok_or(Error::KeyNotRecognized)?; - let (sapling_anchor, sapling_inputs) = proposal_step.shielded_inputs().map_or_else( - || Ok((sapling::Anchor::empty_tree(), vec![])), - |inputs| { - wallet_db.with_sapling_tree_mut::<_, _, Error<_, _, _, _>>(|sapling_tree| { - let anchor = sapling_tree - .root_at_checkpoint_id(&inputs.anchor_height())? - .into(); + let (sapling_anchor, sapling_inputs) = + if proposal_step.involves(PoolType::Shielded(ShieldedProtocol::Sapling)) { + proposal_step.shielded_inputs().map_or_else( + || Ok((Some(sapling::Anchor::empty_tree()), vec![])), + |inputs| { + wallet_db.with_sapling_tree_mut::<_, _, Error<_, _, _, _>>(|sapling_tree| { + let anchor = sapling_tree + .root_at_checkpoint_id(&inputs.anchor_height())? + .into(); - let sapling_inputs = inputs - .notes() - .iter() - .filter_map(|selected| match selected.note() { - Note::Sapling(note) => { - let key = match selected.spending_key_scope() { - Scope::External => usk.sapling().clone(), - Scope::Internal => usk.sapling().derive_internal(), - }; + let sapling_inputs = inputs + .notes() + .iter() + .filter_map(|selected| match selected.note() { + Note::Sapling(note) => { + let key = match selected.spending_key_scope() { + Scope::External => usk.sapling().clone(), + Scope::Internal => usk.sapling().derive_internal(), + }; - sapling_tree - .witness_at_checkpoint_id_caching( - selected.note_commitment_tree_position(), - &inputs.anchor_height(), - ) - .map(|merkle_path| Some((key, note, merkle_path))) - .map_err(Error::from) - .transpose() - } - #[cfg(feature = "orchard")] - Note::Orchard(_) => None, + sapling_tree + .witness_at_checkpoint_id_caching( + selected.note_commitment_tree_position(), + &inputs.anchor_height(), + ) + .map(|merkle_path| Some((key, note, merkle_path))) + .map_err(Error::from) + .transpose() + } + #[cfg(feature = "orchard")] + Note::Orchard(_) => None, + }) + .collect::, Error<_, _, _, _>>>()?; + + Ok((Some(anchor), sapling_inputs)) }) - .collect::, Error<_, _, _, _>>>()?; - - Ok((anchor, sapling_inputs)) - }) - }, - )?; + }, + )? + } else { + (None, vec![]) + }; #[cfg(feature = "orchard")] - let (orchard_anchor, orchard_inputs) = proposal_step.shielded_inputs().map_or_else( - || Ok((Some(orchard::Anchor::empty_tree()), vec![])), - |inputs| { - wallet_db.with_orchard_tree_mut::<_, _, Error<_, _, _, _>>(|orchard_tree| { - let anchor = orchard_tree - .root_at_checkpoint_id(&inputs.anchor_height())? - .into(); + let (orchard_anchor, orchard_inputs) = + if proposal_step.involves(PoolType::Shielded(ShieldedProtocol::Orchard)) { + proposal_step.shielded_inputs().map_or_else( + || Ok((Some(orchard::Anchor::empty_tree()), vec![])), + |inputs| { + wallet_db.with_orchard_tree_mut::<_, _, Error<_, _, _, _>>(|orchard_tree| { + let anchor = orchard_tree + .root_at_checkpoint_id(&inputs.anchor_height())? + .into(); - let orchard_inputs = inputs - .notes() - .iter() - .filter_map(|selected| match selected.note() { - #[cfg(feature = "orchard")] - Note::Orchard(note) => orchard_tree - .witness_at_checkpoint_id_caching( - selected.note_commitment_tree_position(), - &inputs.anchor_height(), - ) - .map(|merkle_path| Some((note, merkle_path))) - .map_err(Error::from) - .transpose(), - Note::Sapling(_) => None, + let orchard_inputs = inputs + .notes() + .iter() + .filter_map(|selected| match selected.note() { + #[cfg(feature = "orchard")] + Note::Orchard(note) => orchard_tree + .witness_at_checkpoint_id_caching( + selected.note_commitment_tree_position(), + &inputs.anchor_height(), + ) + .map(|merkle_path| Some((note, merkle_path))) + .map_err(Error::from) + .transpose(), + Note::Sapling(_) => None, + }) + .collect::, Error<_, _, _, _>>>()?; + + Ok((Some(anchor), orchard_inputs)) }) - .collect::, Error<_, _, _, _>>>()?; - - Ok((Some(anchor), orchard_inputs)) - }) - }, - )?; + }, + )? + } else { + (None, vec![]) + }; #[cfg(not(feature = "orchard"))] let orchard_anchor = None; @@ -720,7 +738,7 @@ where params.clone(), min_target_height, BuildConfig::Standard { - sapling_anchor: Some(sapling_anchor), + sapling_anchor, orchard_anchor, }, ); @@ -984,7 +1002,6 @@ where Some(memo), )) } - #[cfg(zcash_unstable = "orchard")] ShieldedProtocol::Orchard => { #[cfg(not(feature = "orchard"))] return Err(Error::UnsupportedChangeType(PoolType::Shielded( 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 c38cd970c..45502c379 100644 --- a/zcash_client_backend/src/data_api/wallet/input_selection.rs +++ b/zcash_client_backend/src/data_api/wallet/input_selection.rs @@ -455,9 +455,9 @@ where Err(other) => return Err(other.into()), } - #[cfg(not(zcash_unstable = "orchard"))] + #[cfg(not(feature = "orchard"))] let selectable_pools = &[ShieldedProtocol::Sapling]; - #[cfg(zcash_unstable = "orchard")] + #[cfg(feature = "orchard")] let selectable_pools = &[ShieldedProtocol::Sapling, ShieldedProtocol::Orchard]; shielded_inputs = wallet_db diff --git a/zcash_client_backend/src/fees/common.rs b/zcash_client_backend/src/fees/common.rs index 6d0102b0b..8807a12f7 100644 --- a/zcash_client_backend/src/fees/common.rs +++ b/zcash_client_backend/src/fees/common.rs @@ -34,6 +34,7 @@ pub(crate) fn single_change_output_balance< dust_output_policy: &DustOutputPolicy, default_dust_threshold: NonNegativeAmount, change_memo: Option, + _fallback_change_pool: ShieldedProtocol, ) -> Result> where E: From + From, @@ -86,7 +87,6 @@ where // TODO: implement a less naive strategy for selecting the pool to which change will be sent. #[cfg(feature = "orchard")] - #[allow(clippy::if_same_then_else)] let (change_pool, sapling_change, orchard_change) = if orchard_in.is_positive() || orchard_out.is_positive() { // Send change to Orchard if we're spending any Orchard inputs or creating any Orchard outputs @@ -96,8 +96,12 @@ where // Sapling outputs, so that we avoid pool-crossing. (ShieldedProtocol::Sapling, 1, 0) } else { - // For all other transactions, send change to Orchard. - (ShieldedProtocol::Orchard, 0, 1) + // This is a fully-transparent transaction, so the caller gets to decide + // where to shield change. + match _fallback_change_pool { + ShieldedProtocol::Orchard => (_fallback_change_pool, 0, 1), + ShieldedProtocol::Sapling => (_fallback_change_pool, 1, 0), + } }; #[cfg(not(feature = "orchard"))] let (change_pool, sapling_change) = (ShieldedProtocol::Sapling, 1); diff --git a/zcash_client_backend/src/fees/fixed.rs b/zcash_client_backend/src/fees/fixed.rs index 69aed61f6..295cd0bb7 100644 --- a/zcash_client_backend/src/fees/fixed.rs +++ b/zcash_client_backend/src/fees/fixed.rs @@ -9,6 +9,8 @@ use zcash_primitives::{ }, }; +use crate::ShieldedProtocol; + use super::{ common::single_change_output_balance, sapling as sapling_fees, ChangeError, ChangeStrategy, DustOutputPolicy, TransactionBalance, @@ -22,15 +24,21 @@ use super::orchard as orchard_fees; pub struct SingleOutputChangeStrategy { fee_rule: FixedFeeRule, change_memo: Option, + fallback_change_pool: ShieldedProtocol, } impl SingleOutputChangeStrategy { /// Constructs a new [`SingleOutputChangeStrategy`] with the specified fee rule /// and change memo. - pub fn new(fee_rule: FixedFeeRule, change_memo: Option) -> Self { + pub fn new( + fee_rule: FixedFeeRule, + change_memo: Option, + fallback_change_pool: ShieldedProtocol, + ) -> Self { Self { fee_rule, change_memo, + fallback_change_pool, } } } @@ -65,6 +73,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy { dust_output_policy, self.fee_rule().fixed_fee(), self.change_memo.clone(), + self.fallback_change_pool, ) } } @@ -89,13 +98,15 @@ mod tests { tests::{TestSaplingInput, TestTransparentInput}, ChangeError, ChangeStrategy, ChangeValue, DustOutputPolicy, }, + ShieldedProtocol, }; #[test] fn change_without_dust() { #[allow(deprecated)] let fee_rule = FixedFeeRule::standard(); - let change_strategy = SingleOutputChangeStrategy::new(fee_rule, None); + let change_strategy = + SingleOutputChangeStrategy::new(fee_rule, None, ShieldedProtocol::Sapling); // spend a single Sapling note that is sufficient to pay the fee let result = change_strategy.compute_balance( @@ -136,7 +147,8 @@ mod tests { fn dust_change() { #[allow(deprecated)] let fee_rule = FixedFeeRule::standard(); - let change_strategy = SingleOutputChangeStrategy::new(fee_rule, None); + let change_strategy = + SingleOutputChangeStrategy::new(fee_rule, None, ShieldedProtocol::Sapling); // spend a single Sapling note that is sufficient to pay the fee let result = change_strategy.compute_balance( diff --git a/zcash_client_backend/src/fees/standard.rs b/zcash_client_backend/src/fees/standard.rs index d9d40b1c3..fcc36e707 100644 --- a/zcash_client_backend/src/fees/standard.rs +++ b/zcash_client_backend/src/fees/standard.rs @@ -14,6 +14,8 @@ use zcash_primitives::{ }, }; +use crate::ShieldedProtocol; + use super::{ fixed, sapling as sapling_fees, zip317, ChangeError, ChangeStrategy, DustOutputPolicy, TransactionBalance, @@ -27,15 +29,21 @@ use super::orchard as orchard_fees; pub struct SingleOutputChangeStrategy { fee_rule: StandardFeeRule, change_memo: Option, + fallback_change_pool: ShieldedProtocol, } impl SingleOutputChangeStrategy { /// Constructs a new [`SingleOutputChangeStrategy`] with the specified ZIP 317 /// fee parameters. - pub fn new(fee_rule: StandardFeeRule, change_memo: Option) -> Self { + pub fn new( + fee_rule: StandardFeeRule, + change_memo: Option, + fallback_change_pool: ShieldedProtocol, + ) -> Self { Self { fee_rule, change_memo, + fallback_change_pool, } } } @@ -63,6 +71,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy { StandardFeeRule::PreZip313 => fixed::SingleOutputChangeStrategy::new( FixedFeeRule::non_standard(NonNegativeAmount::const_from_u64(10000)), self.change_memo.clone(), + self.fallback_change_pool, ) .compute_balance( params, @@ -78,6 +87,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy { StandardFeeRule::Zip313 => fixed::SingleOutputChangeStrategy::new( FixedFeeRule::non_standard(NonNegativeAmount::const_from_u64(1000)), self.change_memo.clone(), + self.fallback_change_pool, ) .compute_balance( params, @@ -93,6 +103,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy { StandardFeeRule::Zip317 => zip317::SingleOutputChangeStrategy::new( Zip317FeeRule::standard(), self.change_memo.clone(), + self.fallback_change_pool, ) .compute_balance( params, diff --git a/zcash_client_backend/src/fees/zip317.rs b/zcash_client_backend/src/fees/zip317.rs index f09f50e9e..18f0678fd 100644 --- a/zcash_client_backend/src/fees/zip317.rs +++ b/zcash_client_backend/src/fees/zip317.rs @@ -13,6 +13,8 @@ use zcash_primitives::{ }, }; +use crate::ShieldedProtocol; + use super::{ common::single_change_output_balance, sapling as sapling_fees, ChangeError, ChangeStrategy, DustOutputPolicy, TransactionBalance, @@ -26,15 +28,21 @@ use super::orchard as orchard_fees; pub struct SingleOutputChangeStrategy { fee_rule: Zip317FeeRule, change_memo: Option, + fallback_change_pool: ShieldedProtocol, } impl SingleOutputChangeStrategy { /// Constructs a new [`SingleOutputChangeStrategy`] with the specified ZIP 317 /// fee parameters and change memo. - pub fn new(fee_rule: Zip317FeeRule, change_memo: Option) -> Self { + pub fn new( + fee_rule: Zip317FeeRule, + change_memo: Option, + fallback_change_pool: ShieldedProtocol, + ) -> Self { Self { fee_rule, change_memo, + fallback_change_pool, } } } @@ -145,6 +153,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy { dust_output_policy, self.fee_rule.marginal_fee(), self.change_memo.clone(), + self.fallback_change_pool, ) } } @@ -170,11 +179,16 @@ mod tests { tests::{TestSaplingInput, TestTransparentInput}, ChangeError, ChangeStrategy, ChangeValue, DustOutputPolicy, }, + ShieldedProtocol, }; #[test] fn change_without_dust() { - let change_strategy = SingleOutputChangeStrategy::new(Zip317FeeRule::standard(), None); + let change_strategy = SingleOutputChangeStrategy::new( + Zip317FeeRule::standard(), + None, + ShieldedProtocol::Sapling, + ); // spend a single Sapling note that is sufficient to pay the fee let result = change_strategy.compute_balance( @@ -213,7 +227,11 @@ mod tests { #[test] fn change_with_transparent_payments() { - let change_strategy = SingleOutputChangeStrategy::new(Zip317FeeRule::standard(), None); + let change_strategy = SingleOutputChangeStrategy::new( + Zip317FeeRule::standard(), + None, + ShieldedProtocol::Sapling, + ); // spend a single Sapling note that is sufficient to pay the fee let result = change_strategy.compute_balance( @@ -252,7 +270,11 @@ mod tests { #[test] fn change_with_allowable_dust() { - let change_strategy = SingleOutputChangeStrategy::new(Zip317FeeRule::standard(), None); + let change_strategy = SingleOutputChangeStrategy::new( + Zip317FeeRule::standard(), + None, + ShieldedProtocol::Sapling, + ); // spend a single Sapling note that is sufficient to pay the fee let result = change_strategy.compute_balance( @@ -296,7 +318,11 @@ mod tests { #[test] fn change_with_disallowed_dust() { - let change_strategy = SingleOutputChangeStrategy::new(Zip317FeeRule::standard(), None); + let change_strategy = SingleOutputChangeStrategy::new( + Zip317FeeRule::standard(), + None, + ShieldedProtocol::Sapling, + ); // spend a single Sapling note that is sufficient to pay the fee let result = change_strategy.compute_balance( diff --git a/zcash_client_backend/src/lib.rs b/zcash_client_backend/src/lib.rs index c71fb2026..a4d88cee7 100644 --- a/zcash_client_backend/src/lib.rs +++ b/zcash_client_backend/src/lib.rs @@ -97,7 +97,6 @@ pub enum ShieldedProtocol { /// The Sapling protocol Sapling, /// The Orchard protocol - #[cfg(zcash_unstable = "orchard")] Orchard, } @@ -118,7 +117,6 @@ impl PoolType { Address::Unified(ua) => match self { PoolType::Transparent => ua.transparent().is_some(), PoolType::Shielded(ShieldedProtocol::Sapling) => ua.sapling().is_some(), - #[cfg(zcash_unstable = "orchard")] PoolType::Shielded(ShieldedProtocol::Orchard) => { #[cfg(feature = "orchard")] return ua.orchard().is_some(); @@ -136,7 +134,6 @@ impl fmt::Display for PoolType { match self { PoolType::Transparent => f.write_str("Transparent"), PoolType::Shielded(ShieldedProtocol::Sapling) => f.write_str("Sapling"), - #[cfg(zcash_unstable = "orchard")] PoolType::Shielded(ShieldedProtocol::Orchard) => f.write_str("Orchard"), } } diff --git a/zcash_client_backend/src/proposal.rs b/zcash_client_backend/src/proposal.rs index d16d2819a..b0fad6406 100644 --- a/zcash_client_backend/src/proposal.rs +++ b/zcash_client_backend/src/proposal.rs @@ -490,6 +490,59 @@ impl Step { pub fn is_shielding(&self) -> bool { self.is_shielding } + + /// Returns whether or not this proposal requires interaction with the specified pool + pub fn involves(&self, pool_type: PoolType) -> bool { + match pool_type { + PoolType::Transparent => { + self.is_shielding + || !self.transparent_inputs.is_empty() + || self + .payment_pools() + .values() + .any(|pool| matches!(pool, PoolType::Transparent)) + } + PoolType::Shielded(ShieldedProtocol::Sapling) => { + let sapling_in = self.shielded_inputs.iter().any(|s_in| { + s_in.notes() + .iter() + .any(|note| matches!(note.note(), Note::Sapling(_))) + }); + let sapling_out = self + .payment_pools() + .values() + .any(|pool| matches!(pool, PoolType::Shielded(ShieldedProtocol::Sapling))); + let sapling_change = self + .balance + .proposed_change() + .iter() + .any(|c| c.output_pool() == ShieldedProtocol::Sapling); + + sapling_in || sapling_out || sapling_change + } + #[cfg(not(feature = "orchard"))] + PoolType::Shielded(ShieldedProtocol::Orchard) => false, + #[cfg(feature = "orchard")] + PoolType::Shielded(ShieldedProtocol::Orchard) => { + let orchard_in = self.shielded_inputs.iter().any(|s_in| { + s_in.notes() + .iter() + .any(|note| matches!(note.note(), Note::Orchard(_))) + }); + let orchard_out = self + .payment_pools() + .values() + .any(|pool| matches!(pool, PoolType::Shielded(ShieldedProtocol::Orchard))); + let orchard_change = self + .balance + .proposed_change() + .iter() + .any(|c| c.output_pool() == ShieldedProtocol::Orchard); + + orchard_in || orchard_out || orchard_change + } + } + } } impl Debug for Step { @@ -501,6 +554,7 @@ impl Debug for Step { "shielded_inputs", &self.shielded_inputs().map(|i| i.notes.len()), ) + .field("prior_step_inputs", &self.prior_step_inputs) .field( "anchor_height", &self.shielded_inputs().map(|i| i.anchor_height), diff --git a/zcash_client_backend/src/proto.rs b/zcash_client_backend/src/proto.rs index 088843353..d74381609 100644 --- a/zcash_client_backend/src/proto.rs +++ b/zcash_client_backend/src/proto.rs @@ -319,7 +319,6 @@ fn pool_type(pool_id: i32) -> Result> { match proposal::ValuePool::try_from(pool_id) { Ok(proposal::ValuePool::Transparent) => Ok(PoolType::Transparent), Ok(proposal::ValuePool::Sapling) => Ok(PoolType::Shielded(ShieldedProtocol::Sapling)), - #[cfg(zcash_unstable = "orchard")] Ok(proposal::ValuePool::Orchard) => Ok(PoolType::Shielded(ShieldedProtocol::Orchard)), _ => Err(ProposalDecodingError::ValuePoolNotSupported(pool_id)), } @@ -354,7 +353,6 @@ impl From for proposal::ValuePool { fn from(value: ShieldedProtocol) -> Self { match value { ShieldedProtocol::Sapling => proposal::ValuePool::Sapling, - #[cfg(zcash_unstable = "orchard")] ShieldedProtocol::Orchard => proposal::ValuePool::Orchard, } } diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index 52a0a7df5..2ad5eb1ec 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/zcash_client_sqlite/src/chain.rs @@ -340,6 +340,7 @@ mod tests { scanning::ScanError, wallet::OvkPolicy, zip321::{Payment, TransactionRequest}, + ShieldedProtocol, }; use crate::{ @@ -529,7 +530,7 @@ mod tests { }]) .unwrap(); let input_selector = GreedyInputSelector::new( - SingleOutputChangeStrategy::new(FeeRule::standard(), None), + SingleOutputChangeStrategy::new(FeeRule::standard(), None, ShieldedProtocol::Sapling), DustOutputPolicy::default(), ); assert_matches!( diff --git a/zcash_client_sqlite/src/testing.rs b/zcash_client_sqlite/src/testing.rs index a98429beb..84330a4de 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -22,7 +22,6 @@ use sapling::{ zip32::DiversifiableFullViewingKey, Note, Nullifier, PaymentAddress, }; -use zcash_client_backend::fees::{standard, DustOutputPolicy}; #[allow(deprecated)] use zcash_client_backend::{ address::Address, @@ -45,6 +44,10 @@ use zcash_client_backend::{ wallet::OvkPolicy, zip321, }; +use zcash_client_backend::{ + fees::{standard, DustOutputPolicy}, + ShieldedProtocol, +}; use zcash_note_encryption::Domain; use zcash_primitives::{ block::BlockHash, @@ -465,6 +468,7 @@ impl TestState { ovk_policy, min_confirmations, change_memo, + ShieldedProtocol::Sapling, ) } @@ -567,6 +571,7 @@ impl TestState { amount, memo, change_memo, + ShieldedProtocol::Sapling, ); if let Ok(proposal) = &result { @@ -1058,7 +1063,8 @@ pub(crate) fn input_selector( standard::SingleOutputChangeStrategy, > { let change_memo = change_memo.map(|m| MemoBytes::from(m.parse::().unwrap())); - let change_strategy = standard::SingleOutputChangeStrategy::new(fee_rule, change_memo); + let change_strategy = + standard::SingleOutputChangeStrategy::new(fee_rule, change_memo, ShieldedProtocol::Sapling); GreedyInputSelector::new(change_strategy, DustOutputPolicy::default()) } diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 321a54dcf..b0b1aa03e 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -137,7 +137,6 @@ pub(crate) fn pool_code(pool_type: PoolType) -> i64 { match pool_type { PoolType::Transparent => 0i64, PoolType::Shielded(ShieldedProtocol::Sapling) => 2i64, - #[cfg(zcash_unstable = "orchard")] PoolType::Shielded(ShieldedProtocol::Orchard) => 3i64, } } @@ -818,7 +817,6 @@ pub(crate) fn get_received_memo( ) .optional()? .flatten(), - #[cfg(zcash_unstable = "orchard")] _ => { return Err(SqliteClientError::UnsupportedPoolType(PoolType::Shielded( note_id.protocol(), @@ -2232,6 +2230,8 @@ mod tests { #[test] #[cfg(feature = "transparent-inputs")] fn transparent_balance_across_shielding() { + use zcash_client_backend::ShieldedProtocol; + let mut st = TestBuilder::new() .with_block_cache() .with_test_account(AccountBirthday::from_sapling_activation) @@ -2316,6 +2316,7 @@ mod tests { fixed::SingleOutputChangeStrategy::new( FixedFeeRule::non_standard(NonNegativeAmount::ZERO), None, + ShieldedProtocol::Sapling, ), DustOutputPolicy::default(), ); diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index b7401cf7f..c6cf86a15 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -579,8 +579,11 @@ pub(crate) mod tests { let fee_rule = StandardFeeRule::PreZip313; let change_memo = "Test change memo".parse::().unwrap(); - let change_strategy = - standard::SingleOutputChangeStrategy::new(fee_rule, Some(change_memo.clone().into())); + let change_strategy = standard::SingleOutputChangeStrategy::new( + fee_rule, + Some(change_memo.clone().into()), + ShieldedProtocol::Sapling, + ); let input_selector = &GreedyInputSelector::new(change_strategy, DustOutputPolicy::default()); @@ -731,7 +734,7 @@ pub(crate) mod tests { let fee_rule = StandardFeeRule::Zip317; let input_selector = GreedyInputSelector::new( - standard::SingleOutputChangeStrategy::new(fee_rule, None), + standard::SingleOutputChangeStrategy::new(fee_rule, None, ShieldedProtocol::Sapling), DustOutputPolicy::default(), ); let proposal0 = st @@ -1455,7 +1458,7 @@ pub(crate) mod tests { #[allow(deprecated)] let fee_rule = FixedFeeRule::standard(); let input_selector = GreedyInputSelector::new( - fixed::SingleOutputChangeStrategy::new(fee_rule, None), + fixed::SingleOutputChangeStrategy::new(fee_rule, None, ShieldedProtocol::Sapling), DustOutputPolicy::default(), ); @@ -1657,7 +1660,7 @@ pub(crate) mod tests { let fee_rule = StandardFeeRule::PreZip313; let input_selector = GreedyInputSelector::new( - standard::SingleOutputChangeStrategy::new(fee_rule, None), + standard::SingleOutputChangeStrategy::new(fee_rule, None, ShieldedProtocol::Sapling), DustOutputPolicy::default(), ); diff --git a/zcash_keys/CHANGELOG.md b/zcash_keys/CHANGELOG.md index 752a27f29..56fab2845 100644 --- a/zcash_keys/CHANGELOG.md +++ b/zcash_keys/CHANGELOG.md @@ -14,7 +14,7 @@ The entries below are relative to the `zcash_client_backend` crate as of - `address` - `encoding` - `keys` -- `zcash_keys::address::UnifiedAddress::unknown`: +- `zcash_keys::address::UnifiedAddress::{unknown, has_orchard, has_sapling, has_transparent}`: - `zcash_keys::keys`: - `AddressGenerationError` - `UnifiedAddressRequest` diff --git a/zcash_keys/src/address.rs b/zcash_keys/src/address.rs index dcb3b8a61..7ba589f82 100644 --- a/zcash_keys/src/address.rs +++ b/zcash_keys/src/address.rs @@ -108,17 +108,37 @@ impl UnifiedAddress { } } + /// Returns whether this address has an Orchard receiver. + /// + /// This method is available irrespective of whether the `orchard` feature flag is enabled. + pub fn has_orchard(&self) -> bool { + #[cfg(not(feature = "orchard"))] + return false; + #[cfg(feature = "orchard")] + return self.orchard.is_some(); + } + /// Returns the Orchard receiver within this Unified Address, if any. #[cfg(feature = "orchard")] pub fn orchard(&self) -> Option<&orchard::Address> { self.orchard.as_ref() } + /// Returns whether this address has a Sapling receiver. + pub fn has_sapling(&self) -> bool { + self.sapling.is_some() + } + /// Returns the Sapling receiver within this Unified Address, if any. pub fn sapling(&self) -> Option<&PaymentAddress> { self.sapling.as_ref() } + /// Returns whether this address has a Transparent receiver. + pub fn has_transparent(&self) -> bool { + self.transparent.is_some() + } + /// Returns the transparent receiver within this Unified Address, if any. pub fn transparent(&self) -> Option<&TransparentAddress> { self.transparent.as_ref()