diff --git a/cost-model/src/cost_model.rs b/cost-model/src/cost_model.rs index 1232e5c3c..752d0f78f 100644 --- a/cost-model/src/cost_model.rs +++ b/cost-model/src/cost_model.rs @@ -588,7 +588,7 @@ mod tests { } #[test] - fn test_cost_model_calculate_cost_enabled_feature_with_limit() { + fn test_cost_model_calculate_cost_with_limit() { let (mint_keypair, start_hash) = test_setup(); let to_keypair = Keypair::new(); let data_limit = 32 * 1024u32; @@ -626,39 +626,6 @@ mod tests { ); } - #[test] - fn test_cost_model_calculate_cost_disabled_feature_with_limit() { - let (mint_keypair, start_hash) = test_setup(); - let to_keypair = Keypair::new(); - let data_limit = 32 * 1024u32; - let tx = - SanitizedTransaction::from_transaction_for_tests(Transaction::new_signed_with_payer( - &[ - system_instruction::transfer(&mint_keypair.pubkey(), &to_keypair.pubkey(), 2), - ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(data_limit), - ], - Some(&mint_keypair.pubkey()), - &[&mint_keypair], - start_hash, - )); - - let feature_set = FeatureSet::default(); - assert!(!feature_set.is_active(&include_loaded_accounts_data_size_in_fee_calculation::id())); - let expected_account_cost = WRITE_LOCK_UNITS * 2; - // with features all disabled, builtins and loaded account size don't cost CU - let expected_execution_cost = 0; - let expected_loaded_accounts_data_size_cost = 0; - - 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.builtins_execution_cost()); - assert_eq!(2, tx_cost.writable_accounts().len()); - assert_eq!( - expected_loaded_accounts_data_size_cost, - tx_cost.loaded_accounts_data_size_cost() - ); - } - #[allow(clippy::field_reassign_with_default)] #[test] fn test_calculate_loaded_accounts_data_size_cost() { diff --git a/program-runtime/src/compute_budget_processor.rs b/program-runtime/src/compute_budget_processor.rs index 5bb469513..5d66da158 100644 --- a/program-runtime/src/compute_budget_processor.rs +++ b/program-runtime/src/compute_budget_processor.rs @@ -8,7 +8,7 @@ use { borsh1::try_from_slice_unchecked, compute_budget::{self, ComputeBudgetInstruction}, entrypoint::HEAP_LENGTH as MIN_HEAP_FRAME_BYTES, - feature_set::{add_set_tx_loaded_accounts_data_size_instruction, FeatureSet}, + feature_set::FeatureSet, fee::FeeBudgetLimits, instruction::{CompiledInstruction, InstructionError}, pubkey::Pubkey, @@ -69,11 +69,8 @@ impl From for FeeBudgetLimits { /// are retrieved and returned, pub fn process_compute_budget_instructions<'a>( instructions: impl Iterator, - feature_set: &FeatureSet, + _feature_set: &FeatureSet, ) -> Result { - let support_set_loaded_accounts_data_size_limit_ix = - feature_set.is_active(&add_set_tx_loaded_accounts_data_size_instruction::id()); - let mut num_non_compute_budget_instructions: u32 = 0; let mut updated_compute_unit_limit = None; let mut updated_compute_unit_price = None; @@ -111,9 +108,7 @@ pub fn process_compute_budget_instructions<'a>( } updated_compute_unit_price = Some(micro_lamports); } - Ok(ComputeBudgetInstruction::SetLoadedAccountsDataSizeLimit(bytes)) - if support_set_loaded_accounts_data_size_limit_ix => - { + Ok(ComputeBudgetInstruction::SetLoadedAccountsDataSizeLimit(bytes)) => { if updated_loaded_accounts_data_size_limit.is_some() { return Err(duplicate_instruction_error); } @@ -176,26 +171,20 @@ mod tests { }; macro_rules! test { - ( $instructions: expr, $expected_result: expr, $support_set_loaded_accounts_data_size_limit_ix: expr ) => { + ( $instructions: expr, $expected_result: 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 mut feature_set = FeatureSet::default(); - if $support_set_loaded_accounts_data_size_limit_ix { - feature_set.activate(&add_set_tx_loaded_accounts_data_size_instruction::id(), 0); - } + let feature_set = FeatureSet::default(); let result = process_compute_budget_instructions( tx.message().program_instructions_iter(), &feature_set, ); assert_eq!($expected_result, result); }; - ( $instructions: expr, $expected_result: expr ) => { - test!($instructions, $expected_result, false); - }; } #[test] @@ -411,128 +400,78 @@ mod tests { #[test] fn test_process_loaded_accounts_data_size_limit_instruction() { - // Assert for empty instructions, change value of support_set_loaded_accounts_data_size_limit_ix - // will not change results, which should all be default - for support_set_loaded_accounts_data_size_limit_ix in [true, false] { - test!( - &[], - Ok(ComputeBudgetLimits { - compute_unit_limit: 0, - ..ComputeBudgetLimits::default() - }), - support_set_loaded_accounts_data_size_limit_ix - ); - } + test!( + &[], + Ok(ComputeBudgetLimits { + compute_unit_limit: 0, + ..ComputeBudgetLimits::default() + }) + ); // Assert when set_loaded_accounts_data_size_limit presents, - // if support_set_loaded_accounts_data_size_limit_ix then - // budget is set with data_size - // else - // return InstructionError + // budget is set with data_size let data_size = 1; - for support_set_loaded_accounts_data_size_limit_ix in [true, false] { - let expected_result = if support_set_loaded_accounts_data_size_limit_ix { - Ok(ComputeBudgetLimits { - compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, - loaded_accounts_bytes: data_size, - ..ComputeBudgetLimits::default() - }) - } else { - Err(TransactionError::InstructionError( - 0, - InstructionError::InvalidInstructionData, - )) - }; + let expected_result = Ok(ComputeBudgetLimits { + compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, + loaded_accounts_bytes: data_size, + ..ComputeBudgetLimits::default() + }); - test!( - &[ - ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(data_size), - Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), - ], - expected_result, - support_set_loaded_accounts_data_size_limit_ix - ); - } + test!( + &[ + ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(data_size), + Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), + ], + expected_result + ); // Assert when set_loaded_accounts_data_size_limit presents, with greater than max value - // if support_set_loaded_accounts_data_size_limit_ix then - // budget is set to max data size - // else - // return InstructionError + // budget is set to max data size let data_size = MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES + 1; - for support_set_loaded_accounts_data_size_limit_ix in [true, false] { - let expected_result = if support_set_loaded_accounts_data_size_limit_ix { - Ok(ComputeBudgetLimits { - compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, - loaded_accounts_bytes: MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, - ..ComputeBudgetLimits::default() - }) - } else { - Err(TransactionError::InstructionError( - 0, - InstructionError::InvalidInstructionData, - )) - }; + let expected_result = Ok(ComputeBudgetLimits { + compute_unit_limit: DEFAULT_INSTRUCTION_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, - support_set_loaded_accounts_data_size_limit_ix - ); - } + test!( + &[ + ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(data_size), + Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), + ], + expected_result + ); // Assert when set_loaded_accounts_data_size_limit is not presented - // if support_set_loaded_accounts_data_size_limit_ix then - // budget is set to default data size - // else - // return - for support_set_loaded_accounts_data_size_limit_ix in [true, false] { - let expected_result = Ok(ComputeBudgetLimits { - compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, - loaded_accounts_bytes: MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, - ..ComputeBudgetLimits::default() - }); + // budget is set to default data size + let expected_result = Ok(ComputeBudgetLimits { + compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, + loaded_accounts_bytes: MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, + ..ComputeBudgetLimits::default() + }); - test!( - &[Instruction::new_with_bincode( - Pubkey::new_unique(), - &0_u8, - vec![] - ),], - expected_result, - support_set_loaded_accounts_data_size_limit_ix - ); - } + test!( + &[Instruction::new_with_bincode( + Pubkey::new_unique(), + &0_u8, + vec![] + ),], + expected_result + ); // Assert when set_loaded_accounts_data_size_limit presents more than once, - // if support_set_loaded_accounts_data_size_limit_ix then - // return DuplicateInstruction - // else - // return InstructionError + // return DuplicateInstruction let data_size = MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES; - for support_set_loaded_accounts_data_size_limit_ix in [true, false] { - let expected_result = if support_set_loaded_accounts_data_size_limit_ix { - Err(TransactionError::DuplicateInstruction(2)) - } else { - Err(TransactionError::InstructionError( - 1, - InstructionError::InvalidInstructionData, - )) - }; + let expected_result = Err(TransactionError::DuplicateInstruction(2)); - test!( - &[ - Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), - ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(data_size), - ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(data_size), - ], - expected_result, - support_set_loaded_accounts_data_size_limit_ix - ); - } + test!( + &[ + Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), + ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(data_size), + ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(data_size), + ], + expected_result + ); } #[test] @@ -550,8 +489,7 @@ mod tests { Hash::default(), )); - let mut feature_set = FeatureSet::default(); - feature_set.activate(&add_set_tx_loaded_accounts_data_size_instruction::id(), 0); + let feature_set = FeatureSet::default(); let result = process_compute_budget_instructions( transaction.message().program_instructions_iter(), diff --git a/runtime/src/accounts/mod.rs b/runtime/src/accounts/mod.rs index c6314d891..1f9db1618 100644 --- a/runtime/src/accounts/mod.rs +++ b/runtime/src/accounts/mod.rs @@ -1544,23 +1544,12 @@ mod tests { Ok(Some(NonZeroUsize::new(99).unwrap())); let result_invalid_limit = Err(TransactionError::InvalidLoadedAccountsDataSizeLimit); - let mut feature_set = FeatureSet::default(); + let feature_set = FeatureSet::default(); - // if `add_set_tx_loaded_accounts_data_size_instruction` is disabled, - // the result will always be default limit (64MiB) - test(tx_not_set_limit, &feature_set, &result_default_limit); - test(tx_set_limit_99, &feature_set, &result_default_limit); - test(tx_set_limit_0, &feature_set, &result_default_limit); - - // if `add_set_tx_loaded_accounts_data_size_instruction` is enabled, - // the results are: + // 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 - feature_set.activate( - &solana_sdk::feature_set::add_set_tx_loaded_accounts_data_size_instruction::id(), - 0, - ); 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); diff --git a/runtime/src/transaction_priority_details.rs b/runtime/src/transaction_priority_details.rs index 41f6c8d74..c5a3d6396 100644 --- a/runtime/src/transaction_priority_details.rs +++ b/runtime/src/transaction_priority_details.rs @@ -24,11 +24,7 @@ pub trait GetTransactionPriorityDetails { instructions: impl Iterator, _round_compute_unit_price_enabled: bool, ) -> Option { - let mut feature_set = FeatureSet::default(); - feature_set.activate( - &solana_sdk::feature_set::add_set_tx_loaded_accounts_data_size_instruction::id(), - 0, - ); + let feature_set = FeatureSet::default(); let compute_budget_limits = process_compute_budget_instructions(instructions, &feature_set).ok()?;