From a420bd8d564f7277f92a30c61ba0727f8853ca3b Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Sat, 3 Aug 2024 04:07:59 +0000 Subject: [PATCH] feat: support committing fee-only transactions --- runtime/src/bank.rs | 142 ++++++++++++++--------- runtime/src/bank/tests.rs | 52 +++++---- sdk/src/feature_set.rs | 5 + svm/src/account_saver.rs | 88 ++++++++------ svm/src/rollback_accounts.rs | 16 +++ svm/src/transaction_execution_result.rs | 24 +--- svm/src/transaction_processing_result.rs | 61 +++++++++- svm/src/transaction_processor.rs | 23 ++-- svm/tests/conformance.rs | 3 +- svm/tests/integration_test.rs | 8 ++ 10 files changed, 274 insertions(+), 148 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 4fab9ce7405d5f..b1a315533f0754 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -159,7 +159,8 @@ use { }, transaction_processing_callback::TransactionProcessingCallback, transaction_processing_result::{ - TransactionProcessingResult, TransactionProcessingResultExtensions, + ProcessedTransaction, TransactionProcessingResult, + TransactionProcessingResultExtensions, }, transaction_processor::{ ExecutionRecordingConfig, TransactionBatchProcessor, TransactionLogMessages, @@ -3091,14 +3092,13 @@ impl Bank { assert_eq!(sanitized_txs.len(), processing_results.len()); for (tx, processing_result) in sanitized_txs.iter().zip(processing_results) { if let Ok(processed_tx) = &processing_result { - let details = &processed_tx.execution_details; // Add the message hash to the status cache to ensure that this message // won't be processed again with a different signature. status_cache.insert( tx.message().recent_blockhash(), tx.message_hash(), self.slot(), - details.status.clone(), + processed_tx.status(), ); // Add the transaction signature to the status cache so that transaction status // can be queried by transaction signature over RPC. In the future, this should @@ -3107,7 +3107,7 @@ impl Bank { tx.message().recent_blockhash(), tx.signature(), self.slot(), - details.status.clone(), + processed_tx.status(), ); } } @@ -3359,30 +3359,35 @@ impl Bank { let processing_result = processing_results .pop() .unwrap_or(Err(TransactionError::InvalidProgramForExecution)); - let flattened_result = processing_result.flattened_result(); - let (post_simulation_accounts, logs, return_data, inner_instructions) = + let (post_simulation_accounts, result, logs, return_data, inner_instructions) = match processing_result { - Ok(processed_tx) => { - let details = processed_tx.execution_details; - let post_simulation_accounts = processed_tx - .loaded_transaction - .accounts - .into_iter() - .take(number_of_accounts) - .collect::>(); - ( - post_simulation_accounts, - details.log_messages, - details.return_data, - details.inner_instructions, - ) - } - Err(_) => (vec![], None, None, None), + Ok(processed_tx) => match processed_tx { + ProcessedTransaction::Executed(executed_tx) => { + let details = executed_tx.execution_details; + let post_simulation_accounts = executed_tx + .loaded_transaction + .accounts + .into_iter() + .take(number_of_accounts) + .collect::>(); + ( + post_simulation_accounts, + details.status, + details.log_messages, + details.return_data, + details.inner_instructions, + ) + } + ProcessedTransaction::FeesOnly(fees_only_tx) => { + (vec![], Err(fees_only_tx.load_error), None, None, None) + } + }, + Err(error) => (vec![], Err(error), None, None, None), }; let logs = logs.unwrap_or_default(); TransactionSimulationResult { - result: flattened_result, + result, logs, post_simulation_accounts, units_consumed, @@ -3562,7 +3567,8 @@ impl Bank { .filter_map(|(processing_result, transaction)| { // Skip log collection for unprocessed transactions let processed_tx = processing_result.processed_transaction()?; - let execution_details = &processed_tx.execution_details; + // Skip log collection for unexecuted transactions + let execution_details = processed_tx.execution_details()?; Self::collect_transaction_logs( &transaction_log_collector_config, transaction, @@ -3712,7 +3718,7 @@ impl Bank { .iter() .for_each(|processing_result| match processing_result { Ok(processed_tx) => { - fees += processed_tx.loaded_transaction.fee_details.total_fee(); + fees += processed_tx.fee_details().total_fee(); } Err(_) => {} }); @@ -3731,8 +3737,7 @@ impl Bank { .iter() .for_each(|processing_result| match processing_result { Ok(processed_tx) => { - accumulated_fee_details - .accumulate(&processed_tx.loaded_transaction.fee_details); + accumulated_fee_details.accumulate(&processed_tx.fee_details()); } Err(_) => {} }); @@ -3805,7 +3810,9 @@ impl Bank { let ((), update_executors_us) = measure_us!({ let mut cache = None; for processing_result in &processing_results { - if let Some(executed_tx) = processing_result.processed_transaction() { + if let Some(ProcessedTransaction::Executed(executed_tx)) = + processing_result.processed_transaction() + { let programs_modified_by_tx = &executed_tx.programs_modified_by_tx; if executed_tx.was_successful() && !programs_modified_by_tx.is_empty() { cache @@ -3821,7 +3828,7 @@ impl Bank { let accounts_data_len_delta = processing_results .iter() .filter_map(|processing_result| processing_result.processed_transaction()) - .map(|processed_tx| &processed_tx.execution_details) + .filter_map(|processed_tx| processed_tx.execution_details()) .filter_map(|details| { details .status @@ -3859,37 +3866,52 @@ impl Bank { ) -> Vec { processing_results .into_iter() - .map(|processing_result| { - let processed_tx = processing_result?; - let execution_details = processed_tx.execution_details; - let LoadedTransaction { - rent_debits, - accounts: loaded_accounts, - loaded_accounts_data_size, - fee_details, - .. - } = processed_tx.loaded_transaction; - - // Rent is only collected for successfully executed transactions - let rent_debits = if execution_details.was_successful() { - rent_debits - } else { - RentDebits::default() - }; + .map(|processing_result| match processing_result? { + ProcessedTransaction::Executed(executed_tx) => { + let execution_details = executed_tx.execution_details; + let LoadedTransaction { + rent_debits, + accounts: loaded_accounts, + loaded_accounts_data_size, + fee_details, + .. + } = executed_tx.loaded_transaction; + + // Rent is only collected for successfully executed transactions + let rent_debits = if execution_details.was_successful() { + rent_debits + } else { + RentDebits::default() + }; - Ok(CommittedTransaction { - status: execution_details.status, - log_messages: execution_details.log_messages, - inner_instructions: execution_details.inner_instructions, - return_data: execution_details.return_data, - executed_units: execution_details.executed_units, - fee_details, - rent_debits, + Ok(CommittedTransaction { + status: execution_details.status, + log_messages: execution_details.log_messages, + inner_instructions: execution_details.inner_instructions, + return_data: execution_details.return_data, + executed_units: execution_details.executed_units, + fee_details, + rent_debits, + loaded_account_stats: TransactionLoadedAccountsStats { + loaded_accounts_count: loaded_accounts.len(), + loaded_accounts_data_size, + }, + }) + } + ProcessedTransaction::FeesOnly(fees_only_tx) => Ok(CommittedTransaction { + status: Err(fees_only_tx.load_error), + log_messages: None, + inner_instructions: None, + return_data: None, + executed_units: 0, + rent_debits: RentDebits::default(), + fee_details: fees_only_tx.fee_details, loaded_account_stats: TransactionLoadedAccountsStats { - loaded_accounts_count: loaded_accounts.len(), - loaded_accounts_data_size, + loaded_accounts_count: fees_only_tx.rollback_accounts.count(), + loaded_accounts_data_size: fees_only_tx.rollback_accounts.data_size() + as u32, }, - }) + }), }) .collect() } @@ -3898,6 +3920,7 @@ impl Bank { let collected_rent = processing_results .iter() .filter_map(|processing_result| processing_result.processed_transaction()) + .filter_map(|processed_tx| processed_tx.executed_transaction()) .filter(|executed_tx| executed_tx.was_successful()) .map(|executed_tx| executed_tx.loaded_transaction.rent) .sum(); @@ -5845,6 +5868,11 @@ impl Bank { .processed_transaction() .map(|processed_tx| (tx, processed_tx)) }) + .filter_map(|(tx, processed_tx)| { + processed_tx + .executed_transaction() + .map(|executed_tx| (tx, executed_tx)) + }) .filter(|(_, executed_tx)| executed_tx.was_successful()) .flat_map(|(tx, executed_tx)| { let num_account_keys = tx.message().account_keys().len(); diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 85d51a10f8e2e9..b7168f6bc1f13b 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -236,21 +236,23 @@ fn new_processing_result( status: Result<()>, fee_details: FeeDetails, ) -> TransactionProcessingResult { - Ok(ExecutedTransaction { - loaded_transaction: LoadedTransaction { - fee_details, - ..LoadedTransaction::default() - }, - execution_details: TransactionExecutionDetails { - status, - log_messages: None, - inner_instructions: None, - return_data: None, - executed_units: 0, - accounts_data_len_delta: 0, + Ok(ProcessedTransaction::Executed(Box::new( + ExecutedTransaction { + loaded_transaction: LoadedTransaction { + fee_details, + ..LoadedTransaction::default() + }, + execution_details: TransactionExecutionDetails { + status, + log_messages: None, + inner_instructions: None, + return_data: None, + executed_units: 0, + accounts_data_len_delta: 0, + }, + programs_modified_by_tx: HashMap::new(), }, - programs_modified_by_tx: HashMap::new(), - }) + ))) } impl Bank { @@ -6930,6 +6932,12 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { assert!(slot_versions.is_empty()); } + // Advance bank to get a new last blockhash for duplicate transaction below + goto_end_of_slot(bank.clone()); + let bank = bank_client + .advance_slot(1, bank_forks.as_ref(), &mint_keypair.pubkey()) + .unwrap(); + // Load program file let mut file = File::open("../programs/bpf_loader/test_elfs/out/noop_aligned.so") .expect("file open failed"); @@ -6995,8 +7003,8 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { let program_cache = bank.transaction_processor.program_cache.read().unwrap(); let slot_versions = program_cache.get_slot_versions_for_tests(&program_keypair.pubkey()); assert_eq!(slot_versions.len(), 1); - assert_eq!(slot_versions[0].deployment_slot, 0); - assert_eq!(slot_versions[0].effective_slot, 0); + assert_eq!(slot_versions[0].deployment_slot, bank.slot()); + assert_eq!(slot_versions[0].effective_slot, bank.slot()); assert!(matches!( slot_versions[0].program, ProgramCacheEntryType::Closed, @@ -7019,8 +7027,8 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { let program_cache = bank.transaction_processor.program_cache.read().unwrap(); let slot_versions = program_cache.get_slot_versions_for_tests(&buffer_address); assert_eq!(slot_versions.len(), 1); - assert_eq!(slot_versions[0].deployment_slot, 0); - assert_eq!(slot_versions[0].effective_slot, 0); + assert_eq!(slot_versions[0].deployment_slot, bank.slot()); + assert_eq!(slot_versions[0].effective_slot, bank.slot()); assert!(matches!( slot_versions[0].program, ProgramCacheEntryType::Closed, @@ -7121,14 +7129,14 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { let program_cache = bank.transaction_processor.program_cache.read().unwrap(); let slot_versions = program_cache.get_slot_versions_for_tests(&program_keypair.pubkey()); assert_eq!(slot_versions.len(), 2); - assert_eq!(slot_versions[0].deployment_slot, 0); - assert_eq!(slot_versions[0].effective_slot, 0); + assert_eq!(slot_versions[0].deployment_slot, bank.slot() - 1); + assert_eq!(slot_versions[0].effective_slot, bank.slot() - 1); assert!(matches!( slot_versions[0].program, ProgramCacheEntryType::Closed, )); - assert_eq!(slot_versions[1].deployment_slot, 0); - assert_eq!(slot_versions[1].effective_slot, 1); + assert_eq!(slot_versions[1].deployment_slot, bank.slot() - 1); + assert_eq!(slot_versions[1].effective_slot, bank.slot()); assert!(matches!( slot_versions[1].program, ProgramCacheEntryType::Loaded(_), diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 4626240a949de4..621b63c2c76fca 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -841,6 +841,10 @@ pub mod vote_only_retransmitter_signed_fec_sets { solana_sdk::declare_id!("RfEcA95xnhuwooVAhUUksEJLZBF7xKCLuqrJoqk4Zph"); } +pub mod enable_transaction_failure_fees { + solana_sdk::declare_id!("PaymEPK2oqwT9TXAVfadjztH2H6KfLEB9Hhd5Q5frvP"); +} + pub mod enable_turbine_extended_fanout_experiments { solana_sdk::declare_id!("BZn14Liea52wtBwrXUxTv6vojuTTmfc7XGEDTXrvMD7b"); } @@ -1050,6 +1054,7 @@ lazy_static! { (move_stake_and_move_lamports_ixs::id(), "Enable MoveStake and MoveLamports stake program instructions #1610"), (ed25519_precompile_verify_strict::id(), "Use strict verification in ed25519 precompile SIMD-0152"), (vote_only_retransmitter_signed_fec_sets::id(), "vote only on retransmitter signed fec sets"), + (enable_transaction_failure_fees::id(), "Enable fees for some additional transaction failures SIMD-0082"), (enable_turbine_extended_fanout_experiments::id(), "enable turbine extended fanout experiments #"), /*************** ADD NEW FEATURES HERE ***************/ ] diff --git a/svm/src/account_saver.rs b/svm/src/account_saver.rs index 2657e7a7cb9717..eaf2ff1303d0d3 100644 --- a/svm/src/account_saver.rs +++ b/svm/src/account_saver.rs @@ -2,7 +2,8 @@ use { crate::{ rollback_accounts::RollbackAccounts, transaction_processing_result::{ - TransactionProcessingResult, TransactionProcessingResultExtensions, + ProcessedTransaction, TransactionProcessingResult, + TransactionProcessingResultExtensions, }, }, solana_sdk::{ @@ -26,12 +27,15 @@ fn max_number_of_accounts_to_collect( .processed_transaction() .map(|processed_tx| (processed_tx, tx)) }) - .map( - |(processed_tx, tx)| match processed_tx.execution_details.status { - Ok(_) => tx.message().num_write_locks() as usize, - Err(_) => processed_tx.loaded_transaction.rollback_accounts.count(), - }, - ) + .map(|(processed_tx, tx)| match processed_tx { + ProcessedTransaction::Executed(executed_tx) => { + match executed_tx.execution_details.status { + Ok(_) => tx.message().num_write_locks() as usize, + Err(_) => executed_tx.loaded_transaction.rollback_accounts.count(), + } + } + ProcessedTransaction::FeesOnly(fees_only_tx) => fees_only_tx.rollback_accounts.count(), + }) .sum() } @@ -53,22 +57,36 @@ pub fn collect_accounts_to_store<'a>( continue; }; - if processed_tx.execution_details.status.is_ok() { - collect_accounts_for_successful_tx( - &mut accounts, - &mut transactions, - transaction, - &processed_tx.loaded_transaction.accounts, - ); - } else { - collect_accounts_for_failed_tx( - &mut accounts, - &mut transactions, - transaction, - &mut processed_tx.loaded_transaction.rollback_accounts, - durable_nonce, - lamports_per_signature, - ); + match processed_tx { + ProcessedTransaction::Executed(executed_tx) => { + if executed_tx.execution_details.status.is_ok() { + collect_accounts_for_successful_tx( + &mut accounts, + &mut transactions, + transaction, + &executed_tx.loaded_transaction.accounts, + ); + } else { + collect_accounts_for_failed_tx( + &mut accounts, + &mut transactions, + transaction, + &mut executed_tx.loaded_transaction.rollback_accounts, + durable_nonce, + lamports_per_signature, + ); + } + } + ProcessedTransaction::FeesOnly(fees_only_tx) => { + collect_accounts_for_failed_tx( + &mut accounts, + &mut transactions, + transaction, + &mut fees_only_tx.rollback_accounts, + durable_nonce, + lamports_per_signature, + ); + } } } (accounts, transactions) @@ -186,18 +204,20 @@ mod tests { status: Result<()>, loaded_transaction: LoadedTransaction, ) -> TransactionProcessingResult { - Ok(ExecutedTransaction { - execution_details: TransactionExecutionDetails { - status, - log_messages: None, - inner_instructions: None, - return_data: None, - executed_units: 0, - accounts_data_len_delta: 0, + Ok(ProcessedTransaction::Executed(Box::new( + ExecutedTransaction { + execution_details: TransactionExecutionDetails { + status, + log_messages: None, + inner_instructions: None, + return_data: None, + executed_units: 0, + accounts_data_len_delta: 0, + }, + loaded_transaction, + programs_modified_by_tx: HashMap::new(), }, - loaded_transaction, - programs_modified_by_tx: HashMap::new(), - }) + ))) } #[test] diff --git a/svm/src/rollback_accounts.rs b/svm/src/rollback_accounts.rs index 71b670d37c4f85..c2c02f2f80bd43 100644 --- a/svm/src/rollback_accounts.rs +++ b/svm/src/rollback_accounts.rs @@ -79,6 +79,22 @@ impl RollbackAccounts { Self::SeparateNonceAndFeePayer { .. } => 2, } } + + /// Size of accounts tracked for rollback, used when calculating the actual + /// cost of transaction processing in the cost model. + pub fn data_size(&self) -> usize { + match self { + Self::FeePayerOnly { fee_payer_account } => fee_payer_account.data().len(), + Self::SameNonceAndFeePayer { nonce } => nonce.account().data().len(), + Self::SeparateNonceAndFeePayer { + nonce, + fee_payer_account, + } => fee_payer_account + .data() + .len() + .saturating_add(nonce.account().data().len()), + } + } } #[cfg(test)] diff --git a/svm/src/transaction_execution_result.rs b/svm/src/transaction_execution_result.rs index 6a41294ddb975e..4be551de4e223d 100644 --- a/svm/src/transaction_execution_result.rs +++ b/svm/src/transaction_execution_result.rs @@ -7,11 +7,7 @@ pub use solana_sdk::inner_instruction::{InnerInstruction, InnerInstructionsList} use { crate::account_loader::LoadedTransaction, solana_program_runtime::loaded_programs::ProgramCacheEntry, - solana_sdk::{ - pubkey::Pubkey, - transaction::{self, TransactionError}, - transaction_context::TransactionReturnData, - }, + solana_sdk::{pubkey::Pubkey, transaction, transaction_context::TransactionReturnData}, std::{collections::HashMap, sync::Arc}, }; @@ -21,22 +17,6 @@ pub struct TransactionLoadedAccountsStats { pub loaded_accounts_count: usize, } -/// Type safe representation of a transaction execution attempt which -/// differentiates between a transaction that was executed (will be -/// committed to the ledger) and a transaction which wasn't executed -/// and will be dropped. -/// -/// Note: `Result` is not -/// used because it's easy to forget that the inner `details.status` field -/// is what should be checked to detect a successful transaction. This -/// enum provides a convenience method `Self::was_executed_successfully` to -/// make such checks hard to do incorrectly. -#[derive(Debug, Clone)] -pub enum TransactionExecutionResult { - Executed(Box), - NotExecuted(TransactionError), -} - #[derive(Debug, Clone)] pub struct ExecutedTransaction { pub loaded_transaction: LoadedTransaction, @@ -46,7 +26,7 @@ pub struct ExecutedTransaction { impl ExecutedTransaction { pub fn was_successful(&self) -> bool { - self.execution_details.status.is_ok() + self.execution_details.was_successful() } } diff --git a/svm/src/transaction_processing_result.rs b/svm/src/transaction_processing_result.rs index 0ad68e0d18a803..d06efe25d540a1 100644 --- a/svm/src/transaction_processing_result.rs +++ b/svm/src/transaction_processing_result.rs @@ -1,10 +1,15 @@ use { - crate::transaction_execution_result::ExecutedTransaction, - solana_sdk::transaction::Result as TransactionResult, + crate::{ + account_loader::FeesOnlyTransaction, + transaction_execution_result::{ExecutedTransaction, TransactionExecutionDetails}, + }, + solana_sdk::{ + fee::FeeDetails, + transaction::{Result as TransactionResult, TransactionError}, + }, }; pub type TransactionProcessingResult = TransactionResult; -pub type ProcessedTransaction = ExecutedTransaction; pub trait TransactionProcessingResultExtensions { fn was_processed(&self) -> bool; @@ -14,6 +19,15 @@ pub trait TransactionProcessingResultExtensions { fn flattened_result(&self) -> TransactionResult<()>; } +pub enum ProcessedTransaction { + /// Transaction was executed, but if execution failed, all account state changes + /// will be rolled back except deducted fees and any advanced nonces + Executed(Box), + /// Transaction was not able to be executed but fees are able to be + /// collected and any nonces are advanceable + FeesOnly(Box), +} + impl TransactionProcessingResultExtensions for TransactionProcessingResult { fn was_processed(&self) -> bool { self.is_ok() @@ -21,7 +35,7 @@ impl TransactionProcessingResultExtensions for TransactionProcessingResult { fn was_processed_with_successful_result(&self) -> bool { match self { - Ok(processed_tx) => processed_tx.was_successful(), + Ok(processed_tx) => processed_tx.was_processed_with_successful_result(), Err(_) => false, } } @@ -43,6 +57,43 @@ impl TransactionProcessingResultExtensions for TransactionProcessingResult { fn flattened_result(&self) -> TransactionResult<()> { self.as_ref() .map_err(|err| err.clone()) - .and_then(|processed_tx| processed_tx.execution_details.status.clone()) + .and_then(|processed_tx| processed_tx.status()) + } +} + +impl ProcessedTransaction { + fn was_processed_with_successful_result(&self) -> bool { + match self { + Self::Executed(executed_tx) => executed_tx.execution_details.status.is_ok(), + Self::FeesOnly(_) => false, + } + } + + pub fn status(&self) -> TransactionResult<()> { + match self { + Self::Executed(executed_tx) => executed_tx.execution_details.status.clone(), + Self::FeesOnly(details) => Err(TransactionError::clone(&details.load_error)), + } + } + + pub fn fee_details(&self) -> FeeDetails { + match self { + Self::Executed(executed_tx) => executed_tx.loaded_transaction.fee_details, + Self::FeesOnly(details) => details.fee_details, + } + } + + pub fn executed_transaction(&self) -> Option<&ExecutedTransaction> { + match self { + Self::Executed(context) => Some(context), + Self::FeesOnly { .. } => None, + } + } + + pub fn execution_details(&self) -> Option<&TransactionExecutionDetails> { + match self { + Self::Executed(context) => Some(&context.execution_details), + Self::FeesOnly { .. } => None, + } } } diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index b43fb3429557e4..6ce1a48df3e4a5 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -15,7 +15,7 @@ use { transaction_error_metrics::TransactionErrorMetrics, transaction_execution_result::{ExecutedTransaction, TransactionExecutionDetails}, transaction_processing_callback::TransactionProcessingCallback, - transaction_processing_result::TransactionProcessingResult, + transaction_processing_result::{ProcessedTransaction, TransactionProcessingResult}, }, log::debug, percentage::Percentage, @@ -36,7 +36,7 @@ use { solana_sdk::{ account::{AccountSharedData, ReadableAccount, PROGRAM_OWNERS}, clock::{Epoch, Slot}, - feature_set::{remove_rounding_in_fee_calculation, FeatureSet}, + feature_set::{self, remove_rounding_in_fee_calculation, FeatureSet}, fee::{FeeBudgetLimits, FeeStructure}, hash::Hash, inner_instruction::{InnerInstruction, InnerInstructionsList}, @@ -268,12 +268,12 @@ impl TransactionBatchProcessor { ); if program_cache_for_tx_batch.hit_max_limit { - const ERROR: TransactionError = TransactionError::ProgramCacheHitMaxLimit; - let processing_results = vec![Err(ERROR); sanitized_txs.len()]; return LoadAndExecuteSanitizedTransactionsOutput { error_metrics, execute_timings, - processing_results, + processing_results: (0..sanitized_txs.len()) + .map(|_| Err(TransactionError::ProgramCacheHitMaxLimit)) + .collect(), }; } @@ -293,13 +293,22 @@ impl TransactionBatchProcessor { &program_cache_for_tx_batch, )); + let enable_transaction_failure_fees = environment + .feature_set + .is_active(&feature_set::enable_transaction_failure_fees::id()); let (processing_results, execution_us): (Vec, u64) = measure_us!(loaded_transactions .into_iter() .zip(sanitized_txs.iter()) .map(|(load_result, tx)| match load_result { TransactionLoadResult::NotLoaded(err) => Err(err), - TransactionLoadResult::FeesOnly(fees_only_tx) => Err(fees_only_tx.load_error), + TransactionLoadResult::FeesOnly(fees_only_tx) => { + if enable_transaction_failure_fees { + Ok(ProcessedTransaction::FeesOnly(Box::new(fees_only_tx))) + } else { + Err(fees_only_tx.load_error) + } + } TransactionLoadResult::Loaded(loaded_transaction) => { let executed_tx = self.execute_loaded_transaction( tx, @@ -317,7 +326,7 @@ impl TransactionBatchProcessor { program_cache_for_tx_batch.merge(&executed_tx.programs_modified_by_tx); } - Ok(executed_tx) + Ok(ProcessedTransaction::Executed(Box::new(executed_tx))) } }) .collect()); diff --git a/svm/tests/conformance.rs b/svm/tests/conformance.rs index 5b32c2e164c2d2..5015e1fadd9d0c 100644 --- a/svm/tests/conformance.rs +++ b/svm/tests/conformance.rs @@ -336,9 +336,10 @@ fn run_fixture(fixture: InstrFixture, filename: OsString, execute_as_instr: bool return; } - let executed_tx = result.processing_results[0] + let processed_tx = result.processing_results[0] .processed_transaction() .unwrap(); + let executed_tx = processed_tx.executed_transaction().unwrap(); let execution_details = &executed_tx.execution_details; let loaded_accounts = &executed_tx.loaded_transaction.accounts; verify_accounts_and_data( diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 5070bca08e0907..deac83f2d440c0 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -433,6 +433,8 @@ fn svm_integration() { let executed_tx_0 = result.processing_results[0] .processed_transaction() + .unwrap() + .executed_transaction() .unwrap(); assert!(executed_tx_0.was_successful()); let logs = executed_tx_0 @@ -444,6 +446,8 @@ fn svm_integration() { let executed_tx_1 = result.processing_results[1] .processed_transaction() + .unwrap() + .executed_transaction() .unwrap(); assert!(executed_tx_1.was_successful()); @@ -459,6 +463,8 @@ fn svm_integration() { let executed_tx_2 = result.processing_results[2] .processed_transaction() + .unwrap() + .executed_transaction() .unwrap(); let return_data = executed_tx_2 .execution_details @@ -472,6 +478,8 @@ fn svm_integration() { let executed_tx_3 = result.processing_results[3] .processed_transaction() + .unwrap() + .executed_transaction() .unwrap(); assert!(executed_tx_3.execution_details.status.is_err()); assert!(executed_tx_3