Skip to content

Commit

Permalink
cleanup feature code after mainnet-beta activation
Browse files Browse the repository at this point in the history
  • Loading branch information
@tao-stones
tao-stones committed Nov 22, 2023
1 parent 53c723a commit 5288501
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 176 deletions.
35 changes: 1 addition & 34 deletions cost-model/src/cost_model.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down
187 changes: 63 additions & 124 deletions program-runtime/src/compute_budget_processor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@ use {
borsh0_10::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, remove_deprecated_request_unit_ix,
FeatureSet,
},
feature_set::{remove_deprecated_request_unit_ix, FeatureSet},
fee::FeeBudgetLimits,
instruction::{CompiledInstruction, InstructionError},
pubkey::Pubkey,
Expand Down Expand Up @@ -83,8 +80,6 @@ pub fn process_compute_budget_instructions<'a>(
) -> Result<ComputeBudgetLimits, TransactionError> {
let support_request_units_deprecated =
!feature_set.is_active(&remove_deprecated_request_unit_ix::id());
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;
Expand Down Expand Up @@ -139,9 +134,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);
}
Expand Down Expand Up @@ -204,6 +197,7 @@ mod tests {
use {
super::*,
solana_sdk::{
feature_set::add_set_tx_loaded_accounts_data_size_instruction,
hash::Hash,
instruction::Instruction,
message::Message,
Expand All @@ -216,7 +210,7 @@ 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],
Expand All @@ -225,18 +219,13 @@ mod tests {
));
let mut feature_set = FeatureSet::default();
feature_set.activate(&remove_deprecated_request_unit_ix::id(), 0);
if $support_set_loaded_accounts_data_size_limit_ix {
feature_set.activate(&add_set_tx_loaded_accounts_data_size_instruction::id(), 0);
}
feature_set.activate(&add_set_tx_loaded_accounts_data_size_instruction::id(), 0);
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]
Expand Down Expand Up @@ -468,128 +457,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()
});

test!(
&[Instruction::new_with_bincode(
Pubkey::new_unique(),
&0_u8,
vec![]
),],
expected_result,
support_set_loaded_accounts_data_size_limit_ix
);
}
// 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
);

// 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]
Expand Down
22 changes: 4 additions & 18 deletions runtime/src/accounts/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1550,7 +1550,6 @@ mod tests {
solana_sdk::instruction::Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
];

let result_no_limit = Ok(None);
let result_default_limit = Ok(Some(
NonZeroUsize::new(
usize::try_from(compute_budget_processor::MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES)
Expand All @@ -1564,26 +1563,13 @@ mod tests {

let mut feature_set = FeatureSet::default();

// if `cap_transaction_accounts_data_size feature` is disable,
// the result will always be no limit
test(tx_not_set_limit, &feature_set, &result_no_limit);
test(tx_set_limit_99, &feature_set, &result_no_limit);
test(tx_set_limit_0, &feature_set, &result_no_limit);

// if `cap_transaction_accounts_data_size` is enabled, and
// `add_set_tx_loaded_accounts_data_size_instruction` is disabled,
// the result will always be default limit (64MiB)
feature_set.activate(&feature_set::cap_transaction_accounts_data_size::id(), 0);
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 `cap_transaction_accounts_data_size` and
// `add_set_tx_loaded_accounts_data_size_instruction` are both enabled,
// the results are:
// both `cap_transaction_accounts_data_size` and
// `add_set_tx_loaded_accounts_data_size_instruction` are activated in mainnet-beta
// 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(&feature_set::cap_transaction_accounts_data_size::id(), 0);
feature_set.activate(
&solana_sdk::feature_set::add_set_tx_loaded_accounts_data_size_instruction::id(),
0,
Expand Down
2 changes: 2 additions & 0 deletions runtime/src/transaction_priority_details.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ pub trait GetTransactionPriorityDetails {
_round_compute_unit_price_enabled: bool,
) -> Option<TransactionPriorityDetails> {
let mut feature_set = FeatureSet::default();
// this feature was enabled on mainnet-beta since epooch 537
// https://github.com/solana-labs/solana/issues/30366
feature_set.activate(
&solana_sdk::feature_set::add_set_tx_loaded_accounts_data_size_instruction::id(),
0,
Expand Down

0 comments on commit 5288501

Please sign in to comment.