advance nonce when creating rollback accounts, instead of when commit…

…ting

runtime/src/bank.rs

@@ -125,7 +125,6 @@ use {
         message::{AccountKeys, SanitizedMessage},
         native_loader,
         native_token::LAMPORTS_PER_SOL,
-        nonce::state::DurableNonce,
         packet::PACKET_DATA_SIZE,
         precompiles::get_precompiles,
         pubkey::Pubkey,
@@ -3762,9 +3761,7 @@ impl Bank {
     pub fn commit_transactions(
         &self,
         sanitized_txs: &[SanitizedTransaction],
-        mut processing_results: Vec<TransactionProcessingResult>,
-        last_blockhash: Hash,
-        lamports_per_signature: u64,
+        processing_results: Vec<TransactionProcessingResult>,
         processed_counts: &ProcessedTransactionCounts,
         timings: &mut ExecuteTimings,
     ) -> Vec<TransactionCommitResult> {
@@ -3799,13 +3796,8 @@ impl Bank {
         }
 
         let ((), store_accounts_us) = measure_us!({
-            let durable_nonce = DurableNonce::from_blockhash(&last_blockhash);
-            let (accounts_to_store, transactions) = collect_accounts_to_store(
-                sanitized_txs,
-                &mut processing_results,
-                &durable_nonce,
-                lamports_per_signature,
-            );
+            let (accounts_to_store, transactions) =
+                collect_accounts_to_store(sanitized_txs, &processing_results);
             self.rc
                 .accounts
                 .store_cached((self.slot(), accounts_to_store.as_slice()), &transactions);
@@ -4622,13 +4614,9 @@ impl Bank {
             },
         );
 
-        let (last_blockhash, lamports_per_signature) =
-            self.last_blockhash_and_lamports_per_signature();
         let commit_results = self.commit_transactions(
             batch.sanitized_transactions(),
             processing_results,
-            last_blockhash,
-            lamports_per_signature,
             &processed_counts,
             timings,
         );

svm/src/account_saver.rs

@@ -7,8 +7,7 @@ use {
         },
     },
     solana_sdk::{
-        account::AccountSharedData, nonce::state::DurableNonce, pubkey::Pubkey,
-        transaction_context::TransactionAccount,
+        account::AccountSharedData, pubkey::Pubkey, transaction_context::TransactionAccount,
     },
     solana_svm_transaction::svm_message::SVMMessage,
 };
@@ -42,15 +41,13 @@ fn max_number_of_accounts_to_collect(
 
 pub fn collect_accounts_to_store<'a, T: SVMMessage>(
     txs: &'a [T],
-    processing_results: &'a mut [TransactionProcessingResult],
-    durable_nonce: &DurableNonce,
-    lamports_per_signature: u64,
+    processing_results: &'a [TransactionProcessingResult],
 ) -> (Vec<(&'a Pubkey, &'a AccountSharedData)>, Vec<Option<&'a T>>) {
     let collect_capacity = max_number_of_accounts_to_collect(txs, processing_results);
     let mut accounts = Vec::with_capacity(collect_capacity);
     let mut transactions = Vec::with_capacity(collect_capacity);
-    for (processing_result, transaction) in processing_results.iter_mut().zip(txs) {
-        let Some(processed_tx) = processing_result.processed_transaction_mut() else {
+    for (processing_result, transaction) in processing_results.iter().zip(txs) {
+        let Some(processed_tx) = processing_result.processed_transaction() else {
             // Don't store any accounts if tx wasn't executed
             continue;
         };
@@ -69,9 +66,7 @@ pub fn collect_accounts_to_store<'a, T: SVMMessage>(
                         &mut accounts,
                         &mut transactions,
                         transaction,
-                        &mut executed_tx.loaded_transaction.rollback_accounts,
-                        durable_nonce,
-                        lamports_per_signature,
+                        &executed_tx.loaded_transaction.rollback_accounts,
                     );
                 }
             }
@@ -80,9 +75,7 @@ pub fn collect_accounts_to_store<'a, T: SVMMessage>(
                     &mut accounts,
                     &mut transactions,
                     transaction,
-                    &mut fees_only_tx.rollback_accounts,
-                    durable_nonce,
-                    lamports_per_signature,
+                    &fees_only_tx.rollback_accounts,
                 );
             }
         }
@@ -117,37 +110,25 @@ fn collect_accounts_for_failed_tx<'a, T: SVMMessage>(
     collected_accounts: &mut Vec<(&'a Pubkey, &'a AccountSharedData)>,
     collected_account_transactions: &mut Vec<Option<&'a T>>,
     transaction: &'a T,
-    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));
             collected_account_transactions.push(Some(transaction));
         }
         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()));
             collected_account_transactions.push(Some(transaction));
         }
         RollbackAccounts::SeparateNonceAndFeePayer {
             nonce,
             fee_payer_account,
         } => {
-            collected_accounts.push((fee_payer_address, &*fee_payer_account));
+            collected_accounts.push((fee_payer_address, fee_payer_account));
             collected_account_transactions.push(Some(transaction));
 
-            // 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()));
             collected_account_transactions.push(Some(transaction));
         }

svm/src/rollback_accounts.rs

@@ -51,6 +51,10 @@ impl RollbackAccounts {
 
         if let Some(nonce) = nonce {
             if &fee_payer_address == nonce.address() {
+                fee_payer_account
+                    .data_as_mut_slice()
+                    .copy_from_slice(nonce.account().data());
+
                 RollbackAccounts::SameNonceAndFeePayer {
                     nonce: NonceInfo::new(fee_payer_address, fee_payer_account),
                 }

svm/src/transaction_processor.rs

@@ -41,7 +41,8 @@ use {
         fee::{FeeBudgetLimits, FeeStructure},
         hash::Hash,
         inner_instruction::{InnerInstruction, InnerInstructionsList},
-        instruction::{CompiledInstruction, TRANSACTION_LEVEL_STACK_HEIGHT},
+        instruction::{CompiledInstruction, InstructionError, TRANSACTION_LEVEL_STACK_HEIGHT},
+        nonce::state::DurableNonce,
         pubkey::Pubkey,
         rent_collector::RentCollector,
         saturating_add_assign,
@@ -235,6 +236,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
         let mut error_metrics = TransactionErrorMetrics::default();
         let mut execute_timings = ExecuteTimings::default();
 
+        let durable_nonce = DurableNonce::from_blockhash(&environment.blockhash);
         let (validation_results, validate_fees_us) = measure_us!(self.validate_fees(
             callbacks,
             config.account_overrides,
@@ -247,6 +249,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
             environment
                 .rent_collector
                 .unwrap_or(&RentCollector::default()),
+            &durable_nonce,
             &mut error_metrics
         ));
 
@@ -368,6 +371,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
         }
     }
 
+    #[allow(clippy::too_many_arguments)]
     fn validate_fees<CB: TransactionProcessingCallback, T: SVMMessage>(
         &self,
         callbacks: &CB,
@@ -377,6 +381,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
         feature_set: &FeatureSet,
         fee_structure: &FeeStructure,
         rent_collector: &RentCollector,
+        durable_nonce: &DurableNonce,
         error_counters: &mut TransactionErrorMetrics,
     ) -> Vec<TransactionValidationResult> {
         sanitized_txs
@@ -393,6 +398,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
                         feature_set,
                         fee_structure,
                         rent_collector,
+                        durable_nonce,
                         error_counters,
                     )
                 })
@@ -403,6 +409,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
     // Loads transaction fee payer, collects rent if necessary, then calculates
     // transaction fees, and deducts them from the fee payer balance. If the
     // account is not found or has insufficient funds, an error is returned.
+    #[allow(clippy::too_many_arguments)]
     fn validate_transaction_fee_payer<CB: TransactionProcessingCallback>(
         &self,
         callbacks: &CB,
@@ -412,6 +419,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
         feature_set: &FeatureSet,
         fee_structure: &FeeStructure,
         rent_collector: &RentCollector,
+        durable_nonce: &DurableNonce,
         error_counters: &mut TransactionErrorMetrics,
     ) -> transaction::Result<ValidatedTransactionDetails> {
         let compute_budget_limits = process_compute_budget_instructions(
@@ -442,7 +450,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
         .rent_amount;
 
         let CheckedTransactionDetails {
-            nonce,
+            mut nonce,
             lamports_per_signature,
         } = checked_details;
 
@@ -465,8 +473,18 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
             fee_details.total_fee(),
         )?;
 
-        // Capture fee-subtracted fee payer account and original nonce account state
-        // to rollback to if transaction execution fails.
+        if let Some(ref mut nonce_info) = nonce {
+            // Coding instruction 0 is appropriate because it must be the advance nonce instruction
+            // However the nonce info is guaranteed to be valid so this should never occur
+            nonce_info
+                .try_advance_nonce(*durable_nonce, lamports_per_signature)
+                .map_err(|_| {
+                    TransactionError::InstructionError(0, InstructionError::InvalidAccountData)
+                })?;
+        };
+
+        // Capture fee-subtracted fee payer account and next nonce account state
+        // to commit if transaction execution fails.
         let rollback_accounts = RollbackAccounts::new(
             nonce,
             *fee_payer_address,