From da1796f97a129a7ae4b316e37fb777b9fd6c97eb Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 2 Dec 2020 21:34:21 +0000 Subject: [PATCH] Nonce cleanup followup (bp #13868) (#13922) * runtime: Factor out `DurableNoncePartial` finishing logic and add tests (cherry picked from commit 8df2a4bac0954b004556c2fc2e5c6bb7b86d1028) * accounts: Don't assume fee-payer is the first account (cherry picked from commit 47af5933caea0e03930e2ed03d18160903a832a7) * accounts: Replace nonce_rollback unreachable block with descriptive panic (cherry picked from commit be7760caa1725020290a25023046772c7b5dc733) * sdk: Check owner when verifying nonce accounts (cherry picked from commit 274312ebb59ffeac4a8a40f42375c2c6eedb03e7) * runtime: Replace `HashAgeKind` with `NonceRollbackInfo` (cherry picked from commit 404fc1570d722709344876dc91799edd576056bf) * Make `Accounts::is_non_loader_key()` a method on `Message` (cherry picked from commit 17defbff13fab30210c0b454adb5296f44bd4243) Co-authored-by: Trent Nelson --- core/src/banking_stage.rs | 30 +-- core/src/transaction_status_service.rs | 11 +- ledger/src/blockstore_processor.rs | 12 +- runtime/src/accounts.rs | 255 ++++++++------------ runtime/src/bank.rs | 322 ++++++++++++++++--------- runtime/src/bank_utils.rs | 4 +- sdk/program/src/message.rs | 61 +++++ sdk/src/nonce_account.rs | 23 ++ 8 files changed, 426 insertions(+), 292 deletions(-) diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs index b584c63db7..e1338da9d1 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -23,7 +23,7 @@ use solana_perf::{ }; use solana_runtime::{ accounts_db::ErrorCounters, - bank::{Bank, TransactionBalancesSet, TransactionProcessResult}, + bank::{Bank, TransactionBalancesSet, TransactionCheckResult, TransactionExecutionResult}, bank_utils, transaction_batch::TransactionBatch, vote_sender_types::ReplayVoteSender, @@ -460,7 +460,7 @@ impl BankingStage { fn record_transactions( bank_slot: Slot, txs: &[Transaction], - results: &[TransactionProcessResult], + results: &[TransactionExecutionResult], poh: &Arc>, ) -> (Result, Vec) { let mut processed_generation = Measure::start("record::process_generation"); @@ -578,7 +578,7 @@ impl BankingStage { bank.clone(), batch.transactions(), batch.iteration_order_vec(), - tx_results.processing_results, + tx_results.execution_results, TransactionBalancesSet::new(pre_balances, post_balances), inner_instructions, transaction_logs, @@ -719,7 +719,7 @@ impl BankingStage { // This function returns a vector containing index of all valid transactions. A valid // transaction has result Ok() as the value fn filter_valid_transaction_indexes( - valid_txs: &[TransactionProcessResult], + valid_txs: &[TransactionCheckResult], transaction_indexes: &[usize], ) -> Vec { let valid_transactions = valid_txs @@ -1093,7 +1093,6 @@ mod tests { get_tmp_ledger_path, }; use solana_perf::packet::to_packets; - use solana_runtime::bank::HashAgeKind; use solana_sdk::{ instruction::InstructionError, signature::{Keypair, Signer}, @@ -1457,10 +1456,7 @@ mod tests { system_transaction::transfer(&keypair2, &pubkey2, 1, genesis_config.hash()), ]; - let mut results = vec![ - (Ok(()), Some(HashAgeKind::Extant)), - (Ok(()), Some(HashAgeKind::Extant)), - ]; + let mut results = vec![(Ok(()), None), (Ok(()), None)]; let _ = BankingStage::record_transactions( bank.slot(), &transactions, @@ -1476,7 +1472,7 @@ mod tests { 1, SystemError::ResultWithNegativeLamports.into(), )), - Some(HashAgeKind::Extant), + None, ); let (res, retryable) = BankingStage::record_transactions( bank.slot(), @@ -1652,10 +1648,10 @@ mod tests { &[ (Err(TransactionError::BlockhashNotFound), None), (Err(TransactionError::BlockhashNotFound), None), - (Ok(()), Some(HashAgeKind::Extant)), + (Ok(()), None), (Err(TransactionError::BlockhashNotFound), None), - (Ok(()), Some(HashAgeKind::Extant)), - (Ok(()), Some(HashAgeKind::Extant)), + (Ok(()), None), + (Ok(()), None), ], &[2, 4, 5, 9, 11, 13] ), @@ -1665,12 +1661,12 @@ mod tests { assert_eq!( BankingStage::filter_valid_transaction_indexes( &[ - (Ok(()), Some(HashAgeKind::Extant)), + (Ok(()), None), (Err(TransactionError::BlockhashNotFound), None), (Err(TransactionError::BlockhashNotFound), None), - (Ok(()), Some(HashAgeKind::Extant)), - (Ok(()), Some(HashAgeKind::Extant)), - (Ok(()), Some(HashAgeKind::Extant)), + (Ok(()), None), + (Ok(()), None), + (Ok(()), None), ], &[1, 6, 7, 9, 31, 43] ), diff --git a/core/src/transaction_status_service.rs b/core/src/transaction_status_service.rs index 1645f2db8e..3a71742daa 100644 --- a/core/src/transaction_status_service.rs +++ b/core/src/transaction_status_service.rs @@ -1,7 +1,10 @@ use crossbeam_channel::{Receiver, RecvTimeoutError}; use itertools::izip; use solana_ledger::{blockstore::Blockstore, blockstore_processor::TransactionStatusBatch}; -use solana_runtime::{bank::Bank, transaction_utils::OrderedIterator}; +use solana_runtime::{ + bank::{Bank, NonceRollbackInfo}, + transaction_utils::OrderedIterator, +}; use solana_transaction_status::{InnerInstructions, TransactionStatusMeta}; use std::{ sync::{ @@ -58,7 +61,7 @@ impl TransactionStatusService { let slot = bank.slot(); for ( (_, transaction), - (status, hash_age_kind), + (status, nonce_rollback), pre_balances, post_balances, inner_instructions, @@ -72,8 +75,8 @@ impl TransactionStatusService { transaction_logs ) { if Bank::can_commit(&status) && !transaction.signatures.is_empty() { - let fee_calculator = hash_age_kind - .and_then(|hash_age_kind| hash_age_kind.fee_calculator()) + let fee_calculator = nonce_rollback + .map(|nonce_rollback| nonce_rollback.fee_calculator()) .unwrap_or_else(|| { bank.get_fee_calculator(&transaction.message().recent_blockhash) }) diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 9f689ae630..e66af7a7c9 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -16,8 +16,8 @@ use solana_metrics::{datapoint_error, inc_new_counter_debug}; use solana_rayon_threadlimit::get_thread_count; use solana_runtime::{ bank::{ - Bank, InnerInstructionsList, TransactionBalancesSet, TransactionLogMessages, - TransactionProcessResult, TransactionResults, + Bank, InnerInstructionsList, TransactionBalancesSet, TransactionExecutionResult, + TransactionLogMessages, TransactionResults, }, bank_forks::BankForks, bank_utils, @@ -115,7 +115,7 @@ fn execute_batch( let TransactionResults { fee_collection_results, - processing_results, + execution_results, .. } = tx_results; @@ -124,7 +124,7 @@ fn execute_batch( bank.clone(), batch.transactions(), batch.iteration_order_vec(), - processing_results, + execution_results, balances, inner_instructions, transaction_logs, @@ -1029,7 +1029,7 @@ pub struct TransactionStatusBatch { pub bank: Arc, pub transactions: Vec, pub iteration_order: Option>, - pub statuses: Vec, + pub statuses: Vec, pub balances: TransactionBalancesSet, pub inner_instructions: Vec>, pub transaction_logs: Vec, @@ -1041,7 +1041,7 @@ pub fn send_transaction_status_batch( bank: Arc, transactions: &[Transaction], iteration_order: Option>, - statuses: Vec, + statuses: Vec, balances: TransactionBalancesSet, inner_instructions: Vec>, transaction_logs: Vec, diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 1d364f723c..a7680589ab 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -4,7 +4,9 @@ use crate::{ }, accounts_index::{AccountsIndex, Ancestors}, append_vec::StoredAccount, - bank::{HashAgeKind, TransactionProcessResult}, + bank::{ + NonceRollbackFull, NonceRollbackInfo, TransactionCheckResult, TransactionExecutionResult, + }, blockhash_queue::BlockhashQueue, rent_collector::RentCollector, system_instruction_processor::{get_system_account_kind, SystemAccountKind}, @@ -63,7 +65,10 @@ pub type TransactionAccounts = Vec; pub type TransactionRent = u64; pub type TransactionLoaders = Vec>; -pub type TransactionLoadResult = (TransactionAccounts, TransactionLoaders, TransactionRent); +pub type TransactionLoadResult = ( + Result<(TransactionAccounts, TransactionLoaders, TransactionRent)>, + Option, +); pub enum AccountAddressFilter { Exclude, // exclude all addresses matching the filter @@ -147,7 +152,7 @@ impl Accounts { let rent_fix_enabled = feature_set.cumulative_rent_related_fixes_enabled(); for (i, key) in message.account_keys.iter().enumerate() { - let account = if Self::is_non_loader_key(message, key, i) { + let account = if message.is_non_loader_key(key, i) { if payer_index.is_none() { payer_index = Some(i); } @@ -300,12 +305,12 @@ impl Accounts { ancestors: &Ancestors, txs: &[Transaction], txs_iteration_order: Option<&[usize]>, - lock_results: Vec, + lock_results: Vec, hash_queue: &BlockhashQueue, error_counters: &mut ErrorCounters, rent_collector: &RentCollector, feature_set: &FeatureSet, - ) -> Vec<(Result, Option)> { + ) -> Vec { let accounts_index = &self.accounts_db.accounts_index; let fee_config = FeeConfig { @@ -315,10 +320,10 @@ impl Accounts { OrderedIterator::new(txs, txs_iteration_order) .zip(lock_results.into_iter()) .map(|etx| match etx { - ((_, tx), (Ok(()), hash_age_kind)) => { - let fee_calculator = hash_age_kind + ((_, tx), (Ok(()), nonce_rollback)) => { + let fee_calculator = nonce_rollback .as_ref() - .and_then(|hash_age_kind| hash_age_kind.fee_calculator()) + .map(|nonce_rollback| nonce_rollback.fee_calculator()) .unwrap_or_else(|| { hash_queue .get_fee_calculator(&tx.message().recent_blockhash) @@ -327,7 +332,7 @@ impl Accounts { let fee = if let Some(fee_calculator) = fee_calculator { fee_calculator.calculate_fee_with_config(tx.message(), &fee_config) } else { - return (Err(TransactionError::BlockhashNotFound), hash_age_kind); + return (Err(TransactionError::BlockhashNotFound), None); }; let load_res = self.load_tx_accounts( @@ -342,7 +347,7 @@ impl Accounts { ); let (accounts, rents) = match load_res { Ok((a, r)) => (a, r), - Err(e) => return (Err(e), hash_age_kind), + Err(e) => return (Err(e), None), }; let load_res = Self::load_loaders( @@ -354,53 +359,26 @@ impl Accounts { ); let loaders = match load_res { Ok(loaders) => loaders, - Err(e) => return (Err(e), hash_age_kind), + Err(e) => return (Err(e), None), }; - // Update hash_age_kind with fee-subtracted accounts - let hash_age_kind = if let Some(hash_age_kind) = hash_age_kind { - match hash_age_kind { - HashAgeKind::Extant => Some(HashAgeKind::Extant), - HashAgeKind::DurableNoncePartial(pubkey, account) => { - let fee_payer = tx - .message() - .account_keys - .iter() - .enumerate() - .find(|(i, k)| Self::is_non_loader_key(tx.message(), k, *i)) - .map(|(i, k)| (*k, accounts[i].clone())); - if let Some((fee_pubkey, fee_account)) = fee_payer { - if fee_pubkey == pubkey { - Some(HashAgeKind::DurableNonceFull( - pubkey, - fee_account, - None, - )) - } else { - Some(HashAgeKind::DurableNonceFull( - pubkey, - account, - Some(fee_account), - )) - } - } else { - return ( - Err(TransactionError::AccountNotFound), - Some(HashAgeKind::DurableNoncePartial(pubkey, account)), - ); - } - } - HashAgeKind::DurableNonceFull(_, _, _) => { - panic!("update: unexpected HashAgeKind variant") - } + // Update nonce_rollback with fee-subtracted accounts + let nonce_rollback = if let Some(nonce_rollback) = nonce_rollback { + match NonceRollbackFull::from_partial( + nonce_rollback, + tx.message(), + &accounts, + ) { + Ok(nonce_rollback) => Some(nonce_rollback), + Err(e) => return (Err(e), None), } } else { None }; - (Ok((accounts, loaders, rents)), hash_age_kind) + (Ok((accounts, loaders, rents)), nonce_rollback) } - (_, (Err(e), hash_age_kind)) => (Err(e), hash_age_kind), + (_, (Err(e), _nonce_rollback)) => (Err(e), None), }) .collect() } @@ -785,8 +763,8 @@ impl Accounts { slot: Slot, txs: &[Transaction], txs_iteration_order: Option<&[usize]>, - res: &[TransactionProcessResult], - loaded: &mut [(Result, Option)], + res: &[TransactionExecutionResult], + loaded: &mut [TransactionLoadResult], rent_collector: &RentCollector, last_blockhash_with_fee_calculator: &(Hash, FeeCalculator), fix_recent_blockhashes_sysvar_delay: bool, @@ -815,23 +793,19 @@ impl Accounts { self.accounts_db.add_root(slot) } - fn is_non_loader_key(message: &Message, key: &Pubkey, key_index: usize) -> bool { - !message.program_ids().contains(&key) || message.is_key_passed_to_program(key_index) - } - fn collect_accounts_to_store<'a>( &self, txs: &'a [Transaction], txs_iteration_order: Option<&'a [usize]>, - res: &'a [TransactionProcessResult], - loaded: &'a mut [(Result, Option)], + res: &'a [TransactionExecutionResult], + loaded: &'a mut [TransactionLoadResult], rent_collector: &RentCollector, last_blockhash_with_fee_calculator: &(Hash, FeeCalculator), fix_recent_blockhashes_sysvar_delay: bool, rent_fix_enabled: bool, ) -> Vec<(&'a Pubkey, &'a Account)> { let mut accounts = Vec::with_capacity(loaded.len()); - for (i, ((raccs, _hash_age_kind), (_, tx))) in loaded + for (i, ((raccs, _nonce_rollback), (_, tx))) in loaded .iter_mut() .zip(OrderedIterator::new(txs, txs_iteration_order)) .enumerate() @@ -839,46 +813,52 @@ impl Accounts { if raccs.is_err() { continue; } - let (res, hash_age_kind) = &res[i]; - let maybe_nonce = match (res, hash_age_kind) { - (Ok(_), Some(HashAgeKind::DurableNonceFull(pubkey, acc, maybe_fee_account))) => { + let (res, nonce_rollback) = &res[i]; + let maybe_nonce_rollback = match (res, nonce_rollback) { + (Ok(_), Some(nonce_rollback)) => { + let pubkey = nonce_rollback.nonce_address(); + let acc = nonce_rollback.nonce_account(); + let maybe_fee_account = nonce_rollback.fee_account(); Some((pubkey, acc, maybe_fee_account)) } - ( - Err(TransactionError::InstructionError(_, _)), - Some(HashAgeKind::DurableNonceFull(pubkey, acc, maybe_fee_account)), - ) => Some((pubkey, acc, maybe_fee_account)), - (_, Some(HashAgeKind::DurableNoncePartial(_, _))) => { - panic!("collect: unexpected HashAgeKind variant") + (Err(TransactionError::InstructionError(_, _)), Some(nonce_rollback)) => { + let pubkey = nonce_rollback.nonce_address(); + let acc = nonce_rollback.nonce_account(); + let maybe_fee_account = nonce_rollback.fee_account(); + Some((pubkey, acc, maybe_fee_account)) } - (Ok(_), _hash_age_kind) => None, - (Err(_), _hash_age_kind) => continue, + (Ok(_), _nonce_rollback) => None, + (Err(_), _nonce_rollback) => continue, }; let message = &tx.message(); let acc = raccs.as_mut().unwrap(); + let mut fee_payer_index = None; for ((i, key), account) in message .account_keys .iter() .enumerate() .zip(acc.0.iter_mut()) - .filter(|((i, key), _account)| Self::is_non_loader_key(message, key, *i)) + .filter(|((i, key), _account)| message.is_non_loader_key(key, *i)) { let is_nonce_account = prepare_if_nonce_account( account, key, res, - maybe_nonce, + maybe_nonce_rollback, last_blockhash_with_fee_calculator, fix_recent_blockhashes_sysvar_delay, ); - let is_fee_payer = i == 0; + if fee_payer_index.is_none() { + fee_payer_index = Some(i); + } + let is_fee_payer = Some(i) == fee_payer_index; if message.is_writable(i) && (res.is_ok() - || (maybe_nonce.is_some() && (is_nonce_account || is_fee_payer))) + || (maybe_nonce_rollback.is_some() && (is_nonce_account || is_fee_payer))) { if res.is_err() { - match (is_nonce_account, is_fee_payer, maybe_nonce) { + match (is_nonce_account, is_fee_payer, maybe_nonce_rollback) { // nonce is fee-payer, state updated in `prepare_if_nonce_account()` (true, true, Some((_, _, None))) => (), // nonce not fee-payer, state updated in `prepare_if_nonce_account()` @@ -887,7 +867,7 @@ impl Accounts { (false, true, Some((_, _, Some(fee_payer_account)))) => { *account = fee_payer_account.clone(); } - _ => unreachable!(), + _ => panic!("unexpected nonce_rollback condition"), } } if account.rent_epoch == 0 { @@ -909,11 +889,11 @@ pub fn prepare_if_nonce_account( account: &mut Account, account_pubkey: &Pubkey, tx_result: &Result<()>, - maybe_nonce: Option<(&Pubkey, &Account, &Option)>, + maybe_nonce_rollback: Option<(&Pubkey, &Account, Option<&Account>)>, last_blockhash_with_fee_calculator: &(Hash, FeeCalculator), fix_recent_blockhashes_sysvar_delay: bool, ) -> bool { - if let Some((nonce_key, nonce_acc, _maybe_fee_account)) = maybe_nonce { + if let Some((nonce_key, nonce_acc, _maybe_fee_account)) = maybe_nonce_rollback { if account_pubkey == nonce_key { let overwrite = if tx_result.is_err() { // Nonce TX failed with an InstructionError. Roll back @@ -974,7 +954,7 @@ mod tests { // TODO: all the bank tests are bank specific, issue: 2194 use super::*; - use crate::{bank::HashAgeKind, rent_collector::RentCollector}; + use crate::rent_collector::RentCollector; use solana_sdk::{ account::Account, epoch_schedule::EpochSchedule, @@ -999,7 +979,7 @@ mod tests { fee_calculator: &FeeCalculator, rent_collector: &RentCollector, error_counters: &mut ErrorCounters, - ) -> Vec<(Result, Option)> { + ) -> Vec { let mut hash_queue = BlockhashQueue::new(100); hash_queue.register_hash(&tx.message().recent_blockhash, &fee_calculator); let accounts = Accounts::new(Vec::new(), &ClusterType::Development); @@ -1012,7 +992,7 @@ mod tests { &ancestors, &[tx], None, - vec![(Ok(()), Some(HashAgeKind::Extant))], + vec![(Ok(()), None)], &hash_queue, error_counters, rent_collector, @@ -1025,7 +1005,7 @@ mod tests { ka: &[(Pubkey, Account)], fee_calculator: &FeeCalculator, error_counters: &mut ErrorCounters, - ) -> Vec<(Result, Option)> { + ) -> Vec { let rent_collector = RentCollector::default(); load_accounts_with_fee_and_rent(tx, ka, fee_calculator, &rent_collector, error_counters) } @@ -1034,7 +1014,7 @@ mod tests { tx: Transaction, ka: &[(Pubkey, Account)], error_counters: &mut ErrorCounters, - ) -> Vec<(Result, Option)> { + ) -> Vec { let fee_calculator = FeeCalculator::default(); load_accounts_with_fee(tx, ka, &fee_calculator, error_counters) } @@ -1059,10 +1039,7 @@ mod tests { assert_eq!(loaded_accounts.len(), 1); assert_eq!( loaded_accounts[0], - ( - Err(TransactionError::AccountNotFound), - Some(HashAgeKind::Extant) - ) + (Err(TransactionError::AccountNotFound), None,) ); } @@ -1088,10 +1065,7 @@ mod tests { assert_eq!(loaded_accounts.len(), 1); assert_eq!( loaded_accounts[0], - ( - Err(TransactionError::AccountNotFound), - Some(HashAgeKind::Extant) - ), + (Err(TransactionError::AccountNotFound), None,), ); } @@ -1125,10 +1099,7 @@ mod tests { assert_eq!(loaded_accounts.len(), 1); assert_eq!( loaded_accounts[0], - ( - Err(TransactionError::ProgramAccountNotFound), - Some(HashAgeKind::Extant) - ) + (Err(TransactionError::ProgramAccountNotFound), None,) ); } @@ -1162,10 +1133,7 @@ mod tests { assert_eq!(loaded_accounts.len(), 1); assert_eq!( loaded_accounts[0].clone(), - ( - Err(TransactionError::InsufficientFundsForFee), - Some(HashAgeKind::Extant) - ), + (Err(TransactionError::InsufficientFundsForFee), None,), ); } @@ -1195,10 +1163,7 @@ mod tests { assert_eq!(loaded_accounts.len(), 1); assert_eq!( loaded_accounts[0], - ( - Err(TransactionError::InvalidAccountForFee), - Some(HashAgeKind::Extant) - ), + (Err(TransactionError::InvalidAccountForFee), None,), ); } @@ -1246,7 +1211,7 @@ mod tests { &mut error_counters, ); assert_eq!(loaded_accounts.len(), 1); - let (load_res, _hash_age_kind) = &loaded_accounts[0]; + let (load_res, _nonce_rollback) = &loaded_accounts[0]; let (tx_accounts, _loaders, _rents) = load_res.as_ref().unwrap(); assert_eq!(tx_accounts[0].lamports, min_balance); @@ -1260,7 +1225,7 @@ mod tests { &mut error_counters, ); assert_eq!(loaded_accounts.len(), 1); - let (load_res, _hash_age_kind) = &loaded_accounts[0]; + let (load_res, _nonce_rollback) = &loaded_accounts[0]; assert_eq!(*load_res, Err(TransactionError::InsufficientFundsForFee)); // Fee leaves non-zero, but sub-min_balance balance fails @@ -1273,7 +1238,7 @@ mod tests { &mut error_counters, ); assert_eq!(loaded_accounts.len(), 1); - let (load_res, _hash_age_kind) = &loaded_accounts[0]; + let (load_res, _nonce_rollback) = &loaded_accounts[0]; assert_eq!(*load_res, Err(TransactionError::InsufficientFundsForFee)); } @@ -1310,14 +1275,14 @@ mod tests { match &loaded_accounts[0] { ( Ok((transaction_accounts, transaction_loaders, _transaction_rents)), - _hash_age_kind, + _nonce_rollback, ) => { assert_eq!(transaction_accounts.len(), 3); assert_eq!(transaction_accounts[0], accounts[0].1); assert_eq!(transaction_loaders.len(), 1); assert_eq!(transaction_loaders[0].len(), 0); } - (Err(e), _hash_age_kind) => Err(e).unwrap(), + (Err(e), _nonce_rollback) => Err(e).unwrap(), } } @@ -1383,10 +1348,7 @@ mod tests { assert_eq!(loaded_accounts.len(), 1); assert_eq!( loaded_accounts[0], - ( - Err(TransactionError::CallChainTooDeep), - Some(HashAgeKind::Extant) - ) + (Err(TransactionError::CallChainTooDeep), None,) ); } @@ -1421,10 +1383,7 @@ mod tests { assert_eq!(loaded_accounts.len(), 1); assert_eq!( loaded_accounts[0], - ( - Err(TransactionError::InvalidProgramForExecution), - Some(HashAgeKind::Extant) - ) + (Err(TransactionError::InvalidProgramForExecution), None,) ); } @@ -1459,10 +1418,7 @@ mod tests { assert_eq!(loaded_accounts.len(), 1); assert_eq!( loaded_accounts[0], - ( - Err(TransactionError::ProgramAccountNotFound), - Some(HashAgeKind::Extant) - ) + (Err(TransactionError::ProgramAccountNotFound), None,) ); } @@ -1496,10 +1452,7 @@ mod tests { assert_eq!(loaded_accounts.len(), 1); assert_eq!( loaded_accounts[0], - ( - Err(TransactionError::InvalidProgramForExecution), - Some(HashAgeKind::Extant) - ) + (Err(TransactionError::InvalidProgramForExecution), None,) ); } @@ -1548,7 +1501,7 @@ mod tests { match &loaded_accounts[0] { ( Ok((transaction_accounts, transaction_loaders, _transaction_rents)), - _hash_age_kind, + _nonce_rollback, ) => { assert_eq!(transaction_accounts.len(), 3); assert_eq!(transaction_accounts[0], accounts[0].1); @@ -1562,7 +1515,7 @@ mod tests { } } } - (Err(e), _hash_age_kind) => Err(e).unwrap(), + (Err(e), _nonce_rollback) => Err(e).unwrap(), } } @@ -1834,10 +1787,7 @@ mod tests { let tx1 = Transaction::new(&[&keypair1], message, Hash::default()); let txs = vec![tx0, tx1]; - let loaders = vec![ - (Ok(()), Some(HashAgeKind::Extant)), - (Ok(()), Some(HashAgeKind::Extant)), - ]; + let loaders = vec![(Ok(()), None), (Ok(()), None)]; let account0 = Account::new(1, 0, &Pubkey::default()); let account1 = Account::new(2, 0, &Pubkey::default()); @@ -1852,7 +1802,7 @@ mod tests { transaction_loaders0, transaction_rent0, )), - Some(HashAgeKind::Extant), + None, ); let transaction_accounts1 = vec![account1, account2]; @@ -1864,7 +1814,7 @@ mod tests { transaction_loaders1, transaction_rent1, )), - Some(HashAgeKind::Extant), + None, ); let mut loaded = vec![loaded0, loaded1]; @@ -1940,10 +1890,7 @@ mod tests { accounts.accounts_db.clean_accounts(None); } - fn load_accounts_no_store( - accounts: &Accounts, - tx: Transaction, - ) -> Vec<(Result, Option)> { + fn load_accounts_no_store(accounts: &Accounts, tx: Transaction) -> Vec { let rent_collector = RentCollector::default(); let fee_calculator = FeeCalculator::new(10); let mut hash_queue = BlockhashQueue::new(100); @@ -1955,7 +1902,7 @@ mod tests { &ancestors, &[tx], None, - vec![(Ok(()), Some(HashAgeKind::Extant))], + vec![(Ok(()), None)], &hash_queue, &mut error_counters, &rent_collector, @@ -2016,12 +1963,12 @@ mod tests { account: &mut Account, account_pubkey: &Pubkey, tx_result: &Result<()>, - maybe_nonce: Option<(&Pubkey, &Account, &Option)>, + maybe_nonce_rollback: Option<(&Pubkey, &Account, Option<&Account>)>, last_blockhash_with_fee_calculator: &(Hash, FeeCalculator), expect_account: &Account, ) -> bool { // Verify expect_account's relationship - match maybe_nonce { + match maybe_nonce_rollback { Some((nonce_pubkey, _nonce_account, _maybe_fee_account)) if nonce_pubkey == account_pubkey && tx_result.is_ok() => { @@ -2039,7 +1986,7 @@ mod tests { account, account_pubkey, tx_result, - maybe_nonce, + maybe_nonce_rollback, last_blockhash_with_fee_calculator, true, ); @@ -2068,7 +2015,11 @@ mod tests { &mut post_account, &post_account_pubkey, &Ok(()), - Some((&pre_account_pubkey, &pre_account, &maybe_fee_account)), + Some(( + &pre_account_pubkey, + &pre_account, + maybe_fee_account.as_ref() + )), &(last_blockhash, last_fee_calculator), &expect_account, )); @@ -2115,7 +2066,11 @@ mod tests { &mut post_account, &Pubkey::new(&[1u8; 32]), &Ok(()), - Some((&pre_account_pubkey, &pre_account, &maybe_fee_account)), + Some(( + &pre_account_pubkey, + &pre_account, + maybe_fee_account.as_ref() + )), &(last_blockhash, last_fee_calculator), &expect_account, )); @@ -2151,7 +2106,11 @@ mod tests { 0, InstructionError::InvalidArgument, )), - Some((&pre_account_pubkey, &pre_account, &maybe_fee_account)), + Some(( + &pre_account_pubkey, + &pre_account, + maybe_fee_account.as_ref() + )), &(last_blockhash, last_fee_calculator), &expect_account, )); @@ -2185,7 +2144,7 @@ mod tests { let nonce_account_pre = Account::new_data(42, &nonce_state, &system_program::id()).unwrap(); let from_account_pre = Account::new(4242, 0, &Pubkey::default()); - let hash_age_kind = Some(HashAgeKind::DurableNonceFull( + let nonce_rollback = Some(NonceRollbackFull::new( nonce_address, nonce_account_pre.clone(), Some(from_account_pre.clone()), @@ -2195,7 +2154,7 @@ mod tests { 1, InstructionError::InvalidArgument, )), - hash_age_kind.clone(), + nonce_rollback.clone(), )]; let nonce_state = @@ -2223,7 +2182,7 @@ mod tests { let transaction_rent = 0; let loaded = ( Ok((transaction_accounts, transaction_loaders, transaction_rent)), - hash_age_kind, + nonce_rollback, ); let mut loaded = vec![loaded]; @@ -2290,7 +2249,7 @@ mod tests { })); let nonce_account_pre = Account::new_data(42, &nonce_state, &system_program::id()).unwrap(); - let hash_age_kind = Some(HashAgeKind::DurableNonceFull( + let nonce_rollback = Some(NonceRollbackFull::new( nonce_address, nonce_account_pre.clone(), None, @@ -2300,7 +2259,7 @@ mod tests { 1, InstructionError::InvalidArgument, )), - hash_age_kind.clone(), + nonce_rollback.clone(), )]; let nonce_state = @@ -2328,7 +2287,7 @@ mod tests { let transaction_rent = 0; let loaded = ( Ok((transaction_accounts, transaction_loaders, transaction_rent)), - hash_age_kind, + nonce_rollback, ); let mut loaded = vec![loaded]; diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 7a2f8fb606..c51a1b1742 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -371,10 +371,11 @@ impl StatusCacheRc { } } -pub type TransactionProcessResult = (Result<()>, Option); +pub type TransactionCheckResult = (Result<()>, Option); +pub type TransactionExecutionResult = (Result<()>, Option); pub struct TransactionResults { pub fee_collection_results: Vec>, - pub processing_results: Vec, + pub execution_results: Vec, pub overwritten_vote_accounts: Vec, } pub struct TransactionBalancesSet { @@ -446,33 +447,111 @@ pub struct TransactionLogCollector { pub mentioned_address_map: HashMap>, } -#[derive(Clone, Debug, Eq, PartialEq)] -pub enum HashAgeKind { - Extant, - DurableNoncePartial(Pubkey, Account), - DurableNonceFull(Pubkey, Account, Option), +pub trait NonceRollbackInfo { + fn nonce_address(&self) -> &Pubkey; + fn nonce_account(&self) -> &Account; + fn fee_calculator(&self) -> Option; + fn fee_account(&self) -> Option<&Account>; } -impl HashAgeKind { - pub fn is_durable_nonce(&self) -> bool { - match self { - Self::Extant => false, - Self::DurableNoncePartial(_, _) => true, - Self::DurableNonceFull(_, _, _) => true, +#[derive(Clone, Debug, Default, PartialEq)] +pub struct NonceRollbackPartial { + nonce_address: Pubkey, + nonce_account: Account, +} + +impl NonceRollbackPartial { + pub fn new(nonce_address: Pubkey, nonce_account: Account) -> Self { + Self { + nonce_address, + nonce_account, } } +} - pub fn fee_calculator(&self) -> Option> { - match self { - Self::Extant => None, - Self::DurableNoncePartial(_, account) => { - Some(nonce_account::fee_calculator_of(account)) - } - Self::DurableNonceFull(_, account, _) => { - Some(nonce_account::fee_calculator_of(account)) - } +impl NonceRollbackInfo for NonceRollbackPartial { + fn nonce_address(&self) -> &Pubkey { + &self.nonce_address + } + fn nonce_account(&self) -> &Account { + &self.nonce_account + } + fn fee_calculator(&self) -> Option { + nonce_account::fee_calculator_of(&self.nonce_account) + } + fn fee_account(&self) -> Option<&Account> { + None + } +} + +#[derive(Clone, Debug, Default, PartialEq)] +pub struct NonceRollbackFull { + nonce_address: Pubkey, + nonce_account: Account, + fee_account: Option, +} + +impl NonceRollbackFull { + #[cfg(test)] + pub fn new( + nonce_address: Pubkey, + nonce_account: Account, + fee_account: Option, + ) -> Self { + Self { + nonce_address, + nonce_account, + fee_account, } } + pub fn from_partial( + partial: NonceRollbackPartial, + message: &Message, + accounts: &[Account], + ) -> Result { + let NonceRollbackPartial { + nonce_address, + nonce_account, + } = partial; + let fee_payer = message + .account_keys + .iter() + .enumerate() + .find(|(i, k)| message.is_non_loader_key(k, *i)) + .and_then(|(i, k)| accounts.get(i).cloned().map(|a| (*k, a))); + if let Some((fee_pubkey, fee_account)) = fee_payer { + if fee_pubkey == nonce_address { + Ok(Self { + nonce_address, + nonce_account: fee_account, + fee_account: None, + }) + } else { + Ok(Self { + nonce_address, + nonce_account, + fee_account: Some(fee_account), + }) + } + } else { + Err(TransactionError::AccountNotFound) + } + } +} + +impl NonceRollbackInfo for NonceRollbackFull { + fn nonce_address(&self) -> &Pubkey { + &self.nonce_address + } + fn nonce_account(&self) -> &Account { + &self.nonce_account + } + fn fee_calculator(&self) -> Option { + nonce_account::fee_calculator_of(&self.nonce_account) + } + fn fee_account(&self) -> Option<&Account> { + self.fee_account.as_ref() + } } // Bank's common fields shared by all supported snapshot versions for deserialization. @@ -2089,11 +2168,11 @@ impl Bank { &self, txs: &[Transaction], iteration_order: Option<&[usize]>, - res: &[TransactionProcessResult], + res: &[TransactionExecutionResult], ) { let mut status_cache = self.src.status_cache.write().unwrap(); for (i, (_, tx)) in OrderedIterator::new(txs, iteration_order).enumerate() { - let (res, _hash_age_kind) = &res[i]; + let (res, _nonce_rollback) = &res[i]; if Self::can_commit(res) && !tx.signatures.is_empty() { status_cache.insert( &tx.message().recent_blockhash, @@ -2227,9 +2306,9 @@ impl Bank { &self, txs: &[Transaction], iteration_order: Option<&[usize]>, - results: Vec, + results: Vec, error_counters: &mut ErrorCounters, - ) -> Vec<(Result, Option)> { + ) -> Vec { self.rc.accounts.load_accounts( &self.ancestors, txs, @@ -2249,7 +2328,7 @@ impl Bank { lock_results: Vec>, max_age: usize, error_counters: &mut ErrorCounters, - ) -> Vec { + ) -> Vec { let hash_queue = self.blockhash_queue.read().unwrap(); OrderedIterator::new(txs, iteration_order) .zip(lock_results.into_iter()) @@ -2258,9 +2337,9 @@ impl Bank { let message = tx.message(); let hash_age = hash_queue.check_hash_age(&message.recent_blockhash, max_age); if hash_age == Some(true) { - (Ok(()), Some(HashAgeKind::Extant)) + (Ok(()), None) } else if let Some((pubkey, acc)) = self.check_tx_durable_nonce(&tx) { - (Ok(()), Some(HashAgeKind::DurableNoncePartial(pubkey, acc))) + (Ok(()), Some(NonceRollbackPartial::new(pubkey, acc))) } else if hash_age == Some(false) { error_counters.blockhash_too_old += 1; (Err(TransactionError::BlockhashNotFound), None) @@ -2278,9 +2357,9 @@ impl Bank { &self, txs: &[Transaction], iteration_order: Option<&[usize]>, - lock_results: Vec, + lock_results: Vec, error_counters: &mut ErrorCounters, - ) -> Vec { + ) -> Vec { let rcache = self.src.status_cache.read().unwrap(); OrderedIterator::new(txs, iteration_order) .zip(lock_results.into_iter()) @@ -2289,7 +2368,7 @@ impl Bank { return lock_res; } { - let (lock_res, hash_age_kind) = &lock_res; + let (lock_res, _nonce_rollback) = &lock_res; if lock_res.is_ok() && rcache .get_signature_status( @@ -2300,10 +2379,7 @@ impl Bank { .is_some() { error_counters.duplicate_signature += 1; - return ( - Err(TransactionError::DuplicateSignature), - hash_age_kind.clone(), - ); + return (Err(TransactionError::DuplicateSignature), None); } } lock_res @@ -2315,9 +2391,9 @@ impl Bank { &self, txs: &[Transaction], iteration_order: Option<&[usize]>, - lock_results: Vec, + lock_results: Vec, error_counters: &mut ErrorCounters, - ) -> Vec { + ) -> Vec { OrderedIterator::new(txs, iteration_order) .zip(lock_results.into_iter()) .map(|((_, tx), lock_res)| { @@ -2373,7 +2449,7 @@ impl Bank { lock_results: &[Result<()>], max_age: usize, mut error_counters: &mut ErrorCounters, - ) -> Vec { + ) -> Vec { let age_results = self.check_age( txs, iteration_order, @@ -2599,8 +2675,8 @@ impl Bank { enable_cpi_recording: bool, enable_log_recording: bool, ) -> ( - Vec<(Result, Option)>, - Vec, + Vec, + Vec, Vec>, Vec, Vec, @@ -2650,12 +2726,12 @@ impl Bank { .bpf_compute_budget .unwrap_or_else(|| BpfComputeBudget::new(&self.feature_set)); - let executed: Vec = loaded_accounts + let executed: Vec = loaded_accounts .iter_mut() .zip(OrderedIterator::new(txs, batch.iteration_order())) .map(|(accs, (_, tx))| match accs { - (Err(e), hash_age_kind) => (Err(e.clone()), hash_age_kind.clone()), - (Ok((accounts, loaders, _rents)), hash_age_kind) => { + (Err(e), _nonce_rollback) => (Err(e.clone()), None), + (Ok((accounts, loaders, _rents)), nonce_rollback) => { signature_count += u64::from(tx.message().header.num_required_signatures); let executors = self.get_executors(&tx.message, &loaders); @@ -2714,10 +2790,16 @@ impl Bank { self.update_executors(executors); - if let Err(TransactionError::InstructionError(_, _)) = &process_result { - error_counters.instruction_error += 1; - } - (process_result, hash_age_kind.clone()) + let nonce_rollback = + if let Err(TransactionError::InstructionError(_, _)) = &process_result { + error_counters.instruction_error += 1; + nonce_rollback.clone() + } else if process_result.is_err() { + None + } else { + nonce_rollback.clone() + }; + (process_result, nonce_rollback) } }) .collect(); @@ -2736,7 +2818,7 @@ impl Bank { let transaction_log_collector_config = self.transaction_log_collector_config.read().unwrap(); - for (i, ((r, _hash_age_kind), tx)) in executed.iter().zip(txs.iter()).enumerate() { + for (i, ((r, _nonce_rollback), tx)) in executed.iter().zip(txs.iter()).enumerate() { if let Some(debug_keys) = &self.transaction_debug_keys { for key in &tx.message.account_keys { if debug_keys.contains(key) { @@ -2821,7 +2903,7 @@ impl Bank { &self, txs: &[Transaction], iteration_order: Option<&[usize]>, - executed: &[TransactionProcessResult], + executed: &[TransactionExecutionResult], ) -> Vec> { let hash_queue = self.blockhash_queue.read().unwrap(); let mut fees = 0; @@ -2832,10 +2914,10 @@ impl Bank { let results = OrderedIterator::new(txs, iteration_order) .zip(executed.iter()) - .map(|((_, tx), (res, hash_age_kind))| { - let (fee_calculator, is_durable_nonce) = hash_age_kind + .map(|((_, tx), (res, nonce_rollback))| { + let (fee_calculator, is_durable_nonce) = nonce_rollback .as_ref() - .and_then(|hash_age_kind| hash_age_kind.fee_calculator()) + .map(|nonce_rollback| nonce_rollback.fee_calculator()) .map(|maybe_fee_calculator| (maybe_fee_calculator, true)) .unwrap_or_else(|| { ( @@ -2881,8 +2963,8 @@ impl Bank { &self, txs: &[Transaction], iteration_order: Option<&[usize]>, - loaded_accounts: &mut [(Result, Option)], - executed: &[TransactionProcessResult], + loaded_accounts: &mut [TransactionLoadResult], + executed: &[TransactionExecutionResult], tx_count: u64, signature_count: u64, ) -> TransactionResults { @@ -2899,7 +2981,7 @@ impl Bank { if executed .iter() - .any(|(res, _hash_age_kind)| Self::can_commit(res)) + .any(|(res, _nonce_rollback)| Self::can_commit(res)) { self.is_delta.store(true, Relaxed); } @@ -2930,7 +3012,7 @@ impl Bank { TransactionResults { fee_collection_results, - processing_results: executed.to_vec(), + execution_results: executed.to_vec(), overwritten_vote_accounts, } } @@ -3079,12 +3161,12 @@ impl Bank { fn collect_rent( &self, - res: &[TransactionProcessResult], - loaded_accounts: &[(Result, Option)], + res: &[TransactionExecutionResult], + loaded_accounts: &[TransactionLoadResult], ) { let mut collected_rent: u64 = 0; - for (i, (raccs, _hash_age_kind)) in loaded_accounts.iter().enumerate() { - let (res, _hash_age_kind) = &res[i]; + for (i, (raccs, _nonce_rollback)) in loaded_accounts.iter().enumerate() { + let (res, _nonce_rollback) = &res[i]; if res.is_err() || raccs.is_err() { continue; } @@ -4051,16 +4133,16 @@ impl Bank { &self, txs: &[Transaction], iteration_order: Option<&[usize]>, - res: &[TransactionProcessResult], - loaded: &[(Result, Option)], + res: &[TransactionExecutionResult], + loaded: &[TransactionLoadResult], ) -> Vec { let mut overwritten_vote_accounts = vec![]; - for (i, ((raccs, _load_hash_age_kind), (transaction_index, tx))) in loaded + for (i, ((raccs, _load_nonce_rollback), (transaction_index, tx))) in loaded .iter() .zip(OrderedIterator::new(txs, iteration_order)) .enumerate() { - let (res, _res_hash_age_kind) = &res[i]; + let (res, _res_nonce_rollback) = &res[i]; if res.is_err() || raccs.is_err() { continue; } @@ -4550,7 +4632,7 @@ pub(crate) mod tests { poh_config::PohConfig, process_instruction::InvokeContext, rent::Rent, - signature::{Keypair, Signer}, + signature::{keypair_from_seed, Keypair, Signer}, system_instruction::{self, SystemError}, system_program, sysvar::{fees::Fees, rewards::Rewards}, @@ -4568,57 +4650,67 @@ pub(crate) mod tests { use std::{result, thread::Builder, time::Duration}; #[test] - fn test_hash_age_kind_is_durable_nonce() { - assert!( - HashAgeKind::DurableNoncePartial(Pubkey::default(), Account::default()) - .is_durable_nonce() - ); - assert!( - HashAgeKind::DurableNonceFull(Pubkey::default(), Account::default(), None) - .is_durable_nonce() - ); - assert!(HashAgeKind::DurableNonceFull( - Pubkey::default(), - Account::default(), - Some(Account::default()) - ) - .is_durable_nonce()); - assert!(!HashAgeKind::Extant.is_durable_nonce()); - } - - #[test] - fn test_hash_age_kind_fee_calculator() { + fn test_nonce_rollback_info() { + let nonce_authority = keypair_from_seed(&[0; 32]).unwrap(); + let nonce_address = nonce_authority.pubkey(); + let fee_calculator = FeeCalculator::new(42); let state = nonce::state::Versions::new_current(nonce::State::Initialized(nonce::state::Data { authority: Pubkey::default(), blockhash: Hash::new_unique(), - fee_calculator: FeeCalculator::default(), + fee_calculator: fee_calculator.clone(), })); - let account = Account::new_data(42, &state, &system_program::id()).unwrap(); + let nonce_account = Account::new_data(43, &state, &system_program::id()).unwrap(); - assert_eq!(HashAgeKind::Extant.fee_calculator(), None); + // NonceRollbackPartial create + NonceRollbackInfo impl + let partial = NonceRollbackPartial::new(nonce_address, nonce_account.clone()); + assert_eq!(*partial.nonce_address(), nonce_address); + assert_eq!(*partial.nonce_account(), nonce_account); + assert_eq!(partial.fee_calculator(), Some(fee_calculator.clone())); + assert_eq!(partial.fee_account(), None); + + let from = keypair_from_seed(&[1; 32]).unwrap(); + let from_address = from.pubkey(); + let to_address = Pubkey::new_unique(); + let instructions = vec![ + system_instruction::advance_nonce_account(&nonce_address, &nonce_authority.pubkey()), + system_instruction::transfer(&from_address, &to_address, 42), + ]; + let message = Message::new(&instructions, Some(&from_address)); + + let from_account = Account::new(44, 0, &Pubkey::default()); + let to_account = Account::new(45, 0, &Pubkey::default()); + let recent_blockhashes_sysvar_account = Account::new(4, 0, &Pubkey::default()); + let accounts = [ + from_account.clone(), + nonce_account.clone(), + to_account.clone(), + recent_blockhashes_sysvar_account.clone(), + ]; + + // NonceRollbackFull create + NonceRollbackInfo impl + let full = NonceRollbackFull::from_partial(partial.clone(), &message, &accounts).unwrap(); + assert_eq!(*full.nonce_address(), nonce_address); + assert_eq!(*full.nonce_account(), nonce_account); + assert_eq!(full.fee_calculator(), Some(fee_calculator)); + assert_eq!(full.fee_account(), Some(&from_account)); + + let message = Message::new(&instructions, Some(&nonce_address)); + let accounts = [ + nonce_account, + from_account, + to_account, + recent_blockhashes_sysvar_account, + ]; + + // Nonce account is fee-payer + let full = NonceRollbackFull::from_partial(partial.clone(), &message, &accounts).unwrap(); + assert_eq!(full.fee_account(), None); + + // NonceRollbackFull create, fee-payer not in account_keys fails assert_eq!( - HashAgeKind::DurableNoncePartial(Pubkey::default(), account.clone()).fee_calculator(), - Some(Some(FeeCalculator::default())) - ); - assert_eq!( - HashAgeKind::DurableNoncePartial(Pubkey::default(), Account::default()) - .fee_calculator(), - Some(None) - ); - assert_eq!( - HashAgeKind::DurableNonceFull(Pubkey::default(), account, Some(Account::default())) - .fee_calculator(), - Some(Some(FeeCalculator::default())) - ); - assert_eq!( - HashAgeKind::DurableNonceFull( - Pubkey::default(), - Account::default(), - Some(Account::default()) - ) - .fee_calculator(), - Some(None) + NonceRollbackFull::from_partial(partial, &message, &[]).unwrap_err(), + TransactionError::AccountNotFound, ); } @@ -7018,13 +7110,13 @@ pub(crate) mod tests { system_transaction::transfer(&mint_keypair, &key.pubkey(), 5, genesis_config.hash()); let results = vec![ - (Ok(()), Some(HashAgeKind::Extant)), + (Ok(()), None), ( Err(TransactionError::InstructionError( 1, SystemError::ResultWithNegativeLamports.into(), )), - Some(HashAgeKind::Extant), + None, ), ]; let initial_balance = bank.get_balance(&leader); @@ -9012,19 +9104,19 @@ pub(crate) mod tests { assert_eq!(transaction_balances_set.pre_balances.len(), 3); assert_eq!(transaction_balances_set.post_balances.len(), 3); - assert!(transaction_results.processing_results[0].0.is_ok()); + assert!(transaction_results.execution_results[0].0.is_ok()); 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.processing_results[1].0.is_err()); + assert!(transaction_results.execution_results[1].0.is_err()); 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.processing_results[2].0.is_err()); + assert!(transaction_results.execution_results[2].0.is_err()); assert_eq!(transaction_balances_set.pre_balances[2], vec![9, 0, 1]); assert_eq!(transaction_balances_set.post_balances[2], vec![8, 0, 1]); } diff --git a/runtime/src/bank_utils.rs b/runtime/src/bank_utils.rs index e6009471ae..6530269043 100644 --- a/runtime/src/bank_utils.rs +++ b/runtime/src/bank_utils.rs @@ -32,13 +32,13 @@ pub fn find_and_send_votes( vote_sender: Option<&ReplayVoteSender>, ) { let TransactionResults { - processing_results, + execution_results, overwritten_vote_accounts, .. } = tx_results; if let Some(vote_sender) = vote_sender { for old_account in overwritten_vote_accounts { - assert!(processing_results[old_account.transaction_result_index] + assert!(execution_results[old_account.transaction_result_index] .0 .is_ok()); let transaction = &txs[old_account.transaction_index]; diff --git a/sdk/program/src/message.rs b/sdk/program/src/message.rs index ad4ba63f12..c58442500f 100644 --- a/sdk/program/src/message.rs +++ b/sdk/program/src/message.rs @@ -298,6 +298,10 @@ impl Message { false } + pub fn is_non_loader_key(&self, key: &Pubkey, key_index: usize) -> bool { + !self.program_ids().contains(&key) || self.is_key_passed_to_program(key_index) + } + pub fn program_position(&self, index: usize) -> Option { let program_ids = self.program_ids(); program_ids @@ -794,4 +798,61 @@ mod tests { ); } } + + #[test] + fn test_program_ids() { + let key0 = Pubkey::new_unique(); + let key1 = Pubkey::new_unique(); + let loader2 = Pubkey::new_unique(); + let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; + let message = Message::new_with_compiled_instructions( + 1, + 0, + 2, + vec![key0, key1, loader2], + Hash::default(), + instructions, + ); + assert_eq!(message.program_ids(), vec![&loader2]); + } + + #[test] + fn test_is_key_passed_to_program() { + let key0 = Pubkey::new_unique(); + let key1 = Pubkey::new_unique(); + let loader2 = Pubkey::new_unique(); + let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; + let message = Message::new_with_compiled_instructions( + 1, + 0, + 2, + vec![key0, key1, loader2], + Hash::default(), + instructions, + ); + + assert!(message.is_key_passed_to_program(0)); + assert!(message.is_key_passed_to_program(1)); + assert!(!message.is_key_passed_to_program(2)); + } + + #[test] + fn test_is_non_loader_key() { + let key0 = Pubkey::new_unique(); + let key1 = Pubkey::new_unique(); + let loader2 = Pubkey::new_unique(); + let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; + let message = Message::new_with_compiled_instructions( + 1, + 0, + 2, + vec![key0, key1, loader2], + Hash::default(), + instructions, + ); + + assert!(message.is_non_loader_key(&key0, 0)); + assert!(message.is_non_loader_key(&key1, 1)); + assert!(!message.is_non_loader_key(&loader2, 2)); + } } diff --git a/sdk/src/nonce_account.rs b/sdk/src/nonce_account.rs index 0c533b9a6a..d510e18190 100644 --- a/sdk/src/nonce_account.rs +++ b/sdk/src/nonce_account.rs @@ -20,6 +20,9 @@ pub fn create_account(lamports: u64) -> RefCell { } pub fn verify_nonce_account(acc: &Account, hash: &Hash) -> bool { + if acc.owner != crate::system_program::id() { + return false; + } match StateMut::::state(acc).map(|v| v.convert_to_current()) { Ok(State::Initialized(ref data)) => *hash == data.blockhash, _ => false, @@ -35,3 +38,23 @@ pub fn fee_calculator_of(account: &Account) -> Option { _ => None, } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::pubkey::Pubkey; + + #[test] + fn test_verify_bad_account_owner_fails() { + let program_id = Pubkey::new_unique(); + assert_ne!(program_id, crate::system_program::id()); + let account = Account::new_data_with_space( + 42, + &Versions::new_current(State::Uninitialized), + State::size(), + &program_id, + ) + .expect("nonce_account"); + assert!(!verify_nonce_account(&account, &Hash::default())); + } +}