Add feature gate to remove support for RequestUnitsDeprecated instruction (#27503)

* feature gate: remove support for RequestUnitsDeprecated instruction #27500

* review update: stop support deprecated ix for prioritization

* Apply suggestions from code review

Co-authored-by: Justin Starry <justin.m.starry@gmail.com>

Co-authored-by: Justin Starry <justin.m.starry@gmail.com>

program-runtime/src/compute_budget.rs

@@ -130,6 +130,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;
@@ -148,7 +149,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);
                         }
@@ -243,8 +244,11 @@ mod tests {
                 Hash::default(),
             ));
             let mut compute_budget = ComputeBudget::default();
-            let result =
+            let result = compute_budget.process_instructions(
-                compute_budget.process_instructions(tx.message().program_instructions_iter(), true);
+                tx.message().program_instructions_iter(),
+                true,
+                false, /*not support request_units_deprecated*/
+            );
             assert_eq!($expected_result, result);
             assert_eq!(compute_budget, $expected_budget);
         };
@@ -491,5 +495,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()
+        );
     }
 }

program-runtime/src/prioritization_fee.rs

@@ -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),
 }
 
@@ -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

programs/bpf/tests/programs.rs

@@ -3798,6 +3798,7 @@ fn test_program_fees() {
         congestion_multiplier,
         &fee_structure,
         true,
+        false,
     );
     bank_client
         .send_and_confirm_message(&[&mint_keypair], message)
@@ -3819,6 +3820,7 @@ fn test_program_fees() {
         congestion_multiplier,
         &fee_structure,
         true,
+        false,
     );
     assert!(expected_normal_fee < expected_prioritized_fee);
 

runtime/src/accounts.rs

@@ -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,
@@ -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);
@@ -1674,6 +1678,7 @@ mod tests {
             lamports_per_signature,
             &FeeStructure::default(),
             true,
+            false,
         );
         assert_eq!(fee, lamports_per_signature);
 

runtime/src/bank.rs

@@ -112,7 +112,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},
@@ -3728,6 +3728,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()),
         ))
     }
 
@@ -3769,6 +3772,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()),
         )
     }
 
@@ -4570,30 +4576,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) =
+                    let compute_budget =
-                        self.runtime_config.compute_budget
+                        if let Some(compute_budget) = self.runtime_config.compute_budget {
-                    {
+                            compute_budget
-                        compute_budget
+                        } else {
-                    } else {
+                            let mut compute_budget =
-                        let mut compute_budget =
+                                ComputeBudget::new(compute_budget::MAX_COMPUTE_UNIT_LIMIT as u64);
-                            ComputeBudget::new(compute_budget::MAX_COMPUTE_UNIT_LIMIT as u64);
 
-                        let mut compute_budget_process_transaction_time =
+                            let mut compute_budget_process_transaction_time =
-                            Measure::start("compute_budget_process_transaction_time");
+                                Measure::start("compute_budget_process_transaction_time");
-                        let process_transaction_result = compute_budget
+                            let process_transaction_result = compute_budget.process_instructions(
-                            .process_instructions(tx.message().program_instructions_iter(), true);
+                                tx.message().program_instructions_iter(),
-                        compute_budget_process_transaction_time.stop();
+                                true,
-                        saturating_add_assign!(
+                                !self
-                            timings
+                                    .feature_set
-                                .execute_accessories
+                                    .is_active(&remove_deprecated_request_unit_ix::id()),
-                                .compute_budget_process_transaction_us,
+                            );
-                            compute_budget_process_transaction_time.as_us()
+                            compute_budget_process_transaction_time.stop();
-                        );
+                            saturating_add_assign!(
-                        if let Err(err) = process_transaction_result {
+                                timings
-                            return TransactionExecutionResult::NotExecuted(err);
+                                    .execute_accessories
-                        }
+                                    .compute_budget_process_transaction_us,
-                        compute_budget
+                                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,
@@ -4866,6 +4876,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;
@@ -4881,6 +4892,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();
@@ -4946,6 +4958,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
@@ -10937,6 +10952,7 @@ pub(crate) mod tests {
                 .lamports_per_signature,
             &FeeStructure::default(),
             true,
+            false,
         );
 
         let (expected_fee_collected, expected_fee_burned) =
@@ -11118,6 +11134,7 @@ pub(crate) mod tests {
             cheap_lamports_per_signature,
             &FeeStructure::default(),
             true,
+            false,
         );
         assert_eq!(
             bank.get_balance(&mint_keypair.pubkey()),
@@ -11135,6 +11152,7 @@ pub(crate) mod tests {
             expensive_lamports_per_signature,
             &FeeStructure::default(),
             true,
+            false,
         );
         assert_eq!(
             bank.get_balance(&mint_keypair.pubkey()),
@@ -11250,6 +11268,7 @@ pub(crate) mod tests {
                                 .lamports_per_signature,
                             &FeeStructure::default(),
                             true,
+                            false,
                         ) * 2
                     )
                     .0
@@ -17797,6 +17816,7 @@ pub(crate) mod tests {
                     ..FeeStructure::default()
                 },
                 true,
+                false,
             ),
             0
         );
@@ -17811,6 +17831,7 @@ pub(crate) mod tests {
                     ..FeeStructure::default()
                 },
                 true,
+                false,
             ),
             1
         );
@@ -17830,6 +17851,7 @@ pub(crate) mod tests {
                     ..FeeStructure::default()
                 },
                 true,
+                false,
             ),
             4
         );
@@ -17849,7 +17871,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
         );
 
@@ -17861,7 +17883,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
         );
 
@@ -17894,7 +17916,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()
@@ -17932,7 +17954,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];
@@ -17941,7 +17966,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]

runtime/src/transaction_priority_details.rs

@@ -23,7 +23,8 @@ pub trait GetTransactionPriorityDetails {
         let prioritization_fee_details = compute_budget
             .process_instructions(
                 instructions,
-                true, // use default units per instruction
+                true,  // use default units per instruction
+                false, // stop supporting prioritization by request_units_deprecated instruction
             )
             .ok()?;
         Some(TransactionPriorityDetails {

sdk/src/compute_budget.rs

@@ -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,

sdk/src/feature_set.rs

@@ -518,6 +518,10 @@ pub mod epoch_accounts_hash {
     solana_sdk::declare_id!("5GpmAKxaGsWWbPp4bNXFLJxZVvG92ctxf7jQnzTQjF3n");
 }
 
+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> = [
@@ -642,6 +646,7 @@ lazy_static! {
         (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"),
         (epoch_accounts_hash::id(), "enable epoch accounts hash calculation #27539"),
+        (remove_deprecated_request_unit_ix::id(), "remove support for RequestUnitsDeprecated instruction #27500"),
         /*************** ADD NEW FEATURES HERE ***************/
     ]
     .iter()