From 212ca352d38be6cdc841122db21547df47adab9b Mon Sep 17 00:00:00 2001 From: Tao Zhu Date: Sun, 3 Mar 2024 17:08:55 +0000 Subject: [PATCH 1/4] Combine builtin and BPF execution cost into programs_execution_cost since VM has started to consume CUs uniformly --- core/src/banking_stage.rs | 3 +- core/src/banking_stage/qos_service.rs | 42 ++++++++------------------- cost-model/src/cost_model.rs | 26 +++++++---------- cost-model/src/cost_tracker.rs | 2 +- cost-model/src/transaction_cost.rs | 28 +++++++----------- 5 files changed, 35 insertions(+), 66 deletions(-) diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs index 652f2569f8fd43..603ff55f0003b4 100644 --- a/core/src/banking_stage.rs +++ b/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)] diff --git a/core/src/banking_stage/qos_service.rs b/core/src/banking_stage/qos_service.rs index 77f05c73a3bc12..a342dd7f5ae304 100644 --- a/core/src/banking_stage/qos_service.rs +++ b/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,22 +504,15 @@ impl QosServiceMetrics { i64 ), ( - "estimated_builtins_execute_cu", - self.stats - .estimated_builtins_execute_cu - .swap(0, Ordering::Relaxed), - i64 - ), - ( - "estimated_bpf_execute_cu", + "estimated_programs_execute_cu", self.stats - .estimated_bpf_execute_cu + .estimated_programs_execute_cu .swap(0, Ordering::Relaxed), i64 ), ( - "actual_bpf_execute_cu", - self.stats.actual_bpf_execute_cu.swap(0, Ordering::Relaxed), + "actual_programs_execute_cu", + self.stats.actual_programs_execute_cu.swap(0, Ordering::Relaxed), i64 ), ( diff --git a/cost-model/src/cost_model.rs b/cost-model/src/cost_model.rs index 1e15735426737f..eb7869621f1e06 100644 --- a/cost-model/src/cost_model.rs +++ b/cost-model/src/cost_model.rs @@ -93,21 +93,21 @@ 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); + programs_execution_costs = if let Some(builtin_cost) = BUILT_IN_INSTRUCTION_COSTS.get(program_id) { + programs_execution_costs.saturating_add(*builtin_cost) } else { - bpf_costs = bpf_costs + has_user_space_instructions = true; + programs_execution_costs .saturating_add(u64::from(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT)) - .min(u64::from(MAX_COMPUTE_UNIT_LIMIT)); } + .min(u64::from(MAX_COMPUTE_UNIT_LIMIT)); data_bytes_len_total = data_bytes_len_total.saturating_add(instruction.data.len() as u64); @@ -120,8 +120,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 +130,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 +144,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; } diff --git a/cost-model/src/cost_tracker.rs b/cost-model/src/cost_tracker.rs index 9d2b3b624afeb4..d5679553e651a4 100644 --- a/cost-model/src/cost_tracker.rs +++ b/cost-model/src/cost_tracker.rs @@ -105,7 +105,7 @@ impl CostTracker { estimated_tx_cost: &TransactionCost, actual_execution_units: u64, ) { - let estimated_execution_units = estimated_tx_cost.bpf_execution_cost(); + let estimated_execution_units = estimated_tx_cost.programs_execution_cost(); match actual_execution_units.cmp(&estimated_execution_units) { Ordering::Equal => (), Ordering::Greater => { diff --git a/cost-model/src/transaction_cost.rs b/cost-model/src/transaction_cost.rs index e765eee3bc7038..f8dac2f6f17643 100644 --- a/cost-model/src/transaction_cost.rs +++ b/cost-model/src/transaction_cost.rs @@ -24,10 +24,10 @@ impl TransactionCost { } } - pub fn bpf_execution_cost(&self) -> u64 { + pub fn programs_execution_cost(&self) -> u64 { match self { - Self::SimpleVote { .. } => 0, - Self::Transaction(usage_cost) => usage_cost.bpf_execution_cost, + Self::SimpleVote { .. } => solana_vote_program::vote_processor::DEFAULT_COMPUTE_UNITS, + Self::Transaction(usage_cost) => usage_cost.programs_execution_cost, } } @@ -74,13 +74,6 @@ impl TransactionCost { } } - pub fn builtins_execution_cost(&self) -> u64 { - match self { - Self::SimpleVote { .. } => solana_vote_program::vote_processor::DEFAULT_COMPUTE_UNITS, - Self::Transaction(usage_cost) => usage_cost.builtins_execution_cost, - } - } - pub fn writable_accounts(&self) -> &[Pubkey] { match self { Self::SimpleVote { writable_accounts } => writable_accounts, @@ -98,8 +91,10 @@ pub struct UsageCostDetails { pub signature_cost: u64, pub write_lock_cost: u64, pub data_bytes_cost: u64, - pub builtins_execution_cost: u64, - pub bpf_execution_cost: u64, + // since https://github.com/solana-labs/solana/issues/30620 activated everywhere, + // all builtins consume compute units same as bpf. It is no longer necessary + // to separate builtin execution cost from bpf's + pub programs_execution_cost: u64, pub loaded_accounts_data_size_cost: u64, pub account_data_size: u64, } @@ -111,8 +106,7 @@ impl Default for UsageCostDetails { signature_cost: 0u64, write_lock_cost: 0u64, data_bytes_cost: 0u64, - builtins_execution_cost: 0u64, - bpf_execution_cost: 0u64, + programs_execution_cost: 0u64, loaded_accounts_data_size_cost: 0u64, account_data_size: 0u64, } @@ -129,8 +123,7 @@ impl PartialEq for UsageCostDetails { self.signature_cost == other.signature_cost && self.write_lock_cost == other.write_lock_cost && self.data_bytes_cost == other.data_bytes_cost - && self.builtins_execution_cost == other.builtins_execution_cost - && self.bpf_execution_cost == other.bpf_execution_cost + && self.programs_execution_cost == other.programs_execution_cost && self.loaded_accounts_data_size_cost == other.loaded_accounts_data_size_cost && self.account_data_size == other.account_data_size && to_hash_set(&self.writable_accounts) == to_hash_set(&other.writable_accounts) @@ -157,8 +150,7 @@ impl UsageCostDetails { self.signature_cost .saturating_add(self.write_lock_cost) .saturating_add(self.data_bytes_cost) - .saturating_add(self.builtins_execution_cost) - .saturating_add(self.bpf_execution_cost) + .saturating_add(self.programs_execution_cost) .saturating_add(self.loaded_accounts_data_size_cost) } } From 5b3fa84e51ae509d44b29a4e66411e7f10447575 Mon Sep 17 00:00:00 2001 From: Tao Zhu Date: Mon, 4 Mar 2024 17:30:22 +0000 Subject: [PATCH 2/4] update tests --- core/src/banking_stage/consumer.rs | 15 ++--- core/src/banking_stage/qos_service.rs | 25 ++++---- cost-model/src/cost_model.rs | 84 +++++++++++++++++---------- cost-model/src/cost_tracker.rs | 14 ++--- 4 files changed, 79 insertions(+), 59 deletions(-) diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index f4ac6c6040eda8..957e190c873f64 100644 --- a/core/src/banking_stage/consumer.rs +++ b/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() diff --git a/core/src/banking_stage/qos_service.rs b/core/src/banking_stage/qos_service.rs index a342dd7f5ae304..8c1507ae3fb91c 100644 --- a/core/src/banking_stage/qos_service.rs +++ b/core/src/banking_stage/qos_service.rs @@ -512,7 +512,9 @@ impl QosServiceMetrics { ), ( "actual_programs_execute_cu", - self.stats.actual_programs_execute_cu.swap(0, Ordering::Relaxed), + self.stats + .actual_programs_execute_cu + .swap(0, Ordering::Relaxed), i64 ), ( @@ -717,7 +719,7 @@ mod tests { let committed_status: Vec = 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(); @@ -844,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, } } @@ -880,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) @@ -891,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 { @@ -904,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!( @@ -921,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 ); } } diff --git a/cost-model/src/cost_model.rs b/cost-model/src/cost_model.rs index eb7869621f1e06..4900a4be2c7303 100644 --- a/cost-model/src/cost_model.rs +++ b/cost-model/src/cost_model.rs @@ -100,14 +100,15 @@ impl CostModel { let mut has_user_space_instructions = false; for (program_id, instruction) in transaction.message().program_instructions_iter() { - programs_execution_costs = if let Some(builtin_cost) = BUILT_IN_INSTRUCTION_COSTS.get(program_id) { - programs_execution_costs.saturating_add(*builtin_cost) - } else { - has_user_space_instructions = true; - programs_execution_costs - .saturating_add(u64::from(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT)) - } - .min(u64::from(MAX_COMPUTE_UNIT_LIMIT)); + programs_execution_costs = + if let Some(builtin_cost) = BUILT_IN_INSTRUCTION_COSTS.get(program_id) { + programs_execution_costs.saturating_add(*builtin_cost) + } else { + has_user_space_instructions = true; + programs_execution_costs + .saturating_add(u64::from(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT)) + } + .min(u64::from(MAX_COMPUTE_UNIT_LIMIT)); data_bytes_len_total = data_bytes_len_total.saturating_add(instruction.data.len() as u64); @@ -300,8 +301,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); } @@ -329,8 +329,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); } @@ -392,13 +394,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); } @@ -442,8 +439,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] @@ -470,8 +466,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); } @@ -502,8 +497,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); } @@ -563,7 +557,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, @@ -592,7 +586,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, @@ -631,7 +625,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, @@ -662,7 +656,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); } } diff --git a/cost-model/src/cost_tracker.rs b/cost-model/src/cost_tracker.rs index d5679553e651a4..8fb092c36680a0 100644 --- a/cost-model/src/cost_tracker.rs +++ b/cost-model/src/cost_tracker.rs @@ -307,7 +307,7 @@ mod tests { system_transaction::transfer(mint_keypair, &keypair.pubkey(), 2, *start_hash), ); let mut tx_cost = UsageCostDetails::new_with_capacity(1); - tx_cost.bpf_execution_cost = 5; + tx_cost.programs_execution_cost = 5; tx_cost.writable_accounts.push(mint_keypair.pubkey()); (simple_transaction, TransactionCost::Transaction(tx_cost)) @@ -606,7 +606,7 @@ mod tests { { let tx_cost = TransactionCost::Transaction(UsageCostDetails { writable_accounts: vec![acct1, acct2, acct3], - bpf_execution_cost: cost, + programs_execution_cost: cost, ..UsageCostDetails::default() }); assert!(testee.try_add(&tx_cost).is_ok()); @@ -624,7 +624,7 @@ mod tests { { let tx_cost = TransactionCost::Transaction(UsageCostDetails { writable_accounts: vec![acct2], - bpf_execution_cost: cost, + programs_execution_cost: cost, ..UsageCostDetails::default() }); assert!(testee.try_add(&tx_cost).is_ok()); @@ -644,7 +644,7 @@ mod tests { { let tx_cost = TransactionCost::Transaction(UsageCostDetails { writable_accounts: vec![acct1, acct2], - bpf_execution_cost: cost, + programs_execution_cost: cost, ..UsageCostDetails::default() }); assert!(testee.try_add(&tx_cost).is_err()); @@ -668,7 +668,7 @@ mod tests { let mut testee = CostTracker::new(account_max, block_max, block_max); let tx_cost = TransactionCost::Transaction(UsageCostDetails { writable_accounts: vec![acct1, acct2, acct3], - bpf_execution_cost: cost, + programs_execution_cost: cost, ..UsageCostDetails::default() }); let mut expected_block_cost = tx_cost.sum(); @@ -755,7 +755,7 @@ mod tests { let tx_cost = TransactionCost::Transaction(UsageCostDetails { writable_accounts: vec![acct1, acct2, acct3], - bpf_execution_cost: cost, + programs_execution_cost: cost, ..UsageCostDetails::default() }); @@ -802,7 +802,7 @@ mod tests { let cost = 100u64; let tx_cost = TransactionCost::Transaction(UsageCostDetails { writable_accounts: vec![Pubkey::new_unique()], - bpf_execution_cost: cost, + programs_execution_cost: cost, ..UsageCostDetails::default() }); From 2c1398a25e6796a0c968ccbdaeac02e6c96082e1 Mon Sep 17 00:00:00 2001 From: Tao Zhu Date: Mon, 4 Mar 2024 17:49:04 +0000 Subject: [PATCH 3/4] remove a comment --- cost-model/src/transaction_cost.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/cost-model/src/transaction_cost.rs b/cost-model/src/transaction_cost.rs index f8dac2f6f17643..94b07fb599a5a9 100644 --- a/cost-model/src/transaction_cost.rs +++ b/cost-model/src/transaction_cost.rs @@ -91,9 +91,6 @@ pub struct UsageCostDetails { pub signature_cost: u64, pub write_lock_cost: u64, pub data_bytes_cost: u64, - // since https://github.com/solana-labs/solana/issues/30620 activated everywhere, - // all builtins consume compute units same as bpf. It is no longer necessary - // to separate builtin execution cost from bpf's pub programs_execution_cost: u64, pub loaded_accounts_data_size_cost: u64, pub account_data_size: u64, From 79a31b97487bcb504f0e1707ed636abd5a312031 Mon Sep 17 00:00:00 2001 From: Tao Zhu Date: Wed, 6 Mar 2024 00:47:55 +0000 Subject: [PATCH 4/4] apply suggestions from code review --- cost-model/src/cost_model.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/cost-model/src/cost_model.rs b/cost-model/src/cost_model.rs index 4900a4be2c7303..b81ea24402d4df 100644 --- a/cost-model/src/cost_model.rs +++ b/cost-model/src/cost_model.rs @@ -100,15 +100,18 @@ impl CostModel { let mut has_user_space_instructions = false; for (program_id, instruction) in transaction.message().program_instructions_iter() { - programs_execution_costs = + let ix_execution_cost = if let Some(builtin_cost) = BUILT_IN_INSTRUCTION_COSTS.get(program_id) { - programs_execution_costs.saturating_add(*builtin_cost) + *builtin_cost } else { has_user_space_instructions = true; - programs_execution_costs - .saturating_add(u64::from(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT)) - } + 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);