diff --git a/core/benches/banking_stage.rs b/core/benches/banking_stage.rs index dd62835940..044eb4820b 100644 --- a/core/benches/banking_stage.rs +++ b/core/benches/banking_stage.rs @@ -171,7 +171,7 @@ fn bench_banking(bencher: &mut Bencher, tx_type: TransactionType) { // set cost tracker limits to MAX so it will not filter out TXs bank.write_cost_tracker() .unwrap() - .set_limits(std::u64::MAX, std::u64::MAX, std::u64::MAX); + .set_limits(std::u64::MAX, std::u64::MAX); debug!("threads: {} txs: {}", num_threads, txes); diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs index 115679d8ee..03cd7c4742 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -1130,7 +1130,7 @@ impl BankingStage { let tx: Transaction = limited_deserialize(&p.data[0..p.meta.size]).ok()?; tx.verify_precompiles(feature_set).ok()?; - Some((tx, *tx_index, p.meta.is_simple_vote_tx)) + Some((tx, *tx_index)) }) .collect(); banking_stage_stats.cost_tracker_check_count.fetch_add( @@ -1142,18 +1142,18 @@ impl BankingStage { let filtered_transactions_with_packet_indexes: Vec<_> = { verified_transactions_with_packet_indexes .into_iter() - .filter_map(|(tx, tx_index, is_simple_vote_tx)| { + .filter_map(|(tx, tx_index)| { // put transaction into retry queue if it wouldn't fit // into current bank if read_cost_tracker - .would_transaction_fit( - is_simple_vote_tx, - &cost_model - .read() - .unwrap() - .calculate_cost(&tx, demote_program_write_locks), - ) - .is_err() + .would_transaction_fit( + &tx, + &cost_model + .read() + .unwrap() + .calculate_cost(&tx, demote_program_write_locks), + ) + .is_err() { debug!("transaction {:?} would exceed limit", tx); retryable_transaction_packet_indexes.push(tx_index); @@ -1293,7 +1293,7 @@ impl BankingStage { transactions.iter().enumerate().for_each(|(index, tx)| { if unprocessed_tx_indexes.iter().all(|&i| i != index) { bank.write_cost_tracker().unwrap().add_transaction_cost( - tx.is_simple_vote_transaction(), + tx.transaction(), &cost_model .read() .unwrap() diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index a6dc4ac06f..9f736f7562 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -31,7 +31,6 @@ use { cost_model::CostModel, cost_tracker::CostTracker, hardened_unpack::{open_genesis_config, MAX_GENESIS_ARCHIVE_UNPACKED_SIZE}, - hashed_transaction::HashedTransaction, snapshot_utils::{self, SnapshotVersion, DEFAULT_MAX_SNAPSHOTS_TO_RETAIN}, }, solana_sdk::{ @@ -756,9 +755,7 @@ fn compute_slot_cost(blockstore: &Blockstore, slot: Slot) -> Result<(), String> for transaction in &entry.transactions { programs += transaction.message().instructions.len(); let tx_cost = cost_model.calculate_cost(transaction, true); - let hashed_transaction = HashedTransaction::from(transaction); - let result = - cost_tracker.try_add(hashed_transaction.is_simple_vote_transaction(), &tx_cost); + let result = cost_tracker.try_add(transaction, &tx_cost); if result.is_err() { println!( "Slot: {}, CostModel rejected transaction {:?}, reason {:?}", diff --git a/runtime/src/block_cost_limits.rs b/runtime/src/block_cost_limits.rs index 186df94cc7..6eead90cf2 100644 --- a/runtime/src/block_cost_limits.rs +++ b/runtime/src/block_cost_limits.rs @@ -59,9 +59,6 @@ pub const MAX_BLOCK_UNITS: u64 = /// limit is to prevent too many transactions write to same account, therefore /// reduce block's parallelism. pub const MAX_WRITABLE_ACCOUNT_UNITS: u64 = MAX_BLOCK_REPLAY_TIME_US * COMPUTE_UNIT_TO_US_RATIO; -/// Number of compute units that a block can have for vote transactions, -/// sets at ~75% of MAX_BLOCK_UNITS to leave room for non-vote transactions -pub const MAX_VOTE_UNITS: u64 = (MAX_BLOCK_UNITS as f64 * 0.75_f64) as u64; /// max length of account data in a slot (bytes) pub const MAX_ACCOUNT_DATA_LEN: u64 = 100_000_000; diff --git a/runtime/src/cost_tracker.rs b/runtime/src/cost_tracker.rs index 5e7bdf51ad..d2aa08c709 100644 --- a/runtime/src/cost_tracker.rs +++ b/runtime/src/cost_tracker.rs @@ -5,7 +5,7 @@ //! use { crate::{block_cost_limits::*, cost_model::TransactionCost}, - solana_sdk::{clock::Slot, pubkey::Pubkey}, + solana_sdk::{clock::Slot, pubkey::Pubkey, transaction::Transaction}, std::collections::HashMap, }; @@ -16,9 +16,6 @@ pub enum CostTrackerError { /// would exceed block max limit WouldExceedBlockMaxLimit, - /// would exceed vote max limit - WouldExceedVoteMaxLimit, - /// would exceed account max limit WouldExceedAccountMaxLimit, } @@ -27,66 +24,55 @@ pub enum CostTrackerError { pub struct CostTracker { account_cost_limit: u64, block_cost_limit: u64, - vote_cost_limit: u64, cost_by_writable_accounts: HashMap, block_cost: u64, - vote_cost: u64, transaction_count: u64, } impl Default for CostTracker { fn default() -> Self { - CostTracker::new(MAX_WRITABLE_ACCOUNT_UNITS, MAX_BLOCK_UNITS, MAX_VOTE_UNITS) + CostTracker::new(MAX_WRITABLE_ACCOUNT_UNITS, MAX_BLOCK_UNITS) } } impl CostTracker { - pub fn new(account_cost_limit: u64, block_cost_limit: u64, vote_cost_limit: u64) -> Self { + pub fn new(account_cost_limit: u64, block_cost_limit: u64) -> Self { assert!(account_cost_limit <= block_cost_limit); - assert!(vote_cost_limit <= block_cost_limit); Self { account_cost_limit, block_cost_limit, - vote_cost_limit, cost_by_writable_accounts: HashMap::with_capacity(WRITABLE_ACCOUNTS_PER_BLOCK), block_cost: 0, - vote_cost: 0, transaction_count: 0, } } // bench tests needs to reset limits - pub fn set_limits( - &mut self, - account_cost_limit: u64, - block_cost_limit: u64, - vote_cost_limit: u64, - ) { + pub fn set_limits(&mut self, account_cost_limit: u64, block_cost_limit: u64) { self.account_cost_limit = account_cost_limit; self.block_cost_limit = block_cost_limit; - self.vote_cost_limit = vote_cost_limit; } pub fn would_transaction_fit( &self, - is_vote: bool, + _transaction: &Transaction, tx_cost: &TransactionCost, ) -> Result<(), CostTrackerError> { - self.would_fit(&tx_cost.writable_accounts, tx_cost.sum(), is_vote) + self.would_fit(&tx_cost.writable_accounts, &tx_cost.sum()) } - pub fn add_transaction_cost(&mut self, is_vote: bool, tx_cost: &TransactionCost) { - self.add_transaction(&tx_cost.writable_accounts, tx_cost.sum(), is_vote); + pub fn add_transaction_cost(&mut self, _transaction: &Transaction, tx_cost: &TransactionCost) { + self.add_transaction(&tx_cost.writable_accounts, &tx_cost.sum()); } pub fn try_add( &mut self, - is_vote: bool, + _transaction: &Transaction, tx_cost: &TransactionCost, ) -> Result { let cost = tx_cost.sum(); - self.would_fit(&tx_cost.writable_accounts, cost, is_vote)?; - self.add_transaction(&tx_cost.writable_accounts, cost, is_vote); + self.would_fit(&tx_cost.writable_accounts, &cost)?; + self.add_transaction(&tx_cost.writable_accounts, &cost); Ok(self.block_cost) } @@ -102,7 +88,6 @@ impl CostTracker { "cost_tracker_stats", ("bank_slot", bank_slot as i64, i64), ("block_cost", self.block_cost as i64, i64), - ("vote_cost", self.vote_cost as i64, i64), ("transaction_count", self.transaction_count as i64, i64), ( "number_of_accounts", @@ -127,19 +112,14 @@ impl CostTracker { (costliest_account, costliest_account_cost) } - fn would_fit(&self, keys: &[Pubkey], cost: u64, is_vote: bool) -> Result<(), CostTrackerError> { + fn would_fit(&self, keys: &[Pubkey], cost: &u64) -> Result<(), CostTrackerError> { // check against the total package cost - if self.block_cost.saturating_add(cost) > self.block_cost_limit { + if self.block_cost + cost > self.block_cost_limit { return Err(CostTrackerError::WouldExceedBlockMaxLimit); } - // if vote transaction, check if it exceeds vote_transaction_limit - if is_vote && self.vote_cost.saturating_add(cost) > self.vote_cost_limit { - return Err(CostTrackerError::WouldExceedVoteMaxLimit); - } - // check if the transaction itself is more costly than the account_cost_limit - if cost > self.account_cost_limit { + if *cost > self.account_cost_limit { return Err(CostTrackerError::WouldExceedAccountMaxLimit); } @@ -147,7 +127,7 @@ impl CostTracker { for account_key in keys.iter() { match self.cost_by_writable_accounts.get(&account_key) { Some(chained_cost) => { - if chained_cost.saturating_add(cost) > self.account_cost_limit { + if chained_cost + cost > self.account_cost_limit { return Err(CostTrackerError::WouldExceedAccountMaxLimit); } else { continue; @@ -160,19 +140,15 @@ impl CostTracker { Ok(()) } - fn add_transaction(&mut self, keys: &[Pubkey], cost: u64, is_vote: bool) { + fn add_transaction(&mut self, keys: &[Pubkey], cost: &u64) { for account_key in keys.iter() { - let account_cost = self + *self .cost_by_writable_accounts .entry(*account_key) - .or_insert(0); - *account_cost = account_cost.saturating_add(cost); + .or_insert(0) += cost; } - self.block_cost = self.block_cost.saturating_add(cost); - if is_vote { - self.vote_cost = self.vote_cost.saturating_add(cost); - } - self.transaction_count = self.transaction_count.saturating_add(1); + self.block_cost += cost; + self.transaction_count += 1; } } @@ -218,10 +194,9 @@ mod tests { #[test] fn test_cost_tracker_initialization() { - let testee = CostTracker::new(10, 11, 8); + let testee = CostTracker::new(10, 11); assert_eq!(10, testee.account_cost_limit); assert_eq!(11, testee.block_cost_limit); - assert_eq!(8, testee.vote_cost_limit); assert_eq!(0, testee.cost_by_writable_accounts.len()); assert_eq!(0, testee.block_cost); } @@ -232,28 +207,10 @@ mod tests { let (_tx, keys, cost) = build_simple_transaction(&mint_keypair, &start_hash); // build testee to have capacity for one simple transaction - let mut testee = CostTracker::new(cost, cost, cost); - assert!(testee.would_fit(&keys, cost, false).is_ok()); - testee.add_transaction(&keys, cost, false); + let mut testee = CostTracker::new(cost, cost); + assert!(testee.would_fit(&keys, &cost).is_ok()); + testee.add_transaction(&keys, &cost); assert_eq!(cost, testee.block_cost); - assert_eq!(0, testee.vote_cost); - let (_costliest_account, costliest_account_cost) = testee.find_costliest_account(); - assert_eq!(cost, costliest_account_cost); - } - - #[test] - fn test_cost_tracker_ok_add_one_vote() { - let (mint_keypair, start_hash) = test_setup(); - let (_tx, keys, cost) = build_simple_transaction(&mint_keypair, &start_hash); - - // build testee to have capacity for one simple transaction - let mut testee = CostTracker::new(cost, cost, cost); - assert!(testee.would_fit(&keys, cost, true).is_ok()); - testee.add_transaction(&keys, cost, true); - assert_eq!(cost, testee.block_cost); - assert_eq!(cost, testee.vote_cost); - let (_ccostliest_account, costliest_account_cost) = testee.find_costliest_account(); - assert_eq!(cost, costliest_account_cost); } #[test] @@ -264,19 +221,17 @@ mod tests { let (_tx2, keys2, cost2) = build_simple_transaction(&mint_keypair, &start_hash); // build testee to have capacity for two simple transactions, with same accounts - let mut testee = CostTracker::new(cost1 + cost2, cost1 + cost2, cost1 + cost2); + let mut testee = CostTracker::new(cost1 + cost2, cost1 + cost2); { - assert!(testee.would_fit(&keys1, cost1, false).is_ok()); - testee.add_transaction(&keys1, cost1, false); + assert!(testee.would_fit(&keys1, &cost1).is_ok()); + testee.add_transaction(&keys1, &cost1); } { - assert!(testee.would_fit(&keys2, cost2, false).is_ok()); - testee.add_transaction(&keys2, cost2, false); + assert!(testee.would_fit(&keys2, &cost2).is_ok()); + testee.add_transaction(&keys2, &cost2); } assert_eq!(cost1 + cost2, testee.block_cost); assert_eq!(1, testee.cost_by_writable_accounts.len()); - let (_ccostliest_account, costliest_account_cost) = testee.find_costliest_account(); - assert_eq!(cost1 + cost2, costliest_account_cost); } #[test] @@ -288,19 +243,17 @@ mod tests { let (_tx2, keys2, cost2) = build_simple_transaction(&second_account, &start_hash); // 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); + let mut testee = CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2); { - assert!(testee.would_fit(&keys1, cost1, false).is_ok()); - testee.add_transaction(&keys1, cost1, false); + assert!(testee.would_fit(&keys1, &cost1).is_ok()); + testee.add_transaction(&keys1, &cost1); } { - assert!(testee.would_fit(&keys2, cost2, false).is_ok()); - testee.add_transaction(&keys2, cost2, false); + assert!(testee.would_fit(&keys2, &cost2).is_ok()); + testee.add_transaction(&keys2, &cost2); } assert_eq!(cost1 + cost2, testee.block_cost); assert_eq!(2, 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); } #[test] @@ -311,21 +264,20 @@ mod tests { let (_tx2, keys2, cost2) = build_simple_transaction(&mint_keypair, &start_hash); // 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); + let mut testee = CostTracker::new(cmp::min(cost1, cost2), cost1 + cost2); // should have room for first transaction { - assert!(testee.would_fit(&keys1, cost1, false).is_ok()); - testee.add_transaction(&keys1, cost1, false); + assert!(testee.would_fit(&keys1, &cost1).is_ok()); + testee.add_transaction(&keys1, &cost1); } // but no more sapce on the same chain (same signer account) { - assert!(testee.would_fit(&keys2, cost2, false).is_err()); + assert!(testee.would_fit(&keys2, &cost2).is_err()); } } #[test] fn test_cost_tracker_reach_limit() { - let none_vote = false; let (mint_keypair, start_hash) = test_setup(); // build two transactions with diff accounts let (_tx1, keys1, cost1) = build_simple_transaction(&mint_keypair, &start_hash); @@ -333,50 +285,22 @@ mod tests { let (_tx2, keys2, cost2) = build_simple_transaction(&second_account, &start_hash); // build testee to have capacity for each chain, but not enough room for both transactions - let mut testee = - CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2 - 1, cost1 + cost2 - 1); + let mut testee = CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2 - 1); // should have room for first transaction { - assert!(testee.would_fit(&keys1, cost1, none_vote).is_ok()); - testee.add_transaction(&keys1, cost1, none_vote); + assert!(testee.would_fit(&keys1, &cost1).is_ok()); + testee.add_transaction(&keys1, &cost1); } // but no more room for package as whole { - assert!(testee.would_fit(&keys2, cost2, none_vote).is_err()); - } - } - - #[test] - fn test_cost_tracker_reach_vote_limit() { - let is_vote = true; - let (mint_keypair, start_hash) = test_setup(); - // build two mocking vote transactions with diff accounts - let (_tx1, keys1, cost1) = build_simple_transaction(&mint_keypair, &start_hash); - let second_account = Keypair::new(); - let (_tx2, keys2, cost2) = build_simple_transaction(&second_account, &start_hash); - - // build testee to have capacity for each chain, but not enough room for both votes - let mut testee = CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2, cost1 + cost2 - 1); - // should have room for first vote - { - assert!(testee.would_fit(&keys1, cost1, is_vote).is_ok()); - testee.add_transaction(&keys1, cost1, is_vote); - } - // but no more room for package as whole - { - assert!(testee.would_fit(&keys2, cost2, is_vote).is_err()); - } - // however there is room for tx2, if it is not a vote - { - assert!(testee.would_fit(&keys2, cost2, !is_vote).is_ok()); + assert!(testee.would_fit(&keys2, &cost2).is_err()); } } #[test] fn test_cost_tracker_try_add_is_atomic() { let (mint_keypair, start_hash) = test_setup(); - let (_ctx, _keys, _cost) = build_simple_transaction(&mint_keypair, &start_hash); - let none_vote = false; + let (tx, _keys, _cost) = build_simple_transaction(&mint_keypair, &start_hash); let acct1 = Pubkey::new_unique(); let acct2 = Pubkey::new_unique(); @@ -385,7 +309,7 @@ mod tests { let account_max = cost * 2; let block_max = account_max * 3; // for three accts - let mut testee = CostTracker::new(account_max, block_max, block_max); + let mut testee = CostTracker::new(account_max, block_max); // case 1: a tx writes to 3 accounts, should success, we will have: // | acct1 | $cost | @@ -398,7 +322,7 @@ mod tests { execution_cost: cost, ..TransactionCost::default() }; - assert!(testee.try_add(none_vote, &tx_cost).is_ok()); + assert!(testee.try_add(&tx, &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()); @@ -416,7 +340,7 @@ mod tests { execution_cost: cost, ..TransactionCost::default() }; - assert!(testee.try_add(none_vote, &tx_cost).is_ok()); + assert!(testee.try_add(&tx, &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()); @@ -436,7 +360,7 @@ mod tests { execution_cost: cost, ..TransactionCost::default() }; - assert!(testee.try_add(none_vote, &tx_cost).is_err()); + assert!(testee.try_add(&tx, &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());