Merge pull request #1590 from nuttycom/wallet/remove_deprecated

zcash_client_backend: Remove deprecated and duplicative wallet APIs.

zcash_client_backend/CHANGELOG.md

@@ -13,10 +13,20 @@ and this library adheres to Rust's notion of
   - `WalletSummary::progress`
   - `WalletMeta`
   - `impl Default for wallet::input_selection::GreedyInputSelector`
-- `zcash_client_backend::fees::SplitPolicy`
-- `zcash_client_backend::fees::zip317::MultiOutputChangeStrategy`
+- `zcash_client_backend::fees`
+  - `SplitPolicy`
+  - `StandardFeeRule` has been moved here from `zcash_primitives::fees`. Relative
+    to that type, the deprecated `PreZip313` and `Zip313` variants have been
+    removed.
+  - `zip317::{MultiOutputChangeStrategy, Zip317FeeRule}`
+  - `standard::MultiOutputChangeStrategy`
+- A new feature flag, `non-standard-fees`, has been added. This flag is now
+  required in order to make use of any types or methods that enable non-standard
+  fee calculation.
 
 ### Changed
+- MSRV is now 1.77.0.
+- Migrated to `arti-client 0.23`.
 - `zcash_client_backend::data_api`:
   - `InputSource` has an added method `get_wallet_metadata`
   - `error::Error` has additional variant `Error::Change`. This necessitates
@@ -54,22 +64,28 @@ and this library adheres to Rust's notion of
     and `WalletMeta`, and its `FeeRule` associated type now has an additional
     `Clone` bound. In addition, it defines a new `fetch_wallet_meta` method, and
     the arguments to `compute_balance` have changed.
+  - `zip317::SingleOutputChangeStrategy` has been made polymorphic in the fee
+    rule type, and takes an additional type parameter as a consequence.
   - The following methods now take an additional `DustOutputPolicy` argument,
     and carry an additional type parameter:
     - `fixed::SingleOutputChangeStrategy::new`
     - `standard::SingleOutputChangeStrategy::new`
     - `zip317::SingleOutputChangeStrategy::new`
-
-### Changed
-- MSRV is now 1.77.0.
-- Migrated to `arti-client 0.23`.
+- `zcash_client_backend::proto::ProposalDecodingError` has modified variants.
+  `ProposalDecodingError::FeeRuleNotSpecified` has been removed, and
+  `ProposalDecodingError::FeeRuleNotSupported` has been added to replace it.
+- `zcash_client_backend::data_api::fees::fixed` is now available only via the
+  use of the `non-standard-fees` feature flag.
 
 ### Removed
 - `zcash_client_backend::data_api`:
   - `WalletSummary::scan_progress` and `WalletSummary::recovery_progress` have
     been removed. Use `WalletSummary::progress` instead.
   - `testing::input_selector` use explicit `InputSelector` constructors
     directly instead.
+  - The deprecated `wallet::create_spend_to_address` and `wallet::spend`
+    methods have been removed. Use `propose_transfer` and
+    `create_proposed_transaction` instead.
 - `zcash_client_backend::fees`:
   - `impl From<BalanceError> for ChangeError<...>`
 

zcash_client_backend/Cargo.toml

@@ -211,6 +211,9 @@ test-dependencies = [
     "incrementalmerkletree/test-dependencies",
 ]
 
+## Exposes APIs that allow calculation of non-standard fees.
+non-standard-fees = ["zcash_primitives/non-standard-fees"]
+
 #! ### Experimental features
 
 ## Exposes unstable APIs. Their behaviour may change at any time.

zcash_client_backend/src/data_api/testing.rs

@@ -31,7 +31,7 @@ use zcash_primitives::{
     memo::Memo,
     transaction::{
         components::{amount::NonNegativeAmount, sapling::zip212_enforcement},
-        fees::{FeeRule, StandardFeeRule},
+        fees::FeeRule,
         Transaction, TxId,
     },
 };
@@ -48,7 +48,7 @@ use crate::{
     address::UnifiedAddress,
     fees::{
         standard::{self, SingleOutputChangeStrategy},
-        ChangeStrategy, DustOutputPolicy,
+        ChangeStrategy, DustOutputPolicy, StandardFeeRule,
     },
     keys::{UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey},
     proposal::Proposal,
@@ -59,14 +59,14 @@ use crate::{
     ShieldedProtocol,
 };
 
-#[allow(deprecated)]
+use super::error::Error;
 use super::{
     chain::{scan_cached_blocks, BlockSource, ChainState, CommitmentTreeRoot, ScanSummary},
     scanning::ScanRange,
     wallet::{
-        create_proposed_transactions, create_spend_to_address,
+        create_proposed_transactions,
         input_selection::{GreedyInputSelector, InputSelector},
-        propose_standard_transfer_to_address, propose_transfer, spend,
+        propose_standard_transfer_to_address, propose_transfer,
     },
     Account, AccountBalance, AccountBirthday, AccountPurpose, AccountSource, BlockMetadata,
     DecryptedTransaction, InputSource, NullifierQuery, ScannedBlock, SeedRelevance,
@@ -861,43 +861,7 @@ where
         + WalletCommitmentTrees,
     <DbT as WalletRead>::AccountId: ConditionallySelectable + Default + Send + 'static,
 {
-    /// Invokes [`create_spend_to_address`] with the given arguments.
-    #[allow(deprecated)]
-    #[allow(clippy::type_complexity)]
-    #[allow(clippy::too_many_arguments)]
-    pub fn create_spend_to_address(
-        &mut self,
-        usk: &UnifiedSpendingKey,
-        to: &Address,
-        amount: NonNegativeAmount,
-        memo: Option<MemoBytes>,
-        ovk_policy: OvkPolicy,
-        min_confirmations: NonZeroU32,
-        change_memo: Option<MemoBytes>,
-        fallback_change_pool: ShieldedProtocol,
-    ) -> Result<
-        NonEmpty<TxId>,
-        super::wallet::TransferErrT<DbT, GreedyInputSelector<DbT>, SingleOutputChangeStrategy<DbT>>,
-    > {
-        let prover = LocalTxProver::bundled();
-        let network = self.network().clone();
-        create_spend_to_address(
-            self.wallet_mut(),
-            &network,
-            &prover,
-            &prover,
-            usk,
-            to,
-            amount,
-            memo,
-            ovk_policy,
-            min_confirmations,
-            change_memo,
-            fallback_change_pool,
-        )
-    }
-
-    /// Invokes [`spend`] with the given arguments.
+    /// Prepares and executes the given [`zip321::TransactionRequest`] in a single step.
     #[allow(clippy::type_complexity)]
     pub fn spend<InputsT, ChangeT>(
         &mut self,
@@ -912,20 +876,33 @@ where
         InputsT: InputSelector<InputSource = DbT>,
         ChangeT: ChangeStrategy<MetaSource = DbT>,
     {
-        #![allow(deprecated)]
         let prover = LocalTxProver::bundled();
         let network = self.network().clone();
-        spend(
+
+        let account = self
+            .wallet()
+            .get_account_for_ufvk(&usk.to_unified_full_viewing_key())
+            .map_err(Error::DataSource)?
+            .ok_or(Error::KeyNotRecognized)?;
+
+        let proposal = propose_transfer(
             self.wallet_mut(),
             &network,
-            &prover,
-            &prover,
+            account.id(),
             input_selector,
             change_strategy,
-            usk,
             request,
-            ovk_policy,
             min_confirmations,
+        )?;
+
+        create_proposed_transactions(
+            self.wallet_mut(),
+            &network,
+            &prover,
+            &prover,
+            usk,
+            ovk_policy,
+            &proposal,
         )
     }
 

zcash_client_backend/src/data_api/testing/pool.rs

@@ -17,7 +17,7 @@ use zcash_primitives::{
     legacy::TransparentAddress,
     transaction::{
         components::amount::NonNegativeAmount,
-        fees::{fixed::FeeRule as FixedFeeRule, zip317::FeeRule as Zip317FeeRule, StandardFeeRule},
+        fees::zip317::{FeeRule as Zip317FeeRule, MARGINAL_FEE, MINIMUM_FEE},
         Transaction,
     },
 };
@@ -50,7 +50,7 @@ use crate::{
     fees::{
         self,
         standard::{self, SingleOutputChangeStrategy},
-        DustOutputPolicy, SplitPolicy,
+        DustOutputPolicy, SplitPolicy, StandardFeeRule,
     },
     scanning::ScanError,
     wallet::{Note, NoteId, OvkPolicy, ReceivedNote},
@@ -961,9 +961,8 @@ pub fn proposal_fails_if_not_all_ephemeral_outputs_consumed<T: ShieldedPoolTeste
     );
 }
 
-#[allow(deprecated)]
-pub fn create_to_address_fails_on_incorrect_usk<T: ShieldedPoolTester>(
-    ds_factory: impl DataStoreFactory,
+pub fn create_to_address_fails_on_incorrect_usk<T: ShieldedPoolTester, DSF: DataStoreFactory>(
+    ds_factory: DSF,
 ) {
     let mut st = TestBuilder::new()
         .with_data_store_factory(ds_factory)
@@ -976,23 +975,30 @@ pub fn create_to_address_fails_on_incorrect_usk<T: ShieldedPoolTester>(
     let acct1 = zip32::AccountId::try_from(1).unwrap();
     let usk1 = UnifiedSpendingKey::from_seed(st.network(), &[1u8; 32], acct1).unwrap();
 
+    let input_selector = GreedyInputSelector::<DSF::DataStore>::new();
+    let change_strategy =
+        single_output_change_strategy(StandardFeeRule::Zip317, None, T::SHIELDED_PROTOCOL);
+
+    let req = TransactionRequest::new(vec![Payment::without_memo(
+        to.to_zcash_address(st.network()),
+        NonNegativeAmount::const_from_u64(1),
+    )])
+    .unwrap();
+
     // Attempting to spend with a USK that is not in the wallet results in an error
     assert_matches!(
-        st.create_spend_to_address(
+        st.spend(
+            &input_selector,
+            &change_strategy,
             &usk1,
-            &to,
-            NonNegativeAmount::const_from_u64(1),
-            None,
+            req,
             OvkPolicy::Sender,
             NonZeroU32::new(1).unwrap(),
-            None,
-            T::SHIELDED_PROTOCOL,
         ),
         Err(data_api::error::Error::KeyNotRecognized)
     );
 }
 
-#[allow(deprecated)]
 pub fn proposal_fails_with_no_blocks<T: ShieldedPoolTester, DSF>(ds_factory: DSF)
 where
     DSF: DataStoreFactory,
@@ -1014,7 +1020,7 @@ where
     assert_matches!(
         st.propose_standard_transfer::<Infallible>(
             account_id,
-            StandardFeeRule::PreZip313,
+            StandardFeeRule::Zip317,
             NonZeroU32::new(1).unwrap(),
             &to,
             NonNegativeAmount::const_from_u64(1),
@@ -1581,10 +1587,8 @@ pub fn external_address_change_spends_detected_in_restore_from_seed<T: ShieldedP
     ])
     .unwrap();
 
-    #[allow(deprecated)]
-    let fee_rule = FixedFeeRule::standard();
-    let change_strategy = fees::fixed::SingleOutputChangeStrategy::new(
-        fee_rule,
+    let change_strategy = fees::standard::SingleOutputChangeStrategy::new(
+        StandardFeeRule::Zip317,
         None,
         T::SHIELDED_PROTOCOL,
         DustOutputPolicy::default(),
@@ -1602,7 +1606,7 @@ pub fn external_address_change_spends_detected_in_restore_from_seed<T: ShieldedP
         )
         .unwrap()[0];
 
-    let amount_left = (value - (amount_sent + fee_rule.fixed_fee()).unwrap()).unwrap();
+    let amount_left = (value - (amount_sent + MINIMUM_FEE + MARGINAL_FEE).unwrap()).unwrap();
     let pending_change = (amount_left - amount_legacy_change).unwrap();
 
     // The "legacy change" is not counted by get_pending_change().
@@ -1930,8 +1934,8 @@ pub fn birthday_in_anchor_shard<T: ShieldedPoolTester>(
     assert_eq!(spendable.len(), 1);
 }
 
-pub fn checkpoint_gaps<T: ShieldedPoolTester>(
-    ds_factory: impl DataStoreFactory,
+pub fn checkpoint_gaps<T: ShieldedPoolTester, DSF: DataStoreFactory>(
+    ds_factory: DSF,
     cache: impl TestCache,
 ) {
     let mut st = TestBuilder::new()
@@ -1982,18 +1986,26 @@ pub fn checkpoint_gaps<T: ShieldedPoolTester>(
     .unwrap();
     assert_eq!(spendable.len(), 1);
 
-    // Attempt to spend the note with 5 confirmations
+    let input_selector = GreedyInputSelector::<DSF::DataStore>::new();
+    let change_strategy =
+        single_output_change_strategy(StandardFeeRule::Zip317, None, T::SHIELDED_PROTOCOL);
+
     let to = T::fvk_default_address(&not_our_key);
+    let req = TransactionRequest::new(vec![Payment::without_memo(
+        to.to_zcash_address(st.network()),
+        NonNegativeAmount::const_from_u64(10000),
+    )])
+    .unwrap();
+
+    // Attempt to spend the note with 5 confirmations
     assert_matches!(
-        st.create_spend_to_address(
+        st.spend(
+            &input_selector,
+            &change_strategy,
             account.usk(),
-            &to,
-            NonNegativeAmount::const_from_u64(10000),
-            None,
+            req,
             OvkPolicy::Sender,
             NonZeroU32::new(5).unwrap(),
-            None,
-            T::SHIELDED_PROTOCOL,
         ),
         Ok(_)
     );
@@ -2799,7 +2811,6 @@ pub fn scan_cached_blocks_allows_blocks_out_of_order<T: ShieldedPoolTester>(
     )])
     .unwrap();
 
-    #[allow(deprecated)]
     let input_selector = GreedyInputSelector::new();
     let change_strategy =
         single_output_change_strategy(StandardFeeRule::Zip317, None, T::SHIELDED_PROTOCOL);

zcash_client_backend/src/data_api/testing/transparent.rs

@@ -1,21 +1,19 @@
 use crate::{
     data_api::{
-        testing::{AddressType, TestBuilder, TestState},
-        testing::{DataStoreFactory, ShieldedProtocol, TestCache},
+        testing::{
+            AddressType, DataStoreFactory, ShieldedProtocol, TestBuilder, TestCache, TestState,
+        },
         wallet::input_selection::GreedyInputSelector,
         Account as _, InputSource, WalletRead, WalletWrite,
     },
-    fees::{fixed, DustOutputPolicy},
+    fees::{standard, DustOutputPolicy, StandardFeeRule},
     wallet::WalletTransparentOutput,
 };
 use assert_matches::assert_matches;
 use sapling::zip32::ExtendedSpendingKey;
 use zcash_primitives::{
     block::BlockHash,
-    transaction::{
-        components::{amount::NonNegativeAmount, OutPoint, TxOut},
-        fees::fixed::FeeRule as FixedFeeRule,
-    },
+    transaction::components::{amount::NonNegativeAmount, OutPoint, TxOut},
 };
 
 pub fn put_received_transparent_utxo<DSF>(dsf: DSF)
@@ -198,8 +196,8 @@ where
 
     // Shield the output.
     let input_selector = GreedyInputSelector::new();
-    let change_strategy = fixed::SingleOutputChangeStrategy::new(
-        FixedFeeRule::non_standard(NonNegativeAmount::ZERO),
+    let change_strategy = standard::SingleOutputChangeStrategy::new(
+        StandardFeeRule::Zip317,
         None,
         ShieldedProtocol::Sapling,
         DustOutputPolicy::default(),