diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs index 6fdccb9224..2fd74d6a8b 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -804,22 +804,23 @@ impl BankingStage { fn record_transactions( bank_slot: Slot, txs: &[SanitizedTransaction], - results: &[TransactionExecutionResult], + execution_results: &[TransactionExecutionResult], recorder: &TransactionRecorder, ) -> (Result, Vec) { let mut processed_generation = Measure::start("record::process_generation"); - let (processed_transactions, processed_transactions_indexes): (Vec<_>, Vec<_>) = results - .iter() - .zip(txs) - .enumerate() - .filter_map(|(i, ((r, _n), tx))| { - if Bank::can_commit(r) { - Some((tx.to_versioned_transaction(), i)) - } else { - None - } - }) - .unzip(); + let (processed_transactions, processed_transactions_indexes): (Vec<_>, Vec<_>) = + execution_results + .iter() + .zip(txs) + .enumerate() + .filter_map(|(i, (execution_result, tx))| { + if execution_result.was_executed() { + Some((tx.to_versioned_transaction(), i)) + } else { + None + } + }) + .unzip(); processed_generation.stop(); let num_to_commit = processed_transactions.len(); @@ -885,28 +886,25 @@ impl BankingStage { }; let mut execute_timings = ExecuteTimings::default(); - let ( - mut loaded_accounts, - results, - inner_instructions, - transaction_logs, - mut retryable_txs, - tx_count, - signature_count, - ) = bank.load_and_execute_transactions( - batch, - MAX_PROCESSING_AGE, - transaction_status_sender.is_some(), - transaction_status_sender.is_some(), - &mut execute_timings, - ); + let (mut loaded_accounts, execution_results, mut retryable_txs, tx_count, signature_count) = + bank.load_and_execute_transactions( + batch, + MAX_PROCESSING_AGE, + transaction_status_sender.is_some(), + transaction_status_sender.is_some(), + &mut execute_timings, + ); load_execute_time.stop(); let freeze_lock = bank.freeze_lock(); let mut record_time = Measure::start("record_time"); - let (num_to_commit, retryable_record_txs) = - Self::record_transactions(bank.slot(), batch.sanitized_transactions(), &results, poh); + let (num_to_commit, retryable_record_txs) = Self::record_transactions( + bank.slot(), + batch.sanitized_transactions(), + &execution_results, + poh, + ); inc_new_counter_info!( "banking_stage-record_transactions_num_to_commit", *num_to_commit.as_ref().unwrap_or(&0) @@ -928,7 +926,7 @@ impl BankingStage { let tx_results = bank.commit_transactions( sanitized_txs, &mut loaded_accounts, - &results, + execution_results, tx_count, signature_count, &mut execute_timings, @@ -945,8 +943,6 @@ impl BankingStage { tx_results.execution_results, TransactionBalancesSet::new(pre_balances, post_balances), TransactionTokenBalancesSet::new(pre_token_balances, post_token_balances), - inner_instructions, - transaction_logs, tx_results.rent_debits, ); } @@ -1609,6 +1605,7 @@ mod tests { poh_service::PohService, }, solana_rpc::transaction_status_service::TransactionStatusService, + solana_runtime::bank::TransactionExecutionDetails, solana_sdk::{ hash::Hash, instruction::InstructionError, @@ -1640,6 +1637,15 @@ mod tests { ) } + fn new_execution_result(status: Result<(), TransactionError>) -> TransactionExecutionResult { + TransactionExecutionResult::Executed(TransactionExecutionDetails { + status, + log_messages: None, + inner_instructions: None, + durable_nonce_fee: None, + }) + } + #[test] fn test_banking_stage_shutdown1() { let genesis_config = create_genesis_config(2).genesis_config; @@ -2027,19 +2033,16 @@ mod tests { system_transaction::transfer(&keypair2, &pubkey2, 1, genesis_config.hash()), ]); - let mut results = vec![(Ok(()), None), (Ok(()), None)]; + let mut results = vec![new_execution_result(Ok(())); 2]; let _ = BankingStage::record_transactions(bank.slot(), &txs, &results, &recorder); let (_bank, (entry, _tick_height)) = entry_receiver.recv().unwrap(); assert_eq!(entry.transactions.len(), txs.len()); // InstructionErrors should still be recorded - results[0] = ( - Err(TransactionError::InstructionError( - 1, - SystemError::ResultWithNegativeLamports.into(), - )), - None, - ); + results[0] = new_execution_result(Err(TransactionError::InstructionError( + 1, + SystemError::ResultWithNegativeLamports.into(), + ))); let (res, retryable) = BankingStage::record_transactions(bank.slot(), &txs, &results, &recorder); res.unwrap(); @@ -2048,7 +2051,7 @@ mod tests { assert_eq!(entry.transactions.len(), txs.len()); // Other TransactionErrors should not be recorded - results[0] = (Err(TransactionError::AccountNotFound), None); + results[0] = TransactionExecutionResult::NotExecuted(TransactionError::AccountNotFound); let (res, retryable) = BankingStage::record_transactions(bank.slot(), &txs, &results, &recorder); res.unwrap(); diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 477bc456af..e16053d15b 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -20,8 +20,8 @@ use { accounts_index::AccountSecondaryIndexes, accounts_update_notifier_interface::AccountsUpdateNotifier, bank::{ - Bank, ExecuteTimings, InnerInstructionsList, RentDebits, TransactionBalancesSet, - TransactionExecutionResult, TransactionLogMessages, TransactionResults, + Bank, ExecuteTimings, RentDebits, TransactionBalancesSet, TransactionExecutionResult, + TransactionResults, }, bank_forks::BankForks, bank_utils, @@ -175,15 +175,14 @@ fn execute_batch( let pre_process_units: u64 = aggregate_total_execution_units(timings); - let (tx_results, balances, inner_instructions, transaction_logs) = - batch.bank().load_execute_and_commit_transactions( - batch, - MAX_PROCESSING_AGE, - transaction_status_sender.is_some(), - transaction_status_sender.is_some(), - transaction_status_sender.is_some(), - timings, - ); + let (tx_results, balances) = batch.bank().load_execute_and_commit_transactions( + batch, + MAX_PROCESSING_AGE, + transaction_status_sender.is_some(), + transaction_status_sender.is_some(), + transaction_status_sender.is_some(), + timings, + ); if bank .feature_set @@ -238,8 +237,6 @@ fn execute_batch( execution_results, balances, token_balances, - inner_instructions, - transaction_logs, rent_debits, ); } @@ -1399,11 +1396,9 @@ pub enum TransactionStatusMessage { pub struct TransactionStatusBatch { pub bank: Arc, pub transactions: Vec, - pub statuses: Vec, + pub execution_results: Vec, pub balances: TransactionBalancesSet, pub token_balances: TransactionTokenBalancesSet, - pub inner_instructions: Option>>, - pub transaction_logs: Option>>, pub rent_debits: Vec, } @@ -1418,29 +1413,28 @@ impl TransactionStatusSender { &self, bank: Arc, transactions: Vec, - statuses: Vec, + mut execution_results: Vec, balances: TransactionBalancesSet, token_balances: TransactionTokenBalancesSet, - inner_instructions: Vec>, - transaction_logs: Vec>, rent_debits: Vec, ) { let slot = bank.slot(); - let (inner_instructions, transaction_logs) = if !self.enable_cpi_and_log_storage { - (None, None) - } else { - (Some(inner_instructions), Some(transaction_logs)) - }; + if !self.enable_cpi_and_log_storage { + execution_results.iter_mut().for_each(|execution_result| { + if let TransactionExecutionResult::Executed(details) = execution_result { + details.log_messages.take(); + details.inner_instructions.take(); + } + }); + } if let Err(e) = self .sender .send(TransactionStatusMessage::Batch(TransactionStatusBatch { bank, transactions, - statuses, + execution_results, balances, token_balances, - inner_instructions, - transaction_logs, rent_debits, })) { @@ -3483,8 +3477,6 @@ pub mod tests { .. }, _balances, - _inner_instructions, - _log_messages, ) = batch.bank().load_execute_and_commit_transactions( &batch, MAX_PROCESSING_AGE, diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index d8b7fbe3f6..16338e20ef 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -416,7 +416,7 @@ fn setup_fees(bank: Bank) -> Bank { bank.commit_transactions( &[], // transactions &mut [], // loaded accounts - &[], // transaction execution results + vec![], // transaction execution results 0, // tx count 1, // signature count &mut ExecuteTimings::default(), diff --git a/programs/bpf/tests/programs.rs b/programs/bpf/tests/programs.rs index a5773db269..75782df56c 100644 --- a/programs/bpf/tests/programs.rs +++ b/programs/bpf/tests/programs.rs @@ -17,7 +17,6 @@ use solana_bpf_loader_program::{ use solana_bpf_rust_invoke::instructions::*; use solana_bpf_rust_realloc::instructions::*; use solana_bpf_rust_realloc_invoke::instructions::*; -use solana_cli_output::display::println_transaction; use solana_program_runtime::invoke_context::with_mock_invoke_context; use solana_rbpf::{ elf::Executable, @@ -25,7 +24,10 @@ use solana_rbpf::{ vm::{Config, Tracer}, }; use solana_runtime::{ - bank::{Bank, ExecuteTimings, NonceInfo, TransactionBalancesSet, TransactionResults}, + bank::{ + Bank, DurableNonceFee, ExecuteTimings, TransactionBalancesSet, TransactionExecutionDetails, + TransactionExecutionResult, TransactionResults, + }, bank_client::BankClient, genesis_utils::{create_genesis_config, GenesisConfigInfo}, loader_utils::{ @@ -53,12 +55,13 @@ use solana_sdk::{ }; use solana_transaction_status::{ token_balances::collect_token_balances, ConfirmedTransaction, InnerInstructions, - TransactionStatusMeta, TransactionWithStatusMeta, UiTransactionEncoding, + TransactionStatusMeta, TransactionWithStatusMeta, }; use std::{ - collections::HashMap, convert::TryFrom, env, fs::File, io::Read, path::PathBuf, str::FromStr, + collections::HashMap, env, fs::File, io::Read, path::PathBuf, str::FromStr, sync::Arc, }; +use std::{collections::HashMap, env, fs::File, io::Read, path::PathBuf, str::FromStr, sync::Arc}; /// BPF program file extension const PLATFORM_FILE_EXTENSION_BPF: &str = "so"; @@ -299,7 +302,7 @@ fn process_transaction_and_record_inner( let signature = tx.signatures.get(0).unwrap().clone(); let txs = vec![tx]; let tx_batch = bank.prepare_batch_for_tests(txs); - let (mut results, _, mut inner_instructions, _transaction_logs) = bank + let mut results = bank .load_execute_and_commit_transactions( &tx_batch, MAX_PROCESSING_AGE, @@ -307,20 +310,27 @@ fn process_transaction_and_record_inner( true, false, &mut ExecuteTimings::default(), - ); + ) + .0; let result = results .fee_collection_results .swap_remove(0) .and_then(|_| bank.get_signature_status(&signature).unwrap()); - ( - result, - inner_instructions - .swap_remove(0) - .expect("cpi recording should be enabled"), - ) + let inner_instructions = results + .execution_results + .swap_remove(0) + .details() + .expect("tx should be executed") + .clone() + .inner_instructions + .expect("cpi recording should be enabled"); + (result, inner_instructions) } -fn execute_transactions(bank: &Bank, txs: Vec) -> Vec { +fn execute_transactions( + bank: &Bank, + txs: Vec, +) -> Vec> { let batch = bank.prepare_batch_for_tests(txs.clone()); let mut timings = ExecuteTimings::default(); let mut mint_decimals = HashMap::new(); @@ -334,8 +344,6 @@ fn execute_transactions(bank: &Bank, txs: Vec) -> Vec) -> Vec { + let TransactionExecutionDetails { + status, + log_messages, + inner_instructions, + durable_nonce_fee, + } = details; - let inner_instructions = inner_instructions.map(|inner_instructions| { - inner_instructions - .into_iter() - .enumerate() - .map(|(index, instructions)| InnerInstructions { - index: index as u8, - instructions, + let lamports_per_signature = match durable_nonce_fee { + Some(DurableNonceFee::Valid(lamports_per_signature)) => { + Some(lamports_per_signature) + } + Some(DurableNonceFee::Invalid) => None, + None => bank.get_lamports_per_signature_for_blockhash( + &tx.message().recent_blockhash, + ), + } + .expect("lamports_per_signature must be available"); + let fee = Bank::get_fee_for_message_with_lamports_per_signature( + &SanitizedMessage::try_from(tx.message().clone()).unwrap(), + lamports_per_signature, + ); + + let inner_instructions = inner_instructions.map(|inner_instructions| { + inner_instructions + .into_iter() + .enumerate() + .map(|(index, instructions)| InnerInstructions { + index: index as u8, + instructions, + }) + .filter(|i| !i.instructions.is_empty()) + .collect() + }); + + let tx_status_meta = TransactionStatusMeta { + status, + fee, + pre_balances, + post_balances, + pre_token_balances: Some(pre_token_balances), + post_token_balances: Some(post_token_balances), + inner_instructions, + log_messages, + rewards: None, + }; + + Ok(ConfirmedTransaction { + slot: bank.slot(), + transaction: TransactionWithStatusMeta { + transaction: tx.clone(), + meta: Some(tx_status_meta), + }, + block_time: None, }) - .filter(|i| !i.instructions.is_empty()) - .collect() - }); - - let tx_status_meta = TransactionStatusMeta { - status: execute_result, - fee, - pre_balances, - post_balances, - pre_token_balances: Some(pre_token_balances), - post_token_balances: Some(post_token_balances), - inner_instructions, - log_messages, - rewards: None, - }; - - ConfirmedTransaction { - slot: bank.slot(), - transaction: TransactionWithStatusMeta { - transaction: tx.clone(), - meta: Some(tx_status_meta), - }, - block_time: None, + } + TransactionExecutionResult::NotExecuted(err) => Err(err.clone()), } }, ) .collect() } -fn print_confirmed_tx(name: &str, confirmed_tx: ConfirmedTransaction) { - let block_time = confirmed_tx.block_time; - let tx = confirmed_tx.transaction.transaction.clone(); - let encoded = confirmed_tx.encode(UiTransactionEncoding::JsonParsed); - println!("EXECUTE {} (slot {})", name, encoded.slot); - println_transaction(&tx, &encoded.transaction.meta, " ", None, block_time); -} - #[test] #[cfg(any(feature = "bpf_c", feature = "bpf_rust"))] fn test_program_bpf_sanity() { @@ -2450,43 +2462,35 @@ fn test_program_upgradeable_locks() { execute_transactions(&bank, vec![invoke_tx, upgrade_tx]) }; - if false { - println!("upgrade and invoke"); - for result in &results1 { - print_confirmed_tx("result", result.clone()); - } - println!("invoke and upgrade"); - for result in &results2 { - print_confirmed_tx("result", result.clone()); - } - } + assert!(matches!( + results1[0], + Ok(ConfirmedTransactionWithStatusMeta { + transaction: TransactionWithStatusMeta { + meta: Some(TransactionStatusMeta { status: Ok(()), .. }), + .. + }, + .. + }) + )); + assert_eq!(results1[1], Err(TransactionError::AccountInUse)); - if let Some(ref meta) = results1[0].transaction.meta { - assert_eq!(meta.status, Ok(())); - } else { - panic!("no meta"); - } - if let Some(ref meta) = results1[1].transaction.meta { - assert_eq!(meta.status, Err(TransactionError::AccountInUse)); - } else { - panic!("no meta"); - } - if let Some(ref meta) = results2[0].transaction.meta { - assert_eq!( - meta.status, - Err(TransactionError::InstructionError( - 0, - InstructionError::ProgramFailedToComplete - )) - ); - } else { - panic!("no meta"); - } - if let Some(ref meta) = results2[1].transaction.meta { - assert_eq!(meta.status, Err(TransactionError::AccountInUse)); - } else { - panic!("no meta"); - } + assert!(matches!( + results2[0], + Ok(ConfirmedTransactionWithStatusMeta { + transaction: TransactionWithStatusMeta { + meta: Some(TransactionStatusMeta { + status: Err(TransactionError::InstructionError( + 0, + InstructionError::ProgramFailedToComplete + )), + .. + }), + .. + }, + .. + }) + )); + assert_eq!(results2[1], Err(TransactionError::AccountInUse)); } #[cfg(feature = "bpf_rust")] diff --git a/rpc/src/transaction_status_service.rs b/rpc/src/transaction_status_service.rs index 3377460b88..25618fe22c 100644 --- a/rpc/src/transaction_status_service.rs +++ b/rpc/src/transaction_status_service.rs @@ -6,7 +6,9 @@ use { blockstore::Blockstore, blockstore_processor::{TransactionStatusBatch, TransactionStatusMessage}, }, - solana_runtime::bank::{Bank, InnerInstructionsList, NonceInfo, TransactionLogMessages}, + solana_runtime::bank::{ + Bank, DurableNonceFee, TransactionExecutionDetails, TransactionExecutionResult, + }, solana_transaction_status::{ extract_and_fmt_memos, InnerInstructions, Reward, TransactionStatusMeta, }, @@ -67,58 +69,46 @@ impl TransactionStatusService { TransactionStatusMessage::Batch(TransactionStatusBatch { bank, transactions, - statuses, + execution_results, balances, token_balances, - inner_instructions, - transaction_logs, rent_debits, }) => { let slot = bank.slot(); - let inner_instructions_iter: Box< - dyn Iterator>, - > = if let Some(inner_instructions) = inner_instructions { - Box::new(inner_instructions.into_iter()) - } else { - Box::new(std::iter::repeat_with(|| None)) - }; - let transaction_logs_iter: Box< - dyn Iterator>, - > = if let Some(transaction_logs) = transaction_logs { - Box::new(transaction_logs.into_iter()) - } else { - Box::new(std::iter::repeat_with(|| None)) - }; for ( transaction, - (status, nonce), + execution_result, pre_balances, post_balances, pre_token_balances, post_token_balances, - inner_instructions, - log_messages, rent_debits, ) in izip!( transactions, - statuses, + execution_results, balances.pre_balances, balances.post_balances, token_balances.pre_token_balances, token_balances.post_token_balances, - inner_instructions_iter, - transaction_logs_iter, rent_debits, ) { - if Bank::can_commit(&status) { - let lamports_per_signature = nonce - .map(|nonce| nonce.lamports_per_signature()) - .unwrap_or_else(|| { - bank.get_lamports_per_signature_for_blockhash( - transaction.message().recent_blockhash(), - ) - }) - .expect("lamports_per_signature must be available"); + if let TransactionExecutionResult::Executed(details) = execution_result { + let TransactionExecutionDetails { + status, + log_messages, + inner_instructions, + durable_nonce_fee, + } = details; + let lamports_per_signature = match durable_nonce_fee { + Some(DurableNonceFee::Valid(lamports_per_signature)) => { + Some(lamports_per_signature) + } + Some(DurableNonceFee::Invalid) => None, + None => bank.get_lamports_per_signature_for_blockhash( + transaction.message().recent_blockhash(), + ), + } + .expect("lamports_per_signature must be available"); let fee = Bank::get_fee_for_message_with_lamports_per_signature( transaction.message(), lamports_per_signature, @@ -331,18 +321,21 @@ pub(crate) mod tests { let mut rent_debits = RentDebits::default(); rent_debits.insert(&pubkey, 123, 456); - let transaction_result = ( - Ok(()), - Some( - NonceFull::from_partial( - rollback_partial, - &SanitizedMessage::Legacy(message), - &[(pubkey, nonce_account)], - &rent_debits, - ) - .unwrap(), - ), - ); + let transaction_result = + TransactionExecutionResult::Executed(TransactionExecutionDetails { + status: Ok(()), + log_messages: None, + inner_instructions: None, + durable_nonce_fee: Some(DurableNonceFee::from( + &NonceFull::from_partial( + rollback_partial, + &SanitizedMessage::Legacy(message), + &[(pubkey, nonce_account)], + &rent_debits, + ) + .unwrap(), + )), + }); let balances = TransactionBalancesSet { pre_balances: vec![vec![123456]], @@ -374,11 +367,9 @@ pub(crate) mod tests { let transaction_status_batch = TransactionStatusBatch { bank, transactions: vec![transaction], - statuses: vec![transaction_result], + execution_results: vec![transaction_result], balances, token_balances, - inner_instructions: None, - transaction_logs: None, rent_debits: vec![rent_debits], }; diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index c3805ca96f..e06819bf93 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -1076,26 +1076,35 @@ impl Accounts { leave_nonce_on_success: bool, ) -> Vec<(&'a Pubkey, &'a AccountSharedData)> { let mut accounts = Vec::with_capacity(load_results.len()); - for (i, ((tx_load_result, _), tx)) in load_results.iter_mut().zip(txs).enumerate() { + for (i, ((tx_load_result, nonce), tx)) in load_results.iter_mut().zip(txs).enumerate() { if tx_load_result.is_err() { // Don't store any accounts if tx failed to load continue; } - let (execution_result, nonce) = &execution_results[i]; - let maybe_nonce = match (execution_result, nonce) { - (Ok(_), Some(nonce)) => { + let execution_status = match &execution_results[i] { + TransactionExecutionResult::Executed(details) => &details.status, + // Don't store any accounts if tx wasn't executed + TransactionExecutionResult::NotExecuted(_) => continue, + }; + + let maybe_nonce = match (execution_status, &*nonce) { + (Ok(()), Some(nonce)) => { if leave_nonce_on_success { None } else { Some((nonce, false /* rollback */)) } } - (Err(TransactionError::InstructionError(_, _)), Some(nonce)) => { + (Err(_), Some(nonce)) => { Some((nonce, true /* rollback */)) } - (Ok(_), _) => None, // Success, don't do any additional nonce processing - (Err(_), _) => continue, // Not nonce, don't store any accounts + (Ok(_), None) => None, // Success, don't do any additional nonce processing + (Err(_), None) => { + // Fees for failed transactions which don't use durable nonces are + // deducted in Bank::filter_program_errors_and_collect_fee + continue; + } }; let message = tx.message(); @@ -1113,14 +1122,14 @@ impl Accounts { let is_nonce_account = prepare_if_nonce_account( address, account, - execution_result, + execution_status, is_fee_payer, maybe_nonce, blockhash, lamports_per_signature, ); - if execution_result.is_ok() || is_nonce_account || is_fee_payer { + if execution_status.is_ok() || is_nonce_account || is_fee_payer { if account.rent_epoch() == INITIAL_RENT_EPOCH { let rent = rent_collector.collect_from_created_account( address, @@ -1229,7 +1238,10 @@ pub fn update_accounts_bench(accounts: &Accounts, pubkeys: &[Pubkey], slot: u64) mod tests { use { super::*, - crate::rent_collector::RentCollector, + crate::{ + bank::{DurableNonceFee, TransactionExecutionDetails}, + rent_collector::RentCollector, + }, solana_sdk::{ account::{AccountSharedData, WritableAccount}, epoch_schedule::EpochSchedule, @@ -1262,6 +1274,18 @@ mod tests { )) } + fn new_execution_result( + status: Result<()>, + nonce: Option<&NonceFull>, + ) -> TransactionExecutionResult { + TransactionExecutionResult::Executed(TransactionExecutionDetails { + status, + log_messages: None, + inner_instructions: None, + durable_nonce_fee: nonce.map(DurableNonceFee::from), + }) + } + fn load_accounts_with_fee_and_rent( tx: Transaction, ka: &[(Pubkey, AccountSharedData)], @@ -2668,10 +2692,10 @@ mod tests { .insert_new_readonly(&pubkey); } let txs = vec![tx0, tx1]; - let programs = vec![(Ok(()), None), (Ok(()), None)]; + let execution_results = vec![new_execution_result(Ok(()), None); 2]; let collected_accounts = accounts.collect_accounts_to_store( &txs, - &programs, + &execution_results, loaded.as_mut_slice(), &rent_collector, &Hash::default(), @@ -3091,16 +3115,16 @@ mod tests { AccountShrinkThreshold::default(), ); let txs = vec![tx]; - let programs = vec![( + let execution_results = vec![new_execution_result( Err(TransactionError::InstructionError( 1, InstructionError::InvalidArgument, )), - nonce, + nonce.as_ref(), )]; let collected_accounts = accounts.collect_accounts_to_store( &txs, - &programs, + &execution_results, loaded.as_mut_slice(), &rent_collector, &next_blockhash, @@ -3201,16 +3225,16 @@ mod tests { AccountShrinkThreshold::default(), ); let txs = vec![tx]; - let programs = vec![( + let execution_results = vec![new_execution_result( Err(TransactionError::InstructionError( 1, InstructionError::InvalidArgument, )), - nonce, + nonce.as_ref(), )]; let collected_accounts = accounts.collect_accounts_to_store( &txs, - &programs, + &execution_results, loaded.as_mut_slice(), &rent_collector, &next_blockhash, diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 357cf0796d..de4ea9fde3 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -37,7 +37,10 @@ use solana_sdk::recent_blockhashes_account; use { crate::{ - accounts::{AccountAddressFilter, Accounts, TransactionAccounts, TransactionLoadResult}, + accounts::{ + AccountAddressFilter, Accounts, LoadedTransaction, TransactionAccounts, + TransactionLoadResult, + }, accounts_db::{ AccountShrinkThreshold, AccountsDbConfig, ErrorCounters, SnapshotStorages, ACCOUNTS_DB_CONFIG_FOR_BENCHMARKS, ACCOUNTS_DB_CONFIG_FOR_TESTING, @@ -506,12 +509,91 @@ impl StatusCacheRc { } pub type TransactionCheckResult = (Result<()>, Option); -pub type TransactionExecutionResult = (Result<()>, Option); + pub struct TransactionResults { pub fee_collection_results: Vec>, pub execution_results: Vec, pub rent_debits: Vec, } + +#[derive(Debug, Clone)] +pub struct TransactionExecutionDetails { + pub status: Result<()>, + pub log_messages: Option>, + pub inner_instructions: Option>>, + pub durable_nonce_fee: Option, +} + +/// 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(TransactionExecutionDetails), + NotExecuted(TransactionError), +} + +impl TransactionExecutionResult { + pub fn was_executed_successfully(&self) -> bool { + match self { + Self::Executed(details) => details.status.is_ok(), + Self::NotExecuted { .. } => false, + } + } + + pub fn was_executed(&self) -> bool { + match self { + Self::Executed(_) => true, + Self::NotExecuted(_) => false, + } + } + + pub fn details(&self) -> Option<&TransactionExecutionDetails> { + match self { + Self::Executed(details) => Some(details), + Self::NotExecuted(_) => None, + } + } + + pub fn flattened_result(&self) -> Result<()> { + match self { + Self::Executed(details) => details.status.clone(), + Self::NotExecuted(err) => Err(err.clone()), + } + } +} + +#[derive(Debug, Clone)] +pub enum DurableNonceFee { + Valid(u64), + Invalid, +} + +impl From<&NonceFull> for DurableNonceFee { + fn from(nonce: &NonceFull) -> Self { + match nonce.lamports_per_signature() { + Some(lamports_per_signature) => Self::Valid(lamports_per_signature), + None => Self::Invalid, + } + } +} + +impl DurableNonceFee { + pub fn lamports_per_signature(&self) -> Option { + match self { + Self::Valid(lamports_per_signature) => Some(*lamports_per_signature), + Self::Invalid => None, + } + } +} + pub struct TransactionSimulationResult { pub result: Result<()>, pub logs: TransactionLogMessages, @@ -2995,30 +3077,22 @@ impl Bank { .clear_slot_entries(slot); } - pub fn can_commit(result: &Result<()>) -> bool { - match result { - Ok(_) => true, - Err(TransactionError::InstructionError(_, _)) => true, - Err(_) => false, - } - } - fn update_transaction_statuses( &self, sanitized_txs: &[SanitizedTransaction], - res: &[TransactionExecutionResult], + execution_results: &[TransactionExecutionResult], ) { let mut status_cache = self.src.status_cache.write().unwrap(); - assert_eq!(sanitized_txs.len(), res.len()); - for (tx, (res, _nonce)) in sanitized_txs.iter().zip(res) { - if Self::can_commit(res) { + assert_eq!(sanitized_txs.len(), execution_results.len()); + for (tx, execution_result) in sanitized_txs.iter().zip(execution_results) { + if let TransactionExecutionResult::Executed(details) = execution_result { // 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(), - res.clone(), + details.status.clone(), ); // 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 @@ -3027,7 +3101,7 @@ impl Bank { tx.message().recent_blockhash(), tx.signature(), self.slot(), - res.clone(), + details.status.clone(), ); } } @@ -3163,9 +3237,7 @@ impl Bank { let ( loaded_transactions, - executed, - _inner_instructions, - logs, + mut execution_results, _retryable_transactions, _transaction_count, _signature_count, @@ -3180,8 +3252,6 @@ impl Bank { &mut timings, ); - let result = executed[0].0.clone().map(|_| ()); - let logs = logs.get(0).cloned().flatten().unwrap_or_default(); let post_simulation_accounts = loaded_transactions .into_iter() .next() @@ -3207,8 +3277,16 @@ impl Bank { debug!("simulate_transaction: {:?}", timings); + let execution_result = execution_results.pop().unwrap(); + let flattened_result = execution_result.flattened_result(); + let logs = match execution_result { + TransactionExecutionResult::Executed(details) => details.log_messages, + TransactionExecutionResult::NotExecuted(_) => None, + } + .unwrap_or_default(); + TransactionSimulationResult { - result, + result: flattened_result, logs, post_simulation_accounts, units_consumed, @@ -3506,6 +3584,110 @@ impl Bank { cache.remove(pubkey); } + /// Execute a transaction using the provided loaded accounts and update + /// the executors cache if the transaction was successful. + fn execute_loaded_transaction( + &self, + tx: &SanitizedTransaction, + loaded_transaction: &mut LoadedTransaction, + compute_budget: ComputeBudget, + durable_nonce_fee: Option, + enable_cpi_recording: bool, + enable_log_recording: bool, + execute_details_timings: &mut ExecuteDetailsTimings, + error_counters: &mut ErrorCounters, + ) -> TransactionExecutionResult { + let legacy_message = match tx.message().legacy_message() { + Some(message) => message, + None => { + // TODO: support versioned messages + return TransactionExecutionResult::NotExecuted( + TransactionError::UnsupportedVersion, + ); + } + }; + + let executors = self.get_executors( + tx.message(), + &loaded_transaction.accounts, + &loaded_transaction.program_indices, + ); + + let account_refcells = Self::accounts_to_refcells(&mut loaded_transaction.accounts); + + let instruction_recorders = if enable_cpi_recording { + let ix_count = tx.message().instructions().len(); + let mut recorders = Vec::with_capacity(ix_count); + recorders.resize_with(ix_count, InstructionRecorder::default); + Some(recorders) + } else { + None + }; + + let log_collector = if enable_log_recording { + Some(LogCollector::new_ref()) + } else { + None + }; + + let (blockhash, lamports_per_signature) = self.last_blockhash_and_lamports_per_signature(); + let process_result = MessageProcessor::process_message( + &self.builtin_programs.vec, + legacy_message, + &loaded_transaction.program_indices, + &account_refcells, + self.rent_collector.rent, + log_collector.clone(), + executors.clone(), + instruction_recorders.as_deref(), + self.feature_set.clone(), + compute_budget, + execute_details_timings, + &*self.sysvar_cache.read().unwrap(), + blockhash, + lamports_per_signature, + ); + + let log_messages: Option = + log_collector.and_then(|log_collector| { + Rc::try_unwrap(log_collector) + .map(|log_collector| log_collector.into_inner().into()) + .ok() + }); + + let inner_instructions: Option = + instruction_recorders.and_then(|instruction_recorders| { + instruction_recorders + .into_iter() + .map(|r| r.compile_instructions(tx.message())) + .collect() + }); + + if let Err(e) = + Self::refcells_to_accounts(&mut loaded_transaction.accounts, account_refcells) + { + warn!("Account lifetime mismanagement"); + return TransactionExecutionResult::NotExecuted(e); + } + + let status = process_result + .map(|info| { + self.update_accounts_data_len(info.accounts_data_len_delta); + self.update_executors(executors); + }) + .map_err(|err| { + error_counters.instruction_error += 1; + err + }); + + TransactionExecutionResult::Executed(TransactionExecutionDetails { + status, + log_messages, + inner_instructions, + durable_nonce_fee, + }) + } + #[allow(clippy::type_complexity)] pub fn load_and_execute_transactions( &self, @@ -3517,8 +3699,6 @@ impl Bank { ) -> ( Vec, Vec, - Vec>, - Vec>, Vec, u64, u64, @@ -3567,129 +3747,36 @@ impl Bank { let mut execution_time = Measure::start("execution_time"); let mut signature_count: u64 = 0; - let mut inner_instructions: Vec> = - Vec::with_capacity(sanitized_txs.len()); - let mut transaction_log_messages: Vec>> = - Vec::with_capacity(sanitized_txs.len()); - let executed: Vec = loaded_txs + let execute_details_timings = &mut timings.details; + let execution_results: Vec = loaded_txs .iter_mut() .zip(sanitized_txs.iter()) .map(|(accs, tx)| match accs { - (Err(e), _nonce) => { - transaction_log_messages.push(None); - inner_instructions.push(None); - (Err(e.clone()), None) - } + (Err(e), _nonce) => TransactionExecutionResult::NotExecuted(e.clone()), (Ok(loaded_transaction), nonce) => { let feature_set = self.feature_set.clone(); signature_count += u64::from(tx.message().header().num_required_signatures); let mut compute_budget = self.compute_budget.unwrap_or_else(ComputeBudget::new); - - let mut process_result = if feature_set.is_active(&tx_wide_compute_cap::id()) { - compute_budget.process_transaction(tx, feature_set.clone()) - } else { - Ok(()) - }; - - if process_result.is_ok() { - let executors = self.get_executors( - tx.message(), - &loaded_transaction.accounts, - &loaded_transaction.program_indices, - ); - - let account_refcells = - Self::accounts_to_refcells(&mut loaded_transaction.accounts); - - let instruction_recorders = if enable_cpi_recording { - let ix_count = tx.message().instructions().len(); - let mut recorders = Vec::with_capacity(ix_count); - recorders.resize_with(ix_count, InstructionRecorder::default); - Some(recorders) - } else { - None - }; - - let log_collector = if enable_log_recording { - Some(LogCollector::new_ref()) - } else { - None - }; - - let (blockhash, lamports_per_signature) = - self.last_blockhash_and_lamports_per_signature(); - - if let Some(legacy_message) = tx.message().legacy_message() { - process_result = MessageProcessor::process_message( - &self.builtin_programs.vec, - legacy_message, - &loaded_transaction.program_indices, - &account_refcells, - self.rent_collector.rent, - log_collector.clone(), - executors.clone(), - instruction_recorders.as_deref(), - feature_set, - compute_budget, - &mut timings.details, - &*self.sysvar_cache.read().unwrap(), - blockhash, - lamports_per_signature, - ) - .map(|process_result| { - self.update_accounts_data_len( - process_result.accounts_data_len_delta, - ) - }); - } else { - // TODO: support versioned messages - process_result = Err(TransactionError::UnsupportedVersion); + if feature_set.is_active(&tx_wide_compute_cap::id()) { + if let Err(err) = compute_budget.process_transaction(tx, feature_set) { + return TransactionExecutionResult::NotExecuted(err); } - - let log_messages: Option = - log_collector.and_then(|log_collector| { - Rc::try_unwrap(log_collector) - .map(|log_collector| log_collector.into_inner().into()) - .ok() - }); - transaction_log_messages.push(log_messages); - let inner_instruction_list: Option = - instruction_recorders.and_then(|instruction_recorders| { - instruction_recorders - .into_iter() - .map(|r| r.compile_instructions(tx.message())) - .collect() - }); - inner_instructions.push(inner_instruction_list); - - if let Err(e) = Self::refcells_to_accounts( - &mut loaded_transaction.accounts, - account_refcells, - ) { - warn!("Account lifetime mismanagement"); - process_result = Err(e); - } - - if process_result.is_ok() { - self.update_executors(executors); - } - } else { - transaction_log_messages.push(None); - inner_instructions.push(None); } - let nonce = match &process_result { - Ok(_) => nonce.clone(), // May need to calculate the fee based on the nonce - Err(TransactionError::InstructionError(_, _)) => { - error_counters.instruction_error += 1; - nonce.clone() // May need to advance the nonce - } - _ => None, - }; + let durable_nonce_fee = nonce.as_ref().map(DurableNonceFee::from); - (process_result, nonce) + self.execute_loaded_transaction( + tx, + loaded_transaction, + compute_budget, + durable_nonce_fee, + enable_cpi_recording, + enable_log_recording, + execute_details_timings, + &mut error_counters, + ) } }) .collect(); @@ -3712,17 +3799,18 @@ impl Bank { let transaction_log_collector_config = self.transaction_log_collector_config.read().unwrap(); - for (i, ((r, _nonce), tx)) in executed.iter().zip(sanitized_txs).enumerate() { + for (execution_result, tx) in execution_results.iter().zip(sanitized_txs) { if let Some(debug_keys) = &self.transaction_debug_keys { for key in tx.message().account_keys_iter() { if debug_keys.contains(key) { - info!("slot: {} result: {:?} tx: {:?}", self.slot, r, tx); + let result = execution_result.flattened_result(); + info!("slot: {} result: {:?} tx: {:?}", self.slot, result, tx); break; } } } - if Self::can_commit(r) // Skip log collection for unprocessed transactions + if execution_result.was_executed() // Skip log collection for unprocessed transactions && transaction_log_collector_config.filter != TransactionLogCollectorFilter::None { let mut filtered_mentioned_addresses = Vec::new(); @@ -3753,16 +3841,21 @@ impl Bank { }; if store { - if let Some(log_messages) = transaction_log_messages.get(i).cloned().flatten() { + if let TransactionExecutionResult::Executed(TransactionExecutionDetails { + status, + log_messages: Some(log_messages), + .. + }) = execution_result + { let mut transaction_log_collector = self.transaction_log_collector.write().unwrap(); let transaction_log_index = transaction_log_collector.logs.len(); transaction_log_collector.logs.push(TransactionLogInfo { signature: *tx.signature(), - result: r.clone(), + result: status.clone(), is_vote, - log_messages, + log_messages: log_messages.clone(), }); for key in filtered_mentioned_addresses.into_iter() { transaction_log_collector @@ -3775,13 +3868,16 @@ impl Bank { } } - if r.is_ok() { - tx_count += 1; - } else { - if *err_count == 0 { - debug!("tx error: {:?} {:?}", r, tx); + match execution_result.flattened_result() { + Ok(()) => { + tx_count += 1; + } + Err(err) => { + if *err_count == 0 { + debug!("tx error: {:?} {:?}", err, tx); + } + *err_count += 1; } - *err_count += 1; } } if *err_count > 0 { @@ -3794,9 +3890,7 @@ impl Bank { Self::update_error_counters(&error_counters); ( loaded_txs, - executed, - inner_instructions, - transaction_log_messages, + execution_results, retryable_txs, tx_count, signature_count, @@ -3840,10 +3934,16 @@ impl Bank { let results = txs .iter() .zip(execution_results) - .map(|(tx, (execution_result, nonce))| { - let (lamports_per_signature, is_nonce) = nonce - .as_ref() - .map(|nonce| nonce.lamports_per_signature()) + .map(|(tx, execution_result)| { + let (execution_status, durable_nonce_fee) = match &execution_result { + TransactionExecutionResult::Executed(details) => { + Ok((&details.status, details.durable_nonce_fee.as_ref())) + } + TransactionExecutionResult::NotExecuted(err) => Err(err.clone()), + }?; + + let (lamports_per_signature, is_nonce) = durable_nonce_fee + .map(|durable_nonce_fee| durable_nonce_fee.lamports_per_signature()) .map(|maybe_lamports_per_signature| (maybe_lamports_per_signature, true)) .unwrap_or_else(|| { ( @@ -3856,27 +3956,19 @@ impl Bank { lamports_per_signature.ok_or(TransactionError::BlockhashNotFound)?; let fee = Self::calculate_fee(tx.message(), lamports_per_signature); - match *execution_result { - Err(TransactionError::InstructionError(_, _)) => { - // In case of instruction error, even though no accounts - // were stored we still need to charge the payer the - // fee. - // - //...except nonce accounts, which already have their - // post-load, fee deducted, pre-execute account state - // stored - if !is_nonce { - self.withdraw(tx.message().fee_payer(), fee)?; - } - fees += fee; - Ok(()) - } - Ok(()) => { - fees += fee; - Ok(()) - } - _ => execution_result.clone(), + // In case of instruction error, even though no accounts + // were stored we still need to charge the payer the + // fee. + // + //...except nonce accounts, which already have their + // post-load, fee deducted, pre-execute account state + // stored + if execution_status.is_err() && !is_nonce { + self.withdraw(tx.message().fee_payer(), fee)?; } + + fees += fee; + Ok(()) }) .collect(); @@ -3888,7 +3980,7 @@ impl Bank { &self, sanitized_txs: &[SanitizedTransaction], loaded_txs: &mut [TransactionLoadResult], - executed_results: &[TransactionExecutionResult], + execution_results: Vec, tx_count: u64, signature_count: u64, timings: &mut ExecuteTimings, @@ -3914,10 +4006,7 @@ impl Bank { .fetch_max(processed_tx_count, Relaxed); } - if executed_results - .iter() - .any(|(res, _)| Self::can_commit(res)) - { + if execution_results.iter().any(|result| result.was_executed()) { self.is_delta.store(true, Relaxed); } @@ -3926,7 +4015,7 @@ impl Bank { self.rc.accounts.store_cached( self.slot(), sanitized_txs, - executed_results, + &execution_results, loaded_txs, &self.rent_collector, &blockhash, @@ -3934,10 +4023,10 @@ impl Bank { self.rent_for_sysvars(), self.leave_nonce_on_success(), ); - let rent_debits = self.collect_rent(executed_results, loaded_txs); + let rent_debits = self.collect_rent(&execution_results, loaded_txs); let mut update_stakes_cache_time = Measure::start("update_stakes_cache_time"); - self.update_stakes_cache(sanitized_txs, executed_results, loaded_txs); + self.update_stakes_cache(sanitized_txs, &execution_results, loaded_txs); update_stakes_cache_time.stop(); // once committed there is no way to unroll @@ -3951,13 +4040,13 @@ impl Bank { timings.update_stakes_cache_us = timings .update_stakes_cache_us .saturating_add(update_stakes_cache_time.as_us()); - self.update_transaction_statuses(sanitized_txs, executed_results); + self.update_transaction_statuses(sanitized_txs, &execution_results); let fee_collection_results = - self.filter_program_errors_and_collect_fee(sanitized_txs, executed_results); + self.filter_program_errors_and_collect_fee(sanitized_txs, &execution_results); TransactionResults { fee_collection_results, - execution_results: executed_results.to_vec(), + execution_results, rent_debits, } } @@ -4121,24 +4210,24 @@ impl Bank { fn collect_rent( &self, - res: &[TransactionExecutionResult], + execution_results: &[TransactionExecutionResult], loaded_txs: &mut [TransactionLoadResult], ) -> Vec { let mut collected_rent: u64 = 0; - let mut rent_debits: Vec = Vec::with_capacity(loaded_txs.len()); - for (i, (raccs, _nonce)) in loaded_txs.iter_mut().enumerate() { - let (res, _nonce) = &res[i]; - if res.is_err() || raccs.is_err() { - rent_debits.push(RentDebits::default()); - continue; - } - - let loaded_transaction = raccs.as_mut().unwrap(); - - collected_rent += loaded_transaction.rent; - rent_debits.push(mem::take(&mut loaded_transaction.rent_debits)); - } - + let rent_debits: Vec<_> = loaded_txs + .iter_mut() + .zip(execution_results) + .map(|((load_result, _nonce), execution_result)| { + if let (Ok(loaded_transaction), true) = + (load_result, execution_result.was_executed_successfully()) + { + collected_rent += loaded_transaction.rent; + mem::take(&mut loaded_transaction.rent_debits) + } else { + RentDebits::default() + } + }) + .collect(); self.collected_rent.fetch_add(collected_rent, Relaxed); rent_debits } @@ -4630,38 +4719,26 @@ impl Bank { enable_cpi_recording: bool, enable_log_recording: bool, timings: &mut ExecuteTimings, - ) -> ( - TransactionResults, - TransactionBalancesSet, - Vec>, - Vec>, - ) { + ) -> (TransactionResults, TransactionBalancesSet) { let pre_balances = if collect_balances { self.collect_balances(batch) } else { vec![] }; - let ( - mut loaded_txs, - executed, - inner_instructions, - transaction_logs, - _, - tx_count, - signature_count, - ) = self.load_and_execute_transactions( - batch, - max_age, - enable_cpi_recording, - enable_log_recording, - timings, - ); + let (mut loaded_txs, execution_results, _, tx_count, signature_count) = self + .load_and_execute_transactions( + batch, + max_age, + enable_cpi_recording, + enable_log_recording, + timings, + ); let results = self.commit_transactions( batch.sanitized_transactions(), &mut loaded_txs, - &executed, + execution_results, tx_count, signature_count, timings, @@ -4674,8 +4751,6 @@ impl Bank { ( results, TransactionBalancesSet::new(pre_balances, post_balances), - inner_instructions, - transaction_logs, ) } @@ -5522,26 +5597,24 @@ impl Bank { fn update_stakes_cache( &self, txs: &[SanitizedTransaction], - res: &[TransactionExecutionResult], + execution_results: &[TransactionExecutionResult], loaded_txs: &[TransactionLoadResult], ) { - for (i, ((raccs, _load_nonce), tx)) in loaded_txs.iter().zip(txs).enumerate() { - let (res, _res_nonce) = &res[i]; - if res.is_err() || raccs.is_err() { - continue; - } - - let message = tx.message(); - let loaded_transaction = raccs.as_ref().unwrap(); - - for (_i, (pubkey, account)) in - (0..message.account_keys_len()).zip(loaded_transaction.accounts.iter()) - { - self.stakes_cache.check_and_store( - pubkey, - account, - self.stakes_remove_delegation_if_inactive_enabled(), - ); + for (i, ((load_result, _load_nonce), tx)) in loaded_txs.iter().zip(txs).enumerate() { + if let (Ok(loaded_transaction), true) = ( + load_result, + execution_results[i].was_executed_successfully(), + ) { + let message = tx.message(); + for (_i, (pubkey, account)) in + (0..message.account_keys_len()).zip(loaded_transaction.accounts.iter()) + { + self.stakes_cache.check_and_store( + pubkey, + account, + self.stakes_remove_delegation_if_inactive_enabled(), + ); + } } } } @@ -6303,6 +6376,18 @@ pub(crate) mod tests { Message::new(instructions, payer).try_into().unwrap() } + fn new_execution_result( + status: Result<()>, + nonce: Option<&NonceFull>, + ) -> TransactionExecutionResult { + TransactionExecutionResult::Executed(TransactionExecutionDetails { + status, + log_messages: None, + inner_instructions: None, + durable_nonce_fee: nonce.map(DurableNonceFee::from), + }) + } + #[test] fn test_nonce_info() { let lamports_per_signature = 42; @@ -9042,8 +9127,8 @@ pub(crate) mod tests { )); let results = vec![ - (Ok(()), None), - ( + new_execution_result(Ok(()), None), + new_execution_result( Err(TransactionError::InstructionError( 1, SystemError::ResultWithNegativeLamports.into(), @@ -11411,8 +11496,8 @@ pub(crate) mod tests { let txs = vec![tx0, tx1, tx2]; let lock_result = bank0.prepare_batch_for_tests(txs); - let (transaction_results, transaction_balances_set, inner_instructions, transaction_logs) = - bank0.load_execute_and_commit_transactions( + let (transaction_results, transaction_balances_set) = bank0 + .load_execute_and_commit_transactions( &lock_result, MAX_PROCESSING_AGE, true, @@ -11421,27 +11506,34 @@ pub(crate) mod tests { &mut ExecuteTimings::default(), ); - assert!(inner_instructions.iter().all(Option::is_none)); - assert!(transaction_logs.iter().all(Option::is_none)); - - assert_eq!(inner_instructions.len(), 3); - assert_eq!(transaction_logs.len(), 3); assert_eq!(transaction_balances_set.pre_balances.len(), 3); assert_eq!(transaction_balances_set.post_balances.len(), 3); - assert!(transaction_results.execution_results[0].0.is_ok()); + assert!(transaction_results.execution_results[0].was_executed_successfully()); assert_eq!(transaction_balances_set.pre_balances[0], vec![8, 11, 1]); assert_eq!(transaction_balances_set.post_balances[0], vec![5, 13, 1]); // Failed transactions still produce balance sets // This is a TransactionError - not possible to charge fees - assert!(transaction_results.execution_results[1].0.is_err()); + assert!(matches!( + transaction_results.execution_results[1], + TransactionExecutionResult::NotExecuted(TransactionError::AccountNotFound), + )); assert_eq!(transaction_balances_set.pre_balances[1], vec![0, 0, 1]); assert_eq!(transaction_balances_set.post_balances[1], vec![0, 0, 1]); // Failed transactions still produce balance sets // This is an InstructionError - fees charged - assert!(transaction_results.execution_results[2].0.is_err()); + assert!(matches!( + transaction_results.execution_results[2], + TransactionExecutionResult::Executed(TransactionExecutionDetails { + status: Err(TransactionError::InstructionError( + 0, + InstructionError::Custom(1), + )), + .. + }), + )); assert_eq!(transaction_balances_set.pre_balances[2], vec![9, 0, 1]); assert_eq!(transaction_balances_set.post_balances[2], vec![8, 0, 1]); } @@ -14546,7 +14638,7 @@ pub(crate) mod tests { let txs = vec![tx0, tx1, tx2]; let batch = bank.prepare_batch_for_tests(txs); - let log_results = bank + let execution_results = bank .load_execute_and_commit_transactions( &batch, MAX_PROCESSING_AGE, @@ -14555,11 +14647,28 @@ pub(crate) mod tests { true, &mut ExecuteTimings::default(), ) - .3; - assert_eq!(log_results.len(), 3); - assert!(log_results[0].as_ref().unwrap()[1].contains(&"success".to_string())); - assert!(log_results[1].as_ref().unwrap()[2].contains(&"failed".to_string())); - assert!(log_results[2].as_ref().is_none()); + .0 + .execution_results; + + assert_eq!(execution_results.len(), 3); + + assert!(execution_results[0].details().is_some()); + assert!(execution_results[0] + .details() + .unwrap() + .log_messages + .as_ref() + .unwrap()[1] + .contains(&"success".to_string())); + assert!(execution_results[1].details().is_some()); + assert!(execution_results[1] + .details() + .unwrap() + .log_messages + .as_ref() + .unwrap()[2] + .contains(&"failed".to_string())); + assert!(!execution_results[2].was_executed()); let stored_logs = &bank.transaction_log_collector.read().unwrap().logs; let success_log_info = stored_logs diff --git a/runtime/src/bank_utils.rs b/runtime/src/bank_utils.rs index c1021030dd..771903ba87 100644 --- a/runtime/src/bank_utils.rs +++ b/runtime/src/bank_utils.rs @@ -43,8 +43,8 @@ pub fn find_and_send_votes( sanitized_txs .iter() .zip(execution_results.iter()) - .for_each(|(tx, (result, _nonce))| { - if tx.is_simple_vote_transaction() && result.is_ok() { + .for_each(|(tx, result)| { + if tx.is_simple_vote_transaction() && result.was_executed_successfully() { if let Some(parsed_vote) = vote_transaction::parse_sanitized_vote_transaction(tx) {