From 35f949a502769ffffda825c49d6bcdce1230caf3 Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Tue, 4 Jan 2022 14:25:23 +0800 Subject: [PATCH] Limit number of accounts that a transaction can lock (#22201) (cherry picked from commit 2b5e00d36d58186f1c2bd9b956b488a89573f239) # Conflicts: # accountsdb-plugin-postgres/src/postgres_client/postgres_client_transaction.rs # runtime/src/accounts.rs # runtime/src/bank.rs # sdk/src/feature_set.rs # sdk/src/transaction/error.rs # storage-proto/proto/transaction_by_addr.proto # storage-proto/src/convert.rs --- .../postgres_client_transaction.rs | 12 ++ rpc/src/transaction_status_service.rs | 2 +- runtime/src/accounts.rs | 181 ++++++++++++++++-- runtime/src/bank.rs | 108 ++++++----- sdk/program/src/message/sanitized.rs | 25 +-- sdk/src/feature_set.rs | 16 ++ sdk/src/transaction/error.rs | 20 +- sdk/src/transaction/sanitized.rs | 31 ++- storage-proto/proto/transaction_by_addr.proto | 5 + storage-proto/src/convert.rs | 14 ++ 10 files changed, 306 insertions(+), 108 deletions(-) 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 de521200409f87..cbf8e9e5ffa29b 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,11 @@ pub enum DbTransactionErrorCode { WouldExceedMaxBlockCostLimit, UnsupportedVersion, InvalidWritableAccount, +<<<<<<< HEAD +======= + WouldExceedMaxAccountDataCostLimit, + TooManyAccountLocks, +>>>>>>> 2b5e00d36 (Limit number of accounts that a transaction can lock (#22201)) } impl From<&TransactionError> for DbTransactionErrorCode { @@ -358,6 +363,13 @@ impl From<&TransactionError> for DbTransactionErrorCode { TransactionError::WouldExceedMaxBlockCostLimit => Self::WouldExceedMaxBlockCostLimit, TransactionError::UnsupportedVersion => Self::UnsupportedVersion, TransactionError::InvalidWritableAccount => Self::InvalidWritableAccount, +<<<<<<< HEAD +======= + TransactionError::WouldExceedMaxAccountDataCostLimit => { + Self::WouldExceedMaxAccountDataCostLimit + } + TransactionError::TooManyAccountLocks => Self::TooManyAccountLocks, +>>>>>>> 2b5e00d36 (Limit number of accounts that a transaction can lock (#22201)) } } } diff --git a/rpc/src/transaction_status_service.rs b/rpc/src/transaction_status_service.rs index 9523df641ee765..3377460b88d682 100644 --- a/rpc/src/transaction_status_service.rs +++ b/rpc/src/transaction_status_service.rs @@ -123,7 +123,7 @@ impl TransactionStatusService { transaction.message(), lamports_per_signature, ); - let tx_account_locks = transaction.get_account_locks(); + let tx_account_locks = transaction.get_account_locks_unchecked(); let inner_instructions = inner_instructions.map(|inner_instructions| { inner_instructions diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 076988d9d9595b..dc77ae19ac8e45 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -36,7 +36,12 @@ use { pubkey::Pubkey, system_program, sysvar::{self, instructions::construct_instructions_data}, +<<<<<<< HEAD transaction::{Result, SanitizedTransaction, TransactionError}, +======= + transaction::{Result, SanitizedTransaction, TransactionAccountLocks, TransactionError}, + transaction_context::TransactionAccount, +>>>>>>> 2b5e00d36 (Limit number of accounts that a transaction can lock (#22201)) }, std::{ cmp::Reverse, @@ -952,12 +957,11 @@ impl Accounts { pub fn lock_accounts<'a>( &self, txs: impl Iterator, + feature_set: &FeatureSet, ) -> Vec> { - let keys: Vec<_> = txs.map(|tx| tx.get_account_locks()).collect(); - let account_locks = &mut self.account_locks.lock().unwrap(); - keys.into_iter() - .map(|keys| self.lock_account(account_locks, keys.writable, keys.readonly)) - .collect() + let tx_account_locks_results: Vec> = + txs.map(|tx| tx.get_account_locks(feature_set)).collect(); + self.lock_accounts_inner(tx_account_locks_results) } #[must_use] @@ -966,20 +970,33 @@ impl Accounts { &self, txs: impl Iterator, results: impl Iterator>, + feature_set: &FeatureSet, ) -> Vec> { - let key_results: Vec<_> = txs + let tx_account_locks_results: Vec> = txs .zip(results) .map(|(tx, result)| match result { - Ok(()) => Ok(tx.get_account_locks()), - Err(e) => Err(e), + Ok(()) => tx.get_account_locks(feature_set), + Err(err) => Err(err), }) .collect(); + self.lock_accounts_inner(tx_account_locks_results) + } + + #[must_use] + fn lock_accounts_inner( + &self, + tx_account_locks_results: Vec>, + ) -> Vec> { let account_locks = &mut self.account_locks.lock().unwrap(); - key_results + tx_account_locks_results .into_iter() - .map(|key_result| match key_result { - Ok(keys) => self.lock_account(account_locks, keys.writable, keys.readonly), - Err(e) => Err(e), + .map(|tx_account_locks_result| match tx_account_locks_result { + Ok(tx_account_locks) => self.lock_account( + account_locks, + tx_account_locks.writable, + tx_account_locks.readonly, + ), + Err(err) => Err(err), }) .collect() } @@ -994,12 +1011,23 @@ impl Accounts { let keys: Vec<_> = txs .zip(results) .filter_map(|(tx, res)| match res { +<<<<<<< HEAD Err(TransactionError::AccountInUse) => None, Err(TransactionError::SanitizeFailure) => None, Err(TransactionError::AccountLoadedTwice) => None, Err(TransactionError::WouldExceedMaxBlockCostLimit) => None, Err(TransactionError::WouldExceedMaxAccountCostLimit) => None, _ => Some(tx.get_account_locks()), +======= + Err(TransactionError::AccountLoadedTwice) + | Err(TransactionError::AccountInUse) + | Err(TransactionError::SanitizeFailure) + | Err(TransactionError::TooManyAccountLocks) + | Err(TransactionError::WouldExceedMaxBlockCostLimit) + | Err(TransactionError::WouldExceedMaxAccountCostLimit) + | Err(TransactionError::WouldExceedMaxAccountDataCostLimit) => None, + _ => Some(tx.get_account_locks_unchecked()), +>>>>>>> 2b5e00d36 (Limit number of accounts that a transaction can lock (#22201)) }) .collect(); let mut account_locks = self.account_locks.lock().unwrap(); @@ -1222,12 +1250,12 @@ mod tests { genesis_config::ClusterType, hash::Hash, instruction::{CompiledInstruction, InstructionError}, - message::Message, + message::{Message, MessageHeader}, nonce, nonce_account, rent::Rent, signature::{keypair_from_seed, signers::Signers, Keypair, Signer}, system_instruction, system_program, - transaction::Transaction, + transaction::{Transaction, MAX_TX_ACCOUNT_LOCKS}, }, std::{ convert::TryFrom, @@ -2105,6 +2133,109 @@ mod tests { accounts.bank_hash_at(1); } + #[test] + fn test_lock_accounts_with_duplicates() { + let accounts = Accounts::new_with_config_for_tests( + Vec::new(), + &ClusterType::Development, + AccountSecondaryIndexes::default(), + false, + AccountShrinkThreshold::default(), + ); + + let keypair = Keypair::new(); + let message = Message { + header: MessageHeader { + num_required_signatures: 1, + ..MessageHeader::default() + }, + account_keys: vec![keypair.pubkey(), keypair.pubkey()], + ..Message::default() + }; + + let tx = new_sanitized_tx(&[&keypair], message, Hash::default()); + let results = accounts.lock_accounts([tx].iter(), &FeatureSet::all_enabled()); + assert_eq!(results[0], Err(TransactionError::AccountLoadedTwice)); + } + + #[test] + fn test_lock_accounts_with_too_many_accounts() { + let accounts = Accounts::new_with_config_for_tests( + Vec::new(), + &ClusterType::Development, + AccountSecondaryIndexes::default(), + false, + AccountShrinkThreshold::default(), + ); + + let keypair = Keypair::new(); + + // Allow up to MAX_TX_ACCOUNT_LOCKS + { + let num_account_keys = MAX_TX_ACCOUNT_LOCKS; + let mut account_keys: Vec<_> = (0..num_account_keys) + .map(|_| Pubkey::new_unique()) + .collect(); + account_keys[0] = keypair.pubkey(); + let message = Message { + header: MessageHeader { + num_required_signatures: 1, + ..MessageHeader::default() + }, + account_keys, + ..Message::default() + }; + + let txs = vec![new_sanitized_tx(&[&keypair], message, Hash::default())]; + let results = accounts.lock_accounts(txs.iter(), &FeatureSet::all_enabled()); + assert_eq!(results[0], Ok(())); + accounts.unlock_accounts(txs.iter(), &results); + } + + // Allow over MAX_TX_ACCOUNT_LOCKS before feature activation + { + let num_account_keys = MAX_TX_ACCOUNT_LOCKS + 1; + let mut account_keys: Vec<_> = (0..num_account_keys) + .map(|_| Pubkey::new_unique()) + .collect(); + account_keys[0] = keypair.pubkey(); + let message = Message { + header: MessageHeader { + num_required_signatures: 1, + ..MessageHeader::default() + }, + account_keys, + ..Message::default() + }; + + let txs = vec![new_sanitized_tx(&[&keypair], message, Hash::default())]; + let results = accounts.lock_accounts(txs.iter(), &FeatureSet::default()); + assert_eq!(results[0], Ok(())); + accounts.unlock_accounts(txs.iter(), &results); + } + + // Disallow over MAX_TX_ACCOUNT_LOCKS after feature activation + { + let num_account_keys = MAX_TX_ACCOUNT_LOCKS + 1; + let mut account_keys: Vec<_> = (0..num_account_keys) + .map(|_| Pubkey::new_unique()) + .collect(); + account_keys[0] = keypair.pubkey(); + let message = Message { + header: MessageHeader { + num_required_signatures: 1, + ..MessageHeader::default() + }, + account_keys, + ..Message::default() + }; + + let txs = vec![new_sanitized_tx(&[&keypair], message, Hash::default())]; + let results = accounts.lock_accounts(txs.iter(), &FeatureSet::all_enabled()); + assert_eq!(results[0], Err(TransactionError::TooManyAccountLocks)); + } + } + #[test] fn test_accounts_locks() { let keypair0 = Keypair::new(); @@ -2139,7 +2270,7 @@ mod tests { instructions, ); let tx = new_sanitized_tx(&[&keypair0], message, Hash::default()); - let results0 = accounts.lock_accounts([tx.clone()].iter()); + let results0 = accounts.lock_accounts([tx.clone()].iter(), &FeatureSet::all_enabled()); assert!(results0[0].is_ok()); assert_eq!( @@ -2174,7 +2305,7 @@ mod tests { ); let tx1 = new_sanitized_tx(&[&keypair1], message, Hash::default()); let txs = vec![tx0, tx1]; - let results1 = accounts.lock_accounts(txs.iter()); + let results1 = accounts.lock_accounts(txs.iter(), &FeatureSet::all_enabled()); assert!(results1[0].is_ok()); // Read-only account (keypair1) can be referenced multiple times assert!(results1[1].is_err()); // Read-only account (keypair1) cannot also be locked as writable @@ -2201,7 +2332,7 @@ mod tests { instructions, ); let tx = new_sanitized_tx(&[&keypair1], message, Hash::default()); - let results2 = accounts.lock_accounts([tx].iter()); + let results2 = accounts.lock_accounts([tx].iter(), &FeatureSet::all_enabled()); assert!(results2[0].is_ok()); // Now keypair1 account can be locked as writable // Check that read-only lock with zero references is deleted @@ -2270,7 +2401,9 @@ mod tests { let exit_clone = exit_clone.clone(); loop { let txs = vec![writable_tx.clone()]; - let results = accounts_clone.clone().lock_accounts(txs.iter()); + let results = accounts_clone + .clone() + .lock_accounts(txs.iter(), &FeatureSet::all_enabled()); for result in results.iter() { if result.is_ok() { counter_clone.clone().fetch_add(1, Ordering::SeqCst); @@ -2285,7 +2418,9 @@ mod tests { let counter_clone = counter; for _ in 0..5 { let txs = vec![readonly_tx.clone()]; - let results = accounts_arc.clone().lock_accounts(txs.iter()); + let results = accounts_arc + .clone() + .lock_accounts(txs.iter(), &FeatureSet::all_enabled()); if results[0].is_ok() { let counter_value = counter_clone.clone().load(Ordering::SeqCst); thread::sleep(time::Duration::from_millis(50)); @@ -2331,7 +2466,7 @@ mod tests { instructions, ); let tx = new_sanitized_tx(&[&keypair0], message, Hash::default()); - let results0 = accounts.lock_accounts([tx].iter()); + let results0 = accounts.lock_accounts([tx].iter(), &FeatureSet::all_enabled()); assert!(results0[0].is_ok()); // Instruction program-id account demoted to readonly @@ -2422,7 +2557,11 @@ mod tests { Ok(()), ]; - let results = accounts.lock_accounts_with_results(txs.iter(), qos_results.into_iter()); + let results = accounts.lock_accounts_with_results( + txs.iter(), + qos_results.into_iter(), + &FeatureSet::all_enabled(), + ); assert!(results[0].is_ok()); // Read-only account (keypair0) can be referenced multiple times assert!(results[1].is_err()); // is not locked due to !qos_results[1].is_ok() diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index b34cd51450c633..661ed41f955fb6 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -237,7 +237,11 @@ impl ExecuteTimings { } type BankStatusCache = StatusCache>; +<<<<<<< HEAD #[frozen_abi(digest = "32EjVUc6shHHVPpsnBAVfyBziMgyFzH8qxisLwmwwdS1")] +======= +#[frozen_abi(digest = "6XG6H1FChrDdY39K62KFWj5XfDao4dd24WZgcJkdMu1E")] +>>>>>>> 2b5e00d36 (Limit number of accounts that a transaction can lock (#22201)) pub type BankSlotDelta = SlotDelta>; // Eager rent collection repeats in cyclic manner. @@ -3054,7 +3058,10 @@ impl Bank { .into_iter() .map(SanitizedTransaction::from_transaction_for_tests) .collect::>(); - let lock_results = self.rc.accounts.lock_accounts(sanitized_txs.iter()); + let lock_results = self + .rc + .accounts + .lock_accounts(sanitized_txs.iter(), &FeatureSet::all_enabled()); TransactionBatch::new(lock_results, self, Cow::Owned(sanitized_txs)) } @@ -3070,7 +3077,10 @@ impl Bank { }) }) .collect::>>()?; - let lock_results = self.rc.accounts.lock_accounts(sanitized_txs.iter()); + let lock_results = self + .rc + .accounts + .lock_accounts(sanitized_txs.iter(), &FeatureSet::all_enabled()); Ok(TransactionBatch::new( lock_results, self, @@ -3083,7 +3093,10 @@ impl Bank { &'a self, txs: &'b [SanitizedTransaction], ) -> TransactionBatch<'a, 'b> { - let lock_results = self.rc.accounts.lock_accounts(txs.iter()); + let lock_results = self + .rc + .accounts + .lock_accounts(txs.iter(), &self.feature_set); TransactionBatch::new(lock_results, self, Cow::Borrowed(txs)) } @@ -3095,10 +3108,11 @@ impl Bank { transaction_results: impl Iterator>, ) -> TransactionBatch<'a, 'b> { // this lock_results could be: Ok, AccountInUse, WouldExceedBlockMaxLimit or WouldExceedAccountMaxLimit - let lock_results = self - .rc - .accounts - .lock_accounts_with_results(transactions.iter(), transaction_results); + let lock_results = self.rc.accounts.lock_accounts_with_results( + transactions.iter(), + transaction_results, + &self.feature_set, + ); TransactionBatch::new(lock_results, self, Cow::Borrowed(transactions)) } @@ -3107,7 +3121,9 @@ impl Bank { &'a self, transaction: SanitizedTransaction, ) -> TransactionBatch<'a, '_> { - let mut batch = TransactionBatch::new(vec![Ok(())], self, Cow::Owned(vec![transaction])); + let lock_result = transaction.get_account_locks(&self.feature_set).map(|_| ()); + let mut batch = + TransactionBatch::new(vec![lock_result], self, Cow::Owned(vec![transaction])); batch.needs_unlock = false; batch } @@ -6232,6 +6248,7 @@ pub(crate) mod tests { system_program, sysvar::rewards::Rewards, timing::duration_as_s, + transaction::MAX_TX_ACCOUNT_LOCKS, }, solana_vote_program::{ vote_instruction, @@ -11598,6 +11615,43 @@ pub(crate) mod tests { assert_eq!(result, Err(TransactionError::AccountLoadedTwice)); } + #[test] + fn test_process_transaction_with_too_many_account_locks() { + solana_logger::setup(); + let (genesis_config, mint_keypair) = create_genesis_config(500); + let mut bank = Bank::new_for_tests(&genesis_config); + + let from_pubkey = solana_sdk::pubkey::new_rand(); + let to_pubkey = solana_sdk::pubkey::new_rand(); + + let account_metas = vec![ + AccountMeta::new(from_pubkey, false), + AccountMeta::new(to_pubkey, false), + ]; + + bank.add_builtin( + "mock_vote", + &solana_vote_program::id(), + mock_ok_vote_processor, + ); + + let instruction = + Instruction::new_with_bincode(solana_vote_program::id(), &10, account_metas); + let mut tx = Transaction::new_signed_with_payer( + &[instruction], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + bank.last_blockhash(), + ); + + while tx.message.account_keys.len() <= MAX_TX_ACCOUNT_LOCKS { + tx.message.account_keys.push(solana_sdk::pubkey::new_rand()); + } + + let result = bank.process_transaction(&tx); + assert_eq!(result, Err(TransactionError::TooManyAccountLocks)); + } + #[test] fn test_program_id_as_payer() { solana_logger::setup(); @@ -14970,44 +15024,6 @@ pub(crate) mod tests { } } - #[test] - fn test_verify_transactions_load_duplicate_account() { - let GenesisConfigInfo { genesis_config, .. } = - create_genesis_config_with_leader(42, &solana_sdk::pubkey::new_rand(), 42); - let bank = Bank::new_for_tests(&genesis_config); - - let mut rng = rand::thread_rng(); - let recent_blockhash = hash::new_rand(&mut rng); - let from_keypair = Keypair::new(); - let to_keypair = Keypair::new(); - let from_pubkey = from_keypair.pubkey(); - let to_pubkey = to_keypair.pubkey(); - - let make_transaction = || { - let mut message = Message::new( - &[system_instruction::transfer(&from_pubkey, &to_pubkey, 1)], - Some(&from_pubkey), - ); - let to_index = message - .account_keys - .iter() - .position(|k| k == &to_pubkey) - .unwrap(); - message.account_keys[to_index] = from_pubkey; - Transaction::new(&[&from_keypair], message, recent_blockhash) - }; - - // Duplicate account - { - let tx = make_transaction(); - assert_eq!( - bank.verify_transaction(tx.into(), TransactionVerificationMode::FullVerification) - .err(), - Some(TransactionError::AccountLoadedTwice), - ); - } - } - #[test] fn test_verify_transactions_packet_data_size() { let GenesisConfigInfo { genesis_config, .. } = diff --git a/sdk/program/src/message/sanitized.rs b/sdk/program/src/message/sanitized.rs index b09902774cb300..e934d22a2f2fa5 100644 --- a/sdk/program/src/message/sanitized.rs +++ b/sdk/program/src/message/sanitized.rs @@ -30,8 +30,6 @@ pub enum SanitizeMessageError { ValueOutOfBounds, #[error("invalid value")] InvalidValue, - #[error("duplicate account key")] - DuplicateAccountKey, } impl From for SanitizeMessageError { @@ -48,13 +46,7 @@ impl TryFrom for SanitizedMessage { type Error = SanitizeMessageError; fn try_from(message: LegacyMessage) -> Result { message.sanitize()?; - - let sanitized_msg = Self::Legacy(message); - if sanitized_msg.has_duplicates() { - return Err(SanitizeMessageError::DuplicateAccountKey); - } - - Ok(sanitized_msg) + Ok(Self::Legacy(message)) } } @@ -310,21 +302,6 @@ mod tests { #[test] fn test_try_from_message() { - let dupe_key = Pubkey::new_unique(); - let legacy_message_with_dupes = LegacyMessage { - header: MessageHeader { - num_required_signatures: 1, - ..MessageHeader::default() - }, - account_keys: vec![dupe_key, dupe_key], - ..LegacyMessage::default() - }; - - assert_eq!( - SanitizedMessage::try_from(legacy_message_with_dupes).err(), - Some(SanitizeMessageError::DuplicateAccountKey), - ); - let legacy_message_with_no_signers = LegacyMessage { account_keys: vec![Pubkey::new_unique()], ..LegacyMessage::default() diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 9025be59bf1b61..42a7c8daea24c0 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -275,6 +275,17 @@ pub mod reject_all_elf_rw { solana_sdk::declare_id!("DeMpxgMq51j3rZfNK2hQKZyXknQvqevPSFPJFNTbXxsS"); } +<<<<<<< HEAD +======= +pub mod cap_accounts_data_len { + solana_sdk::declare_id!("capRxUrBjNkkCpjrJxPGfPaWijB7q3JoDfsWXAnt46r"); +} + +pub mod max_tx_account_locks { + solana_sdk::declare_id!("CBkDroRDqm8HwHe6ak9cguPjUomrASEkfmxEaZ5CNNxz"); +} + +>>>>>>> 2b5e00d36 (Limit number of accounts that a transaction can lock (#22201)) lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -338,6 +349,11 @@ lazy_static! { (reject_non_rent_exempt_vote_withdraws::id(), "fail vote withdraw instructions which leave the account non-rent-exempt"), (evict_invalid_stakes_cache_entries::id(), "evict invalid stakes cache entries on epoch boundaries"), (reject_all_elf_rw::id(), "reject all read-write data in program elfs"), +<<<<<<< HEAD +======= + (cap_accounts_data_len::id(), "cap the accounts data len"), + (max_tx_account_locks::id(), "enforce max number of locked accounts per transaction"), +>>>>>>> 2b5e00d36 (Limit number of accounts that a transaction can lock (#22201)) /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/sdk/src/transaction/error.rs b/sdk/src/transaction/error.rs index acf064b9f47a2d..6818c50c03bdec 100644 --- a/sdk/src/transaction/error.rs +++ b/sdk/src/transaction/error.rs @@ -101,6 +101,17 @@ pub enum TransactionError { /// Transaction would exceed max account limit within the block #[error("Transaction would exceed max account limit within the block")] WouldExceedMaxAccountCostLimit, +<<<<<<< HEAD +======= + + /// Transaction would exceed max account data limit within the block + #[error("Transaction would exceed max account data limit within the block")] + WouldExceedMaxAccountDataCostLimit, + + /// Transaction locked too many accounts + #[error("Transaction locked too many accounts")] + TooManyAccountLocks, +>>>>>>> 2b5e00d36 (Limit number of accounts that a transaction can lock (#22201)) } impl From for TransactionError { @@ -110,12 +121,7 @@ impl From for TransactionError { } impl From for TransactionError { - fn from(err: SanitizeMessageError) -> Self { - match err { - SanitizeMessageError::IndexOutOfBounds - | SanitizeMessageError::ValueOutOfBounds - | SanitizeMessageError::InvalidValue => Self::SanitizeFailure, - SanitizeMessageError::DuplicateAccountKey => Self::AccountLoadedTwice, - } + fn from(_err: SanitizeMessageError) -> Self { + Self::SanitizeFailure } } diff --git a/sdk/src/transaction/sanitized.rs b/sdk/src/transaction/sanitized.rs index 9da3e61fa6a0b1..ac0b1671805788 100644 --- a/sdk/src/transaction/sanitized.rs +++ b/sdk/src/transaction/sanitized.rs @@ -20,6 +20,11 @@ use { std::sync::Arc, }; +/// Maximum number of accounts that a transaction may lock. +/// 64 was chosen because it is roughly twice the previous +/// number of account keys that could fit in a legacy tx. +pub const MAX_TX_ACCOUNT_LOCKS: usize = 64; + /// Sanitized transaction and the hash of its message #[derive(Debug, Clone)] pub struct SanitizedTransaction { @@ -59,10 +64,6 @@ impl SanitizedTransaction { }), }; - if message.has_duplicates() { - return Err(TransactionError::AccountLoadedTwice); - } - let is_simple_vote_tx = is_simple_vote_tx.unwrap_or_else(|| { let mut ix_iter = message.program_instructions_iter(); ix_iter.next().map(|(program_id, _ix)| program_id) == Some(&crate::vote::program::id()) @@ -79,10 +80,6 @@ impl SanitizedTransaction { pub fn try_from_legacy_transaction(tx: Transaction) -> Result { tx.sanitize()?; - if tx.message.has_duplicates() { - return Err(TransactionError::AccountLoadedTwice); - } - Ok(Self { message_hash: tx.message.hash(), message: SanitizedMessage::Legacy(tx.message), @@ -143,8 +140,24 @@ impl SanitizedTransaction { } } + /// Validate and return the account keys locked by this transaction + pub fn get_account_locks( + &self, + feature_set: &feature_set::FeatureSet, + ) -> Result { + if self.message.has_duplicates() { + Err(TransactionError::AccountLoadedTwice) + } else if feature_set.is_active(&feature_set::max_tx_account_locks::id()) + && self.message.account_keys_len() > MAX_TX_ACCOUNT_LOCKS + { + Err(TransactionError::TooManyAccountLocks) + } else { + Ok(self.get_account_locks_unchecked()) + } + } + /// Return the list of accounts that must be locked during processing this transaction. - pub fn get_account_locks(&self) -> TransactionAccountLocks { + pub fn get_account_locks_unchecked(&self) -> TransactionAccountLocks { let message = &self.message; let num_readonly_accounts = message.num_readonly_accounts(); let num_writable_accounts = message diff --git a/storage-proto/proto/transaction_by_addr.proto b/storage-proto/proto/transaction_by_addr.proto index 36c17832ee9b85..d1fe1d75477fca 100644 --- a/storage-proto/proto/transaction_by_addr.proto +++ b/storage-proto/proto/transaction_by_addr.proto @@ -45,6 +45,11 @@ enum TransactionErrorType { UNSUPPORTED_VERSION = 18; INVALID_WRITABLE_ACCOUNT = 19; WOULD_EXCEED_MAX_ACCOUNT_COST_LIMIT = 20; +<<<<<<< HEAD +======= + WOULD_EXCEED_MAX_ACCOUNT_DATA_COST_LIMIT = 21; + TOO_MANY_ACCOUNT_LOCKS = 22; +>>>>>>> 2b5e00d36 (Limit number of accounts that a transaction can lock (#22201)) } message InstructionError { diff --git a/storage-proto/src/convert.rs b/storage-proto/src/convert.rs index e8511867c6b3a8..cd3022f54cf530 100644 --- a/storage-proto/src/convert.rs +++ b/storage-proto/src/convert.rs @@ -567,6 +567,11 @@ impl TryFrom for TransactionError { 18 => TransactionError::UnsupportedVersion, 19 => TransactionError::InvalidWritableAccount, 20 => TransactionError::WouldExceedMaxAccountCostLimit, +<<<<<<< HEAD +======= + 21 => TransactionError::WouldExceedMaxAccountDataCostLimit, + 22 => TransactionError::TooManyAccountLocks, +>>>>>>> 2b5e00d36 (Limit number of accounts that a transaction can lock (#22201)) _ => return Err("Invalid TransactionError"), }) } @@ -637,6 +642,15 @@ impl From for tx_by_addr::TransactionError { TransactionError::WouldExceedMaxAccountCostLimit => { tx_by_addr::TransactionErrorType::WouldExceedMaxAccountCostLimit } +<<<<<<< HEAD +======= + TransactionError::WouldExceedMaxAccountDataCostLimit => { + tx_by_addr::TransactionErrorType::WouldExceedMaxAccountDataCostLimit + } + TransactionError::TooManyAccountLocks => { + tx_by_addr::TransactionErrorType::TooManyAccountLocks + } +>>>>>>> 2b5e00d36 (Limit number of accounts that a transaction can lock (#22201)) } as i32, instruction_error: match transaction_error { TransactionError::InstructionError(index, ref instruction_error) => {