Address comments from code review.

zcash_client_backend/src/data_api/wallet.rs

@@ -376,13 +376,13 @@ where
     let sent_outputs = request.payments().iter().enumerate().map(|(i, payment)| {
         let (output_index, recipient) = match &payment.recipient_address {
             // Sapling outputs are shuffled, so we need to look up where the output ended up.
-            // TODO: When we add Orchard support, we will need to trial-decrypt to find them,
-            // and return the appropriate pool type.
             RecipientAddress::Shielded(addr) => {
                 let idx = tx_metadata.output_index(i).expect("An output should exist in the transaction for each shielded payment.");
                 (idx, Recipient::Sapling(addr.clone()))
             }
             RecipientAddress::Unified(addr) => {
+                // TODO: When we add Orchard support, we will need to trial-decrypt to find them,
+                // and return the appropriate pool type.
                 let idx = tx_metadata.output_index(i).expect("An output should exist in the transaction for each shielded payment.");
                 (idx, Recipient::Unified(addr.clone(), PoolType::Sapling))
             }

zcash_client_sqlite/CHANGELOG.md

@@ -76,11 +76,6 @@ and this library adheres to Rust's notion of
   - `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.
 - `zcash_client_sqlite::wallet::init_wallet_db` has been modified to
   take the wallet seed as an argument so that it can correctly perform
   migrations that require re-deriving key material. In particular for
@@ -114,7 +109,6 @@ and this library adheres to Rust's notion of
   `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`
@@ -134,9 +128,6 @@ and this library adheres to Rust's notion of
     - `insert_witness`
     - `prune_witnesses`
     - `update_expired_notes`
-    - `put_sent_utxo`
-    - `insert_sent_note`
-    - `insert_sent_utxo`
     - `get_address`
   - Deprecated in `zcash_client_sqlite::wallet::transact`:
     - `get_spendable_sapling_notes`

zcash_client_sqlite/src/lib.rs

@@ -545,8 +545,9 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> {
                             tx_ref,
                             output.index,
                             &recipient,
-                            Amount::from_u64(output.note.value).unwrap(),
-                            Some(&output.memo)
+                            Amount::from_u64(output.note.value).map_err(|_|
+                                SqliteClientError::CorruptedData("Note value is not a valid Zcash amount.".to_string()))?,
+                            Some(&output.memo),
                         )?;
                     }
                     TransferType::Incoming => {

zcash_client_sqlite/src/prepared.rs

@@ -358,6 +358,8 @@ impl<'a, P: consensus::Parameters> DataConnStmtCache<'a, P> {
     ///
     /// `output_index` is the index within the transaction that contains the recipient output:
     ///
+    /// - If `to` is a Unified address, this is an index into the outputs of the transaction
+    ///   within the bundle associated with the recipient's output pool.
     /// - If `to` is a Sapling address, this is an index into the Sapling outputs of the
     ///   transaction.
     /// - If `to` is a transparent address, this is an index into the transparent outputs of