diff --git a/Cargo.lock b/Cargo.lock index 4005d2f41b0079..a5c433403bb693 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5589,11 +5589,13 @@ dependencies = [ "solana-frozen-abi-macro", "solana-inline-spl", "solana-lattice-hash", + "solana-ledger", "solana-logger", "solana-measure", "solana-metrics", "solana-nohash-hasher", "solana-rayon-threadlimit", + "solana-runtime", "solana-sdk", "solana-stake-program", "solana-svm-transaction", diff --git a/accounts-db/Cargo.toml b/accounts-db/Cargo.toml index 2eb876219da96a..583d65b3091dd3 100644 --- a/accounts-db/Cargo.toml +++ b/accounts-db/Cargo.toml @@ -70,6 +70,8 @@ serde_bytes = { workspace = true } solana-accounts-db = { path = ".", features = ["dev-context-only-utils"] } solana-compute-budget = { workspace = true } solana-logger = { workspace = true } +solana-ledger = { workspace = true } +solana-runtime = { workspace = true, features = ["dev-context-only-utils"] } solana-sdk = { workspace = true, features = ["dev-context-only-utils"] } static_assertions = { workspace = true } strum = { workspace = true, features = ["derive"] } @@ -114,3 +116,4 @@ harness = false [lints] workspace = true + diff --git a/accounts-db/benches/bench_lock_accounts.rs b/accounts-db/benches/bench_lock_accounts.rs index d34166fa7e7844..02e7df511f53a2 100644 --- a/accounts-db/benches/bench_lock_accounts.rs +++ b/accounts-db/benches/bench_lock_accounts.rs @@ -20,10 +20,10 @@ const LOCK_COUNTS: [usize; 2] = [2, 64]; // total transactions per run const TOTAL_TRANSACTIONS: usize = 1024; -fn create_test_transactions(lock_count: usize, read_conflicts: bool) -> Vec { - // keys available to be shared between transactions, depending on mode - // currently, we test batches with no conflicts and batches with reader/reader conflicts - // in the future with SIMD83, we will also test reader/writer and writer/writer conflicts +fn create_test_transactions( + lock_count: usize, + self_conflicting_batch: bool, +) -> Vec { let shared_pubkeys: Vec<_> = (0..lock_count).map(|_| Pubkey::new_unique()).collect(); let mut transactions = vec![]; @@ -37,11 +37,15 @@ fn create_test_transactions(lock_count: usize, read_conflicts: bool) -> Vec Vec = transactions.chunks(batch_size).collect(); group.bench_function(name.as_str(), move |b| { b.iter(|| { for batch in &transaction_batches { - let results = - accounts.lock_accounts(black_box(batch.iter()), MAX_TX_ACCOUNT_LOCKS); + let (results, _) = accounts.lock_accounts( + black_box(batch.iter()), + MAX_TX_ACCOUNT_LOCKS, + self_conflicting_batch, + ); accounts.unlock_accounts(batch.iter().zip(&results)); } }) diff --git a/accounts-db/src/account_locks.rs b/accounts-db/src/account_locks.rs index fc2fba7292b10e..08787d33022df3 100644 --- a/accounts-db/src/account_locks.rs +++ b/accounts-db/src/account_locks.rs @@ -1,18 +1,23 @@ #[cfg(feature = "dev-context-only-utils")] use qualifier_attr::qualifiers; use { + crate::accounts::BatchAccountLocks, ahash::{AHashMap, AHashSet}, solana_sdk::{ message::AccountKeys, pubkey::Pubkey, transaction::{TransactionError, MAX_TX_ACCOUNT_LOCKS}, }, - std::{cell::RefCell, collections::hash_map}, + std::{ + cell::{RefCell, RefMut}, + collections::hash_map, + }, }; #[derive(Debug, Default)] pub struct AccountLocks { - write_locks: AHashSet, + // A key can have multiple outstanding write locks in the case of a self-conflicting batch. + write_locks: AHashMap, readonly_locks: AHashMap, } @@ -21,6 +26,7 @@ impl AccountLocks { /// The bool in the tuple indicates if the account is writable. /// Returns an error if any of the accounts are already locked in a way /// that conflicts with the requested lock. + /// This function will become obsolete after self conflicting batches are allowed. pub fn try_lock_accounts<'a>( &mut self, keys: impl Iterator + Clone, @@ -46,6 +52,44 @@ impl AccountLocks { Ok(()) } + pub fn try_lock_accounts_with_conflicting_batches<'a>( + &mut self, + keys: impl Iterator + Clone, + batch_account_locks: &mut RefMut, + ) -> (Result<(), TransactionError>, bool) { + let mut self_conflicting_batch = false; + + for (key, writable) in keys.clone() { + if writable { + if !self.can_write_lock(key) { + if !(batch_account_locks.writables.contains(key) + || batch_account_locks.readables.contains(key)) + { + return (Err(TransactionError::AccountInUse), false); + } + self_conflicting_batch = true; + } + } else if !self.can_read_lock(key) { + if !batch_account_locks.writables.contains(key) { + return (Err(TransactionError::AccountInUse), false); + } + self_conflicting_batch = true; + } + } + + for (key, writable) in keys { + if writable { + batch_account_locks.insert_write_lock(key); + self.lock_write(key); + } else { + batch_account_locks.insert_read_lock(key); + self.lock_readonly(key); + } + } + + (Ok(()), self_conflicting_batch) + } + /// Unlock the account keys in `keys` after a transaction. /// The bool in the tuple indicates if the account is writable. /// In debug-mode this function will panic if an attempt is made to unlock @@ -69,7 +113,7 @@ impl AccountLocks { #[cfg_attr(feature = "dev-context-only-utils", qualifiers(pub))] fn is_locked_write(&self, key: &Pubkey) -> bool { - self.write_locks.contains(key) + self.write_locks.get(key).map_or(false, |count| *count > 0) } fn can_read_lock(&self, key: &Pubkey) -> bool { @@ -87,7 +131,7 @@ impl AccountLocks { } fn lock_write(&mut self, key: &Pubkey) { - self.write_locks.insert(*key); + *self.write_locks.entry(*key).or_default() += 1; } fn unlock_readonly(&mut self, key: &Pubkey) { @@ -106,11 +150,18 @@ impl AccountLocks { } fn unlock_write(&mut self, key: &Pubkey) { - let removed = self.write_locks.remove(key); - debug_assert!( - removed, - "Attempted to remove a write-lock for a key that wasn't write-locked" - ); + if let hash_map::Entry::Occupied(mut occupied_entry) = self.write_locks.entry(*key) { + let count = occupied_entry.get_mut(); + *count -= 1; + if *count == 0 { + occupied_entry.remove_entry(); + } + } else { + debug_assert!( + false, + "Attempted to remove a write-lock for a key that wasn't write-locked" + ); + } } } @@ -158,7 +209,11 @@ fn has_duplicates(account_keys: AccountKeys) -> bool { #[cfg(test)] mod tests { - use {super::*, solana_sdk::message::v0::LoadedAddresses}; + use {super::*, solana_sdk::message::v0::LoadedAddresses, std::cell::RefCell}; + + thread_local! { + static BATCH_ACCOUNT_LOCKS: RefCell = RefCell::new(BatchAccountLocks::with_capacity(64*128)); + } #[test] fn test_account_locks() { @@ -288,4 +343,64 @@ mod tests { let account_keys = AccountKeys::new(&keys, None); assert!(has_duplicates(account_keys)); } + + #[test] + fn test_account_locks_with_conflicting_batches_and_unlock() { + let mut account_locks = AccountLocks::default(); + + let key1 = Pubkey::new_unique(); + let key2 = Pubkey::new_unique(); + BATCH_ACCOUNT_LOCKS.with(|batch_account_locks| { + let mut batch_account_locks = batch_account_locks.borrow_mut(); + // Add write and read-lock. + let (result, _) = account_locks.try_lock_accounts_with_conflicting_batches( + [(&key1, true), (&key2, false)].into_iter(), + &mut batch_account_locks, + ); + assert!(result.is_ok()); + + // Try to add duplicate write-lock, allowed in conflicting batch. + let (result, _) = account_locks.try_lock_accounts_with_conflicting_batches( + [(&key1, true)].into_iter(), + &mut batch_account_locks, + ); + assert!(result.is_ok()); + + // Try to add write lock on read-locked account, allowed in conflicting batch. + let (result, _) = account_locks.try_lock_accounts_with_conflicting_batches( + [(&key2, true)].into_iter(), + &mut batch_account_locks, + ); + assert!(result.is_ok()); + + // Try to add read lock on write-locked account, allowed in conflicting batch. + let (result, _) = account_locks.try_lock_accounts_with_conflicting_batches( + [(&key1, false)].into_iter(), + &mut batch_account_locks, + ); + assert!(result.is_ok()); + + // Add read lock on read-locked account. + let (result, _) = account_locks.try_lock_accounts_with_conflicting_batches( + [(&key2, false)].into_iter(), + &mut batch_account_locks, + ); + assert!(result.is_ok()); + + // Unlock write and read locks. + account_locks.unlock_accounts([(&key1, true), (&key2, false)].into_iter()); + + // More remaining write-locks and Read-lock. + assert!(account_locks.is_locked_write(&key1)); + assert!(account_locks.is_locked_readonly(&key2)); + + // Unlock remaining write locks + account_locks.unlock_accounts([(&key1, true)].into_iter()); + assert!(!account_locks.is_locked_write(&key1)); + + // Unlock read lock. + account_locks.unlock_accounts([(&key2, false)].into_iter()); + assert!(!account_locks.is_locked_readonly(&key2)); + }) + } } diff --git a/accounts-db/src/accounts.rs b/accounts-db/src/accounts.rs index 8de5431318a3a0..5bbecdb395eadd 100644 --- a/accounts-db/src/accounts.rs +++ b/accounts-db/src/accounts.rs @@ -9,6 +9,7 @@ use { ancestors::Ancestors, storable_accounts::StorableAccounts, }, + ahash::AHashSet, dashmap::DashMap, log::*, solana_sdk::{ @@ -18,13 +19,14 @@ use { message::v0::LoadedAddresses, pubkey::Pubkey, slot_hashes::SlotHashes, - transaction::{Result, SanitizedTransaction}, + transaction::{Result, SanitizedTransaction, MAX_TX_ACCOUNT_LOCKS}, transaction_context::TransactionAccount, }, solana_svm_transaction::{ message_address_table_lookup::SVMMessageAddressTableLookup, svm_message::SVMMessage, }, std::{ + cell::RefCell, cmp::Reverse, collections::{BinaryHeap, HashSet}, ops::RangeBounds, @@ -35,12 +37,46 @@ use { }, }; +// max number of accts in a default size entry 64 * 128 +const BATCH_ACCOUNTS_LOOKUP_TABLE_SIZE: usize = 64 * MAX_TX_ACCOUNT_LOCKS; pub type PubkeyAccountSlot = (Pubkey, AccountSharedData, Slot); struct TransactionAccountLocksIterator<'a, T: SVMMessage> { transaction: &'a T, } +#[derive(Debug, Default)] +pub struct BatchAccountLocks { + pub writables: AHashSet, + pub readables: AHashSet, +} + +thread_local! { + static BATCH_ACCOUNT_LOCKS: RefCell = RefCell::new(BatchAccountLocks::with_capacity(BATCH_ACCOUNTS_LOOKUP_TABLE_SIZE)); +} + +impl BatchAccountLocks { + pub fn with_capacity(capacity: usize) -> Self { + BatchAccountLocks { + writables: AHashSet::with_capacity(capacity), + readables: AHashSet::with_capacity(capacity), + } + } + + pub fn insert_read_lock(&mut self, key: &Pubkey) { + self.readables.insert(*key); + } + + pub fn insert_write_lock(&mut self, key: &Pubkey) { + self.writables.insert(*key); + } + + fn clear(&mut self) { + self.writables.clear(); + self.readables.clear(); + } +} + impl<'a, T: SVMMessage> TransactionAccountLocksIterator<'a, T> { pub(crate) fn new(transaction: &'a T) -> Self { Self { transaction } @@ -518,7 +554,8 @@ impl Accounts { &self, txs: impl Iterator, tx_account_lock_limit: usize, - ) -> Vec> { + allow_self_conflicting_entries: bool, + ) -> (Vec>, bool) { // Validate the account locks, then get iterator if successful validation. let tx_account_locks_results: Vec> = txs .map(|tx| { @@ -526,7 +563,7 @@ impl Accounts { .map(|_| TransactionAccountLocksIterator::new(tx)) }) .collect(); - self.lock_accounts_inner(tx_account_locks_results) + self.lock_accounts_inner(tx_account_locks_results, allow_self_conflicting_entries) } #[must_use] @@ -535,6 +572,7 @@ impl Accounts { txs: impl Iterator, results: impl Iterator>, tx_account_lock_limit: usize, + allow_self_conflicting_entries: bool, ) -> Vec> { // Validate the account locks, then get iterator if successful validation. let tx_account_locks_results: Vec> = txs @@ -545,24 +583,58 @@ impl Accounts { Err(err) => Err(err), }) .collect(); - self.lock_accounts_inner(tx_account_locks_results) + let (res, _) = + self.lock_accounts_inner(tx_account_locks_results, allow_self_conflicting_entries); + res } #[must_use] fn lock_accounts_inner( &self, tx_account_locks_results: Vec>>, - ) -> Vec> { + allow_self_conflicting_entries: bool, + ) -> (Vec>, bool) { let account_locks = &mut self.account_locks.lock().unwrap(); - tx_account_locks_results - .into_iter() - .map(|tx_account_locks_result| match tx_account_locks_result { - Ok(tx_account_locks) => { - account_locks.try_lock_accounts(tx_account_locks.accounts_with_is_writable()) - } - Err(err) => Err(err), + let mut self_conflicting_batch = false; + if allow_self_conflicting_entries { + BATCH_ACCOUNT_LOCKS.with(|batch_account_locks| { + let mut batch_account_locks = batch_account_locks.borrow_mut(); + let (lock_results, self_conflicting_batch) = ( + tx_account_locks_results + .into_iter() + .map(|tx_account_locks_result| match tx_account_locks_result { + Ok(tx_account_locks) => { + let (res, conflicting_tx) = account_locks + .try_lock_accounts_with_conflicting_batches( + tx_account_locks.accounts_with_is_writable(), + &mut batch_account_locks, + ); + self_conflicting_batch = self_conflicting_batch || conflicting_tx; + res + } + Err(err) => Err(err), + }) + .collect(), + self_conflicting_batch, + ); + // clear up the entry level locks lookup table + batch_account_locks.clear(); + (lock_results, self_conflicting_batch) }) - .collect() + } else { + ( + tx_account_locks_results + .into_iter() + .map(|tx_account_locks_result| match tx_account_locks_result { + Ok(tx_account_locks) => account_locks + .try_lock_accounts(tx_account_locks.accounts_with_is_writable()), + Err(err) => Err(err), + }) + .collect(), + // in case the self conflicting batches are not allowed then this flag is always false + self_conflicting_batch, + ) + } } /// Once accounts are unlocked, new transactions that modify that state can enter the pipeline @@ -861,7 +933,9 @@ mod tests { }; let tx = new_sanitized_tx(&[&keypair], message, Hash::default()); - let results = accounts.lock_accounts([tx].iter(), MAX_TX_ACCOUNT_LOCKS); + let (results, self_conflicting_batch) = + accounts.lock_accounts([tx].iter(), MAX_TX_ACCOUNT_LOCKS, false); + assert!(!self_conflicting_batch); assert_eq!(results[0], Err(TransactionError::AccountLoadedTwice)); } @@ -889,7 +963,10 @@ mod tests { }; let txs = vec![new_sanitized_tx(&[&keypair], message, Hash::default())]; - let results = accounts.lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS); + let (results, self_conflicting_batch) = + accounts.lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS, false); + assert!(!self_conflicting_batch); + assert_eq!(results, vec![Ok(())]); accounts.unlock_accounts(txs.iter().zip(&results)); } @@ -911,7 +988,9 @@ mod tests { }; let txs = vec![new_sanitized_tx(&[&keypair], message, Hash::default())]; - let results = accounts.lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS); + let (results, self_conflicting_batch) = + accounts.lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS, false); + assert!(!self_conflicting_batch); assert_eq!(results[0], Err(TransactionError::TooManyAccountLocks)); } } @@ -945,8 +1024,10 @@ mod tests { instructions, ); let tx = new_sanitized_tx(&[&keypair0], message, Hash::default()); - let results0 = accounts.lock_accounts([tx.clone()].iter(), MAX_TX_ACCOUNT_LOCKS); + let (results0, self_conflicting_batch) = + accounts.lock_accounts([tx.clone()].iter(), MAX_TX_ACCOUNT_LOCKS, false); + assert!(!self_conflicting_batch); assert_eq!(results0, vec![Ok(())]); assert!(accounts .account_locks @@ -975,7 +1056,9 @@ mod tests { ); let tx1 = new_sanitized_tx(&[&keypair1], message, Hash::default()); let txs = vec![tx0, tx1]; - let results1 = accounts.lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS); + let (results1, self_conflicting_batch) = + accounts.lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS, false); + assert!(!self_conflicting_batch); assert_eq!( results1, vec![ @@ -1001,7 +1084,262 @@ mod tests { instructions, ); let tx = new_sanitized_tx(&[&keypair1], message, Hash::default()); - let results2 = accounts.lock_accounts([tx].iter(), MAX_TX_ACCOUNT_LOCKS); + let (results2, self_conflicting_batch) = + accounts.lock_accounts([tx].iter(), MAX_TX_ACCOUNT_LOCKS, false); + assert!(!self_conflicting_batch); + assert_eq!( + results2, + vec![Ok(())] // Now keypair1 account can be locked as writable + ); + + // Check that read-only lock with zero references is deleted + assert!(!accounts + .account_locks + .lock() + .unwrap() + .is_locked_readonly(&keypair1.pubkey())); + } + + #[test] + fn test_conflicting_accounts_locks() { + let keypair0 = Keypair::new(); + let keypair1 = Keypair::new(); + let keypair2 = Keypair::new(); + let keypair3 = Keypair::new(); + + let account0 = AccountSharedData::new(1, 0, &Pubkey::default()); + let account1 = AccountSharedData::new(2, 0, &Pubkey::default()); + let account2 = AccountSharedData::new(3, 0, &Pubkey::default()); + let account3 = AccountSharedData::new(4, 0, &Pubkey::default()); + + let accounts_db = AccountsDb::new_single_for_tests(); + let accounts = Accounts::new(Arc::new(accounts_db)); + accounts.store_for_tests(0, &keypair0.pubkey(), &account0); + accounts.store_for_tests(0, &keypair1.pubkey(), &account1); + accounts.store_for_tests(0, &keypair2.pubkey(), &account2); + accounts.store_for_tests(0, &keypair3.pubkey(), &account3); + + // no conflicting locks + let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; + let message = Message::new_with_compiled_instructions( + 1, + 0, + 2, + vec![keypair0.pubkey(), keypair1.pubkey(), native_loader::id()], + Hash::default(), + instructions, + ); + let tx = new_sanitized_tx(&[&keypair0], message, Hash::default()); + let (results0, self_conflicting_batch) = + accounts.lock_accounts([tx.clone()].iter(), MAX_TX_ACCOUNT_LOCKS, true); + + assert!(!self_conflicting_batch); + assert_eq!(results0, vec![Ok(())]); + assert!(accounts + .account_locks + .lock() + .unwrap() + .is_locked_readonly(&keypair1.pubkey())); + assert!(accounts + .account_locks + .lock() + .unwrap() + .is_locked_write(&keypair0.pubkey())); + + accounts.unlock_accounts(iter::once(&tx).zip(&results0)); + + // batch write-read conflict. no outstanding conflicting lock + let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; + let message = Message::new_with_compiled_instructions( + 1, + 0, + 2, + vec![keypair2.pubkey(), keypair1.pubkey(), native_loader::id()], + Hash::default(), + instructions, + ); + let tx0 = new_sanitized_tx(&[&keypair2], message, Hash::default()); + let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; + let message = Message::new_with_compiled_instructions( + 1, + 0, + 2, + vec![keypair1.pubkey(), keypair3.pubkey(), native_loader::id()], + Hash::default(), + instructions, + ); + let tx1 = new_sanitized_tx(&[&keypair1], message, Hash::default()); + let txs = vec![tx0, tx1]; + let (results1, self_conflicting_batch) = + accounts.lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS, true); + + assert!(self_conflicting_batch); + assert_eq!( + results1, + vec![ + Ok(()), // Read-only account (keypair1) can be referenced multiple times + Ok(()), // Read-only account (keypair1) should also be locked as writable + ], + ); + assert!(accounts + .account_locks + .lock() + .unwrap() + .is_locked_readonly(&keypair1.pubkey())); + + accounts.unlock_accounts(txs.iter().zip(&results1)); + + // batch write-write conflict. no outstanding conflicting lock + let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; + let message = Message::new_with_compiled_instructions( + 1, + 0, + 2, + vec![keypair1.pubkey(), keypair2.pubkey(), native_loader::id()], + Hash::default(), + instructions, + ); + let tx0 = new_sanitized_tx(&[&keypair1], message, Hash::default()); + let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; + let message = Message::new_with_compiled_instructions( + 1, + 0, + 2, + vec![keypair1.pubkey(), keypair3.pubkey(), native_loader::id()], + Hash::default(), + instructions, + ); + let tx1 = new_sanitized_tx(&[&keypair1], message, Hash::default()); + let txs = vec![tx0, tx1]; + let (results1, self_conflicting_batch) = + accounts.lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS, true); + + assert!(self_conflicting_batch); + assert_eq!( + results1, + vec![ + Ok(()), + Ok(()), // Both transactions acquire same account as writeable + ], + ); + assert!(accounts + .account_locks + .lock() + .unwrap() + .is_locked_write(&keypair1.pubkey())); + + // batch write-write conflict. outstanding (read/write lock) + let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; + let message = Message::new_with_compiled_instructions( + 1, + 0, + 2, + vec![keypair1.pubkey(), keypair2.pubkey(), native_loader::id()], + Hash::default(), + instructions, + ); + let tx0 = new_sanitized_tx(&[&keypair1], message, Hash::default()); + let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; + let message = Message::new_with_compiled_instructions( + 1, + 0, + 2, + vec![keypair0.pubkey(), keypair3.pubkey(), native_loader::id()], + Hash::default(), + instructions, + ); + let tx1 = new_sanitized_tx(&[&keypair0], message, Hash::default()); + let txs2 = vec![tx0, tx1]; + let (results2, self_conflicting_batch) = + accounts.lock_accounts(txs2.iter(), MAX_TX_ACCOUNT_LOCKS, true); + + assert!(!self_conflicting_batch); + // the first tx conflicts with the previous batch + assert_eq!( + results2, + vec![ + Err(TransactionError::AccountInUse), // outstanding write lock + Ok(()), // outstanding read lock, acquired read lock + ], + ); + assert!(accounts + .account_locks + .lock() + .unwrap() + .is_locked_write(&keypair1.pubkey())); + + accounts.unlock_accounts(txs.iter().zip(&results1)); + accounts.unlock_accounts(txs2.iter().zip(&results2)); + + // batch write-read conflict. outstanding (read/write lock) + let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; + let message = Message::new_with_compiled_instructions( + 1, + 0, + 2, + vec![keypair3.pubkey(), keypair0.pubkey(), native_loader::id()], + Hash::default(), + instructions, + ); + let tx = new_sanitized_tx(&[&keypair3], message, Hash::default()); + let (results0, self_conflicting_batch) = + accounts.lock_accounts([tx.clone()].iter(), MAX_TX_ACCOUNT_LOCKS, true); + assert!(!self_conflicting_batch); + + let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; + let message = Message::new_with_compiled_instructions( + 1, + 0, + 2, + vec![keypair2.pubkey(), keypair1.pubkey(), native_loader::id()], + Hash::default(), + instructions, + ); + let tx0 = new_sanitized_tx(&[&keypair2], message, Hash::default()); + let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; + let message = Message::new_with_compiled_instructions( + 1, + 0, + 2, + vec![keypair1.pubkey(), keypair3.pubkey(), native_loader::id()], + Hash::default(), + instructions, + ); + let tx1 = new_sanitized_tx(&[&keypair1], message, Hash::default()); + let txs = vec![tx0, tx1]; + let (results1, self_conflicting_batch) = + accounts.lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS, true); + + assert!(!self_conflicting_batch); + assert_eq!( + results1, + vec![ + Ok(()), + Err(TransactionError::AccountInUse), // acquiring read lock on (keypair3) which is write locked in another batch + ], + ); + assert!(accounts + .account_locks + .lock() + .unwrap() + .is_locked_write(&keypair3.pubkey())); + + accounts.unlock_accounts(iter::once(&tx).zip(&results0)); + accounts.unlock_accounts(txs.iter().zip(&results1)); + + let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; + let message = Message::new_with_compiled_instructions( + 1, + 0, + 2, + vec![keypair1.pubkey(), keypair3.pubkey(), native_loader::id()], + Hash::default(), + instructions, + ); + let tx = new_sanitized_tx(&[&keypair1], message, Hash::default()); + let (results2, self_conflicting_batch) = + accounts.lock_accounts([tx].iter(), MAX_TX_ACCOUNT_LOCKS, false); + assert!(!self_conflicting_batch); assert_eq!( results2, vec![Ok(())] // Now keypair1 account can be locked as writable @@ -1063,9 +1401,10 @@ mod tests { let exit_clone = exit.clone(); thread::spawn(move || loop { let txs = vec![writable_tx.clone()]; - let results = accounts_clone - .clone() - .lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS); + let (results, _) = + accounts_clone + .clone() + .lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS, false); for result in results.iter() { if result.is_ok() { counter_clone.clone().fetch_add(1, Ordering::SeqCst); @@ -1079,9 +1418,10 @@ 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(), MAX_TX_ACCOUNT_LOCKS); + let (results, _) = + accounts_arc + .clone() + .lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS, false); if results[0].is_ok() { let counter_value = counter_clone.clone().load(Ordering::SeqCst); thread::sleep(time::Duration::from_millis(50)); @@ -1122,8 +1462,9 @@ mod tests { instructions, ); let tx = new_sanitized_tx(&[&keypair0], message, Hash::default()); - let results0 = accounts.lock_accounts([tx].iter(), MAX_TX_ACCOUNT_LOCKS); - + let (results0, self_conflicting_batch) = + accounts.lock_accounts([tx].iter(), MAX_TX_ACCOUNT_LOCKS, false); + assert!(!self_conflicting_batch); assert!(results0[0].is_ok()); // Instruction program-id account demoted to readonly assert!(accounts @@ -1219,6 +1560,7 @@ mod tests { txs.iter(), qos_results.into_iter(), MAX_TX_ACCOUNT_LOCKS, + false, ); assert_eq!( diff --git a/core/src/banking_stage/consume_worker.rs b/core/src/banking_stage/consume_worker.rs index 3902ce8829f163..9451d553803db8 100644 --- a/core/src/banking_stage/consume_worker.rs +++ b/core/src/banking_stage/consume_worker.rs @@ -718,7 +718,7 @@ mod tests { vote_sender_types::ReplayVoteReceiver, }, solana_sdk::{ - genesis_config::GenesisConfig, poh_config::PohConfig, pubkey::Pubkey, + feature_set, genesis_config::GenesisConfig, poh_config::PohConfig, pubkey::Pubkey, signature::Keypair, system_transaction, }, std::{ @@ -931,10 +931,17 @@ mod tests { .unwrap(); let consumed = consumed_receiver.recv().unwrap(); - assert_eq!(consumed.work.batch_id, bid); - assert_eq!(consumed.work.ids, vec![id1, id2]); - assert_eq!(consumed.work.max_age_slots, vec![bank.slot(), bank.slot()]); - assert_eq!(consumed.retryable_indexes, vec![1]); // id2 is retryable since lock conflict + + let allow_self_conflicting_txns = bank + .feature_set + .is_active(&feature_set::allow_self_conflicting_entries::id()); + + if !allow_self_conflicting_txns { + assert_eq!(consumed.work.batch_id, bid); + assert_eq!(consumed.work.ids, vec![id1, id2]); + assert_eq!(consumed.work.max_age_slots, vec![bank.slot(), bank.slot()]); + assert_eq!(consumed.retryable_indexes, vec![1]); // id2 is retryable since lock conflict + } drop(test_frame); let _ = worker_thread.join().unwrap(); diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 39e3dc1fd95d5c..8cd47a44db4380 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -882,7 +882,7 @@ mod tests { self, state::{AddressLookupTable, LookupTableMeta}, }, - compute_budget, + compute_budget, feature_set, fee_calculator::FeeCalculator, hash::Hash, instruction::InstructionError, @@ -1555,53 +1555,60 @@ mod tests { retryable_transaction_indexes, .. } = process_transactions_batch_output.execute_and_commit_transactions_output; - assert_eq!(transaction_counts.processed_with_successful_result_count, 1); - assert!(commit_transactions_result.is_ok()); - // first one should have been committed, second one not committed due to AccountInUse error during - // account locking - let commit_transactions_result = commit_transactions_result.unwrap(); - assert_eq!(commit_transactions_result.len(), 2); - assert_matches!( - commit_transactions_result.first(), - Some(CommitTransactionDetails::Committed { .. }) - ); - assert_matches!( - commit_transactions_result.get(1), - Some(CommitTransactionDetails::NotCommitted) - ); - assert_eq!(retryable_transaction_indexes, vec![1]); - - let expected_block_cost = { - let (actual_programs_execution_cost, actual_loaded_accounts_data_size_cost) = - match commit_transactions_result.first().unwrap() { - CommitTransactionDetails::Committed { - compute_units, - loaded_accounts_data_size, - } => ( - *compute_units, - CostModel::calculate_loaded_accounts_data_size_cost( - *loaded_accounts_data_size, - &bank.feature_set, + let allow_self_conflicting_txns = bank + .feature_set + .is_active(&feature_set::allow_self_conflicting_entries::id()); + + if !allow_self_conflicting_txns { + assert_eq!(transaction_counts.processed_with_successful_result_count, 1); + assert!(commit_transactions_result.is_ok()); + + // first one should have been committed, second one not committed due to AccountInUse error during + // account locking + let commit_transactions_result = commit_transactions_result.unwrap(); + assert_eq!(commit_transactions_result.len(), 2); + assert_matches!( + commit_transactions_result.first(), + Some(CommitTransactionDetails::Committed { .. }) + ); + assert_matches!( + commit_transactions_result.get(1), + Some(CommitTransactionDetails::NotCommitted) + ); + assert_eq!(retryable_transaction_indexes, vec![1]); + + let expected_block_cost = { + let (actual_programs_execution_cost, actual_loaded_accounts_data_size_cost) = + match commit_transactions_result.first().unwrap() { + CommitTransactionDetails::Committed { + compute_units, + loaded_accounts_data_size, + } => ( + *compute_units, + CostModel::calculate_loaded_accounts_data_size_cost( + *loaded_accounts_data_size, + &bank.feature_set, + ), ), - ), - CommitTransactionDetails::NotCommitted => { - unreachable!() - } - }; + CommitTransactionDetails::NotCommitted => { + unreachable!() + } + }; - let mut cost = CostModel::calculate_cost(&transactions[0], &bank.feature_set); - if let TransactionCost::Transaction(ref mut usage_cost) = cost { - usage_cost.programs_execution_cost = actual_programs_execution_cost; - usage_cost.loaded_accounts_data_size_cost = - actual_loaded_accounts_data_size_cost; - } + let mut cost = CostModel::calculate_cost(&transactions[0], &bank.feature_set); + if let TransactionCost::Transaction(ref mut usage_cost) = cost { + usage_cost.programs_execution_cost = actual_programs_execution_cost; + usage_cost.loaded_accounts_data_size_cost = + actual_loaded_accounts_data_size_cost; + } - block_cost + cost.sum() - }; + block_cost + cost.sum() + }; - assert_eq!(get_block_cost(), expected_block_cost); - assert_eq!(get_tx_count(), 2); + assert_eq!(get_block_cost(), expected_block_cost); + assert_eq!(get_tx_count(), 2); + } poh_recorder .read() @@ -1680,16 +1687,22 @@ mod tests { .. } = process_transactions_batch_output.execute_and_commit_transactions_output; - assert_eq!( - transaction_counts, - LeaderProcessedTransactionCounts { - attempted_processing_count: 2, - processed_count: 1, - processed_with_successful_result_count: 1, - } - ); - assert_eq!(retryable_transaction_indexes, vec![1]); - assert!(commit_transactions_result.is_ok()); + let allow_self_conflicting_txns = bank + .feature_set + .is_active(&feature_set::allow_self_conflicting_entries::id()); + + if !allow_self_conflicting_txns { + assert_eq!( + transaction_counts, + LeaderProcessedTransactionCounts { + attempted_processing_count: 2, + processed_count: 1, + processed_with_successful_result_count: 1, + } + ); + assert_eq!(retryable_transaction_indexes, vec![1]); + assert!(commit_transactions_result.is_ok()); + } } Blockstore::destroy(ledger_path.path()).unwrap(); } @@ -1737,25 +1750,31 @@ mod tests { transaction_counts, retryable_transaction_indexes, .. - } = execute_transactions_with_dummy_poh_service(bank, transactions); + } = execute_transactions_with_dummy_poh_service(bank.clone(), transactions); - // All the transactions should have been replayed, but only 1 committed - assert!(!reached_max_poh_height); - assert_eq!( - transaction_counts, - CommittedTransactionsCounts { - attempted_processing_count: transactions_len as u64, - // Both transactions should have been committed, even though one was an error, - // because InstructionErrors are committed - committed_transactions_count: 2, - committed_transactions_with_successful_result_count: 1, - processed_but_failed_commit: 0, - } - ); - assert_eq!( - retryable_transaction_indexes, - (1..transactions_len - 1).collect::>() - ); + let allow_self_conflicting_txns = bank + .feature_set + .is_active(&feature_set::allow_self_conflicting_entries::id()); + + if !allow_self_conflicting_txns { + // All the transactions should have been replayed, but only 1 committed + assert!(!reached_max_poh_height); + assert_eq!( + transaction_counts, + CommittedTransactionsCounts { + attempted_processing_count: transactions_len as u64, + // Both transactions should have been committed, even though one was an error, + // because InstructionErrors are committed + committed_transactions_count: 2, + committed_transactions_with_successful_result_count: 1, + processed_but_failed_commit: 0, + } + ); + assert_eq!( + retryable_transaction_indexes, + (1..transactions_len - 1).collect::>() + ); + } } #[test] @@ -1798,25 +1817,31 @@ mod tests { transaction_counts, retryable_transaction_indexes, .. - } = execute_transactions_with_dummy_poh_service(bank, transactions); + } = execute_transactions_with_dummy_poh_service(bank.clone(), transactions); - // All the transactions should have been replayed, but only 2 committed (first and last) - assert!(!reached_max_poh_height); - assert_eq!( - transaction_counts, - CommittedTransactionsCounts { - attempted_processing_count: transactions_len as u64, - committed_transactions_count: 2, - committed_transactions_with_successful_result_count: 2, - processed_but_failed_commit: 0, - } - ); + let allow_self_conflicting_txns = bank + .feature_set + .is_active(&feature_set::allow_self_conflicting_entries::id()); - // Everything except first and last index of the transactions failed and are last retryable - assert_eq!( - retryable_transaction_indexes, - (1..transactions_len - 1).collect::>() - ); + if !allow_self_conflicting_txns { + // All the transactions should have been replayed, but only 2 committed (first and last) + assert!(!reached_max_poh_height); + assert_eq!( + transaction_counts, + CommittedTransactionsCounts { + attempted_processing_count: transactions_len as u64, + committed_transactions_count: 2, + committed_transactions_with_successful_result_count: 2, + processed_but_failed_commit: 0, + } + ); + + // Everything except first and last index of the transactions failed and are last retryable + assert_eq!( + retryable_transaction_indexes, + (1..transactions_len - 1).collect::>() + ); + } } #[test] @@ -2359,9 +2384,14 @@ mod tests { 1, bank.last_blockhash(), )); + let allow_self_conflicting_txns = bank + .feature_set + .is_active(&feature_set::allow_self_conflicting_entries::id()); + let _ = bank_start.working_bank.accounts().lock_accounts( std::iter::once(&manual_lock_tx), bank_start.working_bank.get_transaction_account_lock_limit(), + allow_self_conflicting_txns, ); let banking_stage_stats = BankingStageStats::default(); diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 5273b4601bfe33..b6be62eab79598 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -644,7 +644,8 @@ fn process_entries( (starting_index..starting_index.saturating_add(transactions.len())).collect(); loop { // try to lock the accounts - let batch = bank.prepare_sanitized_batch(transactions); + let (batch, _self_conflicting_batch) = + bank.prepare_sanitized_batch(transactions); let first_lock_err = first_err(batch.lock_results()); // if locking worked @@ -3532,20 +3533,26 @@ pub mod tests { // keypair2=3 // keypair3=3 - assert!(process_entries_for_tests_without_scheduler( - &bank, - vec![ - entry_1_to_mint, - entry_2_to_3_and_1_to_mint, - entry_conflict_itself, - ], - ) - .is_err()); + let allow_self_conflicting_txns = bank + .feature_set + .is_active(&feature_set::allow_self_conflicting_entries::id()); - // last entry should have been aborted before par_execute_entries - assert_eq!(bank.get_balance(&keypair1.pubkey()), 2); - assert_eq!(bank.get_balance(&keypair2.pubkey()), 2); - assert_eq!(bank.get_balance(&keypair3.pubkey()), 2); + if !allow_self_conflicting_txns { + assert!(process_entries_for_tests_without_scheduler( + &bank, + vec![ + entry_1_to_mint, + entry_2_to_3_and_1_to_mint, + entry_conflict_itself, + ], + ) + .is_err()); + + // last entry should have been aborted before par_execute_entries + assert_eq!(bank.get_balance(&keypair1.pubkey()), 2); + assert_eq!(bank.get_balance(&keypair2.pubkey()), 2); + assert_eq!(bank.get_balance(&keypair3.pubkey()), 2); + } } #[test] @@ -3887,13 +3894,19 @@ pub mod tests { ], ); - assert_eq!( - process_entries_for_tests_without_scheduler(&bank, vec![entry_1_to_mint]), - Err(TransactionError::AccountInUse) - ); + let allow_self_conflicting_txns = bank + .feature_set + .is_active(&feature_set::allow_self_conflicting_entries::id()); - // Should not see duplicate signature error - assert_eq!(bank.process_transaction(&fail_tx), Ok(())); + if !allow_self_conflicting_txns { + assert_eq!( + process_entries_for_tests_without_scheduler(&bank, vec![entry_1_to_mint]), + Err(TransactionError::AccountInUse) + ); + + // Should not see duplicate signature error + assert_eq!(bank.process_transaction(&fail_tx), Ok(())); + } } #[test] @@ -4800,7 +4813,7 @@ pub mod tests { } = create_genesis_config_with_leader(500, &dummy_leader_pubkey, 100); let bank = Arc::new(Bank::new_for_tests(&genesis_config)); let txs = create_test_transactions(&mint_keypair, &genesis_config.hash()); - let batch = bank.prepare_sanitized_batch(&txs); + let (batch, _) = bank.prepare_sanitized_batch(&txs); assert!(batch.needs_unlock()); let transaction_indexes = vec![42, 43, 44]; @@ -4885,7 +4898,7 @@ pub mod tests { }); let bank = BankWithScheduler::new(bank, Some(Box::new(mocked_scheduler))); - let batch = bank.prepare_sanitized_batch(&txs); + let (batch, _self_conflicting_batch) = bank.prepare_sanitized_batch(&txs); let batch_with_indexes = TransactionBatchWithIndexes { batch, transaction_indexes: (0..txs.len()).collect(), diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 7ad9c63ee0e468..957d170057f68f 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3244,10 +3244,14 @@ impl Bank { }) .collect::>>()?; let tx_account_lock_limit = self.get_transaction_account_lock_limit(); - let lock_results = self - .rc - .accounts - .lock_accounts(sanitized_txs.iter(), tx_account_lock_limit); + let allow_self_conflicting_txns = self + .feature_set + .is_active(&feature_set::allow_self_conflicting_entries::id()); + let (lock_results, _self_conflicting_batch) = self.rc.accounts.lock_accounts( + sanitized_txs.iter(), + tx_account_lock_limit, + allow_self_conflicting_txns, + ); Ok(TransactionBatch::new( lock_results, self, @@ -3259,13 +3263,21 @@ impl Bank { pub fn prepare_sanitized_batch<'a, 'b>( &'a self, txs: &'b [SanitizedTransaction], - ) -> TransactionBatch<'a, 'b> { + ) -> (TransactionBatch<'a, 'b>, bool) { let tx_account_lock_limit = self.get_transaction_account_lock_limit(); - let lock_results = self - .rc - .accounts - .lock_accounts(txs.iter(), tx_account_lock_limit); - TransactionBatch::new(lock_results, self, Cow::Borrowed(txs)) + let allow_self_conflicting_txns = self + .feature_set + .is_active(&feature_set::allow_self_conflicting_entries::id()); + + let (lock_results, self_conflicting_batch) = self.rc.accounts.lock_accounts( + txs.iter(), + tx_account_lock_limit, + allow_self_conflicting_txns, + ); + ( + TransactionBatch::new(lock_results, self, Cow::Borrowed(txs)), + self_conflicting_batch, + ) } /// Prepare a locked transaction batch from a list of sanitized transactions, and their cost @@ -3277,10 +3289,15 @@ impl Bank { ) -> TransactionBatch<'a, 'b> { // this lock_results could be: Ok, AccountInUse, WouldExceedBlockMaxLimit or WouldExceedAccountMaxLimit let tx_account_lock_limit = self.get_transaction_account_lock_limit(); + let allow_self_conflicting_txns = self + .feature_set + .is_active(&feature_set::allow_self_conflicting_entries::id()); + let lock_results = self.rc.accounts.lock_accounts_with_results( transactions.iter(), transaction_results, tx_account_lock_limit, + allow_self_conflicting_txns, ); TransactionBatch::new(lock_results, self, Cow::Borrowed(transactions)) } @@ -6702,10 +6719,15 @@ impl Bank { .into_iter() .map(SanitizedTransaction::from_transaction_for_tests) .collect::>(); - let lock_results = self - .rc - .accounts - .lock_accounts(sanitized_txs.iter(), transaction_account_lock_limit); + let allow_self_conflicting_txns = self + .feature_set + .is_active(&feature_set::allow_self_conflicting_entries::id()); + + let (lock_results, _) = self.rc.accounts.lock_accounts( + sanitized_txs.iter(), + transaction_account_lock_limit, + allow_self_conflicting_txns, + ); TransactionBatch::new(lock_results, self, Cow::Owned(sanitized_txs)) } diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 56612762743f97..cfd82bd9587e2d 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -3048,9 +3048,9 @@ fn test_readonly_accounts() { ); let txs = vec![tx0, tx1]; let results = bank.process_transactions(txs.iter()); - // However, an account may not be locked as read-only and writable at the same time. + // An account can be locked as read-only and writable at the same time "in the same entry" after SIMD#83. assert_eq!(results[0], Ok(())); - assert_eq!(results[1], Err(TransactionError::AccountInUse)); + assert_eq!(results[1], Ok(())); } #[test] @@ -9228,7 +9228,8 @@ fn test_tx_log_order() { .as_ref() .unwrap()[2] .contains(&"failed".to_string())); - assert!(commit_results[2].is_err()); + // this tx conflicts with the first tx + assert!(commit_results[2].is_ok()); let stored_logs = &bank.transaction_log_collector.read().unwrap().logs; let success_log_info = stored_logs diff --git a/runtime/src/transaction_batch.rs b/runtime/src/transaction_batch.rs index ecec27e02e93aa..c26382ee40796d 100644 --- a/runtime/src/transaction_batch.rs +++ b/runtime/src/transaction_batch.rs @@ -100,7 +100,9 @@ mod tests { use { super::*, crate::genesis_utils::{create_genesis_config_with_leader, GenesisConfigInfo}, - solana_sdk::{signature::Keypair, system_transaction, transaction::TransactionError}, + solana_sdk::{ + feature_set, signature::Keypair, system_transaction, transaction::TransactionError, + }, }; #[test] @@ -108,21 +110,30 @@ mod tests { let (bank, txs) = setup(false); // Test getting locked accounts - let batch = bank.prepare_sanitized_batch(&txs); + let (batch, self_conflicting_batch) = bank.prepare_sanitized_batch(&txs); // Grab locks assert!(batch.lock_results().iter().all(|x| x.is_ok())); + // Not conflicting batch + assert!(!self_conflicting_batch); + // Trying to grab locks again should fail - let batch2 = bank.prepare_sanitized_batch(&txs); + let (batch2, self_conflicting_batch) = bank.prepare_sanitized_batch(&txs); assert!(batch2.lock_results().iter().all(|x| x.is_err())); + // Not conflicting batch + assert!(!self_conflicting_batch); + // Drop the first set of locks drop(batch); // Now grabbing locks should work again - let batch2 = bank.prepare_sanitized_batch(&txs); + let (batch2, self_conflicting_batch) = bank.prepare_sanitized_batch(&txs); assert!(batch2.lock_results().iter().all(|x| x.is_ok())); + + // Not conflicting batch + assert!(!self_conflicting_batch); } #[test] @@ -134,8 +145,10 @@ mod tests { assert!(batch.lock_results().iter().all(|x| x.is_ok())); // Grab locks - let batch2 = bank.prepare_sanitized_batch(&txs); + let (batch2, self_conflicting_batch) = bank.prepare_sanitized_batch(&txs); assert!(batch2.lock_results().iter().all(|x| x.is_ok())); + // Not conflicting batch + assert!(!self_conflicting_batch); // Prepare another batch without locks let batch3 = bank.prepare_unlocked_batch_from_single_tx(&txs[0]); @@ -146,18 +159,38 @@ mod tests { fn test_unlock_failures() { let (bank, txs) = setup(true); + let allow_self_conflicting_txns = bank + .feature_set + .is_active(&feature_set::allow_self_conflicting_entries::id()); + // Test getting locked accounts - let mut batch = bank.prepare_sanitized_batch(&txs); - assert_eq!( - batch.lock_results, - vec![Ok(()), Err(TransactionError::AccountInUse), Ok(())] - ); - - let qos_results = vec![ - Ok(()), - Err(TransactionError::AccountInUse), - Err(TransactionError::WouldExceedMaxBlockCostLimit), - ]; + let (mut batch, self_conflicting_batch) = bank.prepare_sanitized_batch(&txs); + if !allow_self_conflicting_txns { + assert_eq!( + batch.lock_results, + vec![Ok(()), Err(TransactionError::AccountInUse), Ok(())] + ); + // Not conflicting batch + assert!(!self_conflicting_batch); + } else { + assert_eq!(batch.lock_results, vec![Ok(()), Ok(()), Ok(())]); + // conflicting batch + assert!(self_conflicting_batch); + } + + let qos_results = if !allow_self_conflicting_txns { + vec![ + Ok(()), + Err(TransactionError::AccountInUse), + Err(TransactionError::WouldExceedMaxBlockCostLimit), + ] + } else { + vec![ + Ok(()), + Ok(()), + Err(TransactionError::WouldExceedMaxBlockCostLimit), + ] + }; batch.unlock_failures(qos_results.clone()); assert_eq!(batch.lock_results, qos_results); @@ -165,11 +198,19 @@ mod tests { drop(batch); // The next batch should be able to lock all but the conflicting tx - let batch2 = bank.prepare_sanitized_batch(&txs); - assert_eq!( - batch2.lock_results, - vec![Ok(()), Err(TransactionError::AccountInUse), Ok(())] - ); + let (batch2, self_conflicting_batch) = bank.prepare_sanitized_batch(&txs); + if !allow_self_conflicting_txns { + assert_eq!( + batch2.lock_results, + vec![Ok(()), Err(TransactionError::AccountInUse), Ok(())] + ); + // Not conflicting batch + assert!(!self_conflicting_batch); + } else { + assert_eq!(batch2.lock_results, vec![Ok(()), Ok(()), Ok(())]); + // Not conflicting batch + assert!(self_conflicting_batch); + } } fn setup(insert_conflicting_tx: bool) -> (Bank, Vec) { diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 02cc2594d30593..aaa58655d3c6b6 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -861,6 +861,10 @@ pub mod deprecate_legacy_vote_ixs { solana_sdk::declare_id!("depVvnQ2UysGrhwdiwU42tCadZL8GcBb1i2GYhMopQv"); } +pub mod allow_self_conflicting_entries { + solana_sdk::declare_id!("ENTRYnPAoT5Swwx73YDGzMp3XnNH1kxacyvLosRHza1i"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -1071,6 +1075,7 @@ lazy_static! { (enable_turbine_extended_fanout_experiments::id(), "enable turbine extended fanout experiments #"), (deprecate_legacy_vote_ixs::id(), "Deprecate legacy vote instructions"), (partitioned_epoch_rewards_superfeature::id(), "replaces enable_partitioned_epoch_reward to enable partitioned rewards at epoch boundary SIMD-0118"), + (allow_self_conflicting_entries::id(), "Allow entries with conflicting transactions #1025"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter()