From 798974ce2942b595bc72b9a57265a11615d66cb4 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Mon, 30 Sep 2024 10:02:19 -0500 Subject: [PATCH] calculate_cost takes is_simple_vote_transaction, signature_count_detail --- core/src/banking_stage/consumer.rs | 9 ++- .../forward_packet_batches_by_accounts.rs | 18 ++++- core/src/banking_stage/qos_service.rs | 50 +++++++++++-- .../scheduler_controller.rs | 11 ++- cost-model/benches/cost_model.rs | 20 +++++- cost-model/src/cost_model.rs | 72 +++++++++++++------ cost-model/src/transaction_cost.rs | 19 +++-- ledger-tool/src/main.rs | 9 ++- ledger/src/blockstore_processor.rs | 22 +++++- 9 files changed, 189 insertions(+), 41 deletions(-) diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 4bd6f50cd18b65..85e53e952106e3 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -1577,7 +1577,14 @@ mod tests { } }; - let mut cost = CostModel::calculate_cost(&transactions[0], &bank.feature_set); + let is_simple_vote_transaction = transactions[0].is_simple_vote_transaction(); + let signature_count_detail = transactions[0].message().get_signature_details(); + let mut cost = CostModel::calculate_cost( + &transactions[0], + is_simple_vote_transaction, + &signature_count_detail, + &bank.feature_set, + ); if let TransactionCost::Transaction(ref mut usage_cost) = cost { usage_cost.programs_execution_cost = actual_programs_execution_cost; usage_cost.loaded_accounts_data_size_cost = diff --git a/core/src/banking_stage/forward_packet_batches_by_accounts.rs b/core/src/banking_stage/forward_packet_batches_by_accounts.rs index 1d86cfb9753b1b..b8e949fa9a27c7 100644 --- a/core/src/banking_stage/forward_packet_batches_by_accounts.rs +++ b/core/src/banking_stage/forward_packet_batches_by_accounts.rs @@ -109,7 +109,14 @@ impl ForwardPacketBatchesByAccounts { immutable_packet: Arc, feature_set: &FeatureSet, ) -> bool { - let tx_cost = CostModel::calculate_cost(sanitized_transaction, feature_set); + let is_simple_vote_tx = sanitized_transaction.is_simple_vote_transaction(); + let signature_count_detail = sanitized_transaction.message().get_signature_details(); + let tx_cost = CostModel::calculate_cost( + sanitized_transaction, + is_simple_vote_tx, + &signature_count_detail, + feature_set, + ); if let Ok(updated_costs) = self.cost_tracker.try_add(&tx_cost) { let batch_index = self.get_batch_index_by_updated_costs(&tx_cost, &updated_costs); @@ -195,7 +202,14 @@ mod tests { )); let sanitized_transaction = SanitizedTransaction::from_transaction_for_tests(transaction.clone()); - let tx_cost = CostModel::calculate_cost(&sanitized_transaction, &FeatureSet::all_enabled()); + let is_simple_vote_transaction = sanitized_transaction.is_simple_vote_transaction(); + let signature_count_detail = sanitized_transaction.message().get_signature_details(); + let tx_cost = CostModel::calculate_cost( + &sanitized_transaction, + is_simple_vote_transaction, + &signature_count_detail, + &FeatureSet::all_enabled(), + ); let cost = tx_cost.sum(); let deserialized_packet = DeserializedPacket::new(Packet::from_data(None, transaction).unwrap()).unwrap(); diff --git a/core/src/banking_stage/qos_service.rs b/core/src/banking_stage/qos_service.rs index 21e19be6f0ec52..45db3af95b29e5 100644 --- a/core/src/banking_stage/qos_service.rs +++ b/core/src/banking_stage/qos_service.rs @@ -75,7 +75,18 @@ impl QosService { let mut compute_cost_time = Measure::start("compute_cost_time"); let txs_costs: Vec<_> = transactions .zip(pre_results) - .map(|(tx, pre_result)| pre_result.map(|()| CostModel::calculate_cost(tx, feature_set))) + .map(|(tx, pre_result)| { + pre_result.map(|()| { + let is_simple_vote_tx = tx.is_simple_vote_transaction(); + let signature_count_details = tx.message().get_signature_details(); + CostModel::calculate_cost( + tx, + is_simple_vote_tx, + &signature_count_details, + feature_set, + ) + }) + }) .collect(); compute_cost_time.stop(); self.metrics @@ -654,9 +665,17 @@ mod tests { .iter() .enumerate() .map(|(index, cost)| { + let is_simple_vote_transaction = txs[index].is_simple_vote_transaction(); + let signature_count_details = txs[index].message().get_signature_details(); assert_eq!( cost.as_ref().unwrap().sum(), - CostModel::calculate_cost(&txs[index], &FeatureSet::all_enabled()).sum() + CostModel::calculate_cost( + &txs[index], + is_simple_vote_transaction, + &signature_count_details, + &FeatureSet::all_enabled() + ) + .sum() ); }) .collect_vec(); @@ -682,9 +701,30 @@ mod tests { None, ), ); - let transfer_tx_cost = - CostModel::calculate_cost(&transfer_tx, &FeatureSet::all_enabled()).sum(); - let vote_tx_cost = CostModel::calculate_cost(&vote_tx, &FeatureSet::all_enabled()).sum(); + + let transfer_tx_cost = { + let is_simple_vote_transaction = transfer_tx.is_simple_vote_transaction(); + let signature_count_detail = transfer_tx.message().get_signature_details(); + CostModel::calculate_cost( + &transfer_tx, + is_simple_vote_transaction, + &signature_count_detail, + &FeatureSet::all_enabled(), + ) + .sum() + }; + let vote_tx_cost = { + let is_simple_vote_transaction = vote_tx.is_simple_vote_transaction(); + let signature_count_detail = vote_tx.message().get_signature_details(); + + CostModel::calculate_cost( + &vote_tx, + is_simple_vote_transaction, + &signature_count_detail, + &FeatureSet::all_enabled(), + ) + .sum() + }; // make a vec of txs let txs = vec![transfer_tx.clone(), vote_tx.clone(), transfer_tx, vote_tx]; diff --git a/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs b/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs index 9966a0527d0286..a120419e3f6893 100644 --- a/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs +++ b/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs @@ -637,7 +637,16 @@ impl SchedulerController { fee_budget_limits: &FeeBudgetLimits, bank: &Bank, ) -> (u64, u64) { - let cost = CostModel::calculate_cost(transaction, &bank.feature_set).sum(); + let is_simple_vote_tx = transaction.is_simple_vote_transaction(); + let signature_count_detail = transaction.message().get_signature_details(); + + let cost = CostModel::calculate_cost( + transaction, + is_simple_vote_tx, + &signature_count_detail, + &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. diff --git a/cost-model/benches/cost_model.rs b/cost-model/benches/cost_model.rs index c92ddd7f9b0b1a..7239fccf8d0d4c 100644 --- a/cost-model/benches/cost_model.rs +++ b/cost-model/benches/cost_model.rs @@ -53,7 +53,15 @@ fn bench_cost_model(bencher: &mut Bencher) { bencher.iter(|| { for transaction in &transactions { - let _ = CostModel::calculate_cost(test::black_box(transaction), &feature_set); + let transaction = test::black_box(transaction); + let is_simple_vote_transaction = transaction.is_simple_vote_transaction(); + let signature_count_detail = transaction.message().get_signature_details(); + let _ = CostModel::calculate_cost( + transaction, + is_simple_vote_transaction, + &signature_count_detail, + &feature_set, + ); } }); } @@ -71,7 +79,15 @@ fn bench_cost_model_requested_write_locks(bencher: &mut Bencher) { bencher.iter(|| { for transaction in &transactions { - let _ = CostModel::calculate_cost(test::black_box(transaction), &feature_set); + let transaction = test::black_box(transaction); + let is_simple_vote_transaction = transaction.is_simple_vote_transaction(); + let signature_count_detail = transaction.message().get_signature_details(); + let _ = CostModel::calculate_cost( + transaction, + is_simple_vote_transaction, + &signature_count_detail, + &feature_set, + ); } }); } diff --git a/cost-model/src/cost_model.rs b/cost-model/src/cost_model.rs index cbc2350616fde2..c5b71b028c5cea 100644 --- a/cost-model/src/cost_model.rs +++ b/cost-model/src/cost_model.rs @@ -27,7 +27,6 @@ use { MAX_PERMITTED_DATA_LENGTH, }, system_program, - transaction::SanitizedTransaction, }, solana_svm_transaction::{instruction::SVMInstruction, svm_message::SVMMessage}, }; @@ -43,21 +42,19 @@ enum SystemProgramAccountAllocation { impl CostModel { pub fn calculate_cost( - transaction: &SanitizedTransaction, + transaction: &impl SVMMessage, + is_simple_vote_transaction: bool, + signatures_count_detail: &TransactionSignatureDetails, feature_set: &FeatureSet, ) -> TransactionCost { - if transaction.is_simple_vote_transaction() { + if is_simple_vote_transaction { TransactionCost::SimpleVote { writable_accounts: Self::get_writable_accounts(transaction), } } else { let mut tx_cost = UsageCostDetails::new_with_default_capacity(); - Self::get_signature_cost( - &mut tx_cost, - &transaction.message().get_signature_details(), - feature_set, - ); + Self::get_signature_cost(&mut tx_cost, signatures_count_detail, feature_set); Self::get_write_lock_cost(&mut tx_cost, transaction, feature_set); Self::get_transaction_cost(&mut tx_cost, transaction, feature_set); tx_cost.allocated_accounts_data_size = @@ -71,23 +68,21 @@ impl CostModel { // Calculate executed transaction CU cost, with actual execution and loaded accounts size // costs. pub fn calculate_cost_for_executed_transaction( - transaction: &SanitizedTransaction, + transaction: &impl SVMMessage, + is_simple_vote_transaction: bool, + signatures_count_detail: &TransactionSignatureDetails, actual_programs_execution_cost: u64, actual_loaded_accounts_data_size_bytes: u32, feature_set: &FeatureSet, ) -> TransactionCost { - if transaction.is_simple_vote_transaction() { + if is_simple_vote_transaction { TransactionCost::SimpleVote { writable_accounts: Self::get_writable_accounts(transaction), } } else { let mut tx_cost = UsageCostDetails::new_with_default_capacity(); - Self::get_signature_cost( - &mut tx_cost, - &transaction.message().get_signature_details(), - feature_set, - ); + Self::get_signature_cost(&mut tx_cost, signatures_count_detail, feature_set); Self::get_write_lock_cost(&mut tx_cost, transaction, feature_set); Self::get_instructions_data_cost(&mut tx_cost, transaction); tx_cost.allocated_accounts_data_size = @@ -329,7 +324,7 @@ mod tests { signature::{Keypair, Signer}, system_instruction::{self}, system_program, system_transaction, - transaction::Transaction, + transaction::{SanitizedTransaction, Transaction}, }, }; @@ -577,18 +572,29 @@ mod tests { let simple_transaction = SanitizedTransaction::from_transaction_for_tests( system_transaction::transfer(&mint_keypair, &system_program::id(), 2, start_hash), ); + let is_simple_vote_transaction = simple_transaction.is_simple_vote_transaction(); + let signatures_count_detail = simple_transaction.message().get_signature_details(); // Feature not enabled - write lock is demoted and does not count towards cost { - let tx_cost = CostModel::calculate_cost(&simple_transaction, &FeatureSet::default()); + let tx_cost = CostModel::calculate_cost( + &simple_transaction, + is_simple_vote_transaction, + &signatures_count_detail, + &FeatureSet::default(), + ); assert_eq!(WRITE_LOCK_UNITS, tx_cost.write_lock_cost()); assert_eq!(1, tx_cost.writable_accounts().len()); } // Feature enabled - write lock is demoted but still counts towards cost { - let tx_cost = - CostModel::calculate_cost(&simple_transaction, &FeatureSet::all_enabled()); + let tx_cost = CostModel::calculate_cost( + &simple_transaction, + is_simple_vote_transaction, + &signatures_count_detail, + &FeatureSet::all_enabled(), + ); assert_eq!(2 * WRITE_LOCK_UNITS, tx_cost.write_lock_cost()); assert_eq!(1, tx_cost.writable_accounts().len()); } @@ -755,8 +761,15 @@ mod tests { instructions, ), ); + let is_simple_vote_transaction = tx.is_simple_vote_transaction(); + let signatures_count_detail = tx.message().get_signature_details(); - let tx_cost = CostModel::calculate_cost(&tx, &FeatureSet::all_enabled()); + let tx_cost = CostModel::calculate_cost( + &tx, + is_simple_vote_transaction, + &signatures_count_detail, + &FeatureSet::all_enabled(), + ); assert_eq!(2 + 2, tx_cost.writable_accounts().len()); assert_eq!(signer1.pubkey(), tx_cost.writable_accounts()[0]); assert_eq!(signer2.pubkey(), tx_cost.writable_accounts()[1]); @@ -773,6 +786,8 @@ mod tests { 2, start_hash, )); + let is_simple_vote_transaction = tx.is_simple_vote_transaction(); + let signatures_count_detail = tx.message().get_signature_details(); let expected_account_cost = WRITE_LOCK_UNITS * 2; let expected_execution_cost = BUILTIN_INSTRUCTION_COSTS @@ -785,7 +800,12 @@ mod tests { / ACCOUNT_DATA_COST_PAGE_SIZE * DEFAULT_PAGE_COST; - let tx_cost = CostModel::calculate_cost(&tx, &FeatureSet::all_enabled()); + let tx_cost = CostModel::calculate_cost( + &tx, + is_simple_vote_transaction, + &signatures_count_detail, + &FeatureSet::all_enabled(), + ); 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().len()); @@ -810,6 +830,8 @@ mod tests { &[&mint_keypair], start_hash, )); + let is_simple_vote_transaction = tx.is_simple_vote_transaction(); + let signatures_count_detail = tx.message().get_signature_details(); let feature_set = FeatureSet::all_enabled(); let expected_account_cost = WRITE_LOCK_UNITS * 2; @@ -821,7 +843,12 @@ mod tests { .unwrap(); let expected_loaded_accounts_data_size_cost = (data_limit as u64) / (32 * 1024) * 8; - let tx_cost = CostModel::calculate_cost(&tx, &feature_set); + let tx_cost = CostModel::calculate_cost( + &tx, + is_simple_vote_transaction, + &signatures_count_detail, + &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().len()); @@ -874,6 +901,7 @@ mod tests { &[&mint_keypair], start_hash, )); + // transaction has one builtin instruction, and one ComputeBudget::compute_unit_limit let expected_cost = *BUILTIN_INSTRUCTION_COSTS .get(&solana_system_program::id()) diff --git a/cost-model/src/transaction_cost.rs b/cost-model/src/transaction_cost.rs index 3065162c5ee22b..327e8495bea294 100644 --- a/cost-model/src/transaction_cost.rs +++ b/cost-model/src/transaction_cost.rs @@ -236,7 +236,7 @@ mod tests { .unwrap(); // create a identical sanitized transaction, but identified as non-vote - let none_vote_transaction = SanitizedTransaction::try_create( + let non_vote_transaction = SanitizedTransaction::try_create( VersionedTransaction::from(transaction), MessageHash::Compute, Some(false), @@ -250,11 +250,20 @@ mod tests { // expected non-vote tx cost would include default loaded accounts size cost (16384) additionally let expected_none_vote_cost = 20543; - let vote_cost = CostModel::calculate_cost(&vote_transaction, &FeatureSet::all_enabled()); - let none_vote_cost = - CostModel::calculate_cost(&none_vote_transaction, &FeatureSet::all_enabled()); + let vote_cost = CostModel::calculate_cost( + &vote_transaction, + vote_transaction.is_simple_vote_transaction(), + &vote_transaction.message().get_signature_details(), + &FeatureSet::all_enabled(), + ); + let non_vote_cost = CostModel::calculate_cost( + &non_vote_transaction, + non_vote_transaction.is_simple_vote_transaction(), + &non_vote_transaction.message().get_signature_details(), + &FeatureSet::all_enabled(), + ); assert_eq!(expected_vote_cost, vote_cost.sum()); - assert_eq!(expected_none_vote_cost, none_vote_cost.sum()); + assert_eq!(expected_none_vote_cost, non_vote_cost.sum()); } } diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index 682f2bf8a1aa0f..cc7541a96170a7 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -487,7 +487,14 @@ fn compute_slot_cost( .for_each(|transaction| { num_programs += transaction.message().instructions().len(); - let tx_cost = CostModel::calculate_cost(&transaction, &feature_set); + let is_simple_vote_transaction = transaction.is_simple_vote_transaction(); + let signature_count_detail = transaction.message().get_signature_details(); + let tx_cost = CostModel::calculate_cost( + &transaction, + is_simple_vote_transaction, + &signature_count_detail, + &feature_set, + ); let result = cost_tracker.try_add(&tx_cost); if result.is_err() { println!( diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 986e1a2549b13b..31b720892a2217 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -234,8 +234,12 @@ fn check_block_cost_limits( .zip(sanitized_transactions) .filter_map(|(commit_result, tx)| { if let Ok(committed_tx) = commit_result { + let is_simple_vote_transaction = tx.is_simple_vote_transaction(); + let signature_count_detail = tx.message().get_signature_details(); Some(CostModel::calculate_cost_for_executed_transaction( tx, + is_simple_vote_transaction, + &signature_count_detail, committed_tx.executed_units, committed_tx.loaded_account_stats.loaded_accounts_data_size, &bank.feature_set, @@ -485,7 +489,14 @@ fn rebatch_and_execute_batches( let tx_costs = sanitized_txs .iter() .map(|tx| { - let tx_cost = CostModel::calculate_cost(tx, &bank.feature_set); + let is_simple_vote_tx = tx.is_simple_vote_transaction(); + let signature_count_details = tx.message().get_signature_details(); + let tx_cost = CostModel::calculate_cost( + tx, + is_simple_vote_tx, + &signature_count_details, + &bank.feature_set, + ); let cost = tx_cost.sum(); minimal_tx_cost = std::cmp::min(minimal_tx_cost, cost); total_cost = total_cost.saturating_add(cost); @@ -5085,7 +5096,14 @@ pub mod tests { 1, genesis_config.hash(), )); - let mut tx_cost = CostModel::calculate_cost(&tx, &bank.feature_set); + let is_simple_vote_tx = tx.is_simple_vote_transaction(); + let signature_count_detail = tx.message().get_signature_details(); + let mut tx_cost = CostModel::calculate_cost( + &tx, + is_simple_vote_tx, + &signature_count_detail, + &bank.feature_set, + ); let actual_execution_cu = 1; let actual_loaded_accounts_data_size = 64 * 1024; let TransactionCost::Transaction(ref mut usage_cost_details) = tx_cost else {