Ver Fonte

remove get_builtin_instruction_cost (#7231)

* remove get_builtin_instruction_cost
* simplify BuiltinCost::NotMigrating
Andrew Fitzgerald há 3 meses atrás
pai
commit
21e149ee55

+ 0 - 74
builtins-default-costs/benches/builtin_instruction_costs.rs

@@ -1,74 +0,0 @@
-#![feature(test)]
-extern crate test;
-use {
-    agave_feature_set::FeatureSet,
-    rand::Rng,
-    solana_builtins_default_costs::get_builtin_instruction_cost,
-    solana_pubkey::Pubkey,
-    solana_sdk_ids::{
-        address_lookup_table, bpf_loader, bpf_loader_deprecated, bpf_loader_upgradeable,
-        compute_budget, config, ed25519_program, loader_v4, secp256k1_program, stake,
-        system_program, vote,
-    },
-    test::Bencher,
-};
-
-struct BenchSetup {
-    pubkeys: [Pubkey; 12],
-    feature_set: FeatureSet,
-}
-
-const NUM_TRANSACTIONS_PER_ITER: usize = 1024;
-
-fn setup(all_features_enabled: bool) -> BenchSetup {
-    let pubkeys: [Pubkey; 12] = [
-        stake::id(),
-        config::id(),
-        vote::id(),
-        system_program::id(),
-        compute_budget::id(),
-        address_lookup_table::id(),
-        bpf_loader_upgradeable::id(),
-        bpf_loader_deprecated::id(),
-        bpf_loader::id(),
-        loader_v4::id(),
-        secp256k1_program::id(),
-        ed25519_program::id(),
-    ];
-
-    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_builtins_migrated(bencher: &mut Bencher) {
-    let bench_setup = setup(true);
-
-    bencher.iter(|| {
-        do_hash_find(&bench_setup);
-    });
-}

+ 14 - 117
builtins-default-costs/src/lib.rs

@@ -1,7 +1,7 @@
 #![cfg_attr(feature = "frozen-abi", feature(min_specialization))]
 #![allow(clippy::arithmetic_side_effects)]
 use {
-    agave_feature_set::{self as feature_set, FeatureSet},
+    agave_feature_set::{self as feature_set},
     ahash::AHashMap,
     solana_pubkey::Pubkey,
     solana_sdk_ids::{
@@ -12,7 +12,6 @@ use {
 
 #[derive(Clone)]
 pub struct MigratingBuiltinCost {
-    native_cost: u64,
     core_bpf_migration_feature: Pubkey,
     // encoding positional information explicitly for migration feature item,
     // its value must be correctly corresponding to this object's position
@@ -21,11 +20,6 @@ pub struct MigratingBuiltinCost {
     position: usize,
 }
 
-#[derive(Clone)]
-pub struct NotMigratingBuiltinCost {
-    native_cost: u64,
-}
-
 /// DEVELOPER: when a builtin is migrated to sbpf, please add its corresponding
 /// migration feature ID to BUILTIN_INSTRUCTION_COSTS, and move it from
 /// NON_MIGRATING_BUILTINS_COSTS to MIGRATING_BUILTINS_COSTS, so the builtin's
@@ -35,41 +29,24 @@ pub struct NotMigratingBuiltinCost {
 #[derive(Clone)]
 pub enum BuiltinCost {
     Migrating(MigratingBuiltinCost),
-    NotMigrating(NotMigratingBuiltinCost),
+    NotMigrating,
 }
 
 impl BuiltinCost {
-    fn native_cost(&self) -> u64 {
-        match self {
-            BuiltinCost::Migrating(MigratingBuiltinCost { native_cost, .. }) => *native_cost,
-            BuiltinCost::NotMigrating(NotMigratingBuiltinCost { native_cost }) => *native_cost,
-        }
-    }
-
     fn core_bpf_migration_feature(&self) -> Option<&Pubkey> {
         match self {
             BuiltinCost::Migrating(MigratingBuiltinCost {
                 core_bpf_migration_feature,
                 ..
             }) => Some(core_bpf_migration_feature),
-            BuiltinCost::NotMigrating(_) => None,
+            BuiltinCost::NotMigrating => None,
         }
     }
 
     fn position(&self) -> Option<usize> {
         match self {
             BuiltinCost::Migrating(MigratingBuiltinCost { position, .. }) => Some(*position),
-            BuiltinCost::NotMigrating(_) => None,
-        }
-    }
-
-    fn has_migrated(&self, feature_set: &FeatureSet) -> bool {
-        match self {
-            BuiltinCost::Migrating(MigratingBuiltinCost {
-                core_bpf_migration_feature,
-                ..
-            }) => feature_set.is_active(core_bpf_migration_feature),
-            BuiltinCost::NotMigrating(_) => false,
+            BuiltinCost::NotMigrating => None,
         }
     }
 }
@@ -110,64 +87,21 @@ static_assertions::const_assert_eq!(
 pub const MIGRATING_BUILTINS_COSTS: &[(Pubkey, BuiltinCost)] = &[(
     stake::id(),
     BuiltinCost::Migrating(MigratingBuiltinCost {
-        native_cost: solana_stake_program::stake_instruction::DEFAULT_COMPUTE_UNITS,
         core_bpf_migration_feature: feature_set::migrate_stake_program_to_core_bpf::id(),
         position: 0,
     }),
 )];
 
 const NON_MIGRATING_BUILTINS_COSTS: &[(Pubkey, BuiltinCost)] = &[
-    (
-        vote::id(),
-        BuiltinCost::NotMigrating(NotMigratingBuiltinCost {
-            native_cost: solana_vote_program::vote_processor::DEFAULT_COMPUTE_UNITS,
-        }),
-    ),
-    (
-        system_program::id(),
-        BuiltinCost::NotMigrating(NotMigratingBuiltinCost {
-            native_cost: solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS,
-        }),
-    ),
-    (
-        compute_budget::id(),
-        BuiltinCost::NotMigrating(NotMigratingBuiltinCost {
-            native_cost: solana_compute_budget_program::DEFAULT_COMPUTE_UNITS,
-        }),
-    ),
-    (
-        bpf_loader_upgradeable::id(),
-        BuiltinCost::NotMigrating(NotMigratingBuiltinCost {
-            native_cost: solana_bpf_loader_program::UPGRADEABLE_LOADER_COMPUTE_UNITS,
-        }),
-    ),
-    (
-        bpf_loader_deprecated::id(),
-        BuiltinCost::NotMigrating(NotMigratingBuiltinCost {
-            native_cost: solana_bpf_loader_program::DEPRECATED_LOADER_COMPUTE_UNITS,
-        }),
-    ),
-    (
-        bpf_loader::id(),
-        BuiltinCost::NotMigrating(NotMigratingBuiltinCost {
-            native_cost: solana_bpf_loader_program::DEFAULT_LOADER_COMPUTE_UNITS,
-        }),
-    ),
-    (
-        loader_v4::id(),
-        BuiltinCost::NotMigrating(NotMigratingBuiltinCost {
-            native_cost: solana_loader_v4_program::DEFAULT_COMPUTE_UNITS,
-        }),
-    ),
-    // Note: These are precompile, run directly in bank during sanitizing;
-    (
-        secp256k1_program::id(),
-        BuiltinCost::NotMigrating(NotMigratingBuiltinCost { native_cost: 0 }),
-    ),
-    (
-        ed25519_program::id(),
-        BuiltinCost::NotMigrating(NotMigratingBuiltinCost { native_cost: 0 }),
-    ),
+    (vote::id(), BuiltinCost::NotMigrating),
+    (system_program::id(), BuiltinCost::NotMigrating),
+    (compute_budget::id(), BuiltinCost::NotMigrating),
+    (bpf_loader_upgradeable::id(), BuiltinCost::NotMigrating),
+    (bpf_loader_deprecated::id(), BuiltinCost::NotMigrating),
+    (bpf_loader::id(), BuiltinCost::NotMigrating),
+    (loader_v4::id(), BuiltinCost::NotMigrating),
+    (secp256k1_program::id(), BuiltinCost::NotMigrating),
+    (ed25519_program::id(), BuiltinCost::NotMigrating),
 ];
 
 /// A table of 256 booleans indicates whether the first `u8` of a Pubkey exists in
@@ -182,16 +116,6 @@ pub static MAYBE_BUILTIN_KEY: std::sync::LazyLock<[bool; 256]> = std::sync::Lazy
     temp_table
 });
 
-pub fn get_builtin_instruction_cost<'a>(
-    program_id: &'a Pubkey,
-    feature_set: &'a FeatureSet,
-) -> Option<u64> {
-    BUILTIN_INSTRUCTION_COSTS
-        .get(program_id)
-        .filter(|builtin_cost| !builtin_cost.has_migrated(feature_set))
-        .map(|builtin_cost| builtin_cost.native_cost())
-}
-
 pub enum BuiltinMigrationFeatureIndex {
     NotBuiltin,
     BuiltinNoMigrationFeature,
@@ -220,7 +144,7 @@ const fn validate_position(migrating_builtins: &[(Pubkey, BuiltinCost)]) {
                 position == index,
                 "migration feture must exist and at correct position"
             ),
-            BuiltinCost::NotMigrating(_) => {
+            BuiltinCost::NotMigrating => {
                 panic!("migration feture must exist and at correct position")
             }
         }
@@ -266,33 +190,6 @@ mod test {
             .all(|(_, c)| c.core_bpf_migration_feature().is_none()));
     }
 
-    #[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(&stake::id(), &FeatureSet::default())
-        );
-
-        // None if migration is planned and activated, in which case, it's no longer builtin
-        assert!(get_builtin_instruction_cost(&stake::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()
-        );
-    }
-
     #[test]
     fn test_get_builtin_migration_feature_index() {
         assert!(matches!(