zcash_client_backend: Address comments from code review & fix incorrect `data_api::wallet` documentation.

zcash_client_backend/CHANGELOG.md

@@ -52,13 +52,15 @@ and this library adheres to Rust's notion of
   - `wallet::input_selection::ShieldingSelector` has been
     factored out from the `InputSelector` trait to separate out transparent
     functionality and move it behind the `transparent-inputs` feature flag.
+  - `impl std::error::Error for wallet::input_selection::InputSelectorError`
 - `zcash_client_backend::fees::{standard, sapling}`
 - `zcash_client_backend::fees::ChangeValue::new`
 - `zcash_client_backend::wallet`:
   - `Note`
   - `ReceivedNote`
   - `WalletSaplingOutput::recipient_key_scope`
-  - `wallet::TransparentAddressMetadata` (which replaces `zcash_keys::address::AddressMetadata`).
+  - `TransparentAddressMetadata` (which replaces `zcash_keys::address::AddressMetadata`).
+  - `impl {Debug, Clone} for OvkPolicy`
 - `zcash_client_backend::zip321::TransactionRequest::total`
 - `zcash_client_backend::zip321::parse::Param::name`
 - `zcash_client_backend::proposal`:
@@ -90,8 +92,6 @@ and this library adheres to Rust's notion of
 - `ScannedBlock::{sapling_tree_size, sapling_nullifier_map, sapling_commitments}`
   have been moved to `ScannedBlockSapling` and in that context are now
   named `{tree_size, nullifier_map, commitments}` respectively.
-- `zcash_client_backend::::wallet::input_selection::{Proposal, ShieldedInputs, ProposalError}`
-  have been moved to `zcash_client_backend::proposal`.
 
 ### Changed
 - `zcash_client_backend::data_api`:
@@ -226,6 +226,9 @@ and this library adheres to Rust's notion of
   indices may be preserved.
 - `zcash_client_backend::zip321::to_uri` now returns a `String` rather than an
   `Option<String>` and provides canonical serialization for the empty proposal.
+- `zcash_client_backend::zip321::from_uri` previously stripped payment indices,
+  meaning that round-trip serialization was not supported. Payment indices are
+  now retained.
 - The following fields now have type `NonNegativeAmount` instead of `Amount`:
   - `zcash_client_backend::data_api`:
     - `error::Error::InsufficientFunds.{available, required}`
@@ -256,7 +259,8 @@ and this library adheres to Rust's notion of
 ### Removed
 - `zcash_client_backend::wallet::ReceivedSaplingNote` has been replaced by
   `zcash_client_backend::ReceivedNote`.
-- `zcash_client_backend::wallet::input_selection::Proposal`
+- `zcash_client_backend::::wallet::input_selection::{Proposal, ShieldedInputs, ProposalError}`
+  have been moved to `zcash_client_backend::proposal`.
 - `zcash_client_backend::data_api`
 - `zcash_client_backend::data_api::ScannedBlock::from_parts` has been made crate-private.
 - `zcash_client_backend::data_api::ScannedBlock::into_sapling_commitments` has been

zcash_client_backend/proto/proposal.proto

@@ -5,7 +5,7 @@
 syntax = "proto3";
 package cash.z.wallet.sdk.ffi;
 
-// A data structure that describes the a series of transactions to be created.
+// A data structure that describes a series of transactions to be created.
 message Proposal {
     // The version of this serialization format.
     uint32 protoVersion = 1;

zcash_client_backend/src/data_api/wallet.rs

@@ -79,11 +79,11 @@ where
 }
 
 #[allow(clippy::needless_doctest_main)]
-/// Creates a transaction paying the specified address from the given account.
+/// Creates a transaction or series of transactions paying the specified address from
+/// the given account, and the [`TxId`] corresponding to each newly-created transaction.
 ///
-/// Returns the row index of the newly-created transaction in the `transactions` table
+/// These transactions can be retrieved from the underlying data store using the
-/// within the data database. The caller can read the raw transaction bytes from the `raw`
+/// [`WalletRead::get_transaction`] method.
-/// column in order to broadcast the transaction to the network.
 ///
 /// Do not call this multiple times in parallel, or you will generate transactions that
 /// double-spend the same notes.
@@ -251,15 +251,12 @@ where
     )
 }
 
-/// Constructs a transaction that sends funds as specified by the `request` argument
+/// Constructs a transaction or series of transactions that send funds as specified
-/// and stores it to the wallet's "sent transactions" data store, and returns a
+/// by the `request` argument, stores them to the wallet's "sent transactions" data
-/// unique identifier for the transaction; this identifier is used only for internal
+/// store, and returns the [`TxId`] for each transaction constructed.
-/// reference purposes and is not the same as the transaction's txid, although after v4
-/// transactions have been made invalid in a future network upgrade, the txid could
-/// potentially be used for this type (as it is non-malleable for v5+ transactions).
 ///
-/// This procedure uses the wallet's underlying note selection algorithm to choose
+/// The newly-created transactions can be retrieved from the underlying data store using the
-/// inputs of sufficient value to satisfy the request, if possible.
+/// [`WalletRead::get_transaction`] method.
 ///
 /// Do not call this multiple times in parallel, or you will generate transactions that
 /// double-spend the same notes.
@@ -357,8 +354,8 @@ where
     )
 }
 
-/// Select transaction inputs, compute fees, and construct a proposal for a transaction
+/// Select transaction inputs, compute fees, and construct a proposal for a transaction or series
-/// that can then be authorized and made ready for submission to the network with
+/// of transactions that can then be authorized and made ready for submission to the network with
 /// [`create_proposed_transaction`].
 #[allow(clippy::too_many_arguments)]
 #[allow(clippy::type_complexity)]
@@ -401,9 +398,13 @@ where
         .map_err(Error::from)
 }
 
-/// Proposes a transaction paying the specified address from the given account.
+/// Proposes making a payment to the specified address from the given account.
 ///
-/// Returns the proposal, which may then be executed using [`create_proposed_transaction`]
+/// Returns the proposal, which may then be executed using [`create_proposed_transaction`].
+/// Depending upon the recipient address, more than one transaction may be constructed
+/// in the execution of the returned proposal.
+///
+/// This method uses the basic [`GreedyInputSelector`] for input selection.
 ///
 /// Parameters:
 /// * `wallet_db`: A read/write reference to the wallet database.
@@ -472,6 +473,8 @@ where
     )
 }
 
+/// Constructs a proposal to shield all of the funds belonging to the provided set of
+/// addresses.
 #[cfg(feature = "transparent-inputs")]
 #[allow(clippy::too_many_arguments)]
 #[allow(clippy::type_complexity)]
@@ -513,14 +516,16 @@ where
         .map_err(Error::from)
 }
 
-/// Construct, prove, and sign a transaction using the inputs supplied by the given proposal,
+/// Construct, prove, and sign a transaction or series of transactions using the inputs supplied by
-/// and persist it to the wallet database.
+/// the given proposal, and persist it to the wallet database.
 ///
-/// Returns the database identifier for the newly constructed transaction, or an error if
+/// Returns the database identifier for eacy newly constructed transaction, or an error if
 /// an error occurs in transaction construction, proving, or signing.
 ///
-/// Note: If the payment includes a recipient with an Orchard-only UA, this will attempt
+/// When evaluating multi-step proposals, only transparent outputs of any given step may be spent
-/// to fall back to the transparent receiver until full Orchard support is implemented.
+/// in later steps; attempting to spend a shielded note (including change) output by an earlier
+/// step is not supported, because the ultimate positions of those notes cannot be known and
+/// therefore the required spend proofs for such notes cannot be constructed.
 #[allow(clippy::too_many_arguments)]
 #[allow(clippy::type_complexity)]
 pub fn create_proposed_transactions<DbT, ParamsT, InputsErrT, FeeRuleT, N>(
@@ -964,13 +969,13 @@ where
     Ok(build_result)
 }
 
-/// Constructs a transaction that consumes available transparent UTXOs belonging to
+/// Constructs a transaction that consumes available transparent UTXOs belonging to the specified
-/// the specified secret key, and sends them to the default address for the provided Sapling
+/// secret key, and sends them to the most-preferred receiver of the default internal address for
-/// extended full viewing key.
+/// the provided Unified Spending Key.
 ///
 /// This procedure will not attempt to shield transparent funds if the total amount being shielded
-/// is less than the default fee to send the transaction. Fees will be paid only from the transparent
+/// is less than the default fee to send the transaction. Fees will be paid only from the
-/// UTXOs being consumed.
+/// transparent UTXOs being consumed.
 ///
 /// Parameters:
 /// * `wallet_db`: A read/write reference to the wallet database

zcash_client_backend/src/proposal.rs

@@ -1,3 +1,5 @@
+//! Types related to the construction and evaluation of transaction proposals.
+
 use std::{
     collections::BTreeMap,
     fmt::{self, Debug, Display},
@@ -372,7 +374,7 @@ impl<NoteRef> Step<NoteRef> {
     pub fn transparent_inputs(&self) -> &[WalletTransparentOutput] {
         &self.transparent_inputs
     }
-    /// Returns the Sapling inputs that have been selected to fund the transaction.
+    /// Returns the shielded inputs that have been selected to fund the transaction.
     pub fn shielded_inputs(&self) -> Option<&ShieldedInputs<NoteRef>> {
         self.shielded_inputs.as_ref()
     }

zcash_client_backend/src/proto.rs

@@ -458,7 +458,6 @@ impl proposal::Proposal {
                     fee_required: step.balance().fee_required().into(),
                 });
 
-                #[allow(deprecated)]
                 proposal::ProposalStep {
                     transaction_request,
                     payment_output_pools,

zcash_client_backend/src/proto/proposal.rs

@@ -1,4 +1,4 @@
-/// A data structure that describes the a series of transactions to be created.
+/// A data structure that describes a series of transactions to be created.
 #[allow(clippy::derive_partial_eq_without_eq)]
 #[derive(Clone, PartialEq, ::prost::Message)]
 pub struct Proposal {