Apply suggestions from code review

Co-authored-by: Jack Grigg <[email protected]>

Author: nuttycom

zcash_client_backend/src/data_api.rs

@@ -931,14 +931,15 @@ pub trait WalletRead {
     ///
     /// This is equivalent to (but may be implemented more efficiently than):
     /// ```compile_fail
-    /// if let Some(result) = self.get_transparent_receivers(account)?.get(address) {
-    ///     return Ok(result.clone());
-    /// }
-    /// Ok(self
+    /// Ok(if let Some(result) = self.get_transparent_receivers(account)?.get(address) {
+    ///     Ok(result.clone())
+    /// } else {
+    ///    self
     ///     .get_known_ephemeral_addresses(account, None)?
     ///     .into_iter()
     ///     .find(|(known_addr, _)| known_addr == address)
-    ///     .map(|(_, metadata)| metadata))
+    ///     .map(|(_, metadata)| metadata)
+    /// })
     /// ```
     ///
     /// Returns `Ok(None)` if the address is not recognized, or we do not have metadata for it.
@@ -950,14 +951,16 @@ pub trait WalletRead {
         address: &TransparentAddress,
     ) -> Result<Option<TransparentAddressMetadata>, Self::Error> {
         // This should be overridden.
-        if let Some(result) = self.get_transparent_receivers(account)?.get(address) {
-            return Ok(result.clone());
-        }
-        Ok(self
-            .get_known_ephemeral_addresses(account, None)?
-            .into_iter()
-            .find(|(known_addr, _)| known_addr == address)
-            .map(|(_, metadata)| metadata))
+        Ok(
+            if let Some(result) = self.get_transparent_receivers(account)?.get(address) {
+                result.clone()
+            } else {
+                self.get_known_ephemeral_addresses(account, None)?
+                    .into_iter()
+                    .find(|(known_addr, _)| known_addr == address)
+                    .map(|(_, metadata)| metadata)
+            },
+        )
     }
 
     /// Returns a vector of ephemeral transparent addresses associated with the given
@@ -1001,7 +1004,7 @@ pub trait WalletRead {
         Ok(vec![])
     }
 
-    /// If a given transparent address has been reserved, i.e. would be included in
+    /// If a given ephemeral address might have been reserved, i.e. would be included in
     /// the map returned by `get_known_ephemeral_addresses(account_id, false)` for any
     /// of the wallet's accounts, then return `Ok(Some(account_id))`. Otherwise return
     /// `Ok(None)`.

zcash_client_backend/src/data_api/wallet/input_selection.rs

@@ -384,6 +384,9 @@ where
         let mut tr1_payments = vec![];
         #[cfg(feature = "transparent-inputs")]
         let mut tr1_payment_pools = BTreeMap::new();
+        // this balance value is just used for overflow checking; the actual value of ephemeral
+        // outputs will be computed from the constructed `tr1_transparent_outputs` value
+        // constructed below.
         #[cfg(feature = "transparent-inputs")]
         let mut total_ephemeral = NonNegativeAmount::ZERO;
 

zcash_client_backend/src/fees.rs

@@ -330,9 +330,11 @@ impl Default for DustOutputPolicy {
     }
 }
 
-/// `EphemeralBalance` describes the ephemeral input or output value for
-/// a transaction. It is use in the computation of fees are relevant to transactions using
-/// ephemeral transparent outputs.
+/// `EphemeralBalance` describes the ephemeral input or output value for a transaction. It is used
+/// in fee computation for series of transactions that use an ephemeral transparent output in an
+/// intermediate step, such as when sending from a shielded pool to a [ZIP 320] "TEX" address.
+///
+/// [ZIP 320]: https://zips.z.cash/zip-0320
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum EphemeralBalance {
     Input(NonNegativeAmount),
@@ -388,16 +390,12 @@ pub trait ChangeStrategy {
     /// inputs from most to least preferred to spend within each pool, so that the most
     /// preferred ones are less likely to be indicated to remove.
     ///
-    #[cfg_attr(
-        feature = "transparent-inputs",
-        doc = "`ephemeral_parameters` can be used to specify variations on how balance
-               and fees are computed that are relevant to transactions using ephemeral
-               transparent outputs; see [`EphemeralParameters::new`]."
-    )]
-    #[cfg_attr(
-        not(feature = "transparent-inputs"),
-        doc = "`ephemeral_parameters` should be set to `&EphemeralParameters::NONE`."
-    )]
+    /// - `ephemeral_balance`: if the transaction is to be constructed with either an
+    ///   ephemeral transparent input or an ephemeral transparent output this argument
+    ///   may be used to provide the value of that input or output. The value of this
+    ///   output should be `None` in the case that there are no such items.
+    ///
+    /// [ZIP 320]: https://zips.z.cash/zip-0320
     #[allow(clippy::too_many_arguments)]
     fn compute_balance<P: consensus::Parameters, NoteRefT: Clone>(
         &self,

zcash_client_sqlite/src/wallet/transparent/ephemeral.rs

@@ -187,7 +187,7 @@ pub(crate) fn find_index_for_ephemeral_address_str(
     Ok(conn
         .query_row(
             "SELECT address_index FROM ephemeral_addresses
-        WHERE account_id = :account_id AND address = :address",
+            WHERE account_id = :account_id AND address = :address",
             named_params![":account_id": account_id.0, ":address": &address_str],
             |row| row.get::<_, u32>(0),
         )

zcash_primitives/src/legacy/keys.rs

@@ -21,10 +21,11 @@ use super::TransparentAddress;
 pub struct TransparentKeyScope(u32);
 
 impl TransparentKeyScope {
-    /// Returns an arbitrary custom `TransparentKeyScope`. This should be used
-    /// with care: funds associated with keys derived under a custom scope may
-    /// not be recoverable if the wallet seed is restored in another wallet.
-    /// It is usually preferable to use standardized key scopes.
+    /// Returns an arbitrary custom `TransparentKeyScope`.
+    ///
+    /// This should be used with care: funds associated with keys derived under a custom
+    /// scope may not be recoverable if the wallet seed is restored in another wallet. It
+    /// is usually preferable to use standardized key scopes.
     pub const fn custom(i: u32) -> Option<Self> {
         if i < (1 << 31) {
             Some(TransparentKeyScope(i))