Combine builtin and BPF compute cost in cost model (#29)

* Combine builtin and BPF execution cost into programs_execution_cost since VM has started to consume CUs uniformly

* update tests

* apply suggestions from code review

core/src/banking_stage.rs

@@ -285,8 +285,7 @@ pub struct BatchedTransactionCostDetails {
     pub batched_signature_cost: u64,
     pub batched_write_lock_cost: u64,
     pub batched_data_bytes_cost: u64,
-    pub batched_builtins_execute_cost: u64,
-    pub batched_bpf_execute_cost: u64,
+    pub batched_programs_execute_cost: u64,
 }
 
 #[derive(Debug, Default)]

core/src/banking_stage/consumer.rs

@@ -1549,16 +1549,17 @@ mod tests {
             assert_eq!(retryable_transaction_indexes, vec![1]);
 
             let expected_block_cost = if !apply_cost_tracker_during_replay_enabled {
-                let actual_bpf_execution_cost = match commit_transactions_result.first().unwrap() {
-                    CommitTransactionDetails::Committed { compute_units } => *compute_units,
-                    CommitTransactionDetails::NotCommitted => {
-                        unreachable!()
-                    }
-                };
+                let actual_programs_execution_cost =
+                    match commit_transactions_result.first().unwrap() {
+                        CommitTransactionDetails::Committed { compute_units } => *compute_units,
+                        CommitTransactionDetails::NotCommitted => {
+                            unreachable!()
+                        }
+                    };
 
                 let mut cost = CostModel::calculate_cost(&transactions[0], &bank.feature_set);
                 if let TransactionCost::Transaction(ref mut usage_cost) = cost {
-                    usage_cost.bpf_execution_cost = actual_bpf_execution_cost;
+                    usage_cost.programs_execution_cost = actual_programs_execution_cost;
                 }
 
                 block_cost + cost.sum()

core/src/banking_stage/qos_service.rs

@@ -236,14 +236,10 @@ impl QosService {
             batched_transaction_details.costs.batched_data_bytes_cost,
             Ordering::Relaxed,
         );
-        self.metrics.stats.estimated_builtins_execute_cu.fetch_add(
+        self.metrics.stats.estimated_programs_execute_cu.fetch_add(
             batched_transaction_details
                 .costs
-                .batched_builtins_execute_cost,
-            Ordering::Relaxed,
-        );
-        self.metrics.stats.estimated_bpf_execute_cu.fetch_add(
-            batched_transaction_details.costs.batched_bpf_execute_cost,
+                .batched_programs_execute_cost,
             Ordering::Relaxed,
         );
 
@@ -297,7 +293,7 @@ impl QosService {
     pub fn accumulate_actual_execute_cu(&self, units: u64) {
         self.metrics
             .stats
-            .actual_bpf_execute_cu
+            .actual_programs_execute_cu
             .fetch_add(units, Ordering::Relaxed);
     }
 
@@ -331,12 +327,8 @@ impl QosService {
                 saturating_add_assign!(
                     batched_transaction_details
                         .costs
-                        .batched_builtins_execute_cost,
-                    cost.builtins_execution_cost()
-                );
-                saturating_add_assign!(
-                    batched_transaction_details.costs.batched_bpf_execute_cost,
-                    cost.bpf_execution_cost()
+                        .batched_programs_execute_cost,
+                    cost.programs_execution_cost()
                 );
             }
             Err(transaction_error) => match transaction_error {
@@ -427,14 +419,11 @@ struct QosServiceMetricsStats {
     /// accumulated estimated instruction data Compute Units to be packed into block
     estimated_data_bytes_cu: AtomicU64,
 
-    /// accumulated estimated builtin programs Compute Units to be packed into block
-    estimated_builtins_execute_cu: AtomicU64,
-
-    /// accumulated estimated SBF program Compute Units to be packed into block
-    estimated_bpf_execute_cu: AtomicU64,
+    /// accumulated estimated program Compute Units to be packed into block
+    estimated_programs_execute_cu: AtomicU64,
 
     /// accumulated actual program Compute Units that have been packed into block
-    actual_bpf_execute_cu: AtomicU64,
+    actual_programs_execute_cu: AtomicU64,
 
     /// accumulated actual program execute micro-sec that have been packed into block
     actual_execute_time_us: AtomicU64,
@@ -515,24 +504,19 @@ impl QosServiceMetrics {
                     i64
                 ),
                 (
-                    "estimated_builtins_execute_cu",
+                    "estimated_programs_execute_cu",
                     self.stats
-                        .estimated_builtins_execute_cu
+                        .estimated_programs_execute_cu
                         .swap(0, Ordering::Relaxed),
                     i64
                 ),
                 (
-                    "estimated_bpf_execute_cu",
+                    "actual_programs_execute_cu",
                     self.stats
-                        .estimated_bpf_execute_cu
+                        .actual_programs_execute_cu
                         .swap(0, Ordering::Relaxed),
                     i64
                 ),
-                (
-                    "actual_bpf_execute_cu",
-                    self.stats.actual_bpf_execute_cu.swap(0, Ordering::Relaxed),
-                    i64
-                ),
                 (
                     "actual_execute_time_us",
                     self.stats.actual_execute_time_us.swap(0, Ordering::Relaxed),
@@ -735,7 +719,7 @@ mod tests {
             let committed_status: Vec<CommitTransactionDetails> = qos_cost_results
                 .iter()
                 .map(|tx_cost| CommitTransactionDetails::Committed {
-                    compute_units: tx_cost.as_ref().unwrap().bpf_execution_cost()
+                    compute_units: tx_cost.as_ref().unwrap().programs_execution_cost()
                         + execute_units_adjustment,
                 })
                 .collect();
@@ -862,7 +846,7 @@ mod tests {
                         CommitTransactionDetails::NotCommitted
                     } else {
                         CommitTransactionDetails::Committed {
-                            compute_units: tx_cost.as_ref().unwrap().bpf_execution_cost()
+                            compute_units: tx_cost.as_ref().unwrap().programs_execution_cost()
                                 + execute_units_adjustment,
                         }
                     }
@@ -898,8 +882,7 @@ mod tests {
         let signature_cost = 1;
         let write_lock_cost = 2;
         let data_bytes_cost = 3;
-        let builtins_execution_cost = 4;
-        let bpf_execution_cost = 10;
+        let programs_execution_cost = 10;
         let num_txs = 4;
 
         let tx_cost_results: Vec<_> = (0..num_txs)
@@ -909,8 +892,7 @@ mod tests {
                         signature_cost,
                         write_lock_cost,
                         data_bytes_cost,
-                        builtins_execution_cost,
-                        bpf_execution_cost,
+                        programs_execution_cost,
                         ..UsageCostDetails::default()
                     }))
                 } else {
@@ -922,8 +904,7 @@ mod tests {
         let expected_signatures = signature_cost * (num_txs / 2);
         let expected_write_locks = write_lock_cost * (num_txs / 2);
         let expected_data_bytes = data_bytes_cost * (num_txs / 2);
-        let expected_builtins_execution_costs = builtins_execution_cost * (num_txs / 2);
-        let expected_bpf_execution_costs = bpf_execution_cost * (num_txs / 2);
+        let expected_programs_execution_costs = programs_execution_cost * (num_txs / 2);
         let batched_transaction_details =
             QosService::accumulate_batched_transaction_costs(tx_cost_results.iter());
         assert_eq!(
@@ -939,14 +920,10 @@ mod tests {
             batched_transaction_details.costs.batched_data_bytes_cost
         );
         assert_eq!(
-            expected_builtins_execution_costs,
+            expected_programs_execution_costs,
             batched_transaction_details
                 .costs
-                .batched_builtins_execute_cost
-        );
-        assert_eq!(
-            expected_bpf_execution_costs,
-            batched_transaction_details.costs.batched_bpf_execute_cost
+                .batched_programs_execute_cost
         );
     }
 }

cost-model/src/cost_model.rs

@@ -93,21 +93,25 @@ impl CostModel {
         transaction: &SanitizedTransaction,
         feature_set: &FeatureSet,
     ) {
-        let mut builtin_costs = 0u64;
-        let mut bpf_costs = 0u64;
+        let mut programs_execution_costs = 0u64;
         let mut loaded_accounts_data_size_cost = 0u64;
         let mut data_bytes_len_total = 0u64;
         let mut compute_unit_limit_is_set = false;
+        let mut has_user_space_instructions = false;
 
         for (program_id, instruction) in transaction.message().program_instructions_iter() {
-            // to keep the same behavior, look for builtin first
-            if let Some(builtin_cost) = BUILT_IN_INSTRUCTION_COSTS.get(program_id) {
-                builtin_costs = builtin_costs.saturating_add(*builtin_cost);
-            } else {
-                bpf_costs = bpf_costs
-                    .saturating_add(u64::from(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT))
-                    .min(u64::from(MAX_COMPUTE_UNIT_LIMIT));
-            }
+            let ix_execution_cost =
+                if let Some(builtin_cost) = BUILT_IN_INSTRUCTION_COSTS.get(program_id) {
+                    *builtin_cost
+                } else {
+                    has_user_space_instructions = true;
+                    u64::from(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT)
+                };
+
+            programs_execution_costs = programs_execution_costs
+                .saturating_add(ix_execution_cost)
+                .min(u64::from(MAX_COMPUTE_UNIT_LIMIT));
+
             data_bytes_len_total =
                 data_bytes_len_total.saturating_add(instruction.data.len() as u64);
 
@@ -120,8 +124,6 @@ impl CostModel {
             }
         }
 
-        // calculate bpf cost based on compute budget instructions
-
         // 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.message().program_instructions_iter())
@@ -132,8 +134,8 @@ impl CostModel {
                 // 'compute_unit_limit_is_set' flag, because compute_budget does not distinguish
                 // builtin and bpf instructions when calculating default compute-unit-limit. (see
                 // compute_budget.rs test `test_process_mixed_instructions_without_compute_budget`)
-                if bpf_costs > 0 && compute_unit_limit_is_set {
-                    bpf_costs = u64::from(compute_budget_limits.compute_unit_limit);
+                if has_user_space_instructions && compute_unit_limit_is_set {
+                    programs_execution_costs = u64::from(compute_budget_limits.compute_unit_limit);
                 }
 
                 if feature_set
@@ -146,13 +148,11 @@ impl CostModel {
                 }
             }
             Err(_) => {
-                builtin_costs = 0;
-                bpf_costs = 0;
+                programs_execution_costs = 0;
             }
         }
 
-        tx_cost.builtins_execution_cost = builtin_costs;
-        tx_cost.bpf_execution_cost = bpf_costs;
+        tx_cost.programs_execution_cost = programs_execution_costs;
         tx_cost.loaded_accounts_data_size_cost = loaded_accounts_data_size_cost;
         tx_cost.data_bytes_cost = data_bytes_len_total / INSTRUCTION_DATA_BYTES_COST;
     }
@@ -304,8 +304,7 @@ mod tests {
             &simple_transaction,
             &FeatureSet::all_enabled(),
         );
-        assert_eq!(*expected_execution_cost, tx_cost.builtins_execution_cost);
-        assert_eq!(0, tx_cost.bpf_execution_cost);
+        assert_eq!(*expected_execution_cost, tx_cost.programs_execution_cost);
         assert_eq!(3, tx_cost.data_bytes_cost);
     }
 
@@ -333,8 +332,10 @@ mod tests {
             &token_transaction,
             &FeatureSet::all_enabled(),
         );
-        assert_eq!(0, tx_cost.builtins_execution_cost);
-        assert_eq!(200_000, tx_cost.bpf_execution_cost);
+        assert_eq!(
+            DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64,
+            tx_cost.programs_execution_cost
+        );
         assert_eq!(0, tx_cost.data_bytes_cost);
     }
 
@@ -396,13 +397,8 @@ mod tests {
             &token_transaction,
             &FeatureSet::all_enabled(),
         );
-        assert_eq!(
-            *BUILT_IN_INSTRUCTION_COSTS
-                .get(&compute_budget::id())
-                .unwrap(),
-            tx_cost.builtins_execution_cost
-        );
-        assert_eq!(12_345, tx_cost.bpf_execution_cost);
+        // If cu-limit is specified, that would the cost for all programs
+        assert_eq!(12_345, tx_cost.programs_execution_cost);
         assert_eq!(1, tx_cost.data_bytes_cost);
     }
 
@@ -446,8 +442,7 @@ mod tests {
             &token_transaction,
             &FeatureSet::all_enabled(),
         );
-        assert_eq!(0, tx_cost.builtins_execution_cost);
-        assert_eq!(0, tx_cost.bpf_execution_cost);
+        assert_eq!(0, tx_cost.programs_execution_cost);
     }
 
     #[test]
@@ -474,8 +469,7 @@ mod tests {
 
         let mut tx_cost = UsageCostDetails::default();
         CostModel::get_transaction_cost(&mut tx_cost, &tx, &FeatureSet::all_enabled());
-        assert_eq!(expected_cost, tx_cost.builtins_execution_cost);
-        assert_eq!(0, tx_cost.bpf_execution_cost);
+        assert_eq!(expected_cost, tx_cost.programs_execution_cost);
         assert_eq!(6, tx_cost.data_bytes_cost);
     }
 
@@ -506,8 +500,7 @@ mod tests {
         let expected_cost = DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64 * 2;
         let mut tx_cost = UsageCostDetails::default();
         CostModel::get_transaction_cost(&mut tx_cost, &tx, &FeatureSet::all_enabled());
-        assert_eq!(0, tx_cost.builtins_execution_cost);
-        assert_eq!(expected_cost, tx_cost.bpf_execution_cost);
+        assert_eq!(expected_cost, tx_cost.programs_execution_cost);
         assert_eq!(0, tx_cost.data_bytes_cost);
     }
 
@@ -567,7 +560,7 @@ mod tests {
 
         let tx_cost = CostModel::calculate_cost(&tx, &FeatureSet::all_enabled());
         assert_eq!(expected_account_cost, tx_cost.write_lock_cost());
-        assert_eq!(*expected_execution_cost, tx_cost.builtins_execution_cost());
+        assert_eq!(*expected_execution_cost, tx_cost.programs_execution_cost());
         assert_eq!(2, tx_cost.writable_accounts().len());
         assert_eq!(
             expected_loaded_accounts_data_size_cost,
@@ -596,7 +589,7 @@ mod tests {
 
         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.builtins_execution_cost());
+        assert_eq!(*expected_execution_cost, tx_cost.programs_execution_cost());
         assert_eq!(2, tx_cost.writable_accounts().len());
         assert_eq!(
             expected_loaded_accounts_data_size_cost,
@@ -635,7 +628,7 @@ mod tests {
 
         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.builtins_execution_cost());
+        assert_eq!(expected_execution_cost, tx_cost.programs_execution_cost());
         assert_eq!(2, tx_cost.writable_accounts().len());
         assert_eq!(
             expected_loaded_accounts_data_size_cost,
@@ -666,7 +659,37 @@ mod tests {
         let mut tx_cost = UsageCostDetails::default();
         CostModel::get_transaction_cost(&mut tx_cost, &transaction, &FeatureSet::all_enabled());
 
-        assert_eq!(expected_builtin_cost, tx_cost.builtins_execution_cost);
-        assert_eq!(expected_bpf_cost as u64, tx_cost.bpf_execution_cost);
+        assert_eq!(
+            expected_builtin_cost + expected_bpf_cost as u64,
+            tx_cost.programs_execution_cost
+        );
+    }
+
+    #[test]
+    fn test_transaction_cost_with_mix_instruction_with_cu_limit() {
+        let (mint_keypair, start_hash) = test_setup();
+
+        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),
+                ],
+                Some(&mint_keypair.pubkey()),
+                &[&mint_keypair],
+                start_hash,
+            ));
+        // transaction has one builtin instruction, and one ComputeBudget::compute_unit_limit
+        let expected_cost = *BUILT_IN_INSTRUCTION_COSTS
+            .get(&solana_system_program::id())
+            .unwrap()
+            + BUILT_IN_INSTRUCTION_COSTS
+                .get(&compute_budget::id())
+                .unwrap();
+
+        let mut tx_cost = UsageCostDetails::default();
+        CostModel::get_transaction_cost(&mut tx_cost, &transaction, &FeatureSet::all_enabled());
+
+        assert_eq!(expected_cost, tx_cost.programs_execution_cost);
     }
 }