More reliable way to detect expired transactions (#10482)

When the root slot is beyond the last valid slot, we can say
with certainty that the blockhash is expired. Unfortunately,
we still can't say the transaction didn't land. It may have
landed a long time ago and the validator has since purged
its transaction status.

tokens/src/commands.rs

@@ -160,7 +160,7 @@ fn distribute_tokens<T: Client>(
         let fee_payer_pubkey = args.fee_payer.pubkey();
         let message = Message::new_with_payer(&instructions, Some(&fee_payer_pubkey));
         match client.send_message(message, &signers) {
-            Ok((transaction, _last_valid_slot)) => {
+            Ok((transaction, last_valid_slot)) => {
                 db::set_transaction_info(
                     db,
                     &allocation.recipient.parse().unwrap(),
@@ -168,6 +168,7 @@ fn distribute_tokens<T: Client>(
                     &transaction,
                     Some(&new_stake_account_address),
                     false,
+                    last_valid_slot,
                 )?;
             }
             Err(e) => {
@@ -332,20 +333,20 @@ fn update_finalized_transactions<T: Client>(
             if info.finalized_date.is_some() {
                 None
             } else {
-                Some(&info.transaction)
+                Some((&info.transaction, info.last_valid_slot))
             }
         })
         .collect();
     let unconfirmed_signatures: Vec<_> = unconfirmed_transactions
         .iter()
-        .map(|tx| tx.signatures[0])
+        .map(|(tx, _slot)| tx.signatures[0])
         .filter(|sig| *sig != Signature::default()) // Filter out dry-run signatures
         .collect();
     let transaction_statuses = client.get_signature_statuses(&unconfirmed_signatures)?;
-    let recent_blockhashes = client.get_recent_blockhashes()?;
+    let root_slot = client.get_slot()?;
 
     let mut confirmations = None;
-    for (transaction, opt_transaction_status) in unconfirmed_transactions
+    for ((transaction, last_valid_slot), opt_transaction_status) in unconfirmed_transactions
         .into_iter()
         .zip(transaction_statuses.into_iter())
     {
@@ -353,8 +354,8 @@ fn update_finalized_transactions<T: Client>(
             db,
             &transaction.signatures[0],
             opt_transaction_status,
-            &transaction.message.recent_blockhash,
+            last_valid_slot,
-            &recent_blockhashes,
+            root_slot,
         ) {
             Ok(Some(confs)) => {
                 confirmations = Some(cmp::min(confs, confirmations.unwrap_or(usize::MAX)));

tokens/src/db.rs

@@ -1,7 +1,7 @@
 use chrono::prelude::*;
 use pickledb::{error::Error, PickleDb, PickleDbDumpPolicy};
 use serde::{Deserialize, Serialize};
-use solana_sdk::{hash::Hash, pubkey::Pubkey, signature::Signature, transaction::Transaction};
+use solana_sdk::{clock::Slot, pubkey::Pubkey, signature::Signature, transaction::Transaction};
 use solana_transaction_status::TransactionStatus;
 use std::{cmp::Ordering, fs, io, path::Path};
 
@@ -12,6 +12,7 @@ pub struct TransactionInfo {
     pub new_stake_account_address: Option<Pubkey>,
     pub finalized_date: Option<DateTime<Utc>>,
     pub transaction: Transaction,
+    pub last_valid_slot: Slot,
 }
 
 #[derive(Serialize, Deserialize, Debug, Default, PartialEq)]
@@ -35,6 +36,7 @@ impl Default for TransactionInfo {
             new_stake_account_address: None,
             finalized_date: None,
             transaction,
+            last_valid_slot: 0,
         }
     }
 }
@@ -103,6 +105,7 @@ pub fn set_transaction_info(
     transaction: &Transaction,
     new_stake_account_address: Option<&Pubkey>,
     finalized: bool,
+    last_valid_slot: Slot,
 ) -> Result<(), Error> {
     let finalized_date = if finalized { Some(Utc::now()) } else { None };
     let transaction_info = TransactionInfo {
@@ -111,6 +114,7 @@ pub fn set_transaction_info(
         new_stake_account_address: new_stake_account_address.cloned(),
         finalized_date,
         transaction: transaction.clone(),
+        last_valid_slot,
     };
     let signature = transaction.signatures[0];
     db.set(&signature.to_string(), &transaction_info)?;
@@ -119,20 +123,22 @@ pub fn set_transaction_info(
 
 // Set the finalized bit in the database if the transaction is rooted.
 // Remove the TransactionInfo from the database if the transaction failed.
-// Return the number of confirmations on the transaction or None if finalized.
+// Return the number of confirmations on the transaction or None if either
+// finalized or discarded.
 pub fn update_finalized_transaction(
     db: &mut PickleDb,
     signature: &Signature,
     opt_transaction_status: Option<TransactionStatus>,
-    blockhash: &Hash,
+    last_valid_slot: Slot,
-    recent_blockhashes: &[Hash],
+    root_slot: Slot,
 ) -> Result<Option<usize>, Error> {
     if opt_transaction_status.is_none() {
-        if !recent_blockhashes.contains(blockhash) {
+        if root_slot > last_valid_slot {
             eprintln!(
-                "Signature not found {} and blockhash not found, likely expired",
+                "Signature not found {} and blockhash expired. Transaction either dropped or the validator purged the transaction status.",
                 signature
             );
+
             // Don't discard the transaction, because we are not certain the
             // blockhash is expired. Instead, return None to signal that
             // we don't need to wait for confirmations.
@@ -161,7 +167,7 @@ pub fn update_finalized_transaction(
         return Ok(None);
     }
 
-    // Transaction is rooted. Set finalized in the database.
+    // Transaction is rooted. Set the finalized date in the database.
     let mut transaction_info = db.get::<TransactionInfo>(&signature.to_string()).unwrap();
     transaction_info.finalized_date = Some(Utc::now());
     db.set(&signature.to_string(), &transaction_info)?;
@@ -230,12 +236,10 @@ mod tests {
         let mut db =
             PickleDb::new_yaml(NamedTempFile::new().unwrap(), PickleDbDumpPolicy::NeverDump);
         let signature = Signature::default();
-        let blockhash = Hash::default();
         let transaction_info = TransactionInfo::default();
         db.set(&signature.to_string(), &transaction_info).unwrap();
         assert!(matches!(
-            update_finalized_transaction(&mut db, &signature, None, &blockhash, &[blockhash])
+            update_finalized_transaction(&mut db, &signature, None, 0, 0).unwrap(),
-                .unwrap(),
             Some(0)
         ));
 
@@ -247,7 +251,7 @@ mod tests {
 
         // Same as before, but now with an expired blockhash
         assert_eq!(
-            update_finalized_transaction(&mut db, &signature, None, &blockhash, &[]).unwrap(),
+            update_finalized_transaction(&mut db, &signature, None, 0, 1).unwrap(),
             None
         );
 
@@ -264,7 +268,6 @@ mod tests {
         let mut db =
             PickleDb::new_yaml(NamedTempFile::new().unwrap(), PickleDbDumpPolicy::NeverDump);
         let signature = Signature::default();
-        let blockhash = Hash::default();
         let transaction_info = TransactionInfo::default();
         db.set(&signature.to_string(), &transaction_info).unwrap();
         let transaction_status = TransactionStatus {
@@ -274,14 +277,8 @@ mod tests {
             err: None,
         };
         assert_eq!(
-            update_finalized_transaction(
+            update_finalized_transaction(&mut db, &signature, Some(transaction_status), 0, 0)
-                &mut db,
+                .unwrap(),
-                &signature,
-                Some(transaction_status),
-                &blockhash,
-                &[blockhash]
-            )
-            .unwrap(),
             Some(1)
         );
 
@@ -298,7 +295,6 @@ mod tests {
         let mut db =
             PickleDb::new_yaml(NamedTempFile::new().unwrap(), PickleDbDumpPolicy::NeverDump);
         let signature = Signature::default();
-        let blockhash = Hash::default();
         let transaction_info = TransactionInfo::default();
         db.set(&signature.to_string(), &transaction_info).unwrap();
         let status = Err(TransactionError::AccountNotFound);
@@ -309,14 +305,8 @@ mod tests {
             err: None,
         };
         assert_eq!(
-            update_finalized_transaction(
+            update_finalized_transaction(&mut db, &signature, Some(transaction_status), 0, 0)
-                &mut db,
+                .unwrap(),
-                &signature,
-                Some(transaction_status),
-                &blockhash,
-                &[blockhash]
-            )
-            .unwrap(),
             None
         );
 
@@ -330,7 +320,6 @@ mod tests {
         let mut db =
             PickleDb::new_yaml(NamedTempFile::new().unwrap(), PickleDbDumpPolicy::NeverDump);
         let signature = Signature::default();
-        let blockhash = Hash::default();
         let transaction_info = TransactionInfo::default();
         db.set(&signature.to_string(), &transaction_info).unwrap();
         let transaction_status = TransactionStatus {
@@ -340,14 +329,8 @@ mod tests {
             err: None,
         };
         assert_eq!(
-            update_finalized_transaction(
+            update_finalized_transaction(&mut db, &signature, Some(transaction_status), 0, 0)
-                &mut db,
+                .unwrap(),
-                &signature,
-                Some(transaction_status),
-                &blockhash,
-                &[blockhash]
-            )
-            .unwrap(),
             None
         );
 

tokens/src/thin_client.rs

@@ -12,10 +12,6 @@ use solana_sdk::{
     signature::{Signature, Signer},
     signers::Signers,
     system_instruction,
-    sysvar::{
-        recent_blockhashes::{self, RecentBlockhashes},
-        Sysvar,
-    },
     transaction::Transaction,
     transport::{Result, TransportError},
 };
@@ -29,6 +25,7 @@ pub trait Client {
     ) -> Result<Vec<Option<TransactionStatus>>>;
     fn get_balance1(&self, pubkey: &Pubkey) -> Result<u64>;
     fn get_fees1(&self) -> Result<(Hash, FeeCalculator, Slot)>;
+    fn get_slot1(&self) -> Result<Slot>;
     fn get_account1(&self, pubkey: &Pubkey) -> Result<Option<Account>>;
 }
 
@@ -64,6 +61,11 @@ impl Client for RpcClient {
         Ok(result.value)
     }
 
+    fn get_slot1(&self) -> Result<Slot> {
+        self.get_slot()
+            .map_err(|e| TransportError::Custom(e.to_string()))
+    }
+
     fn get_account1(&self, pubkey: &Pubkey) -> Result<Option<Account>> {
         self.get_account(pubkey)
             .map(Some)
@@ -103,6 +105,10 @@ impl Client for BankClient {
         self.get_recent_blockhash_with_commitment(CommitmentConfig::default())
     }
 
+    fn get_slot1(&self) -> Result<Slot> {
+        self.get_slot()
+    }
+
     fn get_account1(&self, pubkey: &Pubkey) -> Result<Option<Account>> {
         self.get_account(pubkey)
     }
@@ -170,6 +176,10 @@ impl<C: Client> ThinClient<C> {
         self.client.get_fees1()
     }
 
+    pub fn get_slot(&self) -> Result<Slot> {
+        self.client.get_slot1()
+    }
+
     pub fn get_balance(&self, pubkey: &Pubkey) -> Result<u64> {
         self.client.get_balance1(pubkey)
     }
@@ -177,12 +187,4 @@ impl<C: Client> ThinClient<C> {
     pub fn get_account(&self, pubkey: &Pubkey) -> Result<Option<Account>> {
         self.client.get_account1(pubkey)
     }
-
-    pub fn get_recent_blockhashes(&self) -> Result<Vec<Hash>> {
-        let opt_blockhashes_account = self.get_account(&recent_blockhashes::id())?;
-        let blockhashes_account = opt_blockhashes_account.unwrap();
-        let recent_blockhashes = RecentBlockhashes::from_account(&blockhashes_account).unwrap();
-        let hashes = recent_blockhashes.iter().map(|x| x.blockhash).collect();
-        Ok(hashes)
-    }
 }