From 59eb55990c2ea95c1455203e768ddf1ade3c098c Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Thu, 9 Nov 2023 14:40:04 -0800 Subject: [PATCH] Move filter_executable_program_accounts to bank.rs (#34004) --- accounts-db/src/accounts.rs | 272 +----------------------------------- runtime/src/bank.rs | 58 +++++++- runtime/src/bank/tests.rs | 194 +++++++++++++++++++++++++ 3 files changed, 251 insertions(+), 273 deletions(-) diff --git a/accounts-db/src/accounts.rs b/accounts-db/src/accounts.rs index 0ac199e6633522..b11763b1dd5048 100644 --- a/accounts-db/src/accounts.rs +++ b/accounts-db/src/accounts.rs @@ -59,10 +59,7 @@ use { solana_system_program::{get_system_account_kind, SystemAccountKind}, std::{ cmp::Reverse, - collections::{ - hash_map::{self, Entry}, - BinaryHeap, HashMap, HashSet, - }, + collections::{hash_map, BinaryHeap, HashMap, HashSet}, num::NonZeroUsize, ops::RangeBounds, path::PathBuf, @@ -638,59 +635,6 @@ impl Accounts { ) } - /// Returns a hash map of executable program accounts (program accounts that are not writable - /// in the given transactions), and their owners, for the transactions with a valid - /// blockhash or nonce. - pub fn filter_executable_program_accounts<'a>( - &self, - ancestors: &Ancestors, - txs: &[SanitizedTransaction], - lock_results: &mut [TransactionCheckResult], - program_owners: &'a [Pubkey], - hash_queue: &BlockhashQueue, - ) -> HashMap { - let mut result: HashMap = 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| match result.entry(*key) { - Entry::Occupied(mut entry) => { - let (_, count) = entry.get_mut(); - saturating_add_assign!(*count, 1); - } - Entry::Vacant(entry) => { - if let Ok(index) = self.accounts_db.account_matches_owners( - ancestors, - key, - program_owners, - ) { - program_owners - .get(index) - .map(|owner| entry.insert((owner, 1))); - } - } - }); - } 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, @@ -2000,220 +1944,6 @@ mod tests { ); } - #[test] - fn test_filter_executable_program_accounts() { - let mut tx_accounts: Vec = 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 owners = &[program1_pubkey, program2_pubkey]; - let programs = accounts.filter_executable_program_accounts( - &ancestors, - &[sanitized_tx1, sanitized_tx2], - &mut [(Ok(()), None), (Ok(()), None)], - owners, - &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, 2) - ); - assert_eq!( - programs - .get(&account4_pubkey) - .expect("failed to find the program account"), - &(&program2_pubkey, 1) - ); - } - - #[test] - fn test_filter_executable_program_accounts_invalid_blockhash() { - let mut tx_accounts: Vec = 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 owners = &[program1_pubkey, program2_pubkey]; - let mut lock_results = vec![(Ok(()), None), (Ok(()), None)]; - let programs = accounts.filter_executable_program_accounts( - &ancestors, - &[sanitized_tx1, sanitized_tx2], - &mut lock_results, - owners, - &hash_queue, - ); - - // The result should contain only account3_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, 1) - ); - assert_eq!(lock_results[1].0, Err(TransactionError::BlockhashNotFound)); - } - #[test] fn test_load_accounts_multiple_loaders() { let mut accounts: Vec = Vec::new(); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 2b8cbe34926100..3bf2d720933443 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -184,7 +184,7 @@ use { std::{ borrow::Cow, cell::RefCell, - collections::{HashMap, HashSet}, + collections::{hash_map::Entry, HashMap, HashSet}, convert::TryFrom, fmt, mem, ops::{AddAssign, RangeInclusive}, @@ -5116,6 +5116,60 @@ impl Bank { loaded_programs_for_txs } + /// Returns a hash map of executable program accounts (program accounts that are not writable + /// in the given transactions), and their owners, for the transactions with a valid + /// blockhash or nonce. + fn filter_executable_program_accounts<'a>( + &self, + ancestors: &Ancestors, + txs: &[SanitizedTransaction], + lock_results: &mut [TransactionCheckResult], + program_owners: &'a [Pubkey], + hash_queue: &BlockhashQueue, + ) -> HashMap { + let mut result: HashMap = 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| match result.entry(*key) { + Entry::Occupied(mut entry) => { + let (_, count) = entry.get_mut(); + saturating_add_assign!(*count, 1); + } + Entry::Vacant(entry) => { + if let Ok(index) = self + .rc + .accounts + .accounts_db + .account_matches_owners(ancestors, key, program_owners) + { + program_owners + .get(index) + .map(|owner| entry.insert((owner, 1))); + } + } + }); + } 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::type_complexity)] pub fn load_and_execute_transactions( &self, @@ -5183,7 +5237,7 @@ impl Bank { bpf_loader_deprecated::id(), loader_v4::id(), ]; - let mut program_accounts_map = self.rc.accounts.filter_executable_program_accounts( + let mut program_accounts_map = self.filter_executable_program_accounts( &self.ancestors, sanitized_txs, &mut check_results, diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index cddac40fe3761f..50d68c6cd82288 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -13536,3 +13536,197 @@ fn test_last_restart_slot() { assert!(!last_restart_slot_dirty(&bank7)); assert_eq!(get_last_restart_slot(&bank7), Some(6)); } + +#[test] +fn test_filter_executable_program_accounts() { + 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(); + + let (genesis_config, _mint_keypair) = create_genesis_config(10); + let bank = Bank::new_for_tests(&genesis_config); + bank.store_account( + &non_program_pubkey1, + &AccountSharedData::new(1, 10, &account5_pubkey), + ); + bank.store_account( + &non_program_pubkey2, + &AccountSharedData::new(1, 10, &account5_pubkey), + ); + bank.store_account( + &program1_pubkey, + &AccountSharedData::new(40, 1, &account5_pubkey), + ); + bank.store_account( + &program2_pubkey, + &AccountSharedData::new(40, 1, &account5_pubkey), + ); + bank.store_account( + &account1_pubkey, + &AccountSharedData::new(1, 10, &non_program_pubkey1), + ); + bank.store_account( + &account2_pubkey, + &AccountSharedData::new(1, 10, &non_program_pubkey2), + ); + bank.store_account( + &account3_pubkey, + &AccountSharedData::new(40, 1, &program1_pubkey), + ); + bank.store_account( + &account4_pubkey, + &AccountSharedData::new(40, 1, &program2_pubkey), + ); + + 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 owners = &[program1_pubkey, program2_pubkey]; + let programs = bank.filter_executable_program_accounts( + &ancestors, + &[sanitized_tx1, sanitized_tx2], + &mut [(Ok(()), None), (Ok(()), None)], + owners, + &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, 2) + ); + assert_eq!( + programs + .get(&account4_pubkey) + .expect("failed to find the program account"), + &(&program2_pubkey, 1) + ); +} + +#[test] +fn test_filter_executable_program_accounts_invalid_blockhash() { + 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(); + + let (genesis_config, _mint_keypair) = create_genesis_config(10); + let bank = Bank::new_for_tests(&genesis_config); + bank.store_account( + &non_program_pubkey1, + &AccountSharedData::new(1, 10, &account5_pubkey), + ); + bank.store_account( + &non_program_pubkey2, + &AccountSharedData::new(1, 10, &account5_pubkey), + ); + bank.store_account( + &program1_pubkey, + &AccountSharedData::new(40, 1, &account5_pubkey), + ); + bank.store_account( + &program2_pubkey, + &AccountSharedData::new(40, 1, &account5_pubkey), + ); + bank.store_account( + &account1_pubkey, + &AccountSharedData::new(1, 10, &non_program_pubkey1), + ); + bank.store_account( + &account2_pubkey, + &AccountSharedData::new(1, 10, &non_program_pubkey2), + ); + bank.store_account( + &account3_pubkey, + &AccountSharedData::new(40, 1, &program1_pubkey), + ); + bank.store_account( + &account4_pubkey, + &AccountSharedData::new(40, 1, &program2_pubkey), + ); + + 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 owners = &[program1_pubkey, program2_pubkey]; + let mut lock_results = vec![(Ok(()), None), (Ok(()), None)]; + let programs = bank.filter_executable_program_accounts( + &ancestors, + &[sanitized_tx1, sanitized_tx2], + &mut lock_results, + owners, + &hash_queue, + ); + + // The result should contain only account3_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, 1) + ); + assert_eq!(lock_results[1].0, Err(TransactionError::BlockhashNotFound)); +}