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 5aa7e2785..f48b2456c 100644 --- a/accountsdb-plugin-postgres/src/postgres_client/postgres_client_transaction.rs +++ b/accountsdb-plugin-postgres/src/postgres_client/postgres_client_transaction.rs @@ -330,6 +330,7 @@ pub enum DbTransactionErrorCode { WouldExceedMaxBlockCostLimit, UnsupportedVersion, InvalidWritableAccount, + WouldExceedMaxAccountDataCostLimit, } impl From<&TransactionError> for DbTransactionErrorCode { @@ -358,6 +359,9 @@ impl From<&TransactionError> for DbTransactionErrorCode { TransactionError::WouldExceedMaxBlockCostLimit => Self::WouldExceedMaxBlockCostLimit, TransactionError::UnsupportedVersion => Self::UnsupportedVersion, TransactionError::InvalidWritableAccount => Self::InvalidWritableAccount, + TransactionError::WouldExceedMaxAccountDataCostLimit => { + Self::WouldExceedMaxAccountDataCostLimit + } } } } diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs index 3328c5c14..15898d4ff 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -949,8 +949,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 + // WouldExceedMaxAccountCostLimit, 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 435ed5644..de50031ce 100644 --- a/core/src/qos_service.rs +++ b/core/src/qos_service.rs @@ -133,6 +133,10 @@ impl QosService { 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) + } } } }) @@ -165,6 +169,7 @@ struct QosServiceMetrics { selected_txs_count: AtomicU64, retried_txs_per_block_limit_count: AtomicU64, retried_txs_per_account_limit_count: AtomicU64, + retried_txs_per_account_data_limit_count: AtomicU64, } impl QosServiceMetrics { @@ -204,6 +209,12 @@ impl QosServiceMetrics { .swap(0, Ordering::Relaxed) as i64, i64 ), + ( + "retried_txs_per_account_data_limit_count", + self.retried_txs_per_account_data_limit_count + .swap(0, Ordering::Relaxed) as i64, + i64 + ), ); } } diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 514a97fef..1370d415e 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -1032,11 +1032,12 @@ impl Accounts { let keys: Vec<_> = txs .zip(results) .filter_map(|(tx, res)| match res { - Err(TransactionError::AccountInUse) => None, - Err(TransactionError::SanitizeFailure) => None, - Err(TransactionError::AccountLoadedTwice) => None, - Err(TransactionError::WouldExceedMaxBlockCostLimit) => None, - Err(TransactionError::WouldExceedMaxAccountCostLimit) => None, + Err(TransactionError::AccountInUse) + | Err(TransactionError::SanitizeFailure) + | Err(TransactionError::AccountLoadedTwice) + | Err(TransactionError::WouldExceedMaxBlockCostLimit) + | Err(TransactionError::WouldExceedMaxAccountCostLimit) + | Err(TransactionError::WouldExceedMaxAccountDataCostLimit) => None, _ => Some(tx.get_account_locks(demote_program_write_locks)), }) .collect(); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 36886dcf3..5beef12b9 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -240,7 +240,7 @@ impl ExecuteTimings { } type BankStatusCache = StatusCache>; -#[frozen_abi(digest = "32EjVUc6shHHVPpsnBAVfyBziMgyFzH8qxisLwmwwdS1")] +#[frozen_abi(digest = "GcfJc94Hb3s7gzF7Uh4YxLSiQf1MvUtMmtU45BvinkVT")] pub type BankSlotDelta = SlotDelta>; // Eager rent collection repeats in cyclic manner. @@ -3525,7 +3525,8 @@ impl Bank { Some(index) } Err(TransactionError::WouldExceedMaxBlockCostLimit) - | Err(TransactionError::WouldExceedMaxAccountCostLimit) => Some(index), + | Err(TransactionError::WouldExceedMaxAccountCostLimit) + | Err(TransactionError::WouldExceedMaxAccountDataCostLimit) => Some(index), Err(_) => None, Ok(_) => None, }) diff --git a/runtime/src/block_cost_limits.rs b/runtime/src/block_cost_limits.rs index 97be8a062..4c6846d63 100644 --- a/runtime/src/block_cost_limits.rs +++ b/runtime/src/block_cost_limits.rs @@ -58,3 +58,6 @@ pub const MAX_BLOCK_UNITS: u64 = /// limit is to prevent too many transactions write to same account, threrefore /// reduce block's paralellism. pub const MAX_WRITABLE_ACCOUNT_UNITS: u64 = MAX_BLOCK_REPLAY_TIME_US * COMPUTE_UNIT_TO_US_RATIO; + +/// max len 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 a69b4c9f4..ab74f9de5 100644 --- a/runtime/src/cost_model.rs +++ b/runtime/src/cost_model.rs @@ -10,7 +10,10 @@ use { execute_cost_table::ExecuteCostTable, }, log::*, - solana_sdk::{pubkey::Pubkey, transaction::SanitizedTransaction}, + solana_sdk::{ + instruction::CompiledInstruction, program_utils::limited_deserialize, pubkey::Pubkey, + system_instruction::SystemInstruction, system_program, transaction::SanitizedTransaction, + }, std::collections::HashMap, }; @@ -27,6 +30,7 @@ pub struct TransactionCost { // `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, } impl Default for TransactionCost { @@ -38,6 +42,7 @@ impl Default for TransactionCost { data_bytes_cost: 0u64, execution_cost: 0u64, cost_weight: 1u32, + account_data_size: 0u64, } } } @@ -118,6 +123,7 @@ impl CostModel { 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); tx_cost @@ -201,6 +207,59 @@ impl CostModel { } } + fn calculate_account_data_size_on_deserialized_system_instruction( + instruction: SystemInstruction, + ) -> u64 { + match instruction { + SystemInstruction::CreateAccount { + lamports: _lamports, + space, + owner: _owner, + } => space, + SystemInstruction::CreateAccountWithSeed { + base: _base, + seed: _seed, + lamports: _lamports, + space, + owner: _owner, + } => space, + SystemInstruction::Allocate { space } => space, + SystemInstruction::AllocateWithSeed { + base: _base, + seed: _seed, + space, + owner: _owner, + } => space, + _ => 0, + } + } + + fn calculate_account_data_size_on_instruction( + program_id: &Pubkey, + instruction: &CompiledInstruction, + ) -> u64 { + if program_id == &system_program::id() { + if let Ok(instruction) = limited_deserialize(&instruction.data) { + return Self::calculate_account_data_size_on_deserialized_system_instruction( + instruction, + ); + } + } + 0 + } + + /// eventually, potentially determine account data size of all writable accounts + /// at the moment, calculate account data size of account creation + fn calculate_account_data_size(&self, transaction: &SanitizedTransaction) -> u64 { + transaction + .message() + .program_instructions_iter() + .map(|(program_id, instruction)| { + Self::calculate_account_data_size_on_instruction(program_id, instruction) + }) + .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 @@ -272,6 +331,53 @@ mod tests { ); } + #[test] + fn test_cost_model_data_len_cost() { + let lamports = 0; + let owner = Pubkey::default(); + let seed = String::default(); + let space = 100; + let base = Pubkey::default(); + for instruction in [ + SystemInstruction::CreateAccount { + lamports, + space, + owner, + }, + SystemInstruction::CreateAccountWithSeed { + base, + seed: seed.clone(), + lamports, + space, + owner, + }, + SystemInstruction::Allocate { space }, + SystemInstruction::AllocateWithSeed { + base, + seed, + space, + owner, + }, + ] { + assert_eq!( + space, + CostModel::calculate_account_data_size_on_deserialized_system_instruction( + instruction + ) + ); + } + assert_eq!( + 0, + CostModel::calculate_account_data_size_on_deserialized_system_instruction( + SystemInstruction::TransferWithSeed { + lamports, + from_seed: String::default(), + from_owner: Pubkey::default(), + } + ) + ); + } + #[test] fn test_cost_model_simple_transaction() { let (mint_keypair, start_hash) = test_setup(); diff --git a/runtime/src/cost_tracker.rs b/runtime/src/cost_tracker.rs index 4ad4525fe..d682b4119 100644 --- a/runtime/src/cost_tracker.rs +++ b/runtime/src/cost_tracker.rs @@ -18,6 +18,8 @@ pub enum CostTrackerError { /// would exceed account max limit WouldExceedAccountMaxLimit, + + WouldExceedAccountDataMaxLimit, } #[derive(AbiExample, Debug)] @@ -27,6 +29,7 @@ pub struct CostTracker { cost_by_writable_accounts: HashMap, block_cost: u64, transaction_count: u64, + account_data_size: u64, } impl Default for CostTracker { @@ -44,6 +47,7 @@ impl CostTracker { cost_by_writable_accounts: HashMap::with_capacity(WRITABLE_ACCOUNTS_PER_BLOCK), block_cost: 0, transaction_count: 0, + account_data_size: 0, } } @@ -58,7 +62,11 @@ impl CostTracker { _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(), + tx_cost.account_data_size, + ) } pub fn add_transaction_cost( @@ -66,7 +74,11 @@ impl CostTracker { _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(), + tx_cost.account_data_size, + ); } pub fn try_add( @@ -75,8 +87,8 @@ impl CostTracker { 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); + 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); Ok(self.block_cost) } @@ -100,6 +112,7 @@ impl CostTracker { ), ("costliest_account", costliest_account.to_string(), String), ("costliest_account_cost", costliest_account_cost as i64, i64), + ("account_data_size", self.account_data_size, i64), ); } @@ -116,17 +129,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, + account_data_len: u64, + ) -> Result<(), CostTrackerError> { // check against the total package cost if self.block_cost + cost > self.block_cost_limit { return Err(CostTrackerError::WouldExceedBlockMaxLimit); } // 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); } + if self.account_data_size.saturating_add(account_data_len) > MAX_ACCOUNT_DATA_LEN { + return Err(CostTrackerError::WouldExceedAccountDataMaxLimit); + } + // check each account against account_cost_limit, for account_key in keys.iter() { match self.cost_by_writable_accounts.get(account_key) { @@ -144,7 +166,7 @@ impl CostTracker { Ok(()) } - fn add_transaction(&mut self, keys: &[Pubkey], cost: &u64) { + fn add_transaction(&mut self, keys: &[Pubkey], cost: u64, account_data_size: u64) { for account_key in keys.iter() { *self .cost_by_writable_accounts @@ -153,6 +175,7 @@ impl CostTracker { } self.block_cost += cost; self.transaction_count += 1; + self.account_data_size = self.account_data_size.saturating_add(account_data_size); } } @@ -212,11 +235,24 @@ mod tests { // 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); + assert!(testee.would_fit(&keys, cost, 0).is_ok()); + testee.add_transaction(&keys, cost, 0); assert_eq!(cost, testee.block_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); + + // 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 old = testee.account_data_size; + testee.add_transaction(&keys, cost, 1); + assert_eq!(old + 1, testee.account_data_size); + } + #[test] fn test_cost_tracker_ok_add_two_same_accounts() { let (mint_keypair, start_hash) = test_setup(); @@ -227,12 +263,12 @@ mod tests { // build testee to have capacity for two simple transactions, with same accounts let mut testee = CostTracker::new(cost1 + cost2, cost1 + cost2); { - assert!(testee.would_fit(&keys1, &cost1).is_ok()); - testee.add_transaction(&keys1, &cost1); + assert!(testee.would_fit(&keys1, cost1, 0).is_ok()); + testee.add_transaction(&keys1, cost1, 0); } { - assert!(testee.would_fit(&keys2, &cost2).is_ok()); - testee.add_transaction(&keys2, &cost2); + assert!(testee.would_fit(&keys2, cost2, 0).is_ok()); + testee.add_transaction(&keys2, cost2, 0); } assert_eq!(cost1 + cost2, testee.block_cost); assert_eq!(1, testee.cost_by_writable_accounts.len()); @@ -249,12 +285,12 @@ mod tests { // build testee to have capacity for two simple transactions, with same accounts let mut testee = CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2); { - assert!(testee.would_fit(&keys1, &cost1).is_ok()); - testee.add_transaction(&keys1, &cost1); + assert!(testee.would_fit(&keys1, cost1, 0).is_ok()); + testee.add_transaction(&keys1, cost1, 0); } { - assert!(testee.would_fit(&keys2, &cost2).is_ok()); - testee.add_transaction(&keys2, &cost2); + assert!(testee.would_fit(&keys2, cost2, 0).is_ok()); + testee.add_transaction(&keys2, cost2, 0); } assert_eq!(cost1 + cost2, testee.block_cost); assert_eq!(2, testee.cost_by_writable_accounts.len()); @@ -271,12 +307,12 @@ mod tests { let mut testee = CostTracker::new(cmp::min(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, 0).is_ok()); + testee.add_transaction(&keys1, cost1, 0); } // 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, 0).is_err()); } } @@ -292,15 +328,34 @@ mod tests { let mut testee = CostTracker::new(cmp::max(cost1, cost2), 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, 0).is_ok()); + testee.add_transaction(&keys1, cost1, 0); } // but no more room for package as whole { - assert!(testee.would_fit(&keys2, &cost2).is_err()); + assert!(testee.would_fit(&keys2, cost2, 0).is_err()); } } + #[test] + 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 second_account = Keypair::new(); + 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); + assert!(testee + .would_fit(&keys2, cost2, MAX_ACCOUNT_DATA_LEN) + .is_ok()); + // data is too big + assert!(testee + .would_fit(&keys2, cost2, MAX_ACCOUNT_DATA_LEN + 1) + .is_err()); + } + #[test] fn test_cost_tracker_try_add_is_atomic() { let (mint_keypair, start_hash) = test_setup(); diff --git a/sdk/src/transaction/error.rs b/sdk/src/transaction/error.rs index acf064b9f..60ed8c39f 100644 --- a/sdk/src/transaction/error.rs +++ b/sdk/src/transaction/error.rs @@ -101,6 +101,10 @@ pub enum TransactionError { /// Transaction would exceed max account limit within the block #[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, } impl From for TransactionError { diff --git a/storage-proto/proto/transaction_by_addr.proto b/storage-proto/proto/transaction_by_addr.proto index 36c17832e..d1a40cfd7 100644 --- a/storage-proto/proto/transaction_by_addr.proto +++ b/storage-proto/proto/transaction_by_addr.proto @@ -45,6 +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; } message InstructionError { diff --git a/storage-proto/src/convert.rs b/storage-proto/src/convert.rs index e8511867c..636dcc3f9 100644 --- a/storage-proto/src/convert.rs +++ b/storage-proto/src/convert.rs @@ -567,6 +567,7 @@ impl TryFrom for TransactionError { 18 => TransactionError::UnsupportedVersion, 19 => TransactionError::InvalidWritableAccount, 20 => TransactionError::WouldExceedMaxAccountCostLimit, + 21 => TransactionError::WouldExceedMaxAccountDataCostLimit, _ => return Err("Invalid TransactionError"), }) } @@ -637,6 +638,9 @@ impl From for tx_by_addr::TransactionError { TransactionError::WouldExceedMaxAccountCostLimit => { tx_by_addr::TransactionErrorType::WouldExceedMaxAccountCostLimit } + TransactionError::WouldExceedMaxAccountDataCostLimit => { + tx_by_addr::TransactionErrorType::WouldExceedMaxAccountDataCostLimit + } } as i32, instruction_error: match transaction_error { TransactionError::InstructionError(index, ref instruction_error) => {