diff --git a/accountsdb-plugin-manager/src/transaction_notifier.rs b/accountsdb-plugin-manager/src/transaction_notifier.rs index 02429501ae..5607812ac9 100644 --- a/accountsdb-plugin-manager/src/transaction_notifier.rs +++ b/accountsdb-plugin-manager/src/transaction_notifier.rs @@ -8,7 +8,6 @@ use { solana_measure::measure::Measure, solana_metrics::*, solana_rpc::transaction_notifier_interface::TransactionNotifier, - solana_runtime::bank, solana_sdk::{clock::Slot, signature::Signature, transaction::SanitizedTransaction}, solana_transaction_status::TransactionStatusMeta, std::sync::{Arc, RwLock}, @@ -85,7 +84,7 @@ impl TransactionNotifierImpl { ) -> ReplicaTransactionInfo<'a> { ReplicaTransactionInfo { signature, - is_vote: bank::is_simple_vote_transaction(transaction), + is_vote: transaction.is_simple_vote_transaction(), transaction, transaction_status_meta, } diff --git a/accountsdb-plugin-postgres/src/postgres_client/postgres_client_transaction.rs b/accountsdb-plugin-postgres/src/postgres_client/postgres_client_transaction.rs index c06f9eaf4f..d20c189b53 100644 --- a/accountsdb-plugin-postgres/src/postgres_client/postgres_client_transaction.rs +++ b/accountsdb-plugin-postgres/src/postgres_client/postgres_client_transaction.rs @@ -337,6 +337,7 @@ pub enum DbTransactionErrorCode { InvalidAddressLookupTableData, InvalidAddressLookupTableIndex, InvalidRentPayingAccount, + WouldExceedMaxVoteCostLimit, } impl From<&TransactionError> for DbTransactionErrorCode { @@ -363,6 +364,7 @@ impl From<&TransactionError> for DbTransactionErrorCode { Self::WouldExceedMaxAccountCostLimit } TransactionError::WouldExceedMaxBlockCostLimit => Self::WouldExceedMaxBlockCostLimit, + TransactionError::WouldExceedMaxVoteCostLimit => Self::WouldExceedMaxVoteCostLimit, TransactionError::UnsupportedVersion => Self::UnsupportedVersion, TransactionError::InvalidWritableAccount => Self::InvalidWritableAccount, TransactionError::WouldExceedMaxAccountDataCostLimit => { diff --git a/core/benches/banking_stage.rs b/core/benches/banking_stage.rs index dbb0961af1..2199fb51b3 100644 --- a/core/benches/banking_stage.rs +++ b/core/benches/banking_stage.rs @@ -174,7 +174,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); + .set_limits(std::u64::MAX, 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 538a253c20..370ad4cfaa 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -991,8 +991,8 @@ impl BankingStage { bank.prepare_sanitized_batch_with_results(txs, transactions_qos_results.into_iter()); lock_time.stop(); - // retryable_txs includes AccountInUse, WouldExceedMaxBlockCostLimit and - // WouldExceedMaxAccountCostLimit + // retryable_txs includes AccountInUse, WouldExceedMaxBlockCostLimit, + // WouldExceedMaxVoteCostLimit and WouldExceedMaxAccountCostLimit let (result, mut retryable_txs) = Self::process_and_record_transactions_locked( bank, poh, diff --git a/core/src/qos_service.rs b/core/src/qos_service.rs index feea54cac1..ee8769eca5 100644 --- a/core/src/qos_service.rs +++ b/core/src/qos_service.rs @@ -128,6 +128,10 @@ impl QosService { self.metrics.retried_txs_per_block_limit_count.fetch_add(1, Ordering::Relaxed); Err(TransactionError::WouldExceedMaxBlockCostLimit) } + CostTrackerError::WouldExceedVoteMaxLimit => { + self.metrics.retried_txs_per_vote_limit_count.fetch_add(1, Ordering::Relaxed); + Err(TransactionError::WouldExceedMaxVoteCostLimit) + } CostTrackerError::WouldExceedAccountMaxLimit => { self.metrics.retried_txs_per_account_limit_count.fetch_add(1, Ordering::Relaxed); Err(TransactionError::WouldExceedMaxAccountCostLimit) @@ -163,6 +167,7 @@ struct QosServiceMetrics { cost_tracking_time: AtomicU64, selected_txs_count: AtomicU64, retried_txs_per_block_limit_count: AtomicU64, + retried_txs_per_vote_limit_count: AtomicU64, retried_txs_per_account_limit_count: AtomicU64, } @@ -197,6 +202,12 @@ impl QosServiceMetrics { .swap(0, Ordering::Relaxed) as i64, i64 ), + ( + "retried_txs_per_vote_limit_count", + self.retried_txs_per_vote_limit_count + .swap(0, Ordering::Relaxed) as i64, + i64 + ), ( "retried_txs_per_account_limit_count", self.retried_txs_per_account_limit_count @@ -292,6 +303,7 @@ mod tests { .unwrap() .calculate_cost(&transfer_tx) .sum(); + let vote_tx_cost = cost_model.read().unwrap().calculate_cost(&vote_tx).sum(); // make a vec of txs let txs = vec![transfer_tx.clone(), vote_tx.clone(), transfer_tx, vote_tx]; @@ -299,19 +311,19 @@ mod tests { let qos_service = QosService::new(cost_model); let txs_costs = qos_service.compute_transaction_costs(txs.iter()); - // set cost tracker limit to fit 1 transfer tx, vote tx bypasses limit check - let cost_limit = transfer_tx_cost; + // set cost tracker limit to fit 1 transfer tx and 1 vote tx + let cost_limit = transfer_tx_cost + vote_tx_cost; bank.write_cost_tracker() .unwrap() - .set_limits(cost_limit, cost_limit); + .set_limits(cost_limit, cost_limit, cost_limit); let results = qos_service.select_transactions_per_cost(txs.iter(), txs_costs.iter(), &bank); - // verify that first transfer tx and all votes are allowed + // verify that first transfer tx and first votes are allowed assert_eq!(results.len(), txs.len()); assert!(results[0].is_ok()); assert!(results[1].is_ok()); assert!(results[2].is_err()); - assert!(results[3].is_ok()); + assert!(results[3].is_err()); } #[test] diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 78da9e95e4..828f097beb 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -1052,6 +1052,7 @@ impl Accounts { | Err(TransactionError::SanitizeFailure) | Err(TransactionError::TooManyAccountLocks) | Err(TransactionError::WouldExceedMaxBlockCostLimit) + | Err(TransactionError::WouldExceedMaxVoteCostLimit) | Err(TransactionError::WouldExceedMaxAccountCostLimit) | Err(TransactionError::WouldExceedMaxAccountDataCostLimit) => None, _ => Some(tx.get_account_locks_unchecked()), diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 5e67a7999f..49a9c84176 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3850,6 +3850,7 @@ impl Bank { Some(index) } Err(TransactionError::WouldExceedMaxBlockCostLimit) + | Err(TransactionError::WouldExceedMaxVoteCostLimit) | Err(TransactionError::WouldExceedMaxAccountCostLimit) => Some(index), Err(_) => None, Ok(_) => None, @@ -6433,7 +6434,7 @@ pub fn goto_end_of_slot(bank: &mut Bank) { } } -pub fn is_simple_vote_transaction(transaction: &SanitizedTransaction) -> bool { +fn is_simple_vote_transaction(transaction: &SanitizedTransaction) -> bool { if transaction.message().instructions().len() == 1 { let (program_pubkey, instruction) = transaction .message() diff --git a/runtime/src/block_cost_limits.rs b/runtime/src/block_cost_limits.rs index 6eead90cf2..186df94cc7 100644 --- a/runtime/src/block_cost_limits.rs +++ b/runtime/src/block_cost_limits.rs @@ -59,6 +59,9 @@ 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_model.rs b/runtime/src/cost_model.rs index 75d5af43da..6891e1e868 100644 --- a/runtime/src/cost_model.rs +++ b/runtime/src/cost_model.rs @@ -5,10 +5,7 @@ //! The main function is `calculate_cost` which returns &TransactionCost. //! use { - crate::{ - bank::is_simple_vote_transaction, block_cost_limits::*, - execute_cost_table::ExecuteCostTable, - }, + crate::{block_cost_limits::*, execute_cost_table::ExecuteCostTable}, log::*, solana_sdk::{pubkey::Pubkey, transaction::SanitizedTransaction}, std::collections::HashMap, @@ -24,9 +21,6 @@ pub struct TransactionCost { pub write_lock_cost: u64, pub data_bytes_cost: u64, pub execution_cost: u64, - // `cost_weight` is a multiplier could be applied to transaction cost, - // if set to zero allows the transaction to bypass cost limit check. - pub cost_weight: u32, } impl Default for TransactionCost { @@ -37,7 +31,6 @@ impl Default for TransactionCost { write_lock_cost: 0u64, data_bytes_cost: 0u64, execution_cost: 0u64, - cost_weight: 1u32, } } } @@ -56,7 +49,6 @@ impl TransactionCost { self.write_lock_cost = 0; self.data_bytes_cost = 0; self.execution_cost = 0; - self.cost_weight = 1; } pub fn sum(&self) -> u64 { @@ -113,7 +105,6 @@ impl CostModel { self.get_write_lock_cost(&mut tx_cost, transaction); tx_cost.data_bytes_cost = self.get_data_bytes_cost(transaction); tx_cost.execution_cost = self.get_transaction_cost(transaction); - tx_cost.cost_weight = self.calculate_cost_weight(transaction); debug!("transaction {:?} has cost {:?}", transaction, tx_cost); tx_cost @@ -195,15 +186,6 @@ impl CostModel { } cost } - - fn calculate_cost_weight(&self, transaction: &SanitizedTransaction) -> u32 { - if is_simple_vote_transaction(transaction) { - // vote has zero cost weight, so it bypasses block cost limit checking - 0u32 - } else { - 1u32 - } - } } #[cfg(test)] @@ -224,7 +206,6 @@ mod tests { system_program, system_transaction, transaction::Transaction, }, - solana_vote_program::vote_transaction, std::{ str::FromStr, sync::{Arc, RwLock}, @@ -424,7 +405,6 @@ mod tests { assert_eq!(expected_account_cost, tx_cost.write_lock_cost); assert_eq!(expected_execution_cost, tx_cost.execution_cost); assert_eq!(2, tx_cost.writable_accounts.len()); - assert_eq!(1u32, tx_cost.cost_weight); } #[test] @@ -530,31 +510,4 @@ mod tests { .get_cost(&solana_vote_program::id()) .is_some()); } - - #[test] - fn test_calculate_cost_weight() { - let (mint_keypair, start_hash) = test_setup(); - - let keypair = Keypair::new(); - let simple_transaction = SanitizedTransaction::from_transaction_for_tests( - system_transaction::transfer(&mint_keypair, &keypair.pubkey(), 2, start_hash), - ); - let vote_transaction = SanitizedTransaction::from_transaction_for_tests( - vote_transaction::new_vote_transaction( - vec![42], - Hash::default(), - Hash::default(), - &keypair, - &keypair, - &keypair, - None, - ), - ); - - let testee = CostModel::default(); - - // For now, vote has zero weight, everything else is neutral, for now - assert_eq!(1u32, testee.calculate_cost_weight(&simple_transaction)); - assert_eq!(0u32, testee.calculate_cost_weight(&vote_transaction)); - } } diff --git a/runtime/src/cost_tracker.rs b/runtime/src/cost_tracker.rs index 4ad4525fe3..2a81ac22ad 100644 --- a/runtime/src/cost_tracker.rs +++ b/runtime/src/cost_tracker.rs @@ -16,6 +16,9 @@ pub enum CostTrackerError { /// would exceed block max limit WouldExceedBlockMaxLimit, + /// would exceed vote max limit + WouldExceedVoteMaxLimit, + /// would exceed account max limit WouldExceedAccountMaxLimit, } @@ -24,59 +27,70 @@ 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) + CostTracker::new(MAX_WRITABLE_ACCOUNT_UNITS, MAX_BLOCK_UNITS, MAX_VOTE_UNITS) } } impl CostTracker { - pub fn new(account_cost_limit: u64, block_cost_limit: u64) -> Self { + pub fn new(account_cost_limit: u64, block_cost_limit: u64, vote_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) { + pub fn set_limits( + &mut self, + account_cost_limit: u64, + block_cost_limit: u64, + vote_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, - _transaction: &SanitizedTransaction, + transaction: &SanitizedTransaction, tx_cost: &TransactionCost, ) -> Result<(), CostTrackerError> { - self.would_fit(&tx_cost.writable_accounts, &tx_cost.sum()) + self.would_fit(&tx_cost.writable_accounts, tx_cost.sum(), transaction) } pub fn add_transaction_cost( &mut self, - _transaction: &SanitizedTransaction, + transaction: &SanitizedTransaction, tx_cost: &TransactionCost, ) { - self.add_transaction(&tx_cost.writable_accounts, &tx_cost.sum()); + self.add_transaction(&tx_cost.writable_accounts, tx_cost.sum(), transaction); } pub fn try_add( &mut self, - _transaction: &SanitizedTransaction, + transaction: &SanitizedTransaction, tx_cost: &TransactionCost, ) -> Result { - let cost = tx_cost.sum() * tx_cost.cost_weight as u64; - self.would_fit(&tx_cost.writable_accounts, &cost)?; - self.add_transaction(&tx_cost.writable_accounts, &cost); + let cost = tx_cost.sum(); + self.would_fit(&tx_cost.writable_accounts, cost, transaction)?; + self.add_transaction(&tx_cost.writable_accounts, cost, transaction); Ok(self.block_cost) } @@ -92,6 +106,7 @@ 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", @@ -116,14 +131,26 @@ impl CostTracker { (costliest_account, costliest_account_cost) } - fn would_fit(&self, keys: &[Pubkey], cost: &u64) -> Result<(), CostTrackerError> { + fn would_fit( + &self, + keys: &[Pubkey], + cost: u64, + transaction: &SanitizedTransaction, + ) -> Result<(), CostTrackerError> { // check against the total package cost - if self.block_cost + cost > self.block_cost_limit { + if self.block_cost.saturating_add(cost) > self.block_cost_limit { return Err(CostTrackerError::WouldExceedBlockMaxLimit); } + // if vote transaction, check if it exceeds vote_transaction_limit + if transaction.is_simple_vote_transaction() + && 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); } @@ -131,7 +158,7 @@ impl CostTracker { for account_key in keys.iter() { match self.cost_by_writable_accounts.get(account_key) { Some(chained_cost) => { - if chained_cost + cost > self.account_cost_limit { + if chained_cost.saturating_add(cost) > self.account_cost_limit { return Err(CostTrackerError::WouldExceedAccountMaxLimit); } else { continue; @@ -144,15 +171,19 @@ impl CostTracker { Ok(()) } - fn add_transaction(&mut self, keys: &[Pubkey], cost: &u64) { + fn add_transaction(&mut self, keys: &[Pubkey], cost: u64, transaction: &SanitizedTransaction) { for account_key in keys.iter() { - *self + let account_cost = self .cost_by_writable_accounts .entry(*account_key) - .or_insert(0) += cost; + .or_insert(0); + *account_cost = account_cost.saturating_add(cost); } - self.block_cost += cost; - self.transaction_count += 1; + self.block_cost = self.block_cost.saturating_add(cost); + if transaction.is_simple_vote_transaction() { + self.vote_cost = self.vote_cost.saturating_add(cost); + } + self.transaction_count = self.transaction_count.saturating_add(1); } } @@ -168,8 +199,9 @@ mod tests { hash::Hash, signature::{Keypair, Signer}, system_transaction, - transaction::Transaction, + transaction::{TransactionError, VersionedTransaction}, }, + solana_vote_program::vote_transaction, std::{cmp, sync::Arc}, }; @@ -188,19 +220,46 @@ mod tests { fn build_simple_transaction( mint_keypair: &Keypair, start_hash: &Hash, - ) -> (Transaction, Vec, u64) { + ) -> (SanitizedTransaction, Vec, u64) { let keypair = Keypair::new(); - let simple_transaction = - system_transaction::transfer(mint_keypair, &keypair.pubkey(), 2, *start_hash); + let simple_transaction = SanitizedTransaction::from_transaction_for_tests( + system_transaction::transfer(mint_keypair, &keypair.pubkey(), 2, *start_hash), + ); (simple_transaction, vec![mint_keypair.pubkey()], 5) } + fn build_simple_vote_transaction( + mint_keypair: &Keypair, + start_hash: &Hash, + ) -> (SanitizedTransaction, Vec, u64) { + let keypair = Keypair::new(); + let transaction = vote_transaction::new_vote_transaction( + vec![42], + Hash::default(), + *start_hash, + mint_keypair, + &keypair, + &keypair, + None, + ); + let message_hash = transaction.message.hash(); + let vote_transaction = SanitizedTransaction::try_create( + VersionedTransaction::from(transaction), + message_hash, + Some(true), + |_| Err(TransactionError::UnsupportedVersion), + ) + .unwrap(); + (vote_transaction, vec![mint_keypair.pubkey()], 10) + } + #[test] fn test_cost_tracker_initialization() { - let testee = CostTracker::new(10, 11); + let testee = CostTracker::new(10, 11, 8); 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); } @@ -208,75 +267,97 @@ mod tests { #[test] fn test_cost_tracker_ok_add_one() { let (mint_keypair, start_hash) = test_setup(); - let (_tx, keys, cost) = build_simple_transaction(&mint_keypair, &start_hash); + 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); - assert!(testee.would_fit(&keys, &cost).is_ok()); - testee.add_transaction(&keys, &cost); + let mut testee = CostTracker::new(cost, cost, cost); + assert!(testee.would_fit(&keys, cost, &tx).is_ok()); + testee.add_transaction(&keys, cost, &tx); 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_vote_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, &tx).is_ok()); + testee.add_transaction(&keys, cost, &tx); + 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] 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, keys1, cost1) = build_simple_transaction(&mint_keypair, &start_hash); - let (_tx2, keys2, cost2) = build_simple_transaction(&mint_keypair, &start_hash); + let (tx1, keys1, cost1) = build_simple_transaction(&mint_keypair, &start_hash); + 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); + let mut testee = CostTracker::new(cost1 + cost2, cost1 + cost2, cost1 + cost2); { - assert!(testee.would_fit(&keys1, &cost1).is_ok()); - testee.add_transaction(&keys1, &cost1); + assert!(testee.would_fit(&keys1, cost1, &tx1).is_ok()); + testee.add_transaction(&keys1, cost1, &tx1); } { - assert!(testee.would_fit(&keys2, &cost2).is_ok()); - testee.add_transaction(&keys2, &cost2); + assert!(testee.would_fit(&keys2, cost2, &tx2).is_ok()); + testee.add_transaction(&keys2, cost2, &tx2); } 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] fn test_cost_tracker_ok_add_two_diff_accounts() { let (mint_keypair, start_hash) = test_setup(); // build two transactions with diff accounts - let (_tx1, keys1, cost1) = build_simple_transaction(&mint_keypair, &start_hash); + 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); + 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); + let mut testee = CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2, cost1 + cost2); { - assert!(testee.would_fit(&keys1, &cost1).is_ok()); - testee.add_transaction(&keys1, &cost1); + assert!(testee.would_fit(&keys1, cost1, &tx1).is_ok()); + testee.add_transaction(&keys1, cost1, &tx1); } { - assert!(testee.would_fit(&keys2, &cost2).is_ok()); - testee.add_transaction(&keys2, &cost2); + assert!(testee.would_fit(&keys2, cost2, &tx2).is_ok()); + testee.add_transaction(&keys2, cost2, &tx2); } 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] fn test_cost_tracker_chain_reach_limit() { let (mint_keypair, start_hash) = test_setup(); // build two transactions with same signed account - let (_tx1, keys1, cost1) = build_simple_transaction(&mint_keypair, &start_hash); - let (_tx2, keys2, cost2) = build_simple_transaction(&mint_keypair, &start_hash); + let (tx1, keys1, cost1) = build_simple_transaction(&mint_keypair, &start_hash); + 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); + let mut testee = CostTracker::new(cmp::min(cost1, cost2), cost1 + cost2, cost1 + cost2); // should have room for first transaction { - assert!(testee.would_fit(&keys1, &cost1).is_ok()); - testee.add_transaction(&keys1, &cost1); + assert!(testee.would_fit(&keys1, cost1, &tx1).is_ok()); + testee.add_transaction(&keys1, cost1, &tx1); } // but no more sapce on the same chain (same signer account) { - assert!(testee.would_fit(&keys2, &cost2).is_err()); + assert!(testee.would_fit(&keys2, cost2, &tx2).is_err()); } } @@ -284,28 +365,56 @@ mod tests { fn test_cost_tracker_reach_limit() { let (mint_keypair, start_hash) = test_setup(); // build two transactions with diff accounts - let (_tx1, keys1, cost1) = build_simple_transaction(&mint_keypair, &start_hash); + 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); + 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); + let mut testee = + CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2 - 1, cost1 + cost2 - 1); // should have room for first transaction { - assert!(testee.would_fit(&keys1, &cost1).is_ok()); - testee.add_transaction(&keys1, &cost1); + assert!(testee.would_fit(&keys1, cost1, &tx1).is_ok()); + testee.add_transaction(&keys1, cost1, &tx1); } // but no more room for package as whole { - assert!(testee.would_fit(&keys2, &cost2).is_err()); + assert!(testee.would_fit(&keys2, cost2, &tx2).is_err()); + } + } + + #[test] + fn test_cost_tracker_reach_vote_limit() { + let (mint_keypair, start_hash) = test_setup(); + // build two mocking vote transactions with diff accounts + let (tx1, keys1, cost1) = build_simple_vote_transaction(&mint_keypair, &start_hash); + let second_account = Keypair::new(); + let (tx2, keys2, cost2) = build_simple_vote_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, &tx1).is_ok()); + testee.add_transaction(&keys1, cost1, &tx1); + } + // but no more room for package as whole + { + assert!(testee.would_fit(&keys2, cost2, &tx2).is_err()); + } + // however there is room for none-vote tx3 + { + let third_account = Keypair::new(); + let (tx3, keys3, cost3) = build_simple_transaction(&third_account, &start_hash); + assert!(testee.would_fit(&keys3, cost3, &tx3).is_ok()); } } #[test] fn test_cost_tracker_try_add_is_atomic() { let (mint_keypair, start_hash) = test_setup(); - let (tx, _keys, _cost) = build_simple_transaction(&mint_keypair, &start_hash); - let tx = SanitizedTransaction::from_transaction_for_tests(tx); + // build two mocking vote transactions with diff accounts + let (tx1, _keys1, _cost1) = build_simple_vote_transaction(&mint_keypair, &start_hash); let acct1 = Pubkey::new_unique(); let acct2 = Pubkey::new_unique(); @@ -314,7 +423,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); + let mut testee = CostTracker::new(account_max, block_max, block_max); // case 1: a tx writes to 3 accounts, should success, we will have: // | acct1 | $cost | @@ -327,7 +436,7 @@ mod tests { execution_cost: cost, ..TransactionCost::default() }; - assert!(testee.try_add(&tx, &tx_cost).is_ok()); + assert!(testee.try_add(&tx1, &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()); @@ -345,7 +454,7 @@ mod tests { execution_cost: cost, ..TransactionCost::default() }; - assert!(testee.try_add(&tx, &tx_cost).is_ok()); + assert!(testee.try_add(&tx1, &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()); @@ -365,7 +474,7 @@ mod tests { execution_cost: cost, ..TransactionCost::default() }; - assert!(testee.try_add(&tx, &tx_cost).is_err()); + assert!(testee.try_add(&tx1, &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()); @@ -373,26 +482,4 @@ mod tests { assert_eq!(acct2, costliest_account); } } - - #[test] - fn test_try_add_with_cost_weight() { - let (mint_keypair, start_hash) = test_setup(); - let (tx, _keys, _cost) = build_simple_transaction(&mint_keypair, &start_hash); - let tx = SanitizedTransaction::from_transaction_for_tests(tx); - - let limit = 100u64; - let mut testee = CostTracker::new(limit, limit); - - let mut cost = TransactionCost { - execution_cost: limit + 1, - ..TransactionCost::default() - }; - - // cost exceed limit by 1, will not fit - assert!(testee.try_add(&tx, &cost).is_err()); - - cost.cost_weight = 0u32; - // setting cost_weight to zero will allow this tx - assert!(testee.try_add(&tx, &cost).is_ok()); - } } diff --git a/sdk/src/transaction/error.rs b/sdk/src/transaction/error.rs index 72812bbacf..44e92a4861 100644 --- a/sdk/src/transaction/error.rs +++ b/sdk/src/transaction/error.rs @@ -131,6 +131,10 @@ pub enum TransactionError { "Transaction leaves an account with data with a lower balance than rent-exempt minimum" )] InvalidRentPayingAccount, + + /// Transaction would exceed max Vote Cost Limit + #[error("Transaction would exceed max Vote Cost Limit")] + WouldExceedMaxVoteCostLimit, } impl From for TransactionError { diff --git a/storage-proto/proto/transaction_by_addr.proto b/storage-proto/proto/transaction_by_addr.proto index cc63498876..ee88455e66 100644 --- a/storage-proto/proto/transaction_by_addr.proto +++ b/storage-proto/proto/transaction_by_addr.proto @@ -52,6 +52,7 @@ enum TransactionErrorType { INVALID_ADDRESS_LOOKUP_TABLE_DATA = 25; INVALID_ADDRESS_LOOKUP_TABLE_INDEX = 26; INVALID_RENT_PAYING_ACCOUNT = 27; + WOULD_EXCEED_MAX_VOTE_COST_LIMIT = 28; } message InstructionError { diff --git a/storage-proto/src/convert.rs b/storage-proto/src/convert.rs index e5b1807f1a..d88690d3f7 100644 --- a/storage-proto/src/convert.rs +++ b/storage-proto/src/convert.rs @@ -575,6 +575,7 @@ impl TryFrom for TransactionError { 25 => TransactionError::InvalidAddressLookupTableData, 26 => TransactionError::InvalidAddressLookupTableIndex, 27 => TransactionError::InvalidRentPayingAccount, + 28 => TransactionError::WouldExceedMaxVoteCostLimit, _ => return Err("Invalid TransactionError"), }) } @@ -663,10 +664,12 @@ impl From for tx_by_addr::TransactionError { TransactionError::InvalidAddressLookupTableIndex => { tx_by_addr::TransactionErrorType::InvalidAddressLookupTableIndex } - TransactionError::InvalidRentPayingAccount => { tx_by_addr::TransactionErrorType::InvalidRentPayingAccount } + TransactionError::WouldExceedMaxVoteCostLimit => { + tx_by_addr::TransactionErrorType::WouldExceedMaxVoteCostLimit + } } as i32, instruction_error: match transaction_error { TransactionError::InstructionError(index, ref instruction_error) => { @@ -1081,6 +1084,14 @@ mod test { tx_by_addr_transaction_error.try_into().unwrap() ); + let transaction_error = TransactionError::WouldExceedMaxVoteCostLimit; + let tx_by_addr_transaction_error: tx_by_addr::TransactionError = + transaction_error.clone().into(); + assert_eq!( + transaction_error, + tx_by_addr_transaction_error.try_into().unwrap() + ); + let transaction_error = TransactionError::WouldExceedMaxAccountCostLimit; let tx_by_addr_transaction_error: tx_by_addr::TransactionError = transaction_error.clone().into();