diff --git a/core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs b/core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs index f1be7339f..e0b53a97a 100644 --- a/core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs +++ b/core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs @@ -191,7 +191,7 @@ impl PrioGraphScheduler { saturating_add_assign!(num_scheduled, 1); let sanitized_transaction_ttl = transaction_state.transition_to_pending(); - let cost = transaction_state.transaction_cost().sum(); + let cost = transaction_state.cost(); let SanitizedTransactionTTL { transaction, @@ -490,12 +490,9 @@ mod tests { crate::banking_stage::consumer::TARGET_NUM_TRANSACTIONS_PER_BATCH, crossbeam_channel::{unbounded, Receiver}, itertools::Itertools, - solana_cost_model::cost_model::CostModel, - solana_runtime::compute_budget_details::ComputeBudgetDetails, solana_sdk::{ - compute_budget::ComputeBudgetInstruction, feature_set::FeatureSet, hash::Hash, - message::Message, pubkey::Pubkey, signature::Keypair, signer::Signer, - system_instruction, transaction::Transaction, + compute_budget::ComputeBudgetInstruction, hash::Hash, message::Message, pubkey::Pubkey, + signature::Keypair, signer::Signer, system_instruction, transaction::Transaction, }, std::borrow::Borrow, }; @@ -572,19 +569,16 @@ mod tests { lamports, compute_unit_price, ); - let transaction_cost = CostModel::calculate_cost(&transaction, &FeatureSet::default()); let transaction_ttl = SanitizedTransactionTTL { transaction, max_age_slot: Slot::MAX, }; + const TEST_TRANSACTION_COST: u64 = 5000; container.insert_new_transaction( id, transaction_ttl, - ComputeBudgetDetails { - compute_unit_price, - compute_unit_limit: 1, - }, - transaction_cost, + compute_unit_price, + TEST_TRANSACTION_COST, ); } diff --git a/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs b/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs index aaf275359..a5c0fa134 100644 --- a/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs +++ b/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs @@ -20,10 +20,12 @@ use { itertools::MinMaxResult, solana_cost_model::cost_model::CostModel, solana_measure::measure_us, + solana_program_runtime::compute_budget_processor::process_compute_budget_instructions, solana_runtime::{bank::Bank, bank_forks::BankForks}, solana_sdk::{ - clock::MAX_PROCESSING_AGE, saturating_add_assign, timing::AtomicInterval, - transaction::SanitizedTransaction, + clock::MAX_PROCESSING_AGE, + feature_set::include_loaded_accounts_data_size_in_fee_calculation, fee::FeeBudgetLimits, + saturating_add_assign, timing::AtomicInterval, transaction::SanitizedTransaction, }, solana_svm::transaction_error_metrics::TransactionErrorMetrics, std::{ @@ -100,7 +102,7 @@ impl SchedulerController { // Reset intervals when appropriate, regardless of report. let should_report = self.count_metrics.has_data(); self.count_metrics - .update_prioritization_stats(self.container.get_min_max_prioritization_fees()); + .update_priority_stats(self.container.get_min_max_priority()); self.count_metrics.maybe_report_and_reset(should_report); self.timing_metrics.maybe_report_and_reset(should_report); self.worker_metrics @@ -311,21 +313,24 @@ impl SchedulerController { let mut error_counts = TransactionErrorMetrics::default(); for chunk in packets.chunks(CHUNK_SIZE) { let mut post_sanitization_count: usize = 0; - let (transactions, compute_budget_details): (Vec<_>, Vec<_>) = chunk + let (transactions, fee_budget_limits_vec): (Vec<_>, Vec<_>) = chunk .iter() .filter_map(|packet| { - packet - .build_sanitized_transaction(feature_set, vote_only, bank.as_ref()) - .map(|tx| (tx, packet.compute_budget_details())) + packet.build_sanitized_transaction(feature_set, vote_only, bank.as_ref()) }) .inspect(|_| saturating_add_assign!(post_sanitization_count, 1)) - .filter(|(tx, _)| { + .filter(|tx| { SanitizedTransaction::validate_account_locks( tx.message(), transaction_account_lock_limit, ) .is_ok() }) + .filter_map(|tx| { + process_compute_budget_instructions(tx.message().program_instructions_iter()) + .map(|compute_budget| (tx, compute_budget.into())) + .ok() + }) .unzip(); let check_results = bank.check_transactions( @@ -337,16 +342,17 @@ impl SchedulerController { let post_lock_validation_count = transactions.len(); let mut post_transaction_check_count: usize = 0; - for ((transaction, compute_budget_details), _) in transactions + for ((transaction, fee_budget_limits), _) in transactions .into_iter() - .zip(compute_budget_details) + .zip(fee_budget_limits_vec) .zip(check_results) .filter(|(_, check_result)| check_result.0.is_ok()) { saturating_add_assign!(post_transaction_check_count, 1); let transaction_id = self.transaction_id_generator.next(); - let transaction_cost = CostModel::calculate_cost(&transaction, &bank.feature_set); + let (priority, cost) = + Self::calculate_priority_and_cost(&transaction, &fee_budget_limits, &bank); let transaction_ttl = SanitizedTransactionTTL { transaction, max_age_slot: last_slot_in_epoch, @@ -355,8 +361,8 @@ impl SchedulerController { if self.container.insert_new_transaction( transaction_id, transaction_ttl, - compute_budget_details, - transaction_cost, + priority, + cost, ) { saturating_add_assign!(self.count_metrics.num_dropped_on_capacity, 1); } @@ -384,6 +390,51 @@ impl SchedulerController { ); } } + + /// 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: &SanitizedTransaction, + fee_budget_limits: &FeeBudgetLimits, + bank: &Bank, + ) -> (u64, u64) { + let cost = CostModel::calculate_cost(transaction, &bank.feature_set).sum(); + let fee = bank.fee_structure.calculate_fee( + transaction.message(), + 5_000, // this just needs to be non-zero + fee_budget_limits, + bank.feature_set + .is_active(&include_loaded_accounts_data_size_in_fee_calculation::id()), + ); + + // 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; + ( + fee.saturating_mul(MULTIPLIER) + .saturating_div(cost.saturating_add(1)), + cost, + ) + } } #[derive(Default)] @@ -475,16 +526,8 @@ impl SchedulerCountMetrics { i64 ), ("num_dropped_on_capacity", self.num_dropped_on_capacity, i64), - ( - "min_prioritization_fees", - self.get_min_prioritization_fees(), - i64 - ), - ( - "max_prioritization_fees", - self.get_max_prioritization_fees(), - i64 - ) + ("min_priority", self.get_min_priority(), i64), + ("max_priority", self.get_max_priority(), i64) ); } @@ -524,8 +567,8 @@ impl SchedulerCountMetrics { self.max_prioritization_fees = 0; } - pub fn update_prioritization_stats(&mut self, min_max_fees: MinMaxResult) { - // update min/max priotization fees + pub fn update_priority_stats(&mut self, min_max_fees: MinMaxResult) { + // update min/max priority match min_max_fees { itertools::MinMaxResult::NoElements => { // do nothing @@ -541,7 +584,7 @@ impl SchedulerCountMetrics { } } - pub fn get_min_prioritization_fees(&self) -> u64 { + pub fn get_min_priority(&self) -> u64 { // to avoid getting u64::max recorded by metrics / in case of edge cases if self.min_prioritization_fees != u64::MAX { self.min_prioritization_fees @@ -550,7 +593,7 @@ impl SchedulerCountMetrics { } } - pub fn get_max_prioritization_fees(&self) -> u64 { + pub fn get_max_priority(&self) -> u64 { self.max_prioritization_fees } } @@ -728,7 +771,7 @@ mod tests { from_keypair: &Keypair, to_pubkey: &Pubkey, lamports: u64, - priority: u64, + compute_unit_price: u64, recent_blockhash: Hash, ) -> Transaction { // Fund the sending key, so that the transaction does not get filtered by the fee-payer check. @@ -743,7 +786,7 @@ mod tests { } let transfer = system_instruction::transfer(&from_keypair.pubkey(), to_pubkey, lamports); - let prioritization = ComputeBudgetInstruction::set_compute_unit_price(priority); + let prioritization = ComputeBudgetInstruction::set_compute_unit_price(compute_unit_price); let message = Message::new(&[transfer, prioritization], Some(&from_keypair.pubkey())); Transaction::new(&vec![from_keypair], message, recent_blockhash) } @@ -999,7 +1042,7 @@ mod tests { &Keypair::new(), &Pubkey::new_unique(), 1, - i, + i * 10, bank.last_blockhash(), ) }) diff --git a/core/src/banking_stage/transaction_scheduler/transaction_state.rs b/core/src/banking_stage/transaction_scheduler/transaction_state.rs index e8878e25c..727140545 100644 --- a/core/src/banking_stage/transaction_scheduler/transaction_state.rs +++ b/core/src/banking_stage/transaction_scheduler/transaction_state.rs @@ -1,8 +1,4 @@ -use { - solana_cost_model::transaction_cost::TransactionCost, - solana_runtime::compute_budget_details::ComputeBudgetDetails, - solana_sdk::{slot_history::Slot, transaction::SanitizedTransaction}, -}; +use solana_sdk::{clock::Slot, transaction::SanitizedTransaction}; /// Simple wrapper type to tie a sanitized transaction to max age slot. pub(crate) struct SanitizedTransactionTTL { @@ -34,77 +30,38 @@ pub(crate) enum TransactionState { /// The transaction is available for scheduling. Unprocessed { transaction_ttl: SanitizedTransactionTTL, - compute_budget_details: ComputeBudgetDetails, - transaction_cost: TransactionCost, - forwarded: bool, + priority: u64, + cost: u64, }, /// The transaction is currently scheduled or being processed. - Pending { - compute_budget_details: ComputeBudgetDetails, - transaction_cost: TransactionCost, - forwarded: bool, - }, + Pending { priority: u64, cost: u64 }, } impl TransactionState { /// Creates a new `TransactionState` in the `Unprocessed` state. - pub(crate) fn new( - transaction_ttl: SanitizedTransactionTTL, - compute_budget_details: ComputeBudgetDetails, - transaction_cost: TransactionCost, - ) -> Self { + pub(crate) fn new(transaction_ttl: SanitizedTransactionTTL, priority: u64, cost: u64) -> Self { Self::Unprocessed { transaction_ttl, - compute_budget_details, - transaction_cost, - forwarded: false, + priority, + cost, } } - /// Returns a reference to the compute budget details of the transaction. - pub(crate) fn compute_budget_details(&self) -> &ComputeBudgetDetails { + /// Return the priority of the transaction. + /// This is *not* the same as the `compute_unit_price` of the transaction. + /// The priority is used to order transactions for processing. + pub(crate) fn priority(&self) -> u64 { match self { - Self::Unprocessed { - compute_budget_details, - .. - } => compute_budget_details, - Self::Pending { - compute_budget_details, - .. - } => compute_budget_details, + Self::Unprocessed { priority, .. } => *priority, + Self::Pending { priority, .. } => *priority, } } - /// Returns a reference to the transaction cost of the transaction. - pub(crate) fn transaction_cost(&self) -> &TransactionCost { + /// Return the cost of the transaction. + pub(crate) fn cost(&self) -> u64 { match self { - Self::Unprocessed { - transaction_cost, .. - } => transaction_cost, - Self::Pending { - transaction_cost, .. - } => transaction_cost, - } - } - - /// Returns the compute unit price of the transaction. - pub(crate) fn compute_unit_price(&self) -> u64 { - self.compute_budget_details().compute_unit_price - } - - /// Returns whether or not the transaction has already been forwarded. - pub(crate) fn forwarded(&self) -> bool { - match self { - Self::Unprocessed { forwarded, .. } => *forwarded, - Self::Pending { forwarded, .. } => *forwarded, - } - } - - /// Sets the transaction as forwarded. - pub(crate) fn set_forwarded(&mut self) { - match self { - Self::Unprocessed { forwarded, .. } => *forwarded = true, - Self::Pending { forwarded, .. } => *forwarded = true, + Self::Unprocessed { cost, .. } => *cost, + Self::Pending { cost, .. } => *cost, } } @@ -119,15 +76,10 @@ impl TransactionState { match self.take() { TransactionState::Unprocessed { transaction_ttl, - compute_budget_details, - transaction_cost, - forwarded, + priority, + cost, } => { - *self = TransactionState::Pending { - compute_budget_details, - transaction_cost, - forwarded, - }; + *self = TransactionState::Pending { priority, cost }; transaction_ttl } TransactionState::Pending { .. } => { @@ -145,16 +97,11 @@ impl TransactionState { pub(crate) fn transition_to_unprocessed(&mut self, transaction_ttl: SanitizedTransactionTTL) { match self.take() { TransactionState::Unprocessed { .. } => panic!("already unprocessed"), - TransactionState::Pending { - compute_budget_details, - transaction_cost, - forwarded, - } => { + TransactionState::Pending { priority, cost } => { *self = Self::Unprocessed { transaction_ttl, - compute_budget_details, - transaction_cost, - forwarded, + priority, + cost, } } } @@ -179,14 +126,8 @@ impl TransactionState { core::mem::replace( self, Self::Pending { - compute_budget_details: ComputeBudgetDetails { - compute_unit_price: 0, - compute_unit_limit: 0, - }, - transaction_cost: TransactionCost::SimpleVote { - writable_accounts: vec![], - }, - forwarded: false, + priority: 0, + cost: 0, }, ) } @@ -196,7 +137,6 @@ impl TransactionState { mod tests { use { super::*, - solana_cost_model::transaction_cost::UsageCostDetails, solana_sdk::{ compute_budget::ComputeBudgetInstruction, hash::Hash, message::Message, signature::Keypair, signer::Signer, system_instruction, transaction::Transaction, @@ -215,24 +155,13 @@ mod tests { ]; let message = Message::new(&ixs, Some(&from_keypair.pubkey())); let tx = Transaction::new(&[&from_keypair], message, Hash::default()); - let transaction_cost = TransactionCost::Transaction(UsageCostDetails { - signature_cost: 5000, - ..UsageCostDetails::default() - }); let transaction_ttl = SanitizedTransactionTTL { transaction: SanitizedTransaction::from_transaction_for_tests(tx), max_age_slot: Slot::MAX, }; - - TransactionState::new( - transaction_ttl, - ComputeBudgetDetails { - compute_unit_price, - compute_unit_limit: 0, - }, - transaction_cost, - ) + const TEST_TRANSACTION_COST: u64 = 5000; + TransactionState::new(transaction_ttl, compute_unit_price, TEST_TRANSACTION_COST) } #[test] @@ -294,16 +223,16 @@ mod tests { } #[test] - fn test_compute_unit_price() { - let compute_unit_price = 15; - let mut transaction_state = create_transaction_state(compute_unit_price); - assert_eq!(transaction_state.compute_unit_price(), compute_unit_price); + fn test_priority() { + let priority = 15; + let mut transaction_state = create_transaction_state(priority); + assert_eq!(transaction_state.priority(), priority); // ensure compute unit price is not lost through state transitions let transaction_ttl = transaction_state.transition_to_pending(); - assert_eq!(transaction_state.compute_unit_price(), compute_unit_price); + assert_eq!(transaction_state.priority(), priority); transaction_state.transition_to_unprocessed(transaction_ttl); - assert_eq!(transaction_state.compute_unit_price(), compute_unit_price); + assert_eq!(transaction_state.priority(), priority); } #[test] diff --git a/core/src/banking_stage/transaction_scheduler/transaction_state_container.rs b/core/src/banking_stage/transaction_scheduler/transaction_state_container.rs index e314a3e49..a627375a0 100644 --- a/core/src/banking_stage/transaction_scheduler/transaction_state_container.rs +++ b/core/src/banking_stage/transaction_scheduler/transaction_state_container.rs @@ -6,8 +6,6 @@ use { crate::banking_stage::scheduler_messages::TransactionId, itertools::MinMaxResult, min_max_heap::MinMaxHeap, - solana_cost_model::transaction_cost::TransactionCost, - solana_runtime::compute_budget_details::ComputeBudgetDetails, std::collections::HashMap, }; @@ -99,14 +97,13 @@ impl TransactionStateContainer { &mut self, transaction_id: TransactionId, transaction_ttl: SanitizedTransactionTTL, - compute_budget_details: ComputeBudgetDetails, - transaction_cost: TransactionCost, + priority: u64, + cost: u64, ) -> bool { - let priority_id = - TransactionPriorityId::new(compute_budget_details.compute_unit_price, transaction_id); + let priority_id = TransactionPriorityId::new(priority, transaction_id); self.id_to_transaction_state.insert( transaction_id, - TransactionState::new(transaction_ttl, compute_budget_details, transaction_cost), + TransactionState::new(transaction_ttl, priority, cost), ); self.push_id_into_queue(priority_id) } @@ -121,8 +118,7 @@ impl TransactionStateContainer { let transaction_state = self .get_mut_transaction_state(&transaction_id) .expect("transaction must exist"); - let priority_id = - TransactionPriorityId::new(transaction_state.compute_unit_price(), transaction_id); + let priority_id = TransactionPriorityId::new(transaction_state.priority(), transaction_id); transaction_state.transition_to_unprocessed(transaction_ttl); self.push_id_into_queue(priority_id); } @@ -148,7 +144,7 @@ impl TransactionStateContainer { .expect("transaction must exist"); } - pub(crate) fn get_min_max_prioritization_fees(&self) -> MinMaxResult { + pub(crate) fn get_min_max_priority(&self) -> MinMaxResult { match self.priority_queue.peek_min() { Some(min) => match self.priority_queue.peek_max() { Some(max) => MinMaxResult::MinMax(min.priority, max.priority), @@ -163,10 +159,8 @@ impl TransactionStateContainer { mod tests { use { super::*, - solana_cost_model::cost_model::CostModel, solana_sdk::{ compute_budget::ComputeBudgetInstruction, - feature_set::FeatureSet, hash::Hash, message::Message, signature::Keypair, @@ -177,13 +171,8 @@ mod tests { }, }; - fn test_transaction( - priority: u64, - ) -> ( - SanitizedTransactionTTL, - ComputeBudgetDetails, - TransactionCost, - ) { + /// Returns (transaction_ttl, priority, cost) + fn test_transaction(priority: u64) -> (SanitizedTransactionTTL, u64, u64) { let from_keypair = Keypair::new(); let ixs = vec![ system_instruction::transfer( @@ -199,31 +188,23 @@ mod tests { message, Hash::default(), )); - let transaction_cost = CostModel::calculate_cost(&tx, &FeatureSet::default()); let transaction_ttl = SanitizedTransactionTTL { transaction: tx, max_age_slot: Slot::MAX, }; - ( - transaction_ttl, - ComputeBudgetDetails { - compute_unit_price: priority, - compute_unit_limit: 0, - }, - transaction_cost, - ) + const TEST_TRANSACTION_COST: u64 = 5000; + (transaction_ttl, priority, TEST_TRANSACTION_COST) } fn push_to_container(container: &mut TransactionStateContainer, num: usize) { for id in 0..num as u64 { let priority = id; - let (transaction_ttl, compute_budget_details, transaction_cost) = - test_transaction(priority); + let (transaction_ttl, priority, cost) = test_transaction(priority); container.insert_new_transaction( TransactionId::new(id), transaction_ttl, - compute_budget_details, - transaction_cost, + priority, + cost, ); } } @@ -248,7 +229,7 @@ mod tests { container .id_to_transaction_state .iter() - .map(|ts| ts.1.compute_unit_price()) + .map(|ts| ts.1.priority()) .next() .unwrap(), 4