From 61fe6fb6572bd0ab91be308c60f5bed053e51606 Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Mon, 26 Aug 2024 08:26:43 -0700 Subject: [PATCH 01/15] write integration tests for nonce transactions these pass against master as-written --- svm/tests/integration_test.rs | 294 ++++++++++++++++++++++++++++++++-- svm/tests/mock_bank.rs | 37 ++++- 2 files changed, 312 insertions(+), 19 deletions(-) diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 08f6db09101e04..b521eb6fe37bb6 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -6,20 +6,25 @@ use { MockBankCallback, MockForkGraph, WALLCLOCK_TIME, }, solana_sdk::{ - account::{AccountSharedData, WritableAccount}, + account::{AccountSharedData, ReadableAccount, WritableAccount}, clock::Slot, + feature_set::{self, FeatureSet}, hash::Hash, instruction::{AccountMeta, Instruction}, native_token::LAMPORTS_PER_SOL, + nonce::{self, state::DurableNonce}, pubkey::Pubkey, signature::Signer, signer::keypair::Keypair, system_instruction, system_program, system_transaction, + sysvar::rent::Rent, transaction::{SanitizedTransaction, Transaction, TransactionError}, transaction_context::TransactionReturnData, }, solana_svm::{ account_loader::{CheckedTransactionDetails, TransactionCheckResult}, + nonce_info::NonceInfo, + rollback_accounts::RollbackAccounts, transaction_execution_result::TransactionExecutionDetails, transaction_processing_result::ProcessedTransaction, transaction_processor::{ @@ -27,6 +32,7 @@ use { TransactionProcessingEnvironment, }, }, + solana_svm_transaction::svm_message::SVMMessage, solana_type_overrides::sync::{Arc, RwLock}, std::collections::{HashMap, HashSet}, test_case::test_case, @@ -40,22 +46,25 @@ const EXECUTION_SLOT: u64 = 5; // The execution slot must be greater than the de const EXECUTION_EPOCH: u64 = 2; // The execution epoch must be greater than the deployment epoch const LAMPORTS_PER_SIGNATURE: u64 = 5000; -pub type AccountMap = HashMap; +pub type AccountsMap = HashMap; // container for a transaction batch and all data needed to run and verify it against svm #[derive(Debug, Default)] pub struct SvmTestEntry { + // features are disabled by default; these will be enabled + pub enabled_features: Vec, + // programs to deploy to the new svm before transaction execution pub initial_programs: Vec<(String, Slot)>, // accounts to deploy to the new svm before transaction execution - pub initial_accounts: AccountMap, + pub initial_accounts: AccountsMap, // transactions to execute and transaction-specific checks to perform on the results from svm pub transaction_batch: Vec, // expected final account states, checked after transaction execution - pub final_accounts: AccountMap, + pub final_accounts: AccountsMap, } impl SvmTestEntry { @@ -123,6 +132,27 @@ impl SvmTestEntry { }); } + // convenience function that adds a nonce transaction that is expected to succeed + pub fn push_nonce_transaction(&mut self, transaction: Transaction, nonce_info: NonceInfo) { + self.transaction_batch.push(TransactionBatchItem { + transaction, + ..TransactionBatchItem::with_nonce(nonce_info) + }); + } + + // convenience function that adds a nonce transaction that is expected to execute but fail + pub fn push_failed_nonce_transaction( + &mut self, + transaction: Transaction, + nonce_info: NonceInfo, + ) { + self.transaction_batch.push(TransactionBatchItem { + transaction, + asserts: TransactionBatchItemAsserts::failed(), + ..TransactionBatchItem::with_nonce(nonce_info) + }); + } + // internal helper to gather SanitizedTransaction objects for execution fn prepare_transactions(&self) -> (Vec, Vec) { self.transaction_batch @@ -155,6 +185,18 @@ pub struct TransactionBatchItem { pub asserts: TransactionBatchItemAsserts, } +impl TransactionBatchItem { + fn with_nonce(nonce_info: NonceInfo) -> Self { + Self { + check_result: Ok(CheckedTransactionDetails { + nonce: Some(nonce_info), + lamports_per_signature: LAMPORTS_PER_SIGNATURE, + }), + ..Self::default() + } + } +} + impl Default for TransactionBatchItem { fn default() -> Self { Self { @@ -546,8 +588,182 @@ fn simple_transfer() -> Vec { vec![test_entry] } +fn simple_nonce_fee_only(enable_fee_only_transactions: bool) -> Vec { + let mut test_entry = SvmTestEntry::default(); + if enable_fee_only_transactions { + test_entry + .enabled_features + .push(feature_set::enable_transaction_loading_failure_fees::id()); + } + + let program_name = "hello-solana".to_string(); + let real_program_id = program_address(&program_name); + test_entry + .initial_programs + .push((program_name, DEPLOYMENT_SLOT)); + + // create and return a transaction, fee payer, and nonce info + // sets up initial account states but not final ones + let mk_nonce_transaction = |test_entry: &mut SvmTestEntry, program_id, fake_fee_payer: bool| { + let fee_payer_keypair = Keypair::new(); + let fee_payer = fee_payer_keypair.pubkey(); + + if !fake_fee_payer { + let mut fee_payer_data = AccountSharedData::default(); + fee_payer_data.set_lamports(LAMPORTS_PER_SOL); + test_entry.add_initial_account(fee_payer, &fee_payer_data); + } + + let nonce_size = nonce::State::size(); + let nonce_balance = Rent::default().minimum_balance(nonce_size); + let nonce_pubkey = Pubkey::new_unique(); + let nonce_initial_hash = DurableNonce::from_blockhash(&Hash::new_unique()); + let nonce_data = + nonce::state::Data::new(fee_payer, nonce_initial_hash, LAMPORTS_PER_SIGNATURE); + let nonce_account = AccountSharedData::new_data( + nonce_balance, + &nonce::state::Versions::new(nonce::State::Initialized(nonce_data.clone())), + &system_program::id(), + ) + .unwrap(); + let nonce_info = NonceInfo::new(nonce_pubkey, nonce_account.clone()); + + test_entry.add_initial_account(nonce_pubkey, &nonce_account); + + let instructions = vec![ + system_instruction::advance_nonce_account(&nonce_pubkey, &fee_payer), + Instruction::new_with_bytes(program_id, &[], vec![]), + ]; + + let transaction = Transaction::new_signed_with_payer( + &instructions, + Some(&fee_payer), + &[&fee_payer_keypair], + nonce_data.blockhash(), + ); + + (transaction, fee_payer, nonce_info) + }; + + // successful nonce transaction, regardless of features + { + let (transaction, fee_payer, mut nonce_info) = + mk_nonce_transaction(&mut test_entry, real_program_id, false); + test_entry.push_nonce_transaction(transaction, nonce_info.clone()); + + test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE); + + nonce_info + .try_advance_nonce( + DurableNonce::from_blockhash(&Hash::default()), + LAMPORTS_PER_SIGNATURE, + ) + .unwrap(); + test_entry + .final_accounts + .get_mut(nonce_info.address()) + .unwrap() + .data_as_mut_slice() + .copy_from_slice(nonce_info.account().data()); + } + + // non-executing nonce transaction (fee payer doesnt exist) regardless of features + { + let (transaction, _fee_payer, nonce_info) = + mk_nonce_transaction(&mut test_entry, real_program_id, true); + + test_entry + .final_accounts + .get_mut(nonce_info.address()) + .unwrap() + .set_rent_epoch(0); + + test_entry.transaction_batch.push(TransactionBatchItem { + transaction, + asserts: TransactionBatchItemAsserts::not_executed(), + ..TransactionBatchItem::with_nonce(nonce_info) + }); + } + + // failing nonce transaction (bad system instruction) regardless of features + { + let (transaction, fee_payer, mut nonce_info) = + mk_nonce_transaction(&mut test_entry, system_program::id(), false); + test_entry.push_failed_nonce_transaction(transaction, nonce_info.clone()); + + test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE); + + nonce_info + .try_advance_nonce( + DurableNonce::from_blockhash(&Hash::default()), + LAMPORTS_PER_SIGNATURE, + ) + .unwrap(); + test_entry + .final_accounts + .get_mut(nonce_info.address()) + .unwrap() + .data_as_mut_slice() + .copy_from_slice(nonce_info.account().data()); + } + + // and this (program doesnt exist) will be a non-executing transaction without the feature + // or a fee-only transaction with it. which is identical to failed *except* rent is not updated + // HANA and i need to update my verbiage to include this new strange creature + { + let (transaction, fee_payer, mut nonce_info) = + mk_nonce_transaction(&mut test_entry, Pubkey::new_unique(), false); + + if enable_fee_only_transactions { + test_entry.push_failed_nonce_transaction(transaction, nonce_info.clone()); + + test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE); + + nonce_info + .try_advance_nonce( + DurableNonce::from_blockhash(&Hash::default()), + LAMPORTS_PER_SIGNATURE, + ) + .unwrap(); + test_entry + .final_accounts + .get_mut(nonce_info.address()) + .unwrap() + .data_as_mut_slice() + .copy_from_slice(nonce_info.account().data()); + test_entry + .final_accounts + .get_mut(nonce_info.address()) + .unwrap() + .set_rent_epoch(0); + } else { + test_entry + .final_accounts + .get_mut(&fee_payer) + .unwrap() + .set_rent_epoch(0); + + test_entry + .final_accounts + .get_mut(nonce_info.address()) + .unwrap() + .set_rent_epoch(0); + + test_entry.transaction_batch.push(TransactionBatchItem { + transaction, + asserts: TransactionBatchItemAsserts::not_executed(), + ..TransactionBatchItem::with_nonce(nonce_info) + }); + } + } + + vec![test_entry] +} + #[test_case(program_medley())] #[test_case(simple_transfer())] +#[test_case(simple_nonce_fee_only(false))] +#[test_case(simple_nonce_fee_only(true))] fn svm_integration(test_entries: Vec) { for test_entry in test_entries { execute_test_entry(test_entry); @@ -596,37 +812,79 @@ fn execute_test_entry(test_entry: SvmTestEntry) { ..Default::default() }; + let mut feature_set = FeatureSet::default(); + for feature_id in &test_entry.enabled_features { + feature_set.activate(feature_id, 0); + } + + let processing_environment = TransactionProcessingEnvironment { + feature_set: feature_set.into(), + lamports_per_signature: LAMPORTS_PER_SIGNATURE, + ..TransactionProcessingEnvironment::default() + }; + // execute transaction batch let (transactions, check_results) = test_entry.prepare_transactions(); let batch_output = batch_processor.load_and_execute_sanitized_transactions( &mock_bank, &transactions, check_results, - &TransactionProcessingEnvironment::default(), + &processing_environment, &processing_config, ); // build a hashmap of final account states incrementally, starting with all initial states, updating to all final states // NOTE with SIMD-83 an account may appear multiple times in the same batch let mut final_accounts_actual = test_entry.initial_accounts.clone(); - for processed_transaction in batch_output - .processing_results - .iter() - .filter_map(|r| r.as_ref().ok()) - { + + for (index, processed_transaction) in batch_output.processing_results.iter().enumerate() { match processed_transaction { - ProcessedTransaction::Executed(executed_transaction) => { + Ok(ProcessedTransaction::Executed(executed_transaction)) => { for (pubkey, account_data) in executed_transaction.loaded_transaction.accounts.clone() { final_accounts_actual.insert(pubkey, account_data); } } - // NOTE this is a possible state with `feature_set::enable_transaction_loading_failure_fees` enabled - // by using `TransactionProcessingEnvironment::default()` we have all features disabled - // in other words, this will be unreachable until we are ready to test fee-only transactions - // (or the feature is activated on mainnet and removed... but we should do it before then!) - ProcessedTransaction::FeesOnly(_) => unreachable!(), + Ok(ProcessedTransaction::FeesOnly(fees_only_transaction)) => { + let fee_payer = transactions[index].fee_payer(); + + match fees_only_transaction.rollback_accounts.clone() { + RollbackAccounts::FeePayerOnly { fee_payer_account } => { + final_accounts_actual.insert(*fee_payer, fee_payer_account); + } + RollbackAccounts::SameNonceAndFeePayer { nonce } => { + let mut new_nonce = nonce.clone(); + new_nonce + .try_advance_nonce( + DurableNonce::from_blockhash(&Hash::default()), + LAMPORTS_PER_SIGNATURE, + ) + .unwrap(); + + final_accounts_actual + .insert(*new_nonce.address(), new_nonce.account().clone()); + } + RollbackAccounts::SeparateNonceAndFeePayer { + nonce, + fee_payer_account, + } => { + final_accounts_actual.insert(*fee_payer, fee_payer_account); + + let mut new_nonce = nonce.clone(); + new_nonce + .try_advance_nonce( + DurableNonce::from_blockhash(&Hash::default()), + LAMPORTS_PER_SIGNATURE, + ) + .unwrap(); + + final_accounts_actual + .insert(*new_nonce.address(), new_nonce.account().clone()); + } + } + } + Err(_) => {} } } @@ -650,7 +908,9 @@ fn execute_test_entry(test_entry: SvmTestEntry) { match processing_result { Ok(ProcessedTransaction::Executed(executed_transaction)) => test_item_asserts .check_executed_transaction(&executed_transaction.execution_details), - Ok(ProcessedTransaction::FeesOnly(_)) => unreachable!(), + // HANA we have to expand our semantics, there are four cases now + // succeeded, failed-executed, failed-processed, no-op + Ok(ProcessedTransaction::FeesOnly(_)) => assert!(!test_item_asserts.succeeded), Err(_) => assert!(!test_item_asserts.executed), } } diff --git a/svm/tests/mock_bank.rs b/svm/tests/mock_bank.rs index 6ece85bfcc1385..fe7c2a604aeb5c 100644 --- a/svm/tests/mock_bank.rs +++ b/svm/tests/mock_bank.rs @@ -1,7 +1,9 @@ +#[allow(deprecated)] +use solana_sdk::sysvar::recent_blockhashes::{Entry as BlockhashesEntry, RecentBlockhashes}; use { solana_bpf_loader_program::syscalls::{ - SyscallAbort, SyscallGetClockSysvar, SyscallInvokeSignedRust, SyscallLog, SyscallMemcpy, - SyscallMemset, SyscallSetReturnData, + SyscallAbort, SyscallGetClockSysvar, SyscallGetRentSysvar, SyscallInvokeSignedRust, + SyscallLog, SyscallMemcpy, SyscallMemset, SyscallSetReturnData, }, solana_compute_budget::compute_budget::ComputeBudget, solana_program_runtime::{ @@ -21,6 +23,7 @@ use { feature_set::FeatureSet, native_loader, pubkey::Pubkey, + rent::Rent, slot_hashes::Slot, sysvar::SysvarId, }, @@ -201,6 +204,8 @@ pub fn create_executable_environment( program_cache.fork_graph = Some(Arc::downgrade(&fork_graph)); // We must fill in the sysvar cache entries + + // clock contents are important because we use them for a sysvar loading test let clock = Clock { slot: DEPLOYMENT_SLOT, epoch_start_timestamp: WALLCLOCK_TIME.saturating_sub(10) as UnixTimestamp, @@ -216,6 +221,30 @@ pub fn create_executable_environment( .write() .unwrap() .insert(Clock::id(), account_data); + + // default rent is fine + let rent = Rent::default(); + + let mut account_data = AccountSharedData::default(); + account_data.set_data(bincode::serialize(&rent).unwrap()); + mock_bank + .account_shared_data + .write() + .unwrap() + .insert(Rent::id(), account_data); + + // advance nonce asserts recent blockhashes is non-empty but then just gets the blockhash from ic + #[allow(deprecated)] + let recent_blockhashes = vec![BlockhashesEntry::default()]; + + let mut account_data = AccountSharedData::default(); + account_data.set_data(bincode::serialize(&recent_blockhashes).unwrap()); + #[allow(deprecated)] + mock_bank + .account_shared_data + .write() + .unwrap() + .insert(RecentBlockhashes::id(), account_data); } #[allow(unused)] @@ -304,5 +333,9 @@ fn create_custom_environment<'a>() -> BuiltinProgram> { .register_function_hashed(*b"sol_get_clock_sysvar", SyscallGetClockSysvar::vm) .expect("Registration failed"); + function_registry + .register_function_hashed(*b"sol_get_rent_sysvar", SyscallGetRentSysvar::vm) + .expect("Registration failed"); + BuiltinProgram::new_loader(vm_config, function_registry) } From be9d83d836e819581fd921452e9cc08cc88224da Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Mon, 26 Aug 2024 22:03:56 -0700 Subject: [PATCH 02/15] advance nonce when creating rollback accounts previously rollback accounts carried the fee payer that was to be committed, but an untouched nonce now, the account saver can use the nonce and fee payer from rollback accounts identically, possibly merging them for a fee-paying nonce --- core/src/banking_stage/committer.rs | 7 +------ core/src/banking_stage/consumer.rs | 14 +------------- runtime/src/account_saver.rs | 30 ++++++----------------------- runtime/src/bank.rs | 13 ++----------- svm/src/rollback_accounts.rs | 4 ++++ svm/src/transaction_processor.rs | 24 +++++++++++++++++++---- 6 files changed, 34 insertions(+), 58 deletions(-) diff --git a/core/src/banking_stage/committer.rs b/core/src/banking_stage/committer.rs index 4c5bf8bbf5382b..ff27104251759e 100644 --- a/core/src/banking_stage/committer.rs +++ b/core/src/banking_stage/committer.rs @@ -12,7 +12,7 @@ use { transaction_batch::TransactionBatch, vote_sender_types::ReplayVoteSender, }, - solana_sdk::{hash::Hash, pubkey::Pubkey, saturating_add_assign}, + solana_sdk::{pubkey::Pubkey, saturating_add_assign}, solana_svm::{ transaction_commit_result::{TransactionCommitResult, TransactionCommitResultExtensions}, transaction_processing_result::{ @@ -65,13 +65,10 @@ impl Committer { self.transaction_status_sender.is_some() } - #[allow(clippy::too_many_arguments)] pub(super) fn commit_transactions( &self, batch: &TransactionBatch, processing_results: Vec, - last_blockhash: Hash, - lamports_per_signature: u64, starting_transaction_index: Option, bank: &Arc, pre_balance_info: &mut PreBalanceInfo, @@ -87,8 +84,6 @@ impl Committer { let (commit_results, commit_time_us) = measure_us!(bank.commit_transactions( batch.sanitized_transactions(), processing_results, - last_blockhash, - lamports_per_signature, processed_counts, &mut execute_and_commit_timings.execute_timings, )); diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 39e3dc1fd95d5c..e1f1b67fdb41a5 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -665,17 +665,7 @@ impl Consumer { let (freeze_lock, freeze_lock_us) = measure_us!(bank.freeze_lock()); execute_and_commit_timings.freeze_lock_us = freeze_lock_us; - - // In order to avoid a race condition, leaders must get the last - // blockhash *before* recording transactions because recording - // transactions will only succeed if the block max tick height hasn't - // been reached yet. If they get the last blockhash *after* recording - // transactions, the block max tick height could have already been - // reached and the blockhash queue could have already been updated with - // a new blockhash. - let ((last_blockhash, lamports_per_signature), last_blockhash_us) = - measure_us!(bank.last_blockhash_and_lamports_per_signature()); - execute_and_commit_timings.last_blockhash_us = last_blockhash_us; + execute_and_commit_timings.last_blockhash_us = 0; let (record_transactions_summary, record_us) = measure_us!(self .transaction_recorder @@ -713,8 +703,6 @@ impl Consumer { self.committer.commit_transactions( batch, processing_results, - last_blockhash, - lamports_per_signature, starting_transaction_index, bank, &mut pre_balance_info, diff --git a/runtime/src/account_saver.rs b/runtime/src/account_saver.rs index da4188b87f441c..9406255134ac72 100644 --- a/runtime/src/account_saver.rs +++ b/runtime/src/account_saver.rs @@ -52,8 +52,6 @@ pub fn collect_accounts_to_store<'a, T: SVMMessage>( txs: &'a [T], txs_refs: &'a Option>>, processing_results: &'a mut [TransactionProcessingResult], - durable_nonce: &DurableNonce, - lamports_per_signature: u64, ) -> ( Vec<(&'a Pubkey, &'a AccountSharedData)>, Option>, @@ -66,7 +64,7 @@ pub fn collect_accounts_to_store<'a, T: SVMMessage>( for (index, (processing_result, transaction)) in processing_results.iter_mut().zip(txs).enumerate() { - let Some(processed_tx) = processing_result.processed_transaction_mut() else { + let Some(processed_tx) = processing_result.processed_transaction() else { // Don't store any accounts if tx wasn't executed continue; }; @@ -88,9 +86,7 @@ pub fn collect_accounts_to_store<'a, T: SVMMessage>( &mut transactions, transaction, transaction_ref, - &mut executed_tx.loaded_transaction.rollback_accounts, - durable_nonce, - lamports_per_signature, + &executed_tx.loaded_transaction.rollback_accounts, ); } } @@ -100,9 +96,7 @@ pub fn collect_accounts_to_store<'a, T: SVMMessage>( &mut transactions, transaction, transaction_ref, - &mut fees_only_tx.rollback_accounts, - durable_nonce, - lamports_per_signature, + &fees_only_tx.rollback_accounts, ); } } @@ -142,25 +136,18 @@ fn collect_accounts_for_failed_tx<'a, T: SVMMessage>( collected_account_transactions: &mut Option>, transaction: &'a T, transaction_ref: Option<&'a SanitizedTransaction>, - rollback_accounts: &'a mut RollbackAccounts, - durable_nonce: &DurableNonce, - lamports_per_signature: u64, + rollback_accounts: &'a RollbackAccounts, ) { let fee_payer_address = transaction.fee_payer(); match rollback_accounts { RollbackAccounts::FeePayerOnly { fee_payer_account } => { - collected_accounts.push((fee_payer_address, &*fee_payer_account)); + collected_accounts.push((fee_payer_address, fee_payer_account)); if let Some(collected_account_transactions) = collected_account_transactions { collected_account_transactions .push(transaction_ref.expect("transaction ref must exist if collecting")); } } RollbackAccounts::SameNonceAndFeePayer { nonce } => { - // Since we know we are dealing with a valid nonce account, - // unwrap is safe here - nonce - .try_advance_nonce(*durable_nonce, lamports_per_signature) - .unwrap(); collected_accounts.push((nonce.address(), nonce.account())); if let Some(collected_account_transactions) = collected_account_transactions { collected_account_transactions @@ -171,17 +158,12 @@ fn collect_accounts_for_failed_tx<'a, T: SVMMessage>( nonce, fee_payer_account, } => { - collected_accounts.push((fee_payer_address, &*fee_payer_account)); + collected_accounts.push((fee_payer_address, fee_payer_account)); if let Some(collected_account_transactions) = collected_account_transactions { collected_account_transactions .push(transaction_ref.expect("transaction ref must exist if collecting")); } - // Since we know we are dealing with a valid nonce account, - // unwrap is safe here - nonce - .try_advance_nonce(*durable_nonce, lamports_per_signature) - .unwrap(); collected_accounts.push((nonce.address(), nonce.account())); if let Some(collected_account_transactions) = collected_account_transactions { collected_account_transactions diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 7ad9c63ee0e468..e1316ee47803e4 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -127,7 +127,6 @@ use { message::{AccountKeys, SanitizedMessage}, native_loader, native_token::LAMPORTS_PER_SOL, - nonce::state::DurableNonce, packet::PACKET_DATA_SIZE, precompiles::get_precompiles, pubkey::Pubkey, @@ -3761,9 +3760,7 @@ impl Bank { pub fn commit_transactions( &self, sanitized_txs: &[SanitizedTransaction], - mut processing_results: Vec, - last_blockhash: Hash, - lamports_per_signature: u64, + processing_results: Vec, processed_counts: &ProcessedTransactionCounts, timings: &mut ExecuteTimings, ) -> Vec { @@ -3812,9 +3809,7 @@ impl Bank { let (accounts_to_store, transactions) = collect_accounts_to_store( sanitized_txs, &maybe_transaction_refs, - &mut processing_results, - &durable_nonce, - lamports_per_signature, + &processing_results, ); self.rc.accounts.store_cached( (self.slot(), accounts_to_store.as_slice()), @@ -4633,13 +4628,9 @@ impl Bank { }, ); - let (last_blockhash, lamports_per_signature) = - self.last_blockhash_and_lamports_per_signature(); let commit_results = self.commit_transactions( batch.sanitized_transactions(), processing_results, - last_blockhash, - lamports_per_signature, &processed_counts, timings, ); diff --git a/svm/src/rollback_accounts.rs b/svm/src/rollback_accounts.rs index c2c02f2f80bd43..d16bfc66094237 100644 --- a/svm/src/rollback_accounts.rs +++ b/svm/src/rollback_accounts.rs @@ -51,6 +51,10 @@ impl RollbackAccounts { if let Some(nonce) = nonce { if &fee_payer_address == nonce.address() { + fee_payer_account + .data_as_mut_slice() + .copy_from_slice(nonce.account().data()); + RollbackAccounts::SameNonceAndFeePayer { nonce: NonceInfo::new(fee_payer_address, fee_payer_account), } diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index c6a5e43149949e..a69eb993cdee0f 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -42,7 +42,8 @@ use { fee::{FeeBudgetLimits, FeeStructure}, hash::Hash, inner_instruction::{InnerInstruction, InnerInstructionsList}, - instruction::{CompiledInstruction, TRANSACTION_LEVEL_STACK_HEIGHT}, + instruction::{CompiledInstruction, InstructionError, TRANSACTION_LEVEL_STACK_HEIGHT}, + nonce::state::DurableNonce, pubkey::Pubkey, rent_collector::RentCollector, saturating_add_assign, @@ -237,6 +238,7 @@ impl TransactionBatchProcessor { let mut error_metrics = TransactionErrorMetrics::default(); let mut execute_timings = ExecuteTimings::default(); + let durable_nonce = DurableNonce::from_blockhash(&environment.blockhash); let (validation_results, validate_fees_us) = measure_us!(self.validate_fees( callbacks, config.account_overrides, @@ -249,6 +251,7 @@ impl TransactionBatchProcessor { environment .rent_collector .unwrap_or(&RentCollector::default()), + &durable_nonce, &mut error_metrics )); @@ -370,6 +373,7 @@ impl TransactionBatchProcessor { } } + #[allow(clippy::too_many_arguments)] fn validate_fees( &self, callbacks: &CB, @@ -395,6 +399,7 @@ impl TransactionBatchProcessor { feature_set, fee_structure, rent_collector, + durable_nonce, error_counters, ) }) @@ -405,6 +410,7 @@ impl TransactionBatchProcessor { // Loads transaction fee payer, collects rent if necessary, then calculates // transaction fees, and deducts them from the fee payer balance. If the // account is not found or has insufficient funds, an error is returned. + #[allow(clippy::too_many_arguments)] fn validate_transaction_fee_payer( &self, callbacks: &CB, @@ -450,7 +456,7 @@ impl TransactionBatchProcessor { .rent_amount; let CheckedTransactionDetails { - nonce, + mut nonce, lamports_per_signature, } = checked_details; @@ -473,8 +479,18 @@ impl TransactionBatchProcessor { fee_details.total_fee(), )?; - // Capture fee-subtracted fee payer account and original nonce account state - // to rollback to if transaction execution fails. + if let Some(ref mut nonce_info) = nonce { + // Coding instruction 0 is appropriate because it must be the advance nonce instruction + // However the nonce info is guaranteed to be valid so this should never occur + nonce_info + .try_advance_nonce(*durable_nonce, lamports_per_signature) + .map_err(|_| { + TransactionError::InstructionError(0, InstructionError::InvalidAccountData) + })?; + }; + + // Capture fee-subtracted fee payer account and next nonce account state + // to commit if transaction execution fails. let rollback_accounts = RollbackAccounts::new( nonce, *fee_payer_address, From ccd218d9392c6d3b699e5edc44b37ef239dfc9b7 Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Mon, 26 Aug 2024 23:55:51 -0700 Subject: [PATCH 03/15] fix nonce-related unit tests --- runtime/src/account_saver.rs | 14 ++++++-------- svm/src/transaction_processor.rs | 25 ++++++++++++++++++++----- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/runtime/src/account_saver.rs b/runtime/src/account_saver.rs index 9406255134ac72..5e1040c6d97129 100644 --- a/runtime/src/account_saver.rs +++ b/runtime/src/account_saver.rs @@ -186,7 +186,7 @@ mod tests { message::Message, native_loader, nonce::{ - state::{Data as NonceData, Versions as NonceVersions}, + state::{Data as NonceData, DurableNonce, Versions as NonceVersions}, State as NonceState, }, nonce_account, @@ -297,7 +297,7 @@ mod tests { }; let txs = vec![tx0.clone(), tx1.clone()]; - let mut processing_results = vec![ + let processing_results = vec![ new_executed_processing_result(Ok(()), loaded0), new_executed_processing_result(Ok(()), loaded1), ]; @@ -365,7 +365,7 @@ mod tests { }; let txs = vec![tx]; - let mut processing_results = vec![new_executed_processing_result( + let processing_results = vec![new_executed_processing_result( Err(TransactionError::InstructionError( 1, InstructionError::InvalidArgument, @@ -465,9 +465,8 @@ mod tests { loaded_accounts_data_size: 0, }; - let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique()); let txs = vec![tx]; - let mut processing_results = vec![new_executed_processing_result( + let processing_results = vec![new_executed_processing_result( Err(TransactionError::InstructionError( 1, InstructionError::InvalidArgument, @@ -579,9 +578,8 @@ mod tests { loaded_accounts_data_size: 0, }; - let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique()); let txs = vec![tx]; - let mut processing_results = vec![new_executed_processing_result( + let processing_results = vec![new_executed_processing_result( Err(TransactionError::InstructionError( 1, InstructionError::InvalidArgument, @@ -640,7 +638,7 @@ mod tests { let from_account_pre = AccountSharedData::new(4242, 0, &Pubkey::default()); let txs = vec![tx]; - let mut processing_results = vec![Ok(ProcessedTransaction::FeesOnly(Box::new( + let processing_results = vec![Ok(ProcessedTransaction::FeesOnly(Box::new( FeesOnlyTransaction { load_error: TransactionError::InvalidProgramForExecution, fee_details: FeeDetails::default(), diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index a69eb993cdee0f..48040f8c9907a4 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -1912,6 +1912,7 @@ mod tests { &FeatureSet::default(), &FeeStructure::default(), &rent_collector, + &DurableNonce::default(), &mut error_counters, ); @@ -1990,6 +1991,7 @@ mod tests { &FeatureSet::default(), &FeeStructure::default(), &rent_collector, + &DurableNonce::default(), &mut error_counters, ); @@ -2041,6 +2043,7 @@ mod tests { &FeatureSet::default(), &FeeStructure::default(), &RentCollector::default(), + &DurableNonce::default(), &mut error_counters, ); @@ -2075,6 +2078,7 @@ mod tests { &FeatureSet::default(), &FeeStructure::default(), &RentCollector::default(), + &DurableNonce::default(), &mut error_counters, ); @@ -2113,6 +2117,7 @@ mod tests { &FeatureSet::default(), &FeeStructure::default(), &rent_collector, + &DurableNonce::default(), &mut error_counters, ); @@ -2149,6 +2154,7 @@ mod tests { &FeatureSet::default(), &FeeStructure::default(), &RentCollector::default(), + &DurableNonce::default(), &mut error_counters, ); @@ -2181,6 +2187,7 @@ mod tests { &FeatureSet::default(), &FeeStructure::default(), &RentCollector::default(), + &DurableNonce::default(), &mut error_counters, ); @@ -2194,13 +2201,14 @@ mod tests { let lamports_per_signature = 5000; let rent_collector = RentCollector::default(); let compute_unit_limit = 2 * solana_compute_budget_program::DEFAULT_COMPUTE_UNITS; + let last_blockhash = Hash::new_unique(); let message = new_unchecked_sanitized_message(Message::new_with_blockhash( &[ ComputeBudgetInstruction::set_compute_unit_limit(compute_unit_limit as u32), ComputeBudgetInstruction::set_compute_unit_price(1_000_000), ], Some(&Pubkey::new_unique()), - &Hash::new_unique(), + &last_blockhash, )); let compute_budget_limits = process_compute_budget_instructions(SVMMessage::program_instructions_iter(&message)) @@ -2230,10 +2238,14 @@ mod tests { let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); - let nonce = Some(NonceInfo::new( - *fee_payer_address, - fee_payer_account.clone(), - )); + + let durable_nonce = DurableNonce::from_blockhash(&last_blockhash); + let mut nonce_info = NonceInfo::new(*fee_payer_address, fee_payer_account.clone()); + nonce_info + .try_advance_nonce(durable_nonce, lamports_per_signature) + .unwrap(); + let nonce = Some(nonce_info); + let result = batch_processor.validate_transaction_fee_payer( &mock_bank, None, @@ -2245,6 +2257,7 @@ mod tests { &feature_set, &FeeStructure::default(), &rent_collector, + &durable_nonce, &mut error_counters, ); @@ -2307,6 +2320,7 @@ mod tests { &feature_set, &FeeStructure::default(), &rent_collector, + &DurableNonce::default(), &mut error_counters, ); @@ -2361,6 +2375,7 @@ mod tests { &FeatureSet::default(), &FeeStructure::default(), &rent_collector, + &DurableNonce::default(), &mut error_counters, ); assert!( From d6b611bca31add8a141aa06d57d451d2172a035e Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Tue, 27 Aug 2024 00:14:18 -0700 Subject: [PATCH 04/15] remove nonce hack from integration test --- svm/tests/integration_test.rs | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index b521eb6fe37bb6..2ab611f060bb4c 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -854,33 +854,14 @@ fn execute_test_entry(test_entry: SvmTestEntry) { final_accounts_actual.insert(*fee_payer, fee_payer_account); } RollbackAccounts::SameNonceAndFeePayer { nonce } => { - let mut new_nonce = nonce.clone(); - new_nonce - .try_advance_nonce( - DurableNonce::from_blockhash(&Hash::default()), - LAMPORTS_PER_SIGNATURE, - ) - .unwrap(); - - final_accounts_actual - .insert(*new_nonce.address(), new_nonce.account().clone()); + final_accounts_actual.insert(*nonce.address(), nonce.account().clone()); } RollbackAccounts::SeparateNonceAndFeePayer { nonce, fee_payer_account, } => { final_accounts_actual.insert(*fee_payer, fee_payer_account); - - let mut new_nonce = nonce.clone(); - new_nonce - .try_advance_nonce( - DurableNonce::from_blockhash(&Hash::default()), - LAMPORTS_PER_SIGNATURE, - ) - .unwrap(); - - final_accounts_actual - .insert(*new_nonce.address(), new_nonce.account().clone()); + final_accounts_actual.insert(*nonce.address(), nonce.account().clone()); } } } From 079a5a100cc00891f37f92f9436ff6c0f6a3c628 Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Tue, 27 Aug 2024 10:47:00 -0700 Subject: [PATCH 05/15] support four tx states in svm integration --- svm/tests/integration_test.rs | 211 +++++++++++++++++++++------------- 1 file changed, 130 insertions(+), 81 deletions(-) diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 2ab611f060bb4c..99f5c2344c3712 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -117,38 +117,43 @@ impl SvmTestEntry { // convenience function that adds a transaction that is expected to succeed pub fn push_transaction(&mut self, transaction: Transaction) { - self.transaction_batch.push(TransactionBatchItem { - transaction, - ..TransactionBatchItem::default() - }); + self.push_transaction_with_status(transaction, ExecutionStatus::Succeeded) } - // convenience function that adds a transaction that is expected to execute but fail - pub fn push_failed_transaction(&mut self, transaction: Transaction) { + // convenience function that adds a transaction with an expected execution status + pub fn push_transaction_with_status( + &mut self, + transaction: Transaction, + status: ExecutionStatus, + ) { self.transaction_batch.push(TransactionBatchItem { transaction, - asserts: TransactionBatchItemAsserts::failed(), + asserts: TransactionBatchItemAsserts { + status, + ..TransactionBatchItemAsserts::default() + }, ..TransactionBatchItem::default() }); } // convenience function that adds a nonce transaction that is expected to succeed pub fn push_nonce_transaction(&mut self, transaction: Transaction, nonce_info: NonceInfo) { - self.transaction_batch.push(TransactionBatchItem { - transaction, - ..TransactionBatchItem::with_nonce(nonce_info) - }); + self.push_nonce_transaction_with_status(transaction, nonce_info, ExecutionStatus::Succeeded) } - // convenience function that adds a nonce transaction that is expected to execute but fail - pub fn push_failed_nonce_transaction( + // convenience function that adds a nonce transaction with an expected execution status + pub fn push_nonce_transaction_with_status( &mut self, transaction: Transaction, nonce_info: NonceInfo, + status: ExecutionStatus, ) { self.transaction_batch.push(TransactionBatchItem { transaction, - asserts: TransactionBatchItemAsserts::failed(), + asserts: TransactionBatchItemAsserts { + status, + ..TransactionBatchItemAsserts::default() + }, ..TransactionBatchItem::with_nonce(nonce_info) }); } @@ -205,7 +210,7 @@ impl Default for TransactionBatchItem { nonce: None, lamports_per_signature: LAMPORTS_PER_SIGNATURE, }), - asserts: TransactionBatchItemAsserts::succeeded(), + asserts: TransactionBatchItemAsserts::default(), } } } @@ -213,45 +218,33 @@ impl Default for TransactionBatchItem { // asserts for a given transaction in a batch // we can automatically check whether it executed, whether it succeeded // log items we expect to see (exect match only), and rodata -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Default)] pub struct TransactionBatchItemAsserts { - pub executed: bool, - pub succeeded: bool, + pub status: ExecutionStatus, pub logs: Vec, pub return_data: ReturnDataAssert, } impl TransactionBatchItemAsserts { - pub fn succeeded() -> Self { - Self { - executed: true, - succeeded: true, - logs: vec![], - return_data: ReturnDataAssert::Skip, - } + pub fn succeeded(&self) -> bool { + self.status.succeeded() } - pub fn failed() -> Self { - Self { - executed: true, - succeeded: false, - logs: vec![], - return_data: ReturnDataAssert::Skip, - } + pub fn executed(&self) -> bool { + self.status.executed() } - pub fn not_executed() -> Self { - Self { - executed: false, - succeeded: false, - logs: vec![], - return_data: ReturnDataAssert::Skip, - } + pub fn processed(&self) -> bool { + self.status.processed() + } + + pub fn discarded(&self) -> bool { + self.status.discarded() } pub fn check_executed_transaction(&self, execution_details: &TransactionExecutionDetails) { - assert!(self.executed); - assert_eq!(self.succeeded, execution_details.status.is_ok()); + assert!(self.executed()); + assert_eq!(self.succeeded(), execution_details.status.is_ok()); if !self.logs.is_empty() { let actual_logs = execution_details.log_messages.as_ref().unwrap(); @@ -269,7 +262,51 @@ impl TransactionBatchItemAsserts { } } -#[derive(Clone, Debug, Default, PartialEq)] +impl From for TransactionBatchItemAsserts { + fn from(status: ExecutionStatus) -> Self { + Self { + status, + ..Self::default() + } + } +} + +// states a transaction can end in after a trip through the batch processor: +// * discarded: no-op. not even processed. a flawed transaction excluded from the entry +// * processed-failed: aka fee (and nonce) only. charged and added to an entry but not executed, would have failed invariably +// * executed-failed: failed during execution. as above, fees charged and nonce advanced +// * succeeded: what we all aspire to be in our transaction processing lifecycles +#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord)] +pub enum ExecutionStatus { + Discarded, + ProcessedFailed, + ExecutedFailed, + #[default] + Succeeded, +} + +// note we avoid the word "failed" because it is confusing +// the batch processor uses it to mean "executed and not succeeded" +// but intuitively (and from the point of a user) it could just as likely mean "any state other than succeeded" +impl ExecutionStatus { + pub fn succeeded(self) -> bool { + self == Self::Succeeded + } + + pub fn executed(self) -> bool { + self > Self::ProcessedFailed + } + + pub fn processed(self) -> bool { + self != Self::Discarded + } + + pub fn discarded(self) -> bool { + self == Self::Discarded + } +} + +#[derive(Clone, Debug, Default, PartialEq, Eq)] pub enum ReturnDataAssert { Some(TransactionReturnData), None, @@ -439,12 +476,15 @@ fn program_medley() -> Vec { ], ); - test_entry.push_failed_transaction(Transaction::new_signed_with_payer( - &[instruction], - Some(&fee_payer), - &[&fee_payer_keypair, &sender_keypair], - Hash::default(), - )); + test_entry.push_transaction_with_status( + Transaction::new_signed_with_payer( + &[instruction], + Some(&fee_payer), + &[&fee_payer_keypair, &sender_keypair], + Hash::default(), + ), + ExecutionStatus::ExecutedFailed, + ); test_entry.transaction_batch[3] .asserts @@ -467,7 +507,7 @@ fn program_medley() -> Vec { Hash::default(), ), check_result: Err(TransactionError::BlockhashNotFound), - asserts: TransactionBatchItemAsserts::not_executed(), + asserts: ExecutionStatus::Discarded.into(), }); } @@ -515,12 +555,15 @@ fn simple_transfer() -> Vec { source_data.set_lamports(transfer_amount - 1); test_entry.add_initial_account(source, &source_data); - test_entry.push_failed_transaction(system_transaction::transfer( - &source_keypair, - &Pubkey::new_unique(), - transfer_amount, - Hash::default(), - )); + test_entry.push_transaction_with_status( + system_transaction::transfer( + &source_keypair, + &Pubkey::new_unique(), + transfer_amount, + Hash::default(), + ), + ExecutionStatus::ExecutedFailed, + ); test_entry.decrease_expected_lamports(&source, LAMPORTS_PER_SIGNATURE); } @@ -535,23 +578,22 @@ fn simple_transfer() -> Vec { Hash::default(), ), check_result: Err(TransactionError::BlockhashNotFound), - asserts: TransactionBatchItemAsserts::not_executed(), + asserts: ExecutionStatus::Discarded.into(), }); } // 3: a non-executable transfer that fails loading the fee-payer // NOTE when we support the processed/executed distinction, this is NOT processed { - test_entry.transaction_batch.push(TransactionBatchItem { - transaction: system_transaction::transfer( + test_entry.push_transaction_with_status( + system_transaction::transfer( &Keypair::new(), &Pubkey::new_unique(), transfer_amount, Hash::default(), ), - asserts: TransactionBatchItemAsserts::not_executed(), - ..TransactionBatchItem::default() - }); + ExecutionStatus::Discarded, + ); } // 4: a non-executable transfer that fails loading the program @@ -573,16 +615,15 @@ fn simple_transfer() -> Vec { system_instruction::transfer(&source, &Pubkey::new_unique(), transfer_amount); instruction.program_id = Pubkey::new_unique(); - test_entry.transaction_batch.push(TransactionBatchItem { - transaction: Transaction::new_signed_with_payer( + test_entry.push_transaction_with_status( + Transaction::new_signed_with_payer( &[instruction], Some(&source), &[&source_keypair], Hash::default(), ), - asserts: TransactionBatchItemAsserts::not_executed(), - ..TransactionBatchItem::default() - }); + ExecutionStatus::Discarded, + ); } vec![test_entry] @@ -678,18 +719,22 @@ fn simple_nonce_fee_only(enable_fee_only_transactions: bool) -> Vec Vec Vec test_item_asserts .check_executed_transaction(&executed_transaction.execution_details), - // HANA we have to expand our semantics, there are four cases now - // succeeded, failed-executed, failed-processed, no-op - Ok(ProcessedTransaction::FeesOnly(_)) => assert!(!test_item_asserts.succeeded), - Err(_) => assert!(!test_item_asserts.executed), + Ok(ProcessedTransaction::FeesOnly(_)) => { + assert!(test_item_asserts.processed()); + assert!(!test_item_asserts.executed()); + } + Err(_) => assert!(test_item_asserts.discarded()), } } } From b459333db026f9d171fae69a5811b7ee674fe219 Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Wed, 28 Aug 2024 19:44:29 -0700 Subject: [PATCH 06/15] advance nonce in check transactions --- runtime/src/bank.rs | 7 ++-- runtime/src/bank/check_transactions.rs | 45 ++++++++++++++++++++------ svm/src/transaction_processor.rs | 20 ++---------- 3 files changed, 43 insertions(+), 29 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index e1316ee47803e4..968160be15cf90 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3015,8 +3015,11 @@ impl Bank { blockhash_queue.get_lamports_per_signature(message.recent_blockhash()) } .or_else(|| { - self.load_message_nonce_account(message) - .map(|(_nonce, nonce_data)| nonce_data.get_lamports_per_signature()) + self.load_message_nonce_account(message).map( + |(_nonce_address, _nonce_account, 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 d966d986fb8305..a7779fb3652f5c 100644 --- a/runtime/src/bank/check_transactions.rs +++ b/runtime/src/bank/check_transactions.rs @@ -3,13 +3,21 @@ use { solana_accounts_db::blockhash_queue::BlockhashQueue, solana_perf::perf_libs, solana_sdk::{ + account::AccountSharedData, + account_utils::StateMut, clock::{ MAX_PROCESSING_AGE, MAX_TRANSACTION_FORWARDING_DELAY, MAX_TRANSACTION_FORWARDING_DELAY_GPU, }, message::SanitizedMessage, - nonce::{self, state::DurableNonce, NONCED_TX_MARKER_IX_INDEX}, + nonce::{ + state::{ + Data as NonceData, DurableNonce, State as NonceState, Versions as NonceVersions, + }, + NONCED_TX_MARKER_IX_INDEX, + }, nonce_account, + pubkey::Pubkey, transaction::{Result as TransactionResult, SanitizedTransaction, TransactionError}, }, solana_svm::{ @@ -102,12 +110,12 @@ impl Bank { nonce: None, lamports_per_signature: hash_info.lamports_per_signature(), }) - } else if let Some((nonce, nonce_data)) = - self.check_and_load_message_nonce_account(tx.message(), next_durable_nonce) + } else if let Some((nonce, lamports_per_signature)) = + self.check_load_and_advance_message_nonce_account(tx.message(), next_durable_nonce) { Ok(CheckedTransactionDetails { nonce: Some(nonce), - lamports_per_signature: nonce_data.get_lamports_per_signature(), + lamports_per_signature, }) } else { error_counters.blockhash_not_found += 1; @@ -115,14 +123,33 @@ impl Bank { } } - pub(super) fn check_and_load_message_nonce_account( + pub(super) fn check_load_and_advance_message_nonce_account( &self, message: &SanitizedMessage, next_durable_nonce: &DurableNonce, - ) -> Option<(NonceInfo, nonce::state::Data)> { + ) -> Option<(NonceInfo, u64)> { let nonce_is_advanceable = message.recent_blockhash() != next_durable_nonce.as_hash(); if nonce_is_advanceable { - self.load_message_nonce_account(message) + if let Some((nonce_address, mut nonce_account, nonce_data)) = + self.load_message_nonce_account(message) + { + let lamports_per_signature = nonce_data.get_lamports_per_signature(); + let next_nonce_state = NonceState::new_initialized( + &nonce_data.authority, + *next_durable_nonce, + lamports_per_signature, + ); + nonce_account + .set_state(&NonceVersions::new(next_nonce_state)) + .ok()?; + + Some(( + NonceInfo::new(nonce_address, nonce_account), + lamports_per_signature, + )) + } else { + None + } } else { None } @@ -131,7 +158,7 @@ impl Bank { pub(super) fn load_message_nonce_account( &self, message: &SanitizedMessage, - ) -> Option<(NonceInfo, nonce::state::Data)> { + ) -> Option<(Pubkey, AccountSharedData, NonceData)> { let nonce_address = message.get_durable_nonce()?; let nonce_account = self.get_account_with_fixed_root(nonce_address)?; let nonce_data = @@ -144,7 +171,7 @@ impl Bank { return None; } - Some((NonceInfo::new(*nonce_address, nonce_account), nonce_data)) + Some((*nonce_address, nonce_account, nonce_data)) } fn check_status_cache( diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 48040f8c9907a4..2661e21a2cb282 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -42,8 +42,7 @@ use { fee::{FeeBudgetLimits, FeeStructure}, hash::Hash, inner_instruction::{InnerInstruction, InnerInstructionsList}, - instruction::{CompiledInstruction, InstructionError, TRANSACTION_LEVEL_STACK_HEIGHT}, - nonce::state::DurableNonce, + instruction::{CompiledInstruction, TRANSACTION_LEVEL_STACK_HEIGHT}, pubkey::Pubkey, rent_collector::RentCollector, saturating_add_assign, @@ -238,7 +237,6 @@ impl TransactionBatchProcessor { let mut error_metrics = TransactionErrorMetrics::default(); let mut execute_timings = ExecuteTimings::default(); - let durable_nonce = DurableNonce::from_blockhash(&environment.blockhash); let (validation_results, validate_fees_us) = measure_us!(self.validate_fees( callbacks, config.account_overrides, @@ -251,7 +249,6 @@ impl TransactionBatchProcessor { environment .rent_collector .unwrap_or(&RentCollector::default()), - &durable_nonce, &mut error_metrics )); @@ -373,7 +370,6 @@ impl TransactionBatchProcessor { } } - #[allow(clippy::too_many_arguments)] fn validate_fees( &self, callbacks: &CB, @@ -399,7 +395,6 @@ impl TransactionBatchProcessor { feature_set, fee_structure, rent_collector, - durable_nonce, error_counters, ) }) @@ -410,7 +405,6 @@ impl TransactionBatchProcessor { // Loads transaction fee payer, collects rent if necessary, then calculates // transaction fees, and deducts them from the fee payer balance. If the // account is not found or has insufficient funds, an error is returned. - #[allow(clippy::too_many_arguments)] fn validate_transaction_fee_payer( &self, callbacks: &CB, @@ -456,7 +450,7 @@ impl TransactionBatchProcessor { .rent_amount; let CheckedTransactionDetails { - mut nonce, + nonce, lamports_per_signature, } = checked_details; @@ -479,16 +473,6 @@ impl TransactionBatchProcessor { fee_details.total_fee(), )?; - if let Some(ref mut nonce_info) = nonce { - // Coding instruction 0 is appropriate because it must be the advance nonce instruction - // However the nonce info is guaranteed to be valid so this should never occur - nonce_info - .try_advance_nonce(*durable_nonce, lamports_per_signature) - .map_err(|_| { - TransactionError::InstructionError(0, InstructionError::InvalidAccountData) - })?; - }; - // Capture fee-subtracted fee payer account and next nonce account state // to commit if transaction execution fails. let rollback_accounts = RollbackAccounts::new( From fe278f306af570a2d940725fb88ca9f2c02e5b3b Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Thu, 29 Aug 2024 00:57:37 -0700 Subject: [PATCH 07/15] fix tests for new interfaces --- runtime/src/bank/check_transactions.rs | 19 +++++++++++++------ runtime/src/bank/tests.rs | 2 +- svm/src/transaction_processor.rs | 14 +++----------- svm/tests/integration_test.rs | 22 ++++++++++++++++++---- 4 files changed, 35 insertions(+), 22 deletions(-) diff --git a/runtime/src/bank/check_transactions.rs b/runtime/src/bank/check_transactions.rs index a7779fb3652f5c..41ac5c04620051 100644 --- a/runtime/src/bank/check_transactions.rs +++ b/runtime/src/bank/check_transactions.rs @@ -250,9 +250,16 @@ mod tests { )); let nonce_account = bank.get_account(&nonce_pubkey).unwrap(); let nonce_data = get_nonce_data_from_account(&nonce_account).unwrap(); + + let lamports_per_signature = nonce_data.get_lamports_per_signature(); + let mut expected_nonce_info = NonceInfo::new(nonce_pubkey, nonce_account); + expected_nonce_info + .try_advance_nonce(bank.next_durable_nonce(), lamports_per_signature) + .unwrap(); + assert_eq!( - bank.check_and_load_message_nonce_account(&message, &bank.next_durable_nonce()), - Some((NonceInfo::new(nonce_pubkey, nonce_account), nonce_data)) + bank.check_load_and_advance_message_nonce_account(&message, &bank.next_durable_nonce()), + Some((expected_nonce_info, lamports_per_signature)), ); } @@ -280,7 +287,7 @@ mod tests { &nonce_hash, )); assert!(bank - .check_and_load_message_nonce_account(&message, &bank.next_durable_nonce()) + .check_load_and_advance_message_nonce_account(&message, &bank.next_durable_nonce()) .is_none()); } @@ -309,7 +316,7 @@ mod tests { ); message.instructions[0].accounts.clear(); assert!(bank - .check_and_load_message_nonce_account( + .check_load_and_advance_message_nonce_account( &new_sanitized_message(message), &bank.next_durable_nonce(), ) @@ -342,7 +349,7 @@ mod tests { &nonce_hash, )); assert!(bank - .check_and_load_message_nonce_account(&message, &bank.next_durable_nonce()) + .check_load_and_advance_message_nonce_account(&message, &bank.next_durable_nonce()) .is_none()); } @@ -369,7 +376,7 @@ mod tests { &Hash::default(), )); assert!(bank - .check_and_load_message_nonce_account(&message, &bank.next_durable_nonce()) + .check_load_and_advance_message_nonce_account(&message, &bank.next_durable_nonce()) .is_none()); } } diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 56612762743f97..c570dea828b802 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -5733,7 +5733,7 @@ fn test_check_ro_durable_nonce_fails() { Err(TransactionError::BlockhashNotFound) ); assert_eq!( - bank.check_and_load_message_nonce_account( + bank.check_load_and_advance_message_nonce_account( &new_sanitized_message(tx.message().clone()), &bank.next_durable_nonce(), ), diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 2661e21a2cb282..e3792c3bef2790 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -224,6 +224,8 @@ impl TransactionBatchProcessor { self.sysvar_cache.read().unwrap() } + // XXX OK WHEN BACK need to fix svm integration and bank tests + /// Main entrypoint to the SVM. pub fn load_and_execute_sanitized_transactions( &self, @@ -1010,7 +1012,7 @@ mod tests { fee_calculator::FeeCalculator, hash::Hash, message::{LegacyMessage, Message, MessageHeader, SanitizedMessage}, - nonce, + nonce::{self, state::DurableNonce}, rent_collector::{RentCollector, RENT_EXEMPT_RENT_EPOCH}, rent_debits::RentDebits, reserved_account_keys::ReservedAccountKeys, @@ -1896,7 +1898,6 @@ mod tests { &FeatureSet::default(), &FeeStructure::default(), &rent_collector, - &DurableNonce::default(), &mut error_counters, ); @@ -1975,7 +1976,6 @@ mod tests { &FeatureSet::default(), &FeeStructure::default(), &rent_collector, - &DurableNonce::default(), &mut error_counters, ); @@ -2027,7 +2027,6 @@ mod tests { &FeatureSet::default(), &FeeStructure::default(), &RentCollector::default(), - &DurableNonce::default(), &mut error_counters, ); @@ -2062,7 +2061,6 @@ mod tests { &FeatureSet::default(), &FeeStructure::default(), &RentCollector::default(), - &DurableNonce::default(), &mut error_counters, ); @@ -2101,7 +2099,6 @@ mod tests { &FeatureSet::default(), &FeeStructure::default(), &rent_collector, - &DurableNonce::default(), &mut error_counters, ); @@ -2138,7 +2135,6 @@ mod tests { &FeatureSet::default(), &FeeStructure::default(), &RentCollector::default(), - &DurableNonce::default(), &mut error_counters, ); @@ -2171,7 +2167,6 @@ mod tests { &FeatureSet::default(), &FeeStructure::default(), &RentCollector::default(), - &DurableNonce::default(), &mut error_counters, ); @@ -2241,7 +2236,6 @@ mod tests { &feature_set, &FeeStructure::default(), &rent_collector, - &durable_nonce, &mut error_counters, ); @@ -2304,7 +2298,6 @@ mod tests { &feature_set, &FeeStructure::default(), &rent_collector, - &DurableNonce::default(), &mut error_counters, ); @@ -2359,7 +2352,6 @@ mod tests { &FeatureSet::default(), &FeeStructure::default(), &rent_collector, - &DurableNonce::default(), &mut error_counters, ); assert!( diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 99f5c2344c3712..32478ef9a129d5 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -45,6 +45,7 @@ const DEPLOYMENT_SLOT: u64 = 0; const EXECUTION_SLOT: u64 = 5; // The execution slot must be greater than the deployment slot const EXECUTION_EPOCH: u64 = 2; // The execution epoch must be greater than the deployment epoch const LAMPORTS_PER_SIGNATURE: u64 = 5000; +const LAST_BLOCKHASH: Hash = Hash::new_from_array([7; 32]); // Arbitrary constant hash for advancing nonces pub type AccountsMap = HashMap; @@ -137,17 +138,26 @@ impl SvmTestEntry { } // convenience function that adds a nonce transaction that is expected to succeed + // we accept the prior nonce state and advance it for the check status, since this happens before svm pub fn push_nonce_transaction(&mut self, transaction: Transaction, nonce_info: NonceInfo) { self.push_nonce_transaction_with_status(transaction, nonce_info, ExecutionStatus::Succeeded) } // convenience function that adds a nonce transaction with an expected execution status + // we accept the prior nonce state and advance it for the check status, since this happens before svm pub fn push_nonce_transaction_with_status( &mut self, transaction: Transaction, - nonce_info: NonceInfo, + mut nonce_info: NonceInfo, status: ExecutionStatus, ) { + nonce_info + .try_advance_nonce( + DurableNonce::from_blockhash(&LAST_BLOCKHASH), + LAMPORTS_PER_SIGNATURE, + ) + .unwrap(); + self.transaction_batch.push(TransactionBatchItem { transaction, asserts: TransactionBatchItemAsserts { @@ -696,10 +706,11 @@ fn simple_nonce_fee_only(enable_fee_only_transactions: bool) -> Vec Vec Vec Date: Thu, 29 Aug 2024 01:03:14 -0700 Subject: [PATCH 08/15] remove last blockhash timing and tx result mutator --- core/src/banking_stage/consume_worker.rs | 10 ---------- core/src/banking_stage/consumer.rs | 1 - core/src/banking_stage/leader_slot_timing_metrics.rs | 3 --- svm/src/transaction_processing_result.rs | 8 -------- svm/src/transaction_processor.rs | 2 -- 5 files changed, 24 deletions(-) diff --git a/core/src/banking_stage/consume_worker.rs b/core/src/banking_stage/consume_worker.rs index 3902ce8829f163..b676168bb04d4d 100644 --- a/core/src/banking_stage/consume_worker.rs +++ b/core/src/banking_stage/consume_worker.rs @@ -275,7 +275,6 @@ impl ConsumeWorkerMetrics { collect_balances_us, load_execute_us, freeze_lock_us, - last_blockhash_us, record_us, commit_us, find_and_send_votes_us, @@ -291,9 +290,6 @@ impl ConsumeWorkerMetrics { self.timing_metrics .freeze_lock_us .fetch_add(*freeze_lock_us, Ordering::Relaxed); - self.timing_metrics - .last_blockhash_us - .fetch_add(*last_blockhash_us, Ordering::Relaxed); self.timing_metrics .record_us .fetch_add(*record_us, Ordering::Relaxed); @@ -494,7 +490,6 @@ struct ConsumeWorkerTimingMetrics { collect_balances_us: AtomicU64, load_execute_us: AtomicU64, freeze_lock_us: AtomicU64, - last_blockhash_us: AtomicU64, record_us: AtomicU64, commit_us: AtomicU64, find_and_send_votes_us: AtomicU64, @@ -527,11 +522,6 @@ impl ConsumeWorkerTimingMetrics { self.freeze_lock_us.swap(0, Ordering::Relaxed), i64 ), - ( - "last_blockhash_us", - self.last_blockhash_us.swap(0, Ordering::Relaxed), - i64 - ), ("record_us", self.record_us.swap(0, Ordering::Relaxed), i64), ("commit_us", self.commit_us.swap(0, Ordering::Relaxed), i64), ( diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index e1f1b67fdb41a5..c3a1e777987dc2 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -665,7 +665,6 @@ impl Consumer { let (freeze_lock, freeze_lock_us) = measure_us!(bank.freeze_lock()); execute_and_commit_timings.freeze_lock_us = freeze_lock_us; - execute_and_commit_timings.last_blockhash_us = 0; let (record_transactions_summary, record_us) = measure_us!(self .transaction_recorder diff --git a/core/src/banking_stage/leader_slot_timing_metrics.rs b/core/src/banking_stage/leader_slot_timing_metrics.rs index 0de9296ce91aac..31b3dc0a24e7ca 100644 --- a/core/src/banking_stage/leader_slot_timing_metrics.rs +++ b/core/src/banking_stage/leader_slot_timing_metrics.rs @@ -10,7 +10,6 @@ pub struct LeaderExecuteAndCommitTimings { pub collect_balances_us: u64, pub load_execute_us: u64, pub freeze_lock_us: u64, - pub last_blockhash_us: u64, pub record_us: u64, pub commit_us: u64, pub find_and_send_votes_us: u64, @@ -23,7 +22,6 @@ impl LeaderExecuteAndCommitTimings { saturating_add_assign!(self.collect_balances_us, other.collect_balances_us); saturating_add_assign!(self.load_execute_us, other.load_execute_us); saturating_add_assign!(self.freeze_lock_us, other.freeze_lock_us); - saturating_add_assign!(self.last_blockhash_us, other.last_blockhash_us); saturating_add_assign!(self.record_us, other.record_us); saturating_add_assign!(self.commit_us, other.commit_us); saturating_add_assign!(self.find_and_send_votes_us, other.find_and_send_votes_us); @@ -40,7 +38,6 @@ impl LeaderExecuteAndCommitTimings { ("collect_balances_us", self.collect_balances_us as i64, i64), ("load_execute_us", self.load_execute_us as i64, i64), ("freeze_lock_us", self.freeze_lock_us as i64, i64), - ("last_blockhash_us", self.last_blockhash_us as i64, i64), ("record_us", self.record_us as i64, i64), ("commit_us", self.commit_us as i64, i64), ( diff --git a/svm/src/transaction_processing_result.rs b/svm/src/transaction_processing_result.rs index 7802b9ac213808..0658b5035fda0f 100644 --- a/svm/src/transaction_processing_result.rs +++ b/svm/src/transaction_processing_result.rs @@ -15,7 +15,6 @@ pub trait TransactionProcessingResultExtensions { fn was_processed(&self) -> bool; fn was_processed_with_successful_result(&self) -> bool; fn processed_transaction(&self) -> Option<&ProcessedTransaction>; - fn processed_transaction_mut(&mut self) -> Option<&mut ProcessedTransaction>; fn flattened_result(&self) -> TransactionResult<()>; } @@ -48,13 +47,6 @@ impl TransactionProcessingResultExtensions for TransactionProcessingResult { } } - fn processed_transaction_mut(&mut self) -> Option<&mut ProcessedTransaction> { - match self { - Ok(processed_tx) => Some(processed_tx), - Err(_) => None, - } - } - fn flattened_result(&self) -> TransactionResult<()> { self.as_ref() .map_err(|err| err.clone()) diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index e3792c3bef2790..91eb57478cec21 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -224,8 +224,6 @@ impl TransactionBatchProcessor { self.sysvar_cache.read().unwrap() } - // XXX OK WHEN BACK need to fix svm integration and bank tests - /// Main entrypoint to the SVM. pub fn load_and_execute_sanitized_transactions( &self, From 6a8efc3be792cddd123aee88225ab6a2f30d0cf3 Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Thu, 29 Aug 2024 01:13:38 -0700 Subject: [PATCH 09/15] change copy mut to proper accountshareddata function --- svm/src/rollback_accounts.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/svm/src/rollback_accounts.rs b/svm/src/rollback_accounts.rs index d16bfc66094237..cbb1903936d6a9 100644 --- a/svm/src/rollback_accounts.rs +++ b/svm/src/rollback_accounts.rs @@ -51,9 +51,7 @@ impl RollbackAccounts { if let Some(nonce) = nonce { if &fee_payer_address == nonce.address() { - fee_payer_account - .data_as_mut_slice() - .copy_from_slice(nonce.account().data()); + fee_payer_account.set_data_from_slice(nonce.account().data()); RollbackAccounts::SameNonceAndFeePayer { nonce: NonceInfo::new(fee_payer_address, fee_payer_account), From b598ac0a5186c6ed68cf5279a8f7e5e7967736b3 Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Fri, 30 Aug 2024 04:12:02 -0700 Subject: [PATCH 10/15] adddress feedback --- runtime/src/bank/check_transactions.rs | 66 +++++++++++++++----------- svm/src/rollback_accounts.rs | 6 ++- svm/src/transaction_processor.rs | 12 ++--- svm/tests/integration_test.rs | 52 ++++++++++++++------ svm/tests/mock_bank.rs | 3 +- 5 files changed, 88 insertions(+), 51 deletions(-) diff --git a/runtime/src/bank/check_transactions.rs b/runtime/src/bank/check_transactions.rs index 41ac5c04620051..2170933a259c96 100644 --- a/runtime/src/bank/check_transactions.rs +++ b/runtime/src/bank/check_transactions.rs @@ -129,30 +129,27 @@ impl Bank { next_durable_nonce: &DurableNonce, ) -> Option<(NonceInfo, u64)> { let nonce_is_advanceable = message.recent_blockhash() != next_durable_nonce.as_hash(); - if nonce_is_advanceable { - if let Some((nonce_address, mut nonce_account, nonce_data)) = - self.load_message_nonce_account(message) - { - let lamports_per_signature = nonce_data.get_lamports_per_signature(); - let next_nonce_state = NonceState::new_initialized( - &nonce_data.authority, - *next_durable_nonce, - lamports_per_signature, - ); - nonce_account - .set_state(&NonceVersions::new(next_nonce_state)) - .ok()?; - - Some(( - NonceInfo::new(nonce_address, nonce_account), - lamports_per_signature, - )) - } else { - None - } - } else { - None + if !nonce_is_advanceable { + return None; } + + let (nonce_address, mut nonce_account, nonce_data) = + self.load_message_nonce_account(message)?; + + let lamports_per_signature = self.get_lamports_per_signature(); + let next_nonce_state = NonceState::new_initialized( + &nonce_data.authority, + *next_durable_nonce, + lamports_per_signature, + ); + nonce_account + .set_state(&NonceVersions::new(next_nonce_state)) + .ok()?; + + Some(( + NonceInfo::new(nonce_address, nonce_account), + lamports_per_signature, + )) } pub(super) fn load_message_nonce_account( @@ -238,6 +235,7 @@ mod tests { .unwrap(); let custodian_pubkey = custodian_keypair.pubkey(); let nonce_pubkey = nonce_keypair.pubkey(); + let stale_lamports_per_signature = 42; let nonce_hash = get_nonce_blockhash(&bank, &nonce_pubkey).unwrap(); let message = new_sanitized_message(Message::new_with_blockhash( @@ -248,18 +246,32 @@ mod tests { Some(&custodian_pubkey), &nonce_hash, )); - let nonce_account = bank.get_account(&nonce_pubkey).unwrap(); + + // set a spurious lamports_per_signature value + let mut nonce_account = bank.get_account(&nonce_pubkey).unwrap(); let nonce_data = get_nonce_data_from_account(&nonce_account).unwrap(); + nonce_account + .set_state(&NonceVersions::new(NonceState::new_initialized( + &nonce_data.authority, + nonce_data.durable_nonce, + stale_lamports_per_signature, + ))) + .unwrap(); + bank.store_account(&nonce_pubkey, &nonce_account); - let lamports_per_signature = nonce_data.get_lamports_per_signature(); + let nonce_account = bank.get_account(&nonce_pubkey).unwrap(); + let current_lamports_per_signature = bank.get_lamports_per_signature(); let mut expected_nonce_info = NonceInfo::new(nonce_pubkey, nonce_account); expected_nonce_info - .try_advance_nonce(bank.next_durable_nonce(), lamports_per_signature) + .try_advance_nonce(bank.next_durable_nonce(), current_lamports_per_signature) .unwrap(); + // we now expect to: + // * advance the nonce to the current value + // * set the bank's lamports_per_signature in the nonce data assert_eq!( bank.check_load_and_advance_message_nonce_account(&message, &bank.next_durable_nonce()), - Some((expected_nonce_info, lamports_per_signature)), + Some((expected_nonce_info, current_lamports_per_signature)), ); } diff --git a/svm/src/rollback_accounts.rs b/svm/src/rollback_accounts.rs index cbb1903936d6a9..1a9a764e131186 100644 --- a/svm/src/rollback_accounts.rs +++ b/svm/src/rollback_accounts.rs @@ -51,6 +51,10 @@ impl RollbackAccounts { if let Some(nonce) = nonce { if &fee_payer_address == nonce.address() { + // `nonce` contains an AccountSharedData which has already been advanced to the current DurableNonce + // `fee_payer_account` is an AccountSharedData as it currently exists on-chain + // thus if the nonce account is being used as the fee payer, we need to update that data here + // so we capture both the data change for the nonce and the lamports/rent epoch change for the fee payer fee_payer_account.set_data_from_slice(nonce.account().data()); RollbackAccounts::SameNonceAndFeePayer { @@ -65,7 +69,7 @@ impl RollbackAccounts { } 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 + // rollback fee payer account'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. diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 91eb57478cec21..581edad62ac098 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -1010,7 +1010,7 @@ mod tests { fee_calculator::FeeCalculator, hash::Hash, message::{LegacyMessage, Message, MessageHeader, SanitizedMessage}, - nonce::{self, state::DurableNonce}, + nonce, rent_collector::{RentCollector, RENT_EXEMPT_RENT_EPOCH}, rent_debits::RentDebits, reserved_account_keys::ReservedAccountKeys, @@ -2216,12 +2216,10 @@ mod tests { let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); - let durable_nonce = DurableNonce::from_blockhash(&last_blockhash); - let mut nonce_info = NonceInfo::new(*fee_payer_address, fee_payer_account.clone()); - nonce_info - .try_advance_nonce(durable_nonce, lamports_per_signature) - .unwrap(); - let nonce = Some(nonce_info); + let nonce = Some(NonceInfo::new( + *fee_payer_address, + fee_payer_account.clone(), + )); let result = batch_processor.validate_transaction_fee_payer( &mock_bank, diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 32478ef9a129d5..44bc0ef8c50510 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -639,7 +639,10 @@ fn simple_transfer() -> Vec { vec![test_entry] } -fn simple_nonce_fee_only(enable_fee_only_transactions: bool) -> Vec { +fn simple_nonce_fee_only( + enable_fee_only_transactions: bool, + fee_paying_nonce: bool, +) -> Vec { let mut test_entry = SvmTestEntry::default(); if enable_fee_only_transactions { test_entry @@ -655,19 +658,31 @@ fn simple_nonce_fee_only(enable_fee_only_transactions: bool) -> Vec Vec Vec Vec Vec) { for test_entry in test_entries { execute_test_entry(test_entry); diff --git a/svm/tests/mock_bank.rs b/svm/tests/mock_bank.rs index fe7c2a604aeb5c..93045a82a7c448 100644 --- a/svm/tests/mock_bank.rs +++ b/svm/tests/mock_bank.rs @@ -233,7 +233,8 @@ pub fn create_executable_environment( .unwrap() .insert(Rent::id(), account_data); - // advance nonce asserts recent blockhashes is non-empty but then just gets the blockhash from ic + // SystemInstruction::AdvanceNonceAccount asserts RecentBlockhashes is non-empty + // but then just gets the blockhash from InvokeContext. so the sysvar doesnt need real entries #[allow(deprecated)] let recent_blockhashes = vec![BlockhashesEntry::default()]; From 585db2dfc6d82f04dbf85402071f500c44fbbc6b Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Fri, 30 Aug 2024 04:24:58 -0700 Subject: [PATCH 11/15] please clippy --- svm/tests/integration_test.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 44bc0ef8c50510..53d9d04f445183 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -680,7 +680,7 @@ fn simple_nonce_fee_only( fee_payer_data.set_lamports(LAMPORTS_PER_SOL); test_entry.add_initial_account(fee_payer, &fee_payer_data); } else if fee_paying_nonce { - nonce_balance += LAMPORTS_PER_SOL; + nonce_balance = nonce_balance.saturating_add(LAMPORTS_PER_SOL); } let nonce_initial_hash = DurableNonce::from_blockhash(&Hash::new_unique()); @@ -743,8 +743,8 @@ fn simple_nonce_fee_only( test_entry .final_accounts - .get_mut(nonce_info.address()) - .map(|a| a.set_rent_epoch(0)); + .entry(*nonce_info.address()) + .and_modify(|account| account.set_rent_epoch(0)); test_entry.push_nonce_transaction_with_status( transaction, From 0fa5a62f990b24e1b72bdf10764c1e3ecee48f07 Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Tue, 3 Sep 2024 15:48:35 -0700 Subject: [PATCH 12/15] fix merge issues --- runtime/src/account_saver.rs | 26 +++++++++----------------- runtime/src/bank.rs | 2 -- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/runtime/src/account_saver.rs b/runtime/src/account_saver.rs index 5e1040c6d97129..87bb1ac96c9284 100644 --- a/runtime/src/account_saver.rs +++ b/runtime/src/account_saver.rs @@ -1,8 +1,12 @@ use { core::borrow::Borrow, solana_sdk::{ +<<<<<<< HEAD account::AccountSharedData, nonce::state::DurableNonce, pubkey::Pubkey, transaction::SanitizedTransaction, transaction_context::TransactionAccount, +======= + account::AccountSharedData, pubkey::Pubkey, transaction_context::TransactionAccount, +>>>>>>> adb1a4159c (fix merge issues) }, solana_svm::{ rollback_accounts::RollbackAccounts, @@ -309,9 +313,7 @@ mod tests { let (collected_accounts, transactions) = collect_accounts_to_store( &txs, &transaction_refs, - &mut processing_results, - &DurableNonce::default(), - 0, + &processing_results, ); assert_eq!(collected_accounts.len(), 2); assert!(collected_accounts @@ -374,16 +376,13 @@ mod tests { )]; let max_collected_accounts = max_number_of_accounts_to_collect(&txs, &processing_results); assert_eq!(max_collected_accounts, 1); - let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique()); for collect_transactions in [false, true] { let transaction_refs = collect_transactions.then(|| txs.iter().collect::>()); let (collected_accounts, transactions) = collect_accounts_to_store( &txs, &transaction_refs, - &mut processing_results, - &durable_nonce, - 0, + &processing_results, ); assert_eq!(collected_accounts.len(), 1); assert_eq!( @@ -481,9 +480,7 @@ mod tests { let (collected_accounts, transactions) = collect_accounts_to_store( &txs, &transaction_refs, - &mut processing_results, - &durable_nonce, - 0, + &processing_results, ); assert_eq!(collected_accounts.len(), 2); assert_eq!( @@ -594,9 +591,7 @@ mod tests { let (collected_accounts, transactions) = collect_accounts_to_store( &txs, &transaction_refs, - &mut processing_results, - &durable_nonce, - 0, + &processing_results, ); assert_eq!(collected_accounts.len(), 1); let collected_nonce_account = collected_accounts @@ -649,16 +644,13 @@ mod tests { )))]; let max_collected_accounts = max_number_of_accounts_to_collect(&txs, &processing_results); assert_eq!(max_collected_accounts, 1); - let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique()); for collect_transactions in [false, true] { let transaction_refs = collect_transactions.then(|| txs.iter().collect::>()); let (collected_accounts, transactions) = collect_accounts_to_store( &txs, &transaction_refs, - &mut processing_results, - &durable_nonce, - 0, + &processing_results, ); assert_eq!(collected_accounts.len(), 1); assert_eq!( diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 968160be15cf90..9143efc4f38a3d 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3798,8 +3798,6 @@ impl Bank { } let ((), store_accounts_us) = measure_us!({ - let durable_nonce = DurableNonce::from_blockhash(&last_blockhash); - // If geyser is present, we must collect `SanitizedTransaction` // references in order to comply with that interface - until it // is changed. From a5fe3e18ba69ecd797e09017c5c660f23e694254 Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Tue, 3 Sep 2024 17:40:32 -0700 Subject: [PATCH 13/15] get lamports_per_signature from blockhash --- runtime/src/bank/check_transactions.rs | 60 ++++++++++++++++++-------- runtime/src/bank/tests.rs | 2 + 2 files changed, 45 insertions(+), 17 deletions(-) diff --git a/runtime/src/bank/check_transactions.rs b/runtime/src/bank/check_transactions.rs index 2170933a259c96..fd98d8c32c0f9b 100644 --- a/runtime/src/bank/check_transactions.rs +++ b/runtime/src/bank/check_transactions.rs @@ -79,6 +79,10 @@ 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 lamports_per_signature = hash_queue + .get_lamports_per_signature(&last_blockhash) + .unwrap(); sanitized_txs .iter() @@ -89,6 +93,7 @@ impl Bank { max_age, &next_durable_nonce, &hash_queue, + lamports_per_signature, error_counters, ), Err(e) => Err(e.clone()), @@ -102,6 +107,7 @@ impl Bank { max_age: usize, next_durable_nonce: &DurableNonce, hash_queue: &BlockhashQueue, + lamports_per_signature: u64, error_counters: &mut TransactionErrorMetrics, ) -> TransactionCheckResult { let recent_blockhash = tx.message().recent_blockhash(); @@ -110,9 +116,11 @@ impl Bank { nonce: None, lamports_per_signature: hash_info.lamports_per_signature(), }) - } else if let Some((nonce, lamports_per_signature)) = - self.check_load_and_advance_message_nonce_account(tx.message(), next_durable_nonce) - { + } else if let Some(nonce) = self.check_load_and_advance_message_nonce_account( + tx.message(), + next_durable_nonce, + lamports_per_signature, + ) { Ok(CheckedTransactionDetails { nonce: Some(nonce), lamports_per_signature, @@ -127,7 +135,8 @@ impl Bank { &self, message: &SanitizedMessage, next_durable_nonce: &DurableNonce, - ) -> Option<(NonceInfo, u64)> { + lamports_per_signature: u64, + ) -> Option { let nonce_is_advanceable = message.recent_blockhash() != next_durable_nonce.as_hash(); if !nonce_is_advanceable { return None; @@ -136,7 +145,6 @@ impl Bank { let (nonce_address, mut nonce_account, nonce_data) = self.load_message_nonce_account(message)?; - let lamports_per_signature = self.get_lamports_per_signature(); let next_nonce_state = NonceState::new_initialized( &nonce_data.authority, *next_durable_nonce, @@ -146,10 +154,7 @@ impl Bank { .set_state(&NonceVersions::new(next_nonce_state)) .ok()?; - Some(( - NonceInfo::new(nonce_address, nonce_account), - lamports_per_signature, - )) + Some(NonceInfo::new(nonce_address, nonce_account)) } pub(super) fn load_message_nonce_account( @@ -260,18 +265,22 @@ mod tests { bank.store_account(&nonce_pubkey, &nonce_account); let nonce_account = bank.get_account(&nonce_pubkey).unwrap(); - let current_lamports_per_signature = bank.get_lamports_per_signature(); + let (_, 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(), current_lamports_per_signature) + .try_advance_nonce(bank.next_durable_nonce(), lamports_per_signature) .unwrap(); // we now expect to: // * advance the nonce to the current value - // * set the bank's lamports_per_signature in the nonce data + // * set the blockhash queue's last blockhash's lamports_per_signature value in the nonce data assert_eq!( - bank.check_load_and_advance_message_nonce_account(&message, &bank.next_durable_nonce()), - Some((expected_nonce_info, current_lamports_per_signature)), + bank.check_load_and_advance_message_nonce_account( + &message, + &bank.next_durable_nonce(), + lamports_per_signature + ), + Some(expected_nonce_info), ); } @@ -298,8 +307,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()) + .check_load_and_advance_message_nonce_account( + &message, + &bank.next_durable_nonce(), + lamports_per_signature + ) .is_none()); } @@ -327,10 +341,12 @@ 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( &new_sanitized_message(message), &bank.next_durable_nonce(), + lamports_per_signature, ) .is_none()); } @@ -360,8 +376,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()) + .check_load_and_advance_message_nonce_account( + &message, + &bank.next_durable_nonce(), + lamports_per_signature + ) .is_none()); } @@ -387,8 +408,13 @@ 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()) + .check_load_and_advance_message_nonce_account( + &message, + &bank.next_durable_nonce(), + lamports_per_signature + ) .is_none()); } } diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index c570dea828b802..26f7cfb961b534 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -5732,10 +5732,12 @@ 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( &new_sanitized_message(tx.message().clone()), &bank.next_durable_nonce(), + lamports_per_signature, ), None ); From 3e633ec778d2a81896e404b0069446ccc6a0e419 Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Fri, 6 Sep 2024 01:12:03 -0700 Subject: [PATCH 14/15] revert niche lps change --- runtime/src/bank/check_transactions.rs | 47 +++++++++++++++----------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/runtime/src/bank/check_transactions.rs b/runtime/src/bank/check_transactions.rs index fd98d8c32c0f9b..0f0d70f15b07ab 100644 --- a/runtime/src/bank/check_transactions.rs +++ b/runtime/src/bank/check_transactions.rs @@ -80,7 +80,7 @@ impl Bank { 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 lamports_per_signature = hash_queue + let next_lamports_per_signature = hash_queue .get_lamports_per_signature(&last_blockhash) .unwrap(); @@ -93,7 +93,7 @@ impl Bank { max_age, &next_durable_nonce, &hash_queue, - lamports_per_signature, + next_lamports_per_signature, error_counters, ), Err(e) => Err(e.clone()), @@ -107,7 +107,7 @@ impl Bank { max_age: usize, next_durable_nonce: &DurableNonce, hash_queue: &BlockhashQueue, - lamports_per_signature: u64, + next_lamports_per_signature: u64, error_counters: &mut TransactionErrorMetrics, ) -> TransactionCheckResult { let recent_blockhash = tx.message().recent_blockhash(); @@ -116,14 +116,16 @@ impl Bank { nonce: None, lamports_per_signature: hash_info.lamports_per_signature(), }) - } else if let Some(nonce) = self.check_load_and_advance_message_nonce_account( - tx.message(), - next_durable_nonce, - lamports_per_signature, - ) { + } else if let Some((nonce, previous_lamports_per_signature)) = self + .check_load_and_advance_message_nonce_account( + tx.message(), + next_durable_nonce, + next_lamports_per_signature, + ) + { Ok(CheckedTransactionDetails { nonce: Some(nonce), - lamports_per_signature, + lamports_per_signature: previous_lamports_per_signature, }) } else { error_counters.blockhash_not_found += 1; @@ -135,8 +137,8 @@ impl Bank { &self, message: &SanitizedMessage, next_durable_nonce: &DurableNonce, - lamports_per_signature: u64, - ) -> Option { + next_lamports_per_signature: u64, + ) -> Option<(NonceInfo, u64)> { let nonce_is_advanceable = message.recent_blockhash() != next_durable_nonce.as_hash(); if !nonce_is_advanceable { return None; @@ -145,16 +147,20 @@ impl Bank { let (nonce_address, mut nonce_account, nonce_data) = self.load_message_nonce_account(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, - lamports_per_signature, + next_lamports_per_signature, ); nonce_account .set_state(&NonceVersions::new(next_nonce_state)) .ok()?; - Some(NonceInfo::new(nonce_address, nonce_account)) + Some(( + NonceInfo::new(nonce_address, nonce_account), + previous_lamports_per_signature, + )) } pub(super) fn load_message_nonce_account( @@ -229,6 +235,7 @@ mod tests { #[test] fn test_check_and_load_message_nonce_account_ok() { + const STALE_LAMPORTS_PER_SIGNATURE: u64 = 42; let (bank, _mint_keypair, custodian_keypair, nonce_keypair, _) = setup_nonce_with_bank( 10_000_000, |_| {}, @@ -240,7 +247,6 @@ mod tests { .unwrap(); let custodian_pubkey = custodian_keypair.pubkey(); let nonce_pubkey = nonce_keypair.pubkey(); - let stale_lamports_per_signature = 42; let nonce_hash = get_nonce_blockhash(&bank, &nonce_pubkey).unwrap(); let message = new_sanitized_message(Message::new_with_blockhash( @@ -259,28 +265,29 @@ mod tests { .set_state(&NonceVersions::new(NonceState::new_initialized( &nonce_data.authority, nonce_data.durable_nonce, - stale_lamports_per_signature, + STALE_LAMPORTS_PER_SIGNATURE, ))) .unwrap(); bank.store_account(&nonce_pubkey, &nonce_account); let nonce_account = bank.get_account(&nonce_pubkey).unwrap(); - let (_, lamports_per_signature) = bank.last_blockhash_and_lamports_per_signature(); + 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(), lamports_per_signature) + .try_advance_nonce(bank.next_durable_nonce(), next_lamports_per_signature) .unwrap(); // we now expect to: - // * advance the nonce to the current value + // * 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(), - lamports_per_signature + next_lamports_per_signature ), - Some(expected_nonce_info), + Some((expected_nonce_info, STALE_LAMPORTS_PER_SIGNATURE)), ); } From 3aa9d06acc00e7375633d5f86ef82bdf243c8e2c Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Tue, 10 Sep 2024 07:24:15 -0700 Subject: [PATCH 15/15] fix merge issues again --- runtime/src/account_saver.rs | 48 +++++++++++------------------------- 1 file changed, 14 insertions(+), 34 deletions(-) diff --git a/runtime/src/account_saver.rs b/runtime/src/account_saver.rs index 87bb1ac96c9284..941e4934175af3 100644 --- a/runtime/src/account_saver.rs +++ b/runtime/src/account_saver.rs @@ -1,12 +1,8 @@ use { core::borrow::Borrow, solana_sdk::{ -<<<<<<< HEAD - account::AccountSharedData, nonce::state::DurableNonce, pubkey::Pubkey, - transaction::SanitizedTransaction, transaction_context::TransactionAccount, -======= - account::AccountSharedData, pubkey::Pubkey, transaction_context::TransactionAccount, ->>>>>>> adb1a4159c (fix merge issues) + account::AccountSharedData, pubkey::Pubkey, transaction::SanitizedTransaction, + transaction_context::TransactionAccount, }, solana_svm::{ rollback_accounts::RollbackAccounts, @@ -55,7 +51,7 @@ fn max_number_of_accounts_to_collect( pub fn collect_accounts_to_store<'a, T: SVMMessage>( txs: &'a [T], txs_refs: &'a Option>>, - processing_results: &'a mut [TransactionProcessingResult], + processing_results: &'a [TransactionProcessingResult], ) -> ( Vec<(&'a Pubkey, &'a AccountSharedData)>, Option>, @@ -65,8 +61,7 @@ pub fn collect_accounts_to_store<'a, T: SVMMessage>( let mut transactions = txs_refs .is_some() .then(|| Vec::with_capacity(collect_capacity)); - for (index, (processing_result, transaction)) in - processing_results.iter_mut().zip(txs).enumerate() + for (index, (processing_result, transaction)) in processing_results.iter().zip(txs).enumerate() { let Some(processed_tx) = processing_result.processed_transaction() else { // Don't store any accounts if tx wasn't executed @@ -310,11 +305,8 @@ mod tests { for collect_transactions in [false, true] { let transaction_refs = collect_transactions.then(|| txs.iter().collect::>()); - let (collected_accounts, transactions) = collect_accounts_to_store( - &txs, - &transaction_refs, - &processing_results, - ); + let (collected_accounts, transactions) = + collect_accounts_to_store(&txs, &transaction_refs, &processing_results); assert_eq!(collected_accounts.len(), 2); assert!(collected_accounts .iter() @@ -379,11 +371,8 @@ mod tests { for collect_transactions in [false, true] { let transaction_refs = collect_transactions.then(|| txs.iter().collect::>()); - let (collected_accounts, transactions) = collect_accounts_to_store( - &txs, - &transaction_refs, - &processing_results, - ); + let (collected_accounts, transactions) = + collect_accounts_to_store(&txs, &transaction_refs, &processing_results); assert_eq!(collected_accounts.len(), 1); assert_eq!( collected_accounts @@ -477,11 +466,8 @@ mod tests { for collect_transactions in [false, true] { let transaction_refs = collect_transactions.then(|| txs.iter().collect::>()); - let (collected_accounts, transactions) = collect_accounts_to_store( - &txs, - &transaction_refs, - &processing_results, - ); + let (collected_accounts, transactions) = + collect_accounts_to_store(&txs, &transaction_refs, &processing_results); assert_eq!(collected_accounts.len(), 2); assert_eq!( collected_accounts @@ -588,11 +574,8 @@ mod tests { for collect_transactions in [false, true] { let transaction_refs = collect_transactions.then(|| txs.iter().collect::>()); - let (collected_accounts, transactions) = collect_accounts_to_store( - &txs, - &transaction_refs, - &processing_results, - ); + let (collected_accounts, transactions) = + collect_accounts_to_store(&txs, &transaction_refs, &processing_results); assert_eq!(collected_accounts.len(), 1); let collected_nonce_account = collected_accounts .iter() @@ -647,11 +630,8 @@ mod tests { for collect_transactions in [false, true] { let transaction_refs = collect_transactions.then(|| txs.iter().collect::>()); - let (collected_accounts, transactions) = collect_accounts_to_store( - &txs, - &transaction_refs, - &processing_results, - ); + let (collected_accounts, transactions) = + collect_accounts_to_store(&txs, &transaction_refs, &processing_results); assert_eq!(collected_accounts.len(), 1); assert_eq!( collected_accounts