Skip to content

Commit

Permalink
Increase transaction account lock limit from 64 to 128 (solana-labs#2…
Browse files Browse the repository at this point in the history 
…7242)
  • Loading branch information
@jstarry
jstarry committed Sep 15, 2022
1 parent 0fafcce commit bbe22ea
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 37 deletions.
33 changes: 17 additions & 16 deletions runtime/src/accounts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1090,10 +1090,11 @@ impl Accounts {
pub fn lock_accounts<'a>(
&self,
txs: impl Iterator<Item = &'a SanitizedTransaction>,
feature_set: &FeatureSet,
tx_account_lock_limit: usize,
) -> Vec<Result<()>> {
let tx_account_locks_results: Vec<Result<_>> =
txs.map(|tx| tx.get_account_locks(feature_set)).collect();
let tx_account_locks_results: Vec<Result<_>> = txs
.map(|tx| tx.get_account_locks(tx_account_lock_limit))
.collect();
self.lock_accounts_inner(tx_account_locks_results)
}

Expand All @@ -1103,12 +1104,12 @@ impl Accounts {
&self,
txs: impl Iterator<Item = &'a SanitizedTransaction>,
results: impl Iterator<Item = &'a Result<()>>,
feature_set: &FeatureSet,
tx_account_lock_limit: usize,
) -> Vec<Result<()>> {
let tx_account_locks_results: Vec<Result<_>> = txs
.zip(results)
.map(|(tx, result)| match result {
Ok(()) => tx.get_account_locks(feature_set),
Ok(()) => tx.get_account_locks(tx_account_lock_limit),
Err(err) => Err(err.clone()),
})
.collect();
Expand Down Expand Up @@ -2479,7 +2480,7 @@ mod tests {
};

let tx = new_sanitized_tx(&[&keypair], message, Hash::default());
let results = accounts.lock_accounts([tx].iter(), &FeatureSet::all_enabled());
let results = accounts.lock_accounts([tx].iter(), MAX_TX_ACCOUNT_LOCKS);
assert_eq!(results[0], Err(TransactionError::AccountLoadedTwice));
}

Expand Down Expand Up @@ -2512,7 +2513,7 @@ mod tests {
};

let txs = vec![new_sanitized_tx(&[&keypair], message, Hash::default())];
let results = accounts.lock_accounts(txs.iter(), &FeatureSet::all_enabled());
let results = accounts.lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS);
assert_eq!(results[0], Ok(()));
accounts.unlock_accounts(txs.iter(), &results);
}
Expand All @@ -2534,7 +2535,7 @@ mod tests {
};

let txs = vec![new_sanitized_tx(&[&keypair], message, Hash::default())];
let results = accounts.lock_accounts(txs.iter(), &FeatureSet::default());
let results = accounts.lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS);
assert_eq!(results[0], Ok(()));
accounts.unlock_accounts(txs.iter(), &results);
}
Expand All @@ -2556,7 +2557,7 @@ mod tests {
};

let txs = vec![new_sanitized_tx(&[&keypair], message, Hash::default())];
let results = accounts.lock_accounts(txs.iter(), &FeatureSet::all_enabled());
let results = accounts.lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS);
assert_eq!(results[0], Err(TransactionError::TooManyAccountLocks));
}
}
Expand Down Expand Up @@ -2595,7 +2596,7 @@ mod tests {
instructions,
);
let tx = new_sanitized_tx(&[&keypair0], message, Hash::default());
let results0 = accounts.lock_accounts([tx.clone()].iter(), &FeatureSet::all_enabled());
let results0 = accounts.lock_accounts([tx.clone()].iter(), MAX_TX_ACCOUNT_LOCKS);

assert!(results0[0].is_ok());
assert_eq!(
Expand Down Expand Up @@ -2630,7 +2631,7 @@ mod tests {
);
let tx1 = new_sanitized_tx(&[&keypair1], message, Hash::default());
let txs = vec![tx0, tx1];
let results1 = accounts.lock_accounts(txs.iter(), &FeatureSet::all_enabled());
let results1 = accounts.lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS);

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
Expand All @@ -2657,7 +2658,7 @@ mod tests {
instructions,
);
let tx = new_sanitized_tx(&[&keypair1], message, Hash::default());
let results2 = accounts.lock_accounts([tx].iter(), &FeatureSet::all_enabled());
let results2 = accounts.lock_accounts([tx].iter(), MAX_TX_ACCOUNT_LOCKS);
assert!(results2[0].is_ok()); // Now keypair1 account can be locked as writable

// Check that read-only lock with zero references is deleted
Expand Down Expand Up @@ -2728,7 +2729,7 @@ mod tests {
let txs = vec![writable_tx.clone()];
let results = accounts_clone
.clone()
.lock_accounts(txs.iter(), &FeatureSet::all_enabled());
.lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS);
for result in results.iter() {
if result.is_ok() {
counter_clone.clone().fetch_add(1, Ordering::SeqCst);
Expand All @@ -2745,7 +2746,7 @@ mod tests {
let txs = vec![readonly_tx.clone()];
let results = accounts_arc
.clone()
.lock_accounts(txs.iter(), &FeatureSet::all_enabled());
.lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS);
if results[0].is_ok() {
let counter_value = counter_clone.clone().load(Ordering::SeqCst);
thread::sleep(time::Duration::from_millis(50));
Expand Down Expand Up @@ -2791,7 +2792,7 @@ mod tests {
instructions,
);
let tx = new_sanitized_tx(&[&keypair0], message, Hash::default());
let results0 = accounts.lock_accounts([tx].iter(), &FeatureSet::all_enabled());
let results0 = accounts.lock_accounts([tx].iter(), MAX_TX_ACCOUNT_LOCKS);

assert!(results0[0].is_ok());
// Instruction program-id account demoted to readonly
Expand Down Expand Up @@ -2885,7 +2886,7 @@ mod tests {
let results = accounts.lock_accounts_with_results(
txs.iter(),
qos_results.iter(),
&FeatureSet::all_enabled(),
MAX_TX_ACCOUNT_LOCKS,
);

assert!(results[0].is_ok()); // Read-only account (keypair0) can be referenced multiple times
Expand Down
42 changes: 28 additions & 14 deletions runtime/src/bank.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ use {
timing::years_as_slots,
transaction::{
MessageHash, Result, SanitizedTransaction, Transaction, TransactionError,
TransactionVerificationMode, VersionedTransaction,
TransactionVerificationMode, VersionedTransaction, MAX_TX_ACCOUNT_LOCKS,
},
transaction_context::{InstructionTrace, TransactionAccount, TransactionContext},
},
Expand Down Expand Up @@ -3510,16 +3510,28 @@ impl Bank {
tick_height % self.ticks_per_slot == 0
}

/// Get the max number of accounts that a transaction may lock in this block
pub fn get_transaction_account_lock_limit(&self) -> usize {
if self
.feature_set
.is_active(&feature_set::increase_tx_account_lock_limit::id())
{
MAX_TX_ACCOUNT_LOCKS
} else {
64
}
}

/// Prepare a transaction batch from a list of legacy transactions. Used for tests only.
pub fn prepare_batch_for_tests(&self, txs: Vec<Transaction>) -> TransactionBatch {
let sanitized_txs = txs
.into_iter()
.map(SanitizedTransaction::from_transaction_for_tests)
.collect::<Vec<_>>();
let lock_results = self
.rc
.accounts
.lock_accounts(sanitized_txs.iter(), &FeatureSet::all_enabled());
let lock_results = self.rc.accounts.lock_accounts(
sanitized_txs.iter(),
self.get_transaction_account_lock_limit(),
);
TransactionBatch::new(lock_results, self, Cow::Owned(sanitized_txs))
}

Expand All @@ -3539,10 +3551,10 @@ impl Bank {
)
})
.collect::<Result<Vec<_>>>()?;
let lock_results = self
.rc
.accounts
.lock_accounts(sanitized_txs.iter(), &FeatureSet::all_enabled());
let lock_results = self.rc.accounts.lock_accounts(
sanitized_txs.iter(),
self.get_transaction_account_lock_limit(),
);
Ok(TransactionBatch::new(
lock_results,
self,
Expand All @@ -3558,7 +3570,7 @@ impl Bank {
let lock_results = self
.rc
.accounts
.lock_accounts(txs.iter(), &self.feature_set);
.lock_accounts(txs.iter(), self.get_transaction_account_lock_limit());
TransactionBatch::new(lock_results, self, Cow::Borrowed(txs))
}

Expand All @@ -3573,7 +3585,7 @@ impl Bank {
let lock_results = self.rc.accounts.lock_accounts_with_results(
transactions.iter(),
transaction_results,
&self.feature_set,
self.get_transaction_account_lock_limit(),
);
TransactionBatch::new(lock_results, self, Cow::Borrowed(transactions))
}
Expand All @@ -3583,7 +3595,9 @@ impl Bank {
&'a self,
transaction: SanitizedTransaction,
) -> TransactionBatch<'a, '_> {
let lock_result = transaction.get_account_locks(&self.feature_set).map(|_| ());
let lock_result = transaction
.get_account_locks(self.get_transaction_account_lock_limit())
.map(|_| ());
let mut batch =
TransactionBatch::new(vec![lock_result], self, Cow::Owned(vec![transaction]));
batch.set_needs_unlock(false);
Expand Down Expand Up @@ -7007,7 +7021,6 @@ pub(crate) mod tests {
system_program,
sysvar::rewards::Rewards,
timing::duration_as_s,
transaction::MAX_TX_ACCOUNT_LOCKS,
transaction_context::InstructionContext,
},
solana_vote_program::{
Expand Down Expand Up @@ -13060,7 +13073,8 @@ pub(crate) mod tests {
bank.last_blockhash(),
);

while tx.message.account_keys.len() <= MAX_TX_ACCOUNT_LOCKS {
let transaction_account_lock_limit = bank.get_transaction_account_lock_limit();
while tx.message.account_keys.len() <= transaction_account_lock_limit {
tx.message.account_keys.push(solana_sdk::pubkey::new_rand());
}

Expand Down
5 changes: 5 additions & 0 deletions sdk/src/feature_set.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,10 @@ pub mod check_ping_ancestor_requests {
solana_sdk::declare_id!("AXLB87anNaUQtqBSsxkm4gvNzYY985aLtNtpJC94uWLJ");
}

pub mod increase_tx_account_lock_limit {
solana_sdk::declare_id!("9LZdXeKGeBV6hRLdxS1rHbHoEUsKqesCC2ZAPTPKJAbK");
}

lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
Expand Down Expand Up @@ -518,6 +522,7 @@ lazy_static! {
(prevent_crediting_accounts_that_end_rent_paying::id(), "prevent crediting rent paying accounts #26606"),
(sign_repair_requests::id(), "sign repair requests #26834"),
(check_ping_ancestor_requests::id(), "ancestor hash repair socket ping/pong support #26963"),
(increase_tx_account_lock_limit::id(), "increase tx account lock limit to 128 #27241"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()
Expand Down
12 changes: 5 additions & 7 deletions sdk/src/transaction/sanitized.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ use {
};

/// 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;
/// 128 was chosen because it is the minimum number of accounts
/// needed for the Neon EVM implementation.
pub const MAX_TX_ACCOUNT_LOCKS: usize = 128;

/// Sanitized transaction and the hash of its message
#[derive(Debug, Clone)]
Expand Down Expand Up @@ -210,13 +210,11 @@ impl SanitizedTransaction {
/// Validate and return the account keys locked by this transaction
pub fn get_account_locks(
&self,
feature_set: &feature_set::FeatureSet,
tx_account_lock_limit: usize,
) -> Result<TransactionAccountLocks> {
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
{
} else if self.message.account_keys().len() > tx_account_lock_limit {
Err(TransactionError::TooManyAccountLocks)
} else {
Ok(self.get_account_locks_unchecked())
Expand Down

0 comments on commit bbe22ea

Please sign in to comment.