Merge pull request #1978 from zcash/merge/zcash_client_sqlite-0.18.3

Post-release merge of `zcash_client_sqlite-0.18.3`

Author: nuttycom

Cargo.lock

@@ -302,8 +302,8 @@ user-login = "str4d"
 user-name = "Jack Grigg"
 
 [[publisher.zcash_client_sqlite]]
-version = "0.18.2"
-when = "2025-09-29"
+version = "0.18.3"
+when = "2025-10-01"
 user-id = 169181
 user-login = "nuttycom"
 user-name = "Kris Nuttycombe"

supply-chain/imports.lock

@@ -10,6 +10,16 @@ workspace.
 
 ## [Unreleased]
 
+## [0.18.3] - 2025-09-30
+
+### Changed
+- The `zcash_client_sqlite` implementation of `WalletWrite::update_chain_tip`
+  now ensures that a transaction status request is queued for any transactions
+  for which we do not have mined-height information and which are known to be
+  unexpired. 
+- Transaction status requests are no longer deleted until the transaction in
+  question is positively known to be expired.
+
 ## [0.18.2] - 2025-09-28
 
 ### Changed

zcash_client_sqlite/CHANGELOG.md

@@ -1,7 +1,7 @@
 [package]
 name = "zcash_client_sqlite"
 description = "An SQLite-based Zcash light client"
-version = "0.18.2"
+version = "0.18.3"
 authors = [
     "Jack Grigg <jack@z.cash>",
     "Kris Nuttycombe <kris@electriccoin.co>"

zcash_client_sqlite/Cargo.toml

@@ -896,7 +896,7 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters, CL, R> WalletRea
     }
 
     fn get_tx_height(&self, txid: TxId) -> Result<Option<BlockHeight>, Self::Error> {
-        wallet::get_tx_height(self.conn.borrow(), txid).map_err(SqliteClientError::from)
+        wallet::get_tx_height(self.conn.borrow(), txid)
     }
 
     fn get_unified_full_viewing_keys(

zcash_client_sqlite/src/lib.rs

@@ -130,7 +130,7 @@ use crate::{
         encoding::LEGACY_ADDRESS_INDEX_NULL,
     },
     AccountRef, AccountUuid, AddressRef, SqlTransaction, TransferType, TxRef,
-    WalletCommitmentTrees, WalletDb, PRUNING_DEPTH,
+    WalletCommitmentTrees, WalletDb, PRUNING_DEPTH, VERIFY_LOOKAHEAD,
 };
 
 #[cfg(feature = "transparent-inputs")]
@@ -3010,12 +3010,21 @@ pub(crate) fn set_transaction_status(
     match status {
         TransactionStatus::TxidNotRecognized | TransactionStatus::NotInMainChain => {
             // Remove the txid from the retrieval queue unless an unexpired transaction having that
-            // txid exists in the transactions table.
+            // txid exists in the transactions table, with a `VERIFY_LOOKAHEAD` buffer.
             if let Some(chain_tip) = chain_tip_height(conn)? {
                 conn.execute(
                     "DELETE FROM tx_retrieval_queue
                      WHERE txid = :txid
-                     AND request_expiry <= :chain_tip",
+                     AND request_expiry <= :chain_tip
+                     AND txid NOT IN (
+                        SELECT tx.txid
+                        FROM transactions tx
+                        WHERE tx.mined_height IS NULL
+                        AND (
+                            tx.expiry_height == 0 -- tx will never expire
+                            OR tx.expiry_height > :chain_tip -- tx is unexpired
+                        )
+                     )",
                     named_params![
                         ":txid": txid.as_ref(),
                         ":chain_tip": u32::from(chain_tip)
@@ -3054,6 +3063,67 @@ pub(crate) fn set_transaction_status(
     Ok(())
 }
 
+pub(crate) fn refresh_status_requests(
+    conn: &rusqlite::Transaction,
+) -> Result<(), SqliteClientError> {
+    if let Some(chain_tip) = chain_tip_height(conn)? {
+        let mut unmined_query = conn.prepare(
+            "SELECT
+                t.txid,
+                t.expiry_height,
+                r.query_type IS NOT NULL AS status_request_exists
+             FROM transactions t
+             LEFT OUTER JOIN tx_retrieval_queue r ON r.txid = t.txid
+             WHERE t.mined_height IS NULL
+             AND (
+                t.expiry_height IS NULL -- expiry is unknown
+                OR t.expiry_height = 0 -- tx will note expire
+                OR t.expiry_height > :chain_tip -- tx is unexpired
+             )
+             AND (
+                r.query_type IS NULL
+                OR r.query_type = :status_type
+             )",
+        )?;
+
+        let mut rows = unmined_query.query(named_params! {
+            ":chain_tip": u32::from(chain_tip),
+            ":status_type": TxQueryType::Status.code(),
+        })?;
+        while let Some(row) = rows.next()? {
+            let txid = row.get::<_, Vec<u8>>("txid")?;
+            let tx_expiry = row.get::<_, Option<u32>>("expiry_height")?;
+
+            if row.get::<_, bool>("status_request_exists")? {
+                conn.execute(
+                    "UPDATE tx_retrieval_queue
+                     SET request_expiry = :new_expiry
+                     WHERE txid = :txid
+                     AND query_type = :status_type",
+                    named_params! {
+                        ":new_expiry": tx_retrieval_expiry(chain_tip, tx_expiry),
+                        ":txid": &txid[..],
+                        ":status_type": TxQueryType::Status.code(),
+                    },
+                )?;
+            } else {
+                conn.execute(
+                    "INSERT INTO tx_retrieval_queue (txid, query_type, request_expiry)
+                     VALUES (:txid, :status_type, :new_expiry)
+                     ON CONFLICT (txid) DO NOTHING",
+                    named_params! {
+                        ":new_expiry": tx_retrieval_expiry(chain_tip, tx_expiry),
+                        ":txid": &txid[..],
+                        ":status_type": TxQueryType::Status.code(),
+                    },
+                )?;
+            }
+        }
+    }
+
+    Ok(())
+}
+
 /// Truncates the database to at most the given height.
 ///
 /// If the requested height is greater than or equal to the height of the last scanned
@@ -4118,7 +4188,12 @@ pub(crate) fn queue_tx_retrieval(
     txids: impl Iterator<Item = TxId>,
     dependent_tx_ref: Option<TxRef>,
 ) -> Result<(), SqliteClientError> {
-    let chain_tip = chain_tip_height(conn)?;
+    let chain_tip = match chain_tip_height(conn)? {
+        Some(h) => h,
+        None => {
+            return Ok(());
+        }
+    };
 
     let mut q_tx_expiry = conn.prepare_cached(
         "SELECT expiry_height
@@ -4150,7 +4225,7 @@ pub(crate) fn queue_tx_retrieval(
     )?;
 
     for txid in txids {
-        let request_expiry = q_tx_expiry
+        let tx_expiry = q_tx_expiry
             .query_row(
                 named_params! {
                     ":txid": txid.as_ref(),
@@ -4165,16 +4240,34 @@ pub(crate) fn queue_tx_retrieval(
             ":status_type": TxQueryType::Status.code(),
             ":enhancement_type": TxQueryType::Enhancement.code(),
             ":dependent_transaction_id": dependent_tx_ref.map(|r| r.0),
-            ":request_expiry": request_expiry.map_or_else(
-                || chain_tip.map(|h| u32::from(h) + DEFAULT_TX_EXPIRY_DELTA),
-                Some
-            )
+            ":request_expiry": tx_retrieval_expiry(chain_tip, tx_expiry)
         })?;
     }
 
     Ok(())
 }
 
+// Computes a block height at which transaction status requests should be considered
+// expired. Note that even expired requests will be serviced; expiry is only used
+// to determine when a request can be deleted and not retried.
+//
+// TODO: the underlying `request_expiry` mechanism that this method is being used to configure is
+// not sufficiently precise and should probably be scrapped in favor of something better; however,
+// this will do for now.
+fn tx_retrieval_expiry(chain_tip_height: BlockHeight, tx_expiry: Option<u32>) -> u32 {
+    match tx_expiry {
+        // If the transaction never expires or we don't know its expiry, keep checking for the next
+        // expiry delta from the chain tip.
+        Some(0) | None => u32::from(chain_tip_height) + DEFAULT_TX_EXPIRY_DELTA,
+        // If the transaction expires in the future, the request can expire with the transaction.
+        Some(h) if h > u32::from(chain_tip_height) => h + VERIFY_LOOKAHEAD,
+        // The transaction has already expired; we can keep checking for a few blocks, but if we
+        // don't get a positive response for the transaction being mined soon, we can assume it
+        // will never be mined.
+        Some(_) => u32::from(chain_tip_height) + VERIFY_LOOKAHEAD,
+    }
+}
+
 /// Returns the vector of [`TransactionDataRequest`]s that represents the information needed by the
 /// wallet backend in order to be able to present a complete view of wallet history and memo data.
 pub(crate) fn transaction_data_requests(

zcash_client_sqlite/src/wallet.rs

@@ -15,6 +15,7 @@ use zcash_protocol::{
     ShieldedProtocol,
 };
 
+use crate::wallet::refresh_status_requests;
 use crate::TableConstants;
 use crate::{
     error::SqliteClientError,
@@ -548,13 +549,17 @@ pub(crate) fn update_chain_tip<P: consensus::Parameters>(
         None => tip_entry.block_range().clone(),
     };
 
+    // persist the updated scan queue entries
     replace_queue_entries::<SqliteClientError>(
         conn,
         &query_range,
         tip_shard_entry.into_iter().chain(Some(tip_entry)),
         false,
     )?;
 
+    // ensure that transaction status requests exist for any unmined, unexpired transactions
+    refresh_status_requests(conn)?;
+
     Ok(())
 }
 
@@ -1702,7 +1707,7 @@ pub(crate) mod tests {
             // Add blocks up to the chain tip.
             let mut chain_tip_height = spanning_block_height;
             for _ in 0..110 {
-                let (h, res, _) = st.generate_next_block_multi(&vec![fake_output(false)]);
+                let (h, res, _) = st.generate_next_block_multi(&[fake_output(false)]);
                 for c in res.orchard() {
                     final_orchard_tree.append(*c);
                 }