From 3e9af14f3a145070773c719ad104b6a02aefd718 Mon Sep 17 00:00:00 2001 From: Tao Zhu <82401714+tao-stones@users.noreply.github.com> Date: Wed, 4 Dec 2024 22:49:59 -0500 Subject: [PATCH] Fix reserve minimal compute units for builtins (#3799) - Add feature gate, issue #2562; - Implement SIMD-170; --------- Co-authored-by: Justin Starry --- builtins-default-costs/src/lib.rs | 5 + compute-budget/src/compute_budget_limits.rs | 3 + core/src/banking_stage/consumer.rs | 3 +- .../immutable_deserialized_packet.rs | 8 + .../receive_and_buffer.rs | 2 +- cost-model/src/cost_model.rs | 292 ++++++++++++------ cost-model/src/transaction_cost.rs | 7 +- .../benches/compute_budget.rs | 61 ++-- programs/sbf/tests/programs.rs | 15 +- .../process_compute_budget_instructions.rs | 234 +++++++------- .../src/builtin_programs_filter.rs | 105 +++++++ .../src/compute_budget_instruction_details.rs | 285 +++++++++++------ .../src/compute_budget_program_id_filter.rs | 1 - .../src/instructions_processor.rs | 124 ++++++-- runtime-transaction/src/lib.rs | 1 + .../src/runtime_transaction.rs | 2 +- .../runtime_transaction/sdk_transactions.rs | 23 +- runtime/src/bank.rs | 7 +- runtime/src/bank/tests.rs | 7 +- runtime/src/prioritization_fee_cache.rs | 2 +- sdk/feature-set/src/lib.rs | 5 + sdk/message/src/sanitized.rs | 2 +- sdk/message/src/versions/sanitized.rs | 2 +- svm-transaction/src/svm_message.rs | 2 +- .../src/svm_message/sanitized_message.rs | 2 +- .../src/svm_message/sanitized_transaction.rs | 2 +- svm/src/transaction_processor.rs | 25 +- .../src/resolved_transaction_view.rs | 2 +- transaction-view/src/transaction_view.rs | 4 +- 29 files changed, 860 insertions(+), 373 deletions(-) create mode 100644 runtime-transaction/src/builtin_programs_filter.rs diff --git a/builtins-default-costs/src/lib.rs b/builtins-default-costs/src/lib.rs index 48e2d76936e83d..2e167c667cc84d 100644 --- a/builtins-default-costs/src/lib.rs +++ b/builtins-default-costs/src/lib.rs @@ -162,6 +162,11 @@ pub fn get_builtin_instruction_cost<'a>( .map(|builtin_cost| builtin_cost.native_cost) } +#[inline] +pub fn is_builtin_program(program_id: &Pubkey) -> bool { + BUILTIN_INSTRUCTION_COSTS.contains_key(program_id) +} + #[cfg(test)] mod test { use super::*; diff --git a/compute-budget/src/compute_budget_limits.rs b/compute-budget/src/compute_budget_limits.rs index 90652b7772b6ed..ac951a2014a36a 100644 --- a/compute-budget/src/compute_budget_limits.rs +++ b/compute-budget/src/compute_budget_limits.rs @@ -7,6 +7,9 @@ use { /// default heap page cost = 0.5 * 15 ~= 8CU/page pub const DEFAULT_HEAP_COST: u64 = 8; pub const DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT: u32 = 200_000; +// SIMD-170 defines max CUs to be allocated for any builtin program instructions, that +// have not been migrated to sBPF programs. +pub const MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT: u32 = 3_000; pub const MAX_COMPUTE_UNIT_LIMIT: u32 = 1_400_000; pub const MAX_HEAP_FRAME_BYTES: u32 = 256 * 1024; pub const MIN_HEAP_FRAME_BYTES: u32 = HEAP_LENGTH as u32; diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index b28a46cd51218a..01ee8c879d22d8 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -578,7 +578,7 @@ impl Consumer { .filter_map(|transaction| { transaction .compute_budget_instruction_details() - .sanitize_and_convert_to_compute_budget_limits() + .sanitize_and_convert_to_compute_budget_limits(&bank.feature_set) .ok() .map(|limits| limits.compute_unit_price) }) @@ -760,6 +760,7 @@ impl Consumer { let fee_payer = message.fee_payer(); let fee_budget_limits = FeeBudgetLimits::from(process_compute_budget_instructions( message.program_instructions_iter(), + &bank.feature_set, )?); let fee = solana_fee::calculate_fee( message, diff --git a/core/src/banking_stage/immutable_deserialized_packet.rs b/core/src/banking_stage/immutable_deserialized_packet.rs index afd5d34dc4fd16..eec31181f6ed63 100644 --- a/core/src/banking_stage/immutable_deserialized_packet.rs +++ b/core/src/banking_stage/immutable_deserialized_packet.rs @@ -10,6 +10,7 @@ use { solana_sanitize::SanitizeError, solana_sdk::{ clock::Slot, + feature_set::FeatureSet, hash::Hash, message::{v0::LoadedAddresses, AddressLoaderError, Message, SimpleAddressLoader}, pubkey::Pubkey, @@ -45,6 +46,12 @@ pub enum DeserializedPacketError { FailedFilter(#[from] PacketFilterFailure), } +lazy_static::lazy_static! { + // Make a dummy feature_set with all features enabled to + // fetch compute_unit_price and compute_unit_limit for legacy leader. + static ref FEATURE_SET: FeatureSet = FeatureSet::all_enabled(); +} + #[derive(Debug)] pub struct ImmutableDeserializedPacket { original_packet: Packet, @@ -73,6 +80,7 @@ impl ImmutableDeserializedPacket { .get_message() .program_instructions_iter() .map(|(pubkey, ix)| (pubkey, SVMInstruction::from(ix))), + &FEATURE_SET, ) .map_err(|_| DeserializedPacketError::PrioritizationFailure)?; diff --git a/core/src/banking_stage/transaction_scheduler/receive_and_buffer.rs b/core/src/banking_stage/transaction_scheduler/receive_and_buffer.rs index 52d35f93e54355..21afe448d151d3 100644 --- a/core/src/banking_stage/transaction_scheduler/receive_and_buffer.rs +++ b/core/src/banking_stage/transaction_scheduler/receive_and_buffer.rs @@ -196,7 +196,7 @@ impl SanitizedTransactionReceiveAndBuffer { }) .filter_map(|(packet, tx, deactivation_slot)| { tx.compute_budget_instruction_details() - .sanitize_and_convert_to_compute_budget_limits() + .sanitize_and_convert_to_compute_budget_limits(&working_bank.feature_set) .map(|compute_budget| { (packet, tx, deactivation_slot, compute_budget.into()) }) diff --git a/cost-model/src/cost_model.rs b/cost-model/src/cost_model.rs index cce3b1015b1a36..612492b8b1126f 100644 --- a/cost-model/src/cost_model.rs +++ b/cost-model/src/cost_model.rs @@ -154,6 +154,24 @@ impl CostModel { fn get_transaction_cost( transaction: &impl TransactionWithMeta, feature_set: &FeatureSet, + ) -> (u64, u64, u64) { + if feature_set.is_active(&feature_set::reserve_minimal_cus_for_builtin_instructions::id()) { + let data_bytes_cost = Self::get_instructions_data_cost(transaction); + let (programs_execution_cost, loaded_accounts_data_size_cost) = + Self::get_estimated_execution_cost(transaction, feature_set); + ( + programs_execution_cost, + loaded_accounts_data_size_cost, + data_bytes_cost, + ) + } else { + Self::get_transaction_cost_without_minimal_builtin_cus(transaction, feature_set) + } + } + + fn get_transaction_cost_without_minimal_builtin_cus( + transaction: &impl TransactionWithMeta, + feature_set: &FeatureSet, ) -> (u64, u64, u64) { let mut programs_execution_costs = 0u64; let mut loaded_accounts_data_size_cost = 0u64; @@ -191,7 +209,7 @@ impl CostModel { // as no execution cost by cost model. match transaction .compute_budget_instruction_details() - .sanitize_and_convert_to_compute_budget_limits() + .sanitize_and_convert_to_compute_budget_limits(feature_set) { Ok(compute_budget_limits) => { // if tx contained user-space instructions and a more accurate @@ -224,6 +242,30 @@ impl CostModel { ) } + /// Return (programs_execution_cost, loaded_accounts_data_size_cost) + fn get_estimated_execution_cost( + transaction: &impl TransactionWithMeta, + feature_set: &FeatureSet, + ) -> (u64, u64) { + // if failed to process compute_budget instructions, the transaction will not be executed + // by `bank`, therefore it should be considered as no execution cost by cost model. + let (programs_execution_costs, loaded_accounts_data_size_cost) = match transaction + .compute_budget_instruction_details() + .sanitize_and_convert_to_compute_budget_limits(feature_set) + { + Ok(compute_budget_limits) => ( + u64::from(compute_budget_limits.compute_unit_limit), + Self::calculate_loaded_accounts_data_size_cost( + compute_budget_limits.loaded_accounts_bytes.get(), + feature_set, + ), + ), + Err(_) => (0, 0), + }; + + (programs_execution_costs, loaded_accounts_data_size_cost) + } + /// Return the instruction data bytes cost. fn get_instructions_data_cost(transaction: &impl SVMMessage) -> u64 { let ix_data_bytes_len_total: u64 = transaction @@ -316,7 +358,9 @@ mod tests { use { super::*, itertools::Itertools, - log::debug, + solana_compute_budget::compute_budget_limits::{ + DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT, + }, solana_runtime_transaction::runtime_transaction::RuntimeTransaction, solana_sdk::{ compute_budget::{self, ComputeBudgetInstruction}, @@ -515,20 +559,22 @@ mod tests { let simple_transaction = RuntimeTransaction::from_transaction_for_tests( system_transaction::transfer(&mint_keypair, &keypair.pubkey(), 2, start_hash), ); - debug!( - "system_transaction simple_transaction {:?}", - simple_transaction - ); - - // expected cost for one system transfer instructions - let expected_execution_cost = - solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS; - let (program_execution_cost, _loaded_accounts_data_size_cost, data_bytes_cost) = - CostModel::get_transaction_cost(&simple_transaction, &FeatureSet::all_enabled()); + for (feature_set, expected_execution_cost) in [ + ( + FeatureSet::default(), + solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS, + ), + ( + FeatureSet::all_enabled(), + u64::from(MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT), + ), + ] { + let (program_execution_cost, _loaded_accounts_data_size_cost, _data_bytes_cost) = + CostModel::get_transaction_cost(&simple_transaction, &feature_set); - assert_eq!(expected_execution_cost, program_execution_cost); - assert_eq!(3, data_bytes_cost); + assert_eq!(expected_execution_cost, program_execution_cost); + } } #[test] @@ -547,15 +593,23 @@ mod tests { instructions, ); let token_transaction = RuntimeTransaction::from_transaction_for_tests(tx); - debug!("token_transaction {:?}", token_transaction); - let (program_execution_cost, _loaded_accounts_data_size_cost, data_bytes_cost) = - CostModel::get_transaction_cost(&token_transaction, &FeatureSet::all_enabled()); - assert_eq!( - DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64, - program_execution_cost - ); - assert_eq!(0, data_bytes_cost); + for (feature_set, expected_execution_cost) in [ + ( + FeatureSet::default(), + DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64, + ), + ( + FeatureSet::all_enabled(), + DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64, + ), + ] { + let (program_execution_cost, _loaded_accounts_data_size_cost, data_bytes_cost) = + CostModel::get_transaction_cost(&token_transaction, &feature_set); + + assert_eq!(expected_execution_cost, program_execution_cost); + assert_eq!(0, data_bytes_cost); + } } #[test] @@ -587,12 +641,13 @@ mod tests { #[test] fn test_cost_model_compute_budget_transaction() { let (mint_keypair, start_hash) = test_setup(); + let expected_cu_limit = 12_345; let instructions = vec![ CompiledInstruction::new(3, &(), vec![1, 2, 0]), CompiledInstruction::new_from_raw_parts( 4, - ComputeBudgetInstruction::SetComputeUnitLimit(12_345) + ComputeBudgetInstruction::SetComputeUnitLimit(expected_cu_limit) .pack() .unwrap(), vec![], @@ -610,12 +665,17 @@ mod tests { ); let token_transaction = RuntimeTransaction::from_transaction_for_tests(tx); - let (program_execution_cost, _loaded_accounts_data_size_cost, data_bytes_cost) = - CostModel::get_transaction_cost(&token_transaction, &FeatureSet::all_enabled()); - // If cu-limit is specified, that would the cost for all programs - assert_eq!(12_345, program_execution_cost); - assert_eq!(1, data_bytes_cost); + for (feature_set, expected_execution_cost) in [ + (FeatureSet::default(), expected_cu_limit as u64), + (FeatureSet::all_enabled(), expected_cu_limit as u64), + ] { + let (program_execution_cost, _loaded_accounts_data_size_cost, data_bytes_cost) = + CostModel::get_transaction_cost(&token_transaction, &feature_set); + + assert_eq!(expected_execution_cost, program_execution_cost); + assert_eq!(1, data_bytes_cost); + } } #[test] @@ -652,9 +712,11 @@ mod tests { ); let token_transaction = RuntimeTransaction::from_transaction_for_tests(tx); - let (program_execution_cost, _loaded_accounts_data_size_cost, _data_bytes_cost) = - CostModel::get_transaction_cost(&token_transaction, &FeatureSet::all_enabled()); - assert_eq!(0, program_execution_cost); + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + let (program_execution_cost, _loaded_accounts_data_size_cost, _data_bytes_cost) = + CostModel::get_transaction_cost(&token_transaction, &feature_set); + assert_eq!(0, program_execution_cost); + } } #[test] @@ -671,16 +733,23 @@ mod tests { message, start_hash, )); - debug!("many transfer transaction {:?}", tx); // expected cost for two system transfer instructions - let program_cost = solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS; - let expected_cost = program_cost * 2; - - let (program_execution_cost, _loaded_accounts_data_size_cost, data_bytes_cost) = - CostModel::get_transaction_cost(&tx, &FeatureSet::all_enabled()); - assert_eq!(expected_cost, program_execution_cost); - assert_eq!(6, data_bytes_cost); + for (feature_set, expected_execution_cost) in [ + ( + FeatureSet::default(), + 2 * solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS, + ), + ( + FeatureSet::all_enabled(), + 2 * u64::from(MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT), + ), + ] { + let (programs_execution_cost, _loaded_accounts_data_size_cost, data_bytes_cost) = + CostModel::get_transaction_cost(&tx, &feature_set); + assert_eq!(expected_execution_cost, programs_execution_cost); + assert_eq!(6, data_bytes_cost); + } } #[test] @@ -705,13 +774,22 @@ mod tests { instructions, ), ); - debug!("many random transaction {:?}", tx); - let expected_cost = DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64 * 2; - let (program_execution_cost, _loaded_accounts_data_size_cost, data_bytes_cost) = - CostModel::get_transaction_cost(&tx, &FeatureSet::all_enabled()); - assert_eq!(expected_cost, program_execution_cost); - assert_eq!(0, data_bytes_cost); + for (feature_set, expected_cost) in [ + ( + FeatureSet::default(), + DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64 * 2, + ), + ( + FeatureSet::all_enabled(), + DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64 * 2, + ), + ] { + let (program_execution_cost, _loaded_accounts_data_size_cost, data_bytes_cost) = + CostModel::get_transaction_cost(&tx, &feature_set); + assert_eq!(expected_cost, program_execution_cost); + assert_eq!(0, data_bytes_cost); + } } #[test] @@ -757,23 +835,32 @@ mod tests { )); let expected_account_cost = WRITE_LOCK_UNITS * 2; - let expected_execution_cost = - solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS; - const DEFAULT_PAGE_COST: u64 = 8; - let expected_loaded_accounts_data_size_cost = - solana_compute_budget::compute_budget_limits::MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES.get() - as u64 - / ACCOUNT_DATA_COST_PAGE_SIZE - * DEFAULT_PAGE_COST; - - let tx_cost = CostModel::calculate_cost(&tx, &FeatureSet::all_enabled()); - assert_eq!(expected_account_cost, tx_cost.write_lock_cost()); - assert_eq!(expected_execution_cost, tx_cost.programs_execution_cost()); - assert_eq!(2, tx_cost.writable_accounts().count()); - assert_eq!( - expected_loaded_accounts_data_size_cost, - tx_cost.loaded_accounts_data_size_cost() - ); + for (feature_set, expected_execution_cost) in [ + ( + FeatureSet::default(), + solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS, + ), + ( + FeatureSet::all_enabled(), + u64::from(MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT), + ), + ] { + const DEFAULT_PAGE_COST: u64 = 8; + let expected_loaded_accounts_data_size_cost = + solana_compute_budget::compute_budget_limits::MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES + .get() as u64 + / ACCOUNT_DATA_COST_PAGE_SIZE + * DEFAULT_PAGE_COST; + + let tx_cost = CostModel::calculate_cost(&tx, &feature_set); + assert_eq!(expected_account_cost, tx_cost.write_lock_cost()); + assert_eq!(expected_execution_cost, tx_cost.programs_execution_cost()); + assert_eq!(2, tx_cost.writable_accounts().count()); + assert_eq!( + expected_loaded_accounts_data_size_cost, + tx_cost.loaded_accounts_data_size_cost() + ); + } } #[test] @@ -792,20 +879,29 @@ mod tests { start_hash, )); - let feature_set = FeatureSet::all_enabled(); let expected_account_cost = WRITE_LOCK_UNITS * 2; - let expected_execution_cost = solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS - + solana_compute_budget_program::DEFAULT_COMPUTE_UNITS; - let expected_loaded_accounts_data_size_cost = (data_limit as u64) / (32 * 1024) * 8; - - let tx_cost = CostModel::calculate_cost(&tx, &feature_set); - assert_eq!(expected_account_cost, tx_cost.write_lock_cost()); - assert_eq!(expected_execution_cost, tx_cost.programs_execution_cost()); - assert_eq!(2, tx_cost.writable_accounts().count()); - assert_eq!( - expected_loaded_accounts_data_size_cost, - tx_cost.loaded_accounts_data_size_cost() - ); + for (feature_set, expected_execution_cost) in [ + ( + FeatureSet::default(), + solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS + + solana_compute_budget_program::DEFAULT_COMPUTE_UNITS, + ), + ( + FeatureSet::all_enabled(), + 2 * u64::from(MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT), + ), + ] { + let expected_loaded_accounts_data_size_cost = (data_limit as u64) / (32 * 1024) * 8; + + let tx_cost = CostModel::calculate_cost(&tx, &feature_set); + assert_eq!(expected_account_cost, tx_cost.write_lock_cost()); + assert_eq!(expected_execution_cost, tx_cost.programs_execution_cost()); + assert_eq!(2, tx_cost.writable_accounts().count()); + assert_eq!( + expected_loaded_accounts_data_size_cost, + tx_cost.loaded_accounts_data_size_cost() + ); + } } #[test] @@ -823,38 +919,52 @@ mod tests { start_hash, )); // transaction has one builtin instruction, and one bpf instruction, no ComputeBudget::compute_unit_limit - let expected_builtin_cost = solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS; - let expected_bpf_cost = DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT; - - let (program_execution_cost, _loaded_accounts_data_size_cost, _data_bytes_cost) = - CostModel::get_transaction_cost(&transaction, &FeatureSet::all_enabled()); + for (feature_set, expected_execution_cost) in [ + ( + FeatureSet::default(), + solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS + + DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64, + ), + ( + FeatureSet::all_enabled(), + u64::from(MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT) + + u64::from(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT), + ), + ] { + let (programs_execution_cost, _loaded_accounts_data_size_cost, _data_bytes_cost) = + CostModel::get_transaction_cost(&transaction, &feature_set); - assert_eq!( - expected_builtin_cost + expected_bpf_cost as u64, - program_execution_cost - ); + assert_eq!(expected_execution_cost, programs_execution_cost); + } } #[test] fn test_transaction_cost_with_mix_instruction_with_cu_limit() { let (mint_keypair, start_hash) = test_setup(); + let cu_limit: u32 = 12_345; let transaction = RuntimeTransaction::from_transaction_for_tests(Transaction::new_signed_with_payer( &[ system_instruction::transfer(&mint_keypair.pubkey(), &Pubkey::new_unique(), 2), - ComputeBudgetInstruction::set_compute_unit_limit(12_345), + ComputeBudgetInstruction::set_compute_unit_limit(cu_limit), ], Some(&mint_keypair.pubkey()), &[&mint_keypair], start_hash, )); - // transaction has one builtin instruction, and one ComputeBudget::compute_unit_limit - let expected_cost = solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS - + solana_compute_budget_program::DEFAULT_COMPUTE_UNITS; + for (feature_set, expected_execution_cost) in [ + ( + FeatureSet::default(), + solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS + + solana_compute_budget_program::DEFAULT_COMPUTE_UNITS, + ), + (FeatureSet::all_enabled(), cu_limit as u64), + ] { + let (programs_execution_cost, _loaded_accounts_data_size_cost, _data_bytes_cost) = + CostModel::get_transaction_cost(&transaction, &feature_set); - let (program_execution_cost, _loaded_accounts_data_size_cost, _data_bytes_cost) = - CostModel::get_transaction_cost(&transaction, &FeatureSet::all_enabled()); - assert_eq!(expected_cost, program_execution_cost); + assert_eq!(expected_execution_cost, programs_execution_cost); + } } } diff --git a/cost-model/src/transaction_cost.rs b/cost-model/src/transaction_cost.rs index 2c704394a9ccc5..1b5f7a6265390f 100644 --- a/cost-model/src/transaction_cost.rs +++ b/cost-model/src/transaction_cost.rs @@ -186,7 +186,8 @@ impl solana_svm_transaction::svm_message::SVMMessage for WritableKeysTransaction fn program_instructions_iter( &self, - ) -> impl Iterator { + ) -> impl Iterator + Clone + { core::iter::empty() } @@ -323,8 +324,8 @@ mod tests { // expected vote tx cost: 2 write locks, 1 sig, 1 vote ix, 8cu of loaded accounts size, let expected_vote_cost = SIMPLE_VOTE_USAGE_COST; - // expected non-vote tx cost would include default loaded accounts size cost (16384) additionally - let expected_none_vote_cost = 20543; + // expected non-vote tx cost would include default loaded accounts size cost (16384) additionally, and 3_000 for instruction + let expected_none_vote_cost = 21443; let vote_cost = CostModel::calculate_cost(&vote_transaction, &FeatureSet::all_enabled()); let none_vote_cost = diff --git a/programs/compute-budget-bench/benches/compute_budget.rs b/programs/compute-budget-bench/benches/compute_budget.rs index 83b88302ad987c..7c758d45c7ed05 100644 --- a/programs/compute-budget-bench/benches/compute_budget.rs +++ b/programs/compute-budget-bench/benches/compute_budget.rs @@ -2,7 +2,10 @@ use { criterion::{black_box, criterion_group, criterion_main, Criterion}, solana_compute_budget::compute_budget_limits::ComputeBudgetLimits, solana_runtime_transaction::instructions_processor::process_compute_budget_instructions, - solana_sdk::{compute_budget::ComputeBudgetInstruction, instruction::CompiledInstruction}, + solana_sdk::{ + compute_budget::ComputeBudgetInstruction, feature_set::FeatureSet, + instruction::CompiledInstruction, + }, solana_svm_transaction::instruction::SVMInstruction, std::num::NonZero, }; @@ -19,15 +22,19 @@ fn bench_request_heap_frame(c: &mut Criterion) { vec![], ), )]; + let feature_set = FeatureSet::default(); c.bench_function("request_heap_limit", |bencher| { bencher.iter(|| { assert_eq!( - process_compute_budget_instructions(black_box( - instruction - .iter() - .map(|(id, ix)| (id, SVMInstruction::from(ix))) - )), + process_compute_budget_instructions( + black_box( + instruction + .iter() + .map(|(id, ix)| (id, SVMInstruction::from(ix))) + ), + black_box(&feature_set) + ), Ok(ComputeBudgetLimits { updated_heap_bytes: ONE_PAGE, compute_unit_limit: 0, @@ -48,15 +55,19 @@ fn bench_set_compute_unit_limit(c: &mut Criterion) { vec![], ), )]; + let feature_set = FeatureSet::default(); c.bench_function("set_compute_unit_limit", |bencher| { bencher.iter(|| { assert_eq!( - process_compute_budget_instructions(black_box( - instruction - .iter() - .map(|(id, ix)| (id, SVMInstruction::from(ix))) - )), + process_compute_budget_instructions( + black_box( + instruction + .iter() + .map(|(id, ix)| (id, SVMInstruction::from(ix))) + ), + black_box(&feature_set) + ), Ok(ComputeBudgetLimits { updated_heap_bytes: ONE_PAGE, compute_unit_limit: 1024, @@ -77,15 +88,19 @@ fn bench_set_compute_unit_price(c: &mut Criterion) { vec![], ), )]; + let feature_set = FeatureSet::default(); c.bench_function("set_compute_unit_price", |bencher| { bencher.iter(|| { assert_eq!( - process_compute_budget_instructions(black_box( - instruction - .iter() - .map(|(id, ix)| (id, SVMInstruction::from(ix))) - )), + process_compute_budget_instructions( + black_box( + instruction + .iter() + .map(|(id, ix)| (id, SVMInstruction::from(ix))) + ), + black_box(&feature_set) + ), Ok(ComputeBudgetLimits { updated_heap_bytes: ONE_PAGE, compute_unit_limit: 0, @@ -106,15 +121,19 @@ fn bench_set_loaded_accounts_data_size_limit(c: &mut Criterion) { vec![], ), )]; + let feature_set = FeatureSet::default(); c.bench_function("set_loaded_accounts_data_size_limit", |bencher| { bencher.iter(|| { assert_eq!( - process_compute_budget_instructions(black_box( - instruction - .iter() - .map(|(id, ix)| (id, SVMInstruction::from(ix))) - )), + process_compute_budget_instructions( + black_box( + instruction + .iter() + .map(|(id, ix)| (id, SVMInstruction::from(ix))) + ), + black_box(&feature_set) + ), Ok(ComputeBudgetLimits { updated_heap_bytes: ONE_PAGE, compute_unit_limit: 0, diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index f4421d19716aef..8a0a5e14e6b569 100644 --- a/programs/sbf/tests/programs.rs +++ b/programs/sbf/tests/programs.rs @@ -3857,6 +3857,7 @@ fn test_program_fees() { FeeStructure::new(0.000005, 0.0, vec![(200, 0.0000005), (1400000, 0.000005)]); bank.set_fee_structure(&fee_structure); let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests(); + let feature_set = bank.feature_set.clone(); let mut bank_client = BankClient::new_shared(bank); let authority_keypair = Keypair::new(); @@ -3880,9 +3881,10 @@ fn test_program_fees() { ) .unwrap(); let fee_budget_limits = FeeBudgetLimits::from( - process_compute_budget_instructions(SVMMessage::program_instructions_iter( - &sanitized_message, - )) + process_compute_budget_instructions( + SVMMessage::program_instructions_iter(&sanitized_message), + &feature_set, + ) .unwrap_or_default(), ); let expected_normal_fee = solana_fee::calculate_fee( @@ -3912,9 +3914,10 @@ fn test_program_fees() { ) .unwrap(); let fee_budget_limits = FeeBudgetLimits::from( - process_compute_budget_instructions(SVMMessage::program_instructions_iter( - &sanitized_message, - )) + process_compute_budget_instructions( + SVMMessage::program_instructions_iter(&sanitized_message), + &feature_set, + ) .unwrap_or_default(), ); let expected_prioritized_fee = solana_fee::calculate_fee( diff --git a/runtime-transaction/benches/process_compute_budget_instructions.rs b/runtime-transaction/benches/process_compute_budget_instructions.rs index 76f6b590948875..c120b5681b5c29 100644 --- a/runtime-transaction/benches/process_compute_budget_instructions.rs +++ b/runtime-transaction/benches/process_compute_budget_instructions.rs @@ -3,6 +3,7 @@ use { solana_runtime_transaction::instructions_processor::process_compute_budget_instructions, solana_sdk::{ compute_budget::ComputeBudgetInstruction, + feature_set::FeatureSet, instruction::Instruction, message::Message, pubkey::Pubkey, @@ -28,134 +29,153 @@ fn build_sanitized_transaction( } fn bench_process_compute_budget_instructions_empty(c: &mut Criterion) { - c.benchmark_group("bench_process_compute_budget_instructions_empty") - .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) - .bench_function("0 instructions", |bencher| { - let tx = build_sanitized_transaction(&Keypair::new(), &[]); - bencher.iter(|| { - (0..NUM_TRANSACTIONS_PER_ITER).for_each(|_| { - assert!(process_compute_budget_instructions(black_box( - SVMMessage::program_instructions_iter(&tx) - )) - .is_ok()) - }) + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + c.benchmark_group("bench_process_compute_budget_instructions_empty") + .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) + .bench_function("0 instructions", |bencher| { + let tx = build_sanitized_transaction(&Keypair::new(), &[]); + bencher.iter(|| { + (0..NUM_TRANSACTIONS_PER_ITER).for_each(|_| { + assert!(process_compute_budget_instructions( + black_box(SVMMessage::program_instructions_iter(&tx)), + black_box(&feature_set), + ) + .is_ok()) + }) + }); }); - }); + } } fn bench_process_compute_budget_instructions_no_builtins(c: &mut Criterion) { let num_instructions = 4; - c.benchmark_group("bench_process_compute_budget_instructions_no_builtins") - .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) - .bench_function( - format!("{num_instructions} dummy Instructions"), - |bencher| { - let ixs: Vec<_> = (0..num_instructions) - .map(|_| { - Instruction::new_with_bincode( - DUMMY_PROGRAM_ID.parse().unwrap(), - &(), - vec![], - ) - }) - .collect(); + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + c.benchmark_group("bench_process_compute_budget_instructions_no_builtins") + .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) + .bench_function( + format!("{num_instructions} dummy Instructions"), + |bencher| { + let ixs: Vec<_> = (0..num_instructions) + .map(|_| { + Instruction::new_with_bincode( + DUMMY_PROGRAM_ID.parse().unwrap(), + &(), + vec![], + ) + }) + .collect(); + let tx = build_sanitized_transaction(&Keypair::new(), &ixs); + bencher.iter(|| { + (0..NUM_TRANSACTIONS_PER_ITER).for_each(|_| { + assert!(process_compute_budget_instructions( + black_box(SVMMessage::program_instructions_iter(&tx)), + black_box(&feature_set), + ) + .is_ok()) + }) + }); + }, + ); + } +} + +fn bench_process_compute_budget_instructions_compute_budgets(c: &mut Criterion) { + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + c.benchmark_group("bench_process_compute_budget_instructions_compute_budgets") + .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) + .bench_function("4 compute-budget instructions", |bencher| { + let ixs = vec![ + ComputeBudgetInstruction::request_heap_frame(40 * 1024), + ComputeBudgetInstruction::set_compute_unit_limit(u32::MAX), + ComputeBudgetInstruction::set_compute_unit_price(u64::MAX), + ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(u32::MAX), + ]; let tx = build_sanitized_transaction(&Keypair::new(), &ixs); bencher.iter(|| { (0..NUM_TRANSACTIONS_PER_ITER).for_each(|_| { - assert!(process_compute_budget_instructions(black_box( - SVMMessage::program_instructions_iter(&tx) - )) + assert!(process_compute_budget_instructions( + black_box(SVMMessage::program_instructions_iter(&tx)), + black_box(&feature_set), + ) .is_ok()) }) }); - }, - ); -} - -fn bench_process_compute_budget_instructions_compute_budgets(c: &mut Criterion) { - c.benchmark_group("bench_process_compute_budget_instructions_compute_budgets") - .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) - .bench_function("4 compute-budget instructions", |bencher| { - let ixs = vec![ - ComputeBudgetInstruction::request_heap_frame(40 * 1024), - ComputeBudgetInstruction::set_compute_unit_limit(u32::MAX), - ComputeBudgetInstruction::set_compute_unit_price(u64::MAX), - ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(u32::MAX), - ]; - let tx = build_sanitized_transaction(&Keypair::new(), &ixs); - bencher.iter(|| { - (0..NUM_TRANSACTIONS_PER_ITER).for_each(|_| { - assert!(process_compute_budget_instructions(black_box( - SVMMessage::program_instructions_iter(&tx) - )) - .is_ok()) - }) }); - }); + } } fn bench_process_compute_budget_instructions_builtins(c: &mut Criterion) { - c.benchmark_group("bench_process_compute_budget_instructions_builtins") - .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) - .bench_function("4 dummy builtins", |bencher| { - let ixs = vec![ - Instruction::new_with_bincode(solana_sdk::bpf_loader::id(), &(), vec![]), - Instruction::new_with_bincode(solana_sdk::secp256k1_program::id(), &(), vec![]), - Instruction::new_with_bincode( - solana_sdk::address_lookup_table::program::id(), - &(), - vec![], - ), - Instruction::new_with_bincode(solana_sdk::loader_v4::id(), &(), vec![]), - ]; - let tx = build_sanitized_transaction(&Keypair::new(), &ixs); - bencher.iter(|| { - (0..NUM_TRANSACTIONS_PER_ITER).for_each(|_| { - assert!(process_compute_budget_instructions(black_box( - SVMMessage::program_instructions_iter(&tx) - )) - .is_ok()) - }) + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + c.benchmark_group("bench_process_compute_budget_instructions_builtins") + .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) + .bench_function("4 dummy builtins", |bencher| { + let ixs = vec![ + Instruction::new_with_bincode(solana_sdk::bpf_loader::id(), &(), vec![]), + Instruction::new_with_bincode(solana_sdk::secp256k1_program::id(), &(), vec![]), + Instruction::new_with_bincode( + solana_sdk::address_lookup_table::program::id(), + &(), + vec![], + ), + Instruction::new_with_bincode(solana_sdk::loader_v4::id(), &(), vec![]), + ]; + let tx = build_sanitized_transaction(&Keypair::new(), &ixs); + bencher.iter(|| { + (0..NUM_TRANSACTIONS_PER_ITER).for_each(|_| { + assert!(process_compute_budget_instructions( + black_box(SVMMessage::program_instructions_iter(&tx)), + black_box(&feature_set), + ) + .is_ok()) + }) + }); }); - }); + } } fn bench_process_compute_budget_instructions_mixed(c: &mut Criterion) { let num_instructions = 355; - c.benchmark_group("bench_process_compute_budget_instructions_mixed") - .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) - .bench_function( - format!("{num_instructions} mixed instructions"), - |bencher| { - let payer_keypair = Keypair::new(); - let mut ixs: Vec<_> = (0..num_instructions) - .map(|_| { - Instruction::new_with_bincode( - DUMMY_PROGRAM_ID.parse().unwrap(), - &(), - vec![], - ) - }) - .collect(); - ixs.extend(vec![ - ComputeBudgetInstruction::request_heap_frame(40 * 1024), - ComputeBudgetInstruction::set_compute_unit_limit(u32::MAX), - ComputeBudgetInstruction::set_compute_unit_price(u64::MAX), - ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(u32::MAX), - system_instruction::transfer(&payer_keypair.pubkey(), &Pubkey::new_unique(), 1), - ]); - let tx = build_sanitized_transaction(&payer_keypair, &ixs); + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + c.benchmark_group("bench_process_compute_budget_instructions_mixed") + .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) + .bench_function( + format!("{num_instructions} mixed instructions"), + |bencher| { + let payer_keypair = Keypair::new(); + let mut ixs: Vec<_> = (0..num_instructions) + .map(|_| { + Instruction::new_with_bincode( + DUMMY_PROGRAM_ID.parse().unwrap(), + &(), + vec![], + ) + }) + .collect(); + ixs.extend(vec![ + ComputeBudgetInstruction::request_heap_frame(40 * 1024), + ComputeBudgetInstruction::set_compute_unit_limit(u32::MAX), + ComputeBudgetInstruction::set_compute_unit_price(u64::MAX), + ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(u32::MAX), + system_instruction::transfer( + &payer_keypair.pubkey(), + &Pubkey::new_unique(), + 1, + ), + ]); + let tx = build_sanitized_transaction(&payer_keypair, &ixs); - bencher.iter(|| { - (0..NUM_TRANSACTIONS_PER_ITER).for_each(|_| { - assert!(process_compute_budget_instructions(black_box( - SVMMessage::program_instructions_iter(&tx) - )) - .is_ok()) - }) - }); - }, - ); + bencher.iter(|| { + (0..NUM_TRANSACTIONS_PER_ITER).for_each(|_| { + assert!(process_compute_budget_instructions( + black_box(SVMMessage::program_instructions_iter(&tx)), + black_box(&feature_set), + ) + .is_ok()) + }) + }); + }, + ); + } } criterion_group!( diff --git a/runtime-transaction/src/builtin_programs_filter.rs b/runtime-transaction/src/builtin_programs_filter.rs new file mode 100644 index 00000000000000..fc935a023cfbf8 --- /dev/null +++ b/runtime-transaction/src/builtin_programs_filter.rs @@ -0,0 +1,105 @@ +use { + agave_transaction_view::static_account_keys_frame::MAX_STATIC_ACCOUNTS_PER_PACKET as FILTER_SIZE, + solana_builtins_default_costs::{is_builtin_program, MAYBE_BUILTIN_KEY}, + solana_sdk::pubkey::Pubkey, +}; + +#[derive(Clone, Copy, Debug, PartialEq)] +pub(crate) enum ProgramKind { + NotBuiltin, + Builtin, +} + +pub(crate) struct BuiltinProgramsFilter { + // array of slots for all possible static and sanitized program_id_index, + // each slot indicates if a program_id_index has not been checked (eg, None), + // or already checked with result (eg, Some(ProgramKind)) that can be reused. + program_kind: [Option; FILTER_SIZE as usize], +} + +impl BuiltinProgramsFilter { + pub(crate) fn new() -> Self { + BuiltinProgramsFilter { + program_kind: [None; FILTER_SIZE as usize], + } + } + + pub(crate) fn get_program_kind(&mut self, index: usize, program_id: &Pubkey) -> ProgramKind { + *self + .program_kind + .get_mut(index) + .expect("program id index is sanitized") + .get_or_insert_with(|| Self::check_program_kind(program_id)) + } + + #[inline] + fn check_program_kind(program_id: &Pubkey) -> ProgramKind { + if !MAYBE_BUILTIN_KEY[program_id.as_ref()[0] as usize] { + return ProgramKind::NotBuiltin; + } + + if is_builtin_program(program_id) { + ProgramKind::Builtin + } else { + ProgramKind::NotBuiltin + } + } +} + +#[cfg(test)] +mod test { + use super::*; + + const DUMMY_PROGRAM_ID: &str = "dummmy1111111111111111111111111111111111111"; + + #[test] + fn get_program_kind() { + let mut test_store = BuiltinProgramsFilter::new(); + let mut index = 9; + + // initial state is Unchecked + assert!(test_store.program_kind[index].is_none()); + + // non builtin returns None + assert_eq!( + test_store.get_program_kind(index, &DUMMY_PROGRAM_ID.parse().unwrap()), + ProgramKind::NotBuiltin + ); + // but its state is now checked (eg, Some(...)) + assert_eq!( + test_store.program_kind[index], + Some(ProgramKind::NotBuiltin) + ); + // lookup same `index` will return cached data, will not lookup `program_id` + // again + assert_eq!( + test_store.get_program_kind(index, &solana_sdk::loader_v4::id()), + ProgramKind::NotBuiltin + ); + + // not-migrating builtin + index += 1; + assert_eq!( + test_store.get_program_kind(index, &solana_sdk::loader_v4::id()), + ProgramKind::Builtin, + ); + + // compute-budget + index += 1; + assert_eq!( + test_store.get_program_kind(index, &solana_sdk::compute_budget::id()), + ProgramKind::Builtin, + ); + } + + #[test] + #[should_panic(expected = "program id index is sanitized")] + fn test_get_program_kind_out_of_bound_index() { + let mut test_store = BuiltinProgramsFilter::new(); + assert_eq!( + test_store + .get_program_kind(FILTER_SIZE as usize + 1, &DUMMY_PROGRAM_ID.parse().unwrap(),), + ProgramKind::NotBuiltin + ); + } +} diff --git a/runtime-transaction/src/compute_budget_instruction_details.rs b/runtime-transaction/src/compute_budget_instruction_details.rs index 5b3ef8de0e737e..dba59b8f343d13 100644 --- a/runtime-transaction/src/compute_budget_instruction_details.rs +++ b/runtime-transaction/src/compute_budget_instruction_details.rs @@ -1,9 +1,13 @@ use { - crate::compute_budget_program_id_filter::ComputeBudgetProgramIdFilter, + crate::{ + builtin_programs_filter::{BuiltinProgramsFilter, ProgramKind}, + compute_budget_program_id_filter::ComputeBudgetProgramIdFilter, + }, solana_compute_budget::compute_budget_limits::*, solana_sdk::{ borsh1::try_from_slice_unchecked, compute_budget::ComputeBudgetInstruction, + feature_set::{self, FeatureSet}, instruction::InstructionError, pubkey::Pubkey, saturating_add_assign, @@ -23,17 +27,20 @@ pub struct ComputeBudgetInstructionDetails { requested_compute_unit_price: Option<(u8, u64)>, requested_heap_size: Option<(u8, u32)>, requested_loaded_accounts_data_size_limit: Option<(u8, u32)>, - num_non_compute_budget_instructions: u32, + num_non_compute_budget_instructions: u16, + // Additional builtin program counters + num_builtin_instructions: u16, + num_non_builtin_instructions: u16, } impl ComputeBudgetInstructionDetails { pub fn try_from<'a>( - instructions: impl Iterator)>, + instructions: impl Iterator)> + Clone, ) -> Result { let mut filter = ComputeBudgetProgramIdFilter::new(); - let mut compute_budget_instruction_details = ComputeBudgetInstructionDetails::default(); - for (i, (program_id, instruction)) in instructions.enumerate() { + + for (i, (program_id, instruction)) in instructions.clone().enumerate() { if filter.is_compute_budget_program(instruction.program_id_index as usize, program_id) { compute_budget_instruction_details.process_instruction(i as u8, &instruction)?; } else { @@ -44,10 +51,37 @@ impl ComputeBudgetInstructionDetails { } } + if compute_budget_instruction_details + .requested_compute_unit_limit + .is_none() + { + let mut filter = BuiltinProgramsFilter::new(); + // reiterate to collect builtin details + for (program_id, instruction) in instructions { + match filter.get_program_kind(instruction.program_id_index as usize, program_id) { + ProgramKind::Builtin => { + saturating_add_assign!( + compute_budget_instruction_details.num_builtin_instructions, + 1 + ); + } + ProgramKind::NotBuiltin => { + saturating_add_assign!( + compute_budget_instruction_details.num_non_builtin_instructions, + 1 + ); + } + } + } + } + Ok(compute_budget_instruction_details) } - pub fn sanitize_and_convert_to_compute_budget_limits(&self) -> Result { + pub fn sanitize_and_convert_to_compute_budget_limits( + &self, + feature_set: &FeatureSet, + ) -> Result { // Sanitize requested heap size let updated_heap_bytes = if let Some((index, requested_heap_size)) = self.requested_heap_size { @@ -68,10 +102,7 @@ impl ComputeBudgetInstructionDetails { let compute_unit_limit = self .requested_compute_unit_limit .map_or_else( - || { - self.num_non_compute_budget_instructions - .saturating_mul(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT) - }, + || self.calculate_default_compute_unit_limit(feature_set), |(_index, requested_compute_unit_limit)| requested_compute_unit_limit, ) .min(MAX_COMPUTE_UNIT_LIMIT); @@ -137,9 +168,24 @@ impl ComputeBudgetInstructionDetails { Ok(()) } + #[inline] fn sanitize_requested_heap_size(bytes: u32) -> bool { (MIN_HEAP_FRAME_BYTES..=MAX_HEAP_FRAME_BYTES).contains(&bytes) && bytes % 1024 == 0 } + + fn calculate_default_compute_unit_limit(&self, feature_set: &FeatureSet) -> u32 { + if feature_set.is_active(&feature_set::reserve_minimal_cus_for_builtin_instructions::id()) { + u32::from(self.num_builtin_instructions) + .saturating_mul(MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT) + .saturating_add( + u32::from(self.num_non_builtin_instructions) + .saturating_mul(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT), + ) + } else { + u32::from(self.num_non_compute_budget_instructions) + .saturating_mul(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT) + } + } } #[cfg(test)] @@ -172,14 +218,16 @@ mod test { ComputeBudgetInstruction::request_heap_frame(40 * 1024), Instruction::new_with_bincode(Pubkey::new_unique(), &(), vec![]), ]); - let expected_details = ComputeBudgetInstructionDetails { + let expected_details = Ok(ComputeBudgetInstructionDetails { requested_heap_size: Some((1, 40 * 1024)), num_non_compute_budget_instructions: 2, + num_builtin_instructions: 1, + num_non_builtin_instructions: 2, ..ComputeBudgetInstructionDetails::default() - }; + }); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), - Ok(expected_details) + ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),), + expected_details ); let tx = build_sanitized_transaction(&[ @@ -188,7 +236,7 @@ mod test { ComputeBudgetInstruction::request_heap_frame(41 * 1024), ]); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), + ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),), Err(TransactionError::DuplicateInstruction(2)) ); } @@ -200,14 +248,14 @@ mod test { ComputeBudgetInstruction::set_compute_unit_limit(u32::MAX), Instruction::new_with_bincode(Pubkey::new_unique(), &(), vec![]), ]); - let expected_details = ComputeBudgetInstructionDetails { + let expected_details = Ok(ComputeBudgetInstructionDetails { requested_compute_unit_limit: Some((1, u32::MAX)), num_non_compute_budget_instructions: 2, ..ComputeBudgetInstructionDetails::default() - }; + }); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), - Ok(expected_details) + ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),), + expected_details ); let tx = build_sanitized_transaction(&[ @@ -216,7 +264,7 @@ mod test { ComputeBudgetInstruction::set_compute_unit_limit(u32::MAX), ]); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), + ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),), Err(TransactionError::DuplicateInstruction(2)) ); } @@ -228,14 +276,16 @@ mod test { ComputeBudgetInstruction::set_compute_unit_price(u64::MAX), Instruction::new_with_bincode(Pubkey::new_unique(), &(), vec![]), ]); - let expected_details = ComputeBudgetInstructionDetails { + let expected_details = Ok(ComputeBudgetInstructionDetails { requested_compute_unit_price: Some((1, u64::MAX)), num_non_compute_budget_instructions: 2, + num_builtin_instructions: 1, + num_non_builtin_instructions: 2, ..ComputeBudgetInstructionDetails::default() - }; + }); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), - Ok(expected_details) + ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),), + expected_details ); let tx = build_sanitized_transaction(&[ @@ -244,7 +294,7 @@ mod test { ComputeBudgetInstruction::set_compute_unit_price(u64::MAX), ]); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), + ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),), Err(TransactionError::DuplicateInstruction(2)) ); } @@ -256,14 +306,16 @@ mod test { ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(u32::MAX), Instruction::new_with_bincode(Pubkey::new_unique(), &(), vec![]), ]); - let expected_details = ComputeBudgetInstructionDetails { + let expected_details = Ok(ComputeBudgetInstructionDetails { requested_loaded_accounts_data_size_limit: Some((1, u32::MAX)), num_non_compute_budget_instructions: 2, + num_builtin_instructions: 1, + num_non_builtin_instructions: 2, ..ComputeBudgetInstructionDetails::default() - }; + }); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), - Ok(expected_details) + ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),), + expected_details ); let tx = build_sanitized_transaction(&[ @@ -272,39 +324,67 @@ mod test { ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(u32::MAX), ]); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), + ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),), Err(TransactionError::DuplicateInstruction(2)) ); } + fn prep_feature_minimial_cus_for_builtin_instructions( + is_active: bool, + instruction_details: &ComputeBudgetInstructionDetails, + ) -> (FeatureSet, u32) { + let mut feature_set = FeatureSet::default(); + let ComputeBudgetInstructionDetails { + num_non_compute_budget_instructions, + num_builtin_instructions, + num_non_builtin_instructions, + .. + } = *instruction_details; + let expected_cu_limit = if is_active { + feature_set.activate( + &feature_set::reserve_minimal_cus_for_builtin_instructions::id(), + 0, + ); + u32::from(num_non_builtin_instructions) * DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT + + u32::from(num_builtin_instructions) * MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT + } else { + u32::from(num_non_compute_budget_instructions) * DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT + }; + + (feature_set, expected_cu_limit) + } + #[test] fn test_sanitize_and_convert_to_compute_budget_limits() { // empty details, default ComputeBudgetLimits with 0 compute_unit_limits let instruction_details = ComputeBudgetInstructionDetails::default(); assert_eq!( - instruction_details.sanitize_and_convert_to_compute_budget_limits(), + instruction_details + .sanitize_and_convert_to_compute_budget_limits(&FeatureSet::default()), Ok(ComputeBudgetLimits { compute_unit_limit: 0, ..ComputeBudgetLimits::default() }) ); - let num_non_compute_budget_instructions = 4; - // no compute-budget instructions, all default ComputeBudgetLimits except cu-limit let instruction_details = ComputeBudgetInstructionDetails { - num_non_compute_budget_instructions, + num_non_compute_budget_instructions: 4, + num_builtin_instructions: 1, + num_non_builtin_instructions: 3, ..ComputeBudgetInstructionDetails::default() }; - let expected_compute_unit_limit = - num_non_compute_budget_instructions * DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT; - assert_eq!( - instruction_details.sanitize_and_convert_to_compute_budget_limits(), - Ok(ComputeBudgetLimits { - compute_unit_limit: expected_compute_unit_limit, - ..ComputeBudgetLimits::default() - }) - ); + for is_active in [true, false] { + let (feature_set, expected_compute_unit_limit) = + prep_feature_minimial_cus_for_builtin_instructions(is_active, &instruction_details); + assert_eq!( + instruction_details.sanitize_and_convert_to_compute_budget_limits(&feature_set), + Ok(ComputeBudgetLimits { + compute_unit_limit: expected_compute_unit_limit, + ..ComputeBudgetLimits::default() + }) + ); + } let expected_heap_size_err = Err(TransactionError::InstructionError( 3, @@ -316,12 +396,16 @@ mod test { requested_compute_unit_price: Some((2, 0)), requested_heap_size: Some((3, 0)), requested_loaded_accounts_data_size_limit: Some((4, 1024)), - num_non_compute_budget_instructions, + ..ComputeBudgetInstructionDetails::default() }; - assert_eq!( - instruction_details.sanitize_and_convert_to_compute_budget_limits(), - expected_heap_size_err - ); + for is_active in [true, false] { + let (feature_set, _expected_compute_unit_limit) = + prep_feature_minimial_cus_for_builtin_instructions(is_active, &instruction_details); + assert_eq!( + instruction_details.sanitize_and_convert_to_compute_budget_limits(&feature_set), + expected_heap_size_err + ); + } // invalid: requested_heap_size can't be less than MIN_HEAP_FRAME_BYTES let instruction_details = ComputeBudgetInstructionDetails { @@ -329,12 +413,16 @@ mod test { requested_compute_unit_price: Some((2, 0)), requested_heap_size: Some((3, MIN_HEAP_FRAME_BYTES - 1)), requested_loaded_accounts_data_size_limit: Some((4, 1024)), - num_non_compute_budget_instructions, + ..ComputeBudgetInstructionDetails::default() }; - assert_eq!( - instruction_details.sanitize_and_convert_to_compute_budget_limits(), - expected_heap_size_err - ); + for is_active in [true, false] { + let (feature_set, _expected_compute_unit_limit) = + prep_feature_minimial_cus_for_builtin_instructions(is_active, &instruction_details); + assert_eq!( + instruction_details.sanitize_and_convert_to_compute_budget_limits(&feature_set), + expected_heap_size_err + ); + } // invalid: requested_heap_size can't be more than MAX_HEAP_FRAME_BYTES let instruction_details = ComputeBudgetInstructionDetails { @@ -342,12 +430,16 @@ mod test { requested_compute_unit_price: Some((2, 0)), requested_heap_size: Some((3, MAX_HEAP_FRAME_BYTES + 1)), requested_loaded_accounts_data_size_limit: Some((4, 1024)), - num_non_compute_budget_instructions, + ..ComputeBudgetInstructionDetails::default() }; - assert_eq!( - instruction_details.sanitize_and_convert_to_compute_budget_limits(), - expected_heap_size_err - ); + for is_active in [true, false] { + let (feature_set, _expected_compute_unit_limit) = + prep_feature_minimial_cus_for_builtin_instructions(is_active, &instruction_details); + assert_eq!( + instruction_details.sanitize_and_convert_to_compute_budget_limits(&feature_set), + expected_heap_size_err + ); + } // invalid: requested_heap_size must be round by 1024 let instruction_details = ComputeBudgetInstructionDetails { @@ -355,12 +447,16 @@ mod test { requested_compute_unit_price: Some((2, 0)), requested_heap_size: Some((3, MIN_HEAP_FRAME_BYTES + 1024 + 1)), requested_loaded_accounts_data_size_limit: Some((4, 1024)), - num_non_compute_budget_instructions, + ..ComputeBudgetInstructionDetails::default() }; - assert_eq!( - instruction_details.sanitize_and_convert_to_compute_budget_limits(), - expected_heap_size_err - ); + for is_active in [true, false] { + let (feature_set, _expected_compute_unit_limit) = + prep_feature_minimial_cus_for_builtin_instructions(is_active, &instruction_details); + assert_eq!( + instruction_details.sanitize_and_convert_to_compute_budget_limits(&feature_set), + expected_heap_size_err + ); + } // invalid: loaded_account_data_size can't be zero let instruction_details = ComputeBudgetInstructionDetails { @@ -368,12 +464,16 @@ mod test { requested_compute_unit_price: Some((2, 0)), requested_heap_size: Some((3, 40 * 1024)), requested_loaded_accounts_data_size_limit: Some((4, 0)), - num_non_compute_budget_instructions, + ..ComputeBudgetInstructionDetails::default() }; - assert_eq!( - instruction_details.sanitize_and_convert_to_compute_budget_limits(), - Err(TransactionError::InvalidLoadedAccountsDataSizeLimit) - ); + for is_active in [true, false] { + let (feature_set, _expected_compute_unit_limit) = + prep_feature_minimial_cus_for_builtin_instructions(is_active, &instruction_details); + assert_eq!( + instruction_details.sanitize_and_convert_to_compute_budget_limits(&feature_set), + Err(TransactionError::InvalidLoadedAccountsDataSizeLimit) + ); + } // valid: acceptable MAX let instruction_details = ComputeBudgetInstructionDetails { @@ -381,17 +481,22 @@ mod test { requested_compute_unit_price: Some((2, u64::MAX)), requested_heap_size: Some((3, MAX_HEAP_FRAME_BYTES)), requested_loaded_accounts_data_size_limit: Some((4, u32::MAX)), - num_non_compute_budget_instructions, + num_non_compute_budget_instructions: 4, + ..ComputeBudgetInstructionDetails::default() }; - assert_eq!( - instruction_details.sanitize_and_convert_to_compute_budget_limits(), - Ok(ComputeBudgetLimits { - updated_heap_bytes: MAX_HEAP_FRAME_BYTES, - compute_unit_limit: MAX_COMPUTE_UNIT_LIMIT, - compute_unit_price: u64::MAX, - loaded_accounts_bytes: MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, - }) - ); + for is_active in [true, false] { + let (feature_set, _expected_compute_unit_limit) = + prep_feature_minimial_cus_for_builtin_instructions(is_active, &instruction_details); + assert_eq!( + instruction_details.sanitize_and_convert_to_compute_budget_limits(&feature_set), + Ok(ComputeBudgetLimits { + updated_heap_bytes: MAX_HEAP_FRAME_BYTES, + compute_unit_limit: MAX_COMPUTE_UNIT_LIMIT, + compute_unit_price: u64::MAX, + loaded_accounts_bytes: MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, + }) + ); + } // valid let val: u32 = 1024 * 40; @@ -400,16 +505,20 @@ mod test { requested_compute_unit_price: Some((2, val as u64)), requested_heap_size: Some((3, val)), requested_loaded_accounts_data_size_limit: Some((4, val)), - num_non_compute_budget_instructions, + ..ComputeBudgetInstructionDetails::default() }; - assert_eq!( - instruction_details.sanitize_and_convert_to_compute_budget_limits(), - Ok(ComputeBudgetLimits { - updated_heap_bytes: val, - compute_unit_limit: val, - compute_unit_price: val as u64, - loaded_accounts_bytes: NonZeroU32::new(val).unwrap(), - }) - ); + for is_active in [true, false] { + let (feature_set, _expected_compute_unit_limit) = + prep_feature_minimial_cus_for_builtin_instructions(is_active, &instruction_details); + assert_eq!( + instruction_details.sanitize_and_convert_to_compute_budget_limits(&feature_set), + Ok(ComputeBudgetLimits { + updated_heap_bytes: val, + compute_unit_limit: val, + compute_unit_price: val as u64, + loaded_accounts_bytes: NonZeroU32::new(val).unwrap(), + }) + ); + } } } diff --git a/runtime-transaction/src/compute_budget_program_id_filter.rs b/runtime-transaction/src/compute_budget_program_id_filter.rs index 87c12784222f90..59c6144a091229 100644 --- a/runtime-transaction/src/compute_budget_program_id_filter.rs +++ b/runtime-transaction/src/compute_budget_program_id_filter.rs @@ -18,7 +18,6 @@ impl ComputeBudgetProgramIdFilter { } } - #[inline] pub(crate) fn is_compute_budget_program(&mut self, index: usize, program_id: &Pubkey) -> bool { *self .flags diff --git a/runtime-transaction/src/instructions_processor.rs b/runtime-transaction/src/instructions_processor.rs index 1edba220096276..6259044feb9023 100644 --- a/runtime-transaction/src/instructions_processor.rs +++ b/runtime-transaction/src/instructions_processor.rs @@ -1,7 +1,7 @@ use { crate::compute_budget_instruction_details::*, solana_compute_budget::compute_budget_limits::*, - solana_sdk::{pubkey::Pubkey, transaction::TransactionError}, + solana_sdk::{feature_set::FeatureSet, pubkey::Pubkey, transaction::TransactionError}, solana_svm_transaction::instruction::SVMInstruction, }; @@ -11,10 +11,11 @@ use { /// If succeeded, the transaction's specific limits/requests (could be default) /// are retrieved and returned, pub fn process_compute_budget_instructions<'a>( - instructions: impl Iterator)>, + instructions: impl Iterator)> + Clone, + feature_set: &FeatureSet, ) -> Result { ComputeBudgetInstructionDetails::try_from(instructions)? - .sanitize_and_convert_to_compute_budget_limits() + .sanitize_and_convert_to_compute_budget_limits(feature_set) } #[cfg(test)] @@ -38,14 +39,22 @@ mod tests { macro_rules! test { ( $instructions: expr, $expected_result: expr) => { + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + test!($instructions, $expected_result, &feature_set); + } + }; + ( $instructions: expr, $expected_result: expr, $feature_set: expr) => { let payer_keypair = Keypair::new(); let tx = SanitizedTransaction::from_transaction_for_tests(Transaction::new( &[&payer_keypair], Message::new($instructions, Some(&payer_keypair.pubkey())), Hash::default(), )); - let result = - process_compute_budget_instructions(SVMMessage::program_instructions_iter(&tx)); + + let result = process_compute_budget_instructions( + SVMMessage::program_instructions_iter(&tx), + $feature_set, + ); assert_eq!($expected_result, result); }; } @@ -131,7 +140,21 @@ mod tests { compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, updated_heap_bytes: 40 * 1024, ..ComputeBudgetLimits::default() - }) + }), + &FeatureSet::default() + ); + test!( + &[ + ComputeBudgetInstruction::request_heap_frame(40 * 1024), + Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), + ], + Ok(ComputeBudgetLimits { + compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT + + MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT, + updated_heap_bytes: 40 * 1024, + ..ComputeBudgetLimits::default() + }), + &FeatureSet::all_enabled() ); test!( &[ @@ -172,7 +195,21 @@ mod tests { compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, updated_heap_bytes: MAX_HEAP_FRAME_BYTES, ..ComputeBudgetLimits::default() - }) + }), + &FeatureSet::default() + ); + test!( + &[ + Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), + ComputeBudgetInstruction::request_heap_frame(MAX_HEAP_FRAME_BYTES), + ], + Ok(ComputeBudgetLimits { + compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT + + MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT, + updated_heap_bytes: MAX_HEAP_FRAME_BYTES, + ..ComputeBudgetLimits::default() + }), + &FeatureSet::all_enabled() ); test!( &[ @@ -279,13 +316,28 @@ mod tests { loaded_accounts_bytes: NonZeroU32::new(data_size).unwrap(), ..ComputeBudgetLimits::default() }); + test!( + &[ + ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(data_size), + Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), + ], + expected_result, + &FeatureSet::default() + ); + let expected_result = Ok(ComputeBudgetLimits { + compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT + + MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT, + loaded_accounts_bytes: NonZeroU32::new(data_size).unwrap(), + ..ComputeBudgetLimits::default() + }); test!( &[ ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(data_size), Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), ], - expected_result + expected_result, + &FeatureSet::all_enabled() ); // Assert when set_loaded_accounts_data_size_limit presents, with greater than max value @@ -296,13 +348,28 @@ mod tests { loaded_accounts_bytes: MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, ..ComputeBudgetLimits::default() }); + test!( + &[ + ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(data_size), + Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), + ], + expected_result, + &FeatureSet::default() + ); + let expected_result = Ok(ComputeBudgetLimits { + compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT + + MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT, + loaded_accounts_bytes: MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, + ..ComputeBudgetLimits::default() + }); test!( &[ ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(data_size), Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), ], - expected_result + expected_result, + &FeatureSet::all_enabled() ); // Assert when set_loaded_accounts_data_size_limit is not presented @@ -352,19 +419,32 @@ mod tests { Hash::default(), )); - let result = process_compute_budget_instructions(SVMMessage::program_instructions_iter( - &transaction, - )); + for (feature_set, expected_result) in [ + ( + FeatureSet::default(), + Ok(ComputeBudgetLimits { + compute_unit_limit: 2 * DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, + ..ComputeBudgetLimits::default() + }), + ), + ( + FeatureSet::all_enabled(), + Ok(ComputeBudgetLimits { + compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT + + MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT, + ..ComputeBudgetLimits::default() + }), + ), + ] { + let result = process_compute_budget_instructions( + SVMMessage::program_instructions_iter(&transaction), + &feature_set, + ); - // assert process_instructions will be successful with default, - // and the default compute_unit_limit is 2 times default: one for bpf ix, one for - // builtin ix. - assert_eq!( - result, - Ok(ComputeBudgetLimits { - compute_unit_limit: 2 * DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, - ..ComputeBudgetLimits::default() - }) - ); + // assert process_instructions will be successful with default, + // and the default compute_unit_limit is 2 times default: one for bpf ix, one for + // builtin ix. + assert_eq!(result, expected_result); + } } } diff --git a/runtime-transaction/src/lib.rs b/runtime-transaction/src/lib.rs index f4d17e5d5daa7a..07731bcdaedac7 100644 --- a/runtime-transaction/src/lib.rs +++ b/runtime-transaction/src/lib.rs @@ -1,6 +1,7 @@ #![cfg_attr(feature = "frozen-abi", feature(min_specialization))] #![allow(clippy::arithmetic_side_effects)] +mod builtin_programs_filter; pub mod compute_budget_instruction_details; mod compute_budget_program_id_filter; pub mod instructions_processor; diff --git a/runtime-transaction/src/runtime_transaction.rs b/runtime-transaction/src/runtime_transaction.rs index c4fa24d99ce50f..0903eb62ad236a 100644 --- a/runtime-transaction/src/runtime_transaction.rs +++ b/runtime-transaction/src/runtime_transaction.rs @@ -86,7 +86,7 @@ impl SVMMessage for RuntimeTransaction { self.transaction.instructions_iter() } - fn program_instructions_iter(&self) -> impl Iterator { + fn program_instructions_iter(&self) -> impl Iterator + Clone { self.transaction.program_instructions_iter() } diff --git a/runtime-transaction/src/runtime_transaction/sdk_transactions.rs b/runtime-transaction/src/runtime_transaction/sdk_transactions.rs index 8790bf090974ec..a82e9c1217fff6 100644 --- a/runtime-transaction/src/runtime_transaction/sdk_transactions.rs +++ b/runtime-transaction/src/runtime_transaction/sdk_transactions.rs @@ -160,6 +160,7 @@ mod tests { }, solana_sdk::{ compute_budget::ComputeBudgetInstruction, + feature_set::FeatureSet, hash::Hash, instruction::Instruction, message::Message, @@ -329,15 +330,17 @@ mod tests { assert_eq!(0, signature_details.num_secp256k1_instruction_signatures()); assert_eq!(0, signature_details.num_ed25519_instruction_signatures()); - let compute_budget_limits = runtime_transaction_static - .compute_budget_instruction_details() - .sanitize_and_convert_to_compute_budget_limits() - .unwrap(); - assert_eq!(compute_unit_limit, compute_budget_limits.compute_unit_limit); - assert_eq!(compute_unit_price, compute_budget_limits.compute_unit_price); - assert_eq!( - loaded_accounts_bytes, - compute_budget_limits.loaded_accounts_bytes.get() - ); + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + let compute_budget_limits = runtime_transaction_static + .compute_budget_instruction_details() + .sanitize_and_convert_to_compute_budget_limits(&feature_set) + .unwrap(); + assert_eq!(compute_unit_limit, compute_budget_limits.compute_unit_limit); + assert_eq!(compute_unit_price, compute_budget_limits.compute_unit_price); + assert_eq!( + loaded_accounts_bytes, + compute_budget_limits.loaded_accounts_bytes.get() + ); + } } } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 68f735b3d011c4..7ff38ba1c2b2f3 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3421,8 +3421,11 @@ impl Bank { lamports_per_signature: u64, ) -> u64 { let fee_budget_limits = FeeBudgetLimits::from( - process_compute_budget_instructions(message.program_instructions_iter()) - .unwrap_or_default(), + process_compute_budget_instructions( + message.program_instructions_iter(), + &self.feature_set, + ) + .unwrap_or_default(), ); solana_fee::calculate_fee( message, diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 5e29644cf17d11..5f1f751865ebb6 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -10194,8 +10194,11 @@ fn calculate_test_fee( fee_structure: &FeeStructure, ) -> u64 { let fee_budget_limits = FeeBudgetLimits::from( - process_compute_budget_instructions(message.program_instructions_iter()) - .unwrap_or_default(), + process_compute_budget_instructions( + message.program_instructions_iter(), + &FeatureSet::default(), + ) + .unwrap_or_default(), ); solana_fee::calculate_fee( message, diff --git a/runtime/src/prioritization_fee_cache.rs b/runtime/src/prioritization_fee_cache.rs index 3e201655ae95a7..3c45cdd825e445 100644 --- a/runtime/src/prioritization_fee_cache.rs +++ b/runtime/src/prioritization_fee_cache.rs @@ -209,7 +209,7 @@ impl PrioritizationFeeCache { let compute_budget_limits = sanitized_transaction .compute_budget_instruction_details() - .sanitize_and_convert_to_compute_budget_limits(); + .sanitize_and_convert_to_compute_budget_limits(&bank.feature_set); let lock_result = validate_account_locks( sanitized_transaction.account_keys(), diff --git a/sdk/feature-set/src/lib.rs b/sdk/feature-set/src/lib.rs index 6725f02212b425..4b5602b6d6ec62 100644 --- a/sdk/feature-set/src/lib.rs +++ b/sdk/feature-set/src/lib.rs @@ -892,6 +892,10 @@ pub mod migrate_stake_program_to_core_bpf { solana_pubkey::declare_id!("6M4oQ6eXneVhtLoiAr4yRYQY43eVLjrKbiDZDJc892yk"); } +pub mod reserve_minimal_cus_for_builtin_instructions { + solana_pubkey::declare_id!("C9oAhLxDBm3ssWtJx1yBGzPY55r2rArHmN1pbQn6HogH"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: AHashMap = [ @@ -1110,6 +1114,7 @@ lazy_static! { (accounts_lt_hash::id(), "enables lattice-based accounts hash #3333"), (enable_secp256r1_precompile::id(), "Enable secp256r1 precompile SIMD-0075"), (migrate_stake_program_to_core_bpf::id(), "Migrate Stake program to Core BPF SIMD-0196 #3655"), + (reserve_minimal_cus_for_builtin_instructions::id(), "Reserve minimal CUs for builtin instructions SIMD-170 #2562"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/sdk/message/src/sanitized.rs b/sdk/message/src/sanitized.rs index e4429ac243cadd..97b9e40abaac93 100644 --- a/sdk/message/src/sanitized.rs +++ b/sdk/message/src/sanitized.rs @@ -177,7 +177,7 @@ impl SanitizedMessage { /// id. pub fn program_instructions_iter( &self, - ) -> impl Iterator { + ) -> impl Iterator + Clone { self.instructions().iter().map(move |ix| { ( self.account_keys() diff --git a/sdk/message/src/versions/sanitized.rs b/sdk/message/src/versions/sanitized.rs index bd95964716c7e9..6466bd65b09dbc 100644 --- a/sdk/message/src/versions/sanitized.rs +++ b/sdk/message/src/versions/sanitized.rs @@ -32,7 +32,7 @@ impl SanitizedVersionedMessage { /// id. pub fn program_instructions_iter( &self, - ) -> impl Iterator { + ) -> impl Iterator + Clone { self.message.instructions().iter().map(move |ix| { ( self.message diff --git a/svm-transaction/src/svm_message.rs b/svm-transaction/src/svm_message.rs index 3b2e6fd9a41d79..0e60567729b296 100644 --- a/svm-transaction/src/svm_message.rs +++ b/svm-transaction/src/svm_message.rs @@ -41,7 +41,7 @@ pub trait SVMMessage: Debug { /// Return an iterator over the instructions in the message, paired with /// the pubkey of the program. - fn program_instructions_iter(&self) -> impl Iterator; + fn program_instructions_iter(&self) -> impl Iterator + Clone; /// Return the account keys. fn account_keys(&self) -> AccountKeys; diff --git a/svm-transaction/src/svm_message/sanitized_message.rs b/svm-transaction/src/svm_message/sanitized_message.rs index c225cb975bd2b1..75b98c3a9ca8e6 100644 --- a/svm-transaction/src/svm_message/sanitized_message.rs +++ b/svm-transaction/src/svm_message/sanitized_message.rs @@ -32,7 +32,7 @@ impl SVMMessage for SanitizedMessage { .map(SVMInstruction::from) } - fn program_instructions_iter(&self) -> impl Iterator { + fn program_instructions_iter(&self) -> impl Iterator + Clone { SanitizedMessage::program_instructions_iter(self) .map(|(pubkey, ix)| (pubkey, SVMInstruction::from(ix))) } diff --git a/svm-transaction/src/svm_message/sanitized_transaction.rs b/svm-transaction/src/svm_message/sanitized_transaction.rs index 8486981140f20a..4bbea93cb0d215 100644 --- a/svm-transaction/src/svm_message/sanitized_transaction.rs +++ b/svm-transaction/src/svm_message/sanitized_transaction.rs @@ -30,7 +30,7 @@ impl SVMMessage for SanitizedTransaction { SVMMessage::instructions_iter(SanitizedTransaction::message(self)) } - fn program_instructions_iter(&self) -> impl Iterator { + fn program_instructions_iter(&self) -> impl Iterator + Clone { SVMMessage::program_instructions_iter(SanitizedTransaction::message(self)) } diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index bff7560a96d7b0..0327a434fcbab3 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -569,6 +569,7 @@ impl TransactionBatchProcessor { ) -> transaction::Result { let compute_budget_limits = process_compute_budget_instructions( message.program_instructions_iter(), + &account_loader.feature_set, ) .inspect_err(|_err| { error_counters.invalid_compute_budget += 1; @@ -2136,9 +2137,11 @@ mod tests { Some(&Pubkey::new_unique()), &Hash::new_unique(), )); - let compute_budget_limits = - process_compute_budget_instructions(SVMMessage::program_instructions_iter(&message)) - .unwrap(); + let compute_budget_limits = process_compute_budget_instructions( + SVMMessage::program_instructions_iter(&message), + &FeatureSet::default(), + ) + .unwrap(); let fee_payer_address = message.fee_payer(); let current_epoch = 42; let rent_collector = RentCollector { @@ -2222,9 +2225,11 @@ mod tests { Some(&Pubkey::new_unique()), &Hash::new_unique(), )); - let compute_budget_limits = - process_compute_budget_instructions(SVMMessage::program_instructions_iter(&message)) - .unwrap(); + let compute_budget_limits = process_compute_budget_instructions( + SVMMessage::program_instructions_iter(&message), + &FeatureSet::default(), + ) + .unwrap(); let fee_payer_address = message.fee_payer(); let mut rent_collector = RentCollector::default(); rent_collector.rent.lamports_per_byte_year = 1_000_000; @@ -2473,9 +2478,11 @@ mod tests { Some(&Pubkey::new_unique()), &last_blockhash, )); - let compute_budget_limits = - process_compute_budget_instructions(SVMMessage::program_instructions_iter(&message)) - .unwrap(); + let compute_budget_limits = process_compute_budget_instructions( + SVMMessage::program_instructions_iter(&message), + &FeatureSet::default(), + ) + .unwrap(); let fee_payer_address = message.fee_payer(); let min_balance = Rent::default().minimum_balance(nonce::State::size()); let transaction_fee = lamports_per_signature; diff --git a/transaction-view/src/resolved_transaction_view.rs b/transaction-view/src/resolved_transaction_view.rs index 3dbcb1273fd601..98d395bc9dc749 100644 --- a/transaction-view/src/resolved_transaction_view.rs +++ b/transaction-view/src/resolved_transaction_view.rs @@ -225,7 +225,7 @@ impl SVMMessage for ResolvedTransactionView { &solana_sdk::pubkey::Pubkey, solana_svm_transaction::instruction::SVMInstruction, ), - > { + > + Clone { self.view.program_instructions_iter() } diff --git a/transaction-view/src/transaction_view.rs b/transaction-view/src/transaction_view.rs index 51e4a7c65523a6..c186902c053f89 100644 --- a/transaction-view/src/transaction_view.rs +++ b/transaction-view/src/transaction_view.rs @@ -169,7 +169,9 @@ impl TransactionView { // Implementation that relies on sanitization checks having been run. impl TransactionView { /// Return an iterator over the instructions paired with their program ids. - pub fn program_instructions_iter(&self) -> impl Iterator { + pub fn program_instructions_iter( + &self, + ) -> impl Iterator + Clone { self.instructions_iter().map(|ix| { let program_id_index = usize::from(ix.program_id_index); let program_id = &self.static_account_keys()[program_id_index];