diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs index 598d459b6d..b2494d9c86 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -46,6 +46,7 @@ use { feature_set, message::Message, pubkey::Pubkey, + saturating_add_assign, timing::{duration_as_ms, timestamp, AtomicInterval}, transaction::{ self, AddressLoader, SanitizedTransaction, TransactionError, VersionedTransaction, @@ -324,6 +325,12 @@ impl BankingStageStats { } } +#[derive(Debug, Default)] +pub struct BatchedTransactionDetails { + pub costs: BatchedTransactionCostDetails, + pub errors: BatchedTransactionErrorDetails, +} + #[derive(Debug, Default)] pub struct BatchedTransactionCostDetails { pub batched_signature_cost: u64, @@ -332,6 +339,15 @@ pub struct BatchedTransactionCostDetails { pub batched_execute_cost: u64, } +#[derive(Debug, Default)] +pub struct BatchedTransactionErrorDetails { + pub batched_retried_txs_per_block_limit_count: u64, + pub batched_retried_txs_per_vote_limit_count: u64, + pub batched_retried_txs_per_account_limit_count: u64, + pub batched_retried_txs_per_account_data_block_limit_count: u64, + pub batched_dropped_txs_per_account_data_total_limit_count: u64, +} + #[derive(Debug, Default)] struct EndOfSlot { next_slot_leader: Option, @@ -1415,27 +1431,74 @@ impl BankingStage { fn accumulate_batched_transaction_costs<'a>( transactions_costs: impl Iterator, transaction_results: impl Iterator>, - ) -> BatchedTransactionCostDetails { - let mut cost_details = BatchedTransactionCostDetails::default(); + ) -> BatchedTransactionDetails { + let mut batched_transaction_details = BatchedTransactionDetails::default(); transactions_costs .zip(transaction_results) - .for_each(|(cost, result)| { - if result.is_ok() { - cost_details.batched_signature_cost = cost_details - .batched_signature_cost - .saturating_add(cost.signature_cost); - cost_details.batched_write_lock_cost = cost_details - .batched_write_lock_cost - .saturating_add(cost.write_lock_cost); - cost_details.batched_data_bytes_cost = cost_details - .batched_data_bytes_cost - .saturating_add(cost.data_bytes_cost); - cost_details.batched_execute_cost = cost_details - .batched_execute_cost - .saturating_add(cost.execution_cost); + .for_each(|(cost, result)| match result { + Ok(_) => { + saturating_add_assign!( + batched_transaction_details.costs.batched_signature_cost, + cost.signature_cost + ); + saturating_add_assign!( + batched_transaction_details.costs.batched_write_lock_cost, + cost.write_lock_cost + ); + saturating_add_assign!( + batched_transaction_details.costs.batched_data_bytes_cost, + cost.data_bytes_cost + ); + saturating_add_assign!( + batched_transaction_details.costs.batched_execute_cost, + cost.execution_cost + ); } + Err(transaction_error) => match transaction_error { + TransactionError::WouldExceedMaxBlockCostLimit => { + saturating_add_assign!( + batched_transaction_details + .errors + .batched_retried_txs_per_block_limit_count, + 1 + ); + } + TransactionError::WouldExceedMaxVoteCostLimit => { + saturating_add_assign!( + batched_transaction_details + .errors + .batched_retried_txs_per_vote_limit_count, + 1 + ); + } + TransactionError::WouldExceedMaxAccountCostLimit => { + saturating_add_assign!( + batched_transaction_details + .errors + .batched_retried_txs_per_account_limit_count, + 1 + ); + } + TransactionError::WouldExceedAccountDataBlockLimit => { + saturating_add_assign!( + batched_transaction_details + .errors + .batched_retried_txs_per_account_data_block_limit_count, + 1 + ); + } + TransactionError::WouldExceedAccountDataTotalLimit => { + saturating_add_assign!( + batched_transaction_details + .errors + .batched_dropped_txs_per_account_data_total_limit_count, + 1 + ); + } + _ => {} + }, }); - cost_details + batched_transaction_details } fn accumulate_execute_units_and_time(execute_timings: &ExecuteTimings) -> (u64, u64) { @@ -4197,12 +4260,24 @@ mod tests { let expected_write_locks = 7; let expected_data_bytes = 9; let expected_executions = 30; - let cost_details = + let batched_transaction_details = BankingStage::accumulate_batched_transaction_costs(tx_costs.iter(), tx_results.iter()); - assert_eq!(expected_signatures, cost_details.batched_signature_cost); - assert_eq!(expected_write_locks, cost_details.batched_write_lock_cost); - assert_eq!(expected_data_bytes, cost_details.batched_data_bytes_cost); - assert_eq!(expected_executions, cost_details.batched_execute_cost); + assert_eq!( + expected_signatures, + batched_transaction_details.costs.batched_signature_cost + ); + assert_eq!( + expected_write_locks, + batched_transaction_details.costs.batched_write_lock_cost + ); + assert_eq!( + expected_data_bytes, + batched_transaction_details.costs.batched_data_bytes_cost + ); + assert_eq!( + expected_executions, + batched_transaction_details.costs.batched_execute_cost + ); } #[test] diff --git a/core/src/qos_service.rs b/core/src/qos_service.rs index 7fabcca262..6e7227c7ec 100644 --- a/core/src/qos_service.rs +++ b/core/src/qos_service.rs @@ -3,7 +3,7 @@ //! how transactions are included in blocks, and optimize those blocks. //! use { - crate::banking_stage::BatchedTransactionCostDetails, + crate::banking_stage::BatchedTransactionDetails, crossbeam_channel::{unbounded, Receiver, Sender}, solana_measure::measure::Measure, solana_runtime::{ @@ -68,8 +68,8 @@ impl QosService { let running_flag = Arc::new(AtomicBool::new(true)); let metrics = Arc::new(QosServiceMetrics::new(id)); - let running_flag_clone = running_flag.clone(); - let metrics_clone = metrics.clone(); + let running_flag_clone = Arc::clone(&running_flag); + let metrics_clone = Arc::clone(&metrics); let reporting_thread = Some( Builder::new() .name("solana-qos-service-metrics-repoting".to_string()) @@ -109,9 +109,11 @@ impl QosService { .collect(); compute_cost_time.stop(); self.metrics + .stats .compute_cost_time .fetch_add(compute_cost_time.as_us(), Ordering::Relaxed); self.metrics + .stats .compute_cost_count .fetch_add(txs_costs.len() as u64, Ordering::Relaxed); txs_costs @@ -134,7 +136,7 @@ impl QosService { .map(|(tx, cost)| match cost_tracker.try_add(tx, cost) { Ok(current_block_cost) => { debug!("slot {:?}, transaction {:?}, cost {:?}, fit into current block, current block cost {}", bank.slot(), tx, cost, current_block_cost); - self.metrics.selected_txs_count.fetch_add(1, Ordering::Relaxed); + self.metrics.stats.selected_txs_count.fetch_add(1, Ordering::Relaxed); num_included += 1; Ok(()) }, @@ -142,20 +144,19 @@ impl QosService { debug!("slot {:?}, transaction {:?}, cost {:?}, not fit into current block, '{:?}'", bank.slot(), tx, cost, e); match e { CostTrackerError::WouldExceedBlockMaxLimit => { - 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) } - CostTrackerError::WouldExceedAccountDataMaxLimit => { - self.metrics.retried_txs_per_account_data_limit_count.fetch_add(1, Ordering::Relaxed); - Err(TransactionError::WouldExceedMaxAccountDataCostLimit) + CostTrackerError::WouldExceedAccountDataBlockLimit => { + Err(TransactionError::WouldExceedAccountDataBlockLimit) + } + CostTrackerError::WouldExceedAccountDataTotalLimit => { + Err(TransactionError::WouldExceedAccountDataTotalLimit) } } } @@ -163,6 +164,7 @@ impl QosService { .collect(); cost_tracking_time.stop(); self.metrics + .stats .cost_tracking_time .fetch_add(cost_tracking_time.as_us(), Ordering::Relaxed); (select_results, num_included) @@ -177,30 +179,82 @@ impl QosService { pub fn accumulate_estimated_transaction_costs( &self, - cost_details: &BatchedTransactionCostDetails, + batched_transaction_details: &BatchedTransactionDetails, ) { + self.metrics.stats.estimated_signature_cu.fetch_add( + batched_transaction_details.costs.batched_signature_cost, + Ordering::Relaxed, + ); + self.metrics.stats.estimated_write_lock_cu.fetch_add( + batched_transaction_details.costs.batched_write_lock_cost, + Ordering::Relaxed, + ); + self.metrics.stats.estimated_data_bytes_cu.fetch_add( + batched_transaction_details.costs.batched_data_bytes_cost, + Ordering::Relaxed, + ); + self.metrics.stats.estimated_execute_cu.fetch_add( + batched_transaction_details.costs.batched_execute_cost, + Ordering::Relaxed, + ); + self.metrics - .estimated_signature_cu - .fetch_add(cost_details.batched_signature_cost, Ordering::Relaxed); + .errors + .retried_txs_per_block_limit_count + .fetch_add( + batched_transaction_details + .errors + .batched_retried_txs_per_block_limit_count, + Ordering::Relaxed, + ); self.metrics - .estimated_write_lock_cu - .fetch_add(cost_details.batched_write_lock_cost, Ordering::Relaxed); + .errors + .retried_txs_per_vote_limit_count + .fetch_add( + batched_transaction_details + .errors + .batched_retried_txs_per_vote_limit_count, + Ordering::Relaxed, + ); self.metrics - .estimated_data_bytes_cu - .fetch_add(cost_details.batched_data_bytes_cost, Ordering::Relaxed); + .errors + .retried_txs_per_account_limit_count + .fetch_add( + batched_transaction_details + .errors + .batched_retried_txs_per_account_limit_count, + Ordering::Relaxed, + ); self.metrics - .estimated_execute_cu - .fetch_add(cost_details.batched_execute_cost, Ordering::Relaxed); + .errors + .retried_txs_per_account_data_block_limit_count + .fetch_add( + batched_transaction_details + .errors + .batched_retried_txs_per_account_data_block_limit_count, + Ordering::Relaxed, + ); + self.metrics + .errors + .dropped_txs_per_account_data_total_limit_count + .fetch_add( + batched_transaction_details + .errors + .batched_dropped_txs_per_account_data_total_limit_count, + Ordering::Relaxed, + ); } pub fn accumulate_actual_execute_cu(&self, units: u64) { self.metrics + .stats .actual_execute_cu .fetch_add(units, Ordering::Relaxed); } pub fn accumulate_actual_execute_time(&self, micro_sec: u64) { self.metrics + .stats .actual_execute_time_us .fetch_add(micro_sec, Ordering::Relaxed); } @@ -223,63 +277,77 @@ impl QosService { } } -#[derive(Default)] +#[derive(Debug, Default)] struct QosServiceMetrics { - // banking_stage creates one QosService instance per working threads, that is uniquely - // identified by id. This field allows to categorize metrics for gossip votes, TPU votes - // and other transactions. + /// banking_stage creates one QosService instance per working threads, that is uniquely + /// identified by id. This field allows to categorize metrics for gossip votes, TPU votes + /// and other transactions. id: u32, - // aggregate metrics per slot + /// aggregate metrics per slot slot: AtomicU64, - // accumulated time in micro-sec spent in computing transaction cost. It is the main performance - // overhead introduced by cost_model + stats: QosServiceMetricsStats, + errors: QosServiceMetricsErrors, +} + +#[derive(Debug, Default)] +struct QosServiceMetricsStats { + /// accumulated time in micro-sec spent in computing transaction cost. It is the main performance + /// overhead introduced by cost_model compute_cost_time: AtomicU64, - // total nummber of transactions in the reporting period to be computed for theit cost. It is - // usually the number of sanitized transactions leader receives. + /// total nummber of transactions in the reporting period to be computed for theit cost. It is + /// usually the number of sanitized transactions leader receives. compute_cost_count: AtomicU64, - // acumulated time in micro-sec spent in tracking each bank's cost. It is the second part of - // overhead introduced + /// acumulated time in micro-sec spent in tracking each bank's cost. It is the second part of + /// overhead introduced cost_tracking_time: AtomicU64, - // number of transactions to be included in blocks + /// number of transactions to be included in blocks selected_txs_count: AtomicU64, - // 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, - - // number of transactions to be queued for retry due to its account data limits - retried_txs_per_account_data_limit_count: AtomicU64, - - // accumulated estimated signature Compute Unites to be packed into block + /// accumulated estimated signature Compute Unites to be packed into block estimated_signature_cu: AtomicU64, - // accumulated estimated write locks Compute Units to be packed into block + /// accumulated estimated write locks Compute Units to be packed into block estimated_write_lock_cu: AtomicU64, - // accumulated estimated instructino data Compute Units to be packed into block + /// accumulated estimated instructino data Compute Units to be packed into block estimated_data_bytes_cu: AtomicU64, - // accumulated estimated program Compute Units to be packed into block + /// accumulated estimated program Compute Units to be packed into block estimated_execute_cu: AtomicU64, - // accumulated actual program Compute Units that have been packed into block + /// accumulated actual program Compute Units that have been packed into block actual_execute_cu: AtomicU64, - // accumulated actual program execute micro-sec that have been packed into block + /// accumulated actual program execute micro-sec that have been packed into block actual_execute_time_us: AtomicU64, } +#[derive(Debug, Default)] +struct QosServiceMetricsErrors { + /// number of transactions to be queued for retry due to their potential to breach block limit + retried_txs_per_block_limit_count: AtomicU64, + + /// number of transactions to be queued for retry due to their potential to breach vote limit + retried_txs_per_vote_limit_count: AtomicU64, + + /// number of transactions to be queued for retry due to their potential to breach writable + /// account limit + retried_txs_per_account_limit_count: AtomicU64, + + /// number of transactions to be queued for retry due to their potential to breach account data + /// block limits + retried_txs_per_account_data_block_limit_count: AtomicU64, + + /// number of transactions to be dropped due to their potential to breach account data total + /// limits + dropped_txs_per_account_data_total_limit_count: AtomicU64, +} + impl QosServiceMetrics { pub fn new(id: u32) -> Self { QosServiceMetrics { @@ -296,76 +364,96 @@ impl QosServiceMetrics { ("bank_slot", bank_slot as i64, i64), ( "compute_cost_time", - self.compute_cost_time.swap(0, Ordering::Relaxed) as i64, + self.stats.compute_cost_time.swap(0, Ordering::Relaxed) as i64, i64 ), ( "compute_cost_count", - self.compute_cost_count.swap(0, Ordering::Relaxed) as i64, + self.stats.compute_cost_count.swap(0, Ordering::Relaxed) as i64, i64 ), ( "cost_tracking_time", - self.cost_tracking_time.swap(0, Ordering::Relaxed) as i64, + self.stats.cost_tracking_time.swap(0, Ordering::Relaxed) as i64, i64 ), ( "selected_txs_count", - self.selected_txs_count.swap(0, Ordering::Relaxed) as i64, + self.stats.selected_txs_count.swap(0, Ordering::Relaxed) as i64, i64 ), + ( + "estimated_signature_cu", + self.stats.estimated_signature_cu.swap(0, Ordering::Relaxed) as i64, + i64 + ), + ( + "estimated_write_lock_cu", + self.stats + .estimated_write_lock_cu + .swap(0, Ordering::Relaxed) as i64, + i64 + ), + ( + "estimated_data_bytes_cu", + self.stats + .estimated_data_bytes_cu + .swap(0, Ordering::Relaxed) as i64, + i64 + ), + ( + "estimated_execute_cu", + self.stats.estimated_execute_cu.swap(0, Ordering::Relaxed) as i64, + i64 + ), + ( + "actual_execute_cu", + self.stats.actual_execute_cu.swap(0, Ordering::Relaxed) as i64, + i64 + ), + ( + "actual_execute_time_us", + self.stats.actual_execute_time_us.swap(0, Ordering::Relaxed) as i64, + i64 + ), + ); + datapoint_info!( + "qos-service-errors", + ("id", self.id as i64, i64), + ("bank_slot", bank_slot as i64, i64), ( "retried_txs_per_block_limit_count", - self.retried_txs_per_block_limit_count + self.errors + .retried_txs_per_block_limit_count .swap(0, Ordering::Relaxed) as i64, i64 ), ( "retried_txs_per_vote_limit_count", - self.retried_txs_per_vote_limit_count + self.errors + .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 + self.errors + .retried_txs_per_account_limit_count .swap(0, Ordering::Relaxed) as i64, i64 ), ( - "retried_txs_per_account_data_limit_count", - self.retried_txs_per_account_data_limit_count + "retried_txs_per_account_data_block_limit_count", + self.errors + .retried_txs_per_account_data_block_limit_count .swap(0, Ordering::Relaxed) as i64, i64 ), ( - "estimated_signature_cu", - self.estimated_signature_cu.swap(0, Ordering::Relaxed) as i64, - i64 - ), - ( - "estimated_write_lock_cu", - self.estimated_write_lock_cu.swap(0, Ordering::Relaxed) as i64, - i64 - ), - ( - "estimated_data_bytes_cu", - self.estimated_data_bytes_cu.swap(0, Ordering::Relaxed) as i64, - i64 - ), - ( - "estimated_execute_cu", - self.estimated_execute_cu.swap(0, Ordering::Relaxed) as i64, - i64 - ), - ( - "actual_execute_cu", - self.actual_execute_cu.swap(0, Ordering::Relaxed) as i64, - i64 - ), - ( - "actual_execute_time_us", - self.actual_execute_time_us.swap(0, Ordering::Relaxed) as i64, + "dropped_txs_per_account_data_total_limit_count", + self.errors + .dropped_txs_per_account_data_total_limit_count + .swap(0, Ordering::Relaxed) as i64, i64 ), ); diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index f384e69077..030d7e677b 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -1108,7 +1108,8 @@ impl Accounts { | Err(TransactionError::WouldExceedMaxBlockCostLimit) | Err(TransactionError::WouldExceedMaxVoteCostLimit) | Err(TransactionError::WouldExceedMaxAccountCostLimit) - | Err(TransactionError::WouldExceedMaxAccountDataCostLimit) => None, + | Err(TransactionError::WouldExceedAccountDataBlockLimit) + | Err(TransactionError::WouldExceedAccountDataTotalLimit) => None, _ => Some(tx.get_account_locks_unchecked()), }) .collect(); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 4b122a0bea..a3eb3812ba 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -75,6 +75,7 @@ use { solana_measure::measure::Measure, solana_metrics::{inc_new_counter_debug, inc_new_counter_info}, solana_program_runtime::{ + accounts_data_meter::MAX_ACCOUNTS_DATA_LEN, compute_budget::ComputeBudget, invoke_context::{ BuiltinProgram, Executor, Executors, ProcessInstructionWithContext, TransactionExecutor, @@ -215,7 +216,7 @@ impl RentDebits { } type BankStatusCache = StatusCache>; -#[frozen_abi(digest = "HdYCU65Jwfv9sF3C8k6ZmjUAaXSkJwazebuur21v8JtY")] +#[frozen_abi(digest = "BQcJmh4VRCiNNtqjKPyphs9ULFbSUKGGfx6hz9SWBtqU")] pub type BankSlotDelta = SlotDelta>; // Eager rent collection repeats in cyclic manner. @@ -1220,7 +1221,7 @@ pub struct Bank { vote_only_bank: bool, - pub cost_tracker: RwLock, + cost_tracker: RwLock, sysvar_cache: RwLock, @@ -1374,8 +1375,17 @@ impl Bank { fee_structure: FeeStructure::default(), }; - let total_accounts_stats = bank.get_total_accounts_stats().unwrap(); - bank.store_accounts_data_len(total_accounts_stats.data_len as u64); + let accounts_data_len = bank.get_total_accounts_stats().unwrap().data_len as u64; + if accounts_data_len != 0 { + bank.store_accounts_data_len(accounts_data_len); + + let cost_tracker = CostTracker::new_with_account_data_size_limit( + bank.feature_set + .is_active(&feature_set::cap_accounts_data_len::id()) + .then(|| MAX_ACCOUNTS_DATA_LEN.saturating_sub(accounts_data_len)), + ); + *bank.write_cost_tracker().unwrap() = cost_tracker; + } bank } @@ -1630,6 +1640,7 @@ impl Bank { let (feature_set, feature_set_time) = Measure::this(|_| parent.feature_set.clone(), (), "feature_set_creation"); + let accounts_data_len = parent.load_accounts_data_len(); let mut new = Bank { rc, src, @@ -1682,7 +1693,7 @@ impl Bank { transaction_debug_keys, transaction_log_collector_config, transaction_log_collector: Arc::new(RwLock::new(TransactionLogCollector::default())), - feature_set, + feature_set: Arc::clone(&feature_set), drop_callback: RwLock::new(OptionalDropCallback( parent .drop_callback @@ -1693,9 +1704,13 @@ impl Bank { .map(|drop_callback| drop_callback.clone_box()), )), freeze_started: AtomicBool::new(false), - cost_tracker: RwLock::new(CostTracker::default()), + cost_tracker: RwLock::new(CostTracker::new_with_account_data_size_limit( + feature_set + .is_active(&feature_set::cap_accounts_data_len::id()) + .then(|| MAX_ACCOUNTS_DATA_LEN.saturating_sub(accounts_data_len)), + )), sysvar_cache: RwLock::new(SysvarCache::default()), - accounts_data_len: AtomicU64::new(parent.load_accounts_data_len()), + accounts_data_len: AtomicU64::new(accounts_data_len), fee_structure: parent.fee_structure.clone(), }; @@ -1926,6 +1941,7 @@ impl Bank { fn new() -> T { T::default() } + let feature_set = new(); let mut bank = Self { rc: bank_rc, src: new(), @@ -1978,11 +1994,15 @@ impl Bank { transaction_debug_keys: debug_keys, transaction_log_collector_config: new(), transaction_log_collector: new(), - feature_set: new(), + feature_set: Arc::clone(&feature_set), drop_callback: RwLock::new(OptionalDropCallback(None)), freeze_started: AtomicBool::new(fields.hash != Hash::default()), vote_only_bank: false, - cost_tracker: RwLock::new(CostTracker::default()), + cost_tracker: RwLock::new(CostTracker::new_with_account_data_size_limit( + feature_set + .is_active(&feature_set::cap_accounts_data_len::id()) + .then(|| MAX_ACCOUNTS_DATA_LEN.saturating_sub(accounts_data_len)), + )), sysvar_cache: RwLock::new(SysvarCache::default()), accounts_data_len: AtomicU64::new(accounts_data_len), fee_structure: FeeStructure::default(), @@ -3971,7 +3991,7 @@ impl Bank { Err(TransactionError::WouldExceedMaxBlockCostLimit) | Err(TransactionError::WouldExceedMaxVoteCostLimit) | Err(TransactionError::WouldExceedMaxAccountCostLimit) - | Err(TransactionError::WouldExceedMaxAccountDataCostLimit) => Some(index), + | Err(TransactionError::WouldExceedAccountDataBlockLimit) => Some(index), Err(_) => None, Ok(_) => None, }) diff --git a/runtime/src/block_cost_limits.rs b/runtime/src/block_cost_limits.rs index c54ce3702f..a1f1db85dc 100644 --- a/runtime/src/block_cost_limits.rs +++ b/runtime/src/block_cost_limits.rs @@ -63,5 +63,5 @@ pub const MAX_WRITABLE_ACCOUNT_UNITS: u64 = MAX_BLOCK_REPLAY_TIME_US * COMPUTE_U /// 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; +/// max length of account data in a block (bytes) +pub const MAX_ACCOUNT_DATA_BLOCK_LEN: u64 = 100_000_000; diff --git a/runtime/src/cost_tracker.rs b/runtime/src/cost_tracker.rs index feb55523ca..2d8ed92efb 100644 --- a/runtime/src/cost_tracker.rs +++ b/runtime/src/cost_tracker.rs @@ -11,7 +11,7 @@ use { const WRITABLE_ACCOUNTS_PER_BLOCK: usize = 512; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum CostTrackerError { /// would exceed block max limit WouldExceedBlockMaxLimit, @@ -22,7 +22,11 @@ pub enum CostTrackerError { /// would exceed account max limit WouldExceedAccountMaxLimit, - WouldExceedAccountDataMaxLimit, + /// would exceed account data block limit + WouldExceedAccountDataBlockLimit, + + /// would exceed account data total limit + WouldExceedAccountDataTotalLimit, } #[derive(AbiExample, Debug)] @@ -35,6 +39,10 @@ pub struct CostTracker { vote_cost: u64, transaction_count: u64, account_data_size: u64, + + /// The amount of total account data size remaining. If `Some`, then do not add transactions + /// that would cause `account_data_size` to exceed this limit. + account_data_size_limit: Option, } impl Default for CostTracker { @@ -54,11 +62,21 @@ impl Default for CostTracker { vote_cost: 0, transaction_count: 0, account_data_size: 0, + account_data_size_limit: None, } } } impl CostTracker { + /// Construct and new CostTracker and set the account data size limit. + #[must_use] + pub fn new_with_account_data_size_limit(account_data_size_limit: Option) -> Self { + Self { + account_data_size_limit, + ..Self::default() + } + } + // bench tests needs to reset limits pub fn set_limits( &mut self, @@ -180,8 +198,17 @@ impl CostTracker { return Err(CostTrackerError::WouldExceedAccountMaxLimit); } - if self.account_data_size.saturating_add(account_data_len) > MAX_ACCOUNT_DATA_LEN { - return Err(CostTrackerError::WouldExceedAccountDataMaxLimit); + // NOTE: Check if the total accounts data size is exceeded *before* the block accounts data + // size. This way, transactions are not unnecessarily retried. + let account_data_size = self.account_data_size.saturating_add(account_data_len); + if let Some(account_data_size_limit) = self.account_data_size_limit { + if account_data_size > account_data_size_limit { + return Err(CostTrackerError::WouldExceedAccountDataTotalLimit); + } + } + + if account_data_size > MAX_ACCOUNT_DATA_BLOCK_LEN { + return Err(CostTrackerError::WouldExceedAccountDataBlockLimit); } // check each account against account_cost_limit, @@ -243,13 +270,19 @@ mod tests { }; impl CostTracker { - fn new(account_cost_limit: u64, block_cost_limit: u64, vote_cost_limit: u64) -> Self { + fn new( + account_cost_limit: u64, + block_cost_limit: u64, + vote_cost_limit: u64, + account_data_size_limit: Option, + ) -> 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, + account_data_size_limit, ..Self::default() } } @@ -306,7 +339,7 @@ mod tests { #[test] fn test_cost_tracker_initialization() { - let testee = CostTracker::new(10, 11, 8); + let testee = CostTracker::new(10, 11, 8, None); assert_eq!(10, testee.account_cost_limit); assert_eq!(11, testee.block_cost_limit); assert_eq!(8, testee.vote_cost_limit); @@ -320,7 +353,7 @@ 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); + let mut testee = CostTracker::new(cost, cost, cost, None); assert!(testee.would_fit(&keys, cost, 0, &tx).is_ok()); testee.add_transaction(&keys, cost, 0, &tx); assert_eq!(cost, testee.block_cost); @@ -335,7 +368,7 @@ mod tests { 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); + let mut testee = CostTracker::new(cost, cost, cost, None); assert!(testee.would_fit(&keys, cost, 0, &tx).is_ok()); testee.add_transaction(&keys, cost, 0, &tx); assert_eq!(cost, testee.block_cost); @@ -350,7 +383,7 @@ 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); + let mut testee = CostTracker::new(cost, cost, cost, None); assert!(testee.would_fit(&keys, cost, 0, &tx).is_ok()); let old = testee.account_data_size; testee.add_transaction(&keys, cost, 1, &tx); @@ -365,7 +398,7 @@ 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, cost1 + cost2, None); { assert!(testee.would_fit(&keys1, cost1, 0, &tx1).is_ok()); testee.add_transaction(&keys1, cost1, 0, &tx1); @@ -389,7 +422,8 @@ 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, cost1 + cost2, None); { assert!(testee.would_fit(&keys1, cost1, 0, &tx1).is_ok()); testee.add_transaction(&keys1, cost1, 0, &tx1); @@ -412,7 +446,8 @@ 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, cost1 + cost2, None); // should have room for first transaction { assert!(testee.would_fit(&keys1, cost1, 0, &tx1).is_ok()); @@ -433,8 +468,12 @@ 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, + cost1 + cost2 - 1, + None, + ); // should have room for first transaction { assert!(testee.would_fit(&keys1, cost1, 0, &tx1).is_ok()); @@ -455,7 +494,12 @@ mod tests { 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); + let mut testee = CostTracker::new( + cmp::max(cost1, cost2), + cost1 + cost2, + cost1 + cost2 - 1, + None, + ); // should have room for first vote { assert!(testee.would_fit(&keys1, cost1, 0, &tx1).is_ok()); @@ -474,7 +518,7 @@ mod tests { } #[test] - fn test_cost_tracker_reach_data_limit() { + fn test_cost_tracker_reach_data_block_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); @@ -482,14 +526,46 @@ mod tests { 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, cost1 + cost2 - 1); + let testee = CostTracker::new( + cmp::max(cost1, cost2), + cost1 + cost2 - 1, + cost1 + cost2 - 1, + None, + ); assert!(testee - .would_fit(&keys1, cost1, MAX_ACCOUNT_DATA_LEN, &tx1) + .would_fit(&keys1, cost1, MAX_ACCOUNT_DATA_BLOCK_LEN, &tx1) .is_ok()); // data is too big + assert_eq!( + testee.would_fit(&keys2, cost2, MAX_ACCOUNT_DATA_BLOCK_LEN + 1, &tx2), + Err(CostTrackerError::WouldExceedAccountDataBlockLimit), + ); + } + + #[test] + fn test_cost_tracker_reach_data_total_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 second_account = Keypair::new(); + let (tx2, keys2, cost2) = build_simple_transaction(&second_account, &start_hash); + + // build testee that passes + let remaining_account_data_size = 1234; + let testee = CostTracker::new( + cmp::max(cost1, cost2), + cost1 + cost2 - 1, + cost1 + cost2 - 1, + Some(remaining_account_data_size), + ); assert!(testee - .would_fit(&keys2, cost2, MAX_ACCOUNT_DATA_LEN + 1, &tx2) - .is_err()); + .would_fit(&keys1, cost1, remaining_account_data_size, &tx1) + .is_ok()); + // data is too big + assert_eq!( + testee.would_fit(&keys2, cost2, remaining_account_data_size + 1, &tx2), + Err(CostTrackerError::WouldExceedAccountDataTotalLimit), + ); } #[test] @@ -505,7 +581,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, block_max, None); // case 1: a tx writes to 3 accounts, should success, we will have: // | acct1 | $cost | diff --git a/sdk/src/transaction/error.rs b/sdk/src/transaction/error.rs index 44e92a4861..f719e894fb 100644 --- a/sdk/src/transaction/error.rs +++ b/sdk/src/transaction/error.rs @@ -102,9 +102,9 @@ pub enum TransactionError { #[error("Transaction would exceed max account limit within the block")] WouldExceedMaxAccountCostLimit, - /// Transaction would exceed max account data limit within the block - #[error("Transaction would exceed max account data limit within the block")] - WouldExceedMaxAccountDataCostLimit, + /// Transaction would exceed account data limit within the block + #[error("Transaction would exceed account data limit within the block")] + WouldExceedAccountDataBlockLimit, /// Transaction locked too many accounts #[error("Transaction locked too many accounts")] @@ -135,6 +135,10 @@ pub enum TransactionError { /// Transaction would exceed max Vote Cost Limit #[error("Transaction would exceed max Vote Cost Limit")] WouldExceedMaxVoteCostLimit, + + /// Transaction would exceed total account data limit + #[error("Transaction would exceed total account data limit")] + WouldExceedAccountDataTotalLimit, } impl From for TransactionError { diff --git a/storage-proto/proto/transaction_by_addr.proto b/storage-proto/proto/transaction_by_addr.proto index cd2bdd55ec..f489690686 100644 --- a/storage-proto/proto/transaction_by_addr.proto +++ b/storage-proto/proto/transaction_by_addr.proto @@ -45,7 +45,7 @@ enum TransactionErrorType { UNSUPPORTED_VERSION = 18; INVALID_WRITABLE_ACCOUNT = 19; WOULD_EXCEED_MAX_ACCOUNT_COST_LIMIT = 20; - WOULD_EXCEED_MAX_ACCOUNT_DATA_COST_LIMIT = 21; + WOULD_EXCEED_ACCOUNT_DATA_BLOCK_LIMIT = 21; TOO_MANY_ACCOUNT_LOCKS = 22; ADDRESS_LOOKUP_TABLE_NOT_FOUND = 23; INVALID_ADDRESS_LOOKUP_TABLE_OWNER = 24; @@ -53,6 +53,7 @@ enum TransactionErrorType { INVALID_ADDRESS_LOOKUP_TABLE_INDEX = 26; INVALID_RENT_PAYING_ACCOUNT = 27; WOULD_EXCEED_MAX_VOTE_COST_LIMIT = 28; + WOULD_EXCEED_ACCOUNT_DATA_TOTAL_LIMIT = 29; } message InstructionError { diff --git a/storage-proto/src/convert.rs b/storage-proto/src/convert.rs index 6f682e789f..fadf65f184 100644 --- a/storage-proto/src/convert.rs +++ b/storage-proto/src/convert.rs @@ -703,7 +703,7 @@ impl TryFrom for TransactionError { 18 => TransactionError::UnsupportedVersion, 19 => TransactionError::InvalidWritableAccount, 20 => TransactionError::WouldExceedMaxAccountCostLimit, - 21 => TransactionError::WouldExceedMaxAccountDataCostLimit, + 21 => TransactionError::WouldExceedAccountDataBlockLimit, 22 => TransactionError::TooManyAccountLocks, 23 => TransactionError::AddressLookupTableNotFound, 24 => TransactionError::InvalidAddressLookupTableOwner, @@ -711,6 +711,7 @@ impl TryFrom for TransactionError { 26 => TransactionError::InvalidAddressLookupTableIndex, 27 => TransactionError::InvalidRentPayingAccount, 28 => TransactionError::WouldExceedMaxVoteCostLimit, + 29 => TransactionError::WouldExceedAccountDataTotalLimit, _ => return Err("Invalid TransactionError"), }) } @@ -781,8 +782,8 @@ impl From for tx_by_addr::TransactionError { TransactionError::WouldExceedMaxAccountCostLimit => { tx_by_addr::TransactionErrorType::WouldExceedMaxAccountCostLimit } - TransactionError::WouldExceedMaxAccountDataCostLimit => { - tx_by_addr::TransactionErrorType::WouldExceedMaxAccountDataCostLimit + TransactionError::WouldExceedAccountDataBlockLimit => { + tx_by_addr::TransactionErrorType::WouldExceedAccountDataBlockLimit } TransactionError::TooManyAccountLocks => { tx_by_addr::TransactionErrorType::TooManyAccountLocks @@ -805,6 +806,9 @@ impl From for tx_by_addr::TransactionError { TransactionError::WouldExceedMaxVoteCostLimit => { tx_by_addr::TransactionErrorType::WouldExceedMaxVoteCostLimit } + TransactionError::WouldExceedAccountDataTotalLimit => { + tx_by_addr::TransactionErrorType::WouldExceedAccountDataTotalLimit + } } as i32, instruction_error: match transaction_error { TransactionError::InstructionError(index, ref instruction_error) => {