More reliable way to detect expired transactions (#10482) (#10505)

automerge

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)
-    }
 }