Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove feature set from compute budget processor #34472

Merged
merged 1 commit into from
Dec 15, 2023
Merged

Remove feature set from compute budget processor #34472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions cost-model/src/cost_model.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,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
Expand Down
8 changes: 2 additions & 6 deletions program-runtime/src/compute_budget.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down Expand Up @@ -183,9 +180,8 @@ impl ComputeBudget {

pub fn try_from_instructions<'a>(
instructions: impl Iterator<Item = (&'a Pubkey, &'a CompiledInstruction)>,
feature_set: &FeatureSet,
) -> Result<Self> {
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,
Expand Down
18 changes: 4 additions & 14 deletions program-runtime/src/compute_budget_processor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
//! Process compute_budget instructions to extract and sanitize limits.
use {
crate::{
compute_budget::DEFAULT_HEAP_COST,
Expand All @@ -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,
Expand Down Expand Up @@ -69,7 +67,6 @@ impl From<ComputeBudgetLimits> for FeeBudgetLimits {
/// are retrieved and returned,
pub fn process_compute_budget_instructions<'a>(
instructions: impl Iterator<Item = (&'a Pubkey, &'a CompiledInstruction)>,
_feature_set: &FeatureSet,
) -> Result<ComputeBudgetLimits, TransactionError> {
let mut num_non_compute_budget_instructions: u32 = 0;
let mut updated_compute_unit_limit = None;
Expand Down Expand Up @@ -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);
};
}
Expand Down Expand Up @@ -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
Expand Down
21 changes: 6 additions & 15 deletions programs/sbf/tests/programs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
Expand All @@ -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,
);
Expand Down
25 changes: 8 additions & 17 deletions runtime/src/accounts/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<Option<NonZeroUsize>> {
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(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1502,7 +1498,6 @@ mod tests {
// an prrivate helper function
fn test(
instructions: &[solana_sdk::instruction::Instruction],
feature_set: &FeatureSet,
expected_result: &Result<Option<NonZeroUsize>>,
) {
let payer_keypair = Keypair::new();
Expand All @@ -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)
);
}

Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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,
Expand Down
10 changes: 3 additions & 7 deletions runtime/src/bank.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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!(
Expand Down
7 changes: 3 additions & 4 deletions runtime/src/bank/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 1 addition & 5 deletions runtime/src/transaction_priority_details.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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},
Expand All @@ -24,10 +23,7 @@ pub trait GetTransactionPriorityDetails {
instructions: impl Iterator<Item = (&'a Pubkey, &'a CompiledInstruction)>,
_round_compute_unit_price_enabled: bool,
) -> Option<TransactionPriorityDetails> {
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),
Expand Down