Apply suggestions from code review

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

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_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))