v2.1: Fix builtin default cost dependents on migration (backport of #…

…3768) (#4415)

* Fix builtin default cost dependents on migration (#3768)

* Add public function to as placeholder to implement fetching default cost depends on feature_set;
Make AhashMap private

* Add BuiltinCost to help determine default cost based on migration status

* test

* fix - after migration feature activation, it should no longer be considered as builtin

* use lazy_static, avoid naked unwrap

* rename

* add comments to BUILINS

(cherry picked from commit d791c9a)

# Conflicts:
#	builtins-default-costs/src/lib.rs
#	cost-model/src/cost_model.rs

* fix merge conflicts

---------

Co-authored-by: Tao Zhu <[email protected]>
Co-authored-by: Tao Zhu <[email protected]>

builtins-default-costs/benches/builtin_instruction_costs.rs

@@ -2,22 +2,23 @@
 extern crate test;
 use {
     rand::Rng,
-    solana_builtins_default_costs::BUILTIN_INSTRUCTION_COSTS,
+    solana_builtins_default_costs::get_builtin_instruction_cost,
     solana_sdk::{
         address_lookup_table, bpf_loader, bpf_loader_deprecated, bpf_loader_upgradeable,
-        compute_budget, ed25519_program, loader_v4, pubkey::Pubkey, secp256k1_program,
+        compute_budget, ed25519_program, feature_set::FeatureSet, loader_v4, pubkey::Pubkey,
+        secp256k1_program,
     },
     test::Bencher,
 };
 
 struct BenchSetup {
     pubkeys: [Pubkey; 12],
+    feature_set: FeatureSet,
 }
 
 const NUM_TRANSACTIONS_PER_ITER: usize = 1024;
-const DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT: u32 = 200_000;
 
-fn setup() -> BenchSetup {
+fn setup(all_features_enabled: bool) -> BenchSetup {
     let pubkeys: [Pubkey; 12] = [
         solana_stake_program::id(),
         solana_config_program::id(),
@@ -33,23 +34,39 @@ fn setup() -> BenchSetup {
         ed25519_program::id(),
     ];
 
-    BenchSetup { pubkeys }
+    let feature_set = if all_features_enabled {
+        FeatureSet::all_enabled()
+    } else {
+        FeatureSet::default()
+    };
+
+    BenchSetup {
+        pubkeys,
+        feature_set,
+    }
+}
+
+fn do_hash_find(setup: &BenchSetup) {
+    for _t in 0..NUM_TRANSACTIONS_PER_ITER {
+        let idx = rand::thread_rng().gen_range(0..setup.pubkeys.len());
+        get_builtin_instruction_cost(&setup.pubkeys[idx], &setup.feature_set);
+    }
+}
+
+#[bench]
+fn bench_hash_find_builtins_not_migrated(bencher: &mut Bencher) {
+    let bench_setup = setup(false);
+
+    bencher.iter(|| {
+        do_hash_find(&bench_setup);
+    });
 }
 
 #[bench]
-fn bench_hash_find(bencher: &mut Bencher) {
-    let BenchSetup { pubkeys } = setup();
+fn bench_hash_find_builtins_migrated(bencher: &mut Bencher) {
+    let bench_setup = setup(true);
 
     bencher.iter(|| {
-        for _t in 0..NUM_TRANSACTIONS_PER_ITER {
-            let idx = rand::thread_rng().gen_range(0..pubkeys.len());
-            let ix_execution_cost =
-                if let Some(builtin_cost) = BUILTIN_INSTRUCTION_COSTS.get(&pubkeys[idx]) {
-                    *builtin_cost
-                } else {
-                    u64::from(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT)
-                };
-            assert!(ix_execution_cost != DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64);
-        }
+        do_hash_find(&bench_setup);
     });
 }

builtins-default-costs/src/lib.rs

@@ -5,11 +5,25 @@ use {
     lazy_static::lazy_static,
     solana_sdk::{
         address_lookup_table, bpf_loader, bpf_loader_deprecated, bpf_loader_upgradeable,
-        compute_budget, ed25519_program, loader_v4, pubkey::Pubkey, secp256k1_program,
+        compute_budget, ed25519_program,
+        feature_set::{self, FeatureSet},
+        loader_v4,
+        pubkey::Pubkey,
+        secp256k1_program,
     },
 };
 
-// Number of compute units for each built-in programs
+/// DEVELOPER: when a builtin is migrated to sbpf, please add its corresponding
+/// migration feature ID to BUILTIN_INSTRUCTION_COSTS, so the builtin's default
+/// cost can be determined properly based on feature status.
+/// When migration completed, eg the feature gate is enabled everywhere, please
+/// remove that builtin entry from BUILTIN_INSTRUCTION_COSTS.
+#[derive(Clone)]
+struct BuiltinCost {
+    native_cost: u64,
+    core_bpf_migration_feature: Option<Pubkey>,
+}
+
 lazy_static! {
     /// Number of compute units for each built-in programs
     ///
@@ -20,21 +34,95 @@ lazy_static! {
     /// calculate the cost of a transaction which is used in replay to enforce
     /// block cost limits as of
     /// https://github.com/solana-labs/solana/issues/29595.
-    pub static ref BUILTIN_INSTRUCTION_COSTS: AHashMap<Pubkey, u64> = [
-        (solana_stake_program::id(), solana_stake_program::stake_instruction::DEFAULT_COMPUTE_UNITS),
-        (solana_config_program::id(), solana_config_program::config_processor::DEFAULT_COMPUTE_UNITS),
-        (solana_vote_program::id(), solana_vote_program::vote_processor::DEFAULT_COMPUTE_UNITS),
-        (solana_system_program::id(), solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS),
-        (compute_budget::id(), solana_compute_budget_program::DEFAULT_COMPUTE_UNITS),
-        (address_lookup_table::program::id(), solana_address_lookup_table_program::processor::DEFAULT_COMPUTE_UNITS),
-        (bpf_loader_upgradeable::id(), solana_bpf_loader_program::UPGRADEABLE_LOADER_COMPUTE_UNITS),
-        (bpf_loader_deprecated::id(), solana_bpf_loader_program::DEPRECATED_LOADER_COMPUTE_UNITS),
-        (bpf_loader::id(), solana_bpf_loader_program::DEFAULT_LOADER_COMPUTE_UNITS),
-        (loader_v4::id(), solana_loader_v4_program::DEFAULT_COMPUTE_UNITS),
-        // Note: These are precompile, run directly in bank during sanitizing;
-        (secp256k1_program::id(), 0),
-        (ed25519_program::id(), 0),
-        // DO NOT ADD MORE ENTRIES TO THIS MAP
+    static ref BUILTIN_INSTRUCTION_COSTS: AHashMap<Pubkey, BuiltinCost> = [
+    (
+        solana_stake_program::id(),
+        BuiltinCost {
+            native_cost: solana_stake_program::stake_instruction::DEFAULT_COMPUTE_UNITS,
+            core_bpf_migration_feature: Some(feature_set::migrate_stake_program_to_core_bpf::id()),
+        },
+    ),
+    (
+        solana_config_program::id(),
+        BuiltinCost {
+            native_cost: solana_config_program::config_processor::DEFAULT_COMPUTE_UNITS,
+            core_bpf_migration_feature: Some(feature_set::migrate_config_program_to_core_bpf::id()),
+        },
+    ),
+    (
+        solana_vote_program::id(),
+        BuiltinCost {
+            native_cost: solana_vote_program::vote_processor::DEFAULT_COMPUTE_UNITS,
+            core_bpf_migration_feature: None,
+        },
+    ),
+    (
+        solana_system_program::id(),
+        BuiltinCost {
+            native_cost: solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS,
+            core_bpf_migration_feature: None,
+        },
+    ),
+    (
+        compute_budget::id(),
+        BuiltinCost {
+            native_cost: solana_compute_budget_program::DEFAULT_COMPUTE_UNITS,
+            core_bpf_migration_feature: None,
+        },
+    ),
+    (
+        address_lookup_table::program::id(),
+        BuiltinCost {
+            native_cost: solana_address_lookup_table_program::processor::DEFAULT_COMPUTE_UNITS,
+            core_bpf_migration_feature: Some(
+                feature_set::migrate_address_lookup_table_program_to_core_bpf::id(),
+            ),
+        },
+    ),
+    (
+        bpf_loader_upgradeable::id(),
+        BuiltinCost {
+            native_cost: solana_bpf_loader_program::UPGRADEABLE_LOADER_COMPUTE_UNITS,
+            core_bpf_migration_feature: None,
+        },
+    ),
+    (
+        bpf_loader_deprecated::id(),
+        BuiltinCost {
+            native_cost: solana_bpf_loader_program::DEPRECATED_LOADER_COMPUTE_UNITS,
+            core_bpf_migration_feature: None,
+        },
+    ),
+    (
+        bpf_loader::id(),
+        BuiltinCost {
+            native_cost: solana_bpf_loader_program::DEFAULT_LOADER_COMPUTE_UNITS,
+            core_bpf_migration_feature: None,
+        },
+    ),
+    (
+        loader_v4::id(),
+        BuiltinCost {
+            native_cost: solana_loader_v4_program::DEFAULT_COMPUTE_UNITS,
+            core_bpf_migration_feature: None,
+        },
+    ),
+    // Note: These are precompile, run directly in bank during sanitizing;
+    (
+        secp256k1_program::id(),
+        BuiltinCost {
+            native_cost: 0,
+            core_bpf_migration_feature: None,
+        },
+    ),
+    (
+        ed25519_program::id(),
+        BuiltinCost {
+            native_cost: 0,
+            core_bpf_migration_feature: None,
+        },
+    ),
+    // DO NOT ADD MORE ENTRIES TO THIS MAP
     ]
     .iter()
     .cloned()
@@ -55,7 +143,62 @@ lazy_static! {
     };
 }
 
+pub fn get_builtin_instruction_cost<'a>(
+    program_id: &'a Pubkey,
+    feature_set: &'a FeatureSet,
+) -> Option<u64> {
+    BUILTIN_INSTRUCTION_COSTS
+        .get(program_id)
+        .filter(
+            // Returns true if builtin program id has no core_bpf_migration_feature or feature is not activated;
+            // otherwise returns false because it's not considered as builtin
+            |builtin_cost| -> bool {
+                builtin_cost
+                    .core_bpf_migration_feature
+                    .map(|feature_id| !feature_set.is_active(&feature_id))
+                    .unwrap_or(true)
+            },
+        )
+        .map(|builtin_cost| builtin_cost.native_cost)
+}
+
 #[inline]
 pub fn is_builtin_program(program_id: &Pubkey) -> bool {
     BUILTIN_INSTRUCTION_COSTS.contains_key(program_id)
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn test_get_builtin_instruction_cost() {
+        // use native cost if no migration planned
+        assert_eq!(
+            Some(solana_compute_budget_program::DEFAULT_COMPUTE_UNITS),
+            get_builtin_instruction_cost(&compute_budget::id(), &FeatureSet::all_enabled())
+        );
+
+        // use native cost if migration is planned but not activated
+        assert_eq!(
+            Some(solana_stake_program::stake_instruction::DEFAULT_COMPUTE_UNITS),
+            get_builtin_instruction_cost(&solana_stake_program::id(), &FeatureSet::default())
+        );
+
+        // None if migration is planned and activated, in which case, it's no longer builtin
+        assert!(get_builtin_instruction_cost(
+            &solana_stake_program::id(),
+            &FeatureSet::all_enabled()
+        )
+        .is_none());
+
+        // None if not builtin
+        assert!(
+            get_builtin_instruction_cost(&Pubkey::new_unique(), &FeatureSet::default()).is_none()
+        );
+        assert!(
+            get_builtin_instruction_cost(&Pubkey::new_unique(), &FeatureSet::all_enabled())
+                .is_none()
+        );
+    }
+}

core/src/banking_stage/packet_filter.rs

@@ -1,10 +1,19 @@
 use {
     super::immutable_deserialized_packet::ImmutableDeserializedPacket,
-    solana_builtins_default_costs::BUILTIN_INSTRUCTION_COSTS,
-    solana_sdk::{ed25519_program, saturating_add_assign, secp256k1_program},
+    lazy_static::lazy_static,
+    solana_builtins_default_costs::get_builtin_instruction_cost,
+    solana_sdk::{
+        ed25519_program, feature_set::FeatureSet, saturating_add_assign, secp256k1_program,
+    },
     thiserror::Error,
 };
 
+lazy_static! {
+    // To calculate the static_builtin_cost_sum conservatively, an all-enabled dummy feature_set
+    // is used. It lowers required minimal compute_unit_limit, aligns with future versions.
+    static ref FEATURE_SET: FeatureSet = FeatureSet::all_enabled();
+}
+
 #[derive(Debug, Error, PartialEq)]
 pub enum PacketFilterFailure {
     #[error("Insufficient compute unit limit")]
@@ -22,8 +31,8 @@ impl ImmutableDeserializedPacket {
     pub fn check_insufficent_compute_unit_limit(&self) -> Result<(), PacketFilterFailure> {
         let mut static_builtin_cost_sum: u64 = 0;
         for (program_id, _) in self.transaction().get_message().program_instructions_iter() {
-            if let Some(ix_cost) = BUILTIN_INSTRUCTION_COSTS.get(program_id) {
-                saturating_add_assign!(static_builtin_cost_sum, *ix_cost);
+            if let Some(ix_cost) = get_builtin_instruction_cost(program_id, &FEATURE_SET) {
+                saturating_add_assign!(static_builtin_cost_sum, ix_cost);
             }
         }
 

cost-model/src/cost_model.rs

@@ -7,7 +7,7 @@
 
 use {
     crate::{block_cost_limits::*, transaction_cost::*},
-    solana_builtins_default_costs::BUILTIN_INSTRUCTION_COSTS,
+    solana_builtins_default_costs::get_builtin_instruction_cost,
     solana_compute_budget::compute_budget_limits::{
         DEFAULT_HEAP_COST, DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, MAX_COMPUTE_UNIT_LIMIT,
     },
@@ -193,8 +193,8 @@ impl CostModel {
 
         for (program_id, instruction) in transaction.program_instructions_iter() {
             let ix_execution_cost =
-                if let Some(builtin_cost) = BUILTIN_INSTRUCTION_COSTS.get(program_id) {
-                    *builtin_cost
+                if let Some(builtin_cost) = get_builtin_instruction_cost(program_id, feature_set) {
+                    builtin_cost
                 } else {
                     has_user_space_instructions = true;
                     u64::from(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT)

runtime/src/bank/builtins/mod.rs

@@ -31,6 +31,11 @@ macro_rules! testable_prototype {
     };
 }
 
+/// DEVELOPER: when a builtin is migrated to sbpf, please add its corresponding
+/// migration feature ID to solana-builtin-default-costs::BUILTIN_INSTRUCTION_COSTS,
+/// so the builtin's default cost can be determined properly based on feature status.
+/// When migration completed, and the feature gate is enabled everywhere, please
+/// remove that builtin entry from solana-builtin-default-costs::BUILTIN_INSTRUCTION_COSTS.
 pub static BUILTINS: &[BuiltinPrototype] = &[
     testable_prototype!(BuiltinPrototype {
         core_bpf_migration_config: None,