Address comments from code review.

Co-authored-by: str4d <jack@electriccoin.co>
Co-authored-by: Daira Hopwood <daira@jacaranda.org>

zcash_client_backend/CHANGELOG.md

@@ -7,34 +7,35 @@ and this library adheres to Rust's notion of
 
 ## [Unreleased]
 ### Added
-- A new feature flag, `transparent-inputs` is now available for use in 
+- Functionality that enables the receiving and spending of transparent funds,
-  compilation of `zcash_primitives`, `zcash_client_backend`, and 
+  behind the new `transparent-inputs` feature flag.
-  `zcash_client_sqlite`. This flag must be enabled to provide access to the
+  - A new `data_api::wallet::shield_transparent_funds` method has been added
-  functionality which enables the receiving and spending of transparent funds.
+    to facilitate the automatic shielding of transparent funds received
+    by the wallet.
+  - `zcash_client_backend::data_api::WalletReadTransparent` read-only operations
+    related to information the wallet maintains about transparent funds.
+  - `zcash_client_backend::data_api::WalletWriteTransparent` operations
+    related persisting information the wallet maintains about transparent funds.
+  - A `zcash_client_backend::wallet::WalletTransparentOutput` type
+    has been added under the `transparent-inputs` feature flag in support
+    of autoshielding functionality.
 - A new `data_api::wallet::spend` method has been added, which is
   intended to supersede the `data_api::wallet::create_spend_to_address`
   method. This new method now constructs transactions via interpretation
-  of a `zcash_client_backend::zip321::TransactionRequest` value. 
+  of a `zcash_client_backend::zip321::TransactionRequest` value.
   This facilitates the implementation of ZIP 321 support in wallets and
   provides substantially greater flexibility in transaction creation.
-- A new `data_api::wallet::shield_transparent_funds` method has been added
+- `zcash_client_backend::zip321::TransactionRequest` methods:
-  to facilitate the automatic shielding of transparent funds received
+  - `TransactionRequest::new` for constructing a request from `Vec<Payment>`.
-  by the wallet.
+  - `TransactionRequest::payments` for accessing the `Payments` that make up a
-- `zcash_client_backend::data_api::WalletRead::get_transaction` has been added
+    request.
-  to provide access to decoded transaction data.
+- New experimental APIs that should be considered unstable, and are
-- `zcash_client_backend::data_api::WalletRead::get_all_nullifiers` has been
+  likely to be modified and/or moved to a different module in a future
-  added. This method provides access to all Sapling nullifiers, including
+  release:
-  for notes that have been previously marked spent.
+  - `zcash_client_backend::keys::{UnifiedSpendingKey`, `UnifiedFullViewingKey`}
-- `zcash_client_backend::data_api::WalletRead::get_unspent_transparent_outputs`
+  - `zcash_client_backend::encoding::AddressCodec`
-  has been added under the `transparent-inputs` feature flag to provide access
+  - `zcash_client_backend::encoding::encode_payment_address`
-  to received transparent UTXOs.
+  - `zcash_client_backend::encoding::encode_transparent_address`
-- A new `zcash_client_backend::encoding::AddressCodec` trait has been added
-  to facilitate address encoding. This new API should be considered unstable
-  and is likely to be removed or changed in an upcoming release.
-- `zcash_client_backend::encoding::encode_payment_address` has been added.
-  This API should be considered unstable.
-- `zcash_client_backend::encoding::encode_transparent_address` has been added.
-  This API should be considered unstable.
 
 ### Changed
 - MSRV is now 1.51.0.
@@ -45,45 +46,57 @@ and this library adheres to Rust's notion of
     been replaced by `ephemeral_key`.
   - `zcash_client_backend::proto::compact_formats::CompactOutput`: the `epk`
     method has been replaced by `ephemeral_key`.
-
+- `data_api::wallet::spend_to_address` now takes a `min_confirmations` 
+  parameter, which the caller can provide to specify the minimum number of
+  confirmations required for notes being selected. A default value of 10
+  confirmations is recommended.
 - Renamed the following in `zcash_client_backend::data_api` to use lower-case
   abbreviations (matching Rust naming conventions):
   - `error::Error::InvalidExtSK` to `Error::InvalidExtSk`
   - `testing::MockWalletDB` to `testing::MockWalletDb`
-- `data_api::WalletRead::get_target_and_anchor_heights` now takes 
+- Changes to the `data_api::WalletRead` trait:
-  a `min_confirmations` argument that is used to compute an upper bound on the
+  - `WalletRead::get_target_and_anchor_heights` now takes
-  anchor height being returned; this had previously been hardcoded to
+    a `min_confirmations` argument that is used to compute an upper bound on
-  `data_api::wallet::ANCHOR_OFFSET`.
+    the anchor height being returned; this had previously been hardcoded to
-- `data_api::WalletRead::get_spendable_notes` has been renamed to 
+    `data_api::wallet::ANCHOR_OFFSET`.
-  `get_spendable_sapling_notes`
+  - `WalletRead::get_spendable_notes` has been renamed to
-- `data_api::WalletRead::select_spendable_notes` has been renamed to 
+    `get_spendable_sapling_notes`
-  `select_spendable_sapling_notes`
+  - `WalletRead::select_spendable_notes` has been renamed to
+    `select_spendable_sapling_notes`
+  - `WalletRead::get_all_nullifiers` has been
+    added. This method provides access to all Sapling nullifiers, including
+    for notes that have been previously marked spent.
 - The `zcash_client_backend::data_api::SentTransaction` type has been
-  substantially modified to accommodate handling of transparent
+  substantially modified to accommodate handling of transparent inputs.
-  inputs.
+  Per-output data has been split out into a new struct `SentTransactionOutput`
-- `data_api::WalletWrite::store_received_tx` has been renamed to 
+  and `SentTransaction` can now contain multiple outputs.
-  `store_decrypted_tx` and it now takes an explicit list of 
+- `data_api::WalletWrite::store_received_tx` has been renamed to
-  `(AccountId, Nullifier)` pairs that allows the library to 
+  `store_decrypted_tx` and it now takes an explicit list of
+  `(AccountId, Nullifier)` pairs that allows the library to
   provide quick access to previously decrypted nullifiers.
-- An `Error::MemoForbidden` error has been added to the 
+- `data_api::ReceivedTransaction` has been renamed to `DecryptedTransaction`,
-  `data_api::error::Error` enum to report the condition where a
+  and its `outputs` field has been renamed to `sapling_outputs`.
-  memo was specified to be sent to a transparent recipient.
+- An `Error::MemoForbidden` error has been added to the
-- The hardcoded `data_api::wallet::ANCHOR_OFFSET` constant has been removed.
+  `data_api::error::Error` enum to report the condition where a memo was
+  specified to be sent to a transparent recipient.
+- If no memo is provided when sending to a shielded recipient, the
+  empty memo will be used
 - `zcash_client_backend::keys::spending_key` has been moved to the
   `zcash_client_backend::keys::sapling` module.
-- Two new types, `UnifiedSpendingKey` and `UnifiedFullViewingKey` 
+- `zcash_client_backend::zip321::MemoError` has been renamed and
-  have been added to the `zcash_client_backend::keys` module. These 
+  expanded into a more comprehensive `Zip321Error` type, and functions in the
-  types should be considered unstable as they are likely to be changed
+  `zip321` module have been updated to use this unified error type. The
-  and/or extracted into a different crate in a future release.
+  following error cases have been added:
-- A `zcash_client_backend::wallet::WalletTransparentOutput` type
+  - `Zip321Error::TooManyPayments(usize)`
-  has been added under the `transparent-inputs` feature flag in support
+  - `Zip321Error::DuplicateParameter(parse::Param, usize)`
-  of autoshielding functionality.
+  - `Zip321Error::TransparentMemo(usize)`
-- `zcash_client_backend::zip321::MemoError` has been renamed and 
+  - `Zip321Error::RecipientMissing(usize)`
-  expanded into a more comprehensive `Zip321Error` type, and 
+  - `Zip321Error::ParseError(String)`
-  functions in hthe `zip321` module have been updated to use
+
-  this unified error type.
+### Removed
-- The `zcash_client_backend::wallet::AccountId` type has been moved
+- The hardcoded `data_api::wallet::ANCHOR_OFFSET` constant.
-  to the `zcash_primitives::zip32` module.
+- `zcash_client_backend::wallet::AccountId` (moved to `zcash_primitives::zip32::AccountId`).
+
 
 ## [0.5.0] - 2021-03-26
 ### Added

zcash_client_backend/src/data_api/wallet.rs

@@ -93,6 +93,23 @@ where
 ///
 /// [ZIP 310]: https://zips.z.cash/zip-0310
 ///
+/// Parameters:
+/// * `wallet_db`: A read/write reference to the wallet database
+/// * `params`: Consensus parameters
+/// * `prover`: The TxProver to use in constructing the shielded transaction.
+/// * `account`: The ZIP32 account identifier associated with the extended spending
+///   key that controls the funds to be used in creating this transaction.  This
+///   procedure will return an error if this does not correctly correspond to `extsk`.
+/// * `extsk`: The extended spending key that controls the funds that will be spent
+///   in the resulting transaction.
+/// * `amount`: The amount to send.
+/// * `to`: The address to which `amount` will be paid.
+/// * `memo`: A memo to be included in the output to the recipient.
+/// * `ovk_policy`: The policy to use for constructing outgoing viewing keys that
+///   can allow the sender to view the resulting notes on the blockchain.
+/// * `min_confirmations`: The minimum number of confirmations that a previously
+///   received note must have in the blockchain in order to be considered for being
+///   spent. A value of 10 confirmations is recommended.
 /// # Examples
 ///
 /// ```
@@ -174,7 +191,9 @@ where
         message: None,
         other_params: vec![],
     }])
-    .unwrap();
+    .expect(
+        "It should not be possible for this to violate ZIP 321 request construction invariants.",
+    );
 
     spend(
         wallet_db,
@@ -189,12 +208,34 @@ where
 }
 
 /// Constructs a transaction that sends funds as specified by the `request` argument
-/// and stores it to the wallet's "sent transactions" data store, and returns the
+/// and stores it to the wallet's "sent transactions" data store, and returns a
-/// identifier for the transaction.
+/// unique identifier for the transaction; this identifier is used only for internal
+/// reference purposes and is not the same as the transaction's txid, although after the
+/// activation of ZIP 244 the txid would be a reasonable value for this type.
 ///
 /// This procedure uses the wallet's underlying note selection algorithm to choose
 /// inputs of sufficient value to satisfy the request, if possible.
 ///
+/// Do not call this multiple times in parallel, or you will generate transactions that
+/// double-spend the same notes.
+///
+/// # Transaction privacy
+///
+/// `ovk_policy` specifies the desired policy for which outgoing viewing key should be
+/// able to decrypt the outputs of this transaction. This is primarily relevant to
+/// wallet recovery from backup; in particular, [`OvkPolicy::Discard`] will prevent the
+/// recipient's address, and the contents of `memo`, from ever being recovered from the
+/// block chain. (The total value sent can always be inferred by the sender from the spent
+/// notes and received change.)
+///
+/// Regardless of the specified policy, `create_spend_to_address` saves `to`, `value`, and
+/// `memo` in `db_data`. This can be deleted independently of `ovk_policy`.
+///
+/// For details on what transaction information is visible to the holder of a full or
+/// outgoing viewing key, refer to [ZIP 310].
+///
+/// [ZIP 310]: https://zips.z.cash/zip-0310
+///
 /// Parameters:
 /// * `wallet_db`: A read/write reference to the wallet database
 /// * `params`: Consensus parameters
@@ -210,7 +251,7 @@ where
 ///   can allow the sender to view the resulting notes on the blockchain.
 /// * `min_confirmations`: The minimum number of confirmations that a previously
 ///   received note must have in the blockchain in order to be considered for being
-///   spent.
+///   spent. A value of 10 confirmations is recommended.
 #[allow(clippy::too_many_arguments)]
 pub fn spend<E, N, P, D, R>(
     wallet_db: &mut D,
@@ -370,6 +411,10 @@ where
 /// * `account`: The ZIP32 account identifier associated with the the extended
 ///   full viewing key. This procedure will return an error if this does not correctly
 ///   correspond to `extfvk`.
+/// * `memo`: A memo to be included in the output to the (internal) recipient.
+///   This can be used to take notes about auto-shielding operations internal
+///   to the wallet that the wallet can use to improve how it represents those
+///   those shielding transactions to the user.
 /// * `min_confirmations`: The minimum number of confirmations that a previously
 ///   received UTXO must have in the blockchain in order to be considered for being
 ///   spent.
@@ -401,20 +446,21 @@ where
         .get_target_and_anchor_heights(min_confirmations)
         .and_then(|x| x.ok_or_else(|| Error::ScanRequired.into()))?;
 
-    // derive the t-address for the extpubkey at child index 0
     let account_pubkey = sk.to_account_pubkey();
     let ovk = OutgoingViewingKey(account_pubkey.internal_ovk().as_bytes());
 
-    // derive own shielded address from the provided extended spending key
+    // derive own shielded address from the provided extended full viewing key
     // TODO: this should become the internal change address derived from
     // the wallet's UFVK
     let z_address = extfvk.default_address().1;
 
-    // get UTXOs from DB
+    // derive the t-address for the extpubkey at the minimum valid child index
     let (taddr, child_index) = account_pubkey
         .derive_external_ivk()
         .unwrap()
         .default_address();
+
+    // get UTXOs from DB
     let utxos = wallet_db.get_unspent_transparent_outputs(&taddr, latest_anchor)?;
     let total_amount = utxos
         .iter()

zcash_client_backend/src/keys.rs

@@ -46,6 +46,7 @@ pub mod sapling {
 }
 
 #[derive(Debug)]
+#[doc(hidden)]
 pub enum DerivationError {
     #[cfg(feature = "transparent-inputs")]
     Transparent(hdwallet::error::Error),
@@ -54,6 +55,7 @@ pub enum DerivationError {
 /// A set of viewing keys that are all associated with a single
 /// ZIP-0032 account identifier.
 #[derive(Clone, Debug)]
+#[doc(hidden)]
 pub struct UnifiedSpendingKey {
     account: AccountId,
     #[cfg(feature = "transparent-inputs")]
@@ -61,6 +63,7 @@ pub struct UnifiedSpendingKey {
     sapling: sapling::ExtendedSpendingKey,
 }
 
+#[doc(hidden)]
 impl UnifiedSpendingKey {
     pub fn from_seed<P: consensus::Parameters>(
         params: &P,
@@ -113,13 +116,17 @@ impl UnifiedSpendingKey {
 /// A set of viewing keys that are all associated with a single
 /// ZIP-0032 account identifier.
 #[derive(Clone, Debug)]
+#[doc(hidden)]
 pub struct UnifiedFullViewingKey {
     account: AccountId,
     #[cfg(feature = "transparent-inputs")]
     transparent: Option<legacy::AccountPubKey>,
+    // TODO: This type is invalid for a UFVK; create a `sapling::DiversifiableFullViewingKey`
+    // to replace it.
     sapling: Option<sapling::ExtendedFullViewingKey>,
 }
 
+#[doc(hidden)]
 impl UnifiedFullViewingKey {
     /// Construct a new unified full viewing key, if the required components are present.
     pub fn new(
@@ -145,9 +152,9 @@ impl UnifiedFullViewingKey {
         self.account
     }
 
-    #[cfg(feature = "transparent-inputs")]
     /// Returns the transparent component of the unified key at the
     /// BIP44 path `m/44'/<coin_type>'/<account>'`.
+    #[cfg(feature = "transparent-inputs")]
     pub fn transparent(&self) -> Option<&legacy::AccountPubKey> {
         self.transparent.as_ref()
     }

zcash_client_sqlite/CHANGELOG.md

@@ -7,6 +7,16 @@ and this library adheres to Rust's notion of
 
 ## [Unreleased]
 ### Added
+- Implementations of `zcash_client_backend::data_api::WalletReadTransparent`
+  and `WalletWriteTransparent` have been added. These implementations
+  are available only when the `transparent-inputs` feature flag is
+  enabled.
+- New error variants:
+  - `SqliteClientError::TransparentAddress`, to support handling of errors in
+    transparent address decoding.
+  - `SqliteClientError::RequestedRewindInvalid`, to report when requested
+    rewinds exceed supported bounds.
+
 ### Changed
 - MSRV is now 1.51.0.
 - Bumped dependencies to `ff 0.11`, `group 0.11`, `jubjub 0.8`.
@@ -16,13 +26,56 @@ and this library adheres to Rust's notion of
   - `zcash_client_sqlite::WalletDB` to `WalletDb`
   - `zcash_client_sqlite::error::SqliteClientError::IncorrectHRPExtFVK` to
     `IncorrectHrpExtFvk`.
-- A new error constructor `SqliteClientError::TransparentAddress` has been added 
+- The SQLite implementations of `zcash_client_backend::data_api::WalletRead`
-  to support handling of errors in transparent address decoding.
+  and `WalletWrite` have been updated to reflect the changes to those
-- A new error constructor `SqliteClientError::RequestedRewindInvalid` has been added 
-  to report when requested rewinds exceed supported bounds.
-- The sqlite implemenations of `zcash_client_backend::data_api::WalletRead`
-  and `WalletWrite` have been updated to reflect the changes to those 
   traits.
+- Renamed the following to reflect their Sapling-specific nature:
+  - `zcash_client_sqlite::wallet`:
+    - `get_spendable_notes` to `get_spendable_sapling_notes`.
+    - `select_spendable_notes` to `select_spendable_sapling_notes`.
+- Altered the arguments to `zcash_client_sqlite::wallet::put_sent_note`
+  to take the components of a `DecryptedOutput` value to allow this
+  method to be used in contexts where a transaction has just been
+  constructed, rather than only in the case that a transaction has
+  been decrypted after being retrieved from the network.
+
+### Deprecated
+- A number of public API methods that are used internally to support the
+  `zcash_client_backend::data_api::{WalletRead, WalletWrite}` interfaces have
+  been deprecated, and will be removed from the public API in a future release.
+  Users should depend upon the versions of these methods exposed via the
+  `zcash_client_backend::data_api` traits mentioned above instead.
+  - Deprecated in `zcash_client_sqlite::wallet`:
+    - `get_address`
+    - `get_extended_full_viewing_keys`
+    - `is_valid_account_extfvk`
+    - `get_balance`
+    - `get_balance_at`
+    - `get_sent_memo`
+    - `block_height_extrema`
+    - `get_tx_height`
+    - `get_block_hash`
+    - `get_rewind_height`
+    - `get_commitment_tree`
+    - `get_witnesses`
+    - `get_nullifiers`
+    - `insert_block`
+    - `put_tx_meta`
+    - `put_tx_data`
+    - `mark_sapling_note_spent`
+    - `delete_utxos_above`
+    - `put_receiverd_note`
+    - `insert_witness`
+    - `prune_witnesses`
+    - `update_expired_notes`
+    - `put_sent_note`
+    - `put_sent_utxo`
+    - `insert_sent_note`
+    - `insert_sent_utxo`
+    - `get_address`
+  - Deprecated in `zcash_client_sqlite::wallet::transact`:
+    - `get_spendable_sapling_notes`
+    - `select_spendable_sapling_notes`
 
 ## [0.3.0] - 2021-03-26
 This release contains a major refactor of the APIs to leverage the new Data

zcash_client_sqlite/src/lib.rs

@@ -111,9 +111,7 @@ impl<P: consensus::Parameters> WalletDb<P> {
     pub fn for_path<F: AsRef<Path>>(path: F, params: P) -> Result<Self, rusqlite::Error> {
         Connection::open(path).map(move |conn| WalletDb { conn, params })
     }
-}
 
-impl<P: consensus::Parameters> WalletDb<P> {
     /// Given a wallet database connection, obtain a handle for the write operations
     /// for that database. This operation may eagerly initialize and cache sqlite
     /// prepared statements that are used in write operations.
@@ -601,7 +599,7 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> {
             if !d_tx.tx.transparent_bundle().iter().any(|b| b.vout.is_empty()) {
                 // If the transaction contains shielded spends from our wallet, we will store z->t
                 // transactions we observe in the same way they would be stored by
-                // create_spend_to_address.
+                // create_spend_to_address. 
                 if let Some((account_id, _)) = nullifiers.iter().find(
                     |(_, nf)|
                         d_tx.tx.sapling_bundle().iter().flat_map(|b| b.shielded_spends.iter())
@@ -787,32 +785,33 @@ mod tests {
         db_data: &WalletDb<Network>,
     ) -> (ExtendedFullViewingKey, Option<TransparentAddress>) {
         let seed = [0u8; 32];
-
+        let account = AccountId(0);
-        let extsk = sapling::spending_key(&seed, network().coin_type(), AccountId(0));
+        let extsk = sapling::spending_key(&seed, network().coin_type(), account);
         let extfvk = ExtendedFullViewingKey::from(&extsk);
 
         #[cfg(feature = "transparent-inputs")]
-        {
+        let (tkey, taddr) = {
-            let tkey = Some(
+            let tkey = legacy::keys::AccountPrivKey::from_seed(&network(), &seed, account)
-                legacy::keys::AccountPrivKey::from_seed(&network(), &seed, AccountId(0))
+                .unwrap()
-                    .unwrap()
+                .to_account_pubkey();
-                    .to_account_pubkey(),
+            let taddr = tkey.derive_external_ivk().unwrap().default_address().0;
-            );
+            (Some(tkey), Some(taddr))
-            let ufvk = UnifiedFullViewingKey::new(AccountId(0), tkey.clone(), Some(extfvk.clone()))
+        };
-                .unwrap();
-            init_accounts_table(db_data, &[ufvk]).unwrap();
-            (
-                extfvk,
-                tkey.map(|k| k.derive_external_ivk().unwrap().default_address().0),
-            )
-        }
 
         #[cfg(not(feature = "transparent-inputs"))]
-        {
+        let taddr = None;
-            let ufvk = UnifiedFullViewingKey::new(AccountId(0), Some(extfvk.clone())).unwrap();
+
-            init_accounts_table(db_data, &[ufvk]).unwrap();
+        let ufvk = UnifiedFullViewingKey::new(
-            (extfvk, None)
+            account,
-        }
+            #[cfg(feature = "transparent-inputs")]
+            tkey,
+            Some(extfvk.clone()),
+        )
+        .unwrap();
+
+        init_accounts_table(db_data, &[ufvk]).unwrap();
+
+        (extfvk, taddr)
     }
 
     /// Create a fake CompactBlock at the given height, containing a single output paying

zcash_client_sqlite/src/wallet.rs

@@ -575,7 +575,7 @@ pub(crate) fn rewind_to_height<P: consensus::Parameters>(
                     FROM received_notes rn
                     LEFT OUTER JOIN transactions tx
                     ON tx.id_tx = rn.tx
-                    WHERE tx.block > ?
+                    WHERE tx.block IS NOT NULL AND tx.block > ?
                 );",
             &[u32::from(block_height)],
         )?;
@@ -588,7 +588,7 @@ pub(crate) fn rewind_to_height<P: consensus::Parameters>(
                     FROM sent_notes sn
                     LEFT OUTER JOIN transactions tx
                     ON tx.id_tx = sn.tx
-                    WHERE tx.block > ?
+                    WHERE tx.block IS NOT NULL AND tx.block > ?
                 );",
             &[u32::from(block_height)],
         )?;
@@ -601,7 +601,7 @@ pub(crate) fn rewind_to_height<P: consensus::Parameters>(
 
         // Un-mine transactions.
         wdb.conn.execute(
-            "UPDATE transactions SET block = NULL, tx_index = NULL WHERE block > ?",
+            "UPDATE transactions SET block = NULL, tx_index = NULL WHERE block IS NOT NULL AND block > ?",
             &[u32::from(block_height)],
         )?;
 
@@ -950,8 +950,7 @@ pub(crate) fn put_received_transparent_utxo<'a, P: consensus::Parameters>(
 }
 
 /// Removes all records of UTXOs that were recorded as having been received
-/// at block heights greater than the given height. Used in the case of chain
+/// at block heights greater than the given height.
-/// rollback.
 #[cfg(feature = "transparent-inputs")]
 #[deprecated(
     note = "This method will be removed in a future update. Use zcash_client_backend::data_api::WalletWrite::rewind_to_height instead."

zcash_primitives/CHANGELOG.md

@@ -82,25 +82,21 @@ and this library adheres to Rust's notion of
   corresponding to the associated full viewing key as specified in
   [ZIP 32](https://zips.z.cash/zip-0032#deriving-a-sapling-internal-spending-key).
 - `zcash_primitives::zip32::sapling_derive_internal_fvk` provides the
-  internal implementation of `ExtendedFullViewingKey.derive_internal`
+  internal implementation of `ExtendedFullViewingKey.derive_internal` but does
-  but does not require a complete extended full viewing key, just
+  not require a complete extended full viewing key, just the full viewing key
-  the full viewing key and the diversifier key. In the future, this
+  and the diversifier key. In the future, this function will likely be
-  function will likely be refactored to become a member function of
+  refactored to become a member function of a new `DiversifiableFullViewingKey`
-  a new `DiversifiableFullViewingKey` type, which represents the ability
+  type, which represents the ability to derive IVKs, OVKs, and addresses, but
-  to derive IVKs, OVKs, and addresses, but not child viewing keys.
+  not child viewing keys.
-- The `zcash_primitives::transaction::Builder::add_sapling_output` method 
-  now takes its `MemoBytes` argument as a required field rather than an
-  optional one. If the empty memo is desired, use
-  `MemoBytes::from(Memo::Empty)` explicitly.
 - A new module `zcash_primitives::legacy::keys` has been added under the
-  `transparent-inputs` feature flag to support types related to supporting 
+  `transparent-inputs` feature flag to support types related to supporting
   transparent components of unified addresses and derivation of OVKs for
   shielding funds from the transparent pool.
-- A `zcash_primitives::transaction::components::amount::Amount::sum` 
+- A `zcash_primitives::transaction::components::amount::Amount::sum`
-  convenience method has been added to facilitate bounds-checked
+  convenience method has been added to facilitate bounds-checked summation of
-  summation of account values.
+  account values.
-- The `zcash_client_backend::wallet::AccountId` type has been moved
+- The `zcash_primitives::zip32::AccountId`, a type-safe wrapper for ZIP 32
-  to the `zcash_primitives::zip32` module.
+  account indices.
 
 ### Changed
 - MSRV is now 1.51.0.
@@ -109,6 +105,9 @@ and this library adheres to Rust's notion of
   `zcash_primitives::sapling`:
   - `zcash_primitives::group_hash`
   - `zcash_primitives::keys`
+    - `zcash_primitives::sapling::keys::{prf_expand, prf_expand_vec, OutgoingViewingKey}`
+      have all been moved into to the this module to reflect the fact that they
+      are used outside of the Sapling protocol.
   - `zcash_primitives::pedersen_hash`
   - `zcash_primitives::primitives::*` (moved into `zcash_primitives::sapling`)
   - `zcash_primitives::prover`
@@ -150,9 +149,10 @@ and this library adheres to Rust's notion of
   `jubjub::ExtendedPoint` to `zcash_note_encryption::EphemeralKeyBytes`.
 - The `epk: jubjub::ExtendedPoint` field of `CompactOutputDescription ` has been
   replaced by `ephemeral_key: zcash_note_encryption::EphemeralKeyBytes`.
-- `zcash_primitives::sapling::keys::{prf_expand, prf_expand_vec, OutgoingViewingKey}` have
+- The `zcash_primitives::transaction::Builder::add_sapling_output` method
-  all been moved to the `zcash_primitives::keys` module to reflect the fact
+  now takes its `MemoBytes` argument as a required field rather than an
-  that they are used outside of the Sapling protocol.
+  optional one. If the empty memo is desired, use
+  `MemoBytes::from(Memo::Empty)` explicitly.
 
 ## [0.5.0] - 2021-03-26
 ### Added

zcash_primitives/src/legacy/keys.rs

@@ -136,9 +136,9 @@ impl AccountPubKey {
 /// Derives the P2PKH transparent address corresponding to the given pubkey.
 #[deprecated(note = "This function will be removed from the public API in an upcoming refactor.")]
 pub fn pubkey_to_address(pubkey: &secp256k1::key::PublicKey) -> TransparentAddress {
-    let mut hash160 = ripemd::Ripemd160::new();
+    TransparentAddress::PublicKey(
-    hash160.update(Sha256::digest(&pubkey.serialize()));
+        *ripemd::Ripemd160::digest(Sha256::digest(&pubkey.serialize())).as_ref(),
-    TransparentAddress::PublicKey(*hash160.finalize().as_ref())
+    )
 }
 
 pub(crate) mod private {