From 4ff93f2c9d44618fb0664543d2f5d2ff66c00fb7 Mon Sep 17 00:00:00 2001 From: hana <81144685+2501babe@users.noreply.github.com> Date: Sat, 27 Dec 2025 17:51:38 -0800 Subject: [PATCH] svm: build NonceInfo during transaction processing (#9455) * svm: build NonceInfo during transaction processing additionally rekey rekey SIMD-0083 lock relaxation --- feature-set/src/lib.rs | 2 +- runtime/src/bank.rs | 7 +- runtime/src/bank/check_transactions.rs | 134 +++-------- runtime/src/bank/tests.rs | 4 +- svm/src/account_loader.rs | 11 +- svm/src/transaction_processor.rs | 299 +++++++++++++++++-------- svm/tests/integration_test.rs | 15 +- 7 files changed, 254 insertions(+), 218 deletions(-) diff --git a/feature-set/src/lib.rs b/feature-set/src/lib.rs index 31b2a6f30c5..1bcb3df59ac 100644 --- a/feature-set/src/lib.rs +++ b/feature-set/src/lib.rs @@ -1080,7 +1080,7 @@ pub mod drop_unchained_merkle_shreds { } pub mod relax_intrabatch_account_locks { - solana_pubkey::declare_id!("ENTRYnPAoT5Swwx73YDGzMp3XnNH1kxacyvLosRHza1i"); + solana_pubkey::declare_id!("4WeHX6QoXCCwqbSFgi6dxnB6QsPo6YApaNTH7P4MLQ99"); } pub mod create_slashing_program { diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index ed539e56274..12037428d93 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -2735,11 +2735,8 @@ impl Bank { blockhash_queue.get_lamports_per_signature(message.recent_blockhash()) } .or_else(|| { - self.load_message_nonce_account(message).map( - |(_nonce_address, _nonce_account, nonce_data)| { - nonce_data.get_lamports_per_signature() - }, - ) + self.load_message_nonce_data(message) + .map(|(_nonce_address, nonce_data)| nonce_data.get_lamports_per_signature()) })?; Some(self.get_fee_for_message_with_lamports_per_signature(message, lamports_per_signature)) } diff --git a/runtime/src/bank/check_transactions.rs b/runtime/src/bank/check_transactions.rs index bb33077519d..5c0ee8d38ed 100644 --- a/runtime/src/bank/check_transactions.rs +++ b/runtime/src/bank/check_transactions.rs @@ -1,18 +1,13 @@ use { super::{Bank, BankStatusCache}, agave_feature_set::{raise_cpi_nesting_limit_to_8, FeatureSet}, - solana_account::{state_traits::StateMut, AccountSharedData}, solana_accounts_db::blockhash_queue::BlockhashQueue, solana_clock::{ MAX_PROCESSING_AGE, MAX_TRANSACTION_FORWARDING_DELAY, MAX_TRANSACTION_FORWARDING_DELAY_GPU, }, solana_fee::{calculate_fee_details, FeeFeatures}, solana_fee_structure::{FeeBudgetLimits, FeeDetails}, - solana_nonce::{ - state::{Data as NonceData, DurableNonce, State as NonceState}, - versions::Versions as NonceVersions, - NONCED_TX_MARKER_IX_INDEX, - }, + solana_nonce::state::{Data as NonceData, DurableNonce}, solana_nonce_account as nonce_account, solana_perf::perf_libs, solana_program_runtime::execution_budget::SVMTransactionExecutionAndFeeBudgetLimits, @@ -20,7 +15,6 @@ use { solana_runtime_transaction::transaction_with_meta::TransactionWithMeta, solana_svm::{ account_loader::{CheckedTransactionDetails, TransactionCheckResult}, - nonce_info::NonceInfo, transaction_error_metrics::TransactionErrorMetrics, }, solana_svm_transaction::svm_message::SVMMessage, @@ -84,10 +78,6 @@ impl Bank { let hash_queue = self.blockhash_queue.read().unwrap(); let last_blockhash = hash_queue.last_hash(); let next_durable_nonce = DurableNonce::from_blockhash(&last_blockhash); - // safe so long as the BlockhashQueue is consistent - let next_lamports_per_signature = hash_queue - .get_lamports_per_signature(&last_blockhash) - .unwrap(); let feature_set: &FeatureSet = &self.feature_set; let fee_features = FeeFeatures::from(feature_set); @@ -136,7 +126,6 @@ impl Bank { max_age, &next_durable_nonce, &hash_queue, - next_lamports_per_signature, error_counters, compute_budget_and_limits, ) @@ -147,7 +136,7 @@ impl Bank { } fn checked_transactions_details_with_test_override( - nonce: Option, + nonce_address: Option, lamports_per_signature: u64, mut compute_budget_and_limits: SVMTransactionExecutionAndFeeBudgetLimits, ) -> CheckedTransactionDetails { @@ -157,7 +146,7 @@ impl Bank { compute_budget_and_limits.fee_details = FeeDetails::default(); } - CheckedTransactionDetails::new(nonce, compute_budget_and_limits) + CheckedTransactionDetails::new(nonce_address, compute_budget_and_limits) } fn check_transaction_age( @@ -166,7 +155,6 @@ impl Bank { max_age: usize, next_durable_nonce: &DurableNonce, hash_queue: &BlockhashQueue, - next_lamports_per_signature: u64, error_counters: &mut TransactionErrorMetrics, compute_budget: SVMTransactionExecutionAndFeeBudgetLimits, ) -> TransactionCheckResult { @@ -177,15 +165,11 @@ impl Bank { hash_info.lamports_per_signature(), compute_budget, )) - } else if let Some((nonce, previous_lamports_per_signature)) = self - .check_load_and_advance_message_nonce_account( - tx, - next_durable_nonce, - next_lamports_per_signature, - ) + } else if let Some((nonce_address, previous_lamports_per_signature)) = + self.check_nonce_transaction_validity(tx, next_durable_nonce) { Ok(Self::checked_transactions_details_with_test_override( - Some(nonce), + Some(nonce_address), previous_lamports_per_signature, compute_budget, )) @@ -195,40 +179,26 @@ impl Bank { } } - pub(super) fn check_load_and_advance_message_nonce_account( + pub(super) fn check_nonce_transaction_validity( &self, message: &impl SVMMessage, next_durable_nonce: &DurableNonce, - next_lamports_per_signature: u64, - ) -> Option<(NonceInfo, u64)> { + ) -> Option<(Pubkey, u64)> { let nonce_is_advanceable = message.recent_blockhash() != next_durable_nonce.as_hash(); if !nonce_is_advanceable { return None; } - let (nonce_address, mut nonce_account, nonce_data) = - self.load_message_nonce_account(message)?; - + let (nonce_address, nonce_data) = self.load_message_nonce_data(message)?; let previous_lamports_per_signature = nonce_data.get_lamports_per_signature(); - let next_nonce_state = NonceState::new_initialized( - &nonce_data.authority, - *next_durable_nonce, - next_lamports_per_signature, - ); - nonce_account - .set_state(&NonceVersions::new(next_nonce_state)) - .ok()?; - Some(( - NonceInfo::new(nonce_address, nonce_account), - previous_lamports_per_signature, - )) + Some((nonce_address, previous_lamports_per_signature)) } - pub(super) fn load_message_nonce_account( + pub(super) fn load_message_nonce_data( &self, message: &impl SVMMessage, - ) -> Option<(Pubkey, AccountSharedData, NonceData)> { + ) -> Option<(Pubkey, NonceData)> { let require_static_nonce_account = self .feature_set .is_active(&agave_feature_set::require_static_nonce_account::id()); @@ -237,14 +207,7 @@ impl Bank { let nonce_data = nonce_account::verify_nonce_account(&nonce_account, message.recent_blockhash())?; - let nonce_is_authorized = message - .get_ix_signers(NONCED_TX_MARKER_IX_INDEX as usize) - .any(|signer| signer == &nonce_data.authority); - if !nonce_is_authorized { - return None; - } - - Some((*nonce_address, nonce_account, nonce_data)) + Some((*nonce_address, nonce_data)) } fn check_status_cache( @@ -294,6 +257,7 @@ mod tests { get_nonce_blockhash, get_nonce_data_from_account, new_sanitized_message, setup_nonce_with_bank, }, + solana_account::state_traits::StateMut, solana_hash::Hash, solana_keypair::Keypair, solana_message::{ @@ -302,6 +266,7 @@ mod tests { Message, MessageHeader, SanitizedMessage, SanitizedVersionedMessage, SimpleAddressLoader, VersionedMessage, }, + solana_nonce::{state::State as NonceState, versions::Versions as NonceVersions}, solana_signer::Signer, solana_system_interface::{ instruction::{self as system_instruction, SystemInstruction}, @@ -312,7 +277,7 @@ mod tests { }; #[test] - fn test_check_and_load_message_nonce_account_ok() { + fn test_check_nonce_transaction_validity_ok() { const STALE_LAMPORTS_PER_SIGNATURE: u64 = 42; let (bank, _mint_keypair, custodian_keypair, nonce_keypair, _) = setup_nonce_with_bank( 10_000_000, @@ -348,29 +313,14 @@ mod tests { .unwrap(); bank.store_account(&nonce_pubkey, &nonce_account); - let nonce_account = bank.get_account(&nonce_pubkey).unwrap(); - let (_, next_lamports_per_signature) = bank.last_blockhash_and_lamports_per_signature(); - let mut expected_nonce_info = NonceInfo::new(nonce_pubkey, nonce_account); - expected_nonce_info - .try_advance_nonce(bank.next_durable_nonce(), next_lamports_per_signature) - .unwrap(); - - // we now expect to: - // * advance the nonce account to the current durable nonce value - // * set the blockhash queue's last blockhash's lamports_per_signature value in the nonce data - // * retrieve the previous lamports_per_signature value set on the nonce data for transaction fee checks assert_eq!( - bank.check_load_and_advance_message_nonce_account( - &message, - &bank.next_durable_nonce(), - next_lamports_per_signature - ), - Some((expected_nonce_info, STALE_LAMPORTS_PER_SIGNATURE)), + bank.check_nonce_transaction_validity(&message, &bank.next_durable_nonce()), + Some((nonce_pubkey, STALE_LAMPORTS_PER_SIGNATURE)), ); } #[test] - fn test_check_and_load_message_nonce_account_not_nonce_fail() { + fn test_check_nonce_transaction_validity_not_nonce_fail() { let (bank, _mint_keypair, custodian_keypair, nonce_keypair, _) = setup_nonce_with_bank( 10_000_000, |_| {}, @@ -392,18 +342,13 @@ mod tests { Some(&custodian_pubkey), &nonce_hash, )); - let (_, lamports_per_signature) = bank.last_blockhash_and_lamports_per_signature(); assert!(bank - .check_load_and_advance_message_nonce_account( - &message, - &bank.next_durable_nonce(), - lamports_per_signature - ) + .check_nonce_transaction_validity(&message, &bank.next_durable_nonce()) .is_none()); } #[test] - fn test_check_and_load_message_nonce_account_missing_ix_pubkey_fail() { + fn test_check_nonce_transaction_validity_missing_ix_pubkey_fail() { let (bank, _mint_keypair, custodian_keypair, nonce_keypair, _) = setup_nonce_with_bank( 10_000_000, |_| {}, @@ -426,18 +371,16 @@ mod tests { &nonce_hash, ); message.instructions[0].accounts.clear(); - let (_, lamports_per_signature) = bank.last_blockhash_and_lamports_per_signature(); assert!(bank - .check_load_and_advance_message_nonce_account( + .check_nonce_transaction_validity( &new_sanitized_message(message), &bank.next_durable_nonce(), - lamports_per_signature, ) .is_none()); } #[test] - fn test_check_and_load_message_nonce_account_nonce_acc_does_not_exist_fail() { + fn test_check_nonce_transaction_validity_nonce_acc_does_not_exist_fail() { let (bank, _mint_keypair, custodian_keypair, nonce_keypair, _) = setup_nonce_with_bank( 10_000_000, |_| {}, @@ -461,18 +404,13 @@ mod tests { Some(&custodian_pubkey), &nonce_hash, )); - let (_, lamports_per_signature) = bank.last_blockhash_and_lamports_per_signature(); assert!(bank - .check_load_and_advance_message_nonce_account( - &message, - &bank.next_durable_nonce(), - lamports_per_signature - ) + .check_nonce_transaction_validity(&message, &bank.next_durable_nonce()) .is_none()); } #[test] - fn test_check_and_load_message_nonce_account_bad_tx_hash_fail() { + fn test_check_nonce_transaction_validity_bad_tx_hash_fail() { let (bank, _mint_keypair, custodian_keypair, nonce_keypair, _) = setup_nonce_with_bank( 10_000_000, |_| {}, @@ -493,19 +431,14 @@ mod tests { Some(&custodian_pubkey), &Hash::default(), )); - let (_, lamports_per_signature) = bank.last_blockhash_and_lamports_per_signature(); assert!(bank - .check_load_and_advance_message_nonce_account( - &message, - &bank.next_durable_nonce(), - lamports_per_signature, - ) + .check_nonce_transaction_validity(&message, &bank.next_durable_nonce()) .is_none()); } - #[test_case(true; "test_check_and_load_message_nonce_account_nonce_is_alt_disallowed")] - #[test_case(false; "test_check_and_load_message_nonce_account_nonce_is_alt_allowed")] - fn test_check_and_load_message_nonce_account_nonce_is_alt(require_static_nonce_account: bool) { + #[test_case(true; "test_check_nonce_transaction_validity_nonce_is_alt_disallowed")] + #[test_case(false; "test_check_nonce_transaction_validity_nonce_is_alt_allowed")] + fn test_check_nonce_transaction_validity_nonce_is_alt(require_static_nonce_account: bool) { let feature_set = if require_static_nonce_account { FeatureSet::all_enabled() } else { @@ -562,14 +495,9 @@ mod tests { ) .unwrap(); - let (_, lamports_per_signature) = bank.last_blockhash_and_lamports_per_signature(); assert_eq!( - bank.check_load_and_advance_message_nonce_account( - &message, - &bank.next_durable_nonce(), - lamports_per_signature - ) - .is_none(), + bank.check_nonce_transaction_validity(&message, &bank.next_durable_nonce()) + .is_none(), require_static_nonce_account, ); } diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 61d226d0a90..aee02a86f84 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -4685,12 +4685,10 @@ fn test_check_ro_durable_nonce_fails() { bank.process_transaction(&tx), Err(TransactionError::BlockhashNotFound) ); - let (_, lamports_per_signature) = bank.last_blockhash_and_lamports_per_signature(); assert_eq!( - bank.check_load_and_advance_message_nonce_account( + bank.check_nonce_transaction_validity( &new_sanitized_message(tx.message().clone()), &bank.next_durable_nonce(), - lamports_per_signature, ), None ); diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index 6bf49c56451..30925252a49 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -3,7 +3,6 @@ use qualifier_attr::{field_qualifiers, qualifiers}; use { crate::{ account_overrides::AccountOverrides, - nonce_info::NonceInfo, rent_calculator::{ check_rent_state_with_account, get_account_rent_state, RENT_EXEMPT_RENT_EPOCH, }, @@ -67,9 +66,9 @@ pub(crate) enum TransactionLoadResult { } #[derive(PartialEq, Eq, Debug, Clone)] -#[cfg_attr(feature = "svm-internal", field_qualifiers(nonce(pub)))] +#[cfg_attr(feature = "svm-internal", field_qualifiers(nonce_address(pub)))] pub struct CheckedTransactionDetails { - pub(crate) nonce: Option, + pub(crate) nonce_address: Option, pub(crate) compute_budget_and_limits: SVMTransactionExecutionAndFeeBudgetLimits, } @@ -77,7 +76,7 @@ pub struct CheckedTransactionDetails { impl Default for CheckedTransactionDetails { fn default() -> Self { Self { - nonce: None, + nonce_address: None, compute_budget_and_limits: SVMTransactionExecutionAndFeeBudgetLimits { budget: SVMTransactionExecutionBudget::default(), loaded_accounts_data_size_limit: NonZeroU32::new(32) @@ -90,11 +89,11 @@ impl Default for CheckedTransactionDetails { impl CheckedTransactionDetails { pub fn new( - nonce: Option, + nonce_address: Option, compute_budget_and_limits: SVMTransactionExecutionAndFeeBudgetLimits, ) -> Self { Self { - nonce, + nonce_address, compute_budget_and_limits, } } diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 1c4b6eda092..ac4e430f3b7 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -29,9 +29,13 @@ use { solana_nonce::{ state::{DurableNonce, State as NonceState}, versions::Versions as NonceVersions, + NONCED_TX_MARKER_IX_INDEX, }, + solana_nonce_account::verify_nonce_account, solana_program_runtime::{ - execution_budget::SVMTransactionExecutionCost, + execution_budget::{ + SVMTransactionExecutionAndFeeBudgetLimits, SVMTransactionExecutionCost, + }, invoke_context::{EnvironmentConfig, InvokeContext}, loaded_programs::{ EpochBoundaryPreparation, ForkGraph, ProgramCache, ProgramCacheEntry, @@ -41,7 +45,6 @@ use { }, solana_pubkey::Pubkey, solana_rent::Rent, - solana_sdk_ids::system_program, solana_svm_callback::TransactionProcessingCallback, solana_svm_feature_set::SVMFeatureSet, solana_svm_log_collector::LogCollector, @@ -433,6 +436,7 @@ impl TransactionBatchProcessor { tx, tx_details, &environment.blockhash, + environment.blockhash_lamports_per_signature, &environment.rent, &mut error_metrics, ) @@ -578,33 +582,38 @@ impl TransactionBatchProcessor { message: &impl SVMMessage, checked_details: CheckedTransactionDetails, environment_blockhash: &Hash, + next_lamports_per_signature: u64, rent: &Rent, error_counters: &mut TransactionErrorMetrics, ) -> TransactionResult { + let CheckedTransactionDetails { + nonce_address, + compute_budget_and_limits, + } = checked_details; + // If this is a nonce transaction, validate the nonce info. // This must be done for every transaction to support SIMD83 because // it may have changed due to use, authorization, or deallocation. - // This function is a successful no-op if given a blockhash transaction. - if let CheckedTransactionDetails { - nonce: Some(ref nonce_info), - compute_budget_and_limits: _, - } = checked_details - { + let nonce_info = if let Some(ref nonce_address) = nonce_address { let next_durable_nonce = DurableNonce::from_blockhash(environment_blockhash); - Self::validate_transaction_nonce( + Some(Self::validate_transaction_nonce( account_loader, message, - nonce_info, + nonce_address, &next_durable_nonce, + next_lamports_per_signature, error_counters, - )?; - } + )?) + } else { + None + }; // Now validate the fee-payer for the transaction unconditionally. Self::validate_transaction_fee_payer( account_loader, message, - checked_details, + nonce_info, + compute_budget_and_limits, rent, error_counters, ) @@ -616,15 +625,11 @@ impl TransactionBatchProcessor { fn validate_transaction_fee_payer( account_loader: &mut AccountLoader, message: &impl SVMMessage, - checked_details: CheckedTransactionDetails, + nonce_info: Option, + compute_budget_and_limits: SVMTransactionExecutionAndFeeBudgetLimits, rent: &Rent, error_counters: &mut TransactionErrorMetrics, ) -> TransactionResult { - let CheckedTransactionDetails { - nonce, - compute_budget_and_limits, - } = checked_details; - let fee_payer_address = message.fee_payer(); // We *must* use load_transaction_account() here because *this* is when the fee-payer @@ -653,7 +658,7 @@ impl TransactionBatchProcessor { // Capture fee-subtracted fee payer account and next nonce account state // to commit if transaction execution fails. let rollback_accounts = RollbackAccounts::new( - nonce, + nonce_info, *fee_payer_address, loaded_fee_payer.account.clone(), fee_payer_loaded_rent_epoch, @@ -671,57 +676,55 @@ impl TransactionBatchProcessor { fn validate_transaction_nonce( account_loader: &mut AccountLoader, message: &impl SVMMessage, - nonce_info: &NonceInfo, + nonce_address: &Pubkey, next_durable_nonce: &DurableNonce, + next_lamports_per_signature: u64, error_counters: &mut TransactionErrorMetrics, - ) -> TransactionResult<()> { + ) -> TransactionResult { // When SIMD83 is enabled, if the nonce has been used in this batch already, we must drop // the transaction. This is the same as if it was used in different batches in the same slot. - // If the nonce account was closed in the batch, we error as if the blockhash didn't validate. - // We must validate the account in case it was reopened, either as a normal system account, - // or a fake nonce account. We must also check the signer in case the authority was changed. - // - // Note these checks are *not* obviated by fee-only transactions. - let nonce_is_valid = account_loader - .load_transaction_account(nonce_info.address(), true) - .and_then(|ref current_nonce| { - system_program::check_id(current_nonce.account.owner()).then_some(())?; - StateMut::::state(¤t_nonce.account).ok() - }) - .and_then( - |current_nonce_versions| match current_nonce_versions.state() { - NonceState::Initialized(ref current_nonce_data) => { - let nonce_can_be_advanced = - ¤t_nonce_data.durable_nonce != next_durable_nonce; - - let nonce_authority_is_valid = message - .account_keys() - .iter() - .enumerate() - .any(|(i, address)| { - address == ¤t_nonce_data.authority && message.is_signer(i) - }); - - if nonce_authority_is_valid { - Some(nonce_can_be_advanced) - } else { - None - } - } - _ => None, - }, + // It is possible that the nonce account was used, closed, closed and reopened, closed and + // spoofed by a non-system program, or had its authority changed. Such a transaction cannot + // be processed, even as fee-only. + + let Some(mut nonce_account) = account_loader + .load_transaction_account(nonce_address, true) + .map(|loaded| loaded.account) + else { + error_counters.account_not_found += 1; + return Err(TransactionError::AccountNotFound); + }; + + // This function verifies: + // * Nonce account owner is SystemProgram + // * Nonce account parses as State::Initialized + // * Stored durable nonce matches the message blockhash + let Some(nonce_data) = verify_nonce_account(&nonce_account, message.recent_blockhash()) + else { + error_counters.blockhash_not_found += 1; + return Err(TransactionError::BlockhashNotFound); + }; + + // We must still check that the nonce account is usable and that its authority has signed. + let nonce_can_be_advanced = &nonce_data.durable_nonce != next_durable_nonce; + let nonce_authority_is_valid = message + .get_ix_signers(NONCED_TX_MARKER_IX_INDEX as usize) + .any(|signer| signer == &nonce_data.authority); + + if nonce_can_be_advanced && nonce_authority_is_valid { + let next_nonce_state = NonceState::new_initialized( + &nonce_data.authority, + *next_durable_nonce, + next_lamports_per_signature, ); + nonce_account + .set_state(&NonceVersions::new(next_nonce_state)) + .expect("Serializing into a validated nonce account cannot fail"); - match nonce_is_valid { - None => { - error_counters.blockhash_not_found += 1; - Err(TransactionError::BlockhashNotFound) - } - Some(false) => { - error_counters.account_not_found += 1; - Err(TransactionError::AccountNotFound) - } - Some(true) => Ok(()), + Ok(NonceInfo::new(*nonce_address, nonce_account)) + } else { + error_counters.blockhash_not_found += 1; + Err(TransactionError::BlockhashNotFound) } } @@ -1128,6 +1131,7 @@ mod tests { solana_sdk_ids::{bpf_loader, loader_v4, system_program, sysvar}, solana_signature::Signature, solana_svm_callback::{AccountState, InvokeContextCallback}, + solana_system_interface::instruction as system_instruction, solana_transaction::{sanitized::SanitizedTransaction, Transaction}, solana_transaction_context::TransactionContext, solana_transaction_error::TransactionError, @@ -1980,6 +1984,7 @@ mod tests { &message, CheckedTransactionDetails::new(None, compute_budget_and_limits), &Hash::default(), + lamports_per_signature, &rent, &mut error_counters, ); @@ -2059,6 +2064,7 @@ mod tests { &message, CheckedTransactionDetails::new(None, compute_budget_and_limits), &Hash::default(), + lamports_per_signature, &rent, &mut error_counters, ); @@ -2098,6 +2104,7 @@ mod tests { #[test] fn test_validate_transaction_fee_payer_not_found() { + let lamports_per_signature = 5000; let message = new_unchecked_sanitized_message(Message::new(&[], Some(&Pubkey::new_unique()))); @@ -2113,6 +2120,7 @@ mod tests { SVMTransactionExecutionAndFeeBudgetLimits::default(), ), &Hash::default(), + lamports_per_signature, &Rent::default(), &mut error_counters, ); @@ -2152,6 +2160,7 @@ mod tests { ), ), &Hash::default(), + lamports_per_signature, &Rent::default(), &mut error_counters, ); @@ -2195,6 +2204,7 @@ mod tests { ), ), &Hash::default(), + lamports_per_signature, &rent, &mut error_counters, ); @@ -2236,6 +2246,7 @@ mod tests { ), ), &Hash::default(), + lamports_per_signature, &Rent::default(), &mut error_counters, ); @@ -2244,34 +2255,145 @@ mod tests { assert_eq!(result, Err(TransactionError::InvalidAccountForFee)); } + #[derive(Debug, PartialEq, Eq)] + enum ValidateNonce { + Success, + NoAccount, + BadOwner, + BlockhashMismatch, + AlreadyUsed, + BadSigner, + } + + #[test_case(ValidateNonce::Success)] + #[test_case(ValidateNonce::NoAccount)] + #[test_case(ValidateNonce::BadOwner)] + #[test_case(ValidateNonce::BlockhashMismatch)] + #[test_case(ValidateNonce::AlreadyUsed)] + #[test_case(ValidateNonce::BadSigner)] + fn test_validate_transaction_nonce(case: ValidateNonce) { + let lamports_per_signature = 5000; + let previous_durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique()); + let nonce_address = Pubkey::new_unique(); + let authority_address = Pubkey::new_unique(); + + let message_blockhash = if case == ValidateNonce::BlockhashMismatch { + Hash::new_unique() + } else { + *previous_durable_nonce.as_hash() + }; + + let message_authority = if case == ValidateNonce::BadSigner { + Pubkey::new_unique() + } else { + authority_address + }; + + let message = new_unchecked_sanitized_message(Message::new_with_blockhash( + &[system_instruction::advance_nonce_account( + &nonce_address, + &message_authority, + )], + Some(&Pubkey::new_unique()), + &message_blockhash, + )); + + let environment_blockhash = Hash::new_unique(); + let next_durable_nonce = DurableNonce::from_blockhash(&environment_blockhash); + + let stored_durable_nonce = if case == ValidateNonce::AlreadyUsed { + next_durable_nonce + } else { + previous_durable_nonce + }; + + let nonce_versions = nonce::versions::Versions::new(nonce::state::State::Initialized( + nonce::state::Data::new( + authority_address, + stored_durable_nonce, + lamports_per_signature, + ), + )); + + let nonce_owner = if case == ValidateNonce::BadOwner { + Pubkey::new_unique() + } else { + system_program::id() + }; + + let nonce_account = AccountSharedData::new_data(1, &nonce_versions, &nonce_owner).unwrap(); + + let mut mock_accounts = HashMap::new(); + + if case != ValidateNonce::NoAccount { + mock_accounts.insert(nonce_address, nonce_account.clone()); + } + + let mock_bank = MockBankCallback { + account_shared_data: Arc::new(RwLock::new(mock_accounts)), + ..Default::default() + }; + let mut account_loader = (&mock_bank).into(); + + let mut error_counters = TransactionErrorMetrics::default(); + let result = TransactionBatchProcessor::::validate_transaction_nonce( + &mut account_loader, + &message, + &nonce_address, + &next_durable_nonce, + lamports_per_signature, + &mut error_counters, + ); + + match case { + ValidateNonce::Success => { + let mut future_nonce_info = NonceInfo::new(nonce_address, nonce_account); + future_nonce_info + .try_advance_nonce(next_durable_nonce, lamports_per_signature) + .unwrap(); + + assert_eq!(result, Ok(future_nonce_info)); + } + ValidateNonce::NoAccount => { + assert_eq!(error_counters.account_not_found.0, 1); + assert_eq!(result, Err(TransactionError::AccountNotFound)); + } + _ => { + assert_eq!(error_counters.blockhash_not_found.0, 1); + assert_eq!(result, Err(TransactionError::BlockhashNotFound)); + } + } + } + #[test_case(false; "informal_loaded_size")] #[test_case(true; "simd186_loaded_size")] fn test_validate_transaction_fee_payer_is_nonce(formalize_loaded_transaction_data_size: bool) { let lamports_per_signature = 5000; let rent = Rent::default(); let compute_unit_limit = 1000u64; - let last_blockhash = Hash::new_unique(); + let previous_durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique()); + let fee_payer_address = &Pubkey::new_unique(); let message = new_unchecked_sanitized_message(Message::new_with_blockhash( &[ + system_instruction::advance_nonce_account(fee_payer_address, fee_payer_address), ComputeBudgetInstruction::set_compute_unit_limit(compute_unit_limit as u32), ComputeBudgetInstruction::set_compute_unit_price(1_000_000), ], - Some(&Pubkey::new_unique()), - &last_blockhash, + Some(fee_payer_address), + previous_durable_nonce.as_hash(), )); let transaction_fee = lamports_per_signature; let compute_budget_and_limits = SVMTransactionExecutionAndFeeBudgetLimits { fee_details: FeeDetails::new(transaction_fee, compute_unit_limit), ..SVMTransactionExecutionAndFeeBudgetLimits::default() }; - let fee_payer_address = message.fee_payer(); let min_balance = Rent::default().minimum_balance(nonce::state::State::size()); let priority_fee = compute_unit_limit; let nonce_versions = nonce::versions::Versions::new(nonce::state::State::Initialized( nonce::state::Data::new( *fee_payer_address, - DurableNonce::default(), + previous_durable_nonce, lamports_per_signature, ), )); @@ -2305,19 +2427,18 @@ mod tests { let mut error_counters = TransactionErrorMetrics::default(); - let tx_details = CheckedTransactionDetails::new( - Some(future_nonce.clone()), - compute_budget_and_limits, - ); + let tx_details = + CheckedTransactionDetails::new(Some(*fee_payer_address), compute_budget_and_limits); let result = TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( - &mut account_loader, - &message, - tx_details, - &environment_blockhash, - &rent, - &mut error_counters, - ); + &mut account_loader, + &message, + tx_details, + &environment_blockhash, + lamports_per_signature, + &rent, + &mut error_counters, + ); let post_validation_fee_payer_account = { let mut account = fee_payer_account.clone(); @@ -2362,11 +2483,6 @@ mod tests { ) .unwrap(); - let mut future_nonce = NonceInfo::new(*fee_payer_address, fee_payer_account.clone()); - future_nonce - .try_advance_nonce(next_durable_nonce, lamports_per_signature) - .unwrap(); - let mut mock_accounts = HashMap::new(); mock_accounts.insert(*fee_payer_address, fee_payer_account.clone()); let mock_bank = MockBankCallback { @@ -2377,19 +2493,18 @@ mod tests { let mut error_counters = TransactionErrorMetrics::default(); - let tx_details = CheckedTransactionDetails::new( - Some(future_nonce.clone()), - compute_budget_and_limits, - ); + let tx_details = + CheckedTransactionDetails::new(Some(*fee_payer_address), compute_budget_and_limits); let result = TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( &mut account_loader, &message, tx_details, &environment_blockhash, + lamports_per_signature, &rent, &mut error_counters, - ); + ); assert_eq!(error_counters.insufficient_funds.0, 1); assert_eq!(result, Err(TransactionError::InsufficientFundsForFee)); @@ -2400,6 +2515,7 @@ mod tests { // validating the fee payer, since that's when the fee payer account is loaded. #[test] fn test_inspect_account_fee_payer() { + let lamports_per_signature = 5000; let fee_payer_address = Pubkey::new_unique(); let fee_payer_account = AccountSharedData::new_rent_epoch( 123_000_000_000, @@ -2433,6 +2549,7 @@ mod tests { ), ), &Hash::default(), + lamports_per_signature, &Rent::default(), &mut TransactionErrorMetrics::default(), ) diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index c3d9cc8ac27..471d57b6faf 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -525,7 +525,7 @@ impl SvmTestEntry { ) }) .unwrap(); - CheckedTransactionDetails::new(tx_details.nonce, compute_budget) + CheckedTransactionDetails::new(tx_details.nonce_address, compute_budget) }); (message, check_result) @@ -552,10 +552,11 @@ pub struct TransactionBatchItem { } impl TransactionBatchItem { + // TODO accept Pubkey and remove all NonceInfo creation from tests fn with_nonce(nonce_info: NonceInfo) -> Self { Self { check_result: Ok(CheckedTransactionDetails::new( - Some(nonce_info), + Some(*nonce_info.address()), SVMTransactionExecutionAndFeeBudgetLimits::default(), )), ..Self::default() @@ -1668,7 +1669,7 @@ fn simd83_nonce_reuse(fee_paying_nonce: bool) -> Vec { Hash::default(), ); - test_entry.push_nonce_transaction(first_transaction, initial_nonce_info.clone()); + test_entry.push_transaction(first_transaction); test_entry.push_nonce_transaction_with_status( second_transaction.clone(), advanced_nonce_info.clone(), @@ -1678,10 +1679,6 @@ fn simd83_nonce_reuse(fee_paying_nonce: bool) -> Vec { test_entries.push(test_entry); } - for test_entry in &mut test_entries { - test_entry.add_initial_program(program_name); - } - // batch 5: // * a successful blockhash transaction that closes the nonce // * a nonce transaction that uses the nonce; this transaction must be dropped @@ -1935,7 +1932,7 @@ fn simd83_nonce_reuse(fee_paying_nonce: bool) -> Vec { ], Some(&fee_payer), &[&fee_payer_keypair, &new_authority_keypair], - *advanced_durable.as_hash(), + *initial_durable.as_hash(), ); test_entry.push_transaction(first_transaction); @@ -3044,8 +3041,8 @@ fn svm_inspect_nonce_load_failure( // by signing with the dummy account we ensure it precedes a separate nonce let transaction = Transaction::new_signed_with_payer( &[ - compute_instruction, advance_instruction, + compute_instruction, fee_only_noop_instruction, ], Some(&fee_payer),