From e070c5ca385284efc45bee10e63ce10550b078fd Mon Sep 17 00:00:00 2001 From: Jack May Date: Tue, 3 May 2022 09:50:06 -0700 Subject: [PATCH] default compute units per instruction (#24899) --- .../developing/programming-model/runtime.md | 8 +- program-runtime/src/compute_budget.rs | 126 ++++++++++++++---- runtime/src/bank.rs | 7 +- sdk/src/feature_set.rs | 5 + 4 files changed, 112 insertions(+), 34 deletions(-) diff --git a/docs/src/developing/programming-model/runtime.md b/docs/src/developing/programming-model/runtime.md index 4512cf8a8f57af..421fe6a3edd6a3 100644 --- a/docs/src/developing/programming-model/runtime.md +++ b/docs/src/developing/programming-model/runtime.md @@ -113,10 +113,10 @@ Budget](#compute-budget). With a transaction-wide compute budget the `max_units` cap is applied to the entire transaction rather than to each instruction within the transaction. The -default number of maximum units will be 1.4M which means the sum of the -compute units used by each instruction in the transaction must not exceed that -value. The default number of maximum units allows is intentionally kept large to -avoid breaking existing client behavior. +default number of maximum units will calculated per instruction which means the +sum of the compute units used by each instruction in the transaction must not +exceed that value. The default number of maximum units allowed matches existing +values to avoid breaking existing client behavior. ### Reduce transaction fees diff --git a/program-runtime/src/compute_budget.rs b/program-runtime/src/compute_budget.rs index dfacf2a57bb121..733632ad3e756d 100644 --- a/program-runtime/src/compute_budget.rs +++ b/program-runtime/src/compute_budget.rs @@ -102,35 +102,54 @@ impl ComputeBudget { &mut self, message: &SanitizedMessage, requestable_heap_size: bool, + default_units_per_instruction: bool, ) -> Result { + let mut num_instructions = message.instructions().len(); let mut requested_additional_fee = 0; + let mut requested_units = None; + let error = TransactionError::InstructionError(0, InstructionError::InvalidInstructionData); - // Compute budget instruction must be in the 1st 3 instructions (avoid - // nonce marker), otherwise ignored - for (program_id, instruction) in message.program_instructions_iter().take(3) { + for (i, (program_id, instruction)) in message.program_instructions_iter().enumerate() { if compute_budget::check_id(program_id) { - match try_from_slice_unchecked(&instruction.data) { - Ok(ComputeBudgetInstruction::RequestUnits { - units, - additional_fee, - }) => { - self.max_units = units.min(MAX_UNITS) as u64; - requested_additional_fee = additional_fee as u64; - } - Ok(ComputeBudgetInstruction::RequestHeapFrame(bytes)) => { - if !requestable_heap_size - || bytes > MAX_HEAP_FRAME_BYTES - || bytes < MIN_HEAP_FRAME_BYTES as u32 - || bytes % 1024 != 0 - { - return Err(error); + // don't include request instructions in default max calc + num_instructions = num_instructions.saturating_sub(1); + + // Compute budget instruction must be in the 1st 3 instructions (avoid + // nonce marker), otherwise ignored + if i < 3 { + match try_from_slice_unchecked(&instruction.data) { + Ok(ComputeBudgetInstruction::RequestUnits { + units, + additional_fee, + }) => { + requested_units = Some(units as u64); + requested_additional_fee = additional_fee as u64; + } + Ok(ComputeBudgetInstruction::RequestHeapFrame(bytes)) => { + if !requestable_heap_size + || bytes > MAX_HEAP_FRAME_BYTES + || bytes < MIN_HEAP_FRAME_BYTES as u32 + || bytes % 1024 != 0 + { + return Err(error); + } + self.heap_size = Some(bytes as usize); } - self.heap_size = Some(bytes as usize); + _ => return Err(error), } - _ => return Err(error), } } } + + self.max_units = if default_units_per_instruction { + requested_units + .or_else(|| Some(num_instructions.saturating_mul(DEFAULT_UNITS as usize) as u64)) + } else { + requested_units + } + .unwrap_or(MAX_UNITS as u64) + .min(MAX_UNITS as u64); + Ok(requested_additional_fee) } } @@ -159,7 +178,7 @@ mod tests { Hash::default(), )); let mut compute_budget = ComputeBudget::default(); - let result = compute_budget.process_message(&tx.message(), true); + let result = compute_budget.process_message(&tx.message(), true, true); assert_eq!($expected_error, result); assert_eq!(compute_budget, $expected_budget); }; @@ -168,7 +187,14 @@ mod tests { #[test] fn test_process_mesage() { // Units - test!(&[], Ok(0), ComputeBudget::default()); + test!( + &[], + Ok(0), + ComputeBudget { + max_units: 0, + ..ComputeBudget::default() + } + ); test!( &[ ComputeBudgetInstruction::request_units(1, 0), @@ -186,7 +212,10 @@ mod tests { Instruction::new_with_bincode(Pubkey::new_unique(), &0, vec![]), ], Ok(0), - ComputeBudget::default() + ComputeBudget { + max_units: MAX_UNITS as u64, + ..ComputeBudget::default() + } ); test!( &[ @@ -204,14 +233,34 @@ mod tests { Instruction::new_with_bincode(Pubkey::new_unique(), &0, vec![]), Instruction::new_with_bincode(Pubkey::new_unique(), &0, vec![]), Instruction::new_with_bincode(Pubkey::new_unique(), &0, vec![]), - ComputeBudgetInstruction::request_units(1, 0), + ComputeBudgetInstruction::request_units(1, 0), // ignored ], Ok(0), - ComputeBudget::default() + ComputeBudget { + max_units: DEFAULT_UNITS as u64 * 3, + ..ComputeBudget::default() + } + ); + + // Additional fee + test!( + &[ComputeBudgetInstruction::request_units(1, 42),], + Ok(42), + ComputeBudget { + max_units: 1, + ..ComputeBudget::default() + } ); // HeapFrame - test!(&[], Ok(0), ComputeBudget::default()); + test!( + &[], + Ok(0), + ComputeBudget { + max_units: 0, + ..ComputeBudget::default() + } + ); test!( &[ ComputeBudgetInstruction::request_heap_frame(40 * 1024), @@ -219,6 +268,7 @@ mod tests { ], Ok(0), ComputeBudget { + max_units: DEFAULT_UNITS as u64, heap_size: Some(40 * 1024), ..ComputeBudget::default() } @@ -263,6 +313,7 @@ mod tests { ], Ok(0), ComputeBudget { + max_units: DEFAULT_UNITS as u64, heap_size: Some(MAX_HEAP_FRAME_BYTES as usize), ..ComputeBudget::default() } @@ -275,7 +326,28 @@ mod tests { ComputeBudgetInstruction::request_heap_frame(1), // ignored ], Ok(0), - ComputeBudget::default() + ComputeBudget { + max_units: DEFAULT_UNITS as u64 * 3, + ..ComputeBudget::default() + } + ); + + test!( + &[ + Instruction::new_with_bincode(Pubkey::new_unique(), &0, vec![]), + Instruction::new_with_bincode(Pubkey::new_unique(), &0, vec![]), + Instruction::new_with_bincode(Pubkey::new_unique(), &0, vec![]), + Instruction::new_with_bincode(Pubkey::new_unique(), &0, vec![]), + Instruction::new_with_bincode(Pubkey::new_unique(), &0, vec![]), + Instruction::new_with_bincode(Pubkey::new_unique(), &0, vec![]), + Instruction::new_with_bincode(Pubkey::new_unique(), &0, vec![]), + Instruction::new_with_bincode(Pubkey::new_unique(), &0, vec![]), + ], + Ok(0), + ComputeBudget { + max_units: DEFAULT_UNITS as u64 * 7, + ..ComputeBudget::default() + } ); // Combined diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 4542e214e4482c..1ef3761d749e4b 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -107,8 +107,8 @@ use { epoch_schedule::EpochSchedule, feature, feature_set::{ - self, disable_fee_calculator, nonce_must_be_writable, requestable_heap_size, - tx_wide_compute_cap, FeatureSet, + self, default_units_per_instruction, disable_fee_calculator, nonce_must_be_writable, + requestable_heap_size, tx_wide_compute_cap, FeatureSet, }, fee::FeeStructure, fee_calculator::{FeeCalculator, FeeRateGovernor}, @@ -4371,6 +4371,7 @@ impl Bank { let process_transaction_result = compute_budget.process_message( tx.message(), feature_set.is_active(&requestable_heap_size::id()), + feature_set.is_active(&default_units_per_instruction::id()), ); compute_budget_process_transaction_time.stop(); saturating_add_assign!( @@ -4597,7 +4598,7 @@ impl Bank { let mut compute_budget = ComputeBudget::default(); let additional_fee = compute_budget - .process_message(message, false) + .process_message(message, false, false) .unwrap_or_default(); let signature_fee = Self::get_num_signatures_in_message(message) .saturating_mul(fee_structure.lamports_per_signature); diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 696b5315ee4cf1..3444b463753984 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -375,6 +375,10 @@ pub mod spl_associated_token_account_v1_1_0 { solana_sdk::declare_id!("FaTa17gVKoqbh38HcfiQonPsAaQViyDCCSg71AubYZw8"); } +pub mod default_units_per_instruction { + solana_sdk::declare_id!("J2QdYx8crLbTVK8nur1jeLsmc3krDbfjoxoea2V1Uy5Q"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -463,6 +467,7 @@ lazy_static! { (update_rewards_from_cached_accounts::id(), "update rewards from cached accounts"), (spl_token_v3_4_0::id(), "SPL Token Program version 3.4.0 release #24740"), (spl_associated_token_account_v1_1_0::id(), "SPL Associated Token Account Program version 1.1.0 release #24741"), + (default_units_per_instruction::id(), "Default max tx-wide compute units calculated per instruction"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter()