From 86c88d7ff6ce69af180ae8abf898af74c60214cc Mon Sep 17 00:00:00 2001 From: Tao Zhu <82401714+taozhu-chicago@users.noreply.github.com> Date: Fri, 15 Dec 2023 15:01:02 -0600 Subject: [PATCH] Remove feature set from compute budget processor (#34472) remove feature_set from compute_budget_processor --- cost-model/src/cost_model.rs | 6 ++--- program-runtime/src/compute_budget.rs | 8 ++---- .../src/compute_budget_processor.rs | 18 +++---------- programs/sbf/tests/programs.rs | 21 +++++----------- runtime/src/accounts/mod.rs | 25 ++++++------------- runtime/src/bank.rs | 10 +++----- runtime/src/bank/tests.rs | 7 +++--- runtime/src/transaction_priority_details.rs | 6 +---- 8 files changed, 29 insertions(+), 72 deletions(-) diff --git a/cost-model/src/cost_model.rs b/cost-model/src/cost_model.rs index 28a1a01e7..ba01ed9fe 100644 --- a/cost-model/src/cost_model.rs +++ b/cost-model/src/cost_model.rs @@ -115,10 +115,8 @@ impl CostModel { // 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. - match process_compute_budget_instructions( - transaction.message().program_instructions_iter(), - feature_set, - ) { + match process_compute_budget_instructions(transaction.message().program_instructions_iter()) + { Ok(compute_budget_limits) => { // if tx contained user-space instructions and a more accurate estimate available correct it, // where "user-space instructions" must be specifically checked by diff --git a/program-runtime/src/compute_budget.rs b/program-runtime/src/compute_budget.rs index a568162c1..0657df5c8 100644 --- a/program-runtime/src/compute_budget.rs +++ b/program-runtime/src/compute_budget.rs @@ -1,9 +1,6 @@ use { crate::compute_budget_processor::{self, process_compute_budget_instructions}, - solana_sdk::{ - feature_set::FeatureSet, instruction::CompiledInstruction, pubkey::Pubkey, - transaction::Result, - }, + solana_sdk::{instruction::CompiledInstruction, pubkey::Pubkey, transaction::Result}, }; #[cfg(RUSTC_WITH_SPECIALIZATION)] @@ -183,9 +180,8 @@ impl ComputeBudget { pub fn try_from_instructions<'a>( instructions: impl Iterator, - feature_set: &FeatureSet, ) -> Result { - let compute_budget_limits = process_compute_budget_instructions(instructions, feature_set)?; + let compute_budget_limits = process_compute_budget_instructions(instructions)?; Ok(ComputeBudget { compute_unit_limit: u64::from(compute_budget_limits.compute_unit_limit), heap_size: compute_budget_limits.updated_heap_bytes, diff --git a/program-runtime/src/compute_budget_processor.rs b/program-runtime/src/compute_budget_processor.rs index 5d66da158..f87bbcd6c 100644 --- a/program-runtime/src/compute_budget_processor.rs +++ b/program-runtime/src/compute_budget_processor.rs @@ -1,4 +1,3 @@ -//! Process compute_budget instructions to extract and sanitize limits. use { crate::{ compute_budget::DEFAULT_HEAP_COST, @@ -8,7 +7,6 @@ use { borsh1::try_from_slice_unchecked, compute_budget::{self, ComputeBudgetInstruction}, entrypoint::HEAP_LENGTH as MIN_HEAP_FRAME_BYTES, - feature_set::FeatureSet, fee::FeeBudgetLimits, instruction::{CompiledInstruction, InstructionError}, pubkey::Pubkey, @@ -69,7 +67,6 @@ impl From for FeeBudgetLimits { /// are retrieved and returned, pub fn process_compute_budget_instructions<'a>( instructions: impl Iterator, - _feature_set: &FeatureSet, ) -> Result { let mut num_non_compute_budget_instructions: u32 = 0; let mut updated_compute_unit_limit = None; @@ -178,11 +175,8 @@ mod tests { Message::new($instructions, Some(&payer_keypair.pubkey())), Hash::default(), )); - let feature_set = FeatureSet::default(); - let result = process_compute_budget_instructions( - tx.message().program_instructions_iter(), - &feature_set, - ); + let result = + process_compute_budget_instructions(tx.message().program_instructions_iter()); assert_eq!($expected_result, result); }; } @@ -489,12 +483,8 @@ mod tests { Hash::default(), )); - let feature_set = FeatureSet::default(); - - let result = process_compute_budget_instructions( - transaction.message().program_instructions_iter(), - &feature_set, - ); + let result = + process_compute_budget_instructions(transaction.message().program_instructions_iter()); // assert process_instructions will be successful with default, // and the default compute_unit_limit is 2 times default: one for bpf ix, one for diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index 12b8f1604..6285cee35 100644 --- a/programs/sbf/tests/programs.rs +++ b/programs/sbf/tests/programs.rs @@ -3868,18 +3868,13 @@ fn test_program_fees() { Some(&mint_keypair.pubkey()), ); - let feature_set = FeatureSet::all_enabled(); - let sanitized_message = SanitizedMessage::try_from(message.clone()).unwrap(); let expected_normal_fee = fee_structure.calculate_fee( &sanitized_message, congestion_multiplier, - &process_compute_budget_instructions( - sanitized_message.program_instructions_iter(), - &feature_set, - ) - .unwrap_or_default() - .into(), + &process_compute_budget_instructions(sanitized_message.program_instructions_iter()) + .unwrap_or_default() + .into(), true, false, ); @@ -3898,16 +3893,12 @@ fn test_program_fees() { Some(&mint_keypair.pubkey()), ); let sanitized_message = SanitizedMessage::try_from(message.clone()).unwrap(); - let feature_set = FeatureSet::all_enabled(); let expected_prioritized_fee = fee_structure.calculate_fee( &sanitized_message, congestion_multiplier, - &process_compute_budget_instructions( - sanitized_message.program_instructions_iter(), - &feature_set, - ) - .unwrap_or_default() - .into(), + &process_compute_budget_instructions(sanitized_message.program_instructions_iter()) + .unwrap_or_default() + .into(), true, false, ); diff --git a/runtime/src/accounts/mod.rs b/runtime/src/accounts/mod.rs index 1f9db1618..30397d7cc 100644 --- a/runtime/src/accounts/mod.rs +++ b/runtime/src/accounts/mod.rs @@ -80,7 +80,6 @@ pub(super) fn load_accounts( lamports_per_signature, &process_compute_budget_instructions( tx.message().program_instructions_iter(), - feature_set, ) .unwrap_or_default() .into(), @@ -170,7 +169,7 @@ fn load_transaction_accounts( feature_set.is_active(&solana_sdk::feature_set::set_exempt_rent_epoch_max::id()); let requested_loaded_accounts_data_size_limit = - get_requested_loaded_accounts_data_size_limit(tx, feature_set)?; + get_requested_loaded_accounts_data_size_limit(tx)?; let mut accumulated_accounts_data_size: usize = 0; let instruction_accounts = message @@ -423,10 +422,9 @@ fn load_transaction_accounts( /// Note, requesting zero bytes will result transaction error fn get_requested_loaded_accounts_data_size_limit( tx: &SanitizedTransaction, - feature_set: &FeatureSet, ) -> Result> { let compute_budget_limits = - process_compute_budget_instructions(tx.message().program_instructions_iter(), feature_set) + process_compute_budget_instructions(tx.message().program_instructions_iter()) .unwrap_or_default(); // sanitize against setting size limit to zero NonZeroUsize::new( @@ -732,13 +730,11 @@ mod tests { instructions, ); - let feature_set = FeatureSet::all_enabled(); - let message = SanitizedMessage::try_from(tx.message().clone()).unwrap(); let fee = FeeStructure::default().calculate_fee( &message, lamports_per_signature, - &process_compute_budget_instructions(message.program_instructions_iter(), &feature_set) + &process_compute_budget_instructions(message.program_instructions_iter()) .unwrap_or_default() .into(), true, @@ -1502,7 +1498,6 @@ mod tests { // an prrivate helper function fn test( instructions: &[solana_sdk::instruction::Instruction], - feature_set: &FeatureSet, expected_result: &Result>, ) { let payer_keypair = Keypair::new(); @@ -1513,7 +1508,7 @@ mod tests { )); assert_eq!( *expected_result, - get_requested_loaded_accounts_data_size_limit(&tx, feature_set) + get_requested_loaded_accounts_data_size_limit(&tx) ); } @@ -1544,15 +1539,13 @@ mod tests { Ok(Some(NonZeroUsize::new(99).unwrap())); let result_invalid_limit = Err(TransactionError::InvalidLoadedAccountsDataSizeLimit); - let feature_set = FeatureSet::default(); - // the results should be: // if tx doesn't set limit, then default limit (64MiB) // if tx sets limit, then requested limit // if tx sets limit to zero, then TransactionError::InvalidLoadedAccountsDataSizeLimit - test(tx_not_set_limit, &feature_set, &result_default_limit); - test(tx_set_limit_99, &feature_set, &result_requested_limit); - test(tx_set_limit_0, &feature_set, &result_invalid_limit); + test(tx_not_set_limit, &result_default_limit); + test(tx_set_limit_99, &result_requested_limit); + test(tx_set_limit_0, &result_invalid_limit); } #[test] @@ -1583,13 +1576,11 @@ mod tests { Hash::default(), ); - let feature_set = FeatureSet::all_enabled(); - let message = SanitizedMessage::try_from(tx.message().clone()).unwrap(); let fee = FeeStructure::default().calculate_fee( &message, lamports_per_signature, - &process_compute_budget_instructions(message.program_instructions_iter(), &feature_set) + &process_compute_budget_instructions(message.program_instructions_iter()) .unwrap_or_default() .into(), true, diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 384da6d61..77cb74d32 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4067,12 +4067,9 @@ impl Bank { self.fee_structure.calculate_fee( message, lamports_per_signature, - &process_compute_budget_instructions( - message.program_instructions_iter(), - &self.feature_set, - ) - .unwrap_or_default() - .into(), + &process_compute_budget_instructions(message.program_instructions_iter()) + .unwrap_or_default() + .into(), self.feature_set .is_active(&remove_congestion_multiplier_from_fee_calculation::id()), self.feature_set @@ -5214,7 +5211,6 @@ impl Bank { Measure::start("compute_budget_process_transaction_time"); let maybe_compute_budget = ComputeBudget::try_from_instructions( tx.message().program_instructions_iter(), - &self.feature_set, ); compute_budget_process_transaction_time.stop(); saturating_add_assign!( diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 8bdd35281..318f5416d 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -10012,10 +10012,9 @@ fn calculate_test_fee( ); } - let budget_limits = - process_compute_budget_instructions(message.program_instructions_iter(), &feature_set) - .unwrap_or_default() - .into(); + let budget_limits = process_compute_budget_instructions(message.program_instructions_iter()) + .unwrap_or_default() + .into(); fee_structure.calculate_fee( message, diff --git a/runtime/src/transaction_priority_details.rs b/runtime/src/transaction_priority_details.rs index c5a3d6396..284acb791 100644 --- a/runtime/src/transaction_priority_details.rs +++ b/runtime/src/transaction_priority_details.rs @@ -1,7 +1,6 @@ use { solana_program_runtime::compute_budget_processor::process_compute_budget_instructions, solana_sdk::{ - feature_set::FeatureSet, instruction::CompiledInstruction, pubkey::Pubkey, transaction::{SanitizedTransaction, SanitizedVersionedTransaction}, @@ -24,10 +23,7 @@ pub trait GetTransactionPriorityDetails { instructions: impl Iterator, _round_compute_unit_price_enabled: bool, ) -> Option { - let feature_set = FeatureSet::default(); - - let compute_budget_limits = - process_compute_budget_instructions(instructions, &feature_set).ok()?; + let compute_budget_limits = process_compute_budget_instructions(instructions).ok()?; Some(TransactionPriorityDetails { priority: compute_budget_limits.compute_unit_price, compute_unit_limit: u64::from(compute_budget_limits.compute_unit_limit),