diff --git a/Cargo.lock b/Cargo.lock index 377654cc683eb1..d8636961fb30e9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6292,6 +6292,7 @@ dependencies = [ "solana-stake-program", "solana-system-program", "solana-vote-program", + "static_assertions", ] [[package]] diff --git a/builtins-default-costs/Cargo.toml b/builtins-default-costs/Cargo.toml index 9c0a1e0753a799..282a3d9dec25ea 100644 --- a/builtins-default-costs/Cargo.toml +++ b/builtins-default-costs/Cargo.toml @@ -35,6 +35,7 @@ name = "solana_builtins_default_costs" [dev-dependencies] rand = "0.8.5" +static_assertions = { workspace = true } [package.metadata.docs.rs] targets = ["x86_64-unknown-linux-gnu"] diff --git a/builtins-default-costs/src/lib.rs b/builtins-default-costs/src/lib.rs index c6349043d3d3cf..ef313cdd5358e0 100644 --- a/builtins-default-costs/src/lib.rs +++ b/builtins-default-costs/src/lib.rs @@ -1,4 +1,5 @@ #![cfg_attr(feature = "frozen-abi", feature(min_specialization))] +#![feature(const_option)] #![allow(clippy::arithmetic_side_effects)] use { ahash::AHashMap, @@ -12,6 +13,16 @@ use { }, }; +#[derive(Clone)] +struct CoreBpfMigrationFeature { + feature_id: Pubkey, + // encoding positional information explicitly for migration feature item, + // its value must be correctly corresponding to this object's position + // in MIGRATING_BUILTINS_COSTS, otherwise a const validation + // `validate_position(MIGRATING_BUILTINS_COSTS)` will fail at compile time. + position: usize, +} + /// 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 @@ -21,40 +32,91 @@ use { #[derive(Clone)] pub struct BuiltinCost { native_cost: u64, - core_bpf_migration_feature: Option, + core_bpf_migration_feature: Option, } -/// Number of compute units for each built-in programs +/// const function validates `position` correctness at compile time. +#[allow(dead_code)] +const fn validate_position(migrating_builtins: &[(Pubkey, BuiltinCost)]) { + let mut index = 0; + while index < migrating_builtins.len() { + assert!( + migrating_builtins[index] + .1 + .core_bpf_migration_feature + .as_ref() + .unwrap() + .position + == index, + "migration feture must exist and at correct position" + ); + index += 1; + } +} +const _: () = validate_position(MIGRATING_BUILTINS_COSTS); + +lazy_static! { + /// Number of compute units for each built-in programs + /// + /// DEVELOPER WARNING: This map CANNOT be modified without causing a + /// consensus failure because this map is used to calculate the compute + /// limit for transactions that don't specify a compute limit themselves as + /// of https://github.com/anza-xyz/agave/issues/2212. It's also used to + /// 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. + static ref BUILTIN_INSTRUCTION_COSTS: AHashMap = + MIGRATING_BUILTINS_COSTS + .iter() + .chain(NON_MIGRATING_BUILTINS_COSTS.iter()) + .cloned() + .collect(); + // DO NOT ADD MORE ENTRIES TO THIS MAP +} + +/// DEVELOPER WARNING: please do not add new entry into MIGRATING_BUILTINS_COSTS or +/// NON_MIGRATING_BUILTINS_COSTS, do so will modify BUILTIN_INSTRUCTION_COSTS therefore +/// cause consensus failure. However, when a builtin started being migrated to core bpf, +/// it MUST be moved from NON_MIGRATING_BUILTINS_COSTS to MIGRATING_BUILTINS_COSTS, then +/// correctly furnishing `core_bpf_migration_feature`. /// -/// DEVELOPER WARNING: This map CANNOT be modified without causing a -/// consensus failure because this map is used to calculate the compute -/// limit for transactions that don't specify a compute limit themselves as -/// of https://github.com/anza-xyz/agave/issues/2212. It's also used to -/// 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. +#[allow(dead_code)] +const TOTAL_COUNT_BUILTS: usize = 12; +#[cfg(test)] +static_assertions::const_assert_eq!( + MIGRATING_BUILTINS_COSTS.len() + NON_MIGRATING_BUILTINS_COSTS.len(), + TOTAL_COUNT_BUILTS +); + pub const MIGRATING_BUILTINS_COSTS: &[(Pubkey, BuiltinCost)] = &[ ( stake::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()), + core_bpf_migration_feature: Some(CoreBpfMigrationFeature { + feature_id: feature_set::migrate_stake_program_to_core_bpf::id(), + position: 0, + }), }, ), ( config::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()), + core_bpf_migration_feature: Some(CoreBpfMigrationFeature { + feature_id: feature_set::migrate_config_program_to_core_bpf::id(), + position: 1, + }), }, ), ( address_lookup_table::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(), - ), + core_bpf_migration_feature: Some(CoreBpfMigrationFeature { + feature_id: feature_set::migrate_address_lookup_table_program_to_core_bpf::id(), + position: 2, + }), }, ), ]; @@ -126,16 +188,6 @@ pub const NON_MIGRATING_BUILTINS_COSTS: &[(Pubkey, BuiltinCost)] = &[ ), ]; -lazy_static! { - static ref BUILTIN_INSTRUCTION_COSTS: AHashMap = - MIGRATING_BUILTINS_COSTS - .iter() - .chain(NON_MIGRATING_BUILTINS_COSTS.iter()) - .cloned() - .collect(); - // DO NOT ADD MORE ENTRIES TO THIS MAP -} - lazy_static! { /// A table of 256 booleans indicates whether the first `u8` of a Pubkey exists in /// BUILTIN_INSTRUCTION_COSTS. If the value is true, the Pubkey might be a builtin key; @@ -162,21 +214,18 @@ pub fn get_builtin_instruction_cost<'a>( |builtin_cost| -> bool { builtin_cost .core_bpf_migration_feature - .map(|feature_id| !feature_set.is_active(&feature_id)) + .as_ref() + .map(|migration_feature| !feature_set.is_active(&migration_feature.feature_id)) .unwrap_or(true) }, ) .map(|builtin_cost| builtin_cost.native_cost) } -lazy_static! { - /// A static list of builtin programs' migration feature IDs. - pub static ref MIGRATION_FEATURES_ID: Vec = { - BUILTIN_INSTRUCTION_COSTS - .values() - .filter_map(|builtin_cost| builtin_cost.core_bpf_migration_feature) - .collect() - }; +pub enum BuiltinMigrationFeatureIndex { + NotBuiltin, + BuiltinNoMigrationFeature, + BuiltinWithMigrationFeature(usize), } /// Given a program pubkey, returns: @@ -184,23 +233,68 @@ lazy_static! { /// - Some, is builtin, but no associated migration feature ID; /// - Some, is builtin, and its associated migration feature ID /// index in MIGRATION_FEATURES_ID. -pub fn get_builtin_migration_feature_index(program_id: &Pubkey) -> Option> { - BUILTIN_INSTRUCTION_COSTS - .get(program_id) - .map(|builtin_cost| { - builtin_cost.core_bpf_migration_feature.map(|id| { - MIGRATION_FEATURES_ID - .iter() - .position(|&x| x == id) - .expect("must be known migration feature ID") - }) +pub fn get_builtin_migration_feature_index(program_id: &Pubkey) -> BuiltinMigrationFeatureIndex { + BUILTIN_INSTRUCTION_COSTS.get(program_id).map_or( + BuiltinMigrationFeatureIndex::NotBuiltin, + |builtin_cost| { + builtin_cost.core_bpf_migration_feature.as_ref().map_or( + BuiltinMigrationFeatureIndex::BuiltinNoMigrationFeature, + |migration_feature| { + BuiltinMigrationFeatureIndex::BuiltinWithMigrationFeature( + migration_feature.position, + ) + }, + ) + }, + ) +} + +/// Helper function to return ref of migration feature Pubkey at position `index` +/// from MIGRATING_BUILTINS_COSTS +pub fn get_migration_feature_id(index: usize) -> &'static Pubkey { + &MIGRATING_BUILTINS_COSTS + .get(index) + .expect("valid index of MIGRATING_BUILTINS_COSTS") + .1 + .core_bpf_migration_feature + .as_ref() + .expect("migrating builtin") + .feature_id +} + +pub fn get_migration_feature_position(feature_id: &Pubkey) -> usize { + MIGRATING_BUILTINS_COSTS + .iter() + .position(|(_, c)| { + c.core_bpf_migration_feature + .as_ref() + .map(|f| f.feature_id) + .unwrap() + == *feature_id }) + .unwrap() } #[cfg(test)] mod test { use super::*; + #[test] + fn test_const_builtin_cost_arrays() { + // sanity check to make sure built-ins are declared in the correct array + assert!(MIGRATING_BUILTINS_COSTS + .iter() + .enumerate() + .all(|(index, (_, c))| { + let migration_feature = &c.core_bpf_migration_feature; + migration_feature.is_some() + && migration_feature.as_ref().map(|f| f.position) == Some(index) + })); + assert!(NON_MIGRATING_BUILTINS_COSTS + .iter() + .all(|(_, c)| c.core_bpf_migration_feature.is_none())); + } + #[test] fn test_get_builtin_instruction_cost() { // use native cost if no migration planned @@ -212,15 +306,11 @@ mod test { // 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()) + 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( - &solana_stake_program::id(), - &FeatureSet::all_enabled() - ) - .is_none()); + assert!(get_builtin_instruction_cost(&stake::id(), &FeatureSet::all_enabled()).is_none()); // None if not builtin assert!( @@ -234,26 +324,61 @@ mod test { #[test] fn test_get_builtin_migration_feature_index() { - assert!(get_builtin_migration_feature_index(&Pubkey::new_unique()).is_none()); - assert_eq!( + assert!(matches!( + get_builtin_migration_feature_index(&Pubkey::new_unique()), + BuiltinMigrationFeatureIndex::NotBuiltin + )); + assert!(matches!( get_builtin_migration_feature_index(&compute_budget::id()), - Some(None) - ); - let feature_index = get_builtin_migration_feature_index(&solana_stake_program::id()); + BuiltinMigrationFeatureIndex::BuiltinNoMigrationFeature, + )); + let feature_index = get_builtin_migration_feature_index(&stake::id()); + assert!(matches!( + feature_index, + BuiltinMigrationFeatureIndex::BuiltinWithMigrationFeature(_) + )); + let BuiltinMigrationFeatureIndex::BuiltinWithMigrationFeature(feature_index) = + feature_index + else { + panic!("expect migrating builtin") + }; assert_eq!( - MIGRATION_FEATURES_ID[feature_index.unwrap().unwrap()], - feature_set::migrate_stake_program_to_core_bpf::id() + get_migration_feature_id(feature_index), + &feature_set::migrate_stake_program_to_core_bpf::id() ); - let feature_index = get_builtin_migration_feature_index(&solana_config_program::id()); + let feature_index = get_builtin_migration_feature_index(&config::id()); + assert!(matches!( + feature_index, + BuiltinMigrationFeatureIndex::BuiltinWithMigrationFeature(_) + )); + let BuiltinMigrationFeatureIndex::BuiltinWithMigrationFeature(feature_index) = + feature_index + else { + panic!("expect migrating builtin") + }; assert_eq!( - MIGRATION_FEATURES_ID[feature_index.unwrap().unwrap()], - feature_set::migrate_config_program_to_core_bpf::id() + get_migration_feature_id(feature_index), + &feature_set::migrate_config_program_to_core_bpf::id() ); - let feature_index = - get_builtin_migration_feature_index(&address_lookup_table::program::id()); + let feature_index = get_builtin_migration_feature_index(&address_lookup_table::id()); + assert!(matches!( + feature_index, + BuiltinMigrationFeatureIndex::BuiltinWithMigrationFeature(_) + )); + let BuiltinMigrationFeatureIndex::BuiltinWithMigrationFeature(feature_index) = + feature_index + else { + panic!("expect migrating builtin") + }; assert_eq!( - MIGRATION_FEATURES_ID[feature_index.unwrap().unwrap()], - feature_set::migrate_address_lookup_table_program_to_core_bpf::id() + get_migration_feature_id(feature_index), + &feature_set::migrate_address_lookup_table_program_to_core_bpf::id() ); } + + #[test] + #[should_panic(expected = "valid index of MIGRATING_BUILTINS_COSTS")] + fn test_get_migration_feature_id_invalid_index() { + let _ = get_migration_feature_id(MIGRATING_BUILTINS_COSTS.len() + 1); + } } diff --git a/runtime-transaction/src/builtin_programs_filter.rs b/runtime-transaction/src/builtin_programs_filter.rs index aa4d949670f7c9..705f155e15f8a3 100644 --- a/runtime-transaction/src/builtin_programs_filter.rs +++ b/runtime-transaction/src/builtin_programs_filter.rs @@ -1,6 +1,8 @@ use { agave_transaction_view::static_account_keys_frame::MAX_STATIC_ACCOUNTS_PER_PACKET as FILTER_SIZE, - solana_builtins_default_costs::{get_builtin_migration_feature_index, MAYBE_BUILTIN_KEY}, + solana_builtins_default_costs::{ + get_builtin_migration_feature_index, BuiltinMigrationFeatureIndex, MAYBE_BUILTIN_KEY, + }, solana_sdk::pubkey::Pubkey, }; @@ -44,21 +46,24 @@ impl BuiltinProgramsFilter { return ProgramKind::NotBuiltin; } - get_builtin_migration_feature_index(program_id).map_or( - ProgramKind::NotBuiltin, - |some_builtin| match some_builtin { - Some(core_bpf_migration_feature_index) => ProgramKind::MigratingBuiltin { - core_bpf_migration_feature_index, - }, - None => ProgramKind::Builtin, + match get_builtin_migration_feature_index(program_id) { + BuiltinMigrationFeatureIndex::NotBuiltin => ProgramKind::NotBuiltin, + BuiltinMigrationFeatureIndex::BuiltinNoMigrationFeature => ProgramKind::Builtin, + BuiltinMigrationFeatureIndex::BuiltinWithMigrationFeature( + core_bpf_migration_feature_index, + ) => ProgramKind::MigratingBuiltin { + core_bpf_migration_feature_index, }, - ) + } } } #[cfg(test)] mod test { - use {super::*, solana_builtins_default_costs::MIGRATION_FEATURES_ID, solana_sdk::feature_set}; + use { + super::*, solana_builtins_default_costs::get_migration_feature_position, + solana_sdk::feature_set, + }; const DUMMY_PROGRAM_ID: &str = "dummmy1111111111111111111111111111111111111"; @@ -120,10 +125,9 @@ mod test { assert_eq!( test_store.get_program_kind(index, &migrating_builtin_pubkey), ProgramKind::MigratingBuiltin { - core_bpf_migration_feature_index: MIGRATION_FEATURES_ID - .iter() - .position(|&x| x == migration_feature_id) - .unwrap(), + core_bpf_migration_feature_index: get_migration_feature_position( + &migration_feature_id + ), } ); } diff --git a/runtime-transaction/src/compute_budget_instruction_details.rs b/runtime-transaction/src/compute_budget_instruction_details.rs index 970b6b8e949318..270cee37ee1ef2 100644 --- a/runtime-transaction/src/compute_budget_instruction_details.rs +++ b/runtime-transaction/src/compute_budget_instruction_details.rs @@ -3,7 +3,7 @@ use { builtin_programs_filter::{BuiltinProgramsFilter, ProgramKind}, compute_budget_program_id_filter::ComputeBudgetProgramIdFilter, }, - solana_builtins_default_costs::MIGRATION_FEATURES_ID, + solana_builtins_default_costs::{get_migration_feature_id, MIGRATING_BUILTINS_COSTS}, solana_compute_budget::compute_budget_limits::*, solana_sdk::{ borsh1::try_from_slice_unchecked, @@ -25,13 +25,13 @@ 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; solana_builtins_default_costs::MIGRATING_BUILTINS_COSTS.len()], + migrating_builtin: [u16; MIGRATING_BUILTINS_COSTS.len()], } impl Default for MigrationBuiltinFeatureCounter { fn default() -> Self { Self { - migrating_builtin: [0; solana_builtins_default_costs::MIGRATING_BUILTINS_COSTS.len()], + migrating_builtin: [0; MIGRATING_BUILTINS_COSTS.len()], } } } @@ -216,7 +216,7 @@ impl ComputeBudgetInstructionDetails { .iter() .enumerate() .fold((0, 0), |(migrated, not_migrated), (index, count)| { - if feature_set.is_active(&MIGRATION_FEATURES_ID[index]) { + if *count > 0 && feature_set.is_active(get_migration_feature_id(index)) { (migrated + count, not_migrated) } else { (migrated, not_migrated + count) @@ -242,6 +242,7 @@ impl ComputeBudgetInstructionDetails { mod test { use { super::*, + solana_builtins_default_costs::get_migration_feature_position, solana_sdk::{ instruction::Instruction, message::Message, @@ -582,10 +583,8 @@ mod test { &Pubkey::new_unique(), ), ]); - let feature_id_index = MIGRATION_FEATURES_ID - .iter() - .position(|id| *id == feature_set::migrate_stake_program_to_core_bpf::id()) - .unwrap(); + let feature_id_index = + get_migration_feature_position(&feature_set::migrate_stake_program_to_core_bpf::id()); let mut expected_details = ComputeBudgetInstructionDetails { num_non_compute_budget_instructions: 2, num_non_builtin_instructions: 1,