v1.18: Scheduler - prioritization fees/cost (#34888)

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::transaction_priority_details::TransactionPriorityDetails,
         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,
     };
@@ -568,20 +565,12 @@ mod tests {
             let id = TransactionId::new(index as u64);
             let transaction =
                 prioritized_tranfers(from_keypair.borrow(), to_pubkeys, lamports, priority);
-            let transaction_cost = CostModel::calculate_cost(&transaction, &FeatureSet::default());
             let transaction_ttl = SanitizedTransactionTTL {
                 transaction,
                 max_age_slot: Slot::MAX,
             };
-            container.insert_new_transaction(
-                id,
-                transaction_ttl,
-                TransactionPriorityDetails {
-                    priority,
-                    compute_unit_limit: 1,
-                },
-                transaction_cost,
-            );
+            const TEST_TRANSACTION_COST: u64 = 5000;
+            container.insert_new_transaction(id, transaction_ttl, priority, TEST_TRANSACTION_COST);
         }
 
         container

core/src/banking_stage/transaction_scheduler/scheduler_controller.rs

@@ -20,10 +20,12 @@ use {
     solana_accounts_db::transaction_error_metrics::TransactionErrorMetrics,
     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,
     },
     std::{
         sync::{Arc, RwLock},
@@ -309,21 +311,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, priority_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.priority_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(
@@ -335,16 +340,17 @@ impl SchedulerController {
             let post_lock_validation_count = transactions.len();
 
             let mut post_transaction_check_count: usize = 0;
-            for ((transaction, priority_details), _) in transactions
+            for ((transaction, fee_budget_limits), _) in transactions
                 .into_iter()
-                .zip(priority_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,
@@ -353,8 +359,8 @@ impl SchedulerController {
                 if self.container.insert_new_transaction(
                     transaction_id,
                     transaction_ttl,
-                    priority_details,
-                    transaction_cost,
+                    priority,
+                    cost,
                 ) {
                     saturating_add_assign!(self.count_metrics.num_dropped_on_capacity, 1);
                 }
@@ -382,6 +388,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)]
@@ -468,7 +519,7 @@ impl SchedulerCountMetrics {
                 self.num_dropped_on_age_and_status,
                 i64
             ),
-            ("num_dropped_on_capacity", self.num_dropped_on_capacity, i64)
+            ("num_dropped_on_capacity", self.num_dropped_on_capacity, i64),
         );
     }
 
@@ -680,7 +731,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.
@@ -695,7 +746,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)
     }
@@ -951,7 +1002,7 @@ mod tests {
                     &Keypair::new(),
                     &Pubkey::new_unique(),
                     1,
-                    i,
+                    i * 10,
                     bank.last_blockhash(),
                 )
             })

core/src/banking_stage/transaction_scheduler/transaction_state.rs

@@ -1,8 +1,4 @@
-use {
-    solana_cost_model::transaction_cost::TransactionCost,
-    solana_runtime::transaction_priority_details::TransactionPriorityDetails,
-    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,33 +30,24 @@ pub(crate) enum TransactionState {
     /// The transaction is available for scheduling.
     Unprocessed {
         transaction_ttl: SanitizedTransactionTTL,
-        transaction_priority_details: TransactionPriorityDetails,
-        transaction_cost: TransactionCost,
-        forwarded: bool,
+        priority: u64,
+        cost: u64,
     },
     /// The transaction is currently scheduled or being processed.
-    Pending {
-        transaction_priority_details: TransactionPriorityDetails,
-        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,
-        transaction_priority_details: TransactionPriorityDetails,
-        transaction_cost: TransactionCost,
-    ) -> Self {
+    pub(crate) fn new(transaction_ttl: SanitizedTransactionTTL, priority: u64, cost: u64) -> Self {
         Self::Unprocessed {
             transaction_ttl,
-            transaction_priority_details,
-            transaction_cost,
-            forwarded: false,
+            priority,
+            cost,
         }
     }
 
+    /*
     /// Returns a reference to the priority details of the transaction.
     pub(crate) fn transaction_priority_details(&self) -> &TransactionPriorityDetails {
         match self {
@@ -107,6 +94,25 @@ impl TransactionState {
             Self::Pending { forwarded, .. } => *forwarded = true,
         }
     }
+    */
+
+    /// 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 { priority, .. } => *priority,
+            Self::Pending { priority, .. } => *priority,
+        }
+    }
+
+    /// Return the cost of the transaction.
+    pub(crate) fn cost(&self) -> u64 {
+        match self {
+            Self::Unprocessed { cost, .. } => *cost,
+            Self::Pending { cost, .. } => *cost,
+        }
+    }
 
     /// Intended to be called when a transaction is scheduled. This method will
     /// transition the transaction from `Unprocessed` to `Pending` and return the
@@ -119,15 +125,10 @@ impl TransactionState {
         match self.take() {
             TransactionState::Unprocessed {
                 transaction_ttl,
-                transaction_priority_details,
-                transaction_cost,
-                forwarded,
+                priority,
+                cost,
             } => {
-                *self = TransactionState::Pending {
-                    transaction_priority_details,
-                    transaction_cost,
-                    forwarded,
-                };
+                *self = TransactionState::Pending { priority, cost };
                 transaction_ttl
             }
             TransactionState::Pending { .. } => {
@@ -145,16 +146,11 @@ impl TransactionState {
     pub(crate) fn transition_to_unprocessed(&mut self, transaction_ttl: SanitizedTransactionTTL) {
         match self.take() {
             TransactionState::Unprocessed { .. } => panic!("already unprocessed"),
-            TransactionState::Pending {
-                transaction_priority_details,
-                transaction_cost,
-                forwarded,
-            } => {
+            TransactionState::Pending { priority, cost } => {
                 *self = Self::Unprocessed {
                     transaction_ttl,
-                    transaction_priority_details,
-                    transaction_cost,
-                    forwarded,
+                    priority,
+                    cost,
                 }
             }
         }
@@ -179,14 +175,8 @@ impl TransactionState {
         core::mem::replace(
             self,
             Self::Pending {
-                transaction_priority_details: TransactionPriorityDetails {
-                    priority: 0,
-                    compute_unit_limit: 0,
-                },
-                transaction_cost: TransactionCost::SimpleVote {
-                    writable_accounts: vec![],
-                },
-                forwarded: false,
+                priority: 0,
+                cost: 0,
             },
         )
     }
@@ -196,7 +186,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 +204,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,
-            TransactionPriorityDetails {
-                priority,
-                compute_unit_limit: 0,
-            },
-            transaction_cost,
-        )
+        const TEST_TRANSACTION_COST: u64 = 5000;
+        TransactionState::new(transaction_ttl, priority, TEST_TRANSACTION_COST)
     }
 
     #[test]
@@ -294,7 +272,7 @@ mod tests {
     }
 
     #[test]
-    fn test_transaction_priority_details() {
+    fn test_priority() {
         let priority = 15;
         let mut transaction_state = create_transaction_state(priority);
         assert_eq!(transaction_state.priority(), priority);