From aeac544aedb0b147b4cfa09f4abeee2c06ff1abe Mon Sep 17 00:00:00 2001
From: Kris Nuttycombe <kris@nutty.land>
Date: Mon, 22 Apr 2024 10:55:25 -0600
Subject: [PATCH] Address comments from code review.

---
 components/zcash_address/src/lib.rs | 21 +++++++++------------
 components/zip321/src/lib.rs        | 12 ++++++++++++
 zcash_client_sqlite/src/wallet.rs   |  1 -
 zcash_keys/src/address.rs           |  7 +++++--
 4 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/components/zcash_address/src/lib.rs b/components/zcash_address/src/lib.rs
index 24445afb6..82c9d6b84 100644
--- a/components/zcash_address/src/lib.rs
+++ b/components/zcash_address/src/lib.rs
@@ -271,25 +271,22 @@ impl ZcashAddress {
 
     /// Returns whether this address has the ability to receive transfers of the given pool type.
     pub fn can_receive_as(&self, pool_type: PoolType) -> bool {
+        use AddressKind::*;
         match &self.kind {
-            AddressKind::Sprout(_) => false,
-            AddressKind::Sapling(_) => pool_type == PoolType::Shielded(ShieldedProtocol::Sapling),
-            AddressKind::Unified(addr) => addr.has_receiver_of_type(pool_type),
-            AddressKind::P2pkh(_) => pool_type == PoolType::Transparent,
-            AddressKind::P2sh(_) => pool_type == PoolType::Transparent,
-            AddressKind::Tex(_) => pool_type == PoolType::Transparent,
+            Sprout(_) => false,
+            Sapling(_) => pool_type == PoolType::Shielded(ShieldedProtocol::Sapling),
+            Unified(addr) => addr.has_receiver_of_type(pool_type),
+            P2pkh(_) | P2sh(_) | Tex(_) => pool_type == PoolType::Transparent,
         }
     }
 
     /// Returns whether this address can receive a memo.
     pub fn can_receive_memo(&self) -> bool {
+        use AddressKind::*;
         match &self.kind {
-            AddressKind::Sprout(_) => true,
-            AddressKind::Sapling(_) => true,
-            AddressKind::Unified(addr) => addr.can_receive_memo(),
-            AddressKind::P2pkh(_) => false,
-            AddressKind::P2sh(_) => false,
-            AddressKind::Tex(_) => false,
+            Sprout(_) | Sapling(_) => true,
+            Unified(addr) => addr.can_receive_memo(),
+            P2pkh(_) | P2sh(_) | Tex(_) => false,
         }
     }
 
diff --git a/components/zip321/src/lib.rs b/components/zip321/src/lib.rs
index 804956e63..f9bfc03d4 100644
--- a/components/zip321/src/lib.rs
+++ b/components/zip321/src/lib.rs
@@ -116,11 +116,23 @@ pub fn memo_from_base64(s: &str) -> Result<MemoBytes, Zip321Error> {
 /// A single payment being requested.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Payment {
+    /// The address to which the payment should be sent.
     recipient_address: ZcashAddress,
+    /// The amount of the payment that is being requested.
     amount: Zatoshis,
+    /// A memo that, if included, must be provided with the payment.
+    /// If a memo is present and [`recipient_address`] is not a shielded
+    /// address, the wallet should report an error.
+    ///
+    /// [`recipient_address`]: #structfield.recipient_address
     memo: Option<MemoBytes>,
+    /// A human-readable label for this payment within the larger structure
+    /// of the transaction request.
     label: Option<String>,
+    /// A human-readable message to be displayed to the user describing the
+    /// purpose of this payment.
     message: Option<String>,
+    /// A list of other arbitrary key/value pairs associated with this payment.
     other_params: Vec<(String, String)>,
 }
 
diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs
index ef1f399e7..132610732 100644
--- a/zcash_client_sqlite/src/wallet.rs
+++ b/zcash_client_sqlite/src/wallet.rs
@@ -2395,7 +2395,6 @@ pub(crate) fn select_receiving_address<P: consensus::Parameters>(
             .transpose()
             .map_err(SqliteClientError::from),
         receiver => {
-            // at present,
             let mut stmt =
                 conn.prepare_cached("SELECT address FROM addresses WHERE account_id = :account")?;
 
diff --git a/zcash_keys/src/address.rs b/zcash_keys/src/address.rs
index 0ea820fe4..7dbaa1023 100644
--- a/zcash_keys/src/address.rs
+++ b/zcash_keys/src/address.rs
@@ -233,8 +233,11 @@ impl UnifiedAddress {
     }
 }
 
-/// An enumeration of protocol-level receiver types. While these correspond to unified address
-/// receiver
+/// An enumeration of protocol-level receiver types.
+///
+/// While these correspond to unified address receiver types, this is a distinct type because it is
+/// used to represent the protocol-level recipient of a transfer, instead of a part of an encoded
+/// address.
 pub enum Receiver {
     #[cfg(feature = "orchard")]
     Orchard(orchard::Address),