Clean up `max_tx_account_locks` feature (#26440)

Clean up max_tx_account_locks feature
This commit is contained in:
Justin Starry 2022-07-06 17:06:03 +02:00 committed by GitHub
parent 5afe4d938d
commit f8dccd4602
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 23 additions and 70 deletions

View File

@ -1108,10 +1108,9 @@ impl Accounts {
pub fn lock_accounts<'a>(
&self,
txs: impl Iterator<Item = &'a SanitizedTransaction>,
feature_set: &FeatureSet,
) -> Vec<Result<()>> {
let tx_account_locks_results: Vec<Result<_>> =
txs.map(|tx| tx.get_account_locks(feature_set)).collect();
txs.map(|tx| tx.get_account_locks()).collect();
self.lock_accounts_inner(tx_account_locks_results)
}
@ -1121,12 +1120,11 @@ impl Accounts {
&self,
txs: impl Iterator<Item = &'a SanitizedTransaction>,
results: impl Iterator<Item = &'a Result<()>>,
feature_set: &FeatureSet,
) -> 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(),
Err(err) => Err(err.clone()),
})
.collect();
@ -2499,7 +2497,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());
assert_eq!(results[0], Err(TransactionError::AccountLoadedTwice));
}
@ -2532,12 +2530,12 @@ 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());
assert_eq!(results[0], Ok(()));
accounts.unlock_accounts(txs.iter(), &results);
}
// Allow over MAX_TX_ACCOUNT_LOCKS before feature activation
// Disallow over MAX_TX_ACCOUNT_LOCKS
{
let num_account_keys = MAX_TX_ACCOUNT_LOCKS + 1;
let mut account_keys: Vec<_> = (0..num_account_keys)
@ -2554,29 +2552,7 @@ mod tests {
};
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());
let results = accounts.lock_accounts(txs.iter());
assert_eq!(results[0], Err(TransactionError::TooManyAccountLocks));
}
}
@ -2615,7 +2591,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());
assert!(results0[0].is_ok());
assert_eq!(
@ -2650,7 +2626,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());
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
@ -2677,7 +2653,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());
assert!(results2[0].is_ok()); // Now keypair1 account can be locked as writable
// Check that read-only lock with zero references is deleted
@ -2746,9 +2722,7 @@ mod tests {
let exit_clone = exit_clone.clone();
loop {
let txs = vec![writable_tx.clone()];
let results = accounts_clone
.clone()
.lock_accounts(txs.iter(), &FeatureSet::all_enabled());
let results = accounts_clone.clone().lock_accounts(txs.iter());
for result in results.iter() {
if result.is_ok() {
counter_clone.clone().fetch_add(1, Ordering::SeqCst);
@ -2763,9 +2737,7 @@ 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(), &FeatureSet::all_enabled());
let results = accounts_arc.clone().lock_accounts(txs.iter());
if results[0].is_ok() {
let counter_value = counter_clone.clone().load(Ordering::SeqCst);
thread::sleep(time::Duration::from_millis(50));
@ -2811,7 +2783,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());
assert!(results0[0].is_ok());
// Instruction program-id account demoted to readonly
@ -2902,11 +2874,7 @@ mod tests {
Ok(()),
];
let results = accounts.lock_accounts_with_results(
txs.iter(),
qos_results.iter(),
&FeatureSet::all_enabled(),
);
let results = accounts.lock_accounts_with_results(txs.iter(), qos_results.iter());
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()

View File

@ -3866,10 +3866,7 @@ impl Bank {
.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());
TransactionBatch::new(lock_results, self, Cow::Owned(sanitized_txs))
}
@ -3889,10 +3886,7 @@ 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());
Ok(TransactionBatch::new(
lock_results,
self,
@ -3905,10 +3899,7 @@ impl Bank {
&'a self,
txs: &'b [SanitizedTransaction],
) -> TransactionBatch<'a, 'b> {
let lock_results = self
.rc
.accounts
.lock_accounts(txs.iter(), &self.feature_set);
let lock_results = self.rc.accounts.lock_accounts(txs.iter());
TransactionBatch::new(lock_results, self, Cow::Borrowed(txs))
}
@ -3920,11 +3911,10 @@ impl Bank {
transaction_results: impl Iterator<Item = &'b Result<()>>,
) -> 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,
&self.feature_set,
);
let lock_results = self
.rc
.accounts
.lock_accounts_with_results(transactions.iter(), transaction_results);
TransactionBatch::new(lock_results, self, Cow::Borrowed(transactions))
}
@ -3933,7 +3923,7 @@ 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().map(|_| ());
let mut batch =
TransactionBatch::new(vec![lock_result], self, Cow::Owned(vec![transaction]));
batch.set_needs_unlock(false);

View File

@ -208,15 +208,10 @@ impl SanitizedTransaction {
}
/// Validate and return the account keys locked by this transaction
pub fn get_account_locks(
&self,
feature_set: &feature_set::FeatureSet,
) -> Result<TransactionAccountLocks> {
pub fn get_account_locks(&self) -> 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() > MAX_TX_ACCOUNT_LOCKS {
Err(TransactionError::TooManyAccountLocks)
} else {
Ok(self.get_account_locks_unchecked())