Apply suggestions from code review

Co-authored-by: Daira-Emma Hopwood <daira@jacaranda.org>

zcash_client_backend/proto/service.proto

@@ -36,8 +36,7 @@ message TxFilter {
 // by GetMempoolStream(), the latest block height.
 //
 // FIXME: the documentation here about mempool status contradicts the documentation
-// for the `height` field (which I hope is correct, and this doesn't give the latest
+// for the `height` field. See https://github.com/zcash/librustzcash/issues/1484
-// block height for mempool transactions.)
 message RawTransaction {
     bytes data = 1;     // exact data returned by Zcash 'getrawtransaction'
     uint64 height = 2;  // height that the transaction was mined (or -1)

zcash_client_backend/src/data_api.rs

@@ -549,8 +549,8 @@ pub enum TransactionDataRequest {
     ///
     /// The caller evaluating this request on behalf of the wallet backend should respond to this
     /// request by determining the status of the specified transaction with respect to the main
-    /// chain; if using `lightwalletd` for access to chain data, this may be performed interpreting
+    /// chain; if using `lightwalletd` for access to chain data, this may be obtained by
-    /// the results of the [`GetTransaction`] RPC method. It should then call
+    /// interpreting the results of the [`GetTransaction`] RPC method. It should then call
     /// [`WalletWrite::set_transaction_status`] to provide the resulting transaction status
     /// information to the wallet backend.
     ///

zcash_client_backend/src/proto/service.rs

@@ -40,8 +40,7 @@ pub struct TxFilter {
 /// by GetMempoolStream(), the latest block height.
 ///
 /// FIXME: the documentation here about mempool status contradicts the documentation
-/// for the `height` field (which I hope is correct, and this doesn't give the latest
+/// for the `height` field. See <https://github.com/zcash/librustzcash/issues/1484>
-/// block height for mempool transactions.)
 #[allow(clippy::derive_partial_eq_without_eq)]
 #[derive(Clone, PartialEq, ::prost::Message)]
 pub struct RawTransaction {

zcash_client_sqlite/src/lib.rs

@@ -593,9 +593,8 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
     }
 
     fn transaction_data_requests(&self) -> Result<Vec<TransactionDataRequest>, Self::Error> {
-        let iter = std::iter::empty();
+        let iter = wallet::transaction_data_requests(self.conn.borrow())?.into_iter();
 
-        let iter = iter.chain(wallet::transaction_data_requests(self.conn.borrow())?.into_iter());
         #[cfg(feature = "transparent-inputs")]
         let iter = iter.chain(
             wallet::transparent::transaction_data_requests(self.conn.borrow(), &self.params)?
@@ -1211,6 +1210,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
 
             #[cfg(feature = "transparent-inputs")]
             let detectable_via_scanning = {
+                #[allow(unused_mut)]
                 let mut detectable_via_scanning = d_tx.tx().sapling_bundle().is_some();
                 #[cfg(feature = "orchard")] {
                     detectable_via_scanning |= d_tx.tx().orchard_bundle().is_some();
@@ -1450,7 +1450,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
                             // in transaction pairs sending to a ZIP 320 address) it becomes
                             // possible that the spend of these outputs is not then later detected
                             // if the transaction that spends them is purely transparent. This is
-                            // particularly a problem in wallet recovery.
+                            // especially a problem in wallet recovery.
                             wallet::transparent::queue_transparent_spend_detection(
                                 wdb.conn.0,
                                 &wdb.params,

zcash_client_sqlite/src/wallet.rs

@@ -1975,7 +1975,7 @@ pub(crate) fn store_transaction_to_be_sent<P: consensus::Parameters>(
     }
 
     // Add the transaction to the set to be queried for transaction status. This is only necessary
-    // at present for fully-transparent transactions, because any transaction with a shielded
+    // at present for fully transparent transactions, because any transaction with a shielded
     // component will be detected via ordinary chain scanning and/or nullifier checking.
     if !detectable_via_scanning {
         queue_tx_retrieval(

zcash_client_sqlite/src/wallet/transparent.rs

@@ -500,7 +500,7 @@ pub(crate) fn transaction_data_requests<P: consensus::Parameters>(
     // because under ordinary circumstances, it is populated via a call from
     // `decrypt_and_store_transaction` on ordinary mined transaction data retrieved from the chain.
     let mut address_request_stmt = conn.prepare_cached(
-        "SELECT ssq.address, IFNULL(t.target_height, t.mined_height + 1)
+        "SELECT ssq.address, IFNULL(t.target_height, t.mined_height)
          FROM transparent_spend_search_queue ssq
          JOIN transactions t ON t.id_tx = ssq.transaction_id
          WHERE t.target_height IS NOT NULL