cleanup feature: enable request heap frame instruction #30076 (#33243)

* cleanup feature: enable request heap frame instruction #30076

* update sbf tests

* removed out dated comments and test
This commit is contained in:
Tao Zhu 2023-09-18 16:06:24 -05:00 committed by GitHub
parent 17c3930bc8
commit 8b8a21a52f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 12 additions and 199 deletions

View File

@ -252,7 +252,6 @@ impl Accounts {
let _process_transaction_result = compute_budget.process_instructions(
tx.message().program_instructions_iter(),
!feature_set.is_active(&remove_deprecated_request_unit_ix::id()),
true, // don't reject txs that use request heap size ix
feature_set.is_active(&add_set_tx_loaded_accounts_data_size_instruction::id()),
);
// sanitize against setting size limit to zero
@ -723,7 +722,7 @@ impl Accounts {
fee_structure.calculate_fee(
tx.message(),
lamports_per_signature,
&ComputeBudget::fee_budget_limits(tx.message().program_instructions_iter(), feature_set, Some(self.accounts_db.expected_cluster_type())),
&ComputeBudget::fee_budget_limits(tx.message().program_instructions_iter(), feature_set),
feature_set.is_active(&remove_congestion_multiplier_from_fee_calculation::id()),
feature_set.is_active(&include_loaded_accounts_data_size_in_fee_calculation::id()),
)
@ -1758,11 +1757,7 @@ mod tests {
let fee = FeeStructure::default().calculate_fee(
&message,
lamports_per_signature,
&ComputeBudget::fee_budget_limits(
message.program_instructions_iter(),
&feature_set,
None,
),
&ComputeBudget::fee_budget_limits(message.program_instructions_iter(), &feature_set),
true,
false,
);
@ -4327,11 +4322,7 @@ mod tests {
let fee = FeeStructure::default().calculate_fee(
&message,
lamports_per_signature,
&ComputeBudget::fee_budget_limits(
message.program_instructions_iter(),
&feature_set,
None,
),
&ComputeBudget::fee_budget_limits(message.program_instructions_iter(), &feature_set),
true,
false,
);

View File

@ -119,17 +119,9 @@ impl CostModel {
// calculate bpf cost based on compute budget instructions
let mut compute_budget = ComputeBudget::default();
// Starting from v1.15, cost model uses compute_budget.set_compute_unit_limit to
// measure bpf_costs (code below), vs earlier versions that use estimated
// bpf instruction costs. The calculated transaction costs are used by leaders
// during block packing, different costs for same transaction due to different versions
// will not impact consensus. So for v1.15+, should call compute budget with
// the feature gate `enable_request_heap_frame_ix` enabled.
let enable_request_heap_frame_ix = true;
let result = compute_budget.process_instructions(
transaction.message().program_instructions_iter(),
!feature_set.is_active(&remove_deprecated_request_unit_ix::id()),
enable_request_heap_frame_ix,
feature_set.is_active(&add_set_tx_loaded_accounts_data_size_instruction::id()),
);

View File

@ -5,11 +5,10 @@ use {
compute_budget::{self, ComputeBudgetInstruction},
entrypoint::HEAP_LENGTH as MIN_HEAP_FRAME_BYTES,
feature_set::{
add_set_tx_loaded_accounts_data_size_instruction, enable_request_heap_frame_ix,
remove_deprecated_request_unit_ix, FeatureSet,
add_set_tx_loaded_accounts_data_size_instruction, remove_deprecated_request_unit_ix,
FeatureSet,
},
fee::FeeBudgetLimits,
genesis_config::ClusterType,
instruction::{CompiledInstruction, InstructionError},
pubkey::Pubkey,
transaction::TransactionError,
@ -191,7 +190,6 @@ impl ComputeBudget {
&mut self,
instructions: impl Iterator<Item = (&'a Pubkey, &'a CompiledInstruction)>,
support_request_units_deprecated: bool,
enable_request_heap_frame_ix: bool,
support_set_loaded_accounts_data_size_limit_ix: bool,
) -> Result<PrioritizationFeeDetails, TransactionError> {
let mut num_non_compute_budget_instructions: u32 = 0;
@ -260,8 +258,7 @@ impl ComputeBudget {
}
if let Some((bytes, i)) = requested_heap_size {
if !enable_request_heap_frame_ix
|| bytes > MAX_HEAP_FRAME_BYTES
if bytes > MAX_HEAP_FRAME_BYTES
|| bytes < MIN_HEAP_FRAME_BYTES as u32
|| bytes % 1024 != 0
{
@ -293,23 +290,13 @@ impl ComputeBudget {
pub fn fee_budget_limits<'a>(
instructions: impl Iterator<Item = (&'a Pubkey, &'a CompiledInstruction)>,
feature_set: &FeatureSet,
maybe_cluster_type: Option<ClusterType>,
) -> FeeBudgetLimits {
let mut compute_budget = Self::default();
// A cluster specific feature gate, when not activated it keeps v1.13 behavior in mainnet-beta;
// once activated for v1.14+, it allows compute_budget::request_heap_frame and
// compute_budget::set_compute_unit_price co-exist in same transaction.
let enable_request_heap_frame_ix = feature_set
.is_active(&enable_request_heap_frame_ix::id())
|| maybe_cluster_type
.and_then(|cluster_type| (cluster_type != ClusterType::MainnetBeta).then_some(0))
.is_some();
let prioritization_fee_details = compute_budget
.process_instructions(
instructions,
!feature_set.is_active(&remove_deprecated_request_unit_ix::id()),
enable_request_heap_frame_ix,
feature_set.is_active(&add_set_tx_loaded_accounts_data_size_instruction::id()),
)
.unwrap_or_default();
@ -369,7 +356,7 @@ mod tests {
};
macro_rules! test {
( $instructions: expr, $expected_result: expr, $expected_budget: expr, $enable_request_heap_frame_ix: expr, $support_set_loaded_accounts_data_size_limit_ix: expr ) => {
( $instructions: expr, $expected_result: expr, $expected_budget: expr, $support_set_loaded_accounts_data_size_limit_ix: expr ) => {
let payer_keypair = Keypair::new();
let tx = SanitizedTransaction::from_transaction_for_tests(Transaction::new(
&[&payer_keypair],
@ -380,20 +367,13 @@ mod tests {
let result = compute_budget.process_instructions(
tx.message().program_instructions_iter(),
false, /*not support request_units_deprecated*/
$enable_request_heap_frame_ix,
$support_set_loaded_accounts_data_size_limit_ix,
);
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,
false
);
test!($instructions, $expected_result, $expected_budget, false);
};
}
@ -654,104 +634,8 @@ mod tests {
);
}
#[test]
fn test_process_instructions_disable_request_heap_frame() {
// assert empty message results default compute budget and fee
test!(
&[],
Ok(PrioritizationFeeDetails::default()),
ComputeBudget {
compute_unit_limit: 0,
..ComputeBudget::default()
},
false,
false
);
// assert requesting heap frame when feature is disable will result instruction error
test!(
&[
ComputeBudgetInstruction::request_heap_frame(40 * 1024),
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
],
Err(TransactionError::InstructionError(
0,
InstructionError::InvalidInstructionData
)),
ComputeBudget::default(),
false,
false
);
test!(
&[
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
ComputeBudgetInstruction::request_heap_frame(MAX_HEAP_FRAME_BYTES),
],
Err(TransactionError::InstructionError(
1,
InstructionError::InvalidInstructionData,
)),
ComputeBudget::default(),
false,
false
);
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(
1,
InstructionError::InvalidInstructionData,
)),
ComputeBudget::default(),
false,
false
);
test!(
&[
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
ComputeBudgetInstruction::set_compute_unit_limit(1),
ComputeBudgetInstruction::request_heap_frame(MAX_HEAP_FRAME_BYTES),
ComputeBudgetInstruction::set_compute_unit_price(u64::MAX),
],
Err(TransactionError::InstructionError(
2,
InstructionError::InvalidInstructionData,
)),
ComputeBudget::default(),
false,
false
);
// assert normal results when not requesting heap frame when the feature is disabled
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![]),
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![]),
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
],
Ok(PrioritizationFeeDetails::default()),
ComputeBudget {
compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64 * 7,
..ComputeBudget::default()
},
false,
false
);
}
#[test]
fn test_process_loaded_accounts_data_size_limit_instruction() {
let enable_request_heap_frame_ix: bool = true;
// 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] {
@ -762,7 +646,6 @@ mod tests {
compute_unit_limit: 0,
..ComputeBudget::default()
},
enable_request_heap_frame_ix,
support_set_loaded_accounts_data_size_limit_ix
);
}
@ -801,7 +684,6 @@ mod tests {
],
expected_result,
expected_budget,
enable_request_heap_frame_ix,
support_set_loaded_accounts_data_size_limit_ix
);
}
@ -840,7 +722,6 @@ mod tests {
],
expected_result,
expected_budget,
enable_request_heap_frame_ix,
support_set_loaded_accounts_data_size_limit_ix
);
}
@ -868,7 +749,6 @@ mod tests {
),],
expected_result,
expected_budget,
enable_request_heap_frame_ix,
support_set_loaded_accounts_data_size_limit_ix
);
}
@ -904,7 +784,6 @@ mod tests {
],
expected_result,
expected_budget,
enable_request_heap_frame_ix,
support_set_loaded_accounts_data_size_limit_ix
);
}
@ -929,7 +808,6 @@ mod tests {
let result = compute_budget.process_instructions(
transaction.message().program_instructions_iter(),
false, //not support request_units_deprecated
true, //enable_request_heap_frame_ix,
true, //support_set_loaded_accounts_data_size_limit_ix,
);

View File

@ -3836,7 +3836,6 @@ fn test_program_fees() {
&ComputeBudget::fee_budget_limits(
sanitized_message.program_instructions_iter(),
&feature_set,
None,
),
true,
false,
@ -3864,7 +3863,6 @@ fn test_program_fees() {
&ComputeBudget::fee_budget_limits(
sanitized_message.program_instructions_iter(),
&feature_set,
None,
),
true,
false,

View File

@ -4092,7 +4092,6 @@ impl Bank {
&ComputeBudget::fee_budget_limits(
message.program_instructions_iter(),
&self.feature_set,
Some(self.cluster_type()),
),
self.feature_set
.is_active(&remove_congestion_multiplier_from_fee_calculation::id()),
@ -5179,7 +5178,6 @@ impl Bank {
!self
.feature_set
.is_active(&remove_deprecated_request_unit_ix::id()),
true, // don't reject txs that use request heap size ix
self.feature_set
.is_active(&add_set_tx_loaded_accounts_data_size_instruction::id()),
);

View File

@ -63,7 +63,7 @@ use {
entrypoint::MAX_PERMITTED_DATA_INCREASE,
epoch_schedule::{EpochSchedule, MINIMUM_SLOTS_PER_EPOCH},
feature::{self, Feature},
feature_set::{self, enable_request_heap_frame_ix, FeatureSet},
feature_set::{self, FeatureSet},
fee::FeeStructure,
fee_calculator::FeeRateGovernor,
genesis_config::{create_genesis_config, ClusterType, GenesisConfig},
@ -2829,7 +2829,6 @@ fn test_bank_tx_compute_unit_fee() {
&FeeStructure::default(),
false,
true,
true,
);
let (expected_fee_collected, expected_fee_burned) =
@ -3011,7 +3010,6 @@ fn test_bank_blockhash_compute_unit_fee_structure() {
&FeeStructure::default(),
false,
true,
true,
);
assert_eq!(
bank.get_balance(&mint_keypair.pubkey()),
@ -3030,7 +3028,6 @@ fn test_bank_blockhash_compute_unit_fee_structure() {
&FeeStructure::default(),
false,
true,
true,
);
assert_eq!(
bank.get_balance(&mint_keypair.pubkey()),
@ -3144,7 +3141,6 @@ fn test_filter_program_errors_and_collect_compute_unit_fee() {
&FeeStructure::default(),
false,
true,
true,
) * 2
)
.0
@ -10074,7 +10070,6 @@ fn calculate_test_fee(
lamports_per_signature: u64,
fee_structure: &FeeStructure,
support_set_accounts_data_size_limit_ix: bool,
enable_request_heap_frame_ix: bool,
remove_congestion_multiplier: bool,
) -> u64 {
let mut feature_set = FeatureSet::all_enabled();
@ -10084,12 +10079,8 @@ fn calculate_test_fee(
feature_set.deactivate(&include_loaded_accounts_data_size_in_fee_calculation::id());
}
if !enable_request_heap_frame_ix {
feature_set.deactivate(&enable_request_heap_frame_ix::id());
}
let budget_limits =
ComputeBudget::fee_budget_limits(message.program_instructions_iter(), &feature_set, None);
ComputeBudget::fee_budget_limits(message.program_instructions_iter(), &feature_set);
fee_structure.calculate_fee(
message,
lamports_per_signature,
@ -10115,7 +10106,6 @@ fn test_calculate_fee() {
},
support_set_accounts_data_size_limit_ix,
true,
true,
),
0
);
@ -10133,7 +10123,6 @@ fn test_calculate_fee() {
},
support_set_accounts_data_size_limit_ix,
true,
true,
),
1
);
@ -10156,7 +10145,6 @@ fn test_calculate_fee() {
},
support_set_accounts_data_size_limit_ix,
true,
true,
),
4
);
@ -10184,7 +10172,6 @@ fn test_calculate_fee_compute_units() {
&fee_structure,
support_set_accounts_data_size_limit_ix,
true,
true,
),
max_fee + lamports_per_signature
);
@ -10204,7 +10191,6 @@ fn test_calculate_fee_compute_units() {
&fee_structure,
support_set_accounts_data_size_limit_ix,
true,
true,
),
max_fee + 3 * lamports_per_signature
);
@ -10246,7 +10232,6 @@ fn test_calculate_fee_compute_units() {
&fee_structure,
support_set_accounts_data_size_limit_ix,
true,
true,
);
assert_eq!(
fee,
@ -10286,7 +10271,6 @@ fn test_calculate_prioritization_fee() {
&fee_structure,
true,
true,
true,
);
assert_eq!(
fee,
@ -10332,7 +10316,6 @@ fn test_calculate_fee_secp256k1() {
&fee_structure,
support_set_accounts_data_size_limit_ix,
true,
true,
),
2
);
@ -10353,7 +10336,6 @@ fn test_calculate_fee_secp256k1() {
&fee_structure,
support_set_accounts_data_size_limit_ix,
true,
true,
),
11
);
@ -11994,7 +11976,6 @@ fn test_calculate_fee_with_congestion_multiplier() {
cheap_lamports_per_signature,
&fee_structure,
true,
true,
remove_congestion_multiplier,
),
signature_fee * signature_count
@ -12016,7 +11997,6 @@ fn test_calculate_fee_with_congestion_multiplier() {
expensive_lamports_per_signature,
&fee_structure,
true,
true,
remove_congestion_multiplier,
),
signature_fee * signature_count / denominator
@ -12047,35 +12027,12 @@ fn test_calculate_fee_with_request_heap_frame_flag() {
))
.unwrap();
// assert when enable_request_heap_frame_ix is enabled, prioritization fee will be counted
// assert when request_heap_frame is presented in tx, prioritization fee will be counted
// into transaction fee
let mut enable_request_heap_frame_ix = true;
assert_eq!(
calculate_test_fee(
&message,
lamports_per_signature,
&fee_structure,
true,
enable_request_heap_frame_ix,
true,
),
calculate_test_fee(&message, lamports_per_signature, &fee_structure, true, true,),
signature_fee + request_cu * lamports_per_cu
);
// assert when enable_request_heap_frame_ix is disabled (an v1.13 behavior), prioritization fee will not be counted
// into transaction fee
enable_request_heap_frame_ix = false;
assert_eq!(
calculate_test_fee(
&message,
lamports_per_signature,
&fee_structure,
true,
enable_request_heap_frame_ix,
true,
),
signature_fee
);
}
#[test]

View File

@ -28,7 +28,6 @@ pub trait GetTransactionPriorityDetails {
.process_instructions(
instructions,
true, // supports prioritization by request_units_deprecated instruction
true, // enable request heap frame instruction
true, // enable support set accounts data size instruction
// TODO: round_compute_unit_price_enabled: bool
)