From 9c9f2dd5bd5d7aee79719fa84797e5fa63db676c Mon Sep 17 00:00:00 2001 From: Tao Zhu Date: Wed, 12 Jan 2022 23:27:19 -0600 Subject: [PATCH] port counting vote CUs to block cost (#22477) --- .../src/transaction_notifier.rs | 3 +- .../postgres_client_transaction.rs | 2 + core/benches/banking_stage.rs | 2 +- core/src/banking_stage.rs | 3 +- core/src/qos_service.rs | 24 +- runtime/src/accounts.rs | 1 + runtime/src/bank.rs | 5 +- runtime/src/block_cost_limits.rs | 3 + runtime/src/cost_model.rs | 49 +--- runtime/src/cost_tracker.rs | 277 ++++++++++++------ sdk/src/transaction/error.rs | 4 + storage-proto/proto/transaction_by_addr.proto | 1 + storage-proto/src/convert.rs | 13 +- 13 files changed, 239 insertions(+), 148 deletions(-) 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 2d6886652b..2390c8855d 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 5ebfe23492..fa398c250d 100644 --- a/core/benches/banking_stage.rs +++ b/core/benches/banking_stage.rs @@ -175,7 +175,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 bee0de635a..951279476a 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -982,7 +982,8 @@ impl BankingStage { lock_time.stop(); // retryable_txs includes AccountInUse, WouldExceedMaxBlockCostLimit - // WouldExceedMaxAccountCostLimit, and WouldExceedMaxAccountDataCostLimit + // WouldExceedMaxAccountCostLimit, WouldExceedMaxVoteCostLimit + // and WouldExceedMaxAccountDataCostLimit 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 40903fc5bd..a14d0c889c 100644 --- a/core/src/qos_service.rs +++ b/core/src/qos_service.rs @@ -137,6 +137,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) @@ -254,6 +258,9 @@ struct QosServiceMetrics { // number of transactions to be queued for retry due to its potential to breach block limit retried_txs_per_block_limit_count: AtomicU64, + // number of transactions to be queued for retry due to its potential to breach vote limit + retried_txs_per_vote_limit_count: AtomicU64, + // number of transactions to be queued for retry due to its potential to breach writable // account limit retried_txs_per_account_limit_count: AtomicU64, @@ -327,6 +334,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 @@ -429,6 +442,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]; @@ -436,18 +450,18 @@ mod tests { let qos_service = QosService::new(cost_model, 1); 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 vote 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()); } } diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index b979710c2d..09147332eb 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -1079,6 +1079,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 83ae9d52e4..0e2e1f69e1 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -218,7 +218,7 @@ impl RentDebits { } type BankStatusCache = StatusCache>; -#[frozen_abi(digest = "Gr2MTwWyUdkbF6FxM6TSwGaC3c5buUirHmh64oAPgg7Z")] +#[frozen_abi(digest = "FPLuTUU5MjwsijzDubxY6BvBEkWULhYNUyY6Puqejb4g")] pub type BankSlotDelta = SlotDelta>; // Eager rent collection repeats in cyclic manner. @@ -3791,6 +3791,7 @@ impl Bank { Some(index) } Err(TransactionError::WouldExceedMaxBlockCostLimit) + | Err(TransactionError::WouldExceedMaxVoteCostLimit) | Err(TransactionError::WouldExceedMaxAccountCostLimit) | Err(TransactionError::WouldExceedMaxAccountDataCostLimit) => Some(index), Err(_) => None, @@ -6416,7 +6417,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 c9f982ec6b..cd8c029d47 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::{ instruction::CompiledInstruction, program_utils::limited_deserialize, pubkey::Pubkey, @@ -27,9 +24,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, pub account_data_size: u64, } @@ -41,7 +35,6 @@ impl Default for TransactionCost { write_lock_cost: 0u64, data_bytes_cost: 0u64, execution_cost: 0u64, - cost_weight: 1u32, account_data_size: 0u64, } } @@ -61,7 +54,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 { @@ -118,7 +110,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); tx_cost.account_data_size = self.calculate_account_data_size(transaction); debug!("transaction {:?} has cost {:?}", transaction, tx_cost); @@ -254,15 +245,6 @@ impl CostModel { }) .sum() } - - 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)] @@ -283,7 +265,6 @@ mod tests { system_program, system_transaction, transaction::Transaction, }, - solana_vote_program::vote_transaction, std::{ str::FromStr, sync::{Arc, RwLock}, @@ -530,7 +511,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] @@ -636,31 +616,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 d682b4119d..dce265b85c 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, @@ -26,69 +29,92 @@ 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, account_data_size: 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, account_data_size: 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(), tx_cost.account_data_size, + 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(), tx_cost.account_data_size, + 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, tx_cost.account_data_size)?; - self.add_transaction(&tx_cost.writable_accounts, cost, tx_cost.account_data_size); + let cost = tx_cost.sum(); + self.would_fit( + &tx_cost.writable_accounts, + cost, + tx_cost.account_data_size, + transaction, + )?; + self.add_transaction( + &tx_cost.writable_accounts, + cost, + tx_cost.account_data_size, + transaction, + ); Ok(self.block_cost) } @@ -104,6 +130,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", @@ -134,12 +161,20 @@ impl CostTracker { keys: &[Pubkey], cost: u64, account_data_len: 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 { return Err(CostTrackerError::WouldExceedAccountMaxLimit); @@ -153,7 +188,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; @@ -166,16 +201,26 @@ impl CostTracker { Ok(()) } - fn add_transaction(&mut self, keys: &[Pubkey], cost: u64, account_data_size: u64) { + fn add_transaction( + &mut self, + keys: &[Pubkey], + cost: u64, + account_data_size: 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 = self.block_cost.saturating_add(cost); + if transaction.is_simple_vote_transaction() { + self.vote_cost = self.vote_cost.saturating_add(cost); } - self.block_cost += cost; - self.transaction_count += 1; self.account_data_size = self.account_data_size.saturating_add(account_data_size); + self.transaction_count = self.transaction_count.saturating_add(1); } } @@ -191,8 +236,9 @@ mod tests { hash::Hash, signature::{Keypair, Signer}, system_transaction, - transaction::Transaction, + transaction::{TransactionError, VersionedTransaction}, }, + solana_vote_program::vote_transaction, std::{cmp, sync::Arc}, }; @@ -211,19 +257,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); } @@ -231,25 +304,43 @@ 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, 0).is_ok()); - testee.add_transaction(&keys, cost, 0); + let mut testee = CostTracker::new(cost, cost, cost); + assert!(testee.would_fit(&keys, cost, 0, &tx).is_ok()); + testee.add_transaction(&keys, cost, 0, &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, 0, &tx).is_ok()); + testee.add_transaction(&keys, cost, 0, &tx); + assert_eq!(cost, testee.block_cost); + assert_eq!(cost, testee.vote_cost); + let (_costliest_account, costliest_account_cost) = testee.find_costliest_account(); + assert_eq!(cost, costliest_account_cost); } #[test] fn test_cost_tracker_add_data() { 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, 0).is_ok()); + let mut testee = CostTracker::new(cost, cost, cost); + assert!(testee.would_fit(&keys, cost, 0, &tx).is_ok()); let old = testee.account_data_size; - testee.add_transaction(&keys, cost, 1); + testee.add_transaction(&keys, cost, 1, &tx); assert_eq!(old + 1, testee.account_data_size); } @@ -257,62 +348,66 @@ 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, 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, 0).is_ok()); - testee.add_transaction(&keys1, cost1, 0); + assert!(testee.would_fit(&keys1, cost1, 0, &tx1).is_ok()); + testee.add_transaction(&keys1, cost1, 0, &tx1); } { - assert!(testee.would_fit(&keys2, cost2, 0).is_ok()); - testee.add_transaction(&keys2, cost2, 0); + assert!(testee.would_fit(&keys2, cost2, 0, &tx2).is_ok()); + testee.add_transaction(&keys2, cost2, 0, &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, 0).is_ok()); - testee.add_transaction(&keys1, cost1, 0); + assert!(testee.would_fit(&keys1, cost1, 0, &tx1).is_ok()); + testee.add_transaction(&keys1, cost1, 0, &tx1); } { - assert!(testee.would_fit(&keys2, cost2, 0).is_ok()); - testee.add_transaction(&keys2, cost2, 0); + assert!(testee.would_fit(&keys2, cost2, 0, &tx2).is_ok()); + testee.add_transaction(&keys2, cost2, 0, &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, 0).is_ok()); - testee.add_transaction(&keys1, cost1, 0); + assert!(testee.would_fit(&keys1, cost1, 0, &tx1).is_ok()); + testee.add_transaction(&keys1, cost1, 0, &tx1); } // but no more sapce on the same chain (same signer account) { - assert!(testee.would_fit(&keys2, cost2, 0).is_err()); + assert!(testee.would_fit(&keys2, cost2, 0, &tx2).is_err()); } } @@ -320,20 +415,48 @@ 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, 0).is_ok()); - testee.add_transaction(&keys1, cost1, 0); + assert!(testee.would_fit(&keys1, cost1, 0, &tx1).is_ok()); + testee.add_transaction(&keys1, cost1, 0, &tx1); } // but no more room for package as whole { - assert!(testee.would_fit(&keys2, cost2, 0).is_err()); + assert!(testee.would_fit(&keys2, cost2, 0, &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, 0, &tx1).is_ok()); + testee.add_transaction(&keys1, cost1, 0, &tx1); + } + // but no more room for package as whole + { + assert!(testee.would_fit(&keys2, cost2, 0, &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, 0, &tx3).is_ok()); } } @@ -341,26 +464,26 @@ mod tests { fn test_cost_tracker_reach_data_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 that passes - let testee = CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2 - 1); + let testee = CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2 - 1, cost1 + cost2 - 1); assert!(testee - .would_fit(&keys2, cost2, MAX_ACCOUNT_DATA_LEN) + .would_fit(&keys1, cost1, MAX_ACCOUNT_DATA_LEN, &tx1) .is_ok()); // data is too big assert!(testee - .would_fit(&keys2, cost2, MAX_ACCOUNT_DATA_LEN + 1) + .would_fit(&keys2, cost2, MAX_ACCOUNT_DATA_LEN + 1, &tx2) .is_err()); } #[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(); @@ -369,7 +492,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 | @@ -382,7 +505,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()); @@ -400,7 +523,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()); @@ -420,7 +543,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()); @@ -428,26 +551,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 0e48ba611c..02698cb619 100644 --- a/storage-proto/src/convert.rs +++ b/storage-proto/src/convert.rs @@ -727,6 +727,7 @@ impl TryFrom for TransactionError { 25 => TransactionError::InvalidAddressLookupTableData, 26 => TransactionError::InvalidAddressLookupTableIndex, 27 => TransactionError::InvalidRentPayingAccount, + 28 => TransactionError::WouldExceedMaxVoteCostLimit, _ => return Err("Invalid TransactionError"), }) } @@ -815,10 +816,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) => { @@ -1233,6 +1236,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();