From 8e9c08fc17d6261bd52867a2b5178fac294d8f78 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Wed, 3 Jul 2024 11:21:23 -0500 Subject: [PATCH 1/3] TransactionCost - remove allocation --- core/src/banking_stage/consumer.rs | 1 + .../forward_packet_batches_by_accounts.rs | 9 +- core/src/banking_stage/qos_service.rs | 43 ++- cost-model/benches/cost_tracker.rs | 46 ++-- cost-model/src/cost_model.rs | 71 ++--- cost-model/src/cost_tracker.rs | 249 +++++++++++------- cost-model/src/transaction_cost.rs | 70 +---- ledger-tool/src/main.rs | 3 +- ledger/src/blockstore_processor.rs | 19 +- 9 files changed, 260 insertions(+), 251 deletions(-) diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 9a0108a3851770..fd2a349a7d8c9c 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -521,6 +521,7 @@ impl Consumer { // were not included in the block should have their cost removed, the rest // should update with their actually consumed units. QosService::remove_or_update_costs( + txs.iter(), transaction_qos_cost_results.iter(), commit_transactions_result.as_ref().ok(), bank, 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 e01ca3b213b81e..af7a57ed8a61a1 100644 --- a/core/src/banking_stage/forward_packet_batches_by_accounts.rs +++ b/core/src/banking_stage/forward_packet_batches_by_accounts.rs @@ -110,7 +110,10 @@ impl ForwardPacketBatchesByAccounts { ) -> bool { let tx_cost = CostModel::calculate_cost(sanitized_transaction, feature_set); - if let Ok(updated_costs) = self.cost_tracker.try_add(&tx_cost) { + if let Ok(updated_costs) = self.cost_tracker.try_add( + CostModel::writable_accounts_iter(sanitized_transaction), + &tx_cost, + ) { let batch_index = self.get_batch_index_by_updated_costs(&tx_cost, &updated_costs); if let Some(forward_batch) = self.forward_batches.get_mut(batch_index) { @@ -349,9 +352,7 @@ mod tests { ForwardPacketBatchesByAccounts::new_with_default_batch_limits(); forward_packet_batches_by_accounts.batch_vote_limit = test_cost + 1; - let transaction_cost = TransactionCost::SimpleVote { - writable_accounts: vec![], - }; + let transaction_cost = TransactionCost::SimpleVote; assert_eq!( 0, forward_packet_batches_by_accounts.get_batch_index_by_updated_costs( diff --git a/core/src/banking_stage/qos_service.rs b/core/src/banking_stage/qos_service.rs index bf8b7df963e392..9f52bfe07cd357 100644 --- a/core/src/banking_stage/qos_service.rs +++ b/core/src/banking_stage/qos_service.rs @@ -105,7 +105,7 @@ impl QosService { .map(|(tx, cost)| { match cost { Ok(cost) => { - match cost_tracker.try_add(&cost) { + match cost_tracker.try_add(CostModel::writable_accounts_iter(tx),&cost) { Ok(UpdatedCosts{updated_block_cost, updated_costliest_account_cost}) => { debug!("slot {:?}, transaction {:?}, cost {:?}, fit into current block, current block cost {}, updated costliest account cost {}", bank.slot(), tx, cost, updated_block_cost, updated_costliest_account_cost); self.metrics.stats.selected_txs_count.fetch_add(1, Ordering::Relaxed); @@ -135,6 +135,7 @@ impl QosService { /// Removes transaction costs from the cost tracker if not committed or recorded, or /// updates the transaction costs for committed transactions. pub fn remove_or_update_costs<'a>( + transactions: impl Iterator, transaction_cost_results: impl Iterator>, transaction_committed_status: Option<&Vec>, bank: &Bank, @@ -142,27 +143,35 @@ impl QosService { match transaction_committed_status { Some(transaction_committed_status) => { Self::remove_or_update_recorded_transaction_costs( + transactions, transaction_cost_results, transaction_committed_status, bank, ) } - None => Self::remove_unrecorded_transaction_costs(transaction_cost_results, bank), + None => Self::remove_unrecorded_transaction_costs( + transactions, + transaction_cost_results, + bank, + ), } } /// For recorded transactions, remove units reserved by uncommitted transaction, or update /// units for committed transactions. fn remove_or_update_recorded_transaction_costs<'a>( + transactions: impl Iterator, transaction_cost_results: impl Iterator>, transaction_committed_status: &Vec, bank: &Bank, ) { let mut cost_tracker = bank.write_cost_tracker().unwrap(); let mut num_included = 0; - transaction_cost_results + transactions + .into_iter() + .zip(transaction_cost_results) .zip(transaction_committed_status) - .for_each(|(tx_cost, transaction_committed_details)| { + .for_each(|((tx, tx_cost), transaction_committed_details)| { // Only transactions that the qos service included have to be // checked for update if let Ok(tx_cost) = tx_cost { @@ -173,6 +182,7 @@ impl QosService { loaded_accounts_data_size, } => { cost_tracker.update_execution_cost( + CostModel::writable_accounts_iter(tx), tx_cost, *compute_units, CostModel::calculate_loaded_accounts_data_size_cost( @@ -182,7 +192,7 @@ impl QosService { ); } CommitTransactionDetails::NotCommitted => { - cost_tracker.remove(tx_cost); + cost_tracker.remove(CostModel::writable_accounts_iter(tx), tx_cost); } } } @@ -192,19 +202,22 @@ impl QosService { /// Remove reserved units for transaction batch that unsuccessfully recorded. fn remove_unrecorded_transaction_costs<'a>( + transactions: impl Iterator, transaction_cost_results: impl Iterator>, bank: &Bank, ) { let mut cost_tracker = bank.write_cost_tracker().unwrap(); let mut num_included = 0; - transaction_cost_results.for_each(|tx_cost| { - // Only transactions that the qos service included have to be - // removed - if let Ok(tx_cost) = tx_cost { - num_included += 1; - cost_tracker.remove(tx_cost); - } - }); + transactions + .zip(transaction_cost_results) + .for_each(|(tx, tx_cost)| { + // Only transactions that the qos service included have to be + // removed + if let Ok(tx_cost) = tx_cost { + num_included += 1; + cost_tracker.remove(CostModel::writable_accounts_iter(tx), tx_cost); + } + }); cost_tracker.sub_transactions_in_flight(num_included); } @@ -760,6 +773,7 @@ mod tests { * transaction_count; QosService::remove_or_update_costs( + txs.iter(), qos_cost_results.iter(), Some(&committed_status), &bank, @@ -811,7 +825,7 @@ mod tests { bank.read_cost_tracker().unwrap().block_cost() ); - QosService::remove_or_update_costs(qos_cost_results.iter(), None, &bank); + QosService::remove_or_update_costs(txs.iter(), qos_cost_results.iter(), None, &bank); assert_eq!(0, bank.read_cost_tracker().unwrap().block_cost()); assert_eq!(0, bank.read_cost_tracker().unwrap().transaction_count()); } @@ -884,6 +898,7 @@ mod tests { .collect(); QosService::remove_or_update_costs( + txs.iter(), qos_cost_results.iter(), Some(&committed_status), &bank, diff --git a/cost-model/benches/cost_tracker.rs b/cost-model/benches/cost_tracker.rs index 0c41ecc4f37cf3..0a4a10c3e5a997 100644 --- a/cost-model/benches/cost_tracker.rs +++ b/cost-model/benches/cost_tracker.rs @@ -12,6 +12,7 @@ use { struct BenchSetup { cost_tracker: CostTracker, + writable_accounts: Vec>, tx_costs: Vec, } @@ -22,19 +23,21 @@ fn setup(num_transactions: usize, contentious_transactions: bool) -> BenchSetup let max_accounts_per_tx = 128; let pubkey = Pubkey::new_unique(); + let mut writable_accounts = Vec::with_capacity(num_transactions); let tx_costs = (0..num_transactions) .map(|_| { let mut usage_cost_details = UsageCostDetails::default(); - (0..max_accounts_per_tx).for_each(|_| { - let writable_account_key = if contentious_transactions { - pubkey - } else { - Pubkey::new_unique() - }; - usage_cost_details - .writable_accounts - .push(writable_account_key) - }); + writable_accounts.push( + (0..max_accounts_per_tx) + .map(|_| { + if contentious_transactions { + pubkey + } else { + Pubkey::new_unique() + } + }) + .collect(), + ); usage_cost_details.programs_execution_cost = 9999; TransactionCost::Transaction(usage_cost_details) }) @@ -42,6 +45,7 @@ fn setup(num_transactions: usize, contentious_transactions: bool) -> BenchSetup BenchSetup { cost_tracker, + writable_accounts, tx_costs, } } @@ -50,15 +54,20 @@ fn setup(num_transactions: usize, contentious_transactions: bool) -> BenchSetup fn bench_cost_tracker_non_contentious_transaction(bencher: &mut Bencher) { let BenchSetup { mut cost_tracker, + writable_accounts, tx_costs, } = setup(1024, false); bencher.iter(|| { - for tx_cost in tx_costs.iter() { - if cost_tracker.try_add(tx_cost).is_err() { + for (writable_accounts, tx_cost) in writable_accounts.iter().zip(tx_costs.iter()) { + if cost_tracker + .try_add(writable_accounts.iter(), tx_cost) + .is_err() + { break; } // stop when hit limits - cost_tracker.update_execution_cost(tx_cost, 0, 0); // update execution cost down to zero + cost_tracker.update_execution_cost(writable_accounts.iter(), tx_cost, 0, 0); + // update execution cost down to zero } }); } @@ -67,15 +76,20 @@ fn bench_cost_tracker_non_contentious_transaction(bencher: &mut Bencher) { fn bench_cost_tracker_contentious_transaction(bencher: &mut Bencher) { let BenchSetup { mut cost_tracker, + writable_accounts, tx_costs, } = setup(1024, true); bencher.iter(|| { - for tx_cost in tx_costs.iter() { - if cost_tracker.try_add(tx_cost).is_err() { + for (writable_accounts, tx_cost) in writable_accounts.iter().zip(tx_costs.iter()) { + if cost_tracker + .try_add(writable_accounts.iter(), tx_cost) + .is_err() + { break; } // stop when hit limits - cost_tracker.update_execution_cost(tx_cost, 0, 0); // update execution cost down to zero + cost_tracker.update_execution_cost(writable_accounts.iter(), tx_cost, 0, 0); + // update execution cost down to zero } }); } diff --git a/cost-model/src/cost_model.rs b/cost-model/src/cost_model.rs index a230e8149874d5..4db9ee73b83de9 100644 --- a/cost-model/src/cost_model.rs +++ b/cost-model/src/cost_model.rs @@ -34,11 +34,9 @@ impl CostModel { feature_set: &FeatureSet, ) -> TransactionCost { if transaction.is_simple_vote_transaction() { - TransactionCost::SimpleVote { - writable_accounts: Self::get_writable_accounts(transaction), - } + TransactionCost::SimpleVote } else { - let mut tx_cost = UsageCostDetails::new_with_default_capacity(); + let mut tx_cost = UsageCostDetails::default(); Self::get_signature_cost(&mut tx_cost, transaction, feature_set); Self::get_write_lock_cost(&mut tx_cost, transaction, feature_set); @@ -60,11 +58,9 @@ impl CostModel { feature_set: &FeatureSet, ) -> TransactionCost { if transaction.is_simple_vote_transaction() { - TransactionCost::SimpleVote { - writable_accounts: Self::get_writable_accounts(transaction), - } + TransactionCost::SimpleVote } else { - let mut tx_cost = UsageCostDetails::new_with_default_capacity(); + let mut tx_cost = UsageCostDetails::default(); Self::get_signature_cost(&mut tx_cost, transaction, feature_set); Self::get_write_lock_cost(&mut tx_cost, transaction, feature_set); @@ -116,20 +112,23 @@ impl CostModel { ); } - fn get_writable_accounts(transaction: &SanitizedTransaction) -> Vec { + pub fn writable_accounts_iter( + transaction: &SanitizedTransaction, + ) -> impl Iterator + Clone { let message = transaction.message(); message .account_keys() .iter() .enumerate() - .filter_map(|(i, k)| { - if message.is_writable(i) { - Some(*k) - } else { - None - } - }) - .collect() + .filter(|(i, _k)| message.is_writable(*i)) + .map(|(_i, k)| k) + } + + fn get_num_actual_writable_accounts(transaction: &SanitizedTransaction) -> u64 { + let message = transaction.message(); + (0..message.account_keys().len()) + .filter(|i| message.is_writable(*i)) + .count() as u64 } fn get_write_lock_cost( @@ -137,12 +136,11 @@ impl CostModel { transaction: &SanitizedTransaction, feature_set: &FeatureSet, ) { - tx_cost.writable_accounts = Self::get_writable_accounts(transaction); let num_write_locks = if feature_set.is_active(&feature_set::cost_model_requested_write_lock_cost::id()) { transaction.message().num_write_locks() } else { - tx_cost.writable_accounts.len() as u64 + Self::get_num_actual_writable_accounts(transaction) }; tx_cost.write_lock_cost = WRITE_LOCK_UNITS.saturating_mul(num_write_locks); } @@ -429,7 +427,6 @@ mod tests { { let tx_cost = CostModel::calculate_cost(&simple_transaction, &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 @@ -437,7 +434,6 @@ mod tests { let tx_cost = CostModel::calculate_cost(&simple_transaction, &FeatureSet::all_enabled()); assert_eq!(2 * WRITE_LOCK_UNITS, tx_cost.write_lock_cost()); - assert_eq!(1, tx_cost.writable_accounts().len()); } } @@ -580,37 +576,6 @@ mod tests { assert_eq!(0, tx_cost.data_bytes_cost); } - #[test] - fn test_cost_model_sort_message_accounts_by_type() { - // construct a transaction with two random instructions with same signer - let signer1 = Keypair::new(); - let signer2 = Keypair::new(); - let key1 = Pubkey::new_unique(); - let key2 = Pubkey::new_unique(); - let prog1 = Pubkey::new_unique(); - let prog2 = Pubkey::new_unique(); - let instructions = vec![ - CompiledInstruction::new(4, &(), vec![0, 2]), - CompiledInstruction::new(5, &(), vec![1, 3]), - ]; - let tx = SanitizedTransaction::from_transaction_for_tests( - Transaction::new_with_compiled_instructions( - &[&signer1, &signer2], - &[key1, key2], - Hash::new_unique(), - vec![prog1, prog2], - instructions, - ), - ); - - let tx_cost = CostModel::calculate_cost(&tx, &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]); - assert_eq!(key1, tx_cost.writable_accounts()[2]); - assert_eq!(key2, tx_cost.writable_accounts()[3]); - } - #[test] fn test_cost_model_calculate_cost_all_default() { let (mint_keypair, start_hash) = test_setup(); @@ -635,7 +600,6 @@ 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.programs_execution_cost()); - assert_eq!(2, tx_cost.writable_accounts().len()); assert_eq!( expected_loaded_accounts_data_size_cost, tx_cost.loaded_accounts_data_size_cost() @@ -671,7 +635,6 @@ 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.programs_execution_cost()); - assert_eq!(2, tx_cost.writable_accounts().len()); assert_eq!( expected_loaded_accounts_data_size_cost, tx_cost.loaded_accounts_data_size_cost() diff --git a/cost-model/src/cost_tracker.rs b/cost-model/src/cost_tracker.rs index 0c731f946ec3b8..f6722cbfe06805 100644 --- a/cost-model/src/cost_tracker.rs +++ b/cost-model/src/cost_tracker.rs @@ -147,17 +147,22 @@ impl CostTracker { .saturating_sub(in_flight_transaction_count); } - pub fn try_add(&mut self, tx_cost: &TransactionCost) -> Result { - self.would_fit(tx_cost)?; - let updated_costliest_account_cost = self.add_transaction_cost(tx_cost); + pub fn try_add<'a>( + &mut self, + writable_accounts: impl Iterator + Clone, + tx_cost: &TransactionCost, + ) -> Result { + self.would_fit(writable_accounts.clone(), tx_cost)?; + let updated_costliest_account_cost = self.add_transaction_cost(writable_accounts, tx_cost); Ok(UpdatedCosts { updated_block_cost: self.block_cost, updated_costliest_account_cost, }) } - pub fn update_execution_cost( + pub fn update_execution_cost<'a>( &mut self, + writable_accounts: impl Iterator, estimated_tx_cost: &TransactionCost, actual_execution_units: u64, actual_loaded_accounts_data_size_cost: u64, @@ -171,12 +176,14 @@ impl CostTracker { Ordering::Equal => (), Ordering::Greater => { self.add_transaction_execution_cost( + writable_accounts, estimated_tx_cost, actual_load_and_execution_units - estimated_load_and_execution_units, ); } Ordering::Less => { self.sub_transaction_execution_cost( + writable_accounts, estimated_tx_cost, estimated_load_and_execution_units - actual_load_and_execution_units, ); @@ -184,8 +191,12 @@ impl CostTracker { } } - pub fn remove(&mut self, tx_cost: &TransactionCost) { - self.remove_transaction_cost(tx_cost); + pub fn remove<'a>( + &mut self, + writable_accounts: impl Iterator, + tx_cost: &TransactionCost, + ) { + self.remove_transaction_cost(writable_accounts, tx_cost); } pub fn block_cost(&self) -> u64 { @@ -249,7 +260,11 @@ impl CostTracker { .unwrap_or_default() } - fn would_fit(&self, tx_cost: &TransactionCost) -> Result<(), CostTrackerError> { + fn would_fit<'a>( + &self, + writable_accounts: impl Iterator, + tx_cost: &TransactionCost, + ) -> Result<(), CostTrackerError> { let cost: u64 = tx_cost.sum(); if tx_cost.is_simple_vote() { @@ -278,7 +293,7 @@ impl CostTracker { } // check each account against account_cost_limit, - for account_key in tx_cost.writable_accounts().iter() { + for account_key in writable_accounts { match self.cost_by_writable_accounts.get(account_key) { Some(chained_cost) => { if chained_cost.saturating_add(cost) > self.account_cost_limit { @@ -295,7 +310,11 @@ impl CostTracker { } // Returns the highest account cost for all write-lock accounts `TransactionCost` updated - fn add_transaction_cost(&mut self, tx_cost: &TransactionCost) -> u64 { + fn add_transaction_cost<'a>( + &mut self, + writable_accounts: impl Iterator, + tx_cost: &TransactionCost, + ) -> u64 { saturating_add_assign!( self.allocated_accounts_data_size, tx_cost.allocated_accounts_data_size() @@ -313,12 +332,16 @@ impl CostTracker { self.ed25519_instruction_signature_count, tx_cost.num_ed25519_instruction_signatures() ); - self.add_transaction_execution_cost(tx_cost, tx_cost.sum()) + self.add_transaction_execution_cost(writable_accounts, tx_cost, tx_cost.sum()) } - fn remove_transaction_cost(&mut self, tx_cost: &TransactionCost) { + fn remove_transaction_cost<'a>( + &mut self, + writable_accounts: impl Iterator, + tx_cost: &TransactionCost, + ) { let cost = tx_cost.sum(); - self.sub_transaction_execution_cost(tx_cost, cost); + self.sub_transaction_execution_cost(writable_accounts, tx_cost, cost); self.allocated_accounts_data_size = self .allocated_accounts_data_size .saturating_sub(tx_cost.allocated_accounts_data_size()); @@ -336,13 +359,14 @@ impl CostTracker { /// Apply additional actual execution units to cost_tracker /// Return the costliest account cost that were updated by `TransactionCost` - fn add_transaction_execution_cost( + fn add_transaction_execution_cost<'a>( &mut self, + writable_accounts: impl Iterator, tx_cost: &TransactionCost, adjustment: u64, ) -> u64 { let mut costliest_account_cost = 0; - for account_key in tx_cost.writable_accounts().iter() { + for account_key in writable_accounts { let account_cost = self .cost_by_writable_accounts .entry(*account_key) @@ -359,8 +383,13 @@ impl CostTracker { } /// Subtract extra execution units from cost_tracker - fn sub_transaction_execution_cost(&mut self, tx_cost: &TransactionCost, adjustment: u64) { - for account_key in tx_cost.writable_accounts().iter() { + fn sub_transaction_execution_cost<'a>( + &mut self, + writable_accounts: impl Iterator, + tx_cost: &TransactionCost, + adjustment: u64, + ) { + for account_key in writable_accounts { let account_cost = self .cost_by_writable_accounts .entry(*account_key) @@ -386,7 +415,7 @@ impl CostTracker { mod tests { use { super::*, - crate::transaction_cost::*, + crate::{cost_model::CostModel, transaction_cost::*}, solana_sdk::{ hash::Hash, reserved_account_keys::ReservedAccountKeys, @@ -426,9 +455,10 @@ mod tests { let simple_transaction = SanitizedTransaction::from_transaction_for_tests( system_transaction::transfer(mint_keypair, &keypair.pubkey(), 2, *start_hash), ); - let mut tx_cost = UsageCostDetails::new_with_capacity(1); - tx_cost.programs_execution_cost = 5; - tx_cost.writable_accounts.push(mint_keypair.pubkey()); + let tx_cost = UsageCostDetails { + programs_execution_cost: 5, + ..UsageCostDetails::default() + }; (simple_transaction, TransactionCost::Transaction(tx_cost)) } @@ -456,11 +486,7 @@ mod tests { ) .unwrap(); - let writable_accounts = vec![mint_keypair.pubkey()]; - ( - vote_transaction, - TransactionCost::SimpleVote { writable_accounts }, - ) + (vote_transaction, TransactionCost::SimpleVote) } #[test] @@ -476,13 +502,15 @@ mod tests { #[test] fn test_cost_tracker_ok_add_one() { let (mint_keypair, start_hash) = test_setup(); - let (_tx, tx_cost) = build_simple_transaction(&mint_keypair, &start_hash); + let (tx, tx_cost) = build_simple_transaction(&mint_keypair, &start_hash); let cost = tx_cost.sum(); // build testee to have capacity for one simple transaction let mut testee = CostTracker::new(cost, cost, cost); - assert!(testee.would_fit(&tx_cost).is_ok()); - testee.add_transaction_cost(&tx_cost); + assert!(testee + .would_fit(CostModel::writable_accounts_iter(&tx), &tx_cost) + .is_ok()); + testee.add_transaction_cost(CostModel::writable_accounts_iter(&tx), &tx_cost); assert_eq!(cost, testee.block_cost); assert_eq!(0, testee.vote_cost); let (_costliest_account, costliest_account_cost) = testee.find_costliest_account(); @@ -492,13 +520,15 @@ mod tests { #[test] fn test_cost_tracker_ok_add_one_vote() { let (mint_keypair, start_hash) = test_setup(); - let (_tx, tx_cost) = build_simple_vote_transaction(&mint_keypair, &start_hash); + let (tx, tx_cost) = build_simple_vote_transaction(&mint_keypair, &start_hash); let cost = tx_cost.sum(); // build testee to have capacity for one simple transaction let mut testee = CostTracker::new(cost, cost, cost); - assert!(testee.would_fit(&tx_cost).is_ok()); - testee.add_transaction_cost(&tx_cost); + assert!(testee + .would_fit(CostModel::writable_accounts_iter(&tx), &tx_cost) + .is_ok()); + testee.add_transaction_cost(CostModel::writable_accounts_iter(&tx), &tx_cost); assert_eq!(cost, testee.block_cost); assert_eq!(cost, testee.vote_cost); let (_costliest_account, costliest_account_cost) = testee.find_costliest_account(); @@ -508,7 +538,7 @@ mod tests { #[test] fn test_cost_tracker_add_data() { let (mint_keypair, start_hash) = test_setup(); - let (_tx, mut tx_cost) = build_simple_transaction(&mint_keypair, &start_hash); + let (tx, mut tx_cost) = build_simple_transaction(&mint_keypair, &start_hash); if let TransactionCost::Transaction(ref mut usage_cost) = tx_cost { usage_cost.allocated_accounts_data_size = 1; } else { @@ -518,9 +548,11 @@ mod tests { // build testee to have capacity for one simple transaction let mut testee = CostTracker::new(cost, cost, cost); - assert!(testee.would_fit(&tx_cost).is_ok()); + assert!(testee + .would_fit(CostModel::writable_accounts_iter(&tx), &tx_cost) + .is_ok()); let old = testee.allocated_accounts_data_size; - testee.add_transaction_cost(&tx_cost); + testee.add_transaction_cost(CostModel::writable_accounts_iter(&tx), &tx_cost); assert_eq!(old + 1, testee.allocated_accounts_data_size); } @@ -528,20 +560,24 @@ mod tests { fn test_cost_tracker_ok_add_two_same_accounts() { let (mint_keypair, start_hash) = test_setup(); // build two transactions with same signed account - let (_tx1, tx_cost1) = build_simple_transaction(&mint_keypair, &start_hash); + let (tx1, tx_cost1) = build_simple_transaction(&mint_keypair, &start_hash); let cost1 = tx_cost1.sum(); - let (_tx2, tx_cost2) = build_simple_transaction(&mint_keypair, &start_hash); + let (tx2, tx_cost2) = build_simple_transaction(&mint_keypair, &start_hash); let cost2 = tx_cost2.sum(); // build testee to have capacity for two simple transactions, with same accounts let mut testee = CostTracker::new(cost1 + cost2, cost1 + cost2, cost1 + cost2); { - assert!(testee.would_fit(&tx_cost1).is_ok()); - testee.add_transaction_cost(&tx_cost1); + assert!(testee + .would_fit(CostModel::writable_accounts_iter(&tx1), &tx_cost1) + .is_ok()); + testee.add_transaction_cost(CostModel::writable_accounts_iter(&tx1), &tx_cost1); } { - assert!(testee.would_fit(&tx_cost2).is_ok()); - testee.add_transaction_cost(&tx_cost2); + assert!(testee + .would_fit(CostModel::writable_accounts_iter(&tx2), &tx_cost2) + .is_ok()); + testee.add_transaction_cost(CostModel::writable_accounts_iter(&tx2), &tx_cost2); } assert_eq!(cost1 + cost2, testee.block_cost); assert_eq!(1, testee.cost_by_writable_accounts.len()); @@ -554,20 +590,24 @@ mod tests { let (mint_keypair, start_hash) = test_setup(); // build two transactions with diff accounts let second_account = Keypair::new(); - let (_tx1, tx_cost1) = build_simple_transaction(&mint_keypair, &start_hash); + let (tx1, tx_cost1) = build_simple_transaction(&mint_keypair, &start_hash); let cost1 = tx_cost1.sum(); - let (_tx2, tx_cost2) = build_simple_transaction(&second_account, &start_hash); + let (tx2, tx_cost2) = build_simple_transaction(&second_account, &start_hash); let cost2 = tx_cost2.sum(); // build testee to have capacity for two simple transactions, with same accounts let mut testee = CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2, cost1 + cost2); { - assert!(testee.would_fit(&tx_cost1).is_ok()); - testee.add_transaction_cost(&tx_cost1); + assert!(testee + .would_fit(CostModel::writable_accounts_iter(&tx1), &tx_cost1) + .is_ok()); + testee.add_transaction_cost(CostModel::writable_accounts_iter(&tx1), &tx_cost1); } { - assert!(testee.would_fit(&tx_cost2).is_ok()); - testee.add_transaction_cost(&tx_cost2); + assert!(testee + .would_fit(CostModel::writable_accounts_iter(&tx2), &tx_cost2) + .is_ok()); + testee.add_transaction_cost(CostModel::writable_accounts_iter(&tx2), &tx_cost2); } assert_eq!(cost1 + cost2, testee.block_cost); assert_eq!(2, testee.cost_by_writable_accounts.len()); @@ -579,21 +619,25 @@ mod tests { fn test_cost_tracker_chain_reach_limit() { let (mint_keypair, start_hash) = test_setup(); // build two transactions with same signed account - let (_tx1, tx_cost1) = build_simple_transaction(&mint_keypair, &start_hash); + let (tx1, tx_cost1) = build_simple_transaction(&mint_keypair, &start_hash); let cost1 = tx_cost1.sum(); - let (_tx2, tx_cost2) = build_simple_transaction(&mint_keypair, &start_hash); + let (tx2, tx_cost2) = build_simple_transaction(&mint_keypair, &start_hash); let cost2 = tx_cost2.sum(); // build testee to have capacity for two simple transactions, but not for same accounts let mut testee = CostTracker::new(cmp::min(cost1, cost2), cost1 + cost2, cost1 + cost2); // should have room for first transaction { - assert!(testee.would_fit(&tx_cost1).is_ok()); - testee.add_transaction_cost(&tx_cost1); + assert!(testee + .would_fit(CostModel::writable_accounts_iter(&tx1), &tx_cost1) + .is_ok()); + testee.add_transaction_cost(CostModel::writable_accounts_iter(&tx1), &tx_cost1); } // but no more sapce on the same chain (same signer account) { - assert!(testee.would_fit(&tx_cost2).is_err()); + assert!(testee + .would_fit(CostModel::writable_accounts_iter(&tx2), &tx_cost2) + .is_err()); } } @@ -602,9 +646,9 @@ mod tests { let (mint_keypair, start_hash) = test_setup(); // build two transactions with diff accounts let second_account = Keypair::new(); - let (_tx1, tx_cost1) = build_simple_transaction(&mint_keypair, &start_hash); + let (tx1, tx_cost1) = build_simple_transaction(&mint_keypair, &start_hash); let cost1 = tx_cost1.sum(); - let (_tx2, tx_cost2) = build_simple_transaction(&second_account, &start_hash); + let (tx2, tx_cost2) = build_simple_transaction(&second_account, &start_hash); let cost2 = tx_cost2.sum(); // build testee to have capacity for each chain, but not enough room for both transactions @@ -612,12 +656,16 @@ mod tests { CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2 - 1, cost1 + cost2 - 1); // should have room for first transaction { - assert!(testee.would_fit(&tx_cost1).is_ok()); - testee.add_transaction_cost(&tx_cost1); + assert!(testee + .would_fit(CostModel::writable_accounts_iter(&tx1), &tx_cost1) + .is_ok()); + testee.add_transaction_cost(CostModel::writable_accounts_iter(&tx1), &tx_cost1); } // but no more room for package as whole { - assert!(testee.would_fit(&tx_cost2).is_err()); + assert!(testee + .would_fit(CostModel::writable_accounts_iter(&tx2), &tx_cost2) + .is_err()); } } @@ -626,8 +674,8 @@ mod tests { let (mint_keypair, start_hash) = test_setup(); // build two mocking vote transactions with diff accounts let second_account = Keypair::new(); - let (_tx1, tx_cost1) = build_simple_vote_transaction(&mint_keypair, &start_hash); - let (_tx2, tx_cost2) = build_simple_vote_transaction(&second_account, &start_hash); + let (tx1, tx_cost1) = build_simple_vote_transaction(&mint_keypair, &start_hash); + let (tx2, tx_cost2) = build_simple_vote_transaction(&second_account, &start_hash); let cost1 = tx_cost1.sum(); let cost2 = tx_cost2.sum(); @@ -635,18 +683,24 @@ mod tests { let mut testee = CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2, cost1 + cost2 - 1); // should have room for first vote { - assert!(testee.would_fit(&tx_cost1).is_ok()); - testee.add_transaction_cost(&tx_cost1); + assert!(testee + .would_fit(CostModel::writable_accounts_iter(&tx1), &tx_cost1) + .is_ok()); + testee.add_transaction_cost(CostModel::writable_accounts_iter(&tx1), &tx_cost1); } // but no more room for package as whole { - assert!(testee.would_fit(&tx_cost2).is_err()); + assert!(testee + .would_fit(CostModel::writable_accounts_iter(&tx2), &tx_cost2) + .is_err()); } // however there is room for none-vote tx3 { let third_account = Keypair::new(); - let (_tx3, tx_cost3) = build_simple_transaction(&third_account, &start_hash); - assert!(testee.would_fit(&tx_cost3).is_ok()); + let (tx3, tx_cost3) = build_simple_transaction(&third_account, &start_hash); + assert!(testee + .would_fit(CostModel::writable_accounts_iter(&tx3), &tx_cost3) + .is_ok()); } } @@ -655,8 +709,8 @@ mod tests { let (mint_keypair, start_hash) = test_setup(); // build two transactions with diff accounts let second_account = Keypair::new(); - let (_tx1, mut tx_cost1) = build_simple_transaction(&mint_keypair, &start_hash); - let (_tx2, mut tx_cost2) = build_simple_transaction(&second_account, &start_hash); + let (tx1, mut tx_cost1) = build_simple_transaction(&mint_keypair, &start_hash); + let (tx2, mut tx_cost2) = build_simple_transaction(&second_account, &start_hash); if let TransactionCost::Transaction(ref mut usage_cost) = tx_cost1 { usage_cost.allocated_accounts_data_size = MAX_BLOCK_ACCOUNTS_DATA_SIZE_DELTA; } else { @@ -672,10 +726,12 @@ mod tests { // build testee that passes let testee = CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2 - 1, cost1 + cost2 - 1); - assert!(testee.would_fit(&tx_cost1).is_ok()); + assert!(testee + .would_fit(CostModel::writable_accounts_iter(&tx1), &tx_cost1) + .is_ok()); // data is too big assert_eq!( - testee.would_fit(&tx_cost2), + testee.would_fit(CostModel::writable_accounts_iter(&tx2), &tx_cost2), Err(CostTrackerError::WouldExceedAccountDataBlockLimit), ); } @@ -685,27 +741,35 @@ mod tests { let (mint_keypair, start_hash) = test_setup(); // build two transactions with diff accounts let second_account = Keypair::new(); - let (_tx1, tx_cost1) = build_simple_transaction(&mint_keypair, &start_hash); - let (_tx2, tx_cost2) = build_simple_transaction(&second_account, &start_hash); + let (tx1, tx_cost1) = build_simple_transaction(&mint_keypair, &start_hash); + let (tx2, tx_cost2) = build_simple_transaction(&second_account, &start_hash); let cost1 = tx_cost1.sum(); let cost2 = tx_cost2.sum(); // build testee let mut testee = CostTracker::new(cost1 + cost2, cost1 + cost2, cost1 + cost2); - assert!(testee.try_add(&tx_cost1).is_ok()); - assert!(testee.try_add(&tx_cost2).is_ok()); + assert!(testee + .try_add(CostModel::writable_accounts_iter(&tx1), &tx_cost1) + .is_ok()); + assert!(testee + .try_add(CostModel::writable_accounts_iter(&tx2), &tx_cost2) + .is_ok()); assert_eq!(cost1 + cost2, testee.block_cost); // removing a tx_cost affects block_cost - testee.remove(&tx_cost1); + testee.remove(CostModel::writable_accounts_iter(&tx1), &tx_cost1); assert_eq!(cost2, testee.block_cost); // add back tx1 - assert!(testee.try_add(&tx_cost1).is_ok()); + assert!(testee + .try_add(CostModel::writable_accounts_iter(&tx1), &tx_cost1) + .is_ok()); assert_eq!(cost1 + cost2, testee.block_cost); // cannot add tx1 again, cost limit would be exceeded - assert!(testee.try_add(&tx_cost1).is_err()); + assert!(testee + .try_add(CostModel::writable_accounts_iter(&tx1), &tx_cost1) + .is_err()); } #[test] @@ -726,11 +790,12 @@ mod tests { // and block_cost = $cost { let tx_cost = TransactionCost::Transaction(UsageCostDetails { - writable_accounts: vec![acct1, acct2, acct3], programs_execution_cost: cost, ..UsageCostDetails::default() }); - assert!(testee.try_add(&tx_cost).is_ok()); + assert!(testee + .try_add([acct1, acct2, acct3].iter(), &tx_cost) + .is_ok()); let (_costliest_account, costliest_account_cost) = testee.find_costliest_account(); assert_eq!(cost, testee.block_cost); assert_eq!(3, testee.cost_by_writable_accounts.len()); @@ -744,11 +809,10 @@ mod tests { // and block_cost = $cost * 2 { let tx_cost = TransactionCost::Transaction(UsageCostDetails { - writable_accounts: vec![acct2], programs_execution_cost: cost, ..UsageCostDetails::default() }); - assert!(testee.try_add(&tx_cost).is_ok()); + assert!(testee.try_add([acct2].iter(), &tx_cost).is_ok()); let (costliest_account, costliest_account_cost) = testee.find_costliest_account(); assert_eq!(cost * 2, testee.block_cost); assert_eq!(3, testee.cost_by_writable_accounts.len()); @@ -764,11 +828,10 @@ mod tests { // and block_cost = $cost * 2 { let tx_cost = TransactionCost::Transaction(UsageCostDetails { - writable_accounts: vec![acct1, acct2], programs_execution_cost: cost, ..UsageCostDetails::default() }); - assert!(testee.try_add(&tx_cost).is_err()); + assert!(testee.try_add([acct1, acct2].iter(), &tx_cost).is_err()); let (costliest_account, costliest_account_cost) = testee.find_costliest_account(); assert_eq!(cost * 2, testee.block_cost); assert_eq!(3, testee.cost_by_writable_accounts.len()); @@ -788,13 +851,13 @@ 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], programs_execution_cost: cost, ..UsageCostDetails::default() }); let mut expected_block_cost = tx_cost.sum(); + let writable_accounts = [acct1, acct2, acct3]; let expected_tx_count = 1; - assert!(testee.try_add(&tx_cost).is_ok()); + assert!(testee.try_add(writable_accounts.iter(), &tx_cost).is_ok()); assert_eq!(expected_block_cost, testee.block_cost()); assert_eq!(expected_tx_count, testee.transaction_count()); testee @@ -807,7 +870,7 @@ mod tests { // adjust up { let adjustment = 50u64; - testee.add_transaction_execution_cost(&tx_cost, adjustment); + testee.add_transaction_execution_cost(writable_accounts.iter(), &tx_cost, adjustment); expected_block_cost += 50; assert_eq!(expected_block_cost, testee.block_cost()); assert_eq!(expected_tx_count, testee.transaction_count()); @@ -822,7 +885,7 @@ mod tests { // adjust down { let adjustment = 50u64; - testee.sub_transaction_execution_cost(&tx_cost, adjustment); + testee.sub_transaction_execution_cost(writable_accounts.iter(), &tx_cost, adjustment); expected_block_cost -= 50; assert_eq!(expected_block_cost, testee.block_cost()); assert_eq!(expected_tx_count, testee.transaction_count()); @@ -836,7 +899,7 @@ mod tests { // adjust overflow { - testee.add_transaction_execution_cost(&tx_cost, u64::MAX); + testee.add_transaction_execution_cost(writable_accounts.iter(), &tx_cost, u64::MAX); // expect block cost set to limit assert_eq!(u64::MAX, testee.block_cost()); assert_eq!(expected_tx_count, testee.transaction_count()); @@ -850,7 +913,7 @@ mod tests { // adjust underflow { - testee.sub_transaction_execution_cost(&tx_cost, u64::MAX); + testee.sub_transaction_execution_cost(writable_accounts.iter(), &tx_cost, u64::MAX); // expect block cost set to limit assert_eq!(u64::MIN, testee.block_cost()); assert_eq!(expected_tx_count, testee.transaction_count()); @@ -872,12 +935,11 @@ mod tests { let estimated_programs_execution_cost = 100; let estimated_loaded_accounts_data_size_cost = 200; let number_writeble_accounts = 3; - let writable_accounts = std::iter::repeat_with(Pubkey::new_unique) + let writable_accounts: Vec<_> = std::iter::repeat_with(Pubkey::new_unique) .take(number_writeble_accounts) .collect(); let tx_cost = TransactionCost::Transaction(UsageCostDetails { - writable_accounts, programs_execution_cost: estimated_programs_execution_cost, loaded_accounts_data_size_cost: estimated_loaded_accounts_data_size_cost, ..UsageCostDetails::default() @@ -893,7 +955,9 @@ mod tests { let test_update_cost_tracker = |execution_cost_adjust: i64, loaded_acounts_data_size_cost_adjust: i64| { let mut cost_tracker = CostTracker::default(); - assert!(cost_tracker.try_add(&tx_cost).is_ok()); + assert!(cost_tracker + .try_add(writable_accounts.iter(), &tx_cost) + .is_ok()); let actual_programs_execution_cost = (estimated_programs_execution_cost as i64 + execution_cost_adjust) as u64; @@ -906,6 +970,7 @@ mod tests { as u64; cost_tracker.update_execution_cost( + writable_accounts.iter(), &tx_cost, actual_programs_execution_cost, actual_loaded_accounts_data_size_cost, @@ -940,12 +1005,12 @@ mod tests { let cost = 100u64; let tx_cost = TransactionCost::Transaction(UsageCostDetails { - writable_accounts: vec![Pubkey::new_unique()], programs_execution_cost: cost, ..UsageCostDetails::default() }); - cost_tracker.add_transaction_cost(&tx_cost); + let writable_accounts = [Pubkey::new_unique()]; + cost_tracker.add_transaction_cost(writable_accounts.iter(), &tx_cost); // assert cost_tracker is reverted to default assert_eq!(1, cost_tracker.transaction_count); assert_eq!(1, cost_tracker.number_of_accounts()); @@ -953,7 +1018,7 @@ mod tests { assert_eq!(0, cost_tracker.vote_cost); assert_eq!(0, cost_tracker.allocated_accounts_data_size); - cost_tracker.remove_transaction_cost(&tx_cost); + cost_tracker.remove_transaction_cost(writable_accounts.iter(), &tx_cost); // assert cost_tracker is reverted to default assert_eq!(0, cost_tracker.transaction_count); assert_eq!(0, cost_tracker.number_of_accounts()); diff --git a/cost-model/src/transaction_cost.rs b/cost-model/src/transaction_cost.rs index 4951e50036ca8b..fae081690fff3c 100644 --- a/cost-model/src/transaction_cost.rs +++ b/cost-model/src/transaction_cost.rs @@ -1,4 +1,4 @@ -use {crate::block_cost_limits, solana_sdk::pubkey::Pubkey}; +use crate::block_cost_limits; /// TransactionCost is used to represent resources required to process /// a transaction, denominated in CU (eg. Compute Units). @@ -12,7 +12,7 @@ const SIMPLE_VOTE_USAGE_COST: u64 = 3428; #[derive(Debug)] pub enum TransactionCost { - SimpleVote { writable_accounts: Vec }, + SimpleVote, Transaction(UsageCostDetails), } @@ -85,13 +85,6 @@ impl TransactionCost { } } - pub fn writable_accounts(&self) -> &[Pubkey] { - match self { - Self::SimpleVote { writable_accounts } => writable_accounts, - Self::Transaction(usage_cost) => &usage_cost.writable_accounts, - } - } - pub fn num_transaction_signatures(&self) -> u64 { match self { Self::SimpleVote { .. } => 1, @@ -114,12 +107,10 @@ impl TransactionCost { } } -const MAX_WRITABLE_ACCOUNTS: usize = 256; - // costs are stored in number of 'compute unit's -#[derive(Debug)] +#[derive(Debug, Default)] +#[cfg_attr(test, derive(PartialEq, Eq))] pub struct UsageCostDetails { - pub writable_accounts: Vec, pub signature_cost: u64, pub write_lock_cost: u64, pub data_bytes_cost: u64, @@ -131,60 +122,7 @@ pub struct UsageCostDetails { pub num_ed25519_instruction_signatures: u64, } -impl Default for UsageCostDetails { - fn default() -> Self { - Self { - writable_accounts: Vec::with_capacity(MAX_WRITABLE_ACCOUNTS), - signature_cost: 0u64, - write_lock_cost: 0u64, - data_bytes_cost: 0u64, - programs_execution_cost: 0u64, - loaded_accounts_data_size_cost: 0u64, - allocated_accounts_data_size: 0u64, - num_transaction_signatures: 0u64, - num_secp256k1_instruction_signatures: 0u64, - num_ed25519_instruction_signatures: 0u64, - } - } -} - -#[cfg(test)] -impl PartialEq for UsageCostDetails { - fn eq(&self, other: &Self) -> bool { - fn to_hash_set(v: &[Pubkey]) -> std::collections::HashSet<&Pubkey> { - v.iter().collect() - } - - self.signature_cost == other.signature_cost - && self.write_lock_cost == other.write_lock_cost - && self.data_bytes_cost == other.data_bytes_cost - && self.programs_execution_cost == other.programs_execution_cost - && self.loaded_accounts_data_size_cost == other.loaded_accounts_data_size_cost - && self.allocated_accounts_data_size == other.allocated_accounts_data_size - && self.num_transaction_signatures == other.num_transaction_signatures - && self.num_secp256k1_instruction_signatures - == other.num_secp256k1_instruction_signatures - && self.num_ed25519_instruction_signatures == other.num_ed25519_instruction_signatures - && to_hash_set(&self.writable_accounts) == to_hash_set(&other.writable_accounts) - } -} - -#[cfg(test)] -impl Eq for UsageCostDetails {} - impl UsageCostDetails { - #[cfg(test)] - pub fn new_with_capacity(capacity: usize) -> Self { - Self { - writable_accounts: Vec::with_capacity(capacity), - ..Self::default() - } - } - - pub fn new_with_default_capacity() -> Self { - Self::default() - } - pub fn sum(&self) -> u64 { self.signature_cost .saturating_add(self.write_lock_cost) diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index 8339c0a14d07ff..9e657b98617cb6 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -489,7 +489,8 @@ fn compute_slot_cost( num_programs += transaction.message().instructions().len(); let tx_cost = CostModel::calculate_cost(&transaction, &feature_set); - let result = cost_tracker.try_add(&tx_cost); + let result = + cost_tracker.try_add(CostModel::writable_accounts_iter(&transaction), &tx_cost); if result.is_err() { println!( "Slot: {slot}, CostModel rejected transaction {transaction:?}, reason \ diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 9234809b245e57..647671936c5339 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -253,7 +253,7 @@ fn check_block_cost_limits( ) -> Result<()> { assert_eq!(loaded_accounts_stats.len(), execution_results.len()); - let tx_costs_with_actual_execution_units: Vec<_> = execution_results + let writable_accounts_and_tx_costs_with_actual_execution_units: Vec<_> = execution_results .iter() .zip(loaded_accounts_stats) .zip(sanitized_transactions) @@ -267,7 +267,7 @@ fn check_block_cost_limits( .map_or(0, |stats| stats.loaded_accounts_data_size), &bank.feature_set, ); - Some(tx_cost) + Some((writable_accounts_iter(tx), tx_cost)) } else { None } @@ -276,15 +276,26 @@ fn check_block_cost_limits( { let mut cost_tracker = bank.write_cost_tracker().unwrap(); - for tx_cost in &tx_costs_with_actual_execution_units { + for (writable_accounts, tx_cost) in + writable_accounts_and_tx_costs_with_actual_execution_units + { cost_tracker - .try_add(tx_cost) + .try_add(writable_accounts, &tx_cost) .map_err(TransactionError::from)?; } } Ok(()) } +fn writable_accounts_iter(tx: &SanitizedTransaction) -> impl Iterator + Clone { + tx.message() + .account_keys() + .iter() + .enumerate() + .filter(|(index, _key)| tx.message().is_writable(*index)) + .map(|(_index, key)| key) +} + #[derive(Default)] pub struct ExecuteBatchesInternalMetrics { execution_timings_per_thread: HashMap, From 7f10d99837e557fd9ee7acf2044fd180001265a6 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Fri, 12 Jul 2024 13:49:44 -0500 Subject: [PATCH 2/3] fix test_cost_tracker_ok_add_two_same_accounts --- cost-model/src/cost_tracker.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cost-model/src/cost_tracker.rs b/cost-model/src/cost_tracker.rs index f6722cbfe06805..edb12b5ae4b6d9 100644 --- a/cost-model/src/cost_tracker.rs +++ b/cost-model/src/cost_tracker.rs @@ -580,7 +580,7 @@ mod tests { testee.add_transaction_cost(CostModel::writable_accounts_iter(&tx2), &tx_cost2); } assert_eq!(cost1 + cost2, testee.block_cost); - assert_eq!(1, testee.cost_by_writable_accounts.len()); + assert_eq!(3, testee.cost_by_writable_accounts.len()); let (_ccostliest_account, costliest_account_cost) = testee.find_costliest_account(); assert_eq!(cost1 + cost2, costliest_account_cost); } From 7235535af7a41212af9c2f295fbf6c22ce5b2a9c Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Fri, 12 Jul 2024 13:50:30 -0500 Subject: [PATCH 3/3] fix test_cost_tracker_ok_add_two_diff_accounts --- cost-model/src/cost_tracker.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cost-model/src/cost_tracker.rs b/cost-model/src/cost_tracker.rs index edb12b5ae4b6d9..c3781693cd22b3 100644 --- a/cost-model/src/cost_tracker.rs +++ b/cost-model/src/cost_tracker.rs @@ -610,7 +610,7 @@ mod tests { testee.add_transaction_cost(CostModel::writable_accounts_iter(&tx2), &tx_cost2); } assert_eq!(cost1 + cost2, testee.block_cost); - assert_eq!(2, testee.cost_by_writable_accounts.len()); + assert_eq!(4, testee.cost_by_writable_accounts.len()); let (_ccostliest_account, costliest_account_cost) = testee.find_costliest_account(); assert_eq!(std::cmp::max(cost1, cost2), costliest_account_cost); }