move filter logic inside load_account and remove filter

HaoranYi · HaoranYi · commit 985c59088512 · 2024-05-03T01:01:07.000Z
diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs
@@ -971,10 +971,6 @@ impl<FG: ForkGraph> ProgramCache<FG> {
         }
     }
 
-    pub fn contains_key(&self, key: &Pubkey) -> bool {
-        self.entries.contains_key(key)
-    }
-
     /// Extracts a subset of the programs relevant to a transaction batch
     /// and returns which program accounts the accounts DB needs to load.
     pub fn extract(
diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs
@@ -4,15 +4,13 @@ use {
         transaction_error_metrics::TransactionErrorMetrics,
         transaction_processing_callback::TransactionProcessingCallback,
     },
-    itertools::Itertools,
     log::warn,
     solana_program_runtime::{
         compute_budget_processor::process_compute_budget_instructions,
-        loaded_programs::{ProgramCacheEntry, ProgramCacheForTxBatch},
+        loaded_programs::ProgramCacheEntry,
     },
     solana_sdk::{
         account::{Account, AccountSharedData, ReadableAccount, WritableAccount},
-        clock::Slot,
         feature_set::{
             self, include_loaded_accounts_data_size_in_fee_calculation,
             remove_rounding_in_fee_calculation,
@@ -32,8 +30,10 @@ use {
         transaction_context::{IndexOfAccount, TransactionAccount},
     },
     solana_system_program::{get_system_account_kind, SystemAccountKind},
-    std::collections::hash_map::Entry,
-    std::{collections::HashMap, num::NonZeroUsize},
+    std::{
+        collections::{hash_map::Entry, HashMap},
+        num::NonZeroUsize,
+    },
 };
 
 // for the load instructions
@@ -113,7 +113,7 @@ pub fn validate_fee_payer(
 pub(crate) fn load_accounts<CB: TransactionProcessingCallback>(
     callbacks: &CB,
     txs: &[SanitizedTransaction],
-    check_results: &[TransactionCheckResult],
+    check_results: &mut [TransactionCheckResult],
     error_counters: &mut TransactionErrorMetrics,
     fee_structure: &FeeStructure,
     account_overrides: Option<&AccountOverrides>,
@@ -138,6 +138,7 @@ pub(crate) fn load_accounts<CB: TransactionProcessingCallback>(
                         feature_set.is_active(&remove_rounding_in_fee_calculation::id()),
                     )
                 } else {
+                    *etx.1 = (Err(TransactionError::BlockhashNotFound), None, None);
                     return (Err(TransactionError::BlockhashNotFound), None);
                 };
 
@@ -408,6 +409,7 @@ fn get_requested_loaded_accounts_data_size_limit(
     )
 }
 
+#[allow(dead_code)]
 fn account_shared_data_from_program(loaded_program: &ProgramCacheEntry) -> AccountSharedData {
     // It's an executable program account. The program is already loaded in the cache.
     // So the account data is not needed. Return a dummy AccountSharedData with meta
@@ -461,7 +463,6 @@ mod tests {
         solana_program_runtime::{
             compute_budget::ComputeBudget,
             compute_budget_processor,
-            loaded_programs::{ProgramCacheEntry, ProgramCacheForTxBatch},
             prioritization_fee::{PrioritizationFeeDetails, PrioritizationFeeType},
         },
         solana_sdk::{
@@ -557,7 +558,7 @@ mod tests {
         load_accounts(
             &callbacks,
             &[sanitized_tx],
-            &[(Ok(()), None, Some(lamports_per_signature))],
+            &mut [(Ok(()), None, Some(lamports_per_signature))],
             error_counters,
             fee_structure,
             None,
@@ -1048,7 +1049,7 @@ mod tests {
         load_accounts(
             &callbacks,
             &[tx],
-            &[(Ok(()), None, Some(10))],
+            &mut [(Ok(()), None, Some(10))],
             &mut error_counters,
             &FeeStructure::default(),
             account_overrides,
@@ -1488,7 +1489,6 @@ mod tests {
         mock_bank.accounts_map.insert(key1.pubkey(), account_data);
 
         let mut error_counter = TransactionErrorMetrics::default();
-        let loaded_programs = ProgramCacheForTxBatch::default();
 
         let sanitized_transaction = SanitizedTransaction::new_for_tests(
             sanitized_message,
@@ -1817,7 +1817,6 @@ mod tests {
             .accounts_map
             .insert(key3.pubkey(), AccountSharedData::default());
         let mut error_counter = TransactionErrorMetrics::default();
-        let loaded_programs = ProgramCacheForTxBatch::default();
 
         let sanitized_transaction = SanitizedTransaction::new_for_tests(
             sanitized_message,
@@ -1879,7 +1878,6 @@ mod tests {
         mock_bank.accounts_map.insert(key3.pubkey(), account_data);
 
         let mut error_counter = TransactionErrorMetrics::default();
-        let loaded_programs = ProgramCacheForTxBatch::default();
 
         let sanitized_transaction = SanitizedTransaction::new_for_tests(
             sanitized_message,
@@ -1967,7 +1965,6 @@ mod tests {
         mock_bank.accounts_map.insert(key3.pubkey(), account_data);
 
         let mut error_counter = TransactionErrorMetrics::default();
-        let loaded_programs = ProgramCacheForTxBatch::default();
 
         let sanitized_transaction = SanitizedTransaction::new_for_tests(
             sanitized_message,
@@ -2048,7 +2045,7 @@ mod tests {
         let loaded_txs = load_accounts(
             &bank,
             &[sanitized_tx.clone()],
-            &[(Ok(()), None, Some(0))],
+            &mut [(Ok(()), None, Some(0))],
             &mut error_counters,
             &FeeStructure::default(),
             None,
@@ -2119,7 +2116,6 @@ mod tests {
         mock_bank.accounts_map.insert(key3.pubkey(), account_data);
 
         let mut error_counter = TransactionErrorMetrics::default();
-        let loaded_programs = ProgramCacheForTxBatch::default();
 
         let sanitized_transaction = SanitizedTransaction::new_for_tests(
             sanitized_message,
@@ -2132,7 +2128,7 @@ mod tests {
         let results = load_accounts(
             &mock_bank,
             &[sanitized_transaction],
-            &[check_result],
+            &mut [check_result],
             &mut error_counter,
             &FeeStructure::default(),
             None,
@@ -2206,7 +2202,7 @@ mod tests {
         let result = load_accounts(
             &mock_bank,
             &[sanitized_transaction.clone()],
-            &[check_result],
+            &mut [check_result],
             &mut TransactionErrorMetrics::default(),
             &fee_structure,
             None,
@@ -2225,7 +2221,7 @@ mod tests {
         let result = load_accounts(
             &mock_bank,
             &[sanitized_transaction.clone()],
-            &[check_result.clone()],
+            &mut [check_result.clone()],
             &mut TransactionErrorMetrics::default(),
             &fee_structure,
             None,
@@ -2244,7 +2240,7 @@ mod tests {
         let result = load_accounts(
             &mock_bank,
             &[sanitized_transaction.clone()],
-            &[check_result],
+            &mut [check_result],
             &mut TransactionErrorMetrics::default(),
             &fee_structure,
             None,
diff --git a/svm/src/program_loader.rs b/svm/src/program_loader.rs
@@ -18,7 +18,7 @@ use {
         loader_v4::{self, LoaderV4State, LoaderV4Status},
         pubkey::Pubkey,
     },
-    std::{collections::HashMap, sync::Arc},
+    std::sync::Arc,
 };
 
 #[derive(Debug)]
diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs
@@ -43,7 +43,7 @@ use {
     },
     std::{
         cell::RefCell,
-        collections::{hash_map::Entry, HashMap, HashSet},
+        collections::{HashMap, HashSet},
         fmt::{Debug, Formatter},
         rc::Rc,
         sync::{atomic::Ordering, Arc, RwLock},
@@ -188,7 +188,6 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
         limit_to_load_programs: bool,
     ) -> LoadAndExecuteSanitizedTransactionsOutput {
         let mut load_time = Measure::start("accounts_load");
-        Self::filter_executable_program_accounts(sanitized_txs, check_results);
         let mut program_accounts_map: HashMap<Pubkey, u64> = HashMap::new();
         let mut loaded_transactions = load_accounts(
             callbacks,
@@ -326,23 +325,6 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
         }
     }
 
-    /// Filter transaction with invalid nonce and blockhash.
-    fn filter_executable_program_accounts(
-        txs: &[SanitizedTransaction],
-        check_results: &mut [TransactionCheckResult],
-    ) {
-        check_results.iter_mut().zip(txs).for_each(|etx| {
-            if let ((Ok(()), _nonce, lamports_per_signature), tx) = etx {
-                if lamports_per_signature.is_none() {
-                    // 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, None);
-                }
-            }
-        });
-    }
-
     fn replenish_program_cache<CB: TransactionProcessingCallback>(
         &self,
         callback: &CB,
@@ -732,10 +714,9 @@ mod tests {
             rent_collector::RentCollector,
             rent_debits::RentDebits,
             reserved_account_keys::ReservedAccountKeys,
-            signature::{Keypair, Signature},
-            signer::Signer,
+            signature::Signature,
             sysvar::{self, rent::Rent},
-            transaction::{SanitizedTransaction, Transaction, TransactionError},
+            transaction::SanitizedTransaction,
             transaction_context::TransactionContext,
         },
     };
@@ -1051,7 +1032,7 @@ mod tests {
         let mut account_maps: HashMap<Pubkey, u64> = HashMap::new();
         account_maps.insert(key, 4);
 
-        let cache = batch_processor.replenish_program_cache(&mock_bank, &account_maps, true);
+        let _cache = batch_processor.replenish_program_cache(&mock_bank, &account_maps, true);
     }
 
     #[test]
diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs
@@ -194,7 +194,7 @@ fn deploy_program(name: String, mock_bank: &mut MockBankCallback) -> Pubkey {
     account_data.set_data(bincode::serialize(&state).unwrap());
     account_data.set_lamports(25);
     account_data.set_owner(bpf_loader_upgradeable::id());
-    // We are no longer fake executable accounts. Executable account will be
+    // We are no longer faking executable accounts. Executable accounts will be
     // loaded from accounts-db, which would require executable flag to be
     // marked.
     account_data.set_executable(true);

Original file line number	Diff line number	Diff line change
`@@ -971,10 +971,6 @@ impl<FG: ForkGraph> ProgramCache<FG> {`
`971`	`971`	`}`
`972`	`972`	`}`
`973`	`973`
`974`		`- pub fn contains_key(&self, key: &Pubkey) -> bool {`
`975`		`- self.entries.contains_key(key)`
`976`		`- }`
`977`		`-`
`978`	`974`	`/// Extracts a subset of the programs relevant to a transaction batch`
`979`	`975`	`/// and returns which program accounts the accounts DB needs to load.`
`980`	`976`	`pub fn extract(`