From 401c542d2a51bdd8efff67879186d8f6e0e22995 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 29 Dec 2021 08:04:12 +0000 Subject: [PATCH] Count compute units even when transaction errors (backport #22059) (#22154) * Count compute units even when transaction errors (#22059) (cherry picked from commit eaa8c67bde5225ca907f431e13fc6cd2d1ddfbb6) # Conflicts: # program-runtime/src/invoke_context.rs # runtime/src/bank.rs # runtime/src/message_processor.rs * Fix merge conflicts Co-authored-by: carllin --- core/src/banking_stage.rs | 46 +++--- core/src/cost_update_service.rs | 2 + core/src/qos_service.rs | 11 +- ledger/src/blockstore_processor.rs | 6 +- program-runtime/src/invoke_context.rs | 197 ++++++++++++++++++++++---- program-runtime/src/timings.rs | 19 ++- program-test/src/lib.rs | 1 + programs/bpf_loader/src/syscalls.rs | 1 + runtime/src/accounts.rs | 38 +++-- runtime/src/bank.rs | 39 +++-- runtime/src/cost_model.rs | 3 +- runtime/src/message_processor.rs | 27 +++- runtime/src/transaction_batch.rs | 8 +- 13 files changed, 305 insertions(+), 93 deletions(-) diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs index 2d51a1244a..353a27cb81 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -26,7 +26,7 @@ use { TransactionExecutionResult, }, bank_utils, - cost_model::CostModel, + cost_model::{CostModel, ExecutionCost}, transaction_batch::TransactionBatch, vote_sender_types::ReplayVoteSender, }, @@ -973,14 +973,20 @@ impl BankingStage { let tx_costs = qos_service.compute_transaction_costs(txs.iter()); let transactions_qos_results = - qos_service.select_transactions_per_cost(txs.iter(), tx_costs.iter(), bank); + qos_service.select_transactions_per_cost(txs.iter(), tx_costs.into_iter(), bank); // Only lock accounts for those transactions are selected for the block; // Once accounts are locked, other threads cannot encode transactions that will modify the // same account state let mut lock_time = Measure::start("lock_time"); - let batch = - bank.prepare_sanitized_batch_with_results(txs, transactions_qos_results.into_iter()); + let batch = bank.prepare_sanitized_batch_with_results( + txs, + transactions_qos_results + .into_iter() + .map(|transaction_cost_result| { + transaction_cost_result.map(|transaction_cost| transaction_cost.execution_cost) + }), + ); lock_time.stop(); // retryable_txs includes AccountInUse, WouldExceedMaxBlockCostLimit and @@ -1077,9 +1083,9 @@ impl BankingStage { fn prepare_filter_for_pending_transactions( transactions_len: usize, pending_tx_indexes: &[usize], - ) -> Vec> { + ) -> Vec> { let mut mask = vec![Err(TransactionError::BlockhashNotFound); transactions_len]; - pending_tx_indexes.iter().for_each(|x| mask[*x] = Ok(())); + pending_tx_indexes.iter().for_each(|x| mask[*x] = Ok(0)); mask } @@ -1170,7 +1176,7 @@ impl BankingStage { let results = bank.check_transactions( transactions, - &filter, + filter.into_iter(), (MAX_PROCESSING_AGE) .saturating_sub(max_tx_fwd_delay) .saturating_sub(FORWARD_TRANSACTIONS_TO_LEADER_AT_SLOT_OFFSET as usize), @@ -2082,20 +2088,20 @@ mod tests { vec![ Err(TransactionError::BlockhashNotFound), Err(TransactionError::BlockhashNotFound), - Ok(()), + Ok(0), Err(TransactionError::BlockhashNotFound), - Ok(()), - Ok(()) + Ok(0), + Ok(0) ] ); assert_eq!( BankingStage::prepare_filter_for_pending_transactions(6, &[0, 2, 3]), vec![ - Ok(()), + Ok(0), Err(TransactionError::BlockhashNotFound), - Ok(()), - Ok(()), + Ok(0), + Ok(0), Err(TransactionError::BlockhashNotFound), Err(TransactionError::BlockhashNotFound), ] @@ -2109,10 +2115,10 @@ mod tests { &[ (Err(TransactionError::BlockhashNotFound), None), (Err(TransactionError::BlockhashNotFound), None), - (Ok(()), None), + (Ok(0), None), (Err(TransactionError::BlockhashNotFound), None), - (Ok(()), None), - (Ok(()), None), + (Ok(0), None), + (Ok(0), None), ], &[2, 4, 5, 9, 11, 13] ), @@ -2122,12 +2128,12 @@ mod tests { assert_eq!( BankingStage::filter_valid_transaction_indexes( &[ - (Ok(()), None), + (Ok(0), None), (Err(TransactionError::BlockhashNotFound), None), (Err(TransactionError::BlockhashNotFound), None), - (Ok(()), None), - (Ok(()), None), - (Ok(()), None), + (Ok(0), None), + (Ok(0), None), + (Ok(0), None), ], &[1, 6, 7, 9, 31, 43] ), diff --git a/core/src/cost_update_service.rs b/core/src/cost_update_service.rs index 0ebee0c09c..52a9df4b7b 100644 --- a/core/src/cost_update_service.rs +++ b/core/src/cost_update_service.rs @@ -246,6 +246,7 @@ mod tests { ProgramTiming { accumulated_us, accumulated_units, + current_cost_model_estimated_units: 0, count, }, ); @@ -281,6 +282,7 @@ mod tests { ProgramTiming { accumulated_us, accumulated_units, + current_cost_model_estimated_units: 0, count, }, ); diff --git a/core/src/qos_service.rs b/core/src/qos_service.rs index feea54cac1..302694bb3b 100644 --- a/core/src/qos_service.rs +++ b/core/src/qos_service.rs @@ -108,18 +108,18 @@ impl QosService { pub fn select_transactions_per_cost<'a>( &self, transactions: impl Iterator, - transactions_costs: impl Iterator, + transactions_costs: impl Iterator, bank: &Arc, - ) -> Vec> { + ) -> Vec> { let mut cost_tracking_time = Measure::start("cost_tracking_time"); let mut cost_tracker = bank.write_cost_tracker().unwrap(); let select_results = transactions .zip(transactions_costs) - .map(|(tx, cost)| match cost_tracker.try_add(tx, cost) { + .map(|(tx, cost)| match cost_tracker.try_add(tx, &cost) { Ok(current_block_cost) => { debug!("slot {:?}, transaction {:?}, cost {:?}, fit into current block, current block cost {}", bank.slot(), tx, cost, current_block_cost); self.metrics.selected_txs_count.fetch_add(1, Ordering::Relaxed); - Ok(()) + Ok(cost) }, Err(e) => { debug!("slot {:?}, transaction {:?}, cost {:?}, not fit into current block, '{:?}'", bank.slot(), tx, cost, e); @@ -304,7 +304,8 @@ mod tests { bank.write_cost_tracker() .unwrap() .set_limits(cost_limit, cost_limit); - let results = qos_service.select_transactions_per_cost(txs.iter(), txs_costs.iter(), &bank); + let results = + qos_service.select_transactions_per_cost(txs.iter(), txs_costs.into_iter(), &bank); // verify that first transfer tx and all votes are allowed assert_eq!(results.len(), txs.len()); diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 477bc456af..00c396cdcd 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -102,10 +102,10 @@ thread_local!(static PAR_THREAD_POOL: RefCell = RefCell::new(rayon:: .unwrap()) ); -fn first_err(results: &[Result<()>]) -> Result<()> { +fn first_err(results: &[Result]) -> Result<()> { for r in results { - if r.is_err() { - return r.clone(); + if let Err(e) = r { + return Err(e.clone()); } } Ok(()) diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 7ae8b4ec8e..83641068e3 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -30,6 +30,12 @@ pub type TransactionAccountRefCells = Vec; pub type ProcessInstructionWithContext = fn(usize, &[u8], &mut InvokeContext) -> Result<(), InstructionError>; +#[derive(Debug, PartialEq)] +pub struct ProcessInstructionResult { + pub compute_units_consumed: u64, + pub result: Result<(), InstructionError>, +} + #[derive(Clone)] pub struct BuiltinProgram { pub program_id: Pubkey, @@ -499,7 +505,8 @@ impl<'a> InvokeContext<'a> { &program_indices, &account_indices, &caller_write_privileges, - )?; + ) + .result?; // Verify the called program has not misbehaved let do_support_realloc = self.feature_set.is_active(&do_support_realloc::id()); @@ -640,37 +647,48 @@ impl<'a> InvokeContext<'a> { program_indices: &[usize], account_indices: &[usize], caller_write_privileges: &[bool], - ) -> Result { + ) -> ProcessInstructionResult { let is_lowest_invocation_level = self.invoke_stack.is_empty(); if !is_lowest_invocation_level { // Verify the calling program hasn't misbehaved - self.verify_and_update(instruction, account_indices, caller_write_privileges)?; + let result = + self.verify_and_update(instruction, account_indices, caller_write_privileges); + if result.is_err() { + return ProcessInstructionResult { + compute_units_consumed: 0, + result, + }; + } } + let mut compute_units_consumed = 0; let result = self .push(message, instruction, program_indices, account_indices) .and_then(|_| { self.return_data = (*instruction.program_id(&message.account_keys), Vec::new()); let pre_remaining_units = self.compute_meter.borrow().get_remaining(); - self.process_executable_chain(&instruction.data)?; + let execution_result = self.process_executable_chain(&instruction.data); let post_remaining_units = self.compute_meter.borrow().get_remaining(); + compute_units_consumed = pre_remaining_units.saturating_sub(post_remaining_units); + execution_result?; // Verify the called program has not misbehaved if is_lowest_invocation_level { - self.verify(message, instruction, program_indices)?; + self.verify(message, instruction, program_indices) } else { let write_privileges: Vec = (0..message.account_keys.len()) .map(|i| message.is_writable(i)) .collect(); - self.verify_and_update(instruction, account_indices, &write_privileges)?; + self.verify_and_update(instruction, account_indices, &write_privileges) } - - Ok(pre_remaining_units.saturating_sub(post_remaining_units)) }); // Pop the invoke_stack to restore previous state self.pop(); - result + ProcessInstructionResult { + compute_units_consumed, + result, + } } /// Calls the instruction's program entrypoint method @@ -983,6 +1001,10 @@ mod tests { ModifyOwned, ModifyNotOwned, ModifyReadonly, + ConsumeComputeUnits { + compute_units_consumed: u64, + desired_result: Result<(), InstructionError>, + }, } #[test] @@ -1052,6 +1074,17 @@ mod tests { .try_account_ref_mut()? .data_as_mut_slice()[0] = 1 } + MockInstruction::ConsumeComputeUnits { + compute_units_consumed, + desired_result, + } => { + invoke_context + .get_compute_meter() + .borrow_mut() + .consume(compute_units_consumed) + .unwrap(); + return desired_result; + } } } else { return Err(InstructionError::InvalidInstructionData); @@ -1260,13 +1293,15 @@ mod tests { .collect::>(); accounts[0].1.borrow_mut().data_as_mut_slice()[0] = 1; assert_eq!( - invoke_context.process_instruction( - &message, - &message.instructions[0], - &program_indices[1..], - &account_indices, - &caller_write_privileges, - ), + invoke_context + .process_instruction( + &message, + &message.instructions[0], + &program_indices[1..], + &account_indices, + &caller_write_privileges, + ) + .result, Err(InstructionError::ExternalAccountDataModified) ); accounts[0].1.borrow_mut().data_as_mut_slice()[0] = 0; @@ -1274,13 +1309,15 @@ mod tests { // readonly account modified by the invoker accounts[2].1.borrow_mut().data_as_mut_slice()[0] = 1; assert_eq!( - invoke_context.process_instruction( - &message, - &message.instructions[0], - &program_indices[1..], - &account_indices, - &caller_write_privileges, - ), + invoke_context + .process_instruction( + &message, + &message.instructions[0], + &program_indices[1..], + &account_indices, + &caller_write_privileges, + ) + .result, Err(InstructionError::ReadonlyDataModified) ); accounts[2].1.borrow_mut().data_as_mut_slice()[0] = 0; @@ -1288,15 +1325,33 @@ mod tests { invoke_context.pop(); let cases = vec![ - (MockInstruction::NoopSuccess, Ok(0)), + ( + MockInstruction::NoopSuccess, + ProcessInstructionResult { + result: Ok(()), + compute_units_consumed: 0, + }, + ), ( MockInstruction::NoopFail, - Err(InstructionError::GenericError), + ProcessInstructionResult { + result: Err(InstructionError::GenericError), + compute_units_consumed: 0, + }, + ), + ( + MockInstruction::ModifyOwned, + ProcessInstructionResult { + result: Ok(()), + compute_units_consumed: 0, + }, ), - (MockInstruction::ModifyOwned, Ok(0)), ( MockInstruction::ModifyNotOwned, - Err(InstructionError::ExternalAccountDataModified), + ProcessInstructionResult { + result: Err(InstructionError::ExternalAccountDataModified), + compute_units_consumed: 0, + }, ), ]; for case in cases { @@ -1497,4 +1552,92 @@ mod tests { ); invoke_context.pop(); } + + #[test] + fn test_process_instruction_compute_budget() { + let caller_program_id = solana_sdk::pubkey::new_rand(); + let callee_program_id = solana_sdk::pubkey::new_rand(); + let owned_account = AccountSharedData::new(42, 1, &callee_program_id); + let not_owned_account = AccountSharedData::new(84, 1, &solana_sdk::pubkey::new_rand()); + let readonly_account = AccountSharedData::new(168, 1, &solana_sdk::pubkey::new_rand()); + let loader_account = AccountSharedData::new(0, 0, &native_loader::id()); + let mut program_account = AccountSharedData::new(1, 0, &native_loader::id()); + program_account.set_executable(true); + + let accounts = vec![ + ( + solana_sdk::pubkey::new_rand(), + Rc::new(RefCell::new(owned_account)), + ), + ( + solana_sdk::pubkey::new_rand(), + Rc::new(RefCell::new(not_owned_account)), + ), + ( + solana_sdk::pubkey::new_rand(), + Rc::new(RefCell::new(readonly_account)), + ), + (caller_program_id, Rc::new(RefCell::new(loader_account))), + (callee_program_id, Rc::new(RefCell::new(program_account))), + ]; + let account_indices = [0, 1, 2]; + let program_indices = [3, 4]; + + let metas = vec![ + AccountMeta::new(accounts[0].0, false), + AccountMeta::new(accounts[1].0, false), + AccountMeta::new_readonly(accounts[2].0, false), + ]; + + let builtin_programs = &[BuiltinProgram { + program_id: callee_program_id, + process_instruction: mock_process_instruction, + }]; + let mut invoke_context = InvokeContext::new_mock(&accounts, builtin_programs); + + let compute_units_consumed = 10; + let desired_results = vec![Ok(()), Err(InstructionError::GenericError)]; + + for desired_result in desired_results { + let caller_instruction = + CompiledInstruction::new(program_indices[0] as u8, &(), vec![0, 1, 2, 3, 4]); + let callee_instruction = Instruction::new_with_bincode( + callee_program_id, + &MockInstruction::ConsumeComputeUnits { + compute_units_consumed, + desired_result: desired_result.clone(), + }, + metas.clone(), + ); + let message = Message::new(&[callee_instruction.clone()], None); + invoke_context + .push(&message, &caller_instruction, &program_indices[..1], &[]) + .unwrap(); + let caller_write_privileges = message + .account_keys + .iter() + .enumerate() + .map(|(i, _)| message.is_writable(i)) + .collect::>(); + let result = invoke_context.process_instruction( + &message, + &message.instructions[0], + &program_indices[1..], + &account_indices, + &caller_write_privileges, + ); + + // Because the instruction had compute cost > 0, then regardless of the execution result, + // the number of compute units consumed should be a non-default which is something greater + // than zero. + assert!(result.compute_units_consumed > 0); + assert_eq!( + result, + ProcessInstructionResult { + compute_units_consumed, + result: desired_result, + } + ); + } + } } diff --git a/program-runtime/src/timings.rs b/program-runtime/src/timings.rs index a61b621e1a..560076ad8b 100644 --- a/program-runtime/src/timings.rs +++ b/program-runtime/src/timings.rs @@ -4,6 +4,7 @@ use {solana_sdk::pubkey::Pubkey, std::collections::HashMap}; pub struct ProgramTiming { pub accumulated_us: u64, pub accumulated_units: u64, + pub current_cost_model_estimated_units: u64, pub count: u32, } @@ -46,10 +47,24 @@ impl ExecuteDetailsTimings { program_timing.count = program_timing.count.saturating_add(other.count); } } - pub fn accumulate_program(&mut self, program_id: &Pubkey, us: u64, units: u64) { + pub fn accumulate_program( + &mut self, + program_id: &Pubkey, + us: u64, + actual_compute_units_consumed: u64, + estimated_execution_cost: u64, + is_error: bool, + ) { let program_timing = self.per_program_timings.entry(*program_id).or_default(); program_timing.accumulated_us = program_timing.accumulated_us.saturating_add(us); - program_timing.accumulated_units = program_timing.accumulated_units.saturating_add(units); + let compute_units_update = if is_error { + std::cmp::max(actual_compute_units_consumed, estimated_execution_cost) + } else { + actual_compute_units_consumed + }; + program_timing.accumulated_units = program_timing + .accumulated_units + .saturating_add(compute_units_update); program_timing.count = program_timing.count.saturating_add(1); } } diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index 0dc4d22e8d..c4749a0e07 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -322,6 +322,7 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs { &account_indices, &caller_privileges, ) + .result .map_err(|err| ProgramError::try_from(err).unwrap_or_else(|err| panic!("{}", err)))?; // Copy writeable account modifications back into the caller's AccountInfos diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index d8a143d834..ea4db31321 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -2372,6 +2372,7 @@ fn call<'a, 'b: 'a>( &account_indices, &caller_write_privileges, ) + .result .map_err(SyscallError::InstructionError)?; // Copy results back to caller diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 076988d9d9..8f10962848 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -13,6 +13,7 @@ use { TransactionExecutionResult, }, blockhash_queue::BlockhashQueue, + cost_model::ExecutionCost, rent_collector::RentCollector, system_instruction_processor::{get_system_account_kind, SystemAccountKind}, }, @@ -114,6 +115,7 @@ pub struct LoadedTransaction { pub program_indices: TransactionProgramIndices, pub rent: TransactionRent, pub rent_debits: RentDebits, + pub estimated_execution_cost: ExecutionCost, } pub type TransactionLoadResult = (Result, Option); @@ -231,6 +233,7 @@ impl Accounts { error_counters: &mut ErrorCounters, rent_collector: &RentCollector, feature_set: &FeatureSet, + estimated_execution_cost: ExecutionCost, ) -> Result { // Copy all the accounts let message = tx.message(); @@ -374,6 +377,7 @@ impl Accounts { program_indices, rent: tx_rent, rent_debits, + estimated_execution_cost, }) } else { error_counters.account_not_found += 1; @@ -467,7 +471,7 @@ impl Accounts { txs.iter() .zip(lock_results) .map(|etx| match etx { - (tx, (Ok(()), nonce)) => { + (tx, (Ok(execution_cost), nonce)) => { let lamports_per_signature = nonce .as_ref() .map(|nonce| nonce.lamports_per_signature()) @@ -487,6 +491,7 @@ impl Accounts { error_counters, rent_collector, feature_set, + execution_cost, ) { Ok(loaded_transaction) => loaded_transaction, Err(e) => return (Err(e), None), @@ -952,11 +957,14 @@ impl Accounts { pub fn lock_accounts<'a>( &self, txs: impl Iterator, - ) -> Vec> { + ) -> Vec> { let keys: Vec<_> = txs.map(|tx| tx.get_account_locks()).collect(); let account_locks = &mut self.account_locks.lock().unwrap(); keys.into_iter() - .map(|keys| self.lock_account(account_locks, keys.writable, keys.readonly)) + .map(|keys| { + self.lock_account(account_locks, keys.writable, keys.readonly) + .map(|_| 0) + }) .collect() } @@ -965,12 +973,12 @@ impl Accounts { pub fn lock_accounts_with_results<'a>( &self, txs: impl Iterator, - results: impl Iterator>, - ) -> Vec> { + results: impl Iterator>, + ) -> Vec> { let key_results: Vec<_> = txs .zip(results) .map(|(tx, result)| match result { - Ok(()) => Ok(tx.get_account_locks()), + Ok(execution_cost) => Ok((tx.get_account_locks(), execution_cost)), Err(e) => Err(e), }) .collect(); @@ -978,7 +986,9 @@ impl Accounts { key_results .into_iter() .map(|key_result| match key_result { - Ok(keys) => self.lock_account(account_locks, keys.writable, keys.readonly), + Ok((keys, execution_cost)) => self + .lock_account(account_locks, keys.writable, keys.readonly) + .map(|_| execution_cost), Err(e) => Err(e), }) .collect() @@ -989,7 +999,7 @@ impl Accounts { pub fn unlock_accounts<'a>( &self, txs: impl Iterator, - results: &[Result<()>], + results: &[Result], ) { let keys: Vec<_> = txs .zip(results) @@ -1273,7 +1283,7 @@ mod tests { accounts.load_accounts( &ancestors, &[sanitized_tx], - vec![(Ok(()), None)], + vec![(Ok(0), None)], &hash_queue, error_counters, rent_collector, @@ -2417,9 +2427,9 @@ mod tests { let txs = vec![tx0, tx1, tx2]; let qos_results = vec![ - Ok(()), + Ok(0), Err(TransactionError::WouldExceedMaxBlockCostLimit), - Ok(()), + Ok(0), ]; let results = accounts.lock_accounts_with_results(txs.iter(), qos_results.into_iter()); @@ -2512,6 +2522,7 @@ mod tests { program_indices: vec![], rent: 0, rent_debits: RentDebits::default(), + estimated_execution_cost: 0, }), None, ); @@ -2522,6 +2533,7 @@ mod tests { program_indices: vec![], rent: 0, rent_debits: RentDebits::default(), + estimated_execution_cost: 0, }), None, ); @@ -2615,7 +2627,7 @@ mod tests { accounts.load_accounts( &ancestors, &[tx], - vec![(Ok(()), None)], + vec![(Ok(0), None)], &hash_queue, &mut error_counters, &rent_collector, @@ -2951,6 +2963,7 @@ mod tests { program_indices: vec![], rent: 0, rent_debits: RentDebits::default(), + estimated_execution_cost: 0, }), nonce.clone(), ); @@ -3061,6 +3074,7 @@ mod tests { program_indices: vec![], rent: 0, rent_debits: RentDebits::default(), + estimated_execution_cost: 0, }), nonce.clone(), ); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 0453fa162d..3a1f59a81c 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -47,6 +47,7 @@ use { ancestors::{Ancestors, AncestorsForSerialization}, blockhash_queue::BlockhashQueue, builtins::{self, ActivationType, Builtin, Builtins}, + cost_model::ExecutionCost, cost_tracker::CostTracker, epoch_stakes::{EpochStakes, NodeVoteAccounts}, inline_spl_token, @@ -502,7 +503,7 @@ impl StatusCacheRc { } } -pub type TransactionCheckResult = (Result<()>, Option); +pub type TransactionCheckResult<'a> = (Result, Option); pub type TransactionExecutionResult = (Result<()>, Option); pub struct TransactionResults { pub fee_collection_results: Vec>, @@ -3092,7 +3093,7 @@ impl Bank { pub fn prepare_sanitized_batch_with_results<'a, 'b>( &'a self, transactions: &'b [SanitizedTransaction], - transaction_results: impl Iterator>, + transaction_results: impl Iterator>, ) -> TransactionBatch<'a, 'b> { // this lock_results could be: Ok, AccountInUse, WouldExceedBlockMaxLimit or WouldExceedAccountMaxLimit let lock_results = self @@ -3107,7 +3108,7 @@ impl Bank { &'a self, transaction: SanitizedTransaction, ) -> TransactionBatch<'a, '_> { - let mut batch = TransactionBatch::new(vec![Ok(())], self, Cow::Owned(vec![transaction])); + let mut batch = TransactionBatch::new(vec![Ok(0)], self, Cow::Owned(vec![transaction])); batch.needs_unlock = false; batch } @@ -3203,23 +3204,29 @@ impl Bank { self.rc.accounts.accounts_db.set_shrink_paths(paths); } - fn check_age<'a>( + fn check_age<'a, T>( &self, txs: impl Iterator, - lock_results: &[Result<()>], + lock_results: impl Iterator, max_age: usize, error_counters: &mut ErrorCounters, - ) -> Vec { + ) -> Vec + where + T: std::borrow::Borrow>, + { let hash_queue = self.blockhash_queue.read().unwrap(); txs.zip(lock_results) - .map(|(tx, lock_res)| match lock_res { - Ok(()) => { + .map(|(tx, lock_res)| match lock_res.borrow() { + Ok(execution_cost) => { let recent_blockhash = tx.message().recent_blockhash(); let hash_age = hash_queue.check_hash_age(recent_blockhash, max_age); if hash_age == Some(true) { - (Ok(()), None) + (Ok(*execution_cost), None) } else if let Some((address, account)) = self.check_transaction_for_nonce(tx) { - (Ok(()), Some(NoncePartial::new(address, account))) + ( + Ok(*execution_cost), + Some(NoncePartial::new(address, account)), + ) } else if hash_age == Some(false) { error_counters.blockhash_too_old += 1; (Err(TransactionError::BlockhashNotFound), None) @@ -3289,13 +3296,16 @@ impl Bank { }) } - pub fn check_transactions( + pub fn check_transactions( &self, sanitized_txs: &[SanitizedTransaction], - lock_results: &[Result<()>], + lock_results: impl Iterator, max_age: usize, error_counters: &mut ErrorCounters, - ) -> Vec { + ) -> Vec + where + T: std::borrow::Borrow>, + { let age_results = self.check_age(sanitized_txs.iter(), lock_results, max_age, error_counters); self.check_status_cache(sanitized_txs, age_results, error_counters) @@ -3518,7 +3528,7 @@ impl Bank { let mut check_time = Measure::start("check_transactions"); let check_results = self.check_transactions( sanitized_txs, - batch.lock_results(), + batch.lock_results().iter(), max_age, &mut error_counters, ); @@ -3597,6 +3607,7 @@ impl Bank { &self.builtin_programs.vec, legacy_message, &loaded_transaction.program_indices, + loaded_transaction.estimated_execution_cost, &account_refcells, self.rent_collector.rent, log_collector.clone(), diff --git a/runtime/src/cost_model.rs b/runtime/src/cost_model.rs index a7fc6d5e86..96d954c79b 100644 --- a/runtime/src/cost_model.rs +++ b/runtime/src/cost_model.rs @@ -15,6 +15,7 @@ use { }; const MAX_WRITABLE_ACCOUNTS: usize = 256; +pub type ExecutionCost = u64; // costs are stored in number of 'compute unit's #[derive(Debug)] @@ -23,7 +24,7 @@ pub struct TransactionCost { pub signature_cost: u64, pub write_lock_cost: u64, pub data_bytes_cost: u64, - pub execution_cost: u64, + pub execution_cost: ExecutionCost, // `cost_weight` is a multiplier could be applied to transaction cost, // if set to zero allows the transaction to bypass cost limit check. pub cost_weight: u32, diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index eccadcca8e..537659135f 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -1,9 +1,13 @@ use { + crate::cost_model::ExecutionCost, serde::{Deserialize, Serialize}, solana_measure::measure::Measure, solana_program_runtime::{ instruction_recorder::InstructionRecorder, - invoke_context::{BuiltinProgram, Executors, InvokeContext, TransactionAccountRefCell}, + invoke_context::{ + BuiltinProgram, Executors, InvokeContext, ProcessInstructionResult, + TransactionAccountRefCell, + }, log_collector::LogCollector, timings::ExecuteDetailsTimings, }, @@ -45,6 +49,7 @@ impl MessageProcessor { builtin_programs: &[BuiltinProgram], message: &Message, program_indices: &[Vec], + estimated_execution_cost: ExecutionCost, accounts: &[TransactionAccountRefCell], rent: Rent, log_collector: Option>>, @@ -105,16 +110,21 @@ impl MessageProcessor { Some(&instruction_recorders[instruction_index]); } let mut time = Measure::start("execute_instruction"); - let compute_meter_consumption = invoke_context - .process_instruction(message, instruction, program_indices, &[], &[]) - .map_err(|err| TransactionError::InstructionError(instruction_index as u8, err))?; + let ProcessInstructionResult { + compute_units_consumed, + result, + } = invoke_context.process_instruction(message, instruction, program_indices, &[], &[]); time.stop(); timings.accumulate_program( instruction.program_id(&message.account_keys), time.as_us(), - compute_meter_consumption, + compute_units_consumed, + estimated_execution_cost, + result.is_err(), ); timings.accumulate(&invoke_context.timings); + result + .map_err(|err| TransactionError::InstructionError(instruction_index as u8, err))?; } Ok(()) } @@ -230,6 +240,7 @@ mod tests { builtin_programs, &message, &program_indices, + 0, &accounts, rent_collector.rent, None, @@ -259,6 +270,7 @@ mod tests { builtin_programs, &message, &program_indices, + 0, &accounts, rent_collector.rent, None, @@ -292,6 +304,7 @@ mod tests { builtin_programs, &message, &program_indices, + 0, &accounts, rent_collector.rent, None, @@ -436,6 +449,7 @@ mod tests { builtin_programs, &message, &program_indices, + 0, &accounts, rent_collector.rent, None, @@ -469,6 +483,7 @@ mod tests { builtin_programs, &message, &program_indices, + 0, &accounts, rent_collector.rent, None, @@ -499,6 +514,7 @@ mod tests { builtin_programs, &message, &program_indices, + 0, &accounts, rent_collector.rent, None, @@ -556,6 +572,7 @@ mod tests { builtin_programs, &message, &[vec![0], vec![1]], + 0, &accounts, RentCollector::default().rent, None, diff --git a/runtime/src/transaction_batch.rs b/runtime/src/transaction_batch.rs index 16bc747d88..cc99fecf4c 100644 --- a/runtime/src/transaction_batch.rs +++ b/runtime/src/transaction_batch.rs @@ -1,12 +1,12 @@ use { - crate::bank::Bank, + crate::{bank::Bank, cost_model::ExecutionCost}, solana_sdk::transaction::{Result, SanitizedTransaction}, std::borrow::Cow, }; // Represents the results of trying to lock a set of accounts pub struct TransactionBatch<'a, 'b> { - lock_results: Vec>, + lock_results: Vec>, bank: &'a Bank, sanitized_txs: Cow<'b, [SanitizedTransaction]>, pub(crate) needs_unlock: bool, @@ -14,7 +14,7 @@ pub struct TransactionBatch<'a, 'b> { impl<'a, 'b> TransactionBatch<'a, 'b> { pub fn new( - lock_results: Vec>, + lock_results: Vec>, bank: &'a Bank, sanitized_txs: Cow<'b, [SanitizedTransaction]>, ) -> Self { @@ -27,7 +27,7 @@ impl<'a, 'b> TransactionBatch<'a, 'b> { } } - pub fn lock_results(&self) -> &Vec> { + pub fn lock_results(&self) -> &Vec> { &self.lock_results }