Skip to content

Commit

Permalink
feature gate: remove support for RequestUnitsDeprecated instruction s…
Browse files Browse the repository at this point in the history
  • Loading branch information
tao-stones committed Aug 31, 2022
1 parent 3d03f7b commit d4f549b
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 34 deletions.
1 change: 1 addition & 0 deletions core/src/transaction_priority_details.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ pub trait GetTransactionPriorityDetails {
.process_instructions(
instructions,
true, // use default units per instruction
true, // support request_units_deprecated instruction
)
.ok()?;
Some(TransactionPriorityDetails {
Expand Down
27 changes: 24 additions & 3 deletions program-runtime/src/compute_budget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ impl ComputeBudget {
&mut self,
instructions: impl Iterator<Item = (&'a Pubkey, &'a CompiledInstruction)>,
default_units_per_instruction: bool,
support_request_units_deprecated: bool,
) -> Result<PrioritizationFeeDetails, TransactionError> {
let mut num_non_compute_budget_instructions: usize = 0;
let mut updated_compute_unit_limit = None;
Expand All @@ -145,7 +146,7 @@ impl ComputeBudget {
Ok(ComputeBudgetInstruction::RequestUnitsDeprecated {
units: compute_unit_limit,
additional_fee,
}) => {
}) if support_request_units_deprecated => {
if updated_compute_unit_limit.is_some() {
return Err(duplicate_instruction_error);
}
Expand Down Expand Up @@ -240,8 +241,11 @@ mod tests {
Hash::default(),
));
let mut compute_budget = ComputeBudget::default();
let result =
compute_budget.process_instructions(tx.message().program_instructions_iter(), true);
let result = compute_budget.process_instructions(
tx.message().program_instructions_iter(),
true,
false, /*not support request_units_deprecated*/
);
assert_eq!($expected_result, result);
assert_eq!(compute_budget, $expected_budget);
};
Expand Down Expand Up @@ -488,5 +492,22 @@ mod tests {
Err(TransactionError::DuplicateInstruction(2)),
ComputeBudget::default()
);

// deprecated
test!(
&[Instruction::new_with_borsh(
compute_budget::id(),
&compute_budget::ComputeBudgetInstruction::RequestUnitsDeprecated {
units: 1_000,
additional_fee: 10
},
vec![]
)],
Err(TransactionError::InstructionError(
0,
InstructionError::InvalidInstructionData,
)),
ComputeBudget::default()
);
}
}
2 changes: 2 additions & 0 deletions program-runtime/src/prioritization_fee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ type MicroLamports = u128;

pub enum PrioritizationFeeType {
ComputeUnitPrice(u64),
/// TODO: remove 'Deprecated' after feature remove_deprecated_request_unit_ix::id() is activated
Deprecated(u64),
}

Expand All @@ -17,6 +18,7 @@ pub struct PrioritizationFeeDetails {
impl PrioritizationFeeDetails {
pub fn new(fee_type: PrioritizationFeeType, compute_unit_limit: u64) -> Self {
match fee_type {
// TODO: remove support of 'Deprecated' after feature remove_deprecated_request_unit_ix::id() is activated
PrioritizationFeeType::Deprecated(fee) => {
let priority = if compute_unit_limit == 0 {
0
Expand Down
7 changes: 6 additions & 1 deletion runtime/src/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ use {
account_utils::StateMut,
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
clock::{BankId, Slot, INITIAL_RENT_EPOCH},
feature_set::{self, use_default_units_in_fee_calculation, FeatureSet},
feature_set::{
self, remove_deprecated_request_unit_ix, use_default_units_in_fee_calculation,
FeatureSet,
},
fee::FeeStructure,
genesis_config::ClusterType,
hash::Hash,
Expand Down Expand Up @@ -553,6 +556,7 @@ impl Accounts {
lamports_per_signature,
fee_structure,
feature_set.is_active(&use_default_units_in_fee_calculation::id()),
!feature_set.is_active(&remove_deprecated_request_unit_ix::id()),
)
} else {
return (Err(TransactionError::BlockhashNotFound), None);
Expand Down Expand Up @@ -1674,6 +1678,7 @@ mod tests {
lamports_per_signature,
&FeeStructure::default(),
true,
false,
);
assert_eq!(fee, lamports_per_signature);

Expand Down
88 changes: 58 additions & 30 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ use {
feature,
feature_set::{
self, disable_fee_calculator, enable_early_verification_of_account_modifications,
use_default_units_in_fee_calculation, FeatureSet,
remove_deprecated_request_unit_ix, use_default_units_in_fee_calculation, FeatureSet,
},
fee::FeeStructure,
fee_calculator::{FeeCalculator, FeeRateGovernor},
Expand Down Expand Up @@ -3726,6 +3726,9 @@ impl Bank {
&self.fee_structure,
self.feature_set
.is_active(&use_default_units_in_fee_calculation::id()),
!self
.feature_set
.is_active(&remove_deprecated_request_unit_ix::id()),
))
}

Expand Down Expand Up @@ -3767,6 +3770,9 @@ impl Bank {
&self.fee_structure,
self.feature_set
.is_active(&use_default_units_in_fee_calculation::id()),
!self
.feature_set
.is_active(&remove_deprecated_request_unit_ix::id()),
)
}

Expand Down Expand Up @@ -4568,30 +4574,34 @@ impl Bank {
.map(|(accs, tx)| match accs {
(Err(e), _nonce) => TransactionExecutionResult::NotExecuted(e.clone()),
(Ok(loaded_transaction), nonce) => {
let compute_budget = if let Some(compute_budget) =
self.runtime_config.compute_budget
{
compute_budget
} else {
let mut compute_budget =
ComputeBudget::new(compute_budget::MAX_COMPUTE_UNIT_LIMIT as u64);

let mut compute_budget_process_transaction_time =
Measure::start("compute_budget_process_transaction_time");
let process_transaction_result = compute_budget
.process_instructions(tx.message().program_instructions_iter(), true);
compute_budget_process_transaction_time.stop();
saturating_add_assign!(
timings
.execute_accessories
.compute_budget_process_transaction_us,
compute_budget_process_transaction_time.as_us()
);
if let Err(err) = process_transaction_result {
return TransactionExecutionResult::NotExecuted(err);
}
compute_budget
};
let compute_budget =
if let Some(compute_budget) = self.runtime_config.compute_budget {
compute_budget
} else {
let mut compute_budget =
ComputeBudget::new(compute_budget::MAX_COMPUTE_UNIT_LIMIT as u64);

let mut compute_budget_process_transaction_time =
Measure::start("compute_budget_process_transaction_time");
let process_transaction_result = compute_budget.process_instructions(
tx.message().program_instructions_iter(),
true,
!self
.feature_set
.is_active(&remove_deprecated_request_unit_ix::id()),
);
compute_budget_process_transaction_time.stop();
saturating_add_assign!(
timings
.execute_accessories
.compute_budget_process_transaction_us,
compute_budget_process_transaction_time.as_us()
);
if let Err(err) = process_transaction_result {
return TransactionExecutionResult::NotExecuted(err);
}
compute_budget
};

self.execute_loaded_transaction(
tx,
Expand Down Expand Up @@ -4864,6 +4874,7 @@ impl Bank {
lamports_per_signature: u64,
fee_structure: &FeeStructure,
use_default_units_per_instruction: bool,
support_request_units_deprecated: bool,
) -> u64 {
// Fee based on compute units and signatures
const BASE_CONGESTION: f64 = 5_000.0;
Expand All @@ -4879,6 +4890,7 @@ impl Bank {
.process_instructions(
message.program_instructions_iter(),
use_default_units_per_instruction,
support_request_units_deprecated,
)
.unwrap_or_default();
let prioritization_fee = prioritization_fee_details.get_fee();
Expand Down Expand Up @@ -4944,6 +4956,9 @@ impl Bank {
&self.fee_structure,
self.feature_set
.is_active(&use_default_units_in_fee_calculation::id()),
!self
.feature_set
.is_active(&remove_deprecated_request_unit_ix::id()),
);

// In case of instruction error, even though no accounts
Expand Down Expand Up @@ -10880,6 +10895,7 @@ pub(crate) mod tests {
.lamports_per_signature,
&FeeStructure::default(),
true,
false,
);

let (expected_fee_collected, expected_fee_burned) =
Expand Down Expand Up @@ -11061,6 +11077,7 @@ pub(crate) mod tests {
cheap_lamports_per_signature,
&FeeStructure::default(),
true,
false,
);
assert_eq!(
bank.get_balance(&mint_keypair.pubkey()),
Expand All @@ -11078,6 +11095,7 @@ pub(crate) mod tests {
expensive_lamports_per_signature,
&FeeStructure::default(),
true,
false,
);
assert_eq!(
bank.get_balance(&mint_keypair.pubkey()),
Expand Down Expand Up @@ -11193,6 +11211,7 @@ pub(crate) mod tests {
.lamports_per_signature,
&FeeStructure::default(),
true,
false,
) * 2
)
.0
Expand Down Expand Up @@ -17740,6 +17759,7 @@ pub(crate) mod tests {
..FeeStructure::default()
},
true,
false,
),
0
);
Expand All @@ -17754,6 +17774,7 @@ pub(crate) mod tests {
..FeeStructure::default()
},
true,
false,
),
1
);
Expand All @@ -17773,6 +17794,7 @@ pub(crate) mod tests {
..FeeStructure::default()
},
true,
false,
),
4
);
Expand All @@ -17792,7 +17814,7 @@ pub(crate) mod tests {
let message =
SanitizedMessage::try_from(Message::new(&[], Some(&Pubkey::new_unique()))).unwrap();
assert_eq!(
Bank::calculate_fee(&message, 1, &fee_structure, true),
Bank::calculate_fee(&message, 1, &fee_structure, true, false),
max_fee + lamports_per_signature
);

Expand All @@ -17804,7 +17826,7 @@ pub(crate) mod tests {
SanitizedMessage::try_from(Message::new(&[ix0, ix1], Some(&Pubkey::new_unique())))
.unwrap();
assert_eq!(
Bank::calculate_fee(&message, 1, &fee_structure, true),
Bank::calculate_fee(&message, 1, &fee_structure, true, false),
max_fee + 3 * lamports_per_signature
);

Expand Down Expand Up @@ -17837,7 +17859,7 @@ pub(crate) mod tests {
Some(&Pubkey::new_unique()),
))
.unwrap();
let fee = Bank::calculate_fee(&message, 1, &fee_structure, true);
let fee = Bank::calculate_fee(&message, 1, &fee_structure, true, false);
assert_eq!(
fee,
lamports_per_signature + prioritization_fee_details.get_fee()
Expand Down Expand Up @@ -17875,7 +17897,10 @@ pub(crate) mod tests {
Some(&key0),
))
.unwrap();
assert_eq!(Bank::calculate_fee(&message, 1, &fee_structure, true), 2);
assert_eq!(
Bank::calculate_fee(&message, 1, &fee_structure, true, false),
2
);

secp_instruction1.data = vec![0];
secp_instruction2.data = vec![10];
Expand All @@ -17884,7 +17909,10 @@ pub(crate) mod tests {
Some(&key0),
))
.unwrap();
assert_eq!(Bank::calculate_fee(&message, 1, &fee_structure, true), 11);
assert_eq!(
Bank::calculate_fee(&message, 1, &fee_structure, true, false),
11
);
}

#[test]
Expand Down
1 change: 1 addition & 0 deletions sdk/src/compute_budget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ crate::declare_id!("ComputeBudget111111111111111111111111111111");
)]
pub enum ComputeBudgetInstruction {
/// Deprecated
/// TODO: after feature remove_deprecated_request_unit_ix::id() is activated, replace it with 'unused'
RequestUnitsDeprecated {
/// Units to request
units: u32,
Expand Down
5 changes: 5 additions & 0 deletions sdk/src/feature_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,10 @@ pub mod cap_accounts_data_allocations_per_transaction {
solana_sdk::declare_id!("9gxu85LYRAcZL38We8MYJ4A9AwgBBPtVBAqebMcT1241");
}

pub mod remove_deprecated_request_unit_ix {
solana_sdk::declare_id!("EfhYd3SafzGT472tYQDUc4dPd2xdEfKs5fwkowUgVt4W");
}

lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
Expand Down Expand Up @@ -637,6 +641,7 @@ lazy_static! {
(stop_sibling_instruction_search_at_parent::id(), "stop the search in get_processed_sibling_instruction when the parent instruction is reached #27289"),
(vote_state_update_root_fix::id(), "fix root in vote state updates #27361"),
(cap_accounts_data_allocations_per_transaction::id(), "cap accounts data allocations per transaction #27375"),
(remove_deprecated_request_unit_ix::id(), "remove support for RequestUnitsDeprecated instruction #27500"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()
Expand Down

0 comments on commit d4f549b

Please sign in to comment.