Skip to content

Commit

Permalink
refactor: rename compute_budget_instruction_details to instruction_de…
Browse files Browse the repository at this point in the history
…tails to better reflect its purpose
  • Loading branch information
tao-stones committed Jan 10, 2025
1 parent f07a570 commit 5b7f854
Show file tree
Hide file tree
Showing 12 changed files with 67 additions and 67 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl Default for MigrationBuiltinFeatureCounter {
#[cfg_attr(test, derive(Eq, PartialEq))]
#[cfg_attr(feature = "dev-context-only-utils", derive(Clone))]
#[derive(Default, Debug)]
pub struct ComputeBudgetInstructionDetails {
pub struct InstructionDetails {
// compute-budget instruction details:
// the first field in tuple is instruction index, second field is the unsanitized value set by user
requested_compute_unit_limit: Option<(u8, u32)>,
Expand All @@ -50,12 +50,12 @@ pub struct ComputeBudgetInstructionDetails {
migrating_builtin_feature_counters: MigrationBuiltinFeatureCounter,
}

impl ComputeBudgetInstructionDetails {
impl InstructionDetails {
pub fn try_from<'a>(
instructions: impl Iterator<Item = (&'a Pubkey, SVMInstruction<'a>)> + Clone,
) -> Result<Self> {
let mut filter = ComputeBudgetProgramIdFilter::new();
let mut compute_budget_instruction_details = ComputeBudgetInstructionDetails::default();
let mut compute_budget_instruction_details = InstructionDetails::default();

for (i, (program_id, instruction)) in instructions.clone().enumerate() {
if filter.is_compute_budget_program(instruction.program_id_index as usize, program_id) {
Expand Down Expand Up @@ -253,15 +253,15 @@ mod test {
ComputeBudgetInstruction::request_heap_frame(40 * 1024),
Instruction::new_with_bincode(Pubkey::new_unique(), &(), vec![]),
]);
let expected_details = Ok(ComputeBudgetInstructionDetails {
let expected_details = Ok(InstructionDetails {
requested_heap_size: Some((1, 40 * 1024)),
num_non_compute_budget_instructions: Saturating(2),
num_non_migratable_builtin_instructions: Saturating(1),
num_non_builtin_instructions: Saturating(2),
..ComputeBudgetInstructionDetails::default()
..InstructionDetails::default()
});
assert_eq!(
ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),),
InstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),),
expected_details
);

Expand All @@ -271,7 +271,7 @@ mod test {
ComputeBudgetInstruction::request_heap_frame(41 * 1024),
]);
assert_eq!(
ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),),
InstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),),
Err(TransactionError::DuplicateInstruction(2))
);
}
Expand All @@ -283,13 +283,13 @@ mod test {
ComputeBudgetInstruction::set_compute_unit_limit(u32::MAX),
Instruction::new_with_bincode(Pubkey::new_unique(), &(), vec![]),
]);
let expected_details = Ok(ComputeBudgetInstructionDetails {
let expected_details = Ok(InstructionDetails {
requested_compute_unit_limit: Some((1, u32::MAX)),
num_non_compute_budget_instructions: Saturating(2),
..ComputeBudgetInstructionDetails::default()
..InstructionDetails::default()
});
assert_eq!(
ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),),
InstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),),
expected_details
);

Expand All @@ -299,7 +299,7 @@ mod test {
ComputeBudgetInstruction::set_compute_unit_limit(u32::MAX),
]);
assert_eq!(
ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),),
InstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),),
Err(TransactionError::DuplicateInstruction(2))
);
}
Expand All @@ -311,15 +311,15 @@ mod test {
ComputeBudgetInstruction::set_compute_unit_price(u64::MAX),
Instruction::new_with_bincode(Pubkey::new_unique(), &(), vec![]),
]);
let expected_details = Ok(ComputeBudgetInstructionDetails {
let expected_details = Ok(InstructionDetails {
requested_compute_unit_price: Some((1, u64::MAX)),
num_non_compute_budget_instructions: Saturating(2),
num_non_migratable_builtin_instructions: Saturating(1),
num_non_builtin_instructions: Saturating(2),
..ComputeBudgetInstructionDetails::default()
..InstructionDetails::default()
});
assert_eq!(
ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),),
InstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),),
expected_details
);

Expand All @@ -329,7 +329,7 @@ mod test {
ComputeBudgetInstruction::set_compute_unit_price(u64::MAX),
]);
assert_eq!(
ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),),
InstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),),
Err(TransactionError::DuplicateInstruction(2))
);
}
Expand All @@ -341,15 +341,15 @@ mod test {
ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(u32::MAX),
Instruction::new_with_bincode(Pubkey::new_unique(), &(), vec![]),
]);
let expected_details = Ok(ComputeBudgetInstructionDetails {
let expected_details = Ok(InstructionDetails {
requested_loaded_accounts_data_size_limit: Some((1, u32::MAX)),
num_non_compute_budget_instructions: Saturating(2),
num_non_migratable_builtin_instructions: Saturating(1),
num_non_builtin_instructions: Saturating(2),
..ComputeBudgetInstructionDetails::default()
..InstructionDetails::default()
});
assert_eq!(
ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),),
InstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),),
expected_details
);

Expand All @@ -359,17 +359,17 @@ mod test {
ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(u32::MAX),
]);
assert_eq!(
ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),),
InstructionDetails::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,
instruction_details: &InstructionDetails,
) -> (FeatureSet, u32) {
let mut feature_set = FeatureSet::default();
let ComputeBudgetInstructionDetails {
let InstructionDetails {
num_non_compute_budget_instructions,
num_non_migratable_builtin_instructions,
num_non_builtin_instructions,
Expand All @@ -394,7 +394,7 @@ mod test {
#[test]
fn test_sanitize_and_convert_to_compute_budget_limits() {
// empty details, default ComputeBudgetLimits with 0 compute_unit_limits
let instruction_details = ComputeBudgetInstructionDetails::default();
let instruction_details = InstructionDetails::default();
assert_eq!(
instruction_details
.sanitize_and_convert_to_compute_budget_limits(&FeatureSet::default()),
Expand All @@ -405,11 +405,11 @@ mod test {
);

// no compute-budget instructions, all default ComputeBudgetLimits except cu-limit
let instruction_details = ComputeBudgetInstructionDetails {
let instruction_details = InstructionDetails {
num_non_compute_budget_instructions: Saturating(4),
num_non_migratable_builtin_instructions: Saturating(1),
num_non_builtin_instructions: Saturating(3),
..ComputeBudgetInstructionDetails::default()
..InstructionDetails::default()
};
for is_active in [true, false] {
let (feature_set, expected_compute_unit_limit) =
Expand All @@ -428,12 +428,12 @@ mod test {
InstructionError::InvalidInstructionData,
));
// invalid: requested_heap_size can't be zero
let instruction_details = ComputeBudgetInstructionDetails {
let instruction_details = InstructionDetails {
requested_compute_unit_limit: Some((1, 0)),
requested_compute_unit_price: Some((2, 0)),
requested_heap_size: Some((3, 0)),
requested_loaded_accounts_data_size_limit: Some((4, 1024)),
..ComputeBudgetInstructionDetails::default()
..InstructionDetails::default()
};
for is_active in [true, false] {
let (feature_set, _expected_compute_unit_limit) =
Expand All @@ -445,12 +445,12 @@ mod test {
}

// invalid: requested_heap_size can't be less than MIN_HEAP_FRAME_BYTES
let instruction_details = ComputeBudgetInstructionDetails {
let instruction_details = InstructionDetails {
requested_compute_unit_limit: Some((1, 0)),
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)),
..ComputeBudgetInstructionDetails::default()
..InstructionDetails::default()
};
for is_active in [true, false] {
let (feature_set, _expected_compute_unit_limit) =
Expand All @@ -462,12 +462,12 @@ mod test {
}

// invalid: requested_heap_size can't be more than MAX_HEAP_FRAME_BYTES
let instruction_details = ComputeBudgetInstructionDetails {
let instruction_details = InstructionDetails {
requested_compute_unit_limit: Some((1, 0)),
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)),
..ComputeBudgetInstructionDetails::default()
..InstructionDetails::default()
};
for is_active in [true, false] {
let (feature_set, _expected_compute_unit_limit) =
Expand All @@ -479,12 +479,12 @@ mod test {
}

// invalid: requested_heap_size must be round by 1024
let instruction_details = ComputeBudgetInstructionDetails {
let instruction_details = InstructionDetails {
requested_compute_unit_limit: Some((1, 0)),
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)),
..ComputeBudgetInstructionDetails::default()
..InstructionDetails::default()
};
for is_active in [true, false] {
let (feature_set, _expected_compute_unit_limit) =
Expand All @@ -496,12 +496,12 @@ mod test {
}

// invalid: loaded_account_data_size can't be zero
let instruction_details = ComputeBudgetInstructionDetails {
let instruction_details = InstructionDetails {
requested_compute_unit_limit: Some((1, 0)),
requested_compute_unit_price: Some((2, 0)),
requested_heap_size: Some((3, 40 * 1024)),
requested_loaded_accounts_data_size_limit: Some((4, 0)),
..ComputeBudgetInstructionDetails::default()
..InstructionDetails::default()
};
for is_active in [true, false] {
let (feature_set, _expected_compute_unit_limit) =
Expand All @@ -513,13 +513,13 @@ mod test {
}

// valid: acceptable MAX
let instruction_details = ComputeBudgetInstructionDetails {
let instruction_details = InstructionDetails {
requested_compute_unit_limit: Some((1, u32::MAX)),
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: Saturating(4),
..ComputeBudgetInstructionDetails::default()
..InstructionDetails::default()
};
for is_active in [true, false] {
let (feature_set, _expected_compute_unit_limit) =
Expand All @@ -537,12 +537,12 @@ mod test {

// valid
let val: u32 = 1024 * 40;
let instruction_details = ComputeBudgetInstructionDetails {
let instruction_details = InstructionDetails {
requested_compute_unit_limit: Some((1, val)),
requested_compute_unit_price: Some((2, val as u64)),
requested_heap_size: Some((3, val)),
requested_loaded_accounts_data_size_limit: Some((4, val)),
..ComputeBudgetInstructionDetails::default()
..InstructionDetails::default()
};
for is_active in [true, false] {
let (feature_set, _expected_compute_unit_limit) =
Expand Down Expand Up @@ -571,17 +571,17 @@ mod test {
]);
let feature_id_index =
get_migration_feature_position(&feature_set::migrate_stake_program_to_core_bpf::id());
let mut expected_details = ComputeBudgetInstructionDetails {
let mut expected_details = InstructionDetails {
num_non_compute_budget_instructions: Saturating(2),
num_non_builtin_instructions: Saturating(1),
..ComputeBudgetInstructionDetails::default()
..InstructionDetails::default()
};
expected_details
.migrating_builtin_feature_counters
.migrating_builtin[feature_id_index] = Saturating(1);
let expected_details = Ok(expected_details);
let details =
ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx));
InstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx));
assert_eq!(details, expected_details);
let details = details.unwrap();

Expand Down
4 changes: 2 additions & 2 deletions compute-budget-instruction/src/instructions_processor.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use {
crate::compute_budget_instruction_details::*, solana_compute_budget::compute_budget_limits::*,
crate::instruction_details::*, solana_compute_budget::compute_budget_limits::*,
solana_feature_set::FeatureSet, solana_pubkey::Pubkey,
solana_svm_transaction::instruction::SVMInstruction,
solana_transaction_error::TransactionError,
Expand All @@ -14,7 +14,7 @@ pub fn process_compute_budget_instructions<'a>(
instructions: impl Iterator<Item = (&'a Pubkey, SVMInstruction<'a>)> + Clone,
feature_set: &FeatureSet,
) -> Result<ComputeBudgetLimits, TransactionError> {
ComputeBudgetInstructionDetails::try_from(instructions)?
InstructionDetails::try_from(instructions)?
.sanitize_and_convert_to_compute_budget_limits(feature_set)
}

Expand Down
2 changes: 1 addition & 1 deletion compute-budget-instruction/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![allow(clippy::arithmetic_side_effects)]

mod builtin_programs_filter;
pub mod compute_budget_instruction_details;
pub mod instruction_details;
mod compute_budget_program_id_filter;
pub mod instructions_processor;
4 changes: 2 additions & 2 deletions core/src/banking_stage/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ impl Consumer {
.iter()
.filter_map(|transaction| {
transaction
.compute_budget_instruction_details()
.instruction_details()
.sanitize_and_convert_to_compute_budget_limits(&bank.feature_set)
.ok()
.map(|limits| limits.compute_unit_price)
Expand Down Expand Up @@ -759,7 +759,7 @@ impl Consumer {
let fee_payer = transaction.fee_payer();
let fee_budget_limits = FeeBudgetLimits::from(
transaction
.compute_budget_instruction_details()
.instruction_details()
.sanitize_and_convert_to_compute_budget_limits(&bank.feature_set)?,
);
let fee = solana_fee::calculate_fee(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ impl SanitizedTransactionReceiveAndBuffer {
.is_ok()
})
.filter_map(|(packet, tx, deactivation_slot)| {
tx.compute_budget_instruction_details()
tx.instruction_details()
.sanitize_and_convert_to_compute_budget_limits(&working_bank.feature_set)
.map(|compute_budget| {
(packet, tx, deactivation_slot, compute_budget.into())
Expand Down
4 changes: 2 additions & 2 deletions cost-model/src/cost_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ impl CostModel {
// will not be executed by `bank`, therefore it should be considered
// as no execution cost by cost model.
match meta
.compute_budget_instruction_details()
.instruction_details()
.sanitize_and_convert_to_compute_budget_limits(feature_set)
{
Ok(compute_budget_limits) => {
Expand Down Expand Up @@ -293,7 +293,7 @@ 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.
let (programs_execution_costs, loaded_accounts_data_size_cost) = match transaction
.compute_budget_instruction_details()
.instruction_details()
.sanitize_and_convert_to_compute_budget_limits(feature_set)
{
Ok(compute_budget_limits) => (
Expand Down
6 changes: 3 additions & 3 deletions cost-model/src/transaction_cost.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#[cfg(feature = "dev-context-only-utils")]
use solana_compute_budget_instruction::compute_budget_instruction_details::ComputeBudgetInstructionDetails;
use solana_compute_budget_instruction::instruction_details::InstructionDetails;
use {
crate::block_cost_limits, solana_pubkey::Pubkey,
solana_runtime_transaction::transaction_meta::StaticMeta,
Expand Down Expand Up @@ -265,8 +265,8 @@ impl solana_runtime_transaction::transaction_meta::StaticMeta for WritableKeysTr
&DUMMY
}

fn compute_budget_instruction_details(&self) -> &ComputeBudgetInstructionDetails {
unimplemented!("WritableKeysTransaction::compute_budget_instruction_details")
fn instruction_details(&self) -> &InstructionDetails {
unimplemented!("WritableKeysTransaction::instruction_details")
}
}

Expand Down
Loading

0 comments on commit 5b7f854

Please sign in to comment.