send-transaction-service: use message hash and blockhash to look up committed tx status (#6585)

* sts: Use message hash to look up tx commitment status

* feedback

banks-server/src/banks_server.rs

@@ -209,6 +209,7 @@ fn simulate_transaction(
 #[tarpc::server]
 impl Banks for BanksServer {
     async fn send_transaction_with_context(self, _: Context, transaction: VersionedTransaction) {
+        let message_hash = transaction.message.hash();
         let blockhash = transaction.message.recent_blockhash();
         let last_valid_block_height = self
             .bank_forks
@@ -219,7 +220,9 @@ impl Banks for BanksServer {
             .unwrap();
         let signature = transaction.signatures.first().cloned().unwrap_or_default();
         let info = TransactionInfo::new(
+            message_hash,
             signature,
+            *blockhash,
             serialize(&transaction).unwrap(),
             last_valid_block_height,
             None,
@@ -322,6 +325,7 @@ impl Banks for BanksServer {
             return Some(Err(err));
         }
 
+        let message_hash = sanitized_transaction.message_hash();
         let blockhash = transaction.message.recent_blockhash();
         let last_valid_block_height = self
             .bank(commitment)
@@ -329,7 +333,9 @@ impl Banks for BanksServer {
             .unwrap();
         let signature = sanitized_transaction.signature();
         let info = TransactionInfo::new(
+            *message_hash,
             *signature,
+            *blockhash,
             serialize(&transaction).unwrap(),
             last_valid_block_height,
             None,

rpc/src/rpc.rs

@@ -2683,14 +2683,18 @@ fn get_token_program_id_and_mint(
 
 fn _send_transaction(
     meta: JsonRpcRequestProcessor,
+    message_hash: Hash,
     signature: Signature,
+    blockhash: Hash,
     wire_transaction: Vec<u8>,
     last_valid_block_height: u64,
     durable_nonce_info: Option<(Pubkey, Hash)>,
     max_retries: Option<usize>,
 ) -> Result<String> {
     let transaction_info = TransactionInfo::new(
+        message_hash,
         signature,
+        blockhash,
         wire_transaction,
         last_valid_block_height,
         durable_nonce_info,
@@ -3800,6 +3804,7 @@ pub mod rpc_full {
                 Error::internal_error()
             })?;
 
+            let message_hash = transaction.message().hash();
             let signature = if !transaction.signatures.is_empty() {
                 transaction.signatures[0]
             } else {
@@ -3808,7 +3813,9 @@ pub mod rpc_full {
 
             _send_transaction(
                 meta,
+                message_hash,
                 signature,
+                blockhash,
                 wire_transaction,
                 last_valid_block_height,
                 None,
@@ -3854,15 +3861,17 @@ pub mod rpc_full {
                 preflight_bank,
                 preflight_bank.get_reserved_account_keys(),
             )?;
+            let blockhash = *transaction.message().recent_blockhash();
+            let message_hash = *transaction.message_hash();
             let signature = *transaction.signature();
 
             let mut last_valid_block_height = preflight_bank
-                .get_blockhash_last_valid_block_height(transaction.message().recent_blockhash())
+                .get_blockhash_last_valid_block_height(&blockhash)
                 .unwrap_or(0);
 
             let durable_nonce_info = transaction
                 .get_durable_nonce()
-                .map(|&pubkey| (pubkey, *transaction.message().recent_blockhash()));
+                .map(|&pubkey| (pubkey, blockhash));
             if durable_nonce_info.is_some() || (skip_preflight && last_valid_block_height == 0) {
                 // While it uses a defined constant, this last_valid_block_height value is chosen arbitrarily.
                 // It provides a fallback timeout for durable-nonce transaction retries in case of
@@ -3931,7 +3940,9 @@ pub mod rpc_full {
 
             _send_transaction(
                 meta,
+                message_hash,
                 signature,
+                blockhash,
                 wire_transaction,
                 last_valid_block_height,
                 durable_nonce_info,

runtime/src/bank.rs

@@ -5273,6 +5273,17 @@ impl Bank {
             .map(|v| v.1)
     }
 
+    pub fn get_committed_transaction_status_and_slot(
+        &self,
+        message_hash: &Hash,
+        transaction_blockhash: &Hash,
+    ) -> Option<(Slot, bool)> {
+        let rcache = self.status_cache.read().unwrap();
+        rcache
+            .get_status(message_hash, transaction_blockhash, &self.ancestors)
+            .map(|(slot, status)| (slot, status.is_ok()))
+    }
+
     pub fn get_signature_status_slot(&self, signature: &Signature) -> Option<(Slot, Result<()>)> {
         let rcache = self.status_cache.read().unwrap();
         rcache.get_status_any_blockhash(signature, &self.ancestors)

send-transaction-service/src/send_transaction_service.rs

@@ -69,7 +69,9 @@ pub struct SendTransactionService {
 }
 
 pub struct TransactionInfo {
+    pub message_hash: Hash,
     pub signature: Signature,
+    pub blockhash: Hash,
     pub wire_transaction: Vec<u8>,
     pub last_valid_block_height: u64,
     pub durable_nonce_info: Option<(Pubkey, Hash)>,
@@ -81,7 +83,9 @@ pub struct TransactionInfo {
 
 impl TransactionInfo {
     pub fn new(
+        message_hash: Hash,
         signature: Signature,
+        blockhash: Hash,
         wire_transaction: Vec<u8>,
         last_valid_block_height: u64,
         durable_nonce_info: Option<(Pubkey, Hash)>,
@@ -89,7 +93,9 @@ impl TransactionInfo {
         last_sent_time: Option<Instant>,
     ) -> Self {
         Self {
+            message_hash,
             signature,
+            blockhash,
             wire_transaction,
             last_valid_block_height,
             durable_nonce_info,
@@ -434,13 +440,22 @@ impl SendTransactionService {
             if transaction_info.durable_nonce_info.is_some() {
                 stats.nonced_transactions.fetch_add(1, Ordering::Relaxed);
             }
-            if root_bank.has_signature(signature) {
+            if root_bank
+                .get_committed_transaction_status_and_slot(
+                    &transaction_info.message_hash,
+                    &transaction_info.blockhash,
+                )
+                .is_some()
+            {
                 info!("Transaction is rooted: {}", signature);
                 result.rooted += 1;
                 stats.rooted_transactions.fetch_add(1, Ordering::Relaxed);
                 return false;
             }
-            let signature_status = working_bank.get_signature_status_slot(signature);
+            let signature_status = working_bank.get_committed_transaction_status_and_slot(
+                &transaction_info.message_hash,
+                &transaction_info.blockhash,
+            );
             if let Some((nonce_pubkey, durable_nonce)) = transaction_info.durable_nonce_info {
                 let nonce_account = working_bank.get_account(&nonce_pubkey).unwrap_or_default();
                 let now = Instant::now();
@@ -518,7 +533,7 @@ impl SendTransactionService {
                     true
                 }
                 Some((_slot, status)) => {
-                    if status.is_err() {
+                    if !status {
                         info!("Dropping failed transaction: {}", signature);
                         result.failed += 1;
                         stats.failed_transactions.fetch_add(1, Ordering::Relaxed);
@@ -624,7 +639,9 @@ mod test {
         let (sender, receiver) = bounded(0);
 
         let dummy_tx_info = || TransactionInfo {
+            message_hash: Hash::default(),
             signature: Signature::default(),
+            blockhash: Hash::default(),
             wire_transaction: vec![0; 128],
             last_valid_block_height: 0,
             durable_nonce_info: None,
@@ -690,9 +707,17 @@ mod test {
             .insert(root_bank)
             .clone_without_scheduler();
 
-        let rooted_signature = root_bank
-            .transfer(1, &mint_keypair, &mint_keypair.pubkey())
-            .unwrap();
+        let (rooted_transaction, rooted_signature) = {
+            let transaction = system_transaction::transfer(
+                &mint_keypair,
+                &mint_keypair.pubkey(),
+                1,
+                root_bank.last_blockhash(),
+            );
+            root_bank.process_transaction(&transaction).unwrap();
+            let signature = transaction.signatures[0];
+            (transaction, signature)
+        };
 
         let working_bank = bank_forks
             .write()
@@ -704,17 +729,28 @@ mod test {
             ))
             .clone_without_scheduler();
 
-        let non_rooted_signature = working_bank
-            .transfer(2, &mint_keypair, &mint_keypair.pubkey())
-            .unwrap();
-
-        let failed_signature = {
-            let blockhash = working_bank.last_blockhash();
-            let transaction =
-                system_transaction::transfer(&mint_keypair, &Pubkey::default(), 1, blockhash);
+        let (non_rooted_transaction, non_rooted_signature) = {
+            let transaction = system_transaction::transfer(
+                &mint_keypair,
+                &mint_keypair.pubkey(),
+                2,
+                working_bank.last_blockhash(),
+            );
+            working_bank.process_transaction(&transaction).unwrap();
             let signature = transaction.signatures[0];
+            (transaction, signature)
+        };
+
+        let (failed_transaction, failed_signature) = {
+            let transaction = system_transaction::transfer(
+                &mint_keypair,
+                &Pubkey::default(),
+                1,
+                working_bank.last_blockhash(),
+            );
             working_bank.process_transaction(&transaction).unwrap_err();
-            signature
+            let signature = transaction.signatures[0];
+            (transaction, signature)
         };
 
         let mut transactions = HashMap::new();
@@ -724,7 +760,9 @@ mod test {
         transactions.insert(
             Signature::default(),
             TransactionInfo::new(
+                Hash::default(),
                 Signature::default(),
+                Hash::default(),
                 vec![],
                 root_bank.block_height() - 1,
                 None,
@@ -760,7 +798,9 @@ mod test {
         transactions.insert(
             rooted_signature,
             TransactionInfo::new(
+                rooted_transaction.message.hash(),
                 rooted_signature,
+                rooted_transaction.message.recent_blockhash,
                 vec![],
                 working_bank.block_height(),
                 None,
@@ -789,7 +829,9 @@ mod test {
         transactions.insert(
             failed_signature,
             TransactionInfo::new(
+                failed_transaction.message.hash(),
                 failed_signature,
+                failed_transaction.message.recent_blockhash,
                 vec![],
                 working_bank.block_height(),
                 None,
@@ -818,7 +860,9 @@ mod test {
         transactions.insert(
             non_rooted_signature,
             TransactionInfo::new(
+                non_rooted_transaction.message.hash(),
                 non_rooted_signature,
+                non_rooted_transaction.message.recent_blockhash,
                 vec![],
                 working_bank.block_height(),
                 None,
@@ -848,7 +892,9 @@ mod test {
         transactions.insert(
             Signature::default(),
             TransactionInfo::new(
+                Hash::default(),
                 Signature::default(),
+                Hash::default(),
                 vec![],
                 working_bank.block_height(),
                 None,
@@ -879,7 +925,9 @@ mod test {
         transactions.insert(
             Signature::from([1; 64]),
             TransactionInfo::new(
+                Hash::default(),
                 Signature::default(),
+                Hash::default(),
                 vec![],
                 working_bank.block_height(),
                 None,
@@ -890,7 +938,9 @@ mod test {
         transactions.insert(
             Signature::from([2; 64]),
             TransactionInfo::new(
+                Hash::default(),
                 Signature::default(),
+                Hash::default(),
                 vec![],
                 working_bank.block_height(),
                 None,
@@ -948,9 +998,17 @@ mod test {
             .insert(root_bank)
             .clone_without_scheduler();
 
-        let rooted_signature = root_bank
-            .transfer(1, &mint_keypair, &mint_keypair.pubkey())
-            .unwrap();
+        let (rooted_transaction, rooted_signature) = {
+            let transaction = system_transaction::transfer(
+                &mint_keypair,
+                &mint_keypair.pubkey(),
+                1,
+                root_bank.last_blockhash(),
+            );
+            root_bank.process_transaction(&transaction).unwrap();
+            let signature = transaction.signatures[0];
+            (transaction, signature)
+        };
 
         let nonce_address = Pubkey::new_unique();
         let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique());
@@ -970,19 +1028,28 @@ mod test {
                 2,
             ))
             .clone_without_scheduler();
-        let non_rooted_signature = working_bank
-            .transfer(2, &mint_keypair, &mint_keypair.pubkey())
-            .unwrap();
+
+        let (non_rooted_transaction, non_rooted_signature) = {
+            let transaction = system_transaction::transfer(
+                &mint_keypair,
+                &mint_keypair.pubkey(),
+                2,
+                working_bank.last_blockhash(),
+            );
+            let signature = transaction.signatures[0];
+            working_bank.process_transaction(&transaction).unwrap();
+            (transaction, signature)
+        };
 
         let last_valid_block_height = working_bank.block_height() + 300;
 
-        let failed_signature = {
+        let (failed_transaction, failed_signature) = {
             let blockhash = working_bank.last_blockhash();
             let transaction =
                 system_transaction::transfer(&mint_keypair, &Pubkey::default(), 1, blockhash);
             let signature = transaction.signatures[0];
             working_bank.process_transaction(&transaction).unwrap_err();
-            signature
+            (transaction, signature)
         };
 
         let mut transactions = HashMap::new();
@@ -991,7 +1058,9 @@ mod test {
         transactions.insert(
             rooted_signature,
             TransactionInfo::new(
+                rooted_transaction.message.hash(),
                 rooted_signature,
+                rooted_transaction.message.recent_blockhash,
                 vec![],
                 last_valid_block_height,
                 Some((nonce_address, *durable_nonce.as_hash())),
@@ -1026,7 +1095,9 @@ mod test {
         transactions.insert(
             rooted_signature,
             TransactionInfo::new(
+                rooted_transaction.message.hash(),
                 rooted_signature,
+                rooted_transaction.message.recent_blockhash,
                 vec![],
                 last_valid_block_height,
                 Some((nonce_address, Hash::new_unique())),
@@ -1056,7 +1127,9 @@ mod test {
         transactions.insert(
             Signature::default(),
             TransactionInfo::new(
+                Hash::default(),
                 Signature::default(),
+                Hash::default(),
                 vec![],
                 last_valid_block_height,
                 Some((nonce_address, Hash::new_unique())),
@@ -1084,7 +1157,9 @@ mod test {
         transactions.insert(
             Signature::default(),
             TransactionInfo::new(
+                Hash::default(),
                 Signature::default(),
+                Hash::default(),
                 vec![],
                 root_bank.block_height() - 1,
                 Some((nonce_address, *durable_nonce.as_hash())),
@@ -1113,7 +1188,9 @@ mod test {
         transactions.insert(
             failed_signature,
             TransactionInfo::new(
+                failed_transaction.message.hash(),
                 failed_signature,
+                failed_transaction.message.recent_blockhash,
                 vec![],
                 last_valid_block_height,
                 Some((nonce_address, Hash::new_unique())), // runtime should advance nonce on failed transactions
@@ -1142,7 +1219,9 @@ mod test {
         transactions.insert(
             non_rooted_signature,
             TransactionInfo::new(
+                non_rooted_transaction.message.hash(),
                 non_rooted_signature,
+                non_rooted_transaction.message.recent_blockhash,
                 vec![],
                 last_valid_block_height,
                 Some((nonce_address, Hash::new_unique())), // runtime advances nonce when transaction lands
@@ -1173,7 +1252,9 @@ mod test {
         transactions.insert(
             Signature::default(),
             TransactionInfo::new(
+                Hash::default(),
                 Signature::default(),
+                Hash::default(),
                 vec![],
                 last_valid_block_height,
                 Some((nonce_address, *durable_nonce.as_hash())),