Skip to content

Commit

Permalink
Find and load missing programs in LoadedPrograms cache
Browse files Browse the repository at this point in the history 
- filter program accounts in a transaction batch
- filter the accounts that are missing in LoadedPrograms cache
- load the programs before processing the transactions
- unit tests
  • Loading branch information
@pgarg66
pgarg66 committed Feb 15, 2023
1 parent a349b74 commit 37cc11f
Show file tree
Hide file tree
Showing 6 changed files with 452 additions and 36 deletions.
10 changes: 10 additions & 0 deletions programs/bpf_loader/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1470,6 +1470,16 @@ pub struct BpfExecutor {
verified_executable: VerifiedExecutable<RequisiteVerifier, InvokeContext<'static>>,
}

impl BpfExecutor {
pub fn new(
verified_executable: VerifiedExecutable<RequisiteVerifier, InvokeContext<'static>>,
) -> Self {
Self {
verified_executable,
}
}
}

// Well, implement Debug for solana_rbpf::vm::Executable in solana-rbpf...
impl Debug for BpfExecutor {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
Expand Down
255 changes: 255 additions & 0 deletions runtime/src/accounts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,49 @@ impl Accounts {
)
}

pub fn filter_program_accounts<'a>(
&self,
ancestors: &Ancestors,
txs: &[SanitizedTransaction],
lock_results: &mut [TransactionCheckResult],
program_owners: &[&'a Pubkey],
hash_queue: &BlockhashQueue,
) -> HashMap<Pubkey, &'a Pubkey> {
let mut result = HashMap::new();
lock_results.iter_mut().zip(txs).for_each(|etx| {
if let ((Ok(()), nonce), tx) = etx {
if nonce
.as_ref()
.map(|nonce| nonce.lamports_per_signature())
.unwrap_or_else(|| {
hash_queue.get_lamports_per_signature(tx.message().recent_blockhash())
})
.is_some()
{
tx.message().account_keys().iter().for_each(|key| {
if !result.contains_key(key) {
if let Ok(index) = self.accounts_db.account_matches_owners(
ancestors,
key,
program_owners,
) {
program_owners
.get(index)
.and_then(|owner| result.insert(*key, *owner));
}
}
});
} else {
// If the transaction's nonce account was not valid, and blockhash is not found,
// the transaction will fail to process. Let's not load any programs from the
// transaction, and update the status of the transaction.
*etx.0 = (Err(TransactionError::BlockhashNotFound), None);
}
}
});
result
}

#[allow(clippy::too_many_arguments)]
pub fn load_accounts(
&self,
Expand Down Expand Up @@ -1883,6 +1926,218 @@ mod tests {
);
}

#[test]
fn test_filter_program_accounts() {
let mut tx_accounts: Vec<TransactionAccount> = Vec::new();

let keypair1 = Keypair::new();
let keypair2 = Keypair::new();

let non_program_pubkey1 = Pubkey::new_unique();
let non_program_pubkey2 = Pubkey::new_unique();
let program1_pubkey = Pubkey::new_unique();
let program2_pubkey = Pubkey::new_unique();
let account1_pubkey = Pubkey::new_unique();
let account2_pubkey = Pubkey::new_unique();
let account3_pubkey = Pubkey::new_unique();
let account4_pubkey = Pubkey::new_unique();

let account5_pubkey = Pubkey::new_unique();

tx_accounts.push((
non_program_pubkey1,
AccountSharedData::new(1, 10, &account5_pubkey),
));
tx_accounts.push((
non_program_pubkey2,
AccountSharedData::new(1, 10, &account5_pubkey),
));
tx_accounts.push((
program1_pubkey,
AccountSharedData::new(40, 1, &account5_pubkey),
));
tx_accounts.push((
program2_pubkey,
AccountSharedData::new(40, 1, &account5_pubkey),
));
tx_accounts.push((
account1_pubkey,
AccountSharedData::new(1, 10, &non_program_pubkey1),
));
tx_accounts.push((
account2_pubkey,
AccountSharedData::new(1, 10, &non_program_pubkey2),
));
tx_accounts.push((
account3_pubkey,
AccountSharedData::new(40, 1, &program1_pubkey),
));
tx_accounts.push((
account4_pubkey,
AccountSharedData::new(40, 1, &program2_pubkey),
));

let accounts = Accounts::new_with_config_for_tests(
Vec::new(),
&ClusterType::Development,
AccountSecondaryIndexes::default(),
AccountShrinkThreshold::default(),
);
for tx_account in tx_accounts.iter() {
accounts.store_for_tests(0, &tx_account.0, &tx_account.1);
}

let mut hash_queue = BlockhashQueue::new(100);

let tx1 = Transaction::new_with_compiled_instructions(
&[&keypair1],
&[non_program_pubkey1],
Hash::new_unique(),
vec![account1_pubkey, account2_pubkey, account3_pubkey],
vec![CompiledInstruction::new(1, &(), vec![0])],
);
hash_queue.register_hash(&tx1.message().recent_blockhash, 0);
let sanitized_tx1 = SanitizedTransaction::from_transaction_for_tests(tx1);

let tx2 = Transaction::new_with_compiled_instructions(
&[&keypair2],
&[non_program_pubkey2],
Hash::new_unique(),
vec![account4_pubkey, account3_pubkey, account2_pubkey],
vec![CompiledInstruction::new(1, &(), vec![0])],
);
hash_queue.register_hash(&tx2.message().recent_blockhash, 0);
let sanitized_tx2 = SanitizedTransaction::from_transaction_for_tests(tx2);

let ancestors = vec![(0, 0)].into_iter().collect();
let programs = accounts.filter_program_accounts(
&ancestors,
&[sanitized_tx1, sanitized_tx2],
&mut [(Ok(()), None), (Ok(()), None)],
&[&program1_pubkey, &program2_pubkey],
&hash_queue,
);

// The result should contain only account3_pubkey, and account4_pubkey as the program accounts
assert_eq!(programs.len(), 2);
assert_eq!(
programs
.get(&account3_pubkey)
.expect("failed to find the program account"),
&&program1_pubkey
);
assert_eq!(
programs
.get(&account4_pubkey)
.expect("failed to find the program account"),
&&program2_pubkey
);
}

#[test]
fn test_filter_program_accounts_invalid_blockhash() {
let mut tx_accounts: Vec<TransactionAccount> = Vec::new();

let keypair1 = Keypair::new();
let keypair2 = Keypair::new();

let non_program_pubkey1 = Pubkey::new_unique();
let non_program_pubkey2 = Pubkey::new_unique();
let program1_pubkey = Pubkey::new_unique();
let program2_pubkey = Pubkey::new_unique();
let account1_pubkey = Pubkey::new_unique();
let account2_pubkey = Pubkey::new_unique();
let account3_pubkey = Pubkey::new_unique();
let account4_pubkey = Pubkey::new_unique();

let account5_pubkey = Pubkey::new_unique();

tx_accounts.push((
non_program_pubkey1,
AccountSharedData::new(1, 10, &account5_pubkey),
));
tx_accounts.push((
non_program_pubkey2,
AccountSharedData::new(1, 10, &account5_pubkey),
));
tx_accounts.push((
program1_pubkey,
AccountSharedData::new(40, 1, &account5_pubkey),
));
tx_accounts.push((
program2_pubkey,
AccountSharedData::new(40, 1, &account5_pubkey),
));
tx_accounts.push((
account1_pubkey,
AccountSharedData::new(1, 10, &non_program_pubkey1),
));
tx_accounts.push((
account2_pubkey,
AccountSharedData::new(1, 10, &non_program_pubkey2),
));
tx_accounts.push((
account3_pubkey,
AccountSharedData::new(40, 1, &program1_pubkey),
));
tx_accounts.push((
account4_pubkey,
AccountSharedData::new(40, 1, &program2_pubkey),
));

let accounts = Accounts::new_with_config_for_tests(
Vec::new(),
&ClusterType::Development,
AccountSecondaryIndexes::default(),
AccountShrinkThreshold::default(),
);
for tx_account in tx_accounts.iter() {
accounts.store_for_tests(0, &tx_account.0, &tx_account.1);
}

let mut hash_queue = BlockhashQueue::new(100);

let tx1 = Transaction::new_with_compiled_instructions(
&[&keypair1],
&[non_program_pubkey1],
Hash::new_unique(),
vec![account1_pubkey, account2_pubkey, account3_pubkey],
vec![CompiledInstruction::new(1, &(), vec![0])],
);
hash_queue.register_hash(&tx1.message().recent_blockhash, 0);
let sanitized_tx1 = SanitizedTransaction::from_transaction_for_tests(tx1);

let tx2 = Transaction::new_with_compiled_instructions(
&[&keypair2],
&[non_program_pubkey2],
Hash::new_unique(),
vec![account4_pubkey, account3_pubkey, account2_pubkey],
vec![CompiledInstruction::new(1, &(), vec![0])],
);
// Let's not register blockhash from tx2. This should cause the tx2 to fail
let sanitized_tx2 = SanitizedTransaction::from_transaction_for_tests(tx2);

let ancestors = vec![(0, 0)].into_iter().collect();
let mut lock_results = vec![(Ok(()), None), (Ok(()), None)];
let programs = accounts.filter_program_accounts(
&ancestors,
&[sanitized_tx1, sanitized_tx2],
&mut lock_results,
&[&program1_pubkey, &program2_pubkey],
&hash_queue,
);

// The result should contain only account3_pubkey, and account4_pubkey as the program accounts
assert_eq!(programs.len(), 1);
assert_eq!(
programs
.get(&account3_pubkey)
.expect("failed to find the program account"),
&&program1_pubkey
);
assert_eq!(lock_results[1].0, Err(TransactionError::BlockhashNotFound));
}

#[test]
fn test_load_accounts_multiple_loaders() {
let mut accounts: Vec<TransactionAccount> = Vec::new();
Expand Down
37 changes: 25 additions & 12 deletions runtime/src/accounts_db.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -821,14 +821,18 @@ impl<'a> LoadedAccountAccessor<'a> {
}
}

fn account_matches_owners(&self, owners: &[&Pubkey]) -> Result<(), MatchAccountOwnerError> {
fn account_matches_owners(&self, owners: &[&Pubkey]) -> Result<usize, MatchAccountOwnerError> {
match self {
LoadedAccountAccessor::Cached(cached_account) => cached_account
.as_ref()
.and_then(|cached_account| {
(!cached_account.account.is_zero_lamport()
&& owners.contains(&cached_account.account.owner()))
.then_some(())
if cached_account.account.is_zero_lamport() {
None
} else {
owners
.iter()
.position(|entry| &cached_account.account.owner() == entry)
}
})
.ok_or(MatchAccountOwnerError::NoMatch),
LoadedAccountAccessor::Stored(maybe_storage_entry) => {
Expand Down Expand Up @@ -4938,22 +4942,31 @@ impl AccountsDb {
self.do_load(ancestors, pubkey, None, load_hint, LoadZeroLamports::None)
}

/// Return Ok(index_of_matching_owner) if the account owner at `offset` is one of the pubkeys in `owners`.
/// Return Err(MatchAccountOwnerError::NoMatch) if the account has 0 lamports or the owner is not one of
/// the pubkeys in `owners`.
/// Return Err(MatchAccountOwnerError::UnableToLoad) if the account could not be accessed.
pub fn account_matches_owners(
&self,
ancestors: &Ancestors,
account: &Pubkey,
owners: &[&Pubkey],
) -> Result<(), MatchAccountOwnerError> {
) -> Result<usize, MatchAccountOwnerError> {
let (slot, storage_location, _maybe_account_accesor) = self
.read_index_for_accessor_or_load_slow(ancestors, account, None, false)
.ok_or(MatchAccountOwnerError::UnableToLoad)?;

if !storage_location.is_cached() {
let result = self.read_only_accounts_cache.load(*account, slot);
if let Some(account) = result {
return (!account.is_zero_lamport() && owners.contains(&account.owner()))
.then_some(())
.ok_or(MatchAccountOwnerError::NoMatch);
return if account.is_zero_lamport() {
Err(MatchAccountOwnerError::NoMatch)
} else {
owners
.iter()
.position(|entry| &account.owner() == entry)
.ok_or(MatchAccountOwnerError::NoMatch)
};
}
}

Expand Down Expand Up @@ -14180,11 +14193,11 @@ pub mod tests {

assert_eq!(
db.account_matches_owners(&Ancestors::default(), &account1_key, &owners_refs),
Ok(())
Ok(0)
);
assert_eq!(
db.account_matches_owners(&Ancestors::default(), &account2_key, &owners_refs),
Ok(())
Ok(1)
);
assert_eq!(
db.account_matches_owners(&Ancestors::default(), &account3_key, &owners_refs),
Expand Down Expand Up @@ -14214,11 +14227,11 @@ pub mod tests {

assert_eq!(
db.account_matches_owners(&Ancestors::default(), &account1_key, &owners_refs),
Ok(())
Ok(0)
);
assert_eq!(
db.account_matches_owners(&Ancestors::default(), &account2_key, &owners_refs),
Ok(())
Ok(1)
);
assert_eq!(
db.account_matches_owners(&Ancestors::default(), &account3_key, &owners_refs),
Expand Down
Loading

0 comments on commit 37cc11f

Please sign in to comment.