From 1c5c3d7cc5f5568ba29585003e91f4bd37f714ce Mon Sep 17 00:00:00 2001 From: Joe Caulfield Date: Wed, 11 Dec 2024 19:15:48 +0900 Subject: [PATCH] integrate migration cost modeling --- builtins/src/cost_modeling.rs | 69 +++++++++ .../src/builtin_programs_filter.rs | 45 +++++- .../src/compute_budget_instruction_details.rs | 137 ++++++++++++++++++ 3 files changed, 248 insertions(+), 3 deletions(-) diff --git a/builtins/src/cost_modeling.rs b/builtins/src/cost_modeling.rs index a23161d056861e..45559854c80083 100644 --- a/builtins/src/cost_modeling.rs +++ b/builtins/src/cost_modeling.rs @@ -9,6 +9,9 @@ use { solana_sdk_ids::{ed25519_program, secp256k1_program}, }; +// Hehe, if you change me, I will break things... +pub const NUM_COST_MODELED_BUILTINS_WITH_MIGRATIONS: usize = 3; + /// Configuration for cost modeling of a builtin program. #[derive(Debug)] pub enum CostModelingConfig { @@ -21,11 +24,24 @@ pub enum CostModelingConfig { NotCostModeled, } +impl CostModelingConfig { + /// Returns `true` if the builtin program is cost modeled. + pub fn is_cost_modeled(&self) -> bool { + matches!(self, Self::CostModeled { .. }) + } +} + struct BuiltinCost { default_cost: u64, core_bpf_migration_feature: Option, } +#[derive(Copy, Clone, Default)] +struct BuiltinWithMigration { + program_id: Pubkey, + feature_id: Pubkey, +} + lazy_static! { static ref BUILTIN_INSTRUCTION_COSTS: AHashMap = BUILTINS .iter() @@ -82,6 +98,27 @@ lazy_static! { }; } +lazy_static! { + // This lazy-static list is designed to panic if any builtin migrations + // are changed without updating `NUM_COST_MODELED_BUILTINS_WITH_MIGRATIONS`. + static ref COST_MODELED_BUILTINS_WITH_MIGRATIONS: [BuiltinWithMigration; NUM_COST_MODELED_BUILTINS_WITH_MIGRATIONS] = { + let mut temp = [BuiltinWithMigration::default(); NUM_COST_MODELED_BUILTINS_WITH_MIGRATIONS]; + let mut i: usize = 0; + for builtin in BUILTINS.iter() { + if builtin.cost_modeling_config.is_cost_modeled() { + if let Some(migration_config) = &builtin.core_bpf_migration_config { + temp[i] = BuiltinWithMigration { + program_id: builtin.program_id, + feature_id: migration_config.feature_id, + }; + i = i.saturating_add(1); + } + } + } + temp + }; +} + pub fn get_builtin_instruction_cost<'a>( program_id: &'a Pubkey, feature_set: &'a FeatureSet, @@ -107,10 +144,42 @@ pub fn is_builtin_program(program_id: &Pubkey) -> bool { BUILTIN_INSTRUCTION_COSTS.contains_key(program_id) } +/// Returns the index of a builtin in `COST_MODELED_BUILTINS_WITH_MIGRATIONS`, +/// if it exists in the list. +pub fn get_migration_feature_index(program_id: &Pubkey) -> Option { + COST_MODELED_BUILTINS_WITH_MIGRATIONS + .iter() + .position(|builtin| builtin.program_id == *program_id) +} + +/// Returns the feature ID of a builtin in `COST_MODELED_BUILTINS_WITH_MIGRATIONS` +/// by index. Panics if the index is out of bounds. +pub fn get_migration_feature_id(index: usize) -> &'static Pubkey { + &COST_MODELED_BUILTINS_WITH_MIGRATIONS[index].feature_id +} + +/// Returns the index of a builtin in `COST_MODELED_BUILTINS_WITH_MIGRATIONS` +/// by feature ID. If the feature ID is not found, returns `None`. +pub fn get_migration_feature_index_from_feature_id(feature_id: &Pubkey) -> Option { + COST_MODELED_BUILTINS_WITH_MIGRATIONS + .iter() + .position(|builtin| builtin.feature_id == *feature_id) +} + #[cfg(test)] mod test { use super::*; + #[test] + fn test_cost_modeled_builtins_with_migrations_compiles() { + // This test is a compile-time check to ensure that the number of + // cost-modeled builtins with migration features matches the constant. + assert_eq!( + COST_MODELED_BUILTINS_WITH_MIGRATIONS.len(), + NUM_COST_MODELED_BUILTINS_WITH_MIGRATIONS + ); + } + #[test] fn test_maybe_builtin_key() { let check = |key: &Pubkey, expected: bool| { diff --git a/compute-budget-instruction/src/builtin_programs_filter.rs b/compute-budget-instruction/src/builtin_programs_filter.rs index d802aa5f2604f8..3668abc143c71c 100644 --- a/compute-budget-instruction/src/builtin_programs_filter.rs +++ b/compute-budget-instruction/src/builtin_programs_filter.rs @@ -1,5 +1,7 @@ use { - solana_builtins::cost_modeling::{is_builtin_program, MAYBE_BUILTIN_KEY}, + solana_builtins::cost_modeling::{ + get_migration_feature_index, is_builtin_program, MAYBE_BUILTIN_KEY, + }, solana_sdk::{packet::PACKET_DATA_SIZE, pubkey::Pubkey}, }; @@ -10,6 +12,9 @@ pub const FILTER_SIZE: u8 = (PACKET_DATA_SIZE / core::mem::size_of::()) pub(crate) enum ProgramKind { NotBuiltin, Builtin, + MigratingBuiltin { + core_bpf_migration_feature_index: usize, + }, } pub(crate) struct BuiltinProgramsFilter { @@ -40,7 +45,11 @@ impl BuiltinProgramsFilter { return ProgramKind::NotBuiltin; } - if is_builtin_program(program_id) { + if let Some(index) = get_migration_feature_index(program_id) { + ProgramKind::MigratingBuiltin { + core_bpf_migration_feature_index: index, + } + } else if is_builtin_program(program_id) { ProgramKind::Builtin } else { ProgramKind::NotBuiltin @@ -50,7 +59,10 @@ impl BuiltinProgramsFilter { #[cfg(test)] mod test { - use super::*; + use { + super::*, solana_builtins::cost_modeling::get_migration_feature_index_from_feature_id, + solana_sdk::feature_set, + }; const DUMMY_PROGRAM_ID: &str = "dummmy1111111111111111111111111111111111111"; @@ -92,6 +104,33 @@ mod test { test_store.get_program_kind(index, &solana_sdk::compute_budget::id()), ProgramKind::Builtin, ); + + // migrating builtins + for (migrating_builtin_pubkey, migration_feature_id) in [ + ( + solana_sdk::stake::program::id(), + feature_set::migrate_stake_program_to_core_bpf::id(), + ), + ( + solana_sdk::config::program::id(), + feature_set::migrate_config_program_to_core_bpf::id(), + ), + ( + solana_sdk::address_lookup_table::program::id(), + feature_set::migrate_address_lookup_table_program_to_core_bpf::id(), + ), + ] { + index += 1; + assert_eq!( + test_store.get_program_kind(index, &migrating_builtin_pubkey), + ProgramKind::MigratingBuiltin { + core_bpf_migration_feature_index: get_migration_feature_index_from_feature_id( + &migration_feature_id + ) + .unwrap(), + } + ); + } } #[test] diff --git a/compute-budget-instruction/src/compute_budget_instruction_details.rs b/compute-budget-instruction/src/compute_budget_instruction_details.rs index dba59b8f343d13..35624d4d5d1637 100644 --- a/compute-budget-instruction/src/compute_budget_instruction_details.rs +++ b/compute-budget-instruction/src/compute_budget_instruction_details.rs @@ -3,6 +3,9 @@ use { builtin_programs_filter::{BuiltinProgramsFilter, ProgramKind}, compute_budget_program_id_filter::ComputeBudgetProgramIdFilter, }, + solana_builtins::cost_modeling::{ + get_migration_feature_id, NUM_COST_MODELED_BUILTINS_WITH_MIGRATIONS, + }, solana_compute_budget::compute_budget_limits::*, solana_sdk::{ borsh1::try_from_slice_unchecked, @@ -17,6 +20,24 @@ use { std::num::NonZeroU32, }; +#[cfg_attr(test, derive(Eq, PartialEq))] +#[cfg_attr(feature = "dev-context-only-utils", derive(Clone))] +#[derive(Debug)] +struct MigrationBuiltinFeatureCounter { + // The vector of counters, matching the size of the static vector MIGRATION_FEATURE_IDS, + // each counter representing the number of times its corresponding feature ID is + // referenced in this transaction. + migrating_builtin: [u16; NUM_COST_MODELED_BUILTINS_WITH_MIGRATIONS], +} + +impl Default for MigrationBuiltinFeatureCounter { + fn default() -> Self { + Self { + migrating_builtin: [0; NUM_COST_MODELED_BUILTINS_WITH_MIGRATIONS], + } + } +} + #[cfg_attr(test, derive(Eq, PartialEq))] #[cfg_attr(feature = "dev-context-only-utils", derive(Clone))] #[derive(Default, Debug)] @@ -31,6 +52,7 @@ pub struct ComputeBudgetInstructionDetails { // Additional builtin program counters num_builtin_instructions: u16, num_non_builtin_instructions: u16, + migrating_builtin_feature_counters: MigrationBuiltinFeatureCounter, } impl ComputeBudgetInstructionDetails { @@ -71,6 +93,20 @@ impl ComputeBudgetInstructionDetails { 1 ); } + ProgramKind::MigratingBuiltin { + core_bpf_migration_feature_index, + } => { + saturating_add_assign!( + *compute_budget_instruction_details + .migrating_builtin_feature_counters + .migrating_builtin + .get_mut(core_bpf_migration_feature_index) + .expect( + "migrating feature index within range of MIGRATION_FEATURE_IDS" + ), + 1 + ); + } } } } @@ -175,10 +211,25 @@ impl ComputeBudgetInstructionDetails { fn calculate_default_compute_unit_limit(&self, feature_set: &FeatureSet) -> u32 { if feature_set.is_active(&feature_set::reserve_minimal_cus_for_builtin_instructions::id()) { + // evaluate if any builtin has migrated with feature_set + let (num_migrated, num_not_migrated) = self + .migrating_builtin_feature_counters + .migrating_builtin + .iter() + .enumerate() + .fold((0, 0), |(migrated, not_migrated), (index, count)| { + if *count > 0 && feature_set.is_active(get_migration_feature_id(index)) { + (migrated + count, not_migrated) + } else { + (migrated, not_migrated + count) + } + }); u32::from(self.num_builtin_instructions) + .saturating_add(u32::from(num_not_migrated)) .saturating_mul(MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT) .saturating_add( u32::from(self.num_non_builtin_instructions) + .saturating_add(u32::from(num_migrated)) .saturating_mul(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT), ) } else { @@ -192,6 +243,7 @@ impl ComputeBudgetInstructionDetails { mod test { use { super::*, + solana_builtins::cost_modeling::get_migration_feature_index_from_feature_id, solana_sdk::{ instruction::Instruction, message::Message, @@ -521,4 +573,89 @@ mod test { ); } } + + #[test] + fn test_builtin_program_migration() { + let tx = build_sanitized_transaction(&[ + Instruction::new_with_bincode(Pubkey::new_unique(), &(), vec![]), + solana_sdk::stake::instruction::delegate_stake( + &Pubkey::new_unique(), + &Pubkey::new_unique(), + &Pubkey::new_unique(), + ), + ]); + let feature_id_index = get_migration_feature_index_from_feature_id( + &feature_set::migrate_stake_program_to_core_bpf::id(), + ) + .unwrap(); + let mut expected_details = ComputeBudgetInstructionDetails { + num_non_compute_budget_instructions: 2, + num_non_builtin_instructions: 1, + ..ComputeBudgetInstructionDetails::default() + }; + expected_details + .migrating_builtin_feature_counters + .migrating_builtin[feature_id_index] = 1; + let expected_details = Ok(expected_details); + let details = + ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)); + assert_eq!(details, expected_details); + let details = details.unwrap(); + + // reserve_minimal_cus_for_builtin_instructions: false; + // migrate_stake_program_to_core_bpf: false; + // expect: 1 bpf ix, 1 non-compute-budget builtin, cu-limit = 2 * 200K + let mut feature_set = FeatureSet::default(); + let cu_limits = details.sanitize_and_convert_to_compute_budget_limits(&feature_set); + assert_eq!( + cu_limits, + Ok(ComputeBudgetLimits { + compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT * 2, + ..ComputeBudgetLimits::default() + }) + ); + + // reserve_minimal_cus_for_builtin_instructions: true; + // migrate_stake_program_to_core_bpf: false; + // expect: 1 bpf ix, 1 non-compute-budget builtin, cu-limit = 200K + 3K + feature_set.activate( + &feature_set::reserve_minimal_cus_for_builtin_instructions::id(), + 0, + ); + let cu_limits = details.sanitize_and_convert_to_compute_budget_limits(&feature_set); + assert_eq!( + cu_limits, + Ok(ComputeBudgetLimits { + compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT + + MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT, + ..ComputeBudgetLimits::default() + }) + ); + + // reserve_minimal_cus_for_builtin_instructions: true; + // migrate_stake_program_to_core_bpf: true; + // expect: 2 bpf ix, cu-limit = 2 * 200K + feature_set.activate(&feature_set::migrate_stake_program_to_core_bpf::id(), 0); + let cu_limits = details.sanitize_and_convert_to_compute_budget_limits(&feature_set); + assert_eq!( + cu_limits, + Ok(ComputeBudgetLimits { + compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT * 2, + ..ComputeBudgetLimits::default() + }) + ); + + // reserve_minimal_cus_for_builtin_instructions: false; + // migrate_stake_program_to_core_bpf: false; + // expect: 1 bpf ix, 1 non-compute-budget builtin, cu-limit = 2 * 200K + feature_set.deactivate(&feature_set::reserve_minimal_cus_for_builtin_instructions::id()); + let cu_limits = details.sanitize_and_convert_to_compute_budget_limits(&feature_set); + assert_eq!( + cu_limits, + Ok(ComputeBudgetLimits { + compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT * 2, + ..ComputeBudgetLimits::default() + }) + ); + } }