From f1d1130c3aabb1dea1615cbac814dfc52eaaa725 Mon Sep 17 00:00:00 2001 From: Tao Zhu Date: Tue, 10 Dec 2024 17:50:12 -0600 Subject: [PATCH] fix cherry-pick conflicts --- Cargo.lock | 1 + builtins-default-costs/src/lib.rs | 58 -------- core/src/banking_stage/consumer.rs | 19 +-- .../immutable_deserialized_packet.rs | 6 +- .../scheduler_controller.rs | 11 +- cost-model/Cargo.toml | 1 + cost-model/src/cost_model.rs | 135 +++--------------- runtime-transaction/src/lib.rs | 6 +- .../src/runtime_transaction.rs | 4 +- runtime/src/prioritization_fee_cache.rs | 7 +- svm/src/transaction_processor.rs | 12 +- 11 files changed, 44 insertions(+), 216 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7008cf8c88e3ac..740d61ce8836d1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6540,6 +6540,7 @@ dependencies = [ "rand 0.8.5", "solana-builtins-default-costs", "solana-compute-budget", + "solana-compute-budget-program", "solana-cost-model", "solana-feature-set", "solana-frozen-abi", diff --git a/builtins-default-costs/src/lib.rs b/builtins-default-costs/src/lib.rs index c5b12884f265db..a484a5100cc26a 100644 --- a/builtins-default-costs/src/lib.rs +++ b/builtins-default-costs/src/lib.rs @@ -54,66 +54,8 @@ lazy_static! { temp_table }; } -<<<<<<< HEAD -======= - -pub fn get_builtin_instruction_cost<'a>( - program_id: &'a Pubkey, - feature_set: &'a FeatureSet, -) -> Option { - 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() - ); - } -} ->>>>>>> 3e9af14f3a (Fix reserve minimal compute units for builtins (#3799)) diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 08dbd8ebaf17a1..f9772552c72ffe 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -574,19 +574,12 @@ impl Consumer { .sanitized_transactions() .iter() .filter_map(|transaction| { -<<<<<<< HEAD - process_compute_budget_instructions(SVMMessage::program_instructions_iter( - transaction, - )) + process_compute_budget_instructions( + SVMMessage::program_instructions_iter(transaction), + &bank.feature_set, + ) .ok() .map(|limits| limits.compute_unit_price) -======= - transaction - .compute_budget_instruction_details() - .sanitize_and_convert_to_compute_budget_limits(&bank.feature_set) - .ok() - .map(|limits| limits.compute_unit_price) ->>>>>>> 3e9af14f3a (Fix reserve minimal compute units for builtins (#3799)) }) .minmax(); let (min_prioritization_fees, max_prioritization_fees) = @@ -765,12 +758,8 @@ impl Consumer { ) -> Result<(), TransactionError> { let fee_payer = message.fee_payer(); let fee_budget_limits = FeeBudgetLimits::from(process_compute_budget_instructions( -<<<<<<< HEAD SVMMessage::program_instructions_iter(message), -======= - message.program_instructions_iter(), &bank.feature_set, ->>>>>>> 3e9af14f3a (Fix reserve minimal compute units for builtins (#3799)) )?); let fee = solana_fee::calculate_fee( message, diff --git a/core/src/banking_stage/immutable_deserialized_packet.rs b/core/src/banking_stage/immutable_deserialized_packet.rs index acde8ce5583b00..346041cc4b98a6 100644 --- a/core/src/banking_stage/immutable_deserialized_packet.rs +++ b/core/src/banking_stage/immutable_deserialized_packet.rs @@ -41,17 +41,13 @@ pub enum DeserializedPacketError { FailedFilter(#[from] PacketFilterFailure), } -<<<<<<< HEAD -#[derive(Debug, Eq)] -======= lazy_static::lazy_static! { // Make a dummy feature_set with all features enabled to // fetch compute_unit_price and compute_unit_limit for legacy leader. static ref FEATURE_SET: FeatureSet = FeatureSet::all_enabled(); } -#[derive(Debug)] ->>>>>>> 3e9af14f3a (Fix reserve minimal compute units for builtins (#3799)) +#[derive(Debug, Eq)] pub struct ImmutableDeserializedPacket { original_packet: Packet, transaction: SanitizedVersionedTransaction, diff --git a/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs b/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs index 4e9cceba472acb..791ff746209800 100644 --- a/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs +++ b/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs @@ -544,11 +544,12 @@ impl SchedulerController { .is_ok() }) .filter_map(|(packet, tx, deactivation_slot)| { - process_compute_budget_instructions(SVMMessage::program_instructions_iter(&tx)) - .map(|compute_budget| { - (packet, tx, deactivation_slot, compute_budget.into()) - }) - .ok() + process_compute_budget_instructions( + SVMMessage::program_instructions_iter(&tx), + &working_bank.feature_set, + ) + .map(|compute_budget| (packet, tx, deactivation_slot, compute_budget.into())) + .ok() }) .for_each(|(packet, tx, deactivation_slot, fee_budget_limits)| { arc_packets.push(packet); diff --git a/cost-model/Cargo.toml b/cost-model/Cargo.toml index 56948b20462a27..c11fbd650c6d4d 100644 --- a/cost-model/Cargo.toml +++ b/cost-model/Cargo.toml @@ -36,6 +36,7 @@ name = "solana_cost_model" itertools = { workspace = true } rand = "0.8.5" # See order-crates-for-publishing.py for using this unusual `path = "."` +solana-compute-budget-program = { workspace = true } solana-cost-model = { path = ".", features = ["dev-context-only-utils"] } solana-logger = { workspace = true } solana-sdk = { workspace = true, features = ["dev-context-only-utils"] } diff --git a/cost-model/src/cost_model.rs b/cost-model/src/cost_model.rs index 7df59673d23a76..6bafd9837ed6dc 100644 --- a/cost-model/src/cost_model.rs +++ b/cost-model/src/cost_model.rs @@ -182,7 +182,7 @@ impl CostModel { } fn get_transaction_cost_without_minimal_builtin_cus( - transaction: &impl TransactionWithMeta, + transaction: &impl SVMMessage, feature_set: &FeatureSet, ) -> (u64, u64, u64) { let mut programs_execution_costs = 0u64; @@ -216,19 +216,12 @@ impl CostModel { } } -<<<<<<< HEAD // if failed to process compute_budget instructions, the transaction will not be executed // by `bank`, therefore it should be considered as no execution cost by cost model. - match process_compute_budget_instructions(transaction.program_instructions_iter()) { -======= - // if failed to process compute budget instructions, the transaction - // will not be executed by `bank`, therefore it should be considered - // as no execution cost by cost model. - match transaction - .compute_budget_instruction_details() - .sanitize_and_convert_to_compute_budget_limits(feature_set) - { ->>>>>>> 3e9af14f3a (Fix reserve minimal compute units for builtins (#3799)) + match process_compute_budget_instructions( + transaction.program_instructions_iter(), + feature_set, + ) { Ok(compute_budget_limits) => { // if tx contained user-space instructions and a more accurate estimate available correct it, // where "user-space instructions" must be specifically checked by @@ -258,35 +251,34 @@ impl CostModel { /// Return (programs_execution_cost, loaded_accounts_data_size_cost) fn get_estimated_execution_cost( - transaction: &impl TransactionWithMeta, + transaction: &impl SVMMessage, feature_set: &FeatureSet, ) -> (u64, u64) { // if failed to process compute_budget instructions, the transaction will not be executed // by `bank`, therefore it should be considered as no execution cost by cost model. - let (programs_execution_costs, loaded_accounts_data_size_cost) = match transaction - .compute_budget_instruction_details() - .sanitize_and_convert_to_compute_budget_limits(feature_set) - { - Ok(compute_budget_limits) => ( - u64::from(compute_budget_limits.compute_unit_limit), - Self::calculate_loaded_accounts_data_size_cost( - compute_budget_limits.loaded_accounts_bytes.get(), - feature_set, + let (programs_execution_costs, loaded_accounts_data_size_cost) = + match process_compute_budget_instructions( + transaction.program_instructions_iter(), + feature_set, + ) { + Ok(compute_budget_limits) => ( + u64::from(compute_budget_limits.compute_unit_limit), + Self::calculate_loaded_accounts_data_size_cost( + compute_budget_limits.loaded_accounts_bytes.get(), + feature_set, + ), ), - ), - Err(_) => (0, 0), - }; + Err(_) => (0, 0), + }; (programs_execution_costs, loaded_accounts_data_size_cost) } /// Return the instruction data bytes cost. - fn get_instructions_data_cost(transaction: &SanitizedTransaction) -> u64 { + fn get_instructions_data_cost(transaction: &impl SVMMessage) -> u64 { let ix_data_bytes_len_total: u64 = transaction - .message() - .instructions() - .iter() - .map(|instruction| instruction.data.len() as u64) + .program_instructions_iter() + .map(|(_, instruction)| instruction.data.len() as u64) .sum(); ix_data_bytes_len_total / INSTRUCTION_DATA_BYTES_COST @@ -374,14 +366,9 @@ mod tests { use { super::*, itertools::Itertools, -<<<<<<< HEAD - log::debug, -======= solana_compute_budget::compute_budget_limits::{ DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT, }, - solana_runtime_transaction::runtime_transaction::RuntimeTransaction, ->>>>>>> 3e9af14f3a (Fix reserve minimal compute units for builtins (#3799)) solana_sdk::{ compute_budget::{self, ComputeBudgetInstruction}, fee::ACCOUNT_DATA_COST_PAGE_SIZE, @@ -580,18 +567,6 @@ mod tests { system_transaction::transfer(&mint_keypair, &keypair.pubkey(), 2, start_hash), ); -<<<<<<< HEAD - // expected cost for one system transfer instructions - let expected_execution_cost = BUILTIN_INSTRUCTION_COSTS - .get(&system_program::id()) - .unwrap(); - - let (program_execution_cost, _loaded_accounts_data_size_cost, data_bytes_cost) = - CostModel::get_transaction_cost(&simple_transaction, &FeatureSet::all_enabled()); - - assert_eq!(*expected_execution_cost, program_execution_cost); - assert_eq!(3, data_bytes_cost); -======= for (feature_set, expected_execution_cost) in [ ( FeatureSet::default(), @@ -607,7 +582,6 @@ mod tests { assert_eq!(expected_execution_cost, program_execution_cost); } ->>>>>>> 3e9af14f3a (Fix reserve minimal compute units for builtins (#3799)) } #[test] @@ -625,12 +599,7 @@ mod tests { vec![Pubkey::new_unique()], instructions, ); -<<<<<<< HEAD let token_transaction = SanitizedTransaction::from_transaction_for_tests(tx); - debug!("token_transaction {:?}", token_transaction); -======= - let token_transaction = RuntimeTransaction::from_transaction_for_tests(tx); ->>>>>>> 3e9af14f3a (Fix reserve minimal compute units for builtins (#3799)) for (feature_set, expected_execution_cost) in [ ( @@ -773,17 +742,6 @@ mod tests { )); // expected cost for two system transfer instructions -<<<<<<< HEAD - let program_cost = BUILTIN_INSTRUCTION_COSTS - .get(&system_program::id()) - .unwrap(); - let expected_cost = program_cost * 2; - - let (program_execution_cost, _loaded_accounts_data_size_cost, data_bytes_cost) = - CostModel::get_transaction_cost(&tx, &FeatureSet::all_enabled()); - assert_eq!(expected_cost, program_execution_cost); - assert_eq!(6, data_bytes_cost); -======= for (feature_set, expected_execution_cost) in [ ( FeatureSet::default(), @@ -799,7 +757,6 @@ mod tests { assert_eq!(expected_execution_cost, programs_execution_cost); assert_eq!(6, data_bytes_cost); } ->>>>>>> 3e9af14f3a (Fix reserve minimal compute units for builtins (#3799)) } #[test] @@ -885,26 +842,6 @@ mod tests { )); let expected_account_cost = WRITE_LOCK_UNITS * 2; -<<<<<<< HEAD - let expected_execution_cost = BUILTIN_INSTRUCTION_COSTS - .get(&system_program::id()) - .unwrap(); - const DEFAULT_PAGE_COST: u64 = 8; - let expected_loaded_accounts_data_size_cost = - solana_compute_budget::compute_budget_limits::MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES.get() - as u64 - / ACCOUNT_DATA_COST_PAGE_SIZE - * DEFAULT_PAGE_COST; - - let tx_cost = CostModel::calculate_cost(&tx, &FeatureSet::all_enabled()); - assert_eq!(expected_account_cost, tx_cost.write_lock_cost()); - assert_eq!(*expected_execution_cost, tx_cost.programs_execution_cost()); - assert_eq!(2, tx_cost.writable_accounts().count()); - assert_eq!( - expected_loaded_accounts_data_size_cost, - tx_cost.loaded_accounts_data_size_cost() - ); -======= for (feature_set, expected_execution_cost) in [ ( FeatureSet::default(), @@ -931,7 +868,6 @@ mod tests { tx_cost.loaded_accounts_data_size_cost() ); } ->>>>>>> 3e9af14f3a (Fix reserve minimal compute units for builtins (#3799)) } #[test] @@ -951,15 +887,6 @@ mod tests { )); let expected_account_cost = WRITE_LOCK_UNITS * 2; -<<<<<<< HEAD - let expected_execution_cost = BUILTIN_INSTRUCTION_COSTS - .get(&system_program::id()) - .unwrap() - + BUILTIN_INSTRUCTION_COSTS - .get(&compute_budget::id()) - .unwrap(); - let expected_loaded_accounts_data_size_cost = (data_limit as u64) / (32 * 1024) * 8; -======= for (feature_set, expected_execution_cost) in [ ( FeatureSet::default(), @@ -972,7 +899,6 @@ mod tests { ), ] { let expected_loaded_accounts_data_size_cost = (data_limit as u64) / (32 * 1024) * 8; ->>>>>>> 3e9af14f3a (Fix reserve minimal compute units for builtins (#3799)) let tx_cost = CostModel::calculate_cost(&tx, &feature_set); assert_eq!(expected_account_cost, tx_cost.write_lock_cost()); @@ -1000,12 +926,6 @@ mod tests { start_hash, )); // transaction has one builtin instruction, and one bpf instruction, no ComputeBudget::compute_unit_limit -<<<<<<< HEAD - let expected_builtin_cost = *BUILTIN_INSTRUCTION_COSTS - .get(&solana_system_program::id()) - .unwrap(); - let expected_bpf_cost = DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT; -======= for (feature_set, expected_execution_cost) in [ ( FeatureSet::default(), @@ -1020,7 +940,6 @@ mod tests { ] { let (programs_execution_cost, _loaded_accounts_data_size_cost, _data_bytes_cost) = CostModel::get_transaction_cost(&transaction, &feature_set); ->>>>>>> 3e9af14f3a (Fix reserve minimal compute units for builtins (#3799)) assert_eq!(expected_execution_cost, programs_execution_cost); } @@ -1041,15 +960,6 @@ mod tests { &[&mint_keypair], start_hash, )); -<<<<<<< HEAD - // transaction has one builtin instruction, and one ComputeBudget::compute_unit_limit - let expected_cost = *BUILTIN_INSTRUCTION_COSTS - .get(&solana_system_program::id()) - .unwrap() - + BUILTIN_INSTRUCTION_COSTS - .get(&compute_budget::id()) - .unwrap(); -======= for (feature_set, expected_execution_cost) in [ ( FeatureSet::default(), @@ -1060,7 +970,6 @@ mod tests { ] { let (programs_execution_cost, _loaded_accounts_data_size_cost, _data_bytes_cost) = CostModel::get_transaction_cost(&transaction, &feature_set); ->>>>>>> 3e9af14f3a (Fix reserve minimal compute units for builtins (#3799)) assert_eq!(expected_execution_cost, programs_execution_cost); } diff --git a/runtime-transaction/src/lib.rs b/runtime-transaction/src/lib.rs index 4f7b214f8f235d..079623a8bb6be1 100644 --- a/runtime-transaction/src/lib.rs +++ b/runtime-transaction/src/lib.rs @@ -1,12 +1,8 @@ #![cfg_attr(feature = "frozen-abi", feature(min_specialization))] #![allow(clippy::arithmetic_side_effects)] -<<<<<<< HEAD -mod compute_budget_instruction_details; -======= mod builtin_programs_filter; -pub mod compute_budget_instruction_details; ->>>>>>> 3e9af14f3a (Fix reserve minimal compute units for builtins (#3799)) +mod compute_budget_instruction_details; mod compute_budget_program_id_filter; pub mod instructions_processor; pub mod runtime_transaction; diff --git a/runtime-transaction/src/runtime_transaction.rs b/runtime-transaction/src/runtime_transaction.rs index 183aa222c2aaaf..a088d87112150c 100644 --- a/runtime-transaction/src/runtime_transaction.rs +++ b/runtime-transaction/src/runtime_transaction.rs @@ -51,10 +51,10 @@ impl StaticMeta for RuntimeTransaction { fn signature_details(&self) -> &TransactionSignatureDetails { &self.meta.signature_details } - fn compute_budget_limits(&self, _feature_set: &FeatureSet) -> Result { + fn compute_budget_limits(&self, feature_set: &FeatureSet) -> Result { self.meta .compute_budget_instruction_details - .sanitize_and_convert_to_compute_budget_limits() + .sanitize_and_convert_to_compute_budget_limits(feature_set) } } diff --git a/runtime/src/prioritization_fee_cache.rs b/runtime/src/prioritization_fee_cache.rs index de35bd77e2f41c..25f7cd9bc11fd3 100644 --- a/runtime/src/prioritization_fee_cache.rs +++ b/runtime/src/prioritization_fee_cache.rs @@ -205,15 +205,10 @@ impl PrioritizationFeeCache { continue; } -<<<<<<< HEAD let compute_budget_limits = process_compute_budget_instructions( SVMMessage::program_instructions_iter(sanitized_transaction), + &bank.feature_set, ); -======= - let compute_budget_limits = sanitized_transaction - .compute_budget_instruction_details() - .sanitize_and_convert_to_compute_budget_limits(&bank.feature_set); ->>>>>>> 3e9af14f3a (Fix reserve minimal compute units for builtins (#3799)) let message = sanitized_transaction.message(); let lock_result = validate_account_locks( diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 554fedfd83f845..aab9a66bd52ba2 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -451,13 +451,11 @@ impl TransactionBatchProcessor { rent_collector: &dyn SVMRentCollector, error_counters: &mut TransactionErrorMetrics, ) -> transaction::Result { - let compute_budget_limits = process_compute_budget_instructions( - message.program_instructions_iter(), - &account_loader.feature_set, - ) - .inspect_err(|_err| { - error_counters.invalid_compute_budget += 1; - })?; + let compute_budget_limits = + process_compute_budget_instructions(message.program_instructions_iter(), feature_set) + .inspect_err(|_err| { + error_counters.invalid_compute_budget += 1; + })?; let fee_payer_address = message.fee_payer(); let mut fee_payer_account = if let Some(fee_payer_account) =