diff --git a/core/src/transaction_priority_details.rs b/core/src/transaction_priority_details.rs index 7f816cad2..6f406d0ed 100644 --- a/core/src/transaction_priority_details.rs +++ b/core/src/transaction_priority_details.rs @@ -24,7 +24,6 @@ pub trait GetTransactionPriorityDetails { .process_instructions( instructions, true, // use default units per instruction - true, // don't reject txs that use set compute unit price ix ) .ok()?; Some(TransactionPriorityDetails { diff --git a/program-runtime/src/compute_budget.rs b/program-runtime/src/compute_budget.rs index 31432f387..60e21b2e9 100644 --- a/program-runtime/src/compute_budget.rs +++ b/program-runtime/src/compute_budget.rs @@ -127,7 +127,6 @@ impl ComputeBudget { &mut self, instructions: impl Iterator, default_units_per_instruction: bool, - support_set_compute_unit_price_ix: bool, ) -> Result { let mut num_non_compute_budget_instructions: usize = 0; let mut updated_compute_unit_limit = None; @@ -136,70 +135,47 @@ impl ComputeBudget { for (i, (program_id, instruction)) in instructions.enumerate() { if compute_budget::check_id(program_id) { - if support_set_compute_unit_price_ix { - let invalid_instruction_data_error = TransactionError::InstructionError( - i as u8, - InstructionError::InvalidInstructionData, - ); - let duplicate_instruction_error = - TransactionError::DuplicateInstruction(i as u8); + let invalid_instruction_data_error = TransactionError::InstructionError( + i as u8, + InstructionError::InvalidInstructionData, + ); + let duplicate_instruction_error = TransactionError::DuplicateInstruction(i as u8); - match try_from_slice_unchecked(&instruction.data) { - Ok(ComputeBudgetInstruction::RequestUnitsDeprecated { - units: compute_unit_limit, - additional_fee, - }) => { - if updated_compute_unit_limit.is_some() { - return Err(duplicate_instruction_error); - } - if prioritization_fee.is_some() { - return Err(duplicate_instruction_error); - } - updated_compute_unit_limit = Some(compute_unit_limit); - prioritization_fee = - Some(PrioritizationFeeType::Deprecated(additional_fee as u64)); + match try_from_slice_unchecked(&instruction.data) { + Ok(ComputeBudgetInstruction::RequestUnitsDeprecated { + units: compute_unit_limit, + additional_fee, + }) => { + if updated_compute_unit_limit.is_some() { + return Err(duplicate_instruction_error); } - Ok(ComputeBudgetInstruction::RequestHeapFrame(bytes)) => { - if requested_heap_size.is_some() { - return Err(duplicate_instruction_error); - } - requested_heap_size = Some((bytes, i as u8)); + if prioritization_fee.is_some() { + return Err(duplicate_instruction_error); } - Ok(ComputeBudgetInstruction::SetComputeUnitLimit(compute_unit_limit)) => { - if updated_compute_unit_limit.is_some() { - return Err(duplicate_instruction_error); - } - updated_compute_unit_limit = Some(compute_unit_limit); - } - Ok(ComputeBudgetInstruction::SetComputeUnitPrice(micro_lamports)) => { - if prioritization_fee.is_some() { - return Err(duplicate_instruction_error); - } - prioritization_fee = - Some(PrioritizationFeeType::ComputeUnitPrice(micro_lamports)); - } - _ => return Err(invalid_instruction_data_error), + updated_compute_unit_limit = Some(compute_unit_limit); + prioritization_fee = + Some(PrioritizationFeeType::Deprecated(additional_fee as u64)); } - } else if i < 3 { - match try_from_slice_unchecked(&instruction.data) { - Ok(ComputeBudgetInstruction::RequestUnitsDeprecated { - units: compute_unit_limit, - additional_fee, - }) => { - updated_compute_unit_limit = Some(compute_unit_limit); - prioritization_fee = - Some(PrioritizationFeeType::Deprecated(additional_fee as u64)); - } - Ok(ComputeBudgetInstruction::RequestHeapFrame(bytes)) => { - requested_heap_size = Some((bytes, 0)); - } - _ => { - return Err(TransactionError::InstructionError( - 0, - InstructionError::InvalidInstructionData, - )) + Ok(ComputeBudgetInstruction::RequestHeapFrame(bytes)) => { + if requested_heap_size.is_some() { + return Err(duplicate_instruction_error); } + requested_heap_size = Some((bytes, i as u8)); } + Ok(ComputeBudgetInstruction::SetComputeUnitLimit(compute_unit_limit)) => { + if updated_compute_unit_limit.is_some() { + return Err(duplicate_instruction_error); + } + updated_compute_unit_limit = Some(compute_unit_limit); + } + Ok(ComputeBudgetInstruction::SetComputeUnitPrice(micro_lamports)) => { + if prioritization_fee.is_some() { + return Err(duplicate_instruction_error); + } + prioritization_fee = + Some(PrioritizationFeeType::ComputeUnitPrice(micro_lamports)); + } + _ => return Err(invalid_instruction_data_error), } } else { // only include non-request instructions in default max calc @@ -255,19 +231,8 @@ mod tests { }, }; - fn request_units_deprecated(units: u32, additional_fee: u32) -> Instruction { - Instruction::new_with_borsh( - compute_budget::id(), - &ComputeBudgetInstruction::RequestUnitsDeprecated { - units, - additional_fee, - }, - vec![], - ) - } - macro_rules! test { - ( $instructions: expr, $expected_result: expr, $expected_budget: expr, $type_change: expr ) => { + ( $instructions: expr, $expected_result: expr, $expected_budget: expr ) => { let payer_keypair = Keypair::new(); let tx = SanitizedTransaction::from_transaction_for_tests(Transaction::new( &[&payer_keypair], @@ -275,16 +240,13 @@ mod tests { Hash::default(), )); let mut compute_budget = ComputeBudget::default(); - let result = compute_budget.process_instructions( - tx.message().program_instructions_iter(), - true, - $type_change, - ); + let result = + compute_budget.process_instructions(tx.message().program_instructions_iter(), true); assert_eq!($expected_result, result); assert_eq!(compute_budget, $expected_budget); }; ( $instructions: expr, $expected_result: expr, $expected_budget: expr) => { - test!($instructions, $expected_result, $expected_budget, true); + test!($instructions, $expected_result, $expected_budget); }; } @@ -346,35 +308,6 @@ mod tests { } ); - test!( - &[ - Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), - Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), - Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), - ComputeBudgetInstruction::set_compute_unit_limit(1), // ignored - ], - Ok(PrioritizationFeeDetails::default()), - ComputeBudget { - compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64 * 3, - ..ComputeBudget::default() - }, - false - ); - - // Prioritization fee - test!( - &[request_units_deprecated(1, 42)], - Ok(PrioritizationFeeDetails::new( - PrioritizationFeeType::Deprecated(42), - 1, - )), - ComputeBudget { - compute_unit_limit: 1, - ..ComputeBudget::default() - }, - false - ); - test!( &[ ComputeBudgetInstruction::set_compute_unit_limit(1), @@ -390,19 +323,6 @@ mod tests { } ); - test!( - &[request_units_deprecated(1, u32::MAX)], - Ok(PrioritizationFeeDetails::new( - PrioritizationFeeType::Deprecated(u32::MAX as u64), - 1 - )), - ComputeBudget { - compute_unit_limit: 1, - ..ComputeBudget::default() - }, - false - ); - // HeapFrame test!( &[], @@ -520,21 +440,6 @@ mod tests { } ); - test!( - &[ - Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), - ComputeBudgetInstruction::request_heap_frame(MAX_HEAP_FRAME_BYTES), - ComputeBudgetInstruction::set_compute_unit_limit(MAX_COMPUTE_UNIT_LIMIT), - ComputeBudgetInstruction::set_compute_unit_price(u64::MAX), - ], - Err(TransactionError::InstructionError( - 0, - InstructionError::InvalidInstructionData, - )), - ComputeBudget::default(), - false - ); - test!( &[ Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), @@ -553,24 +458,6 @@ mod tests { } ); - test!( - &[ - Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), - request_units_deprecated(MAX_COMPUTE_UNIT_LIMIT, u32::MAX), - ComputeBudgetInstruction::request_heap_frame(MIN_HEAP_FRAME_BYTES as u32), - ], - Ok(PrioritizationFeeDetails::new( - PrioritizationFeeType::Deprecated(u32::MAX as u64), - MAX_COMPUTE_UNIT_LIMIT as u64, - )), - ComputeBudget { - compute_unit_limit: MAX_COMPUTE_UNIT_LIMIT as u64, - heap_size: Some(MIN_HEAP_FRAME_BYTES as usize), - ..ComputeBudget::default() - }, - false - ); - // Duplicates test!( &[ diff --git a/programs/bpf/tests/programs.rs b/programs/bpf/tests/programs.rs index 4ea02a2c5..8016050e8 100644 --- a/programs/bpf/tests/programs.rs +++ b/programs/bpf/tests/programs.rs @@ -3798,7 +3798,6 @@ fn test_program_fees() { congestion_multiplier, &fee_structure, true, - true, ); bank_client .send_and_confirm_message(&[&mint_keypair], message) @@ -3820,7 +3819,6 @@ fn test_program_fees() { congestion_multiplier, &fee_structure, true, - true, ); assert!(expected_normal_fee < expected_prioritized_fee); diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index de8c92e60..ae08342c0 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -34,9 +34,7 @@ use { account_utils::StateMut, bpf_loader_upgradeable::{self, UpgradeableLoaderState}, clock::{BankId, Slot, INITIAL_RENT_EPOCH}, - feature_set::{ - self, add_set_compute_unit_price_ix, use_default_units_in_fee_calculation, FeatureSet, - }, + feature_set::{self, use_default_units_in_fee_calculation, FeatureSet}, fee::FeeStructure, genesis_config::ClusterType, hash::Hash, @@ -554,7 +552,6 @@ impl Accounts { tx.message(), lamports_per_signature, fee_structure, - feature_set.is_active(&add_set_compute_unit_price_ix::id()), feature_set.is_active(&use_default_units_in_fee_calculation::id()), ) } else { @@ -1674,7 +1671,6 @@ mod tests { lamports_per_signature, &FeeStructure::default(), true, - true, ); assert_eq!(fee, lamports_per_signature); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 00d3a873b..32ece32f8 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -110,8 +110,7 @@ use { epoch_schedule::EpochSchedule, feature, feature_set::{ - self, add_set_compute_unit_price_ix, disable_fee_calculator, - enable_early_verification_of_account_modifications, + self, disable_fee_calculator, enable_early_verification_of_account_modifications, use_default_units_in_fee_calculation, FeatureSet, }, fee::FeeStructure, @@ -3683,8 +3682,6 @@ impl Bank { message, lamports_per_signature, &self.fee_structure, - self.feature_set - .is_active(&add_set_compute_unit_price_ix::id()), self.feature_set .is_active(&use_default_units_in_fee_calculation::id()), )) @@ -3726,8 +3723,6 @@ impl Bank { message, lamports_per_signature, &self.fee_structure, - self.feature_set - .is_active(&add_set_compute_unit_price_ix::id()), self.feature_set .is_active(&use_default_units_in_fee_calculation::id()), ) @@ -4536,40 +4531,30 @@ impl Bank { .map(|(accs, tx)| match accs { (Err(e), _nonce) => TransactionExecutionResult::NotExecuted(e.clone()), (Ok(loaded_transaction), nonce) => { - let mut feature_set_clone_time = Measure::start("feature_set_clone"); - let feature_set = self.feature_set.clone(); - feature_set_clone_time.stop(); - saturating_add_assign!( - timings.execute_accessories.feature_set_clone_us, - feature_set_clone_time.as_us() - ); + let compute_budget = if let Some(compute_budget) = + self.runtime_config.compute_budget + { + compute_budget + } else { + let mut compute_budget = + ComputeBudget::new(compute_budget::MAX_COMPUTE_UNIT_LIMIT as u64); - let compute_budget = - if let Some(compute_budget) = self.runtime_config.compute_budget { - compute_budget - } else { - let mut compute_budget = - ComputeBudget::new(compute_budget::MAX_COMPUTE_UNIT_LIMIT as u64); - - let mut compute_budget_process_transaction_time = - Measure::start("compute_budget_process_transaction_time"); - let process_transaction_result = compute_budget.process_instructions( - tx.message().program_instructions_iter(), - true, - feature_set.is_active(&add_set_compute_unit_price_ix::id()), - ); - compute_budget_process_transaction_time.stop(); - saturating_add_assign!( - timings - .execute_accessories - .compute_budget_process_transaction_us, - compute_budget_process_transaction_time.as_us() - ); - if let Err(err) = process_transaction_result { - return TransactionExecutionResult::NotExecuted(err); - } - compute_budget - }; + let mut compute_budget_process_transaction_time = + Measure::start("compute_budget_process_transaction_time"); + let process_transaction_result = compute_budget + .process_instructions(tx.message().program_instructions_iter(), true); + compute_budget_process_transaction_time.stop(); + saturating_add_assign!( + timings + .execute_accessories + .compute_budget_process_transaction_us, + compute_budget_process_transaction_time.as_us() + ); + if let Err(err) = process_transaction_result { + return TransactionExecutionResult::NotExecuted(err); + } + compute_budget + }; self.execute_loaded_transaction( tx, @@ -4841,7 +4826,6 @@ impl Bank { message: &SanitizedMessage, lamports_per_signature: u64, fee_structure: &FeeStructure, - support_set_compute_unit_price_ix: bool, use_default_units_per_instruction: bool, ) -> u64 { // Fee based on compute units and signatures @@ -4858,7 +4842,6 @@ impl Bank { .process_instructions( message.program_instructions_iter(), use_default_units_per_instruction, - support_set_compute_unit_price_ix, ) .unwrap_or_default(); let prioritization_fee = prioritization_fee_details.get_fee(); @@ -4922,8 +4905,6 @@ impl Bank { tx.message(), lamports_per_signature, &self.fee_structure, - self.feature_set - .is_active(&add_set_compute_unit_price_ix::id()), self.feature_set .is_active(&use_default_units_in_fee_calculation::id()), ); @@ -10893,7 +10874,6 @@ pub(crate) mod tests { .lamports_per_signature, &FeeStructure::default(), true, - true, ); let (expected_fee_collected, expected_fee_burned) = @@ -11075,7 +11055,6 @@ pub(crate) mod tests { cheap_lamports_per_signature, &FeeStructure::default(), true, - true, ); assert_eq!( bank.get_balance(&mint_keypair.pubkey()), @@ -11093,7 +11072,6 @@ pub(crate) mod tests { expensive_lamports_per_signature, &FeeStructure::default(), true, - true, ); assert_eq!( bank.get_balance(&mint_keypair.pubkey()), @@ -11209,7 +11187,6 @@ pub(crate) mod tests { .lamports_per_signature, &FeeStructure::default(), true, - true, ) * 2 ) .0 @@ -17809,7 +17786,6 @@ pub(crate) mod tests { ..FeeStructure::default() }, true, - true, ), 0 ); @@ -17824,7 +17800,6 @@ pub(crate) mod tests { ..FeeStructure::default() }, true, - true, ), 1 ); @@ -17844,7 +17819,6 @@ pub(crate) mod tests { ..FeeStructure::default() }, true, - true, ), 4 ); @@ -17864,7 +17838,7 @@ pub(crate) mod tests { let message = SanitizedMessage::try_from(Message::new(&[], Some(&Pubkey::new_unique()))).unwrap(); assert_eq!( - Bank::calculate_fee(&message, 1, &fee_structure, true, true,), + Bank::calculate_fee(&message, 1, &fee_structure, true), max_fee + lamports_per_signature ); @@ -17876,7 +17850,7 @@ pub(crate) mod tests { SanitizedMessage::try_from(Message::new(&[ix0, ix1], Some(&Pubkey::new_unique()))) .unwrap(); assert_eq!( - Bank::calculate_fee(&message, 1, &fee_structure, true, true,), + Bank::calculate_fee(&message, 1, &fee_structure, true), max_fee + 3 * lamports_per_signature ); @@ -17909,7 +17883,7 @@ pub(crate) mod tests { Some(&Pubkey::new_unique()), )) .unwrap(); - let fee = Bank::calculate_fee(&message, 1, &fee_structure, true, true); + let fee = Bank::calculate_fee(&message, 1, &fee_structure, true); assert_eq!( fee, lamports_per_signature + prioritization_fee_details.get_fee() @@ -17947,10 +17921,7 @@ pub(crate) mod tests { Some(&key0), )) .unwrap(); - assert_eq!( - Bank::calculate_fee(&message, 1, &fee_structure, true, true,), - 2 - ); + assert_eq!(Bank::calculate_fee(&message, 1, &fee_structure, true), 2); secp_instruction1.data = vec![0]; secp_instruction2.data = vec![10]; @@ -17959,10 +17930,7 @@ pub(crate) mod tests { Some(&key0), )) .unwrap(); - assert_eq!( - Bank::calculate_fee(&message, 1, &fee_structure, true, true,), - 11 - ); + assert_eq!(Bank::calculate_fee(&message, 1, &fee_structure, true), 11); } #[test]