From bbb80bcf1e0aeca8d577c8cba775da14360edaaf Mon Sep 17 00:00:00 2001 From: Tao Zhu <82401714+tao-stones@users.noreply.github.com> Date: Wed, 4 Dec 2024 22:49:59 -0500 Subject: [PATCH] Fix reserve minimal compute units for builtins (#3799) - Add feature gate, issue #2562; - Implement SIMD-170; --------- Co-authored-by: Justin Starry (cherry picked from commit 3e9af14f3a145070773c719ad104b6a02aefd718) # Conflicts: # builtins-default-costs/src/lib.rs # core/src/banking_stage/consumer.rs # core/src/banking_stage/immutable_deserialized_packet.rs # core/src/banking_stage/transaction_scheduler/receive_and_buffer.rs # cost-model/src/cost_model.rs # programs/compute-budget-bench/benches/compute_budget.rs # runtime-transaction/src/lib.rs # runtime-transaction/src/runtime_transaction/sdk_transactions.rs # runtime/src/prioritization_fee_cache.rs --- builtins-default-costs/src/lib.rs | 63 +++ compute-budget/src/compute_budget_limits.rs | 3 + core/src/banking_stage/consumer.rs | 13 + .../immutable_deserialized_packet.rs | 12 + .../receive_and_buffer.rs | 376 ++++++++++++++++++ cost-model/src/cost_model.rs | 283 ++++++++++--- cost-model/src/transaction_cost.rs | 7 +- .../benches/compute_budget.rs | 155 ++++++++ programs/sbf/tests/programs.rs | 15 +- .../process_compute_budget_instructions.rs | 234 ++++++----- .../src/builtin_programs_filter.rs | 105 +++++ .../src/compute_budget_instruction_details.rs | 285 +++++++++---- .../src/compute_budget_program_id_filter.rs | 1 - .../src/instructions_processor.rs | 124 +++++- runtime-transaction/src/lib.rs | 5 + .../src/runtime_transaction.rs | 2 +- .../runtime_transaction/sdk_transactions.rs | 346 ++++++++++++++++ runtime/src/bank.rs | 7 +- runtime/src/bank/tests.rs | 7 +- runtime/src/prioritization_fee_cache.rs | 6 + sdk/feature-set/src/lib.rs | 5 + sdk/program/src/message/sanitized.rs | 2 +- sdk/program/src/message/versions/sanitized.rs | 2 +- svm-transaction/src/svm_message.rs | 2 +- .../src/svm_message/sanitized_message.rs | 2 +- .../src/svm_message/sanitized_transaction.rs | 2 +- svm/src/transaction_processor.rs | 25 +- .../src/resolved_transaction_view.rs | 2 +- transaction-view/src/transaction_view.rs | 4 +- 29 files changed, 1800 insertions(+), 295 deletions(-) create mode 100644 core/src/banking_stage/transaction_scheduler/receive_and_buffer.rs create mode 100644 programs/compute-budget-bench/benches/compute_budget.rs create mode 100644 runtime-transaction/src/builtin_programs_filter.rs create mode 100644 runtime-transaction/src/runtime_transaction/sdk_transactions.rs diff --git a/builtins-default-costs/src/lib.rs b/builtins-default-costs/src/lib.rs index 915064b4b79e35..c5b12884f265db 100644 --- a/builtins-default-costs/src/lib.rs +++ b/builtins-default-costs/src/lib.rs @@ -54,3 +54,66 @@ 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/compute-budget/src/compute_budget_limits.rs b/compute-budget/src/compute_budget_limits.rs index 20731a71430332..e8de8e5910c67d 100644 --- a/compute-budget/src/compute_budget_limits.rs +++ b/compute-budget/src/compute_budget_limits.rs @@ -8,6 +8,9 @@ use { /// default heap page cost = 0.5 * 15 ~= 8CU/page pub const DEFAULT_HEAP_COST: u64 = 8; pub const DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT: u32 = 200_000; +// SIMD-170 defines max CUs to be allocated for any builtin program instructions, that +// have not been migrated to sBPF programs. +pub const MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT: u32 = 3_000; pub const MAX_COMPUTE_UNIT_LIMIT: u32 = 1_400_000; pub const MAX_HEAP_FRAME_BYTES: u32 = 256 * 1024; pub const MIN_HEAP_FRAME_BYTES: u32 = HEAP_LENGTH as u32; diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index bb76753d77951e..08dbd8ebaf17a1 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -574,11 +574,19 @@ impl Consumer { .sanitized_transactions() .iter() .filter_map(|transaction| { +<<<<<<< HEAD process_compute_budget_instructions(SVMMessage::program_instructions_iter( transaction, )) .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) = @@ -757,7 +765,12 @@ 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 978e4f9b935c7e..acde8ce5583b00 100644 --- a/core/src/banking_stage/immutable_deserialized_packet.rs +++ b/core/src/banking_stage/immutable_deserialized_packet.rs @@ -7,6 +7,7 @@ use { solana_sanitize::SanitizeError, solana_sdk::{ clock::Slot, + feature_set::FeatureSet, hash::Hash, message::{v0::LoadedAddresses, AddressLoaderError, Message, SimpleAddressLoader}, pubkey::Pubkey, @@ -40,7 +41,17 @@ 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)) pub struct ImmutableDeserializedPacket { original_packet: Packet, transaction: SanitizedVersionedTransaction, @@ -68,6 +79,7 @@ impl ImmutableDeserializedPacket { .get_message() .program_instructions_iter() .map(|(pubkey, ix)| (pubkey, SVMInstruction::from(ix))), + &FEATURE_SET, ) .map_err(|_| DeserializedPacketError::PrioritizationFailure)?; diff --git a/core/src/banking_stage/transaction_scheduler/receive_and_buffer.rs b/core/src/banking_stage/transaction_scheduler/receive_and_buffer.rs new file mode 100644 index 00000000000000..21afe448d151d3 --- /dev/null +++ b/core/src/banking_stage/transaction_scheduler/receive_and_buffer.rs @@ -0,0 +1,376 @@ +use { + super::{ + scheduler_metrics::{SchedulerCountMetrics, SchedulerTimingMetrics}, + transaction_state_container::StateContainer, + }, + crate::banking_stage::{ + decision_maker::BufferedPacketsDecision, + immutable_deserialized_packet::ImmutableDeserializedPacket, + packet_deserializer::PacketDeserializer, scheduler_messages::MaxAge, + transaction_scheduler::transaction_state::SanitizedTransactionTTL, + TransactionStateContainer, + }, + arrayvec::ArrayVec, + core::time::Duration, + crossbeam_channel::RecvTimeoutError, + solana_accounts_db::account_locks::validate_account_locks, + solana_cost_model::cost_model::CostModel, + solana_measure::measure_us, + solana_runtime::{bank::Bank, bank_forks::BankForks}, + solana_runtime_transaction::{ + runtime_transaction::RuntimeTransaction, transaction_meta::StaticMeta, + transaction_with_meta::TransactionWithMeta, + }, + solana_sdk::{ + address_lookup_table::state::estimate_last_valid_slot, + clock::{Epoch, Slot, MAX_PROCESSING_AGE}, + fee::FeeBudgetLimits, + saturating_add_assign, + transaction::SanitizedTransaction, + }, + solana_svm::transaction_error_metrics::TransactionErrorMetrics, + std::sync::{Arc, RwLock}, +}; + +pub(crate) trait ReceiveAndBuffer { + type Transaction: TransactionWithMeta; + type Container: StateContainer; + + /// Returns whether the packet receiver is still connected. + fn receive_and_buffer_packets( + &mut self, + container: &mut Self::Container, + timing_metrics: &mut SchedulerTimingMetrics, + count_metrics: &mut SchedulerCountMetrics, + decision: &BufferedPacketsDecision, + ) -> bool; +} + +pub(crate) struct SanitizedTransactionReceiveAndBuffer { + /// Packet/Transaction ingress. + packet_receiver: PacketDeserializer, + bank_forks: Arc>, + + forwarding_enabled: bool, +} + +impl ReceiveAndBuffer for SanitizedTransactionReceiveAndBuffer { + type Transaction = RuntimeTransaction; + type Container = TransactionStateContainer; + + /// Returns whether the packet receiver is still connected. + fn receive_and_buffer_packets( + &mut self, + container: &mut Self::Container, + timing_metrics: &mut SchedulerTimingMetrics, + count_metrics: &mut SchedulerCountMetrics, + decision: &BufferedPacketsDecision, + ) -> bool { + let remaining_queue_capacity = container.remaining_capacity(); + + const MAX_PACKET_RECEIVE_TIME: Duration = Duration::from_millis(10); + let (recv_timeout, should_buffer) = match decision { + BufferedPacketsDecision::Consume(_) => ( + if container.is_empty() { + MAX_PACKET_RECEIVE_TIME + } else { + Duration::ZERO + }, + true, + ), + BufferedPacketsDecision::Forward => (MAX_PACKET_RECEIVE_TIME, self.forwarding_enabled), + BufferedPacketsDecision::ForwardAndHold | BufferedPacketsDecision::Hold => { + (MAX_PACKET_RECEIVE_TIME, true) + } + }; + + let (received_packet_results, receive_time_us) = measure_us!(self + .packet_receiver + .receive_packets(recv_timeout, remaining_queue_capacity, |packet| { + packet.check_excessive_precompiles()?; + Ok(packet) + })); + + timing_metrics.update(|timing_metrics| { + saturating_add_assign!(timing_metrics.receive_time_us, receive_time_us); + }); + + match received_packet_results { + Ok(receive_packet_results) => { + let num_received_packets = receive_packet_results.deserialized_packets.len(); + + count_metrics.update(|count_metrics| { + saturating_add_assign!(count_metrics.num_received, num_received_packets); + }); + + if should_buffer { + let (_, buffer_time_us) = measure_us!(self.buffer_packets( + container, + timing_metrics, + count_metrics, + receive_packet_results.deserialized_packets + )); + timing_metrics.update(|timing_metrics| { + saturating_add_assign!(timing_metrics.buffer_time_us, buffer_time_us); + }); + } else { + count_metrics.update(|count_metrics| { + saturating_add_assign!( + count_metrics.num_dropped_on_receive, + num_received_packets + ); + }); + } + } + Err(RecvTimeoutError::Timeout) => {} + Err(RecvTimeoutError::Disconnected) => return false, + } + + true + } +} + +impl SanitizedTransactionReceiveAndBuffer { + pub fn new( + packet_receiver: PacketDeserializer, + bank_forks: Arc>, + forwarding_enabled: bool, + ) -> Self { + Self { + packet_receiver, + bank_forks, + forwarding_enabled, + } + } + + fn buffer_packets( + &mut self, + container: &mut TransactionStateContainer>, + _timing_metrics: &mut SchedulerTimingMetrics, + count_metrics: &mut SchedulerCountMetrics, + packets: Vec, + ) { + // Convert to Arcs + let packets: Vec<_> = packets.into_iter().map(Arc::new).collect(); + // Sanitize packets, generate IDs, and insert into the container. + let (root_bank, working_bank) = { + let bank_forks = self.bank_forks.read().unwrap(); + let root_bank = bank_forks.root_bank(); + let working_bank = bank_forks.working_bank(); + (root_bank, working_bank) + }; + let alt_resolved_slot = root_bank.slot(); + let sanitized_epoch = root_bank.epoch(); + let transaction_account_lock_limit = working_bank.get_transaction_account_lock_limit(); + let vote_only = working_bank.vote_only_bank(); + + const CHUNK_SIZE: usize = 128; + let lock_results: [_; CHUNK_SIZE] = core::array::from_fn(|_| Ok(())); + + let mut arc_packets = ArrayVec::<_, CHUNK_SIZE>::new(); + let mut transactions = ArrayVec::<_, CHUNK_SIZE>::new(); + let mut max_ages = ArrayVec::<_, CHUNK_SIZE>::new(); + let mut fee_budget_limits_vec = ArrayVec::<_, CHUNK_SIZE>::new(); + + let mut error_counts = TransactionErrorMetrics::default(); + for chunk in packets.chunks(CHUNK_SIZE) { + let mut post_sanitization_count: usize = 0; + chunk + .iter() + .filter_map(|packet| { + packet + .build_sanitized_transaction( + vote_only, + root_bank.as_ref(), + root_bank.get_reserved_account_keys(), + ) + .map(|(tx, deactivation_slot)| (packet.clone(), tx, deactivation_slot)) + }) + .inspect(|_| saturating_add_assign!(post_sanitization_count, 1)) + .filter(|(_packet, tx, _deactivation_slot)| { + validate_account_locks( + tx.message().account_keys(), + transaction_account_lock_limit, + ) + .is_ok() + }) + .filter_map(|(packet, tx, deactivation_slot)| { + tx.compute_budget_instruction_details() + .sanitize_and_convert_to_compute_budget_limits(&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); + transactions.push(tx); + max_ages.push(calculate_max_age( + sanitized_epoch, + deactivation_slot, + alt_resolved_slot, + )); + fee_budget_limits_vec.push(fee_budget_limits); + }); + + let check_results = working_bank.check_transactions( + &transactions, + &lock_results[..transactions.len()], + MAX_PROCESSING_AGE, + &mut error_counts, + ); + let post_lock_validation_count = transactions.len(); + + let mut post_transaction_check_count: usize = 0; + let mut num_dropped_on_capacity: usize = 0; + let mut num_buffered: usize = 0; + for ((((packet, transaction), max_age), fee_budget_limits), _check_result) in + arc_packets + .drain(..) + .zip(transactions.drain(..)) + .zip(max_ages.drain(..)) + .zip(fee_budget_limits_vec.drain(..)) + .zip(check_results) + .filter(|(_, check_result)| check_result.is_ok()) + { + saturating_add_assign!(post_transaction_check_count, 1); + + let (priority, cost) = + calculate_priority_and_cost(&transaction, &fee_budget_limits, &working_bank); + let transaction_ttl = SanitizedTransactionTTL { + transaction, + max_age, + }; + + if container.insert_new_transaction(transaction_ttl, packet, priority, cost) { + saturating_add_assign!(num_dropped_on_capacity, 1); + } + saturating_add_assign!(num_buffered, 1); + } + + // Update metrics for transactions that were dropped. + let num_dropped_on_sanitization = chunk.len().saturating_sub(post_sanitization_count); + let num_dropped_on_lock_validation = + post_sanitization_count.saturating_sub(post_lock_validation_count); + let num_dropped_on_transaction_checks = + post_lock_validation_count.saturating_sub(post_transaction_check_count); + + count_metrics.update(|count_metrics| { + saturating_add_assign!( + count_metrics.num_dropped_on_capacity, + num_dropped_on_capacity + ); + saturating_add_assign!(count_metrics.num_buffered, num_buffered); + saturating_add_assign!( + count_metrics.num_dropped_on_sanitization, + num_dropped_on_sanitization + ); + saturating_add_assign!( + count_metrics.num_dropped_on_validate_locks, + num_dropped_on_lock_validation + ); + saturating_add_assign!( + count_metrics.num_dropped_on_receive_transaction_checks, + num_dropped_on_transaction_checks + ); + }); + } + } +} + +/// Calculate priority and cost for a transaction: +/// +/// Cost is calculated through the `CostModel`, +/// and priority is calculated through a formula here that attempts to sell +/// blockspace to the highest bidder. +/// +/// The priority is calculated as: +/// P = R / (1 + C) +/// where P is the priority, R is the reward, +/// and C is the cost towards block-limits. +/// +/// Current minimum costs are on the order of several hundred, +/// so the denominator is effectively C, and the +1 is simply +/// to avoid any division by zero due to a bug - these costs +/// are calculated by the cost-model and are not direct +/// from user input. They should never be zero. +/// Any difference in the prioritization is negligible for +/// the current transaction costs. +fn calculate_priority_and_cost( + transaction: &RuntimeTransaction, + fee_budget_limits: &FeeBudgetLimits, + bank: &Bank, +) -> (u64, u64) { + let cost = CostModel::calculate_cost(transaction, &bank.feature_set).sum(); + let reward = bank.calculate_reward_for_transaction(transaction, fee_budget_limits); + + // We need a multiplier here to avoid rounding down too aggressively. + // For many transactions, the cost will be greater than the fees in terms of raw lamports. + // For the purposes of calculating prioritization, we multiply the fees by a large number so that + // the cost is a small fraction. + // An offset of 1 is used in the denominator to explicitly avoid division by zero. + const MULTIPLIER: u64 = 1_000_000; + ( + reward + .saturating_mul(MULTIPLIER) + .saturating_div(cost.saturating_add(1)), + cost, + ) +} + +/// Given the epoch, the minimum deactivation slot, and the current slot, +/// return the `MaxAge` that should be used for the transaction. This is used +/// to determine the maximum slot that a transaction will be considered valid +/// for, without re-resolving addresses or resanitizing. +/// +/// This function considers the deactivation period of Address Table +/// accounts. If the deactivation period runs past the end of the epoch, +/// then the transaction is considered valid until the end of the epoch. +/// Otherwise, the transaction is considered valid until the deactivation +/// period. +/// +/// Since the deactivation period technically uses blocks rather than +/// slots, the value used here is the lower-bound on the deactivation +/// period, i.e. the transaction's address lookups are valid until +/// AT LEAST this slot. +fn calculate_max_age( + sanitized_epoch: Epoch, + deactivation_slot: Slot, + current_slot: Slot, +) -> MaxAge { + let alt_min_expire_slot = estimate_last_valid_slot(deactivation_slot.min(current_slot)); + MaxAge { + sanitized_epoch, + alt_invalidation_slot: alt_min_expire_slot, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_calculate_max_age() { + let current_slot = 100; + let sanitized_epoch = 10; + + // ALT deactivation slot is delayed + assert_eq!( + calculate_max_age(sanitized_epoch, current_slot - 1, current_slot), + MaxAge { + sanitized_epoch, + alt_invalidation_slot: current_slot - 1 + + solana_sdk::slot_hashes::get_entries() as u64, + } + ); + + // no deactivation slot + assert_eq!( + calculate_max_age(sanitized_epoch, u64::MAX, current_slot), + MaxAge { + sanitized_epoch, + alt_invalidation_slot: current_slot + solana_sdk::slot_hashes::get_entries() as u64, + } + ); + } +} diff --git a/cost-model/src/cost_model.rs b/cost-model/src/cost_model.rs index 6fb9eee2d86158..7df59673d23a76 100644 --- a/cost-model/src/cost_model.rs +++ b/cost-model/src/cost_model.rs @@ -166,6 +166,24 @@ impl CostModel { fn get_transaction_cost( transaction: &impl SVMMessage, feature_set: &FeatureSet, + ) -> (u64, u64, u64) { + if feature_set.is_active(&feature_set::reserve_minimal_cus_for_builtin_instructions::id()) { + let data_bytes_cost = Self::get_instructions_data_cost(transaction); + let (programs_execution_cost, loaded_accounts_data_size_cost) = + Self::get_estimated_execution_cost(transaction, feature_set); + ( + programs_execution_cost, + loaded_accounts_data_size_cost, + data_bytes_cost, + ) + } else { + Self::get_transaction_cost_without_minimal_builtin_cus(transaction, feature_set) + } + } + + fn get_transaction_cost_without_minimal_builtin_cus( + transaction: &impl TransactionWithMeta, + feature_set: &FeatureSet, ) -> (u64, u64, u64) { let mut programs_execution_costs = 0u64; let mut loaded_accounts_data_size_cost = 0u64; @@ -198,9 +216,19 @@ 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)) 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 @@ -228,6 +256,30 @@ impl CostModel { ) } + /// Return (programs_execution_cost, loaded_accounts_data_size_cost) + fn get_estimated_execution_cost( + transaction: &impl TransactionWithMeta, + 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, + ), + ), + 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 { let ix_data_bytes_len_total: u64 = transaction @@ -322,7 +374,14 @@ 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, @@ -520,11 +579,8 @@ mod tests { let simple_transaction = SanitizedTransaction::from_transaction_for_tests( system_transaction::transfer(&mint_keypair, &keypair.pubkey(), 2, start_hash), ); - debug!( - "system_transaction simple_transaction {:?}", - simple_transaction - ); +<<<<<<< HEAD // expected cost for one system transfer instructions let expected_execution_cost = BUILTIN_INSTRUCTION_COSTS .get(&system_program::id()) @@ -535,6 +591,23 @@ mod tests { assert_eq!(*expected_execution_cost, program_execution_cost); assert_eq!(3, data_bytes_cost); +======= + for (feature_set, expected_execution_cost) in [ + ( + FeatureSet::default(), + solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS, + ), + ( + FeatureSet::all_enabled(), + u64::from(MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT), + ), + ] { + let (program_execution_cost, _loaded_accounts_data_size_cost, _data_bytes_cost) = + CostModel::get_transaction_cost(&simple_transaction, &feature_set); + + assert_eq!(expected_execution_cost, program_execution_cost); + } +>>>>>>> 3e9af14f3a (Fix reserve minimal compute units for builtins (#3799)) } #[test] @@ -552,16 +625,29 @@ 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 [ + ( + FeatureSet::default(), + DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64, + ), + ( + FeatureSet::all_enabled(), + DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64, + ), + ] { + let (program_execution_cost, _loaded_accounts_data_size_cost, data_bytes_cost) = + CostModel::get_transaction_cost(&token_transaction, &feature_set); - let (program_execution_cost, _loaded_accounts_data_size_cost, data_bytes_cost) = - CostModel::get_transaction_cost(&token_transaction, &FeatureSet::all_enabled()); - assert_eq!( - DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64, - program_execution_cost - ); - assert_eq!(0, data_bytes_cost); + assert_eq!(expected_execution_cost, program_execution_cost); + assert_eq!(0, data_bytes_cost); + } } #[test] @@ -593,12 +679,13 @@ mod tests { #[test] fn test_cost_model_compute_budget_transaction() { let (mint_keypair, start_hash) = test_setup(); + let expected_cu_limit = 12_345; let instructions = vec![ CompiledInstruction::new(3, &(), vec![1, 2, 0]), CompiledInstruction::new_from_raw_parts( 4, - ComputeBudgetInstruction::SetComputeUnitLimit(12_345) + ComputeBudgetInstruction::SetComputeUnitLimit(expected_cu_limit) .pack() .unwrap(), vec![], @@ -616,12 +703,17 @@ mod tests { ); let token_transaction = SanitizedTransaction::from_transaction_for_tests(tx); - let (program_execution_cost, _loaded_accounts_data_size_cost, data_bytes_cost) = - CostModel::get_transaction_cost(&token_transaction, &FeatureSet::all_enabled()); - // If cu-limit is specified, that would the cost for all programs - assert_eq!(12_345, program_execution_cost); - assert_eq!(1, data_bytes_cost); + for (feature_set, expected_execution_cost) in [ + (FeatureSet::default(), expected_cu_limit as u64), + (FeatureSet::all_enabled(), expected_cu_limit as u64), + ] { + let (program_execution_cost, _loaded_accounts_data_size_cost, data_bytes_cost) = + CostModel::get_transaction_cost(&token_transaction, &feature_set); + + assert_eq!(expected_execution_cost, program_execution_cost); + assert_eq!(1, data_bytes_cost); + } } #[test] @@ -658,9 +750,11 @@ mod tests { ); let token_transaction = SanitizedTransaction::from_transaction_for_tests(tx); - let (program_execution_cost, _loaded_accounts_data_size_cost, _data_bytes_cost) = - CostModel::get_transaction_cost(&token_transaction, &FeatureSet::all_enabled()); - assert_eq!(0, program_execution_cost); + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + let (program_execution_cost, _loaded_accounts_data_size_cost, _data_bytes_cost) = + CostModel::get_transaction_cost(&token_transaction, &feature_set); + assert_eq!(0, program_execution_cost); + } } #[test] @@ -677,9 +771,9 @@ mod tests { message, start_hash, )); - debug!("many transfer transaction {:?}", tx); // expected cost for two system transfer instructions +<<<<<<< HEAD let program_cost = BUILTIN_INSTRUCTION_COSTS .get(&system_program::id()) .unwrap(); @@ -689,6 +783,23 @@ mod tests { 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(), + 2 * solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS, + ), + ( + FeatureSet::all_enabled(), + 2 * u64::from(MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT), + ), + ] { + let (programs_execution_cost, _loaded_accounts_data_size_cost, data_bytes_cost) = + CostModel::get_transaction_cost(&tx, &feature_set); + assert_eq!(expected_execution_cost, programs_execution_cost); + assert_eq!(6, data_bytes_cost); + } +>>>>>>> 3e9af14f3a (Fix reserve minimal compute units for builtins (#3799)) } #[test] @@ -713,13 +824,22 @@ mod tests { instructions, ), ); - debug!("many random transaction {:?}", tx); - let expected_cost = DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64 * 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!(0, data_bytes_cost); + for (feature_set, expected_cost) in [ + ( + FeatureSet::default(), + DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64 * 2, + ), + ( + FeatureSet::all_enabled(), + DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64 * 2, + ), + ] { + let (program_execution_cost, _loaded_accounts_data_size_cost, data_bytes_cost) = + CostModel::get_transaction_cost(&tx, &feature_set); + assert_eq!(expected_cost, program_execution_cost); + assert_eq!(0, data_bytes_cost); + } } #[test] @@ -765,6 +885,7 @@ mod tests { )); let expected_account_cost = WRITE_LOCK_UNITS * 2; +<<<<<<< HEAD let expected_execution_cost = BUILTIN_INSTRUCTION_COSTS .get(&system_program::id()) .unwrap(); @@ -783,6 +904,34 @@ mod tests { expected_loaded_accounts_data_size_cost, tx_cost.loaded_accounts_data_size_cost() ); +======= + for (feature_set, expected_execution_cost) in [ + ( + FeatureSet::default(), + solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS, + ), + ( + FeatureSet::all_enabled(), + u64::from(MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT), + ), + ] { + 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, &feature_set); + 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() + ); + } +>>>>>>> 3e9af14f3a (Fix reserve minimal compute units for builtins (#3799)) } #[test] @@ -801,8 +950,8 @@ mod tests { start_hash, )); - let feature_set = FeatureSet::all_enabled(); let expected_account_cost = WRITE_LOCK_UNITS * 2; +<<<<<<< HEAD let expected_execution_cost = BUILTIN_INSTRUCTION_COSTS .get(&system_program::id()) .unwrap() @@ -810,15 +959,30 @@ mod tests { .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(), + solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS + + solana_compute_budget_program::DEFAULT_COMPUTE_UNITS, + ), + ( + FeatureSet::all_enabled(), + 2 * u64::from(MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT), + ), + ] { + 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()); - 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() - ); + let tx_cost = CostModel::calculate_cost(&tx, &feature_set); + 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() + ); + } } #[test] @@ -836,34 +1000,48 @@ 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(), + solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS + + DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64, + ), + ( + FeatureSet::all_enabled(), + u64::from(MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT) + + u64::from(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT), + ), + ] { + 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)) - let (program_execution_cost, _loaded_accounts_data_size_cost, _data_bytes_cost) = - CostModel::get_transaction_cost(&transaction, &FeatureSet::all_enabled()); - - assert_eq!( - expected_builtin_cost + expected_bpf_cost as u64, - program_execution_cost - ); + assert_eq!(expected_execution_cost, programs_execution_cost); + } } #[test] fn test_transaction_cost_with_mix_instruction_with_cu_limit() { let (mint_keypair, start_hash) = test_setup(); + let cu_limit: u32 = 12_345; let transaction = SanitizedTransaction::from_transaction_for_tests(Transaction::new_signed_with_payer( &[ system_instruction::transfer(&mint_keypair.pubkey(), &Pubkey::new_unique(), 2), - ComputeBudgetInstruction::set_compute_unit_limit(12_345), + ComputeBudgetInstruction::set_compute_unit_limit(cu_limit), ], Some(&mint_keypair.pubkey()), &[&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()) @@ -871,9 +1049,20 @@ mod tests { + BUILTIN_INSTRUCTION_COSTS .get(&compute_budget::id()) .unwrap(); +======= + for (feature_set, expected_execution_cost) in [ + ( + FeatureSet::default(), + solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS + + solana_compute_budget_program::DEFAULT_COMPUTE_UNITS, + ), + (FeatureSet::all_enabled(), cu_limit as u64), + ] { + 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)) - let (program_execution_cost, _loaded_accounts_data_size_cost, _data_bytes_cost) = - CostModel::get_transaction_cost(&transaction, &FeatureSet::all_enabled()); - assert_eq!(expected_cost, program_execution_cost); + assert_eq!(expected_execution_cost, programs_execution_cost); + } } } diff --git a/cost-model/src/transaction_cost.rs b/cost-model/src/transaction_cost.rs index fcfdfdda195aa1..99c9c5aa4a19a0 100644 --- a/cost-model/src/transaction_cost.rs +++ b/cost-model/src/transaction_cost.rs @@ -182,7 +182,8 @@ impl SVMMessage for WritableKeysTransaction { fn program_instructions_iter( &self, - ) -> impl Iterator { + ) -> impl Iterator + Clone + { core::iter::empty() } @@ -272,8 +273,8 @@ mod tests { // expected vote tx cost: 2 write locks, 1 sig, 1 vote ix, 8cu of loaded accounts size, let expected_vote_cost = SIMPLE_VOTE_USAGE_COST; - // expected non-vote tx cost would include default loaded accounts size cost (16384) additionally - let expected_none_vote_cost = 20543; + // expected non-vote tx cost would include default loaded accounts size cost (16384) additionally, and 3_000 for instruction + let expected_none_vote_cost = 21443; let vote_cost = CostModel::calculate_cost(&vote_transaction, &FeatureSet::all_enabled()); let none_vote_cost = diff --git a/programs/compute-budget-bench/benches/compute_budget.rs b/programs/compute-budget-bench/benches/compute_budget.rs new file mode 100644 index 00000000000000..7c758d45c7ed05 --- /dev/null +++ b/programs/compute-budget-bench/benches/compute_budget.rs @@ -0,0 +1,155 @@ +use { + criterion::{black_box, criterion_group, criterion_main, Criterion}, + solana_compute_budget::compute_budget_limits::ComputeBudgetLimits, + solana_runtime_transaction::instructions_processor::process_compute_budget_instructions, + solana_sdk::{ + compute_budget::ComputeBudgetInstruction, feature_set::FeatureSet, + instruction::CompiledInstruction, + }, + solana_svm_transaction::instruction::SVMInstruction, + std::num::NonZero, +}; + +const ONE_PAGE: u32 = 32 * 1024; +const SIXTY_FOUR_MB: u32 = 64 * 1024 * 1024; + +fn bench_request_heap_frame(c: &mut Criterion) { + let instruction = [( + solana_sdk::compute_budget::id(), + CompiledInstruction::new_from_raw_parts( + 0, + ComputeBudgetInstruction::request_heap_frame(ONE_PAGE).data, + vec![], + ), + )]; + let feature_set = FeatureSet::default(); + + c.bench_function("request_heap_limit", |bencher| { + bencher.iter(|| { + assert_eq!( + process_compute_budget_instructions( + black_box( + instruction + .iter() + .map(|(id, ix)| (id, SVMInstruction::from(ix))) + ), + black_box(&feature_set) + ), + Ok(ComputeBudgetLimits { + updated_heap_bytes: ONE_PAGE, + compute_unit_limit: 0, + compute_unit_price: 0, + loaded_accounts_bytes: NonZero::new(SIXTY_FOUR_MB).unwrap() + }) + ) + }) + }); +} + +fn bench_set_compute_unit_limit(c: &mut Criterion) { + let instruction = [( + solana_sdk::compute_budget::id(), + CompiledInstruction::new_from_raw_parts( + 0, + ComputeBudgetInstruction::set_compute_unit_limit(1024).data, + vec![], + ), + )]; + let feature_set = FeatureSet::default(); + + c.bench_function("set_compute_unit_limit", |bencher| { + bencher.iter(|| { + assert_eq!( + process_compute_budget_instructions( + black_box( + instruction + .iter() + .map(|(id, ix)| (id, SVMInstruction::from(ix))) + ), + black_box(&feature_set) + ), + Ok(ComputeBudgetLimits { + updated_heap_bytes: ONE_PAGE, + compute_unit_limit: 1024, + compute_unit_price: 0, + loaded_accounts_bytes: NonZero::new(SIXTY_FOUR_MB).unwrap() + }) + ) + }) + }); +} + +fn bench_set_compute_unit_price(c: &mut Criterion) { + let instruction = [( + solana_sdk::compute_budget::id(), + CompiledInstruction::new_from_raw_parts( + 0, + ComputeBudgetInstruction::set_compute_unit_price(1).data, + vec![], + ), + )]; + let feature_set = FeatureSet::default(); + + c.bench_function("set_compute_unit_price", |bencher| { + bencher.iter(|| { + assert_eq!( + process_compute_budget_instructions( + black_box( + instruction + .iter() + .map(|(id, ix)| (id, SVMInstruction::from(ix))) + ), + black_box(&feature_set) + ), + Ok(ComputeBudgetLimits { + updated_heap_bytes: ONE_PAGE, + compute_unit_limit: 0, + compute_unit_price: 1, + loaded_accounts_bytes: NonZero::new(SIXTY_FOUR_MB).unwrap() + }) + ) + }) + }); +} + +fn bench_set_loaded_accounts_data_size_limit(c: &mut Criterion) { + let instruction = [( + solana_sdk::compute_budget::id(), + CompiledInstruction::new_from_raw_parts( + 0, + ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(1).data, + vec![], + ), + )]; + let feature_set = FeatureSet::default(); + + c.bench_function("set_loaded_accounts_data_size_limit", |bencher| { + bencher.iter(|| { + assert_eq!( + process_compute_budget_instructions( + black_box( + instruction + .iter() + .map(|(id, ix)| (id, SVMInstruction::from(ix))) + ), + black_box(&feature_set) + ), + Ok(ComputeBudgetLimits { + updated_heap_bytes: ONE_PAGE, + compute_unit_limit: 0, + compute_unit_price: 0, + loaded_accounts_bytes: NonZero::new(1).unwrap() + }) + ) + }) + }); +} + +criterion_group!( + benches, + bench_request_heap_frame, + bench_set_compute_unit_limit, + bench_set_compute_unit_price, + bench_set_loaded_accounts_data_size_limit, +); +criterion_main!(benches); diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index e7a51b6cdb151b..7b240348026e5f 100644 --- a/programs/sbf/tests/programs.rs +++ b/programs/sbf/tests/programs.rs @@ -3857,6 +3857,7 @@ fn test_program_fees() { FeeStructure::new(0.000005, 0.0, vec![(200, 0.0000005), (1400000, 0.000005)]); bank.set_fee_structure(&fee_structure); let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests(); + let feature_set = bank.feature_set.clone(); let mut bank_client = BankClient::new_shared(bank); let authority_keypair = Keypair::new(); @@ -3880,9 +3881,10 @@ fn test_program_fees() { ) .unwrap(); let fee_budget_limits = FeeBudgetLimits::from( - process_compute_budget_instructions(SVMMessage::program_instructions_iter( - &sanitized_message, - )) + process_compute_budget_instructions( + SVMMessage::program_instructions_iter(&sanitized_message), + &feature_set, + ) .unwrap_or_default(), ); let expected_normal_fee = solana_fee::calculate_fee( @@ -3912,9 +3914,10 @@ fn test_program_fees() { ) .unwrap(); let fee_budget_limits = FeeBudgetLimits::from( - process_compute_budget_instructions(SVMMessage::program_instructions_iter( - &sanitized_message, - )) + process_compute_budget_instructions( + SVMMessage::program_instructions_iter(&sanitized_message), + &feature_set, + ) .unwrap_or_default(), ); let expected_prioritized_fee = solana_fee::calculate_fee( diff --git a/runtime-transaction/benches/process_compute_budget_instructions.rs b/runtime-transaction/benches/process_compute_budget_instructions.rs index 76f6b590948875..c120b5681b5c29 100644 --- a/runtime-transaction/benches/process_compute_budget_instructions.rs +++ b/runtime-transaction/benches/process_compute_budget_instructions.rs @@ -3,6 +3,7 @@ use { solana_runtime_transaction::instructions_processor::process_compute_budget_instructions, solana_sdk::{ compute_budget::ComputeBudgetInstruction, + feature_set::FeatureSet, instruction::Instruction, message::Message, pubkey::Pubkey, @@ -28,134 +29,153 @@ fn build_sanitized_transaction( } fn bench_process_compute_budget_instructions_empty(c: &mut Criterion) { - c.benchmark_group("bench_process_compute_budget_instructions_empty") - .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) - .bench_function("0 instructions", |bencher| { - let tx = build_sanitized_transaction(&Keypair::new(), &[]); - bencher.iter(|| { - (0..NUM_TRANSACTIONS_PER_ITER).for_each(|_| { - assert!(process_compute_budget_instructions(black_box( - SVMMessage::program_instructions_iter(&tx) - )) - .is_ok()) - }) + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + c.benchmark_group("bench_process_compute_budget_instructions_empty") + .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) + .bench_function("0 instructions", |bencher| { + let tx = build_sanitized_transaction(&Keypair::new(), &[]); + bencher.iter(|| { + (0..NUM_TRANSACTIONS_PER_ITER).for_each(|_| { + assert!(process_compute_budget_instructions( + black_box(SVMMessage::program_instructions_iter(&tx)), + black_box(&feature_set), + ) + .is_ok()) + }) + }); }); - }); + } } fn bench_process_compute_budget_instructions_no_builtins(c: &mut Criterion) { let num_instructions = 4; - c.benchmark_group("bench_process_compute_budget_instructions_no_builtins") - .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) - .bench_function( - format!("{num_instructions} dummy Instructions"), - |bencher| { - let ixs: Vec<_> = (0..num_instructions) - .map(|_| { - Instruction::new_with_bincode( - DUMMY_PROGRAM_ID.parse().unwrap(), - &(), - vec![], - ) - }) - .collect(); + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + c.benchmark_group("bench_process_compute_budget_instructions_no_builtins") + .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) + .bench_function( + format!("{num_instructions} dummy Instructions"), + |bencher| { + let ixs: Vec<_> = (0..num_instructions) + .map(|_| { + Instruction::new_with_bincode( + DUMMY_PROGRAM_ID.parse().unwrap(), + &(), + vec![], + ) + }) + .collect(); + let tx = build_sanitized_transaction(&Keypair::new(), &ixs); + bencher.iter(|| { + (0..NUM_TRANSACTIONS_PER_ITER).for_each(|_| { + assert!(process_compute_budget_instructions( + black_box(SVMMessage::program_instructions_iter(&tx)), + black_box(&feature_set), + ) + .is_ok()) + }) + }); + }, + ); + } +} + +fn bench_process_compute_budget_instructions_compute_budgets(c: &mut Criterion) { + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + c.benchmark_group("bench_process_compute_budget_instructions_compute_budgets") + .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) + .bench_function("4 compute-budget instructions", |bencher| { + let ixs = vec![ + ComputeBudgetInstruction::request_heap_frame(40 * 1024), + ComputeBudgetInstruction::set_compute_unit_limit(u32::MAX), + ComputeBudgetInstruction::set_compute_unit_price(u64::MAX), + ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(u32::MAX), + ]; let tx = build_sanitized_transaction(&Keypair::new(), &ixs); bencher.iter(|| { (0..NUM_TRANSACTIONS_PER_ITER).for_each(|_| { - assert!(process_compute_budget_instructions(black_box( - SVMMessage::program_instructions_iter(&tx) - )) + assert!(process_compute_budget_instructions( + black_box(SVMMessage::program_instructions_iter(&tx)), + black_box(&feature_set), + ) .is_ok()) }) }); - }, - ); -} - -fn bench_process_compute_budget_instructions_compute_budgets(c: &mut Criterion) { - c.benchmark_group("bench_process_compute_budget_instructions_compute_budgets") - .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) - .bench_function("4 compute-budget instructions", |bencher| { - let ixs = vec![ - ComputeBudgetInstruction::request_heap_frame(40 * 1024), - ComputeBudgetInstruction::set_compute_unit_limit(u32::MAX), - ComputeBudgetInstruction::set_compute_unit_price(u64::MAX), - ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(u32::MAX), - ]; - let tx = build_sanitized_transaction(&Keypair::new(), &ixs); - bencher.iter(|| { - (0..NUM_TRANSACTIONS_PER_ITER).for_each(|_| { - assert!(process_compute_budget_instructions(black_box( - SVMMessage::program_instructions_iter(&tx) - )) - .is_ok()) - }) }); - }); + } } fn bench_process_compute_budget_instructions_builtins(c: &mut Criterion) { - c.benchmark_group("bench_process_compute_budget_instructions_builtins") - .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) - .bench_function("4 dummy builtins", |bencher| { - let ixs = vec![ - Instruction::new_with_bincode(solana_sdk::bpf_loader::id(), &(), vec![]), - Instruction::new_with_bincode(solana_sdk::secp256k1_program::id(), &(), vec![]), - Instruction::new_with_bincode( - solana_sdk::address_lookup_table::program::id(), - &(), - vec![], - ), - Instruction::new_with_bincode(solana_sdk::loader_v4::id(), &(), vec![]), - ]; - let tx = build_sanitized_transaction(&Keypair::new(), &ixs); - bencher.iter(|| { - (0..NUM_TRANSACTIONS_PER_ITER).for_each(|_| { - assert!(process_compute_budget_instructions(black_box( - SVMMessage::program_instructions_iter(&tx) - )) - .is_ok()) - }) + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + c.benchmark_group("bench_process_compute_budget_instructions_builtins") + .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) + .bench_function("4 dummy builtins", |bencher| { + let ixs = vec![ + Instruction::new_with_bincode(solana_sdk::bpf_loader::id(), &(), vec![]), + Instruction::new_with_bincode(solana_sdk::secp256k1_program::id(), &(), vec![]), + Instruction::new_with_bincode( + solana_sdk::address_lookup_table::program::id(), + &(), + vec![], + ), + Instruction::new_with_bincode(solana_sdk::loader_v4::id(), &(), vec![]), + ]; + let tx = build_sanitized_transaction(&Keypair::new(), &ixs); + bencher.iter(|| { + (0..NUM_TRANSACTIONS_PER_ITER).for_each(|_| { + assert!(process_compute_budget_instructions( + black_box(SVMMessage::program_instructions_iter(&tx)), + black_box(&feature_set), + ) + .is_ok()) + }) + }); }); - }); + } } fn bench_process_compute_budget_instructions_mixed(c: &mut Criterion) { let num_instructions = 355; - c.benchmark_group("bench_process_compute_budget_instructions_mixed") - .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) - .bench_function( - format!("{num_instructions} mixed instructions"), - |bencher| { - let payer_keypair = Keypair::new(); - let mut ixs: Vec<_> = (0..num_instructions) - .map(|_| { - Instruction::new_with_bincode( - DUMMY_PROGRAM_ID.parse().unwrap(), - &(), - vec![], - ) - }) - .collect(); - ixs.extend(vec![ - ComputeBudgetInstruction::request_heap_frame(40 * 1024), - ComputeBudgetInstruction::set_compute_unit_limit(u32::MAX), - ComputeBudgetInstruction::set_compute_unit_price(u64::MAX), - ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(u32::MAX), - system_instruction::transfer(&payer_keypair.pubkey(), &Pubkey::new_unique(), 1), - ]); - let tx = build_sanitized_transaction(&payer_keypair, &ixs); + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + c.benchmark_group("bench_process_compute_budget_instructions_mixed") + .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) + .bench_function( + format!("{num_instructions} mixed instructions"), + |bencher| { + let payer_keypair = Keypair::new(); + let mut ixs: Vec<_> = (0..num_instructions) + .map(|_| { + Instruction::new_with_bincode( + DUMMY_PROGRAM_ID.parse().unwrap(), + &(), + vec![], + ) + }) + .collect(); + ixs.extend(vec![ + ComputeBudgetInstruction::request_heap_frame(40 * 1024), + ComputeBudgetInstruction::set_compute_unit_limit(u32::MAX), + ComputeBudgetInstruction::set_compute_unit_price(u64::MAX), + ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(u32::MAX), + system_instruction::transfer( + &payer_keypair.pubkey(), + &Pubkey::new_unique(), + 1, + ), + ]); + let tx = build_sanitized_transaction(&payer_keypair, &ixs); - bencher.iter(|| { - (0..NUM_TRANSACTIONS_PER_ITER).for_each(|_| { - assert!(process_compute_budget_instructions(black_box( - SVMMessage::program_instructions_iter(&tx) - )) - .is_ok()) - }) - }); - }, - ); + bencher.iter(|| { + (0..NUM_TRANSACTIONS_PER_ITER).for_each(|_| { + assert!(process_compute_budget_instructions( + black_box(SVMMessage::program_instructions_iter(&tx)), + black_box(&feature_set), + ) + .is_ok()) + }) + }); + }, + ); + } } criterion_group!( diff --git a/runtime-transaction/src/builtin_programs_filter.rs b/runtime-transaction/src/builtin_programs_filter.rs new file mode 100644 index 00000000000000..fc935a023cfbf8 --- /dev/null +++ b/runtime-transaction/src/builtin_programs_filter.rs @@ -0,0 +1,105 @@ +use { + agave_transaction_view::static_account_keys_frame::MAX_STATIC_ACCOUNTS_PER_PACKET as FILTER_SIZE, + solana_builtins_default_costs::{is_builtin_program, MAYBE_BUILTIN_KEY}, + solana_sdk::pubkey::Pubkey, +}; + +#[derive(Clone, Copy, Debug, PartialEq)] +pub(crate) enum ProgramKind { + NotBuiltin, + Builtin, +} + +pub(crate) struct BuiltinProgramsFilter { + // array of slots for all possible static and sanitized program_id_index, + // each slot indicates if a program_id_index has not been checked (eg, None), + // or already checked with result (eg, Some(ProgramKind)) that can be reused. + program_kind: [Option; FILTER_SIZE as usize], +} + +impl BuiltinProgramsFilter { + pub(crate) fn new() -> Self { + BuiltinProgramsFilter { + program_kind: [None; FILTER_SIZE as usize], + } + } + + pub(crate) fn get_program_kind(&mut self, index: usize, program_id: &Pubkey) -> ProgramKind { + *self + .program_kind + .get_mut(index) + .expect("program id index is sanitized") + .get_or_insert_with(|| Self::check_program_kind(program_id)) + } + + #[inline] + fn check_program_kind(program_id: &Pubkey) -> ProgramKind { + if !MAYBE_BUILTIN_KEY[program_id.as_ref()[0] as usize] { + return ProgramKind::NotBuiltin; + } + + if is_builtin_program(program_id) { + ProgramKind::Builtin + } else { + ProgramKind::NotBuiltin + } + } +} + +#[cfg(test)] +mod test { + use super::*; + + const DUMMY_PROGRAM_ID: &str = "dummmy1111111111111111111111111111111111111"; + + #[test] + fn get_program_kind() { + let mut test_store = BuiltinProgramsFilter::new(); + let mut index = 9; + + // initial state is Unchecked + assert!(test_store.program_kind[index].is_none()); + + // non builtin returns None + assert_eq!( + test_store.get_program_kind(index, &DUMMY_PROGRAM_ID.parse().unwrap()), + ProgramKind::NotBuiltin + ); + // but its state is now checked (eg, Some(...)) + assert_eq!( + test_store.program_kind[index], + Some(ProgramKind::NotBuiltin) + ); + // lookup same `index` will return cached data, will not lookup `program_id` + // again + assert_eq!( + test_store.get_program_kind(index, &solana_sdk::loader_v4::id()), + ProgramKind::NotBuiltin + ); + + // not-migrating builtin + index += 1; + assert_eq!( + test_store.get_program_kind(index, &solana_sdk::loader_v4::id()), + ProgramKind::Builtin, + ); + + // compute-budget + index += 1; + assert_eq!( + test_store.get_program_kind(index, &solana_sdk::compute_budget::id()), + ProgramKind::Builtin, + ); + } + + #[test] + #[should_panic(expected = "program id index is sanitized")] + fn test_get_program_kind_out_of_bound_index() { + let mut test_store = BuiltinProgramsFilter::new(); + assert_eq!( + test_store + .get_program_kind(FILTER_SIZE as usize + 1, &DUMMY_PROGRAM_ID.parse().unwrap(),), + ProgramKind::NotBuiltin + ); + } +} diff --git a/runtime-transaction/src/compute_budget_instruction_details.rs b/runtime-transaction/src/compute_budget_instruction_details.rs index ab148ef14ae152..aedc55b07d58e5 100644 --- a/runtime-transaction/src/compute_budget_instruction_details.rs +++ b/runtime-transaction/src/compute_budget_instruction_details.rs @@ -1,9 +1,13 @@ use { - crate::compute_budget_program_id_filter::ComputeBudgetProgramIdFilter, + crate::{ + builtin_programs_filter::{BuiltinProgramsFilter, ProgramKind}, + compute_budget_program_id_filter::ComputeBudgetProgramIdFilter, + }, solana_compute_budget::compute_budget_limits::*, solana_sdk::{ borsh1::try_from_slice_unchecked, compute_budget::ComputeBudgetInstruction, + feature_set::{self, FeatureSet}, instruction::InstructionError, pubkey::Pubkey, saturating_add_assign, @@ -22,17 +26,20 @@ pub(crate) struct ComputeBudgetInstructionDetails { requested_compute_unit_price: Option<(u8, u64)>, requested_heap_size: Option<(u8, u32)>, requested_loaded_accounts_data_size_limit: Option<(u8, u32)>, - num_non_compute_budget_instructions: u32, + num_non_compute_budget_instructions: u16, + // Additional builtin program counters + num_builtin_instructions: u16, + num_non_builtin_instructions: u16, } impl ComputeBudgetInstructionDetails { pub fn try_from<'a>( - instructions: impl Iterator)>, + instructions: impl Iterator)> + Clone, ) -> Result { let mut filter = ComputeBudgetProgramIdFilter::new(); - let mut compute_budget_instruction_details = ComputeBudgetInstructionDetails::default(); - for (i, (program_id, instruction)) in instructions.enumerate() { + + for (i, (program_id, instruction)) in instructions.clone().enumerate() { if filter.is_compute_budget_program(instruction.program_id_index as usize, program_id) { compute_budget_instruction_details.process_instruction(i as u8, &instruction)?; } else { @@ -43,10 +50,37 @@ impl ComputeBudgetInstructionDetails { } } + if compute_budget_instruction_details + .requested_compute_unit_limit + .is_none() + { + let mut filter = BuiltinProgramsFilter::new(); + // reiterate to collect builtin details + for (program_id, instruction) in instructions { + match filter.get_program_kind(instruction.program_id_index as usize, program_id) { + ProgramKind::Builtin => { + saturating_add_assign!( + compute_budget_instruction_details.num_builtin_instructions, + 1 + ); + } + ProgramKind::NotBuiltin => { + saturating_add_assign!( + compute_budget_instruction_details.num_non_builtin_instructions, + 1 + ); + } + } + } + } + Ok(compute_budget_instruction_details) } - pub fn sanitize_and_convert_to_compute_budget_limits(&self) -> Result { + pub fn sanitize_and_convert_to_compute_budget_limits( + &self, + feature_set: &FeatureSet, + ) -> Result { // Sanitize requested heap size let updated_heap_bytes = if let Some((index, requested_heap_size)) = self.requested_heap_size { @@ -67,10 +101,7 @@ impl ComputeBudgetInstructionDetails { let compute_unit_limit = self .requested_compute_unit_limit .map_or_else( - || { - self.num_non_compute_budget_instructions - .saturating_mul(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT) - }, + || self.calculate_default_compute_unit_limit(feature_set), |(_index, requested_compute_unit_limit)| requested_compute_unit_limit, ) .min(MAX_COMPUTE_UNIT_LIMIT); @@ -136,9 +167,24 @@ impl ComputeBudgetInstructionDetails { Ok(()) } + #[inline] fn sanitize_requested_heap_size(bytes: u32) -> bool { (MIN_HEAP_FRAME_BYTES..=MAX_HEAP_FRAME_BYTES).contains(&bytes) && bytes % 1024 == 0 } + + 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()) { + u32::from(self.num_builtin_instructions) + .saturating_mul(MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT) + .saturating_add( + u32::from(self.num_non_builtin_instructions) + .saturating_mul(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT), + ) + } else { + u32::from(self.num_non_compute_budget_instructions) + .saturating_mul(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT) + } + } } #[cfg(test)] @@ -171,14 +217,16 @@ mod test { ComputeBudgetInstruction::request_heap_frame(40 * 1024), Instruction::new_with_bincode(Pubkey::new_unique(), &(), vec![]), ]); - let expected_details = ComputeBudgetInstructionDetails { + let expected_details = Ok(ComputeBudgetInstructionDetails { requested_heap_size: Some((1, 40 * 1024)), num_non_compute_budget_instructions: 2, + num_builtin_instructions: 1, + num_non_builtin_instructions: 2, ..ComputeBudgetInstructionDetails::default() - }; + }); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), - Ok(expected_details) + ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),), + expected_details ); let tx = build_sanitized_transaction(&[ @@ -187,7 +235,7 @@ mod test { ComputeBudgetInstruction::request_heap_frame(41 * 1024), ]); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), + ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),), Err(TransactionError::DuplicateInstruction(2)) ); } @@ -199,14 +247,14 @@ mod test { ComputeBudgetInstruction::set_compute_unit_limit(u32::MAX), Instruction::new_with_bincode(Pubkey::new_unique(), &(), vec![]), ]); - let expected_details = ComputeBudgetInstructionDetails { + let expected_details = Ok(ComputeBudgetInstructionDetails { requested_compute_unit_limit: Some((1, u32::MAX)), num_non_compute_budget_instructions: 2, ..ComputeBudgetInstructionDetails::default() - }; + }); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), - Ok(expected_details) + ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),), + expected_details ); let tx = build_sanitized_transaction(&[ @@ -215,7 +263,7 @@ mod test { ComputeBudgetInstruction::set_compute_unit_limit(u32::MAX), ]); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), + ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),), Err(TransactionError::DuplicateInstruction(2)) ); } @@ -227,14 +275,16 @@ mod test { ComputeBudgetInstruction::set_compute_unit_price(u64::MAX), Instruction::new_with_bincode(Pubkey::new_unique(), &(), vec![]), ]); - let expected_details = ComputeBudgetInstructionDetails { + let expected_details = Ok(ComputeBudgetInstructionDetails { requested_compute_unit_price: Some((1, u64::MAX)), num_non_compute_budget_instructions: 2, + num_builtin_instructions: 1, + num_non_builtin_instructions: 2, ..ComputeBudgetInstructionDetails::default() - }; + }); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), - Ok(expected_details) + ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),), + expected_details ); let tx = build_sanitized_transaction(&[ @@ -243,7 +293,7 @@ mod test { ComputeBudgetInstruction::set_compute_unit_price(u64::MAX), ]); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), + ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),), Err(TransactionError::DuplicateInstruction(2)) ); } @@ -255,14 +305,16 @@ mod test { ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(u32::MAX), Instruction::new_with_bincode(Pubkey::new_unique(), &(), vec![]), ]); - let expected_details = ComputeBudgetInstructionDetails { + let expected_details = Ok(ComputeBudgetInstructionDetails { requested_loaded_accounts_data_size_limit: Some((1, u32::MAX)), num_non_compute_budget_instructions: 2, + num_builtin_instructions: 1, + num_non_builtin_instructions: 2, ..ComputeBudgetInstructionDetails::default() - }; + }); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), - Ok(expected_details) + ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),), + expected_details ); let tx = build_sanitized_transaction(&[ @@ -271,39 +323,67 @@ mod test { ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(u32::MAX), ]); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), + ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx),), Err(TransactionError::DuplicateInstruction(2)) ); } + fn prep_feature_minimial_cus_for_builtin_instructions( + is_active: bool, + instruction_details: &ComputeBudgetInstructionDetails, + ) -> (FeatureSet, u32) { + let mut feature_set = FeatureSet::default(); + let ComputeBudgetInstructionDetails { + num_non_compute_budget_instructions, + num_builtin_instructions, + num_non_builtin_instructions, + .. + } = *instruction_details; + let expected_cu_limit = if is_active { + feature_set.activate( + &feature_set::reserve_minimal_cus_for_builtin_instructions::id(), + 0, + ); + u32::from(num_non_builtin_instructions) * DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT + + u32::from(num_builtin_instructions) * MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT + } else { + u32::from(num_non_compute_budget_instructions) * DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT + }; + + (feature_set, expected_cu_limit) + } + #[test] fn test_sanitize_and_convert_to_compute_budget_limits() { // empty details, default ComputeBudgetLimits with 0 compute_unit_limits let instruction_details = ComputeBudgetInstructionDetails::default(); assert_eq!( - instruction_details.sanitize_and_convert_to_compute_budget_limits(), + instruction_details + .sanitize_and_convert_to_compute_budget_limits(&FeatureSet::default()), Ok(ComputeBudgetLimits { compute_unit_limit: 0, ..ComputeBudgetLimits::default() }) ); - let num_non_compute_budget_instructions = 4; - // no compute-budget instructions, all default ComputeBudgetLimits except cu-limit let instruction_details = ComputeBudgetInstructionDetails { - num_non_compute_budget_instructions, + num_non_compute_budget_instructions: 4, + num_builtin_instructions: 1, + num_non_builtin_instructions: 3, ..ComputeBudgetInstructionDetails::default() }; - let expected_compute_unit_limit = - num_non_compute_budget_instructions * DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT; - assert_eq!( - instruction_details.sanitize_and_convert_to_compute_budget_limits(), - Ok(ComputeBudgetLimits { - compute_unit_limit: expected_compute_unit_limit, - ..ComputeBudgetLimits::default() - }) - ); + for is_active in [true, false] { + let (feature_set, expected_compute_unit_limit) = + prep_feature_minimial_cus_for_builtin_instructions(is_active, &instruction_details); + assert_eq!( + instruction_details.sanitize_and_convert_to_compute_budget_limits(&feature_set), + Ok(ComputeBudgetLimits { + compute_unit_limit: expected_compute_unit_limit, + ..ComputeBudgetLimits::default() + }) + ); + } let expected_heap_size_err = Err(TransactionError::InstructionError( 3, @@ -315,12 +395,16 @@ mod test { requested_compute_unit_price: Some((2, 0)), requested_heap_size: Some((3, 0)), requested_loaded_accounts_data_size_limit: Some((4, 1024)), - num_non_compute_budget_instructions, + ..ComputeBudgetInstructionDetails::default() }; - assert_eq!( - instruction_details.sanitize_and_convert_to_compute_budget_limits(), - expected_heap_size_err - ); + for is_active in [true, false] { + let (feature_set, _expected_compute_unit_limit) = + prep_feature_minimial_cus_for_builtin_instructions(is_active, &instruction_details); + assert_eq!( + instruction_details.sanitize_and_convert_to_compute_budget_limits(&feature_set), + expected_heap_size_err + ); + } // invalid: requested_heap_size can't be less than MIN_HEAP_FRAME_BYTES let instruction_details = ComputeBudgetInstructionDetails { @@ -328,12 +412,16 @@ mod test { requested_compute_unit_price: Some((2, 0)), requested_heap_size: Some((3, MIN_HEAP_FRAME_BYTES - 1)), requested_loaded_accounts_data_size_limit: Some((4, 1024)), - num_non_compute_budget_instructions, + ..ComputeBudgetInstructionDetails::default() }; - assert_eq!( - instruction_details.sanitize_and_convert_to_compute_budget_limits(), - expected_heap_size_err - ); + for is_active in [true, false] { + let (feature_set, _expected_compute_unit_limit) = + prep_feature_minimial_cus_for_builtin_instructions(is_active, &instruction_details); + assert_eq!( + instruction_details.sanitize_and_convert_to_compute_budget_limits(&feature_set), + expected_heap_size_err + ); + } // invalid: requested_heap_size can't be more than MAX_HEAP_FRAME_BYTES let instruction_details = ComputeBudgetInstructionDetails { @@ -341,12 +429,16 @@ mod test { requested_compute_unit_price: Some((2, 0)), requested_heap_size: Some((3, MAX_HEAP_FRAME_BYTES + 1)), requested_loaded_accounts_data_size_limit: Some((4, 1024)), - num_non_compute_budget_instructions, + ..ComputeBudgetInstructionDetails::default() }; - assert_eq!( - instruction_details.sanitize_and_convert_to_compute_budget_limits(), - expected_heap_size_err - ); + for is_active in [true, false] { + let (feature_set, _expected_compute_unit_limit) = + prep_feature_minimial_cus_for_builtin_instructions(is_active, &instruction_details); + assert_eq!( + instruction_details.sanitize_and_convert_to_compute_budget_limits(&feature_set), + expected_heap_size_err + ); + } // invalid: requested_heap_size must be round by 1024 let instruction_details = ComputeBudgetInstructionDetails { @@ -354,12 +446,16 @@ mod test { requested_compute_unit_price: Some((2, 0)), requested_heap_size: Some((3, MIN_HEAP_FRAME_BYTES + 1024 + 1)), requested_loaded_accounts_data_size_limit: Some((4, 1024)), - num_non_compute_budget_instructions, + ..ComputeBudgetInstructionDetails::default() }; - assert_eq!( - instruction_details.sanitize_and_convert_to_compute_budget_limits(), - expected_heap_size_err - ); + for is_active in [true, false] { + let (feature_set, _expected_compute_unit_limit) = + prep_feature_minimial_cus_for_builtin_instructions(is_active, &instruction_details); + assert_eq!( + instruction_details.sanitize_and_convert_to_compute_budget_limits(&feature_set), + expected_heap_size_err + ); + } // invalid: loaded_account_data_size can't be zero let instruction_details = ComputeBudgetInstructionDetails { @@ -367,12 +463,16 @@ mod test { requested_compute_unit_price: Some((2, 0)), requested_heap_size: Some((3, 40 * 1024)), requested_loaded_accounts_data_size_limit: Some((4, 0)), - num_non_compute_budget_instructions, + ..ComputeBudgetInstructionDetails::default() }; - assert_eq!( - instruction_details.sanitize_and_convert_to_compute_budget_limits(), - Err(TransactionError::InvalidLoadedAccountsDataSizeLimit) - ); + for is_active in [true, false] { + let (feature_set, _expected_compute_unit_limit) = + prep_feature_minimial_cus_for_builtin_instructions(is_active, &instruction_details); + assert_eq!( + instruction_details.sanitize_and_convert_to_compute_budget_limits(&feature_set), + Err(TransactionError::InvalidLoadedAccountsDataSizeLimit) + ); + } // valid: acceptable MAX let instruction_details = ComputeBudgetInstructionDetails { @@ -380,17 +480,22 @@ mod test { requested_compute_unit_price: Some((2, u64::MAX)), requested_heap_size: Some((3, MAX_HEAP_FRAME_BYTES)), requested_loaded_accounts_data_size_limit: Some((4, u32::MAX)), - num_non_compute_budget_instructions, + num_non_compute_budget_instructions: 4, + ..ComputeBudgetInstructionDetails::default() }; - assert_eq!( - instruction_details.sanitize_and_convert_to_compute_budget_limits(), - Ok(ComputeBudgetLimits { - updated_heap_bytes: MAX_HEAP_FRAME_BYTES, - compute_unit_limit: MAX_COMPUTE_UNIT_LIMIT, - compute_unit_price: u64::MAX, - loaded_accounts_bytes: MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, - }) - ); + for is_active in [true, false] { + let (feature_set, _expected_compute_unit_limit) = + prep_feature_minimial_cus_for_builtin_instructions(is_active, &instruction_details); + assert_eq!( + instruction_details.sanitize_and_convert_to_compute_budget_limits(&feature_set), + Ok(ComputeBudgetLimits { + updated_heap_bytes: MAX_HEAP_FRAME_BYTES, + compute_unit_limit: MAX_COMPUTE_UNIT_LIMIT, + compute_unit_price: u64::MAX, + loaded_accounts_bytes: MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, + }) + ); + } // valid let val: u32 = 1024 * 40; @@ -399,16 +504,20 @@ mod test { requested_compute_unit_price: Some((2, val as u64)), requested_heap_size: Some((3, val)), requested_loaded_accounts_data_size_limit: Some((4, val)), - num_non_compute_budget_instructions, + ..ComputeBudgetInstructionDetails::default() }; - assert_eq!( - instruction_details.sanitize_and_convert_to_compute_budget_limits(), - Ok(ComputeBudgetLimits { - updated_heap_bytes: val, - compute_unit_limit: val, - compute_unit_price: val as u64, - loaded_accounts_bytes: NonZeroU32::new(val).unwrap(), - }) - ); + for is_active in [true, false] { + let (feature_set, _expected_compute_unit_limit) = + prep_feature_minimial_cus_for_builtin_instructions(is_active, &instruction_details); + assert_eq!( + instruction_details.sanitize_and_convert_to_compute_budget_limits(&feature_set), + Ok(ComputeBudgetLimits { + updated_heap_bytes: val, + compute_unit_limit: val, + compute_unit_price: val as u64, + loaded_accounts_bytes: NonZeroU32::new(val).unwrap(), + }) + ); + } } } diff --git a/runtime-transaction/src/compute_budget_program_id_filter.rs b/runtime-transaction/src/compute_budget_program_id_filter.rs index 87c12784222f90..59c6144a091229 100644 --- a/runtime-transaction/src/compute_budget_program_id_filter.rs +++ b/runtime-transaction/src/compute_budget_program_id_filter.rs @@ -18,7 +18,6 @@ impl ComputeBudgetProgramIdFilter { } } - #[inline] pub(crate) fn is_compute_budget_program(&mut self, index: usize, program_id: &Pubkey) -> bool { *self .flags diff --git a/runtime-transaction/src/instructions_processor.rs b/runtime-transaction/src/instructions_processor.rs index 1edba220096276..6259044feb9023 100644 --- a/runtime-transaction/src/instructions_processor.rs +++ b/runtime-transaction/src/instructions_processor.rs @@ -1,7 +1,7 @@ use { crate::compute_budget_instruction_details::*, solana_compute_budget::compute_budget_limits::*, - solana_sdk::{pubkey::Pubkey, transaction::TransactionError}, + solana_sdk::{feature_set::FeatureSet, pubkey::Pubkey, transaction::TransactionError}, solana_svm_transaction::instruction::SVMInstruction, }; @@ -11,10 +11,11 @@ use { /// If succeeded, the transaction's specific limits/requests (could be default) /// are retrieved and returned, pub fn process_compute_budget_instructions<'a>( - instructions: impl Iterator)>, + instructions: impl Iterator)> + Clone, + feature_set: &FeatureSet, ) -> Result { ComputeBudgetInstructionDetails::try_from(instructions)? - .sanitize_and_convert_to_compute_budget_limits() + .sanitize_and_convert_to_compute_budget_limits(feature_set) } #[cfg(test)] @@ -38,14 +39,22 @@ mod tests { macro_rules! test { ( $instructions: expr, $expected_result: expr) => { + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + test!($instructions, $expected_result, &feature_set); + } + }; + ( $instructions: expr, $expected_result: expr, $feature_set: expr) => { let payer_keypair = Keypair::new(); let tx = SanitizedTransaction::from_transaction_for_tests(Transaction::new( &[&payer_keypair], Message::new($instructions, Some(&payer_keypair.pubkey())), Hash::default(), )); - let result = - process_compute_budget_instructions(SVMMessage::program_instructions_iter(&tx)); + + let result = process_compute_budget_instructions( + SVMMessage::program_instructions_iter(&tx), + $feature_set, + ); assert_eq!($expected_result, result); }; } @@ -131,7 +140,21 @@ mod tests { compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, updated_heap_bytes: 40 * 1024, ..ComputeBudgetLimits::default() - }) + }), + &FeatureSet::default() + ); + test!( + &[ + ComputeBudgetInstruction::request_heap_frame(40 * 1024), + Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), + ], + Ok(ComputeBudgetLimits { + compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT + + MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT, + updated_heap_bytes: 40 * 1024, + ..ComputeBudgetLimits::default() + }), + &FeatureSet::all_enabled() ); test!( &[ @@ -172,7 +195,21 @@ mod tests { compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, updated_heap_bytes: MAX_HEAP_FRAME_BYTES, ..ComputeBudgetLimits::default() - }) + }), + &FeatureSet::default() + ); + test!( + &[ + Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), + ComputeBudgetInstruction::request_heap_frame(MAX_HEAP_FRAME_BYTES), + ], + Ok(ComputeBudgetLimits { + compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT + + MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT, + updated_heap_bytes: MAX_HEAP_FRAME_BYTES, + ..ComputeBudgetLimits::default() + }), + &FeatureSet::all_enabled() ); test!( &[ @@ -279,13 +316,28 @@ mod tests { loaded_accounts_bytes: NonZeroU32::new(data_size).unwrap(), ..ComputeBudgetLimits::default() }); + test!( + &[ + ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(data_size), + Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), + ], + expected_result, + &FeatureSet::default() + ); + let expected_result = Ok(ComputeBudgetLimits { + compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT + + MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT, + loaded_accounts_bytes: NonZeroU32::new(data_size).unwrap(), + ..ComputeBudgetLimits::default() + }); test!( &[ ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(data_size), Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), ], - expected_result + expected_result, + &FeatureSet::all_enabled() ); // Assert when set_loaded_accounts_data_size_limit presents, with greater than max value @@ -296,13 +348,28 @@ mod tests { loaded_accounts_bytes: MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, ..ComputeBudgetLimits::default() }); + test!( + &[ + ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(data_size), + Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), + ], + expected_result, + &FeatureSet::default() + ); + let expected_result = Ok(ComputeBudgetLimits { + compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT + + MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT, + loaded_accounts_bytes: MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, + ..ComputeBudgetLimits::default() + }); test!( &[ ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(data_size), Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), ], - expected_result + expected_result, + &FeatureSet::all_enabled() ); // Assert when set_loaded_accounts_data_size_limit is not presented @@ -352,19 +419,32 @@ mod tests { Hash::default(), )); - let result = process_compute_budget_instructions(SVMMessage::program_instructions_iter( - &transaction, - )); + for (feature_set, expected_result) in [ + ( + FeatureSet::default(), + Ok(ComputeBudgetLimits { + compute_unit_limit: 2 * DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, + ..ComputeBudgetLimits::default() + }), + ), + ( + FeatureSet::all_enabled(), + Ok(ComputeBudgetLimits { + compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT + + MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT, + ..ComputeBudgetLimits::default() + }), + ), + ] { + let result = process_compute_budget_instructions( + SVMMessage::program_instructions_iter(&transaction), + &feature_set, + ); - // assert process_instructions will be successful with default, - // and the default compute_unit_limit is 2 times default: one for bpf ix, one for - // builtin ix. - assert_eq!( - result, - Ok(ComputeBudgetLimits { - compute_unit_limit: 2 * DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, - ..ComputeBudgetLimits::default() - }) - ); + // assert process_instructions will be successful with default, + // and the default compute_unit_limit is 2 times default: one for bpf ix, one for + // builtin ix. + assert_eq!(result, expected_result); + } } } diff --git a/runtime-transaction/src/lib.rs b/runtime-transaction/src/lib.rs index 40c31d4b4d653a..4f7b214f8f235d 100644 --- a/runtime-transaction/src/lib.rs +++ b/runtime-transaction/src/lib.rs @@ -1,7 +1,12 @@ #![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_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 1dbd44bed70bf6..183aa222c2aaaf 100644 --- a/runtime-transaction/src/runtime_transaction.rs +++ b/runtime-transaction/src/runtime_transaction.rs @@ -167,7 +167,7 @@ impl SVMMessage for RuntimeTransaction { self.transaction.instructions_iter() } - fn program_instructions_iter(&self) -> impl Iterator { + fn program_instructions_iter(&self) -> impl Iterator + Clone { self.transaction.program_instructions_iter() } diff --git a/runtime-transaction/src/runtime_transaction/sdk_transactions.rs b/runtime-transaction/src/runtime_transaction/sdk_transactions.rs new file mode 100644 index 00000000000000..a82e9c1217fff6 --- /dev/null +++ b/runtime-transaction/src/runtime_transaction/sdk_transactions.rs @@ -0,0 +1,346 @@ +use { + super::{ComputeBudgetInstructionDetails, RuntimeTransaction}, + crate::{ + signature_details::get_precompile_signature_details, + transaction_meta::{StaticMeta, TransactionMeta}, + transaction_with_meta::TransactionWithMeta, + }, + solana_pubkey::Pubkey, + solana_sdk::{ + message::{AddressLoader, TransactionSignatureDetails}, + simple_vote_transaction_checker::is_simple_vote_transaction, + transaction::{ + MessageHash, Result, SanitizedTransaction, SanitizedVersionedTransaction, + VersionedTransaction, + }, + }, + solana_svm_transaction::instruction::SVMInstruction, + std::{borrow::Cow, collections::HashSet}, +}; + +impl RuntimeTransaction { + pub fn try_from( + sanitized_versioned_tx: SanitizedVersionedTransaction, + message_hash: MessageHash, + is_simple_vote_tx: Option, + ) -> Result { + let message_hash = match message_hash { + MessageHash::Precomputed(hash) => hash, + MessageHash::Compute => sanitized_versioned_tx.get_message().message.hash(), + }; + let is_simple_vote_tx = is_simple_vote_tx + .unwrap_or_else(|| is_simple_vote_transaction(&sanitized_versioned_tx)); + + let precompile_signature_details = get_precompile_signature_details( + sanitized_versioned_tx + .get_message() + .program_instructions_iter() + .map(|(program_id, ix)| (program_id, SVMInstruction::from(ix))), + ); + let signature_details = TransactionSignatureDetails::new( + u64::from( + sanitized_versioned_tx + .get_message() + .message + .header() + .num_required_signatures, + ), + precompile_signature_details.num_secp256k1_instruction_signatures, + precompile_signature_details.num_ed25519_instruction_signatures, + precompile_signature_details.num_secp256r1_instruction_signatures, + ); + let compute_budget_instruction_details = ComputeBudgetInstructionDetails::try_from( + sanitized_versioned_tx + .get_message() + .program_instructions_iter() + .map(|(program_id, ix)| (program_id, SVMInstruction::from(ix))), + )?; + + Ok(Self { + transaction: sanitized_versioned_tx, + meta: TransactionMeta { + message_hash, + is_simple_vote_transaction: is_simple_vote_tx, + signature_details, + compute_budget_instruction_details, + }, + }) + } +} + +impl RuntimeTransaction { + /// Create a new `RuntimeTransaction` from an + /// unsanitized `VersionedTransaction`. + pub fn try_create( + tx: VersionedTransaction, + message_hash: MessageHash, + is_simple_vote_tx: Option, + address_loader: impl AddressLoader, + reserved_account_keys: &HashSet, + ) -> Result { + let statically_loaded_runtime_tx = + RuntimeTransaction::::try_from( + SanitizedVersionedTransaction::try_from(tx)?, + message_hash, + is_simple_vote_tx, + )?; + Self::try_from( + statically_loaded_runtime_tx, + address_loader, + reserved_account_keys, + ) + } + + /// Create a new `RuntimeTransaction` from a + /// `RuntimeTransaction` that already has + /// static metadata loaded. + pub fn try_from( + statically_loaded_runtime_tx: RuntimeTransaction, + address_loader: impl AddressLoader, + reserved_account_keys: &HashSet, + ) -> Result { + let hash = *statically_loaded_runtime_tx.message_hash(); + let is_simple_vote_tx = statically_loaded_runtime_tx.is_simple_vote_transaction(); + let sanitized_transaction = SanitizedTransaction::try_new( + statically_loaded_runtime_tx.transaction, + hash, + is_simple_vote_tx, + address_loader, + reserved_account_keys, + )?; + + let mut tx = Self { + transaction: sanitized_transaction, + meta: statically_loaded_runtime_tx.meta, + }; + tx.load_dynamic_metadata()?; + + Ok(tx) + } + + fn load_dynamic_metadata(&mut self) -> Result<()> { + Ok(()) + } +} + +impl TransactionWithMeta for RuntimeTransaction { + #[inline] + fn as_sanitized_transaction(&self) -> Cow { + Cow::Borrowed(self) + } + + #[inline] + fn to_versioned_transaction(&self) -> VersionedTransaction { + self.transaction.to_versioned_transaction() + } +} + +#[cfg(feature = "dev-context-only-utils")] +impl RuntimeTransaction { + pub fn from_transaction_for_tests(transaction: solana_sdk::transaction::Transaction) -> Self { + let versioned_transaction = VersionedTransaction::from(transaction); + Self::try_create( + versioned_transaction, + MessageHash::Compute, + None, + solana_sdk::message::SimpleAddressLoader::Disabled, + &HashSet::new(), + ) + .expect("failed to create RuntimeTransaction from Transaction") + } +} + +#[cfg(test)] +mod tests { + use { + super::*, + solana_program::{ + system_instruction, + vote::{self, state::Vote}, + }, + solana_sdk::{ + compute_budget::ComputeBudgetInstruction, + feature_set::FeatureSet, + hash::Hash, + instruction::Instruction, + message::Message, + reserved_account_keys::ReservedAccountKeys, + signer::{keypair::Keypair, Signer}, + transaction::{SimpleAddressLoader, Transaction, VersionedTransaction}, + }, + }; + + fn vote_sanitized_versioned_transaction() -> SanitizedVersionedTransaction { + let bank_hash = Hash::new_unique(); + let block_hash = Hash::new_unique(); + let vote_keypair = Keypair::new(); + let node_keypair = Keypair::new(); + let auth_keypair = Keypair::new(); + let votes = Vote::new(vec![1, 2, 3], bank_hash); + let vote_ix = + vote::instruction::vote(&vote_keypair.pubkey(), &auth_keypair.pubkey(), votes); + let mut vote_tx = Transaction::new_with_payer(&[vote_ix], Some(&node_keypair.pubkey())); + vote_tx.partial_sign(&[&node_keypair], block_hash); + vote_tx.partial_sign(&[&auth_keypair], block_hash); + + SanitizedVersionedTransaction::try_from(VersionedTransaction::from(vote_tx)).unwrap() + } + + fn non_vote_sanitized_versioned_transaction() -> SanitizedVersionedTransaction { + TestTransaction::new().to_sanitized_versioned_transaction() + } + + // Simple transfer transaction for testing, it does not support vote instruction + // because simple vote transaction will not request limits + struct TestTransaction { + from_keypair: Keypair, + hash: Hash, + instructions: Vec, + } + + impl TestTransaction { + fn new() -> Self { + let from_keypair = Keypair::new(); + let instructions = vec![system_instruction::transfer( + &from_keypair.pubkey(), + &solana_sdk::pubkey::new_rand(), + 1, + )]; + TestTransaction { + from_keypair, + hash: Hash::new_unique(), + instructions, + } + } + + fn add_compute_unit_limit(&mut self, val: u32) -> &mut TestTransaction { + self.instructions + .push(ComputeBudgetInstruction::set_compute_unit_limit(val)); + self + } + + fn add_compute_unit_price(&mut self, val: u64) -> &mut TestTransaction { + self.instructions + .push(ComputeBudgetInstruction::set_compute_unit_price(val)); + self + } + + fn add_loaded_accounts_bytes(&mut self, val: u32) -> &mut TestTransaction { + self.instructions + .push(ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(val)); + self + } + + fn to_sanitized_versioned_transaction(&self) -> SanitizedVersionedTransaction { + let message = Message::new(&self.instructions, Some(&self.from_keypair.pubkey())); + let tx = Transaction::new(&[&self.from_keypair], message, self.hash); + SanitizedVersionedTransaction::try_from(VersionedTransaction::from(tx)).unwrap() + } + } + + #[test] + fn test_runtime_transaction_is_vote_meta() { + fn get_is_simple_vote( + svt: SanitizedVersionedTransaction, + is_simple_vote: Option, + ) -> bool { + RuntimeTransaction::::try_from( + svt, + MessageHash::Compute, + is_simple_vote, + ) + .unwrap() + .meta + .is_simple_vote_transaction + } + + assert!(!get_is_simple_vote( + non_vote_sanitized_versioned_transaction(), + None + )); + + assert!(get_is_simple_vote( + non_vote_sanitized_versioned_transaction(), + Some(true), // override + )); + + assert!(get_is_simple_vote( + vote_sanitized_versioned_transaction(), + None + )); + + assert!(!get_is_simple_vote( + vote_sanitized_versioned_transaction(), + Some(false), // override + )); + } + + #[test] + fn test_advancing_transaction_type() { + let hash = Hash::new_unique(); + + let statically_loaded_transaction = + RuntimeTransaction::::try_from( + non_vote_sanitized_versioned_transaction(), + MessageHash::Precomputed(hash), + None, + ) + .unwrap(); + + assert_eq!(hash, *statically_loaded_transaction.message_hash()); + assert!(!statically_loaded_transaction.is_simple_vote_transaction()); + + let dynamically_loaded_transaction = RuntimeTransaction::::try_from( + statically_loaded_transaction, + SimpleAddressLoader::Disabled, + &ReservedAccountKeys::empty_key_set(), + ); + let dynamically_loaded_transaction = + dynamically_loaded_transaction.expect("created from statically loaded tx"); + + assert_eq!(hash, *dynamically_loaded_transaction.message_hash()); + assert!(!dynamically_loaded_transaction.is_simple_vote_transaction()); + } + + #[test] + fn test_runtime_transaction_static_meta() { + let hash = Hash::new_unique(); + let compute_unit_limit = 250_000; + let compute_unit_price = 1_000; + let loaded_accounts_bytes = 1_024; + let mut test_transaction = TestTransaction::new(); + + let runtime_transaction_static = + RuntimeTransaction::::try_from( + test_transaction + .add_compute_unit_limit(compute_unit_limit) + .add_compute_unit_price(compute_unit_price) + .add_loaded_accounts_bytes(loaded_accounts_bytes) + .to_sanitized_versioned_transaction(), + MessageHash::Precomputed(hash), + None, + ) + .unwrap(); + + assert_eq!(&hash, runtime_transaction_static.message_hash()); + assert!(!runtime_transaction_static.is_simple_vote_transaction()); + + let signature_details = &runtime_transaction_static.meta.signature_details; + assert_eq!(1, signature_details.num_transaction_signatures()); + assert_eq!(0, signature_details.num_secp256k1_instruction_signatures()); + assert_eq!(0, signature_details.num_ed25519_instruction_signatures()); + + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + let compute_budget_limits = runtime_transaction_static + .compute_budget_instruction_details() + .sanitize_and_convert_to_compute_budget_limits(&feature_set) + .unwrap(); + assert_eq!(compute_unit_limit, compute_budget_limits.compute_unit_limit); + assert_eq!(compute_unit_price, compute_budget_limits.compute_unit_price); + assert_eq!( + loaded_accounts_bytes, + compute_budget_limits.loaded_accounts_bytes.get() + ); + } + } +} diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 237208caf191d9..8036b09904c51f 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3209,8 +3209,11 @@ impl Bank { lamports_per_signature: u64, ) -> u64 { let fee_budget_limits = FeeBudgetLimits::from( - process_compute_budget_instructions(message.program_instructions_iter()) - .unwrap_or_default(), + process_compute_budget_instructions( + message.program_instructions_iter(), + &self.feature_set, + ) + .unwrap_or_default(), ); solana_fee::calculate_fee( message, diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 128ec740d2014e..a666884cc95b48 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -10194,8 +10194,11 @@ fn calculate_test_fee( fee_structure: &FeeStructure, ) -> u64 { let fee_budget_limits = FeeBudgetLimits::from( - process_compute_budget_instructions(message.program_instructions_iter()) - .unwrap_or_default(), + process_compute_budget_instructions( + message.program_instructions_iter(), + &FeatureSet::default(), + ) + .unwrap_or_default(), ); solana_fee::calculate_fee( message, diff --git a/runtime/src/prioritization_fee_cache.rs b/runtime/src/prioritization_fee_cache.rs index efaac3742af72f..de35bd77e2f41c 100644 --- a/runtime/src/prioritization_fee_cache.rs +++ b/runtime/src/prioritization_fee_cache.rs @@ -205,9 +205,15 @@ impl PrioritizationFeeCache { continue; } +<<<<<<< HEAD let compute_budget_limits = process_compute_budget_instructions( SVMMessage::program_instructions_iter(sanitized_transaction), ); +======= + 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/sdk/feature-set/src/lib.rs b/sdk/feature-set/src/lib.rs index fd7c2ad31fe18d..065e9458e33bea 100644 --- a/sdk/feature-set/src/lib.rs +++ b/sdk/feature-set/src/lib.rs @@ -881,6 +881,10 @@ pub mod migrate_stake_program_to_core_bpf { solana_pubkey::declare_id!("6M4oQ6eXneVhtLoiAr4yRYQY43eVLjrKbiDZDJc892yk"); } +pub mod reserve_minimal_cus_for_builtin_instructions { + solana_pubkey::declare_id!("C9oAhLxDBm3ssWtJx1yBGzPY55r2rArHmN1pbQn6HogH"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -1096,6 +1100,7 @@ lazy_static! { (disable_account_loader_special_case::id(), "Disable account loader special case #3513"), (enable_secp256r1_precompile::id(), "Enable secp256r1 precompile SIMD-0075"), (migrate_stake_program_to_core_bpf::id(), "Migrate Stake program to Core BPF SIMD-0196 #3655"), + (reserve_minimal_cus_for_builtin_instructions::id(), "Reserve minimal CUs for builtin instructions SIMD-170 #2562"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/sdk/program/src/message/sanitized.rs b/sdk/program/src/message/sanitized.rs index 792dd1c39104ed..c5c44160c9c23c 100644 --- a/sdk/program/src/message/sanitized.rs +++ b/sdk/program/src/message/sanitized.rs @@ -174,7 +174,7 @@ impl SanitizedMessage { /// id. pub fn program_instructions_iter( &self, - ) -> impl Iterator { + ) -> impl Iterator + Clone { self.instructions().iter().map(move |ix| { ( self.account_keys() diff --git a/sdk/program/src/message/versions/sanitized.rs b/sdk/program/src/message/versions/sanitized.rs index b2d43da131dcd8..bc997fe14282f3 100644 --- a/sdk/program/src/message/versions/sanitized.rs +++ b/sdk/program/src/message/versions/sanitized.rs @@ -33,7 +33,7 @@ impl SanitizedVersionedMessage { /// id. pub fn program_instructions_iter( &self, - ) -> impl Iterator { + ) -> impl Iterator + Clone { self.message.instructions().iter().map(move |ix| { ( self.message diff --git a/svm-transaction/src/svm_message.rs b/svm-transaction/src/svm_message.rs index e9617ecdf4ed23..3d479a5278f5d5 100644 --- a/svm-transaction/src/svm_message.rs +++ b/svm-transaction/src/svm_message.rs @@ -34,7 +34,7 @@ pub trait SVMMessage: Debug { /// Return an iterator over the instructions in the message, paired with /// the pubkey of the program. - fn program_instructions_iter(&self) -> impl Iterator; + fn program_instructions_iter(&self) -> impl Iterator + Clone; /// Return the account keys. fn account_keys(&self) -> AccountKeys; diff --git a/svm-transaction/src/svm_message/sanitized_message.rs b/svm-transaction/src/svm_message/sanitized_message.rs index 66546a6d45f95b..09a06242d66070 100644 --- a/svm-transaction/src/svm_message/sanitized_message.rs +++ b/svm-transaction/src/svm_message/sanitized_message.rs @@ -34,7 +34,7 @@ impl SVMMessage for SanitizedMessage { .map(SVMInstruction::from) } - fn program_instructions_iter(&self) -> impl Iterator { + fn program_instructions_iter(&self) -> impl Iterator + Clone { SanitizedMessage::program_instructions_iter(self) .map(|(pubkey, ix)| (pubkey, SVMInstruction::from(ix))) } diff --git a/svm-transaction/src/svm_message/sanitized_transaction.rs b/svm-transaction/src/svm_message/sanitized_transaction.rs index 6321f27ca88e4f..5a7bdc1df962cd 100644 --- a/svm-transaction/src/svm_message/sanitized_transaction.rs +++ b/svm-transaction/src/svm_message/sanitized_transaction.rs @@ -29,7 +29,7 @@ impl SVMMessage for SanitizedTransaction { SVMMessage::instructions_iter(SanitizedTransaction::message(self)) } - fn program_instructions_iter(&self) -> impl Iterator { + fn program_instructions_iter(&self) -> impl Iterator + Clone { SVMMessage::program_instructions_iter(SanitizedTransaction::message(self)) } diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 61e0907a72d26d..554fedfd83f845 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -453,6 +453,7 @@ impl TransactionBatchProcessor { ) -> 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; @@ -1945,9 +1946,11 @@ mod tests { Some(&Pubkey::new_unique()), &Hash::new_unique(), )); - let compute_budget_limits = - process_compute_budget_instructions(SVMMessage::program_instructions_iter(&message)) - .unwrap(); + let compute_budget_limits = process_compute_budget_instructions( + SVMMessage::program_instructions_iter(&message), + &FeatureSet::default(), + ) + .unwrap(); let fee_payer_address = message.fee_payer(); let current_epoch = 42; let rent_collector = RentCollector { @@ -2031,9 +2034,11 @@ mod tests { Some(&Pubkey::new_unique()), &Hash::new_unique(), )); - let compute_budget_limits = - process_compute_budget_instructions(SVMMessage::program_instructions_iter(&message)) - .unwrap(); + let compute_budget_limits = process_compute_budget_instructions( + SVMMessage::program_instructions_iter(&message), + &FeatureSet::default(), + ) + .unwrap(); let fee_payer_address = message.fee_payer(); let mut rent_collector = RentCollector::default(); rent_collector.rent.lamports_per_byte_year = 1_000_000; @@ -2283,9 +2288,11 @@ mod tests { Some(&Pubkey::new_unique()), &last_blockhash, )); - let compute_budget_limits = - process_compute_budget_instructions(SVMMessage::program_instructions_iter(&message)) - .unwrap(); + let compute_budget_limits = process_compute_budget_instructions( + SVMMessage::program_instructions_iter(&message), + &FeatureSet::default(), + ) + .unwrap(); let fee_payer_address = message.fee_payer(); let min_balance = Rent::default().minimum_balance(nonce::State::size()); let transaction_fee = lamports_per_signature; diff --git a/transaction-view/src/resolved_transaction_view.rs b/transaction-view/src/resolved_transaction_view.rs index 81a6c2f1886314..587c158ddc8e5b 100644 --- a/transaction-view/src/resolved_transaction_view.rs +++ b/transaction-view/src/resolved_transaction_view.rs @@ -204,7 +204,7 @@ impl SVMMessage for ResolvedTransactionView { &solana_sdk::pubkey::Pubkey, solana_svm_transaction::instruction::SVMInstruction, ), - > { + > + Clone { self.view.program_instructions_iter() } diff --git a/transaction-view/src/transaction_view.rs b/transaction-view/src/transaction_view.rs index 1d0c3c9bdc2034..0184ab628b41a2 100644 --- a/transaction-view/src/transaction_view.rs +++ b/transaction-view/src/transaction_view.rs @@ -169,7 +169,9 @@ impl TransactionView { // Implementation that relies on sanitization checks having been run. impl TransactionView { /// Return an iterator over the instructions paired with their program ids. - pub fn program_instructions_iter(&self) -> impl Iterator { + pub fn program_instructions_iter( + &self, + ) -> impl Iterator + Clone { self.instructions_iter().map(|ix| { let program_id_index = usize::from(ix.program_id_index); let program_id = &self.static_account_keys()[program_id_index];