Skip to content

Commit

Permalink
Allow overriding the runtime transaction account lock limit (#26948)
Browse files Browse the repository at this point in the history
* Add --transaction-account-lock-limit cli arg to test-validator

* Allow overriding the tx account lock limit in ProgramTest
  • Loading branch information
jstarry authored Aug 12, 2022
1 parent 7343679 commit 5618e9f
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 24 deletions.
8 changes: 8 additions & 0 deletions program-test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,7 @@ pub struct ProgramTest {
prefer_bpf: bool,
use_bpf_jit: bool,
deactivate_feature_set: HashSet<Pubkey>,
transaction_account_lock_limit: Option<usize>,
}

impl Default for ProgramTest {
Expand Down Expand Up @@ -468,6 +469,7 @@ impl Default for ProgramTest {
prefer_bpf,
use_bpf_jit: false,
deactivate_feature_set: HashSet::default(),
transaction_account_lock_limit: None,
}
}
}
Expand Down Expand Up @@ -500,6 +502,11 @@ impl ProgramTest {
self.compute_max_units = Some(compute_max_units);
}

/// Override the default transaction account lock limit
pub fn set_transaction_account_lock_limit(&mut self, transaction_account_lock_limit: usize) {
self.transaction_account_lock_limit = Some(transaction_account_lock_limit);
}

/// Override the BPF compute budget
#[allow(deprecated)]
#[deprecated(since = "1.8.0", note = "please use `set_compute_max_units` instead")]
Expand Down Expand Up @@ -779,6 +786,7 @@ impl ProgramTest {
compute_unit_limit: max_units,
..ComputeBudget::default()
}),
transaction_account_lock_limit: self.transaction_account_lock_limit,
..RuntimeConfig::default()
}),
);
Expand Down
37 changes: 24 additions & 13 deletions runtime/src/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1119,9 +1119,11 @@ impl Accounts {
pub fn lock_accounts<'a>(
&self,
txs: impl Iterator<Item = &'a SanitizedTransaction>,
tx_account_lock_limit: usize,
) -> Vec<Result<()>> {
let tx_account_locks_results: Vec<Result<_>> =
txs.map(|tx| tx.get_account_locks()).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 @@ -1131,11 +1133,12 @@ impl Accounts {
&self,
txs: impl Iterator<Item = &'a SanitizedTransaction>,
results: impl Iterator<Item = &'a Result<()>>,
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(),
Ok(()) => tx.get_account_locks(tx_account_lock_limit),
Err(err) => Err(err.clone()),
})
.collect();
Expand Down Expand Up @@ -2506,7 +2509,7 @@ mod tests {
};

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

Expand Down Expand Up @@ -2539,7 +2542,7 @@ mod tests {
};

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

let txs = vec![new_sanitized_tx(&[&keypair], message, Hash::default())];
let results = accounts.lock_accounts(txs.iter());
let results = accounts.lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS);
assert_eq!(results[0], Err(TransactionError::TooManyAccountLocks));
}
}
Expand Down Expand Up @@ -2600,7 +2603,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(), MAX_TX_ACCOUNT_LOCKS);

assert!(results0[0].is_ok());
assert_eq!(
Expand Down Expand Up @@ -2635,7 +2638,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(), 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 @@ -2662,7 +2665,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(), 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 @@ -2731,7 +2734,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(), MAX_TX_ACCOUNT_LOCKS);
for result in results.iter() {
if result.is_ok() {
counter_clone.clone().fetch_add(1, Ordering::SeqCst);
Expand All @@ -2746,7 +2751,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(), 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 @@ -2792,7 +2799,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(), MAX_TX_ACCOUNT_LOCKS);

assert!(results0[0].is_ok());
// Instruction program-id account demoted to readonly
Expand Down Expand Up @@ -2883,7 +2890,11 @@ mod tests {
Ok(()),
];

let results = accounts.lock_accounts_with_results(txs.iter(), qos_results.iter());
let results = accounts.lock_accounts_with_results(
txs.iter(),
qos_results.iter(),
MAX_TX_ACCOUNT_LOCKS,
);

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()
Expand Down
46 changes: 37 additions & 9 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ use {
timing::years_as_slots,
transaction::{
MessageHash, Result, SanitizedTransaction, Transaction, TransactionError,
TransactionVerificationMode, VersionedTransaction,
TransactionVerificationMode, VersionedTransaction, MAX_TX_ACCOUNT_LOCKS,
},
transaction_context::{
ExecutionRecord, InstructionTrace, TransactionAccount, TransactionContext,
Expand Down Expand Up @@ -3858,13 +3858,28 @@ impl Bank {
}
}

/// Get the max number of accounts that a transaction may lock in this block
pub fn get_transaction_account_lock_limit(&self) -> usize {
if let Some(transaction_account_lock_limit) =
self.runtime_config.transaction_account_lock_limit
{
transaction_account_lock_limit
} else {
MAX_TX_ACCOUNT_LOCKS
}
}

/// 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 transaction_account_lock_limit = self.get_transaction_account_lock_limit();
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());
let lock_results = self
.rc
.accounts
.lock_accounts(sanitized_txs.iter(), transaction_account_lock_limit);
TransactionBatch::new(lock_results, self, Cow::Owned(sanitized_txs))
}

Expand All @@ -3884,7 +3899,11 @@ impl Bank {
)
})
.collect::<Result<Vec<_>>>()?;
let lock_results = self.rc.accounts.lock_accounts(sanitized_txs.iter());
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);
Ok(TransactionBatch::new(
lock_results,
self,
Expand All @@ -3897,7 +3916,11 @@ impl Bank {
&'a self,
txs: &'b [SanitizedTransaction],
) -> TransactionBatch<'a, 'b> {
let lock_results = self.rc.accounts.lock_accounts(txs.iter());
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))
}

Expand All @@ -3909,10 +3932,12 @@ 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);
let tx_account_lock_limit = self.get_transaction_account_lock_limit();
let lock_results = self.rc.accounts.lock_accounts_with_results(
transactions.iter(),
transaction_results,
tx_account_lock_limit,
);
TransactionBatch::new(lock_results, self, Cow::Borrowed(transactions))
}

Expand All @@ -3921,7 +3946,10 @@ impl Bank {
&'a self,
transaction: SanitizedTransaction,
) -> TransactionBatch<'a, '_> {
let lock_result = transaction.get_account_locks().map(|_| ());
let tx_account_lock_limit = self.get_transaction_account_lock_limit();
let lock_result = transaction
.get_account_locks(tx_account_lock_limit)
.map(|_| ());
let mut batch =
TransactionBatch::new(vec![lock_result], self, Cow::Owned(vec![transaction]));
batch.set_needs_unlock(false);
Expand Down
1 change: 1 addition & 0 deletions runtime/src/runtime_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ pub struct RuntimeConfig {
pub bpf_jit: bool,
pub compute_budget: Option<ComputeBudget>,
pub log_messages_bytes_limit: Option<usize>,
pub transaction_account_lock_limit: Option<usize>,
}
7 changes: 5 additions & 2 deletions sdk/src/transaction/sanitized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,13 @@ impl SanitizedTransaction {
}

/// Validate and return the account keys locked by this transaction
pub fn get_account_locks(&self) -> Result<TransactionAccountLocks> {
pub fn get_account_locks(
&self,
tx_account_lock_limit: usize,
) -> Result<TransactionAccountLocks> {
if self.message.has_duplicates() {
Err(TransactionError::AccountLoadedTwice)
} else if 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
3 changes: 3 additions & 0 deletions test-validator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ pub struct TestValidatorGenesis {
deactivate_feature_set: HashSet<Pubkey>,
compute_unit_limit: Option<u64>,
pub log_messages_bytes_limit: Option<usize>,
pub transaction_account_lock_limit: Option<usize>,
}

impl Default for TestValidatorGenesis {
Expand Down Expand Up @@ -154,6 +155,7 @@ impl Default for TestValidatorGenesis {
deactivate_feature_set: HashSet::<Pubkey>::default(),
compute_unit_limit: Option::<u64>::default(),
log_messages_bytes_limit: Option::<usize>::default(),
transaction_account_lock_limit: Option::<usize>::default(),
}
}
}
Expand Down Expand Up @@ -770,6 +772,7 @@ impl TestValidator {
..ComputeBudget::default()
}),
log_messages_bytes_limit: config.log_messages_bytes_limit,
transaction_account_lock_limit: config.transaction_account_lock_limit,
};

let mut validator_config = ValidatorConfig {
Expand Down
10 changes: 10 additions & 0 deletions validator/src/bin/solana-test-validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,14 @@ fn main() {
.takes_value(true)
.help("Maximum number of bytes written to the program log before truncation")
)
.arg(
Arg::with_name("transaction_account_lock_limit")
.long("transaction-account-lock-limit")
.value_name("NUM_ACCOUNTS")
.validator(is_parsable::<u64>)
.takes_value(true)
.help("Override the runtime's account lock limit per transaction")
)
.get_matches();

let output = if matches.is_present("quiet") {
Expand Down Expand Up @@ -687,6 +695,8 @@ fn main() {
genesis.max_genesis_archive_unpacked_size = Some(u64::MAX);
genesis.accounts_db_caching_enabled = !matches.is_present("no_accounts_db_caching");
genesis.log_messages_bytes_limit = value_t!(matches, "log_messages_bytes_limit", usize).ok();
genesis.transaction_account_lock_limit =
value_t!(matches, "transaction_account_lock_limit", usize).ok();

let tower_storage = Arc::new(FileTowerStorage::new(ledger_path.clone()));

Expand Down

0 comments on commit 5618e9f

Please sign in to comment.