Reorganize SVM concurrent tests (#2209)

Cargo.lock

@@ -7557,6 +7557,7 @@ dependencies = [
  "log",
  "percentage",
  "prost",
+ "qualifier_attr",
  "rand 0.8.5",
  "rustc_version 0.4.0",
  "serde",

programs/sbf/Cargo.lock

@@ -6337,6 +6337,7 @@ dependencies = [
  "itertools 0.12.1",
  "log",
  "percentage",
+ "qualifier_attr",
  "rustc_version",
  "serde",
  "serde_derive",

svm/Cargo.toml

@@ -13,6 +13,7 @@ edition = { workspace = true }
 itertools = { workspace = true }
 log = { workspace = true }
 percentage = { workspace = true }
+qualifier_attr = { workspace = true, optional = true }
 serde = { workspace = true, features = ["rc"] }
 serde_derive = { workspace = true }
 solana-bpf-loader-program = { workspace = true }
@@ -55,7 +56,7 @@ targets = ["x86_64-unknown-linux-gnu"]
 rustc_version = { workspace = true }
 
 [features]
-dev-context-only-utils = []
+dev-context-only-utils = ["dep:qualifier_attr"]
 frozen-abi = [
     "dep:solana-frozen-abi",
     "dep:solana-frozen-abi-macro",

svm/src/transaction_processor.rs

@@ -1,3 +1,5 @@
+#[cfg(feature = "dev-context-only-utils")]
+use qualifier_attr::{field_qualifiers, qualifiers};
 use {
     crate::{
         account_loader::{
@@ -138,6 +140,10 @@ pub struct TransactionProcessingEnvironment<'a> {
 }
 
 #[cfg_attr(feature = "frozen-abi", derive(AbiExample))]
+#[cfg_attr(
+    feature = "dev-context-only-utils",
+    field_qualifiers(slot(pub), epoch(pub))
+)]
 pub struct TransactionBatchProcessor<FG: ForkGraph> {
     /// Bank slot (i.e. block)
     slot: Slot,
@@ -521,6 +527,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
         result
     }
 
+    #[cfg_attr(feature = "dev-context-only-utils", qualifiers(pub))]
     fn replenish_program_cache<CB: TransactionProcessingCallback>(
         &self,
         callback: &CB,
@@ -994,7 +1001,6 @@ mod tests {
         solana_sdk::{
             account::{create_account_shared_data_for_test, WritableAccount},
             bpf_loader,
-            bpf_loader_upgradeable::{self, UpgradeableLoaderState},
             compute_budget::ComputeBudgetInstruction,
             epoch_schedule::EpochSchedule,
             feature_set::FeatureSet,
@@ -1012,12 +1018,6 @@ mod tests {
             transaction::{SanitizedTransaction, Transaction, TransactionError},
             transaction_context::TransactionContext,
         },
-        std::{
-            env,
-            fs::{self, File},
-            io::Read,
-            thread,
-        },
     };
 
     fn new_unchecked_sanitized_message(message: Message) -> SanitizedMessage {
@@ -1842,112 +1842,6 @@ mod tests {
         assert_eq!(entry, Arc::new(program));
     }
 
-    #[test]
-    fn fast_concur_test() {
-        let mut mock_bank = MockBankCallback::default();
-        let batch_processor = TransactionBatchProcessor::<TestForkGraph>::new(5, 5, HashSet::new());
-        let fork_graph = Arc::new(RwLock::new(TestForkGraph {}));
-        batch_processor.program_cache.write().unwrap().fork_graph =
-            Some(Arc::downgrade(&fork_graph));
-
-        let programs = vec![
-            deploy_program("hello-solana".to_string(), &mut mock_bank),
-            deploy_program("simple-transfer".to_string(), &mut mock_bank),
-            deploy_program("clock-sysvar".to_string(), &mut mock_bank),
-        ];
-
-        let account_maps: HashMap<Pubkey, u64> = programs
-            .iter()
-            .enumerate()
-            .map(|(idx, key)| (*key, idx as u64))
-            .collect();
-
-        for _ in 0..10 {
-            let ths: Vec<_> = (0..4)
-                .map(|_| {
-                    let local_bank = mock_bank.clone();
-                    let processor = TransactionBatchProcessor::new_from(
-                        &batch_processor,
-                        batch_processor.slot,
-                        batch_processor.epoch,
-                    );
-                    let maps = account_maps.clone();
-                    let programs = programs.clone();
-                    thread::spawn(move || {
-                        let result =
-                            processor.replenish_program_cache(&local_bank, &maps, false, true);
-                        for key in &programs {
-                            let cache_entry = result.find(key);
-                            assert!(matches!(
-                                cache_entry.unwrap().program,
-                                ProgramCacheEntryType::Loaded(_)
-                            ));
-                        }
-                    })
-                })
-                .collect();
-
-            for th in ths {
-                th.join().unwrap();
-            }
-        }
-    }
-
-    fn deploy_program(name: String, mock_bank: &mut MockBankCallback) -> Pubkey {
-        let program_account = Pubkey::new_unique();
-        let program_data_account = Pubkey::new_unique();
-        let state = UpgradeableLoaderState::Program {
-            programdata_address: program_data_account,
-        };
-
-        // The program account must have funds and hold the executable binary
-        let mut account_data = AccountSharedData::default();
-        account_data.set_data(bincode::serialize(&state).unwrap());
-        account_data.set_lamports(25);
-        account_data.set_owner(bpf_loader_upgradeable::id());
-        mock_bank
-            .account_shared_data
-            .write()
-            .unwrap()
-            .insert(program_account, account_data);
-
-        let mut account_data = AccountSharedData::default();
-        let state = UpgradeableLoaderState::ProgramData {
-            slot: 0,
-            upgrade_authority_address: None,
-        };
-        let mut header = bincode::serialize(&state).unwrap();
-        let mut complement = vec![
-            0;
-            std::cmp::max(
-                0,
-                UpgradeableLoaderState::size_of_programdata_metadata().saturating_sub(header.len())
-            )
-        ];
-
-        let mut dir = env::current_dir().unwrap();
-        dir.push("tests");
-        dir.push("example-programs");
-        dir.push(name.as_str());
-        let name = name.replace('-', "_");
-        dir.push(name + "_program.so");
-        let mut file = File::open(dir.clone()).expect("file not found");
-        let metadata = fs::metadata(dir).expect("Unable to read metadata");
-        let mut buffer = vec![0; metadata.len() as usize];
-        file.read_exact(&mut buffer).expect("Buffer overflow");
-
-        header.append(&mut complement);
-        header.append(&mut buffer);
-        account_data.set_data(header);
-        mock_bank
-            .account_shared_data
-            .write()
-            .unwrap()
-            .insert(program_data_account, account_data);
-
-        program_account
-    }
-
     #[test]
     fn test_validate_transaction_fee_payer_exact_balance() {
         let lamports_per_signature = 5000;

svm/tests/concurrent_tests.rs

@@ -0,0 +1,64 @@
+use {
+    crate::mock_bank::{deploy_program, MockForkGraph},
+    mock_bank::MockBankCallback,
+    solana_program_runtime::loaded_programs::ProgramCacheEntryType,
+    solana_sdk::pubkey::Pubkey,
+    solana_svm::transaction_processor::TransactionBatchProcessor,
+    solana_type_overrides::sync::RwLock,
+    std::{
+        collections::{HashMap, HashSet},
+        sync::Arc,
+        thread,
+    },
+};
+
+mod mock_bank;
+
+#[test]
+fn fast_concur_test() {
+    let mut mock_bank = MockBankCallback::default();
+    let batch_processor = TransactionBatchProcessor::<MockForkGraph>::new(5, 5, HashSet::new());
+    let fork_graph = Arc::new(RwLock::new(MockForkGraph {}));
+    batch_processor.program_cache.write().unwrap().fork_graph = Some(Arc::downgrade(&fork_graph));
+
+    let programs = vec![
+        deploy_program("hello-solana".to_string(), 0, &mut mock_bank),
+        deploy_program("simple-transfer".to_string(), 0, &mut mock_bank),
+        deploy_program("clock-sysvar".to_string(), 0, &mut mock_bank),
+    ];
+
+    let account_maps: HashMap<Pubkey, u64> = programs
+        .iter()
+        .enumerate()
+        .map(|(idx, key)| (*key, idx as u64))
+        .collect();
+
+    for _ in 0..10 {
+        let ths: Vec<_> = (0..4)
+            .map(|_| {
+                let local_bank = mock_bank.clone();
+                let processor = TransactionBatchProcessor::new_from(
+                    &batch_processor,
+                    batch_processor.slot,
+                    batch_processor.epoch,
+                );
+                let maps = account_maps.clone();
+                let programs = programs.clone();
+                thread::spawn(move || {
+                    let result = processor.replenish_program_cache(&local_bank, &maps, false, true);
+                    for key in &programs {
+                        let cache_entry = result.find(key);
+                        assert!(matches!(
+                            cache_entry.unwrap().program,
+                            ProgramCacheEntryType::Loaded(_)
+                        ));
+                    }
+                })
+            })
+            .collect();
+
+        for th in ths {
+            th.join().unwrap();
+        }
+    }
+}

svm/tests/conformance.rs

@@ -197,7 +197,7 @@ fn run_fixture(fixture: InstrFixture, filename: OsString, execute_as_instr: bool
     let mut fee_payer = Pubkey::new_unique();
     let mut mock_bank = MockBankCallback::default();
     {
-        let mut account_data_map = mock_bank.account_shared_data.borrow_mut();
+        let mut account_data_map = mock_bank.account_shared_data.write().unwrap();
         for item in input.accounts {
             let pubkey = Pubkey::new_from_array(item.address.try_into().unwrap());
             let mut account_data = AccountSharedData::default();

svm/tests/integration_test.rs

@@ -1,7 +1,10 @@
 #![cfg(test)]
 
 use {
-    crate::{mock_bank::MockBankCallback, transaction_builder::SanitizedTransactionBuilder},
+    crate::{
+        mock_bank::{deploy_program, MockBankCallback},
+        transaction_builder::SanitizedTransactionBuilder,
+    },
     solana_bpf_loader_program::syscalls::{
         SyscallAbort, SyscallGetClockSysvar, SyscallInvokeSignedRust, SyscallLog, SyscallMemcpy,
         SyscallMemset, SyscallSetReturnData,
@@ -19,7 +22,7 @@ use {
     },
     solana_sdk::{
         account::{AccountSharedData, ReadableAccount, WritableAccount},
-        bpf_loader_upgradeable::{self, UpgradeableLoaderState},
+        bpf_loader_upgradeable::{self},
         clock::{Clock, Epoch, Slot, UnixTimestamp},
         hash::Hash,
         instruction::AccountMeta,
@@ -40,9 +43,6 @@ use {
     std::{
         cmp::Ordering,
         collections::{HashMap, HashSet},
-        env,
-        fs::{self, File},
-        io::Read,
         sync::{Arc, RwLock},
         time::{SystemTime, UNIX_EPOCH},
     },
@@ -161,67 +161,11 @@ fn create_executable_environment(
     account_data.set_data(bincode::serialize(&clock).unwrap());
     mock_bank
         .account_shared_data
-        .borrow_mut()
+        .write()
+        .unwrap()
         .insert(Clock::id(), account_data);
 }
 
-fn load_program(name: String) -> Vec<u8> {
-    // Loading the program file
-    let mut dir = env::current_dir().unwrap();
-    dir.push("tests");
-    dir.push("example-programs");
-    dir.push(name.as_str());
-    let name = name.replace('-', "_");
-    dir.push(name + "_program.so");
-    let mut file = File::open(dir.clone()).expect("file not found");
-    let metadata = fs::metadata(dir).expect("Unable to read metadata");
-    let mut buffer = vec![0; metadata.len() as usize];
-    file.read_exact(&mut buffer).expect("Buffer overflow");
-    buffer
-}
-
-fn deploy_program(name: String, mock_bank: &mut MockBankCallback) -> Pubkey {
-    let program_account = Pubkey::new_unique();
-    let program_data_account = Pubkey::new_unique();
-    let state = UpgradeableLoaderState::Program {
-        programdata_address: program_data_account,
-    };
-
-    // The program account must have funds and hold the executable binary
-    let mut account_data = AccountSharedData::default();
-    account_data.set_data(bincode::serialize(&state).unwrap());
-    account_data.set_lamports(25);
-    account_data.set_owner(bpf_loader_upgradeable::id());
-    mock_bank
-        .account_shared_data
-        .borrow_mut()
-        .insert(program_account, account_data);
-
-    let mut account_data = AccountSharedData::default();
-    let state = UpgradeableLoaderState::ProgramData {
-        slot: DEPLOYMENT_SLOT,
-        upgrade_authority_address: None,
-    };
-    let mut header = bincode::serialize(&state).unwrap();
-    let mut complement = vec![
-        0;
-        std::cmp::max(
-            0,
-            UpgradeableLoaderState::size_of_programdata_metadata().saturating_sub(header.len())
-        )
-    ];
-    let mut buffer = load_program(name);
-    header.append(&mut complement);
-    header.append(&mut buffer);
-    account_data.set_data(header);
-    mock_bank
-        .account_shared_data
-        .borrow_mut()
-        .insert(program_data_account, account_data);
-
-    program_account
-}
-
 fn register_builtins(
     mock_bank: &MockBankCallback,
     batch_processor: &TransactionBatchProcessor<MockForkGraph>,
@@ -261,7 +205,7 @@ fn prepare_transactions(
     let mut transaction_checks = Vec::new();
 
     // A transaction that works without any account
-    let hello_program = deploy_program("hello-solana".to_string(), mock_bank);
+    let hello_program = deploy_program("hello-solana".to_string(), DEPLOYMENT_SLOT, mock_bank);
     let fee_payer = Pubkey::new_unique();
     transaction_builder.create_instruction(hello_program, Vec::new(), HashMap::new(), Vec::new());
 
@@ -279,11 +223,13 @@ fn prepare_transactions(
     account_data.set_lamports(80000);
     mock_bank
         .account_shared_data
-        .borrow_mut()
+        .write()
+        .unwrap()
         .insert(fee_payer, account_data);
 
     // A simple funds transfer between accounts
-    let transfer_program_account = deploy_program("simple-transfer".to_string(), mock_bank);
+    let transfer_program_account =
+        deploy_program("simple-transfer".to_string(), DEPLOYMENT_SLOT, mock_bank);
     let sender = Pubkey::new_unique();
     let recipient = Pubkey::new_unique();
     let fee_payer = Pubkey::new_unique();
@@ -327,7 +273,8 @@ fn prepare_transactions(
     account_data.set_lamports(80000);
     mock_bank
         .account_shared_data
-        .borrow_mut()
+        .write()
+        .unwrap()
         .insert(fee_payer, account_data);
 
     // sender
@@ -335,7 +282,8 @@ fn prepare_transactions(
     account_data.set_lamports(900000);
     mock_bank
         .account_shared_data
-        .borrow_mut()
+        .write()
+        .unwrap()
         .insert(sender, account_data);
 
     // recipient
@@ -343,13 +291,14 @@ fn prepare_transactions(
     account_data.set_lamports(900000);
     mock_bank
         .account_shared_data
-        .borrow_mut()
+        .write()
+        .unwrap()
         .insert(recipient, account_data);
 
     // The system account is set in `create_executable_environment`
 
     // A program that utilizes a Sysvar
-    let program_account = deploy_program("clock-sysvar".to_string(), mock_bank);
+    let program_account = deploy_program("clock-sysvar".to_string(), DEPLOYMENT_SLOT, mock_bank);
     let fee_payer = Pubkey::new_unique();
     transaction_builder.create_instruction(program_account, Vec::new(), HashMap::new(), Vec::new());
 
@@ -366,7 +315,8 @@ fn prepare_transactions(
     account_data.set_lamports(80000);
     mock_bank
         .account_shared_data
-        .borrow_mut()
+        .write()
+        .unwrap()
         .insert(fee_payer, account_data);
 
     // A transaction that fails
@@ -411,7 +361,8 @@ fn prepare_transactions(
     account_data.set_lamports(80000);
     mock_bank
         .account_shared_data
-        .borrow_mut()
+        .write()
+        .unwrap()
         .insert(fee_payer, account_data);
 
     // Sender without enough funds
@@ -419,7 +370,8 @@ fn prepare_transactions(
     account_data.set_lamports(900000);
     mock_bank
         .account_shared_data
-        .borrow_mut()
+        .write()
+        .unwrap()
         .insert(sender, account_data);
 
     // recipient
@@ -427,7 +379,8 @@ fn prepare_transactions(
     account_data.set_lamports(900000);
     mock_bank
         .account_shared_data
-        .borrow_mut()
+        .write()
+        .unwrap()
         .insert(recipient, account_data);
 
     // A transaction whose verification has already failed

svm/tests/mock_bank.rs

@@ -1,7 +1,8 @@
 use {
     solana_program_runtime::loaded_programs::{BlockRelation, ForkGraph},
     solana_sdk::{
-        account::{AccountSharedData, ReadableAccount},
+        account::{AccountSharedData, ReadableAccount, WritableAccount},
+        bpf_loader_upgradeable::{self, UpgradeableLoaderState},
         clock::Epoch,
         feature_set::FeatureSet,
         native_loader,
@@ -9,7 +10,14 @@ use {
         slot_hashes::Slot,
     },
     solana_svm::transaction_processing_callback::TransactionProcessingCallback,
-    std::{cell::RefCell, cmp::Ordering, collections::HashMap, sync::Arc},
+    std::{
+        cmp::Ordering,
+        collections::HashMap,
+        env,
+        fs::{self, File},
+        io::Read,
+        sync::{Arc, RwLock},
+    },
 };
 
 pub struct MockForkGraph {}
@@ -28,15 +36,15 @@ impl ForkGraph for MockForkGraph {
     }
 }
 
-#[derive(Default)]
+#[derive(Default, Clone)]
 pub struct MockBankCallback {
     pub feature_set: Arc<FeatureSet>,
-    pub account_shared_data: RefCell<HashMap<Pubkey, AccountSharedData>>,
+    pub account_shared_data: Arc<RwLock<HashMap<Pubkey, AccountSharedData>>>,
 }
 
 impl TransactionProcessingCallback for MockBankCallback {
     fn account_matches_owners(&self, account: &Pubkey, owners: &[Pubkey]) -> Option<usize> {
-        if let Some(data) = self.account_shared_data.borrow().get(account) {
+        if let Some(data) = self.account_shared_data.read().unwrap().get(account) {
             if data.lamports() == 0 {
                 None
             } else {
@@ -48,21 +56,91 @@ impl TransactionProcessingCallback for MockBankCallback {
     }
 
     fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option<AccountSharedData> {
-        self.account_shared_data.borrow().get(pubkey).cloned()
+        self.account_shared_data
+            .read()
+            .unwrap()
+            .get(pubkey)
+            .cloned()
     }
 
     fn add_builtin_account(&self, name: &str, program_id: &Pubkey) {
         let account_data = native_loader::create_loadable_account_with_fields(name, (5000, 0));
 
         self.account_shared_data
-            .borrow_mut()
+            .write()
+            .unwrap()
             .insert(*program_id, account_data);
     }
 }
 
 impl MockBankCallback {
-    #[allow(dead_code)]
+    #[allow(unused)]
     pub fn override_feature_set(&mut self, new_set: FeatureSet) {
         self.feature_set = Arc::new(new_set)
     }
 }
+
+#[allow(unused)]
+fn load_program(name: String) -> Vec<u8> {
+    // Loading the program file
+    let mut dir = env::current_dir().unwrap();
+    dir.push("tests");
+    dir.push("example-programs");
+    dir.push(name.as_str());
+    let name = name.replace('-', "_");
+    dir.push(name + "_program.so");
+    let mut file = File::open(dir.clone()).expect("file not found");
+    let metadata = fs::metadata(dir).expect("Unable to read metadata");
+    let mut buffer = vec![0; metadata.len() as usize];
+    file.read_exact(&mut buffer).expect("Buffer overflow");
+    buffer
+}
+
+#[allow(unused)]
+pub fn deploy_program(
+    name: String,
+    deployment_slot: Slot,
+    mock_bank: &mut MockBankCallback,
+) -> Pubkey {
+    let program_account = Pubkey::new_unique();
+    let program_data_account = Pubkey::new_unique();
+    let state = UpgradeableLoaderState::Program {
+        programdata_address: program_data_account,
+    };
+
+    // The program account must have funds and hold the executable binary
+    let mut account_data = AccountSharedData::default();
+    account_data.set_data(bincode::serialize(&state).unwrap());
+    account_data.set_lamports(25);
+    account_data.set_owner(bpf_loader_upgradeable::id());
+    mock_bank
+        .account_shared_data
+        .write()
+        .unwrap()
+        .insert(program_account, account_data);
+
+    let mut account_data = AccountSharedData::default();
+    let state = UpgradeableLoaderState::ProgramData {
+        slot: deployment_slot,
+        upgrade_authority_address: None,
+    };
+    let mut header = bincode::serialize(&state).unwrap();
+    let mut complement = vec![
+        0;
+        std::cmp::max(
+            0,
+            UpgradeableLoaderState::size_of_programdata_metadata().saturating_sub(header.len())
+        )
+    ];
+    let mut buffer = load_program(name);
+    header.append(&mut complement);
+    header.append(&mut buffer);
+    account_data.set_data(header);
+    mock_bank
+        .account_shared_data
+        .write()
+        .unwrap()
+        .insert(program_data_account, account_data);
+
+    program_account
+}