svm: advance nonce for fee-only transactions sooner

core/src/banking_stage/committer.rs

@@ -12,7 +12,7 @@ use {
         transaction_batch::TransactionBatch,
         vote_sender_types::ReplayVoteSender,
     },
-    solana_sdk::{hash::Hash, pubkey::Pubkey, saturating_add_assign},
+    solana_sdk::{pubkey::Pubkey, saturating_add_assign},
     solana_svm::{
         transaction_commit_result::{TransactionCommitResult, TransactionCommitResultExtensions},
         transaction_processing_result::{
@@ -65,13 +65,10 @@ impl Committer {
         self.transaction_status_sender.is_some()
     }
 
-    #[allow(clippy::too_many_arguments)]
     pub(super) fn commit_transactions(
         &self,
         batch: &TransactionBatch,
         processing_results: Vec<TransactionProcessingResult>,
-        last_blockhash: Hash,
-        lamports_per_signature: u64,
         starting_transaction_index: Option<usize>,
         bank: &Arc<Bank>,
         pre_balance_info: &mut PreBalanceInfo,
@@ -87,8 +84,6 @@ impl Committer {
         let (commit_results, commit_time_us) = measure_us!(bank.commit_transactions(
             batch.sanitized_transactions(),
             processing_results,
-            last_blockhash,
-            lamports_per_signature,
             processed_counts,
             &mut execute_and_commit_timings.execute_timings,
         ));

core/src/banking_stage/consume_worker.rs

@@ -275,7 +275,6 @@ impl ConsumeWorkerMetrics {
             collect_balances_us,
             load_execute_us,
             freeze_lock_us,
-            last_blockhash_us,
             record_us,
             commit_us,
             find_and_send_votes_us,
@@ -291,9 +290,6 @@ impl ConsumeWorkerMetrics {
         self.timing_metrics
             .freeze_lock_us
             .fetch_add(*freeze_lock_us, Ordering::Relaxed);
-        self.timing_metrics
-            .last_blockhash_us
-            .fetch_add(*last_blockhash_us, Ordering::Relaxed);
         self.timing_metrics
             .record_us
             .fetch_add(*record_us, Ordering::Relaxed);
@@ -494,7 +490,6 @@ struct ConsumeWorkerTimingMetrics {
     collect_balances_us: AtomicU64,
     load_execute_us: AtomicU64,
     freeze_lock_us: AtomicU64,
-    last_blockhash_us: AtomicU64,
     record_us: AtomicU64,
     commit_us: AtomicU64,
     find_and_send_votes_us: AtomicU64,
@@ -527,11 +522,6 @@ impl ConsumeWorkerTimingMetrics {
                 self.freeze_lock_us.swap(0, Ordering::Relaxed),
                 i64
             ),
-            (
-                "last_blockhash_us",
-                self.last_blockhash_us.swap(0, Ordering::Relaxed),
-                i64
-            ),
             ("record_us", self.record_us.swap(0, Ordering::Relaxed), i64),
             ("commit_us", self.commit_us.swap(0, Ordering::Relaxed), i64),
             (

core/src/banking_stage/consumer.rs

@@ -666,17 +666,6 @@ impl Consumer {
         let (freeze_lock, freeze_lock_us) = measure_us!(bank.freeze_lock());
         execute_and_commit_timings.freeze_lock_us = freeze_lock_us;
 
-        // In order to avoid a race condition, leaders must get the last
-        // blockhash *before* recording transactions because recording
-        // transactions will only succeed if the block max tick height hasn't
-        // been reached yet. If they get the last blockhash *after* recording
-        // transactions, the block max tick height could have already been
-        // reached and the blockhash queue could have already been updated with
-        // a new blockhash.
-        let ((last_blockhash, lamports_per_signature), last_blockhash_us) =
-            measure_us!(bank.last_blockhash_and_lamports_per_signature());
-        execute_and_commit_timings.last_blockhash_us = last_blockhash_us;
-
         let (record_transactions_summary, record_us) = measure_us!(self
             .transaction_recorder
             .record_transactions(bank.slot(), processed_transactions));
@@ -713,8 +702,6 @@ impl Consumer {
                 self.committer.commit_transactions(
                     batch,
                     processing_results,
-                    last_blockhash,
-                    lamports_per_signature,
                     starting_transaction_index,
                     bank,
                     &mut pre_balance_info,

core/src/banking_stage/leader_slot_timing_metrics.rs

@@ -10,7 +10,6 @@ pub struct LeaderExecuteAndCommitTimings {
     pub collect_balances_us: u64,
     pub load_execute_us: u64,
     pub freeze_lock_us: u64,
-    pub last_blockhash_us: u64,
     pub record_us: u64,
     pub commit_us: u64,
     pub find_and_send_votes_us: u64,
@@ -23,7 +22,6 @@ impl LeaderExecuteAndCommitTimings {
         saturating_add_assign!(self.collect_balances_us, other.collect_balances_us);
         saturating_add_assign!(self.load_execute_us, other.load_execute_us);
         saturating_add_assign!(self.freeze_lock_us, other.freeze_lock_us);
-        saturating_add_assign!(self.last_blockhash_us, other.last_blockhash_us);
         saturating_add_assign!(self.record_us, other.record_us);
         saturating_add_assign!(self.commit_us, other.commit_us);
         saturating_add_assign!(self.find_and_send_votes_us, other.find_and_send_votes_us);
@@ -40,7 +38,6 @@ impl LeaderExecuteAndCommitTimings {
             ("collect_balances_us", self.collect_balances_us as i64, i64),
             ("load_execute_us", self.load_execute_us as i64, i64),
             ("freeze_lock_us", self.freeze_lock_us as i64, i64),
-            ("last_blockhash_us", self.last_blockhash_us as i64, i64),
             ("record_us", self.record_us as i64, i64),
             ("commit_us", self.commit_us as i64, i64),
             (

runtime/src/account_saver.rs

@@ -1,8 +1,8 @@
 use {
     core::borrow::Borrow,
     solana_sdk::{
-        account::AccountSharedData, nonce::state::DurableNonce, pubkey::Pubkey,
-        transaction::SanitizedTransaction, transaction_context::TransactionAccount,
+        account::AccountSharedData, pubkey::Pubkey, transaction::SanitizedTransaction,
+        transaction_context::TransactionAccount,
     },
     solana_svm::{
         rollback_accounts::RollbackAccounts,
@@ -51,9 +51,7 @@ fn max_number_of_accounts_to_collect(
 pub fn collect_accounts_to_store<'a, T: SVMMessage>(
     txs: &'a [T],
     txs_refs: &'a Option<Vec<impl Borrow<SanitizedTransaction>>>,
-    processing_results: &'a mut [TransactionProcessingResult],
-    durable_nonce: &DurableNonce,
-    lamports_per_signature: u64,
+    processing_results: &'a [TransactionProcessingResult],
 ) -> (
     Vec<(&'a Pubkey, &'a AccountSharedData)>,
     Option<Vec<&'a SanitizedTransaction>>,
@@ -63,10 +61,9 @@ pub fn collect_accounts_to_store<'a, T: SVMMessage>(
     let mut transactions = txs_refs
         .is_some()
         .then(|| Vec::with_capacity(collect_capacity));
-    for (index, (processing_result, transaction)) in
-        processing_results.iter_mut().zip(txs).enumerate()
+    for (index, (processing_result, transaction)) in processing_results.iter().zip(txs).enumerate()
     {
-        let Some(processed_tx) = processing_result.processed_transaction_mut() else {
+        let Some(processed_tx) = processing_result.processed_transaction() else {
             // Don't store any accounts if tx wasn't executed
             continue;
         };
@@ -88,9 +85,7 @@ pub fn collect_accounts_to_store<'a, T: SVMMessage>(
                         &mut transactions,
                         transaction,
                         transaction_ref,
-                        &mut executed_tx.loaded_transaction.rollback_accounts,
-                        durable_nonce,
-                        lamports_per_signature,
+                        &executed_tx.loaded_transaction.rollback_accounts,
                     );
                 }
             }
@@ -100,9 +95,7 @@ pub fn collect_accounts_to_store<'a, T: SVMMessage>(
                     &mut transactions,
                     transaction,
                     transaction_ref,
-                    &mut fees_only_tx.rollback_accounts,
-                    durable_nonce,
-                    lamports_per_signature,
+                    &fees_only_tx.rollback_accounts,
                 );
             }
         }
@@ -142,25 +135,18 @@ fn collect_accounts_for_failed_tx<'a, T: SVMMessage>(
     collected_account_transactions: &mut Option<Vec<&'a SanitizedTransaction>>,
     transaction: &'a T,
     transaction_ref: Option<&'a SanitizedTransaction>,
-    rollback_accounts: &'a mut RollbackAccounts,
-    durable_nonce: &DurableNonce,
-    lamports_per_signature: u64,
+    rollback_accounts: &'a RollbackAccounts,
 ) {
     let fee_payer_address = transaction.fee_payer();
     match rollback_accounts {
         RollbackAccounts::FeePayerOnly { fee_payer_account } => {
-            collected_accounts.push((fee_payer_address, &*fee_payer_account));
+            collected_accounts.push((fee_payer_address, fee_payer_account));
             if let Some(collected_account_transactions) = collected_account_transactions {
                 collected_account_transactions
                     .push(transaction_ref.expect("transaction ref must exist if collecting"));
             }
         }
         RollbackAccounts::SameNonceAndFeePayer { nonce } => {
-            // Since we know we are dealing with a valid nonce account,
-            // unwrap is safe here
-            nonce
-                .try_advance_nonce(*durable_nonce, lamports_per_signature)
-                .unwrap();
             collected_accounts.push((nonce.address(), nonce.account()));
             if let Some(collected_account_transactions) = collected_account_transactions {
                 collected_account_transactions
@@ -171,17 +157,12 @@ fn collect_accounts_for_failed_tx<'a, T: SVMMessage>(
             nonce,
             fee_payer_account,
         } => {
-            collected_accounts.push((fee_payer_address, &*fee_payer_account));
+            collected_accounts.push((fee_payer_address, fee_payer_account));
             if let Some(collected_account_transactions) = collected_account_transactions {
                 collected_account_transactions
                     .push(transaction_ref.expect("transaction ref must exist if collecting"));
             }
 
-            // Since we know we are dealing with a valid nonce account,
-            // unwrap is safe here
-            nonce
-                .try_advance_nonce(*durable_nonce, lamports_per_signature)
-                .unwrap();
             collected_accounts.push((nonce.address(), nonce.account()));
             if let Some(collected_account_transactions) = collected_account_transactions {
                 collected_account_transactions
@@ -204,7 +185,7 @@ mod tests {
             message::Message,
             native_loader,
             nonce::{
-                state::{Data as NonceData, Versions as NonceVersions},
+                state::{Data as NonceData, DurableNonce, Versions as NonceVersions},
                 State as NonceState,
             },
             nonce_account,
@@ -315,7 +296,7 @@ mod tests {
         };
 
         let txs = vec![tx0.clone(), tx1.clone()];
-        let mut processing_results = vec![
+        let processing_results = vec![
             new_executed_processing_result(Ok(()), loaded0),
             new_executed_processing_result(Ok(()), loaded1),
         ];
@@ -324,13 +305,8 @@ mod tests {
 
         for collect_transactions in [false, true] {
             let transaction_refs = collect_transactions.then(|| txs.iter().collect::<Vec<_>>());
-            let (collected_accounts, transactions) = collect_accounts_to_store(
-                &txs,
-                &transaction_refs,
-                &mut processing_results,
-                &DurableNonce::default(),
-                0,
-            );
+            let (collected_accounts, transactions) =
+                collect_accounts_to_store(&txs, &transaction_refs, &processing_results);
             assert_eq!(collected_accounts.len(), 2);
             assert!(collected_accounts
                 .iter()
@@ -383,7 +359,7 @@ mod tests {
         };
 
         let txs = vec![tx];
-        let mut processing_results = vec![new_executed_processing_result(
+        let processing_results = vec![new_executed_processing_result(
             Err(TransactionError::InstructionError(
                 1,
                 InstructionError::InvalidArgument,
@@ -392,17 +368,11 @@ mod tests {
         )];
         let max_collected_accounts = max_number_of_accounts_to_collect(&txs, &processing_results);
         assert_eq!(max_collected_accounts, 1);
-        let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique());
 
         for collect_transactions in [false, true] {
             let transaction_refs = collect_transactions.then(|| txs.iter().collect::<Vec<_>>());
-            let (collected_accounts, transactions) = collect_accounts_to_store(
-                &txs,
-                &transaction_refs,
-                &mut processing_results,
-                &durable_nonce,
-                0,
-            );
+            let (collected_accounts, transactions) =
+                collect_accounts_to_store(&txs, &transaction_refs, &processing_results);
             assert_eq!(collected_accounts.len(), 1);
             assert_eq!(
                 collected_accounts
@@ -483,9 +453,8 @@ mod tests {
             loaded_accounts_data_size: 0,
         };
 
-        let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique());
         let txs = vec![tx];
-        let mut processing_results = vec![new_executed_processing_result(
+        let processing_results = vec![new_executed_processing_result(
             Err(TransactionError::InstructionError(
                 1,
                 InstructionError::InvalidArgument,
@@ -497,13 +466,8 @@ mod tests {
 
         for collect_transactions in [false, true] {
             let transaction_refs = collect_transactions.then(|| txs.iter().collect::<Vec<_>>());
-            let (collected_accounts, transactions) = collect_accounts_to_store(
-                &txs,
-                &transaction_refs,
-                &mut processing_results,
-                &durable_nonce,
-                0,
-            );
+            let (collected_accounts, transactions) =
+                collect_accounts_to_store(&txs, &transaction_refs, &processing_results);
             assert_eq!(collected_accounts.len(), 2);
             assert_eq!(
                 collected_accounts
@@ -597,9 +561,8 @@ mod tests {
             loaded_accounts_data_size: 0,
         };
 
-        let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique());
         let txs = vec![tx];
-        let mut processing_results = vec![new_executed_processing_result(
+        let processing_results = vec![new_executed_processing_result(
             Err(TransactionError::InstructionError(
                 1,
                 InstructionError::InvalidArgument,
@@ -611,13 +574,8 @@ mod tests {
 
         for collect_transactions in [false, true] {
             let transaction_refs = collect_transactions.then(|| txs.iter().collect::<Vec<_>>());
-            let (collected_accounts, transactions) = collect_accounts_to_store(
-                &txs,
-                &transaction_refs,
-                &mut processing_results,
-                &durable_nonce,
-                0,
-            );
+            let (collected_accounts, transactions) =
+                collect_accounts_to_store(&txs, &transaction_refs, &processing_results);
             assert_eq!(collected_accounts.len(), 1);
             let collected_nonce_account = collected_accounts
                 .iter()
@@ -658,7 +616,7 @@ mod tests {
         let from_account_pre = AccountSharedData::new(4242, 0, &Pubkey::default());
 
         let txs = vec![tx];
-        let mut processing_results = vec![Ok(ProcessedTransaction::FeesOnly(Box::new(
+        let processing_results = vec![Ok(ProcessedTransaction::FeesOnly(Box::new(
             FeesOnlyTransaction {
                 load_error: TransactionError::InvalidProgramForExecution,
                 fee_details: FeeDetails::default(),
@@ -669,17 +627,11 @@ mod tests {
         )))];
         let max_collected_accounts = max_number_of_accounts_to_collect(&txs, &processing_results);
         assert_eq!(max_collected_accounts, 1);
-        let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique());
 
         for collect_transactions in [false, true] {
             let transaction_refs = collect_transactions.then(|| txs.iter().collect::<Vec<_>>());
-            let (collected_accounts, transactions) = collect_accounts_to_store(
-                &txs,
-                &transaction_refs,
-                &mut processing_results,
-                &durable_nonce,
-                0,
-            );
+            let (collected_accounts, transactions) =
+                collect_accounts_to_store(&txs, &transaction_refs, &processing_results);
             assert_eq!(collected_accounts.len(), 1);
             assert_eq!(
                 collected_accounts