Browse Source

refactor: consolidate fee deduction for failed transactions (#1636)

Justin Starry 1 year ago
parent
commit
8da4062644

+ 1 - 0
accounts-db/Cargo.toml

@@ -67,6 +67,7 @@ serde_bytes = { workspace = true }
 solana-accounts-db = { path = ".", features = ["dev-context-only-utils"] }
 solana-logger = { workspace = true }
 solana-sdk = { workspace = true, features = ["dev-context-only-utils"] }
+solana-svm = { workspace = true, features = ["dev-context-only-utils"] }
 static_assertions = { workspace = true }
 strum = { workspace = true, features = ["derive"] }
 strum_macros = { workspace = true }

+ 100 - 146
accounts-db/src/accounts.rs

@@ -29,9 +29,8 @@ use {
         transaction_context::TransactionAccount,
     },
     solana_svm::{
-        account_loader::TransactionLoadResult,
-        nonce_info::{NonceFull, NonceInfo},
-        transaction_results::TransactionExecutionResult,
+        account_loader::TransactionLoadResult, nonce_info::NonceInfo,
+        rollback_accounts::RollbackAccounts, transaction_results::TransactionExecutionResult,
     },
     std::{
         cmp::Reverse,
@@ -723,21 +722,6 @@ impl Accounts {
                 TransactionExecutionResult::NotExecuted(_) => continue,
             };
 
-            enum AccountCollectionMode<'a> {
-                Normal,
-                FailedWithNonce { nonce: &'a NonceFull },
-            }
-
-            let collection_mode = match (execution_status, &loaded_transaction.nonce) {
-                (Ok(_), _) => AccountCollectionMode::Normal,
-                (Err(_), Some(nonce)) => AccountCollectionMode::FailedWithNonce { nonce },
-                (Err(_), None) => {
-                    // Fees for failed transactions which don't use durable nonces are
-                    // deducted in Bank::filter_program_errors_and_collect_fee
-                    continue;
-                }
-            };
-
             // Accounts that are invoked and also not passed as an instruction
             // account to a program don't need to be stored because it's assumed
             // to be impossible for a committable transaction to modify an
@@ -755,21 +739,24 @@ impl Accounts {
             };
 
             let message = tx.message();
+            let rollback_accounts = &loaded_transaction.rollback_accounts;
+            let maybe_nonce_address = rollback_accounts.nonce().map(|account| account.address());
+
             for (i, (address, account)) in (0..message.account_keys().len())
                 .zip(loaded_transaction.accounts.iter_mut())
                 .filter(|(i, _)| is_storable_account(message, *i))
             {
                 if message.is_writable(i) {
-                    let should_collect_account = match collection_mode {
-                        AccountCollectionMode::Normal => true,
-                        AccountCollectionMode::FailedWithNonce { nonce } => {
+                    let should_collect_account = match execution_status {
+                        Ok(()) => true,
+                        Err(_) => {
                             let is_fee_payer = i == 0;
-                            let is_nonce_account = address == nonce.address();
-                            post_process_failed_nonce(
+                            let is_nonce_account = Some(&*address) == maybe_nonce_address;
+                            post_process_failed_tx(
                                 account,
                                 is_fee_payer,
                                 is_nonce_account,
-                                nonce,
+                                rollback_accounts,
                                 durable_nonce,
                                 lamports_per_signature,
                             );
@@ -790,41 +777,43 @@ impl Accounts {
     }
 }
 
-fn post_process_failed_nonce(
+fn post_process_failed_tx(
     account: &mut AccountSharedData,
     is_fee_payer: bool,
     is_nonce_account: bool,
-    nonce: &NonceFull,
+    rollback_accounts: &RollbackAccounts,
     &durable_nonce: &DurableNonce,
     lamports_per_signature: u64,
 ) {
+    // For the case of RollbackAccounts::SameNonceAndFeePayer, it's crucial
+    // for `is_nonce_account` to be checked earlier than `is_fee_payer`.
     if is_nonce_account {
-        // The transaction failed which would normally drop the account
-        // processing changes, since this account is now being included
-        // in the accounts written back to the db, roll it back to
-        // pre-processing state.
-        *account = nonce.account().clone();
-
-        // Advance the stored blockhash to prevent fee theft by someone
-        // replaying nonce transactions that have failed with an
-        // `InstructionError`.
-        //
-        // Since we know we are dealing with a valid nonce account,
-        // unwrap is safe here
-        let nonce_versions = StateMut::<NonceVersions>::state(account).unwrap();
-        if let NonceState::Initialized(ref data) = nonce_versions.state() {
-            let nonce_state =
-                NonceState::new_initialized(&data.authority, durable_nonce, lamports_per_signature);
-            let nonce_versions = NonceVersions::new(nonce_state);
-            account.set_state(&nonce_versions).unwrap();
+        if let Some(nonce) = rollback_accounts.nonce() {
+            // The transaction failed which would normally drop the account
+            // processing changes, since this account is now being included
+            // in the accounts written back to the db, roll it back to
+            // pre-processing state.
+            *account = nonce.account().clone();
+
+            // Advance the stored blockhash to prevent fee theft by someone
+            // replaying nonce transactions that have failed with an
+            // `InstructionError`.
+            //
+            // Since we know we are dealing with a valid nonce account,
+            // unwrap is safe here
+            let nonce_versions = StateMut::<NonceVersions>::state(account).unwrap();
+            if let NonceState::Initialized(ref data) = nonce_versions.state() {
+                let nonce_state = NonceState::new_initialized(
+                    &data.authority,
+                    durable_nonce,
+                    lamports_per_signature,
+                );
+                let nonce_versions = NonceVersions::new(nonce_state);
+                account.set_state(&nonce_versions).unwrap();
+            }
         }
     } else if is_fee_payer {
-        if let Some(fee_payer_account) = nonce.fee_payer_account() {
-            // Instruction error and fee-payer for this nonce tx is not
-            // the nonce account itself, rollback the fee payer to the
-            // fee-paid original state.
-            *account = fee_payer_account.clone();
-        }
+        *account = rollback_accounts.fee_payer_account().clone();
     }
 }
 
@@ -840,14 +829,17 @@ mod tests {
             hash::Hash,
             instruction::{CompiledInstruction, InstructionError},
             message::{Message, MessageHeader},
-            native_loader, nonce, nonce_account,
+            native_loader,
+            nonce::state::Data as NonceData,
+            nonce_account,
             rent_debits::RentDebits,
             signature::{keypair_from_seed, signers::Signers, Keypair, Signer},
             system_instruction, system_program,
             transaction::{Transaction, MAX_TX_ACCOUNT_LOCKS},
         },
         solana_svm::{
-            account_loader::LoadedTransaction, transaction_results::TransactionExecutionDetails,
+            account_loader::LoadedTransaction, nonce_info::NoncePartial,
+            transaction_results::TransactionExecutionDetails,
         },
         std::{
             borrow::Cow,
@@ -869,17 +861,13 @@ mod tests {
         ))
     }
 
-    fn new_execution_result(
-        status: Result<()>,
-        nonce: Option<&NonceFull>,
-    ) -> TransactionExecutionResult {
+    fn new_execution_result(status: Result<()>) -> TransactionExecutionResult {
         TransactionExecutionResult::Executed {
             details: TransactionExecutionDetails {
                 status,
                 log_messages: None,
                 inner_instructions: None,
                 fee_details: FeeDetails::default(),
-                is_nonce: nonce.is_some(),
                 return_data: None,
                 executed_units: 0,
                 accounts_data_len_delta: 0,
@@ -1576,8 +1564,8 @@ mod tests {
         let loaded0 = Ok(LoadedTransaction {
             accounts: transaction_accounts0,
             program_indices: vec![],
-            nonce: None,
             fee_details: FeeDetails::default(),
+            rollback_accounts: RollbackAccounts::default(),
             rent: 0,
             rent_debits: RentDebits::default(),
             loaded_accounts_data_size: 0,
@@ -1586,8 +1574,8 @@ mod tests {
         let loaded1 = Ok(LoadedTransaction {
             accounts: transaction_accounts1,
             program_indices: vec![],
-            nonce: None,
             fee_details: FeeDetails::default(),
+            rollback_accounts: RollbackAccounts::default(),
             rent: 0,
             rent_debits: RentDebits::default(),
             loaded_accounts_data_size: 0,
@@ -1605,7 +1593,7 @@ mod tests {
                 .insert_new_readonly(&pubkey);
         }
         let txs = vec![tx0.clone(), tx1.clone()];
-        let execution_results = vec![new_execution_result(Ok(()), None); 2];
+        let execution_results = vec![new_execution_result(Ok(())); 2];
         let (collected_accounts, transactions) = accounts.collect_accounts_to_store(
             &txs,
             &execution_results,
@@ -1662,34 +1650,26 @@ mod tests {
         accounts.accounts_db.clean_accounts_for_tests();
     }
 
-    fn create_accounts_post_process_failed_nonce() -> (
+    fn create_accounts_post_process_failed_tx() -> (
         Pubkey,
         AccountSharedData,
         AccountSharedData,
         DurableNonce,
         u64,
-        Option<AccountSharedData>,
     ) {
-        let data = NonceVersions::new(NonceState::Initialized(nonce::state::Data::default()));
+        let data = NonceVersions::new(NonceState::Initialized(NonceData::default()));
         let account = AccountSharedData::new_data(42, &data, &system_program::id()).unwrap();
         let mut pre_account = account.clone();
         pre_account.set_lamports(43);
         let durable_nonce = DurableNonce::from_blockhash(&Hash::new(&[1u8; 32]));
-        (
-            Pubkey::default(),
-            pre_account,
-            account,
-            durable_nonce,
-            1234,
-            None,
-        )
+        (Pubkey::default(), pre_account, account, durable_nonce, 1234)
     }
 
-    fn run_post_process_failed_nonce_test(
+    fn run_post_process_failed_tx_test(
         account: &mut AccountSharedData,
         is_fee_payer: bool,
         is_nonce_account: bool,
-        nonce: &NonceFull,
+        rollback_accounts: &RollbackAccounts,
         durable_nonce: &DurableNonce,
         lamports_per_signature: u64,
         expect_account: &AccountSharedData,
@@ -1697,17 +1677,17 @@ mod tests {
         // Verify expect_account's relationship
         if !is_fee_payer {
             if is_nonce_account {
-                assert_ne!(expect_account, nonce.account());
+                assert_ne!(expect_account, rollback_accounts.nonce().unwrap().account());
             } else {
                 assert_eq!(expect_account, account);
             }
         }
 
-        post_process_failed_nonce(
+        post_process_failed_tx(
             account,
             is_fee_payer,
             is_nonce_account,
-            nonce,
+            rollback_accounts,
             durable_nonce,
             lamports_per_signature,
         );
@@ -1716,33 +1696,25 @@ mod tests {
     }
 
     #[test]
-    fn test_post_process_failed_nonce_expected() {
-        let (
-            pre_account_address,
-            pre_account,
-            mut post_account,
-            blockhash,
-            lamports_per_signature,
-            maybe_fee_payer_account,
-        ) = create_accounts_post_process_failed_nonce();
-        let nonce = NonceFull::new(
-            pre_account_address,
-            pre_account.clone(),
-            maybe_fee_payer_account,
-        );
+    fn test_post_process_failed_tx_expected() {
+        let (pre_account_address, pre_account, mut post_account, blockhash, lamports_per_signature) =
+            create_accounts_post_process_failed_tx();
+        let rollback_accounts = RollbackAccounts::SameNonceAndFeePayer {
+            nonce: NoncePartial::new(pre_account_address, pre_account.clone()),
+        };
 
         let mut expect_account = pre_account;
         expect_account
             .set_state(&NonceVersions::new(NonceState::Initialized(
-                nonce::state::Data::new(Pubkey::default(), blockhash, lamports_per_signature),
+                NonceData::new(Pubkey::default(), blockhash, lamports_per_signature),
             )))
             .unwrap();
 
-        assert!(run_post_process_failed_nonce_test(
+        assert!(run_post_process_failed_tx_test(
             &mut post_account,
             false, // is_fee_payer
             true,  // is_nonce_account
-            &nonce,
+            &rollback_accounts,
             &blockhash,
             lamports_per_signature,
             &expect_account,
@@ -1750,24 +1722,20 @@ mod tests {
     }
 
     #[test]
-    fn test_post_process_failed_nonce_not_nonce_address() {
-        let (
-            pre_account_address,
-            pre_account,
-            mut post_account,
-            blockhash,
-            lamports_per_signature,
-            maybe_fee_payer_account,
-        ) = create_accounts_post_process_failed_nonce();
+    fn test_post_process_failed_tx_not_nonce_address() {
+        let (pre_account_address, pre_account, mut post_account, blockhash, lamports_per_signature) =
+            create_accounts_post_process_failed_tx();
 
-        let nonce = NonceFull::new(pre_account_address, pre_account, maybe_fee_payer_account);
+        let rollback_accounts = RollbackAccounts::SameNonceAndFeePayer {
+            nonce: NoncePartial::new(pre_account_address, pre_account.clone()),
+        };
 
         let expect_account = post_account.clone();
-        assert!(run_post_process_failed_nonce_test(
+        assert!(run_post_process_failed_tx_test(
             &mut post_account,
             false, // is_fee_payer
             false, // is_nonce_account
-            &nonce,
+            &rollback_accounts,
             &blockhash,
             lamports_per_signature,
             &expect_account,
@@ -1781,27 +1749,26 @@ mod tests {
             AccountSharedData::new_data(42, &(), &system_program::id()).unwrap();
         let post_fee_payer_account =
             AccountSharedData::new_data(84, &[1, 2, 3, 4], &system_program::id()).unwrap();
-        let nonce = NonceFull::new(
-            Pubkey::new_unique(),
-            nonce_account,
-            Some(pre_fee_payer_account.clone()),
-        );
+        let rollback_accounts = RollbackAccounts::SeparateNonceAndFeePayer {
+            nonce: NoncePartial::new(Pubkey::new_unique(), nonce_account),
+            fee_payer_account: pre_fee_payer_account.clone(),
+        };
 
-        assert!(run_post_process_failed_nonce_test(
+        assert!(run_post_process_failed_tx_test(
             &mut post_fee_payer_account.clone(),
             false, // is_fee_payer
             false, // is_nonce_account
-            &nonce,
+            &rollback_accounts,
             &DurableNonce::default(),
             1,
             &post_fee_payer_account,
         ));
 
-        assert!(run_post_process_failed_nonce_test(
+        assert!(run_post_process_failed_tx_test(
             &mut post_fee_payer_account.clone(),
             true,  // is_fee_payer
             false, // is_nonce_account
-            &nonce,
+            &rollback_accounts,
             &DurableNonce::default(),
             1,
             &pre_fee_payer_account,
@@ -1816,7 +1783,7 @@ mod tests {
         let from_address = from.pubkey();
         let to_address = Pubkey::new_unique();
         let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique());
-        let nonce_state = NonceVersions::new(NonceState::Initialized(nonce::state::Data::new(
+        let nonce_state = NonceVersions::new(NonceState::Initialized(NonceData::new(
             nonce_authority.pubkey(),
             durable_nonce,
             0,
@@ -1844,7 +1811,7 @@ mod tests {
         let tx = new_sanitized_tx(&[&nonce_authority, &from], message, blockhash);
 
         let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique());
-        let nonce_state = NonceVersions::new(NonceState::Initialized(nonce::state::Data::new(
+        let nonce_state = NonceVersions::new(NonceState::Initialized(NonceData::new(
             nonce_authority.pubkey(),
             durable_nonce,
             0,
@@ -1853,17 +1820,15 @@ mod tests {
             AccountSharedData::new_data(42, &nonce_state, &system_program::id()).unwrap();
         let from_account_pre = AccountSharedData::new(4242, 0, &Pubkey::default());
 
-        let nonce = Some(NonceFull::new(
-            nonce_address,
-            nonce_account_pre.clone(),
-            Some(from_account_pre.clone()),
-        ));
-
+        let nonce = NoncePartial::new(nonce_address, nonce_account_pre.clone());
         let loaded = Ok(LoadedTransaction {
             accounts: transaction_accounts,
             program_indices: vec![],
-            nonce: nonce.clone(),
             fee_details: FeeDetails::default(),
+            rollback_accounts: RollbackAccounts::SeparateNonceAndFeePayer {
+                nonce: nonce.clone(),
+                fee_payer_account: from_account_pre.clone(),
+            },
             rent: 0,
             rent_debits: RentDebits::default(),
             loaded_accounts_data_size: 0,
@@ -1875,13 +1840,9 @@ mod tests {
         let accounts_db = AccountsDb::new_single_for_tests();
         let accounts = Accounts::new(Arc::new(accounts_db));
         let txs = vec![tx];
-        let execution_results = vec![new_execution_result(
-            Err(TransactionError::InstructionError(
-                1,
-                InstructionError::InvalidArgument,
-            )),
-            nonce.as_ref(),
-        )];
+        let execution_results = vec![new_execution_result(Err(
+            TransactionError::InstructionError(1, InstructionError::InvalidArgument),
+        ))];
         let (collected_accounts, _) = accounts.collect_accounts_to_store(
             &txs,
             &execution_results,
@@ -1923,7 +1884,7 @@ mod tests {
         let from_address = from.pubkey();
         let to_address = Pubkey::new_unique();
         let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique());
-        let nonce_state = NonceVersions::new(NonceState::Initialized(nonce::state::Data::new(
+        let nonce_state = NonceVersions::new(NonceState::Initialized(NonceData::new(
             nonce_authority.pubkey(),
             durable_nonce,
             0,
@@ -1951,7 +1912,7 @@ mod tests {
         let tx = new_sanitized_tx(&[&nonce_authority, &from], message, blockhash);
 
         let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique());
-        let nonce_state = NonceVersions::new(NonceState::Initialized(nonce::state::Data::new(
+        let nonce_state = NonceVersions::new(NonceState::Initialized(NonceData::new(
             nonce_authority.pubkey(),
             durable_nonce,
             0,
@@ -1959,17 +1920,14 @@ mod tests {
         let nonce_account_pre =
             AccountSharedData::new_data(42, &nonce_state, &system_program::id()).unwrap();
 
-        let nonce = Some(NonceFull::new(
-            nonce_address,
-            nonce_account_pre.clone(),
-            None,
-        ));
-
+        let nonce = NoncePartial::new(nonce_address, nonce_account_pre.clone());
         let loaded = Ok(LoadedTransaction {
             accounts: transaction_accounts,
             program_indices: vec![],
-            nonce: nonce.clone(),
             fee_details: FeeDetails::default(),
+            rollback_accounts: RollbackAccounts::SameNonceAndFeePayer {
+                nonce: nonce.clone(),
+            },
             rent: 0,
             rent_debits: RentDebits::default(),
             loaded_accounts_data_size: 0,
@@ -1981,13 +1939,9 @@ mod tests {
         let accounts_db = AccountsDb::new_single_for_tests();
         let accounts = Accounts::new(Arc::new(accounts_db));
         let txs = vec![tx];
-        let execution_results = vec![new_execution_result(
-            Err(TransactionError::InstructionError(
-                1,
-                InstructionError::InvalidArgument,
-            )),
-            nonce.as_ref(),
-        )];
+        let execution_results = vec![new_execution_result(Err(
+            TransactionError::InstructionError(1, InstructionError::InvalidArgument),
+        ))];
         let (collected_accounts, _) = accounts.collect_accounts_to_store(
             &txs,
             &execution_results,

+ 0 - 1
rpc/src/transaction_status_service.rs

@@ -330,7 +330,6 @@ pub(crate) mod tests {
             log_messages: None,
             inner_instructions: None,
             fee_details: FeeDetails::default(),
-            is_nonce: true,
             return_data: None,
             executed_units: 0,
             accounts_data_len_delta: 0,

+ 43 - 90
runtime/src/bank.rs

@@ -182,7 +182,6 @@ use {
             TransactionLoadedAccountsStats, TransactionResults,
         },
     },
-    solana_system_program::{get_system_account_kind, SystemAccountKind},
     solana_vote::vote_account::{VoteAccount, VoteAccountsHashMap},
     solana_vote_program::vote_state::VoteState,
     std::{
@@ -214,6 +213,7 @@ use {
     },
     solana_program_runtime::{loaded_programs::ProgramCacheForTxBatch, sysvar_cache::SysvarCache},
     solana_svm::program_loader::load_program_with_pubkey,
+    solana_system_program::{get_system_account_kind, SystemAccountKind},
 };
 
 /// params to `verify_accounts_hash`
@@ -3939,31 +3939,18 @@ impl Bank {
 
     fn filter_program_errors_and_collect_fee(
         &self,
-        txs: &[SanitizedTransaction],
         execution_results: &[TransactionExecutionResult],
     ) -> Vec<Result<()>> {
         let mut fees = 0;
 
-        let results = txs
+        let results = execution_results
             .iter()
-            .zip(execution_results)
-            .map(|(tx, execution_result)| {
-                let message = tx.message();
-                let details = match &execution_result {
-                    TransactionExecutionResult::Executed { details, .. } => details,
-                    TransactionExecutionResult::NotExecuted(err) => return Err(err.clone()),
-                };
-
-                let fee = details.fee_details.total_fee();
-                self.check_execution_status_and_charge_fee(
-                    message,
-                    &details.status,
-                    details.is_nonce,
-                    fee,
-                )?;
-
-                fees += fee;
-                Ok(())
+            .map(|execution_result| match execution_result {
+                TransactionExecutionResult::Executed { details, .. } => {
+                    fees += details.fee_details.total_fee();
+                    Ok(())
+                }
+                TransactionExecutionResult::NotExecuted(err) => Err(err.clone()),
             })
             .collect();
 
@@ -3974,31 +3961,18 @@ impl Bank {
     // Note: this function is not yet used; next PR will call it behind a feature gate
     fn filter_program_errors_and_collect_fee_details(
         &self,
-        txs: &[SanitizedTransaction],
         execution_results: &[TransactionExecutionResult],
     ) -> Vec<Result<()>> {
         let mut accumulated_fee_details = FeeDetails::default();
 
-        let results = txs
+        let results = execution_results
             .iter()
-            .zip(execution_results)
-            .map(|(tx, execution_result)| {
-                let message = tx.message();
-                let details = match &execution_result {
-                    TransactionExecutionResult::Executed { details, .. } => details,
-                    TransactionExecutionResult::NotExecuted(err) => return Err(err.clone()),
-                };
-
-                self.check_execution_status_and_charge_fee(
-                    message,
-                    &details.status,
-                    details.is_nonce,
-                    details.fee_details.total_fee(),
-                )?;
-
-                accumulated_fee_details.accumulate(&details.fee_details);
-
-                Ok(())
+            .map(|execution_result| match execution_result {
+                TransactionExecutionResult::Executed { details, .. } => {
+                    accumulated_fee_details.accumulate(&details.fee_details);
+                    Ok(())
+                }
+                TransactionExecutionResult::NotExecuted(err) => Err(err.clone()),
             })
             .collect();
 
@@ -4009,27 +3983,6 @@ impl Bank {
         results
     }
 
-    fn check_execution_status_and_charge_fee(
-        &self,
-        message: &SanitizedMessage,
-        execution_status: &transaction::Result<()>,
-        is_nonce: bool,
-        fee: u64,
-    ) -> Result<()> {
-        // In case of instruction error, even though no accounts
-        // were stored we still need to charge the payer the
-        // fee.
-        //
-        //...except nonce accounts, which already have their
-        // post-load, fee deducted, pre-execute account state
-        // stored
-        if execution_status.is_err() && !is_nonce {
-            self.withdraw(message.fee_payer(), fee)?;
-        }
-
-        Ok(())
-    }
-
     /// `committed_transactions_count` is the number of transactions out of `sanitized_txs`
     /// that was executed. Of those, `committed_transactions_count`,
     /// `committed_with_failure_result_count` is the number of executed transactions that returned
@@ -4148,9 +4101,9 @@ impl Bank {
         self.update_transaction_statuses(sanitized_txs, &execution_results);
         let fee_collection_results = if self.feature_set.is_active(&reward_full_priority_fee::id())
         {
-            self.filter_program_errors_and_collect_fee_details(sanitized_txs, &execution_results)
+            self.filter_program_errors_and_collect_fee_details(&execution_results)
         } else {
-            self.filter_program_errors_and_collect_fee(sanitized_txs, &execution_results)
+            self.filter_program_errors_and_collect_fee(&execution_results)
         };
         update_transaction_statuses_time.stop();
         timings.saturating_add_in_place(
@@ -5138,32 +5091,6 @@ impl Bank {
         );
     }
 
-    fn withdraw(&self, pubkey: &Pubkey, lamports: u64) -> Result<()> {
-        match self.get_account_with_fixed_root_no_cache(pubkey) {
-            Some(mut account) => {
-                let min_balance = match get_system_account_kind(&account) {
-                    Some(SystemAccountKind::Nonce) => self
-                        .rent_collector
-                        .rent
-                        .minimum_balance(nonce::State::size()),
-                    _ => 0,
-                };
-
-                lamports
-                    .checked_add(min_balance)
-                    .filter(|required_balance| *required_balance <= account.lamports())
-                    .ok_or(TransactionError::InsufficientFundsForFee)?;
-                account
-                    .checked_sub_lamports(lamports)
-                    .map_err(|_| TransactionError::InsufficientFundsForFee)?;
-                self.store_account(pubkey, &account);
-
-                Ok(())
-            }
-            None => Err(TransactionError::AccountNotFound),
-        }
-    }
-
     pub fn accounts(&self) -> Arc<Accounts> {
         self.rc.accounts.clone()
     }
@@ -7169,6 +7096,32 @@ impl Bank {
             .get_environments_for_epoch(effective_epoch)?;
         load_program_with_pubkey(self, &environments, pubkey, self.slot(), reload)
     }
+
+    pub fn withdraw(&self, pubkey: &Pubkey, lamports: u64) -> Result<()> {
+        match self.get_account_with_fixed_root(pubkey) {
+            Some(mut account) => {
+                let min_balance = match get_system_account_kind(&account) {
+                    Some(SystemAccountKind::Nonce) => self
+                        .rent_collector
+                        .rent
+                        .minimum_balance(nonce::State::size()),
+                    _ => 0,
+                };
+
+                lamports
+                    .checked_add(min_balance)
+                    .filter(|required_balance| *required_balance <= account.lamports())
+                    .ok_or(TransactionError::InsufficientFundsForFee)?;
+                account
+                    .checked_sub_lamports(lamports)
+                    .map_err(|_| TransactionError::InsufficientFundsForFee)?;
+                self.store_account(pubkey, &account);
+
+                Ok(())
+            }
+            None => Err(TransactionError::AccountNotFound),
+        }
+    }
 }
 
 /// Compute how much an account has changed size.  This function is useful when the data size delta

+ 22 - 148
runtime/src/bank/tests.rs

@@ -104,7 +104,7 @@ use {
         transaction_context::TransactionAccount,
     },
     solana_stake_program::stake_state::{self, StakeStateV2},
-    solana_svm::nonce_info::NonceFull,
+    solana_svm::nonce_info::NoncePartial,
     solana_vote_program::{
         vote_instruction,
         vote_state::{
@@ -231,18 +231,13 @@ fn test_race_register_tick_freeze() {
     }
 }
 
-fn new_execution_result(
-    status: Result<()>,
-    nonce: Option<&NonceFull>,
-    fee_details: FeeDetails,
-) -> TransactionExecutionResult {
+fn new_execution_result(status: Result<()>, fee_details: FeeDetails) -> TransactionExecutionResult {
     TransactionExecutionResult::Executed {
         details: TransactionExecutionDetails {
             status,
             log_messages: None,
             inner_instructions: None,
             fee_details,
-            is_nonce: nonce.is_some(),
             return_data: None,
             executed_units: 0,
             accounts_data_len_delta: 0,
@@ -2867,45 +2862,27 @@ fn test_bank_blockhash_compute_unit_fee_structure() {
 #[test]
 fn test_filter_program_errors_and_collect_fee() {
     let leader = solana_sdk::pubkey::new_rand();
-    let GenesisConfigInfo {
-        genesis_config,
-        mint_keypair,
-        ..
-    } = create_genesis_config_with_leader(100_000, &leader, 3);
+    let GenesisConfigInfo { genesis_config, .. } =
+        create_genesis_config_with_leader(100_000, &leader, 3);
     let mut bank = Bank::new_for_tests(&genesis_config);
     // this test is only for when `feature_set::reward_full_priority_fee` inactivated
     bank.deactivate_feature(&feature_set::reward_full_priority_fee::id());
 
-    let key = solana_sdk::pubkey::new_rand();
-    let tx1 = SanitizedTransaction::from_transaction_for_tests(system_transaction::transfer(
-        &mint_keypair,
-        &key,
-        2,
-        genesis_config.hash(),
-    ));
-    let tx2 = SanitizedTransaction::from_transaction_for_tests(system_transaction::transfer(
-        &mint_keypair,
-        &key,
-        5,
-        genesis_config.hash(),
-    ));
-
     let tx_fee = 42;
     let fee_details = FeeDetails::new_for_tests(tx_fee, 0, false);
     let results = vec![
-        new_execution_result(Ok(()), None, fee_details),
+        new_execution_result(Ok(()), fee_details),
         new_execution_result(
             Err(TransactionError::InstructionError(
                 1,
                 SystemError::ResultWithNegativeLamports.into(),
             )),
-            None,
             fee_details,
         ),
     ];
     let initial_balance = bank.get_balance(&leader);
 
-    let results = bank.filter_program_errors_and_collect_fee(&[tx1, tx2], &results);
+    let results = bank.filter_program_errors_and_collect_fee(&results);
     bank.freeze();
     assert_eq!(
         bank.get_balance(&leader),
@@ -2918,45 +2895,27 @@ fn test_filter_program_errors_and_collect_fee() {
 #[test]
 fn test_filter_program_errors_and_collect_priority_fee() {
     let leader = solana_sdk::pubkey::new_rand();
-    let GenesisConfigInfo {
-        genesis_config,
-        mint_keypair,
-        ..
-    } = create_genesis_config_with_leader(1000000, &leader, 3);
+    let GenesisConfigInfo { genesis_config, .. } =
+        create_genesis_config_with_leader(1000000, &leader, 3);
     let mut bank = Bank::new_for_tests(&genesis_config);
     // this test is only for when `feature_set::reward_full_priority_fee` inactivated
     bank.deactivate_feature(&feature_set::reward_full_priority_fee::id());
 
-    let key = solana_sdk::pubkey::new_rand();
-    let tx1 = SanitizedTransaction::from_transaction_for_tests(system_transaction::transfer(
-        &mint_keypair,
-        &key,
-        2,
-        genesis_config.hash(),
-    ));
-    let tx2 = SanitizedTransaction::from_transaction_for_tests(system_transaction::transfer(
-        &mint_keypair,
-        &key,
-        5,
-        genesis_config.hash(),
-    ));
-
     let priority_fee = 42;
     let fee_details: FeeDetails = FeeDetails::new_for_tests(0, priority_fee, false);
     let results = vec![
-        new_execution_result(Ok(()), None, fee_details),
+        new_execution_result(Ok(()), fee_details),
         new_execution_result(
             Err(TransactionError::InstructionError(
                 1,
                 SystemError::ResultWithNegativeLamports.into(),
             )),
-            None,
             fee_details,
         ),
     ];
     let initial_balance = bank.get_balance(&leader);
 
-    let results = bank.filter_program_errors_and_collect_fee(&[tx1, tx2], &results);
+    let results = bank.filter_program_errors_and_collect_fee(&results);
     bank.freeze();
     assert_eq!(
         bank.get_balance(&leader),
@@ -12935,33 +12894,23 @@ fn test_failed_simulation_compute_units() {
 
 #[test]
 fn test_filter_program_errors_and_collect_fee_details() {
-    // TX  | EXECUTION RESULT            | is nonce | COLLECT            | ADDITIONAL          | COLLECT
-    //     |                             |          | (TX_FEE, PRIO_FEE) | WITHDRAW FROM PAYER | RESULT
-    // ------------------------------------------------------------------------------------------------------
-    // tx1 | not executed                | n/a      | (0    , 0)         | 0                   | Original Err
-    // tx2 | executed and no error       | n/a      | (5_000, 1_000)     | 0                   | Ok
-    // tx3 | executed has error          | true     | (5_000, 1_000)     | 0                   | Ok
-    // tx4 | executed has error          | false    | (5_000, 1_000)     | 6_000               | Ok
-    // tx5 | executed error,
-    //         payer insufficient fund   | false    | (0    , 0)         | 0                   | InsufficientFundsForFee
+    // TX  | EXECUTION RESULT            | COLLECT            | COLLECT
+    //     |                             | (TX_FEE, PRIO_FEE) | RESULT
+    // ---------------------------------------------------------------------------------
+    // tx1 | not executed                | (0    , 0)         | Original Err
+    // tx2 | executed and no error       | (5_000, 1_000)     | Ok
+    // tx3 | executed has error          | (5_000, 1_000)     | Ok
     //
     let initial_payer_balance = 7_000;
-    let additional_payer_withdraw = 6_000;
     let tx_fee = 5000;
     let priority_fee = 1000;
     let tx_fee_details = FeeDetails::new_for_tests(tx_fee, priority_fee, false);
     let expected_collected_fee_details = CollectorFeeDetails {
-        transaction_fee: 3 * tx_fee,
-        priority_fee: 3 * priority_fee,
+        transaction_fee: 2 * tx_fee,
+        priority_fee: 2 * priority_fee,
     };
 
-    let expected_collect_results = vec![
-        Err(TransactionError::AccountNotFound),
-        Ok(()),
-        Ok(()),
-        Ok(()),
-        Err(TransactionError::InsufficientFundsForFee),
-    ];
+    let expected_collect_results = vec![Err(TransactionError::AccountNotFound), Ok(()), Ok(())];
 
     let GenesisConfigInfo {
         genesis_config,
@@ -12970,106 +12919,31 @@ fn test_filter_program_errors_and_collect_fee_details() {
     } = create_genesis_config_with_leader(initial_payer_balance, &Pubkey::new_unique(), 3);
     let bank = Bank::new_for_tests(&genesis_config);
 
-    let tx = SanitizedTransaction::from_transaction_for_tests(Transaction::new_signed_with_payer(
-        &[system_instruction::transfer(
-            &mint_keypair.pubkey(),
-            &Pubkey::new_unique(),
-            2,
-        )],
-        Some(&mint_keypair.pubkey()),
-        &[&mint_keypair],
-        genesis_config.hash(),
-    ));
-    let txs = vec![tx.clone(), tx.clone(), tx.clone(), tx.clone(), tx];
-
     let results = vec![
         TransactionExecutionResult::NotExecuted(TransactionError::AccountNotFound),
-        new_execution_result(Ok(()), None, tx_fee_details),
+        new_execution_result(Ok(()), tx_fee_details),
         new_execution_result(
             Err(TransactionError::InstructionError(
                 0,
                 SystemError::ResultWithNegativeLamports.into(),
             )),
-            Some(&NonceFull::new(
-                Pubkey::new_unique(),
-                AccountSharedData::default(),
-                None,
-            )),
             tx_fee_details,
         ),
-        new_execution_result(
-            Err(TransactionError::InstructionError(
-                0,
-                SystemError::ResultWithNegativeLamports.into(),
-            )),
-            None,
-            tx_fee_details,
-        ),
-        new_execution_result(Err(TransactionError::AccountNotFound), None, tx_fee_details),
     ];
 
-    let results = bank.filter_program_errors_and_collect_fee_details(&txs, &results);
+    let results = bank.filter_program_errors_and_collect_fee_details(&results);
 
     assert_eq!(
         expected_collected_fee_details,
         *bank.collector_fee_details.read().unwrap()
     );
     assert_eq!(
-        initial_payer_balance - additional_payer_withdraw,
+        initial_payer_balance,
         bank.get_balance(&mint_keypair.pubkey())
     );
     assert_eq!(expected_collect_results, results);
 }
 
-#[test]
-fn test_check_execution_status_and_charge_fee() {
-    let fee = 5000;
-    let initial_balance = fee - 1000;
-    let tx_error =
-        TransactionError::InstructionError(0, InstructionError::MissingRequiredSignature);
-    let GenesisConfigInfo {
-        mut genesis_config,
-        mint_keypair,
-        ..
-    } = create_genesis_config_with_leader(initial_balance, &Pubkey::new_unique(), 3);
-    genesis_config.fee_rate_governor = FeeRateGovernor::new(5000, 0);
-    let bank = Bank::new_for_tests(&genesis_config);
-    let message = new_sanitized_message(Message::new(
-        &[system_instruction::transfer(
-            &mint_keypair.pubkey(),
-            &Pubkey::new_unique(),
-            1,
-        )],
-        Some(&mint_keypair.pubkey()),
-    ));
-
-    [Ok(()), Err(tx_error)]
-        .iter()
-        .flat_map(|result| [true, false].iter().map(move |is_nonce| (result, is_nonce)))
-        .for_each(|(result, is_nonce)| {
-            if result.is_err() && !is_nonce {
-                assert_eq!(
-                    Err(TransactionError::InsufficientFundsForFee),
-                    bank.check_execution_status_and_charge_fee(&message, result, *is_nonce, fee)
-                );
-                assert_eq!(initial_balance, bank.get_balance(&mint_keypair.pubkey()));
-
-                let small_fee = 1;
-                assert!(bank
-                    .check_execution_status_and_charge_fee(&message, result, *is_nonce, small_fee)
-                    .is_ok());
-                assert_eq!(
-                    initial_balance - small_fee,
-                    bank.get_balance(&mint_keypair.pubkey())
-                );
-            } else {
-                assert!(bank
-                    .check_execution_status_and_charge_fee(&message, result, *is_nonce, fee)
-                    .is_ok());
-                assert_eq!(initial_balance, bank.get_balance(&mint_keypair.pubkey()));
-            }
-        });
-}
 #[test]
 fn test_deploy_last_epoch_slot() {
     solana_logger::setup();

+ 3 - 4
svm/doc/spec.md

@@ -277,11 +277,10 @@ Steps of `load_and_execute_sanitized_transactions`
             - Validate the fee payer and the loaded accounts
             - Validate the programs accounts that have been loaded and checks if they are builtin programs.
             - Return `struct LoadedTransaction` containing the accounts (pubkey and data),
-              indices to the excutabe accounts in `TransactionContext` (or `InstructionContext`),
+              indices to the executable accounts in `TransactionContext` (or `InstructionContext`),
               the transaction rent, and the `struct RentDebit`.
-            - Generate a `NonceFull` struct (holds fee subtracted nonce info) when possible, `None` otherwise.
-    - Returns `TransactionLoadedResult`, a tuple containing the `LoadTransaction` we obtained from `loaded_transaction_accounts`,
-      and a `Option<NonceFull>`.
+            - Generate a `RollbackAccounts` struct which holds fee-subtracted fee payer account and pre-execution nonce state used for rolling back account state on execution failure.
+    - Returns `TransactionLoadedResult`, containing the `LoadTransaction` we obtained from `loaded_transaction_accounts`
 
 3. Execute each loaded transactions
    1. Compute the sum of transaction accounts' balances. This sum is

+ 97 - 56
svm/src/account_loader.rs

@@ -1,8 +1,7 @@
 use {
     crate::{
-        account_overrides::AccountOverrides,
-        account_rent_state::RentState,
-        nonce_info::{NonceFull, NoncePartial},
+        account_overrides::AccountOverrides, account_rent_state::RentState,
+        nonce_info::NoncePartial, rollback_accounts::RollbackAccounts,
         transaction_error_metrics::TransactionErrorMetrics,
         transaction_processing_callback::TransactionProcessingCallback,
     },
@@ -45,7 +44,7 @@ pub struct CheckedTransactionDetails {
 #[derive(PartialEq, Eq, Debug, Clone)]
 #[cfg_attr(feature = "dev-context-only-utils", derive(Default))]
 pub struct ValidatedTransactionDetails {
-    pub nonce: Option<NonceFull>,
+    pub rollback_accounts: RollbackAccounts,
     pub fee_details: FeeDetails,
     pub fee_payer_account: AccountSharedData,
     pub fee_payer_rent_debit: u64,
@@ -55,19 +54,13 @@ pub struct ValidatedTransactionDetails {
 pub struct LoadedTransaction {
     pub accounts: Vec<TransactionAccount>,
     pub program_indices: TransactionProgramIndices,
-    pub nonce: Option<NonceFull>,
     pub fee_details: FeeDetails,
+    pub rollback_accounts: RollbackAccounts,
     pub rent: TransactionRent,
     pub rent_debits: RentDebits,
     pub loaded_accounts_data_size: usize,
 }
 
-impl LoadedTransaction {
-    pub fn fee_payer_account(&self) -> Option<&TransactionAccount> {
-        self.accounts.first()
-    }
-}
-
 /// Collect rent from an account if rent is still enabled and regardless of
 /// whether rent is enabled, set the rent epoch to u64::MAX if the account is
 /// rent exempt.
@@ -363,8 +356,8 @@ fn load_transaction_accounts<CB: TransactionProcessingCallback>(
     Ok(LoadedTransaction {
         accounts,
         program_indices,
-        nonce: tx_details.nonce,
         fee_details: tx_details.fee_details,
+        rollback_accounts: tx_details.rollback_accounts,
         rent: tx_rent,
         rent_debits,
         loaded_accounts_data_size: accumulated_accounts_data_size,
@@ -439,7 +432,7 @@ mod tests {
     use {
         super::*,
         crate::{
-            nonce_info::NonceFull, transaction_account_state_info::TransactionAccountStateInfo,
+            transaction_account_state_info::TransactionAccountStateInfo,
             transaction_processing_callback::TransactionProcessingCallback,
         },
         nonce::state::Versions as NonceVersions,
@@ -1114,6 +1107,70 @@ mod tests {
         assert_eq!(shared_data, expected);
     }
 
+    #[test]
+    fn test_load_transaction_accounts_fee_payer() {
+        let fee_payer_address = Pubkey::new_unique();
+        let message = Message {
+            account_keys: vec![fee_payer_address],
+            header: MessageHeader::default(),
+            instructions: vec![],
+            recent_blockhash: Hash::default(),
+        };
+
+        let sanitized_message = new_unchecked_sanitized_message(message);
+        let mut mock_bank = TestCallbacks::default();
+
+        let fee_payer_balance = 200;
+        let mut fee_payer_account_data = AccountSharedData::default();
+        fee_payer_account_data.set_lamports(fee_payer_balance);
+        mock_bank
+            .accounts_map
+            .insert(fee_payer_address, fee_payer_account_data.clone());
+        let fee_payer_rent_debit = 42;
+
+        let mut error_metrics = TransactionErrorMetrics::default();
+        let loaded_programs = ProgramCacheForTxBatch::default();
+
+        let sanitized_transaction = SanitizedTransaction::new_for_tests(
+            sanitized_message,
+            vec![Signature::new_unique()],
+            false,
+        );
+        let result = load_transaction_accounts(
+            &mock_bank,
+            sanitized_transaction.message(),
+            ValidatedTransactionDetails {
+                rollback_accounts: RollbackAccounts::default(),
+                fee_details: FeeDetails::default(),
+                fee_payer_account: fee_payer_account_data.clone(),
+                fee_payer_rent_debit,
+            },
+            &mut error_metrics,
+            None,
+            &FeatureSet::default(),
+            &RentCollector::default(),
+            &loaded_programs,
+        );
+
+        let expected_rent_debits = {
+            let mut rent_debits = RentDebits::default();
+            rent_debits.insert(&fee_payer_address, fee_payer_rent_debit, fee_payer_balance);
+            rent_debits
+        };
+        assert_eq!(
+            result.unwrap(),
+            LoadedTransaction {
+                accounts: vec![(fee_payer_address, fee_payer_account_data),],
+                program_indices: vec![],
+                fee_details: FeeDetails::default(),
+                rollback_accounts: RollbackAccounts::default(),
+                rent: fee_payer_rent_debit,
+                rent_debits: expected_rent_debits,
+                loaded_accounts_data_size: 0,
+            }
+        );
+    }
+
     #[test]
     fn test_load_transaction_accounts_native_loader() {
         let key1 = Keypair::new();
@@ -1147,14 +1204,13 @@ mod tests {
             vec![Signature::new_unique()],
             false,
         );
-        let fee_details = FeeDetails::new_for_tests(32, 0, false);
         let result = load_transaction_accounts(
             &mock_bank,
             sanitized_transaction.message(),
             ValidatedTransactionDetails {
-                nonce: None,
-                fee_details,
-                fee_payer_account: fee_payer_account_data,
+                rollback_accounts: RollbackAccounts::default(),
+                fee_details: FeeDetails::default(),
+                fee_payer_account: fee_payer_account_data.clone(),
                 fee_payer_rent_debit: 0,
             },
             &mut error_metrics,
@@ -1168,18 +1224,15 @@ mod tests {
             result.unwrap(),
             LoadedTransaction {
                 accounts: vec![
-                    (
-                        key1.pubkey(),
-                        mock_bank.accounts_map[&key1.pubkey()].clone()
-                    ),
+                    (key1.pubkey(), fee_payer_account_data),
                     (
                         native_loader::id(),
                         mock_bank.accounts_map[&native_loader::id()].clone()
                     )
                 ],
                 program_indices: vec![vec![]],
-                nonce: None,
-                fee_details,
+                fee_details: FeeDetails::default(),
+                rollback_accounts: RollbackAccounts::default(),
                 rent: 0,
                 rent_debits: RentDebits::default(),
                 loaded_accounts_data_size: 0,
@@ -1359,14 +1412,13 @@ mod tests {
             vec![Signature::new_unique()],
             false,
         );
-        let fee_details = FeeDetails::new_for_tests(32, 0, false);
         let result = load_transaction_accounts(
             &mock_bank,
             sanitized_transaction.message(),
             ValidatedTransactionDetails {
-                nonce: None,
-                fee_details,
-                fee_payer_account: fee_payer_account_data,
+                rollback_accounts: RollbackAccounts::default(),
+                fee_details: FeeDetails::default(),
+                fee_payer_account: fee_payer_account_data.clone(),
                 fee_payer_rent_debit: 0,
             },
             &mut error_metrics,
@@ -1380,17 +1432,14 @@ mod tests {
             result.unwrap(),
             LoadedTransaction {
                 accounts: vec![
-                    (
-                        key2.pubkey(),
-                        mock_bank.accounts_map[&key2.pubkey()].clone()
-                    ),
+                    (key2.pubkey(), fee_payer_account_data),
                     (
                         key1.pubkey(),
                         mock_bank.accounts_map[&key1.pubkey()].clone()
                     ),
                 ],
-                nonce: None,
-                fee_details,
+                fee_details: FeeDetails::default(),
+                rollback_accounts: RollbackAccounts::default(),
                 program_indices: vec![vec![1]],
                 rent: 0,
                 rent_debits: RentDebits::default(),
@@ -1545,14 +1594,13 @@ mod tests {
             vec![Signature::new_unique()],
             false,
         );
-        let fee_details = FeeDetails::new_for_tests(32, 0, false);
         let result = load_transaction_accounts(
             &mock_bank,
             sanitized_transaction.message(),
             ValidatedTransactionDetails {
-                nonce: None,
-                fee_details,
-                fee_payer_account: fee_payer_account_data,
+                rollback_accounts: RollbackAccounts::default(),
+                fee_details: FeeDetails::default(),
+                fee_payer_account: fee_payer_account_data.clone(),
                 fee_payer_rent_debit: 0,
             },
             &mut error_metrics,
@@ -1566,10 +1614,7 @@ mod tests {
             result.unwrap(),
             LoadedTransaction {
                 accounts: vec![
-                    (
-                        key2.pubkey(),
-                        mock_bank.accounts_map[&key2.pubkey()].clone()
-                    ),
+                    (key2.pubkey(), fee_payer_account_data),
                     (
                         key1.pubkey(),
                         mock_bank.accounts_map[&key1.pubkey()].clone()
@@ -1580,8 +1625,8 @@ mod tests {
                     ),
                 ],
                 program_indices: vec![vec![2, 1]],
-                nonce: None,
-                fee_details,
+                fee_details: FeeDetails::default(),
+                rollback_accounts: RollbackAccounts::default(),
                 rent: 0,
                 rent_debits: RentDebits::default(),
                 loaded_accounts_data_size: 0,
@@ -1640,14 +1685,13 @@ mod tests {
             vec![Signature::new_unique()],
             false,
         );
-        let fee_details = FeeDetails::new_for_tests(32, 0, false);
         let result = load_transaction_accounts(
             &mock_bank,
             sanitized_transaction.message(),
             ValidatedTransactionDetails {
-                nonce: None,
-                fee_details,
-                fee_payer_account: fee_payer_account_data,
+                rollback_accounts: RollbackAccounts::default(),
+                fee_details: FeeDetails::default(),
+                fee_payer_account: fee_payer_account_data.clone(),
                 fee_payer_rent_debit: 0,
             },
             &mut error_metrics,
@@ -1663,10 +1707,7 @@ mod tests {
             result.unwrap(),
             LoadedTransaction {
                 accounts: vec![
-                    (
-                        key2.pubkey(),
-                        mock_bank.accounts_map[&key2.pubkey()].clone()
-                    ),
+                    (key2.pubkey(), fee_payer_account_data),
                     (
                         key1.pubkey(),
                         mock_bank.accounts_map[&key1.pubkey()].clone()
@@ -1678,8 +1719,8 @@ mod tests {
                     ),
                 ],
                 program_indices: vec![vec![3, 1], vec![3, 1]],
-                nonce: None,
-                fee_details,
+                fee_details: FeeDetails::default(),
+                rollback_accounts: RollbackAccounts::default(),
                 rent: 0,
                 rent_debits: RentDebits::default(),
                 loaded_accounts_data_size: 0,
@@ -1800,7 +1841,7 @@ mod tests {
             false,
         );
         let validation_result = Ok(ValidatedTransactionDetails {
-            nonce: None,
+            rollback_accounts: RollbackAccounts::default(),
             fee_details: FeeDetails::default(),
             fee_payer_account: fee_payer_account_data,
             fee_payer_rent_debit: 0,
@@ -1841,8 +1882,8 @@ mod tests {
                     ),
                 ],
                 program_indices: vec![vec![3, 1], vec![3, 1]],
-                nonce: None,
                 fee_details: FeeDetails::default(),
+                rollback_accounts: RollbackAccounts::default(),
                 rent: 0,
                 rent_debits: RentDebits::default(),
                 loaded_accounts_data_size: 0,
@@ -1875,7 +1916,7 @@ mod tests {
         );
 
         let validation_result = Ok(ValidatedTransactionDetails {
-            nonce: Some(NonceFull::default()),
+            rollback_accounts: RollbackAccounts::default(),
             fee_details: FeeDetails::default(),
             fee_payer_account: AccountSharedData::default(),
             fee_payer_rent_debit: 0,

+ 1 - 0
svm/src/lib.rs

@@ -7,6 +7,7 @@ pub mod account_rent_state;
 pub mod message_processor;
 pub mod nonce_info;
 pub mod program_loader;
+pub mod rollback_accounts;
 pub mod runtime_config;
 pub mod transaction_account_state_info;
 pub mod transaction_error_metrics;

+ 10 - 150
svm/src/nonce_info.rs

@@ -1,8 +1,4 @@
-use solana_sdk::{
-    account::{AccountSharedData, ReadableAccount, WritableAccount},
-    nonce_account,
-    pubkey::Pubkey,
-};
+use solana_sdk::{account::AccountSharedData, nonce_account, pubkey::Pubkey};
 
 pub trait NonceInfo {
     fn address(&self) -> &Pubkey;
@@ -39,107 +35,27 @@ impl NonceInfo for NoncePartial {
     }
 }
 
-/// Holds fee subtracted nonce info
-#[derive(Clone, Debug, Default, PartialEq, Eq)]
-pub struct NonceFull {
-    address: Pubkey,
-    account: AccountSharedData,
-    fee_payer_account: Option<AccountSharedData>,
-}
-
-impl NonceFull {
-    pub fn new(
-        address: Pubkey,
-        account: AccountSharedData,
-        fee_payer_account: Option<AccountSharedData>,
-    ) -> Self {
-        Self {
-            address,
-            account,
-            fee_payer_account,
-        }
-    }
-
-    pub fn from_partial(
-        partial: NoncePartial,
-        fee_payer_address: &Pubkey,
-        mut fee_payer_account: AccountSharedData,
-        fee_payer_rent_debit: u64,
-    ) -> Self {
-        fee_payer_account.set_lamports(
-            fee_payer_account
-                .lamports()
-                .saturating_add(fee_payer_rent_debit),
-        );
-
-        let NoncePartial {
-            address: nonce_address,
-            account: nonce_account,
-        } = partial;
-
-        if *fee_payer_address == nonce_address {
-            Self::new(nonce_address, fee_payer_account, None)
-        } else {
-            Self::new(nonce_address, nonce_account, Some(fee_payer_account))
-        }
-    }
-}
-
-impl NonceInfo for NonceFull {
-    fn address(&self) -> &Pubkey {
-        &self.address
-    }
-    fn account(&self) -> &AccountSharedData {
-        &self.account
-    }
-    fn lamports_per_signature(&self) -> Option<u64> {
-        nonce_account::lamports_per_signature_of(&self.account)
-    }
-    fn fee_payer_account(&self) -> Option<&AccountSharedData> {
-        self.fee_payer_account.as_ref()
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use {
         super::*,
         solana_sdk::{
             hash::Hash,
-            instruction::Instruction,
-            message::{Message, SanitizedMessage},
-            nonce::{self, state::DurableNonce},
-            reserved_account_keys::ReservedAccountKeys,
-            signature::{keypair_from_seed, Signer},
-            system_instruction, system_program,
+            nonce::state::{
+                Data as NonceData, DurableNonce, State as NonceState, Versions as NonceVersions,
+            },
+            system_program,
         },
     };
 
-    fn new_sanitized_message(
-        instructions: &[Instruction],
-        payer: Option<&Pubkey>,
-    ) -> SanitizedMessage {
-        SanitizedMessage::try_from_legacy_message(
-            Message::new(instructions, payer),
-            &ReservedAccountKeys::empty_key_set(),
-        )
-        .unwrap()
-    }
-
     #[test]
     fn test_nonce_info() {
-        let lamports_per_signature = 42;
-
-        let nonce_authority = keypair_from_seed(&[0; 32]).unwrap();
-        let nonce_address = nonce_authority.pubkey();
-        let from = keypair_from_seed(&[1; 32]).unwrap();
-        let from_address = from.pubkey();
-        let to_address = Pubkey::new_unique();
-
+        let nonce_address = Pubkey::new_unique();
         let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique());
+        let lamports_per_signature = 42;
         let nonce_account = AccountSharedData::new_data(
             43,
-            &nonce::state::Versions::new(nonce::State::Initialized(nonce::state::Data::new(
+            &NonceVersions::new(NonceState::Initialized(NonceData::new(
                 Pubkey::default(),
                 durable_nonce,
                 lamports_per_signature,
@@ -147,71 +63,15 @@ mod tests {
             &system_program::id(),
         )
         .unwrap();
-        let from_account = AccountSharedData::new(44, 0, &Pubkey::default());
-
-        const TEST_RENT_DEBIT: u64 = 1;
-        let rent_collected_nonce_account = {
-            let mut account = nonce_account.clone();
-            account.set_lamports(nonce_account.lamports() - TEST_RENT_DEBIT);
-            account
-        };
-        let rent_collected_from_account = {
-            let mut account = from_account.clone();
-            account.set_lamports(from_account.lamports() - TEST_RENT_DEBIT);
-            account
-        };
-
-        let instructions = vec![
-            system_instruction::advance_nonce_account(&nonce_address, &nonce_authority.pubkey()),
-            system_instruction::transfer(&from_address, &to_address, 42),
-        ];
 
         // NoncePartial create + NonceInfo impl
-        let partial = NoncePartial::new(nonce_address, rent_collected_nonce_account.clone());
+        let partial = NoncePartial::new(nonce_address, nonce_account.clone());
         assert_eq!(*partial.address(), nonce_address);
-        assert_eq!(*partial.account(), rent_collected_nonce_account);
+        assert_eq!(*partial.account(), nonce_account);
         assert_eq!(
             partial.lamports_per_signature(),
             Some(lamports_per_signature)
         );
         assert_eq!(partial.fee_payer_account(), None);
-
-        // NonceFull create + NonceInfo impl
-        {
-            let message = new_sanitized_message(&instructions, Some(&from_address));
-            let fee_payer_address = message.account_keys().get(0).unwrap();
-            let fee_payer_account = rent_collected_from_account.clone();
-            let full = NonceFull::from_partial(
-                partial.clone(),
-                fee_payer_address,
-                fee_payer_account,
-                TEST_RENT_DEBIT,
-            );
-            assert_eq!(*full.address(), nonce_address);
-            assert_eq!(*full.account(), rent_collected_nonce_account);
-            assert_eq!(full.lamports_per_signature(), Some(lamports_per_signature));
-            assert_eq!(
-                full.fee_payer_account(),
-                Some(&from_account),
-                "rent debit should be refunded in captured fee account"
-            );
-        }
-
-        // Nonce account is fee-payer
-        {
-            let message = new_sanitized_message(&instructions, Some(&nonce_address));
-            let fee_payer_address = message.account_keys().get(0).unwrap();
-            let fee_payer_account = rent_collected_nonce_account;
-            let full = NonceFull::from_partial(
-                partial,
-                fee_payer_address,
-                fee_payer_account,
-                TEST_RENT_DEBIT,
-            );
-            assert_eq!(*full.address(), nonce_address);
-            assert_eq!(*full.account(), nonce_account);
-            assert_eq!(full.lamports_per_signature(), Some(lamports_per_signature));
-            assert_eq!(full.fee_payer_account(), None);
-        }
     }
 }

+ 229 - 0
svm/src/rollback_accounts.rs

@@ -0,0 +1,229 @@
+use {
+    crate::nonce_info::{NonceInfo, NoncePartial},
+    solana_sdk::{
+        account::{AccountSharedData, ReadableAccount, WritableAccount},
+        clock::Epoch,
+        pubkey::Pubkey,
+    },
+};
+
+/// Captured account state used to rollback account state for nonce and fee
+/// payer accounts after a failed executed transaction.
+#[derive(PartialEq, Eq, Debug, Clone)]
+pub enum RollbackAccounts {
+    FeePayerOnly {
+        fee_payer_account: AccountSharedData,
+    },
+    SameNonceAndFeePayer {
+        nonce: NoncePartial,
+    },
+    SeparateNonceAndFeePayer {
+        nonce: NoncePartial,
+        fee_payer_account: AccountSharedData,
+    },
+}
+
+#[cfg(feature = "dev-context-only-utils")]
+impl Default for RollbackAccounts {
+    fn default() -> Self {
+        Self::FeePayerOnly {
+            fee_payer_account: AccountSharedData::default(),
+        }
+    }
+}
+
+impl RollbackAccounts {
+    pub fn new(
+        nonce: Option<NoncePartial>,
+        fee_payer_address: Pubkey,
+        mut fee_payer_account: AccountSharedData,
+        fee_payer_rent_debit: u64,
+        fee_payer_loaded_rent_epoch: Epoch,
+    ) -> Self {
+        // When the fee payer account is rolled back due to transaction failure,
+        // rent should not be charged so credit the previously debited rent
+        // amount.
+        fee_payer_account.set_lamports(
+            fee_payer_account
+                .lamports()
+                .saturating_add(fee_payer_rent_debit),
+        );
+
+        if let Some(nonce) = nonce {
+            if &fee_payer_address == nonce.address() {
+                RollbackAccounts::SameNonceAndFeePayer {
+                    nonce: NoncePartial::new(fee_payer_address, fee_payer_account),
+                }
+            } else {
+                RollbackAccounts::SeparateNonceAndFeePayer {
+                    nonce,
+                    fee_payer_account,
+                }
+            }
+        } else {
+            // When rolling back failed transactions which don't use nonces, the
+            // runtime should not update the fee payer's rent epoch so reset the
+            // rollback fee payer acocunt's rent epoch to its originally loaded
+            // rent epoch value. In the future, a feature gate could be used to
+            // alter this behavior such that rent epoch updates are handled the
+            // same for both nonce and non-nonce failed transactions.
+            fee_payer_account.set_rent_epoch(fee_payer_loaded_rent_epoch);
+            RollbackAccounts::FeePayerOnly { fee_payer_account }
+        }
+    }
+
+    pub fn nonce(&self) -> Option<&NoncePartial> {
+        match self {
+            Self::FeePayerOnly { .. } => None,
+            Self::SameNonceAndFeePayer { nonce } | Self::SeparateNonceAndFeePayer { nonce, .. } => {
+                Some(nonce)
+            }
+        }
+    }
+
+    pub fn fee_payer_account(&self) -> &AccountSharedData {
+        match self {
+            Self::FeePayerOnly { fee_payer_account }
+            | Self::SeparateNonceAndFeePayer {
+                fee_payer_account, ..
+            } => fee_payer_account,
+            Self::SameNonceAndFeePayer { nonce } => nonce.account(),
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use {
+        super::*,
+        solana_sdk::{
+            account::{ReadableAccount, WritableAccount},
+            hash::Hash,
+            nonce::state::{
+                Data as NonceData, DurableNonce, State as NonceState, Versions as NonceVersions,
+            },
+            system_program,
+        },
+    };
+
+    #[test]
+    fn test_new_fee_payer_only() {
+        let fee_payer_address = Pubkey::new_unique();
+        let fee_payer_account = AccountSharedData::new(100, 0, &Pubkey::default());
+        let fee_payer_rent_epoch = fee_payer_account.rent_epoch();
+
+        const TEST_RENT_DEBIT: u64 = 1;
+        let rent_collected_fee_payer_account = {
+            let mut account = fee_payer_account.clone();
+            account.set_lamports(fee_payer_account.lamports() - TEST_RENT_DEBIT);
+            account.set_rent_epoch(fee_payer_rent_epoch + 1);
+            account
+        };
+
+        let rollback_accounts = RollbackAccounts::new(
+            None,
+            fee_payer_address,
+            rent_collected_fee_payer_account,
+            TEST_RENT_DEBIT,
+            fee_payer_rent_epoch,
+        );
+
+        let expected_fee_payer_account = fee_payer_account;
+        match rollback_accounts {
+            RollbackAccounts::FeePayerOnly { fee_payer_account } => {
+                assert_eq!(expected_fee_payer_account, fee_payer_account);
+            }
+            _ => panic!("Expected FeePayerOnly variant"),
+        }
+    }
+
+    #[test]
+    fn test_new_same_nonce_and_fee_payer() {
+        let nonce_address = Pubkey::new_unique();
+        let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique());
+        let lamports_per_signature = 42;
+        let nonce_account = AccountSharedData::new_data(
+            43,
+            &NonceVersions::new(NonceState::Initialized(NonceData::new(
+                Pubkey::default(),
+                durable_nonce,
+                lamports_per_signature,
+            ))),
+            &system_program::id(),
+        )
+        .unwrap();
+
+        const TEST_RENT_DEBIT: u64 = 1;
+        let rent_collected_nonce_account = {
+            let mut account = nonce_account.clone();
+            account.set_lamports(nonce_account.lamports() - TEST_RENT_DEBIT);
+            account
+        };
+
+        let nonce = NoncePartial::new(nonce_address, rent_collected_nonce_account.clone());
+        let rollback_accounts = RollbackAccounts::new(
+            Some(nonce),
+            nonce_address,
+            rent_collected_nonce_account,
+            TEST_RENT_DEBIT,
+            u64::MAX, // ignored
+        );
+
+        match rollback_accounts {
+            RollbackAccounts::SameNonceAndFeePayer { nonce } => {
+                assert_eq!(nonce.address(), &nonce_address);
+                assert_eq!(nonce.account(), &nonce_account);
+            }
+            _ => panic!("Expected SameNonceAndFeePayer variant"),
+        }
+    }
+
+    #[test]
+    fn test_separate_nonce_and_fee_payer() {
+        let nonce_address = Pubkey::new_unique();
+        let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique());
+        let lamports_per_signature = 42;
+        let nonce_account = AccountSharedData::new_data(
+            43,
+            &NonceVersions::new(NonceState::Initialized(NonceData::new(
+                Pubkey::default(),
+                durable_nonce,
+                lamports_per_signature,
+            ))),
+            &system_program::id(),
+        )
+        .unwrap();
+
+        let fee_payer_address = Pubkey::new_unique();
+        let fee_payer_account = AccountSharedData::new(44, 0, &Pubkey::default());
+
+        const TEST_RENT_DEBIT: u64 = 1;
+        let rent_collected_fee_payer_account = {
+            let mut account = fee_payer_account.clone();
+            account.set_lamports(fee_payer_account.lamports() - TEST_RENT_DEBIT);
+            account
+        };
+
+        let nonce = NoncePartial::new(nonce_address, nonce_account.clone());
+        let rollback_accounts = RollbackAccounts::new(
+            Some(nonce),
+            fee_payer_address,
+            rent_collected_fee_payer_account.clone(),
+            TEST_RENT_DEBIT,
+            u64::MAX, // ignored
+        );
+
+        let expected_fee_payer_account = fee_payer_account;
+        match rollback_accounts {
+            RollbackAccounts::SeparateNonceAndFeePayer {
+                nonce,
+                fee_payer_account,
+            } => {
+                assert_eq!(nonce.address(), &nonce_address);
+                assert_eq!(nonce.account(), &nonce_account);
+                assert_eq!(expected_fee_payer_account, fee_payer_account);
+            }
+            _ => panic!("Expected SeparateNonceAndFeePayer variant"),
+        }
+    }
+}

+ 132 - 73
svm/src/transaction_processor.rs

@@ -9,8 +9,8 @@ use {
         },
         account_overrides::AccountOverrides,
         message_processor::MessageProcessor,
-        nonce_info::NonceFull,
         program_loader::load_program_with_pubkey,
+        rollback_accounts::RollbackAccounts,
         transaction_account_state_info::TransactionAccountStateInfo,
         transaction_error_metrics::TransactionErrorMetrics,
         transaction_processing_callback::TransactionProcessingCallback,
@@ -42,7 +42,7 @@ use {
             include_loaded_accounts_data_size_in_fee_calculation,
             remove_rounding_in_fee_calculation, FeatureSet,
         },
-        fee::{FeeDetails, FeeStructure},
+        fee::FeeStructure,
         hash::Hash,
         inner_instruction::{InnerInstruction, InnerInstructionsList},
         instruction::{CompiledInstruction, TRANSACTION_LEVEL_STACK_HEIGHT},
@@ -409,42 +409,18 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
             .iter()
             .zip(check_results)
             .map(|(sanitized_tx, check_result)| {
-                check_result.and_then(
-                    |CheckedTransactionDetails {
-                         nonce,
-                         lamports_per_signature,
-                     }| {
-                        let message = sanitized_tx.borrow().message();
-                        let (fee_details, fee_payer_account, fee_payer_rent_debit) = self
-                            .validate_transaction_fee_payer(
-                                callbacks,
-                                message,
-                                feature_set,
-                                fee_structure,
-                                lamports_per_signature,
-                                rent_collector,
-                                error_counters,
-                            )?;
-
-                        // Update nonce with fee-subtracted accounts
-                        let fee_payer_address = message.fee_payer();
-                        let nonce = nonce.map(|nonce| {
-                            NonceFull::from_partial(
-                                nonce,
-                                fee_payer_address,
-                                fee_payer_account.clone(),
-                                fee_payer_rent_debit,
-                            )
-                        });
-
-                        Ok(ValidatedTransactionDetails {
-                            nonce,
-                            fee_details,
-                            fee_payer_account,
-                            fee_payer_rent_debit,
-                        })
-                    },
-                )
+                check_result.and_then(|checked_details| {
+                    let message = sanitized_tx.borrow().message();
+                    self.validate_transaction_fee_payer(
+                        callbacks,
+                        message,
+                        checked_details,
+                        feature_set,
+                        fee_structure,
+                        rent_collector,
+                        error_counters,
+                    )
+                })
             })
             .collect()
     }
@@ -456,12 +432,12 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
         &self,
         callbacks: &CB,
         message: &SanitizedMessage,
+        checked_details: CheckedTransactionDetails,
         feature_set: &FeatureSet,
         fee_structure: &FeeStructure,
-        lamports_per_signature: u64,
         rent_collector: &RentCollector,
         error_counters: &mut TransactionErrorMetrics,
-    ) -> transaction::Result<(FeeDetails, AccountSharedData, u64)> {
+    ) -> transaction::Result<ValidatedTransactionDetails> {
         let fee_payer_address = message.fee_payer();
         let Some(mut fee_payer_account) = callbacks.get_account_shared_data(fee_payer_address)
         else {
@@ -469,6 +445,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
             return Err(TransactionError::AccountNotFound);
         };
 
+        let fee_payer_loaded_rent_epoch = fee_payer_account.rent_epoch();
         let fee_payer_rent_debit = collect_rent_from_account(
             feature_set,
             rent_collector,
@@ -477,6 +454,11 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
         )
         .rent_amount;
 
+        let CheckedTransactionDetails {
+            nonce,
+            lamports_per_signature,
+        } = checked_details;
+
         let fee_details = fee_structure.calculate_fee_details(
             message,
             lamports_per_signature,
@@ -497,7 +479,22 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
             fee_details.total_fee(),
         )?;
 
-        Ok((fee_details, fee_payer_account, fee_payer_rent_debit))
+        // Capture fee-subtracted fee payer account and original nonce account state
+        // to rollback to if transaction execution fails.
+        let rollback_accounts = RollbackAccounts::new(
+            nonce,
+            *fee_payer_address,
+            fee_payer_account.clone(),
+            fee_payer_rent_debit,
+            fee_payer_loaded_rent_epoch,
+        );
+
+        Ok(ValidatedTransactionDetails {
+            fee_details,
+            fee_payer_account,
+            fee_payer_rent_debit,
+            rollback_accounts,
+        })
     }
 
     /// Returns a map from executable program accounts (all accounts owned by any loader)
@@ -884,7 +881,6 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
                 log_messages,
                 inner_instructions,
                 fee_details: loaded_transaction.fee_details,
-                is_nonce: loaded_transaction.nonce.is_some(),
                 return_data,
                 executed_units,
                 accounts_data_len_delta,
@@ -989,7 +985,10 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
 mod tests {
     use {
         super::*,
-        crate::account_loader::ValidatedTransactionDetails,
+        crate::{
+            account_loader::ValidatedTransactionDetails, nonce_info::NoncePartial,
+            rollback_accounts::RollbackAccounts,
+        },
         solana_program_runtime::loaded_programs::{BlockRelation, ProgramCacheEntryType},
         solana_sdk::{
             account::{create_account_shared_data_for_test, WritableAccount},
@@ -1148,8 +1147,8 @@ mod tests {
         let mut loaded_transaction = LoadedTransaction {
             accounts: vec![(Pubkey::new_unique(), AccountSharedData::default())],
             program_indices: vec![vec![0]],
-            nonce: None,
             fee_details: FeeDetails::default(),
+            rollback_accounts: RollbackAccounts::default(),
             rent: 0,
             rent_debits: RentDebits::default(),
             loaded_accounts_data_size: 32,
@@ -1275,8 +1274,8 @@ mod tests {
                 (key2, AccountSharedData::default()),
             ],
             program_indices: vec![vec![0]],
-            nonce: None,
             fee_details: FeeDetails::default(),
+            rollback_accounts: RollbackAccounts::default(),
             rent: 0,
             rent_debits: RentDebits::default(),
             loaded_accounts_data_size: 0,
@@ -1956,7 +1955,11 @@ mod tests {
             &Hash::new_unique(),
         ));
         let fee_payer_address = message.fee_payer();
-        let rent_collector = RentCollector::default();
+        let current_epoch = 42;
+        let rent_collector = RentCollector {
+            epoch: current_epoch,
+            ..RentCollector::default()
+        };
         let min_balance = rent_collector.rent.minimum_balance(nonce::State::size());
         let transaction_fee = lamports_per_signature;
         let priority_fee = 2_000_000u64;
@@ -1967,7 +1970,14 @@ mod tests {
         so ensure that the starting balance is more than the min balance"
         );
 
-        let fee_payer_account = AccountSharedData::new(starting_balance, 0, &Pubkey::default());
+        let fee_payer_rent_epoch = current_epoch;
+        let fee_payer_rent_debit = 0;
+        let fee_payer_account = AccountSharedData::new_rent_epoch(
+            starting_balance,
+            0,
+            &Pubkey::default(),
+            fee_payer_rent_epoch,
+        );
         let mut mock_accounts = HashMap::new();
         mock_accounts.insert(*fee_payer_address, fee_payer_account.clone());
         let mock_bank = MockBankCallback {
@@ -1979,9 +1989,12 @@ mod tests {
         let result = batch_processor.validate_transaction_fee_payer(
             &mock_bank,
             &message,
+            CheckedTransactionDetails {
+                nonce: None,
+                lamports_per_signature,
+            },
             &FeatureSet::default(),
             &FeeStructure::default(),
-            lamports_per_signature,
             &rent_collector,
             &mut error_counters,
         );
@@ -1995,11 +2008,18 @@ mod tests {
 
         assert_eq!(
             result,
-            Ok((
-                FeeDetails::new_for_tests(transaction_fee, priority_fee, false),
-                post_validation_fee_payer_account,
-                0 // rent due
-            ))
+            Ok(ValidatedTransactionDetails {
+                rollback_accounts: RollbackAccounts::new(
+                    None, // nonce
+                    *fee_payer_address,
+                    post_validation_fee_payer_account.clone(),
+                    fee_payer_rent_debit,
+                    fee_payer_rent_epoch
+                ),
+                fee_details: FeeDetails::new_for_tests(transaction_fee, priority_fee, false),
+                fee_payer_rent_debit,
+                fee_payer_account: post_validation_fee_payer_account,
+            })
         );
     }
 
@@ -2018,14 +2038,14 @@ mod tests {
         let transaction_fee = lamports_per_signature;
         let starting_balance = min_balance - 1;
         let fee_payer_account = AccountSharedData::new(starting_balance, 0, &Pubkey::default());
-        let rent_due = rent_collector
+        let fee_payer_rent_debit = rent_collector
             .get_rent_due(
                 fee_payer_account.lamports(),
                 fee_payer_account.data().len(),
                 fee_payer_account.rent_epoch(),
             )
             .lamports();
-        assert!(rent_due > 0);
+        assert!(fee_payer_rent_debit > 0);
 
         let mut mock_accounts = HashMap::new();
         mock_accounts.insert(*fee_payer_address, fee_payer_account.clone());
@@ -2038,9 +2058,12 @@ mod tests {
         let result = batch_processor.validate_transaction_fee_payer(
             &mock_bank,
             &message,
+            CheckedTransactionDetails {
+                nonce: None,
+                lamports_per_signature,
+            },
             &FeatureSet::default(),
             &FeeStructure::default(),
-            lamports_per_signature,
             &rent_collector,
             &mut error_counters,
         );
@@ -2048,17 +2071,24 @@ mod tests {
         let post_validation_fee_payer_account = {
             let mut account = fee_payer_account.clone();
             account.set_rent_epoch(1);
-            account.set_lamports(starting_balance - transaction_fee - rent_due);
+            account.set_lamports(starting_balance - transaction_fee - fee_payer_rent_debit);
             account
         };
 
         assert_eq!(
             result,
-            Ok((
-                FeeDetails::new_for_tests(transaction_fee, 0, false),
-                post_validation_fee_payer_account,
-                rent_due,
-            ))
+            Ok(ValidatedTransactionDetails {
+                rollback_accounts: RollbackAccounts::new(
+                    None, // nonce
+                    *fee_payer_address,
+                    post_validation_fee_payer_account.clone(),
+                    fee_payer_rent_debit,
+                    0, // rent epoch
+                ),
+                fee_details: FeeDetails::new_for_tests(transaction_fee, 0, false),
+                fee_payer_rent_debit,
+                fee_payer_account: post_validation_fee_payer_account,
+            })
         );
     }
 
@@ -2074,9 +2104,12 @@ mod tests {
         let result = batch_processor.validate_transaction_fee_payer(
             &mock_bank,
             &message,
+            CheckedTransactionDetails {
+                nonce: None,
+                lamports_per_signature,
+            },
             &FeatureSet::default(),
             &FeeStructure::default(),
-            lamports_per_signature,
             &RentCollector::default(),
             &mut error_counters,
         );
@@ -2103,9 +2136,12 @@ mod tests {
         let result = batch_processor.validate_transaction_fee_payer(
             &mock_bank,
             &message,
+            CheckedTransactionDetails {
+                nonce: None,
+                lamports_per_signature,
+            },
             &FeatureSet::default(),
             &FeeStructure::default(),
-            lamports_per_signature,
             &RentCollector::default(),
             &mut error_counters,
         );
@@ -2136,9 +2172,12 @@ mod tests {
         let result = batch_processor.validate_transaction_fee_payer(
             &mock_bank,
             &message,
+            CheckedTransactionDetails {
+                nonce: None,
+                lamports_per_signature,
+            },
             &FeatureSet::default(),
             &FeeStructure::default(),
-            lamports_per_signature,
             &rent_collector,
             &mut error_counters,
         );
@@ -2167,9 +2206,12 @@ mod tests {
         let result = batch_processor.validate_transaction_fee_payer(
             &mock_bank,
             &message,
+            CheckedTransactionDetails {
+                nonce: None,
+                lamports_per_signature,
+            },
             &FeatureSet::default(),
             &FeeStructure::default(),
-            lamports_per_signature,
             &RentCollector::default(),
             &mut error_counters,
         );
@@ -2216,12 +2258,19 @@ mod tests {
 
             let mut error_counters = TransactionErrorMetrics::default();
             let batch_processor = TransactionBatchProcessor::<TestForkGraph>::default();
+            let nonce = Some(NoncePartial::new(
+                *fee_payer_address,
+                fee_payer_account.clone(),
+            ));
             let result = batch_processor.validate_transaction_fee_payer(
                 &mock_bank,
                 &message,
+                CheckedTransactionDetails {
+                    nonce: nonce.clone(),
+                    lamports_per_signature,
+                },
                 &feature_set,
                 &FeeStructure::default(),
-                lamports_per_signature,
                 &rent_collector,
                 &mut error_counters,
             );
@@ -2235,11 +2284,18 @@ mod tests {
 
             assert_eq!(
                 result,
-                Ok((
-                    FeeDetails::new_for_tests(transaction_fee, priority_fee, false),
-                    post_validation_fee_payer_account,
-                    0 // rent due
-                ))
+                Ok(ValidatedTransactionDetails {
+                    rollback_accounts: RollbackAccounts::new(
+                        nonce,
+                        *fee_payer_address,
+                        post_validation_fee_payer_account.clone(),
+                        0, // fee_payer_rent_debit
+                        0, // fee_payer_rent_epoch
+                    ),
+                    fee_details: FeeDetails::new_for_tests(transaction_fee, priority_fee, false),
+                    fee_payer_rent_debit: 0, // rent due
+                    fee_payer_account: post_validation_fee_payer_account,
+                })
             );
         }
 
@@ -2265,9 +2321,12 @@ mod tests {
             let result = batch_processor.validate_transaction_fee_payer(
                 &mock_bank,
                 &message,
+                CheckedTransactionDetails {
+                    nonce: None,
+                    lamports_per_signature,
+                },
                 &feature_set,
                 &FeeStructure::default(),
-                lamports_per_signature,
                 &rent_collector,
                 &mut error_counters,
             );

+ 0 - 1
svm/src/transaction_results.rs

@@ -85,7 +85,6 @@ pub struct TransactionExecutionDetails {
     pub log_messages: Option<Vec<String>>,
     pub inner_instructions: Option<InnerInstructionsList>,
     pub fee_details: FeeDetails,
-    pub is_nonce: bool,
     pub return_data: Option<TransactionReturnData>,
     pub executed_units: u64,
     /// The change in accounts data len for this transaction.