From 31b1b5fe115470061a79a9da5abd65a452bf5adc Mon Sep 17 00:00:00 2001 From: brooks Date: Fri, 23 Aug 2024 09:30:05 -0400 Subject: [PATCH 1/6] Calls inspect_account() on the fee payer --- svm/src/transaction_processor.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 58ca47269d02f5..623ab991b5fcbf 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -15,7 +15,7 @@ use { transaction_account_state_info::TransactionAccountStateInfo, transaction_error_metrics::TransactionErrorMetrics, transaction_execution_result::{ExecutedTransaction, TransactionExecutionDetails}, - transaction_processing_callback::TransactionProcessingCallback, + transaction_processing_callback::{AccountState, TransactionProcessingCallback}, transaction_processing_result::{ProcessedTransaction, TransactionProcessingResult}, }, log::debug, @@ -434,6 +434,12 @@ impl TransactionBatchProcessor { return Err(TransactionError::AccountNotFound); }; + callbacks.inspect_account( + fee_payer_address, + AccountState::Alive(&fee_payer_account), + true, + ); + let fee_payer_loaded_rent_epoch = fee_payer_account.rent_epoch(); let fee_payer_rent_debit = collect_rent_from_account( feature_set, From eb553b9c316dab564ba4cf27355bac65d2025324 Mon Sep 17 00:00:00 2001 From: brooks Date: Tue, 27 Aug 2024 15:15:52 -0400 Subject: [PATCH 2/6] pr: adds tests --- svm/src/account_loader.rs | 125 ++++++++++++++++++++++++++++++- svm/src/transaction_processor.rs | 88 +++++++++++++++++++++- 2 files changed, 209 insertions(+), 4 deletions(-) diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index f743e6bcba5860..0799ac8ba3cec7 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -608,12 +608,15 @@ mod tests { transaction::{Result, SanitizedTransaction, Transaction, TransactionError}, transaction_context::{TransactionAccount, TransactionContext}, }, - std::{borrow::Cow, collections::HashMap, sync::Arc}, + std::{borrow::Cow, cell::RefCell, collections::HashMap, sync::Arc}, }; #[derive(Default)] struct TestCallbacks { accounts_map: HashMap, + #[allow(clippy::type_complexity)] + inspected_accounts: + RefCell, /* is_writable */ bool)>>>, } impl TransactionProcessingCallback for TestCallbacks { @@ -624,6 +627,23 @@ mod tests { fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option { self.accounts_map.get(pubkey).cloned() } + + fn inspect_account( + &self, + address: &Pubkey, + account_state: AccountState, + is_writable: bool, + ) { + let account = match account_state { + AccountState::Dead => None, + AccountState::Alive(account) => Some(account.clone()), + }; + self.inspected_accounts + .borrow_mut() + .entry(*address) + .or_default() + .push((account, is_writable)); + } } fn load_accounts_with_features_and_rent( @@ -640,7 +660,10 @@ mod tests { for (pubkey, account) in accounts { accounts_map.insert(*pubkey, account.clone()); } - let callbacks = TestCallbacks { accounts_map }; + let callbacks = TestCallbacks { + accounts_map, + ..Default::default() + }; load_accounts( &callbacks, &[sanitized_tx], @@ -929,7 +952,10 @@ mod tests { for (pubkey, account) in accounts { accounts_map.insert(*pubkey, account.clone()); } - let callbacks = TestCallbacks { accounts_map }; + let callbacks = TestCallbacks { + accounts_map, + ..Default::default() + }; load_accounts( &callbacks, &[tx], @@ -2108,4 +2134,97 @@ mod tests { assert_eq!(account.rent_epoch(), 0); assert_eq!(account.lamports(), 0); } + + // Ensure `TransactionProcessingCallback::inspect_account()` is called when + // loading accounts for transaction processing. + #[test] + fn test_inspect_account_non_fee_payer() { + let mut mock_bank = TestCallbacks::default(); + + let address0 = Pubkey::new_unique(); // <-- fee payer + let address1 = Pubkey::new_unique(); // <-- initially alive + let address2 = Pubkey::new_unique(); // <-- initially dead + let address3 = Pubkey::new_unique(); // <-- program + + let mut account0 = AccountSharedData::default(); + account0.set_lamports(1_000_000_000); + mock_bank.accounts_map.insert(address0, account0.clone()); + + let mut account1 = AccountSharedData::default(); + account1.set_lamports(2_000_000_000); + mock_bank.accounts_map.insert(address1, account1.clone()); + + // account2 *not* added to the bank's accounts_map + + let mut account3 = AccountSharedData::default(); + account3.set_lamports(4_000_000_000); + account3.set_executable(true); + account3.set_owner(native_loader::id()); + mock_bank.accounts_map.insert(address3, account3.clone()); + + let message = Message { + account_keys: vec![address0, address1, address2, address3], + header: MessageHeader::default(), + instructions: vec![ + CompiledInstruction { + program_id_index: 3, + accounts: vec![0], + data: vec![], + }, + CompiledInstruction { + program_id_index: 3, + accounts: vec![1, 2], + data: vec![], + }, + CompiledInstruction { + program_id_index: 3, + accounts: vec![1], + data: vec![], + }, + ], + recent_blockhash: Hash::new_unique(), + }; + let sanitized_message = new_unchecked_sanitized_message(message); + let sanitized_transaction = SanitizedTransaction::new_for_tests( + sanitized_message, + vec![Signature::new_unique()], + false, + ); + let validation_result = Ok(ValidatedTransactionDetails { + loaded_fee_payer_account: LoadedTransactionAccount { + account: account0.clone(), + ..LoadedTransactionAccount::default() + }, + ..ValidatedTransactionDetails::default() + }); + let _load_results = load_accounts( + &mock_bank, + &[sanitized_transaction], + vec![validation_result], + &mut TransactionErrorMetrics::default(), + None, + &FeatureSet::default(), + &RentCollector::default(), + &ProgramCacheForTxBatch::default(), + ); + + // ensure the loaded accounts are inspected + let mut actual_inspected_accounts: Vec<_> = mock_bank + .inspected_accounts + .borrow() + .iter() + .map(|(k, v)| (*k, v.clone())) + .collect(); + actual_inspected_accounts.sort_unstable_by(|a, b| a.0.cmp(&b.0)); + + let mut expected_inspected_accounts = vec![ + // *not* key0, since it is loaded during fee payer validation + (address1, vec![(Some(account1), true)]), + (address2, vec![(None, true)]), + (address3, vec![(Some(account3), false)]), + ]; + expected_inspected_accounts.sort_unstable_by(|a, b| a.0.cmp(&b.0)); + + assert_eq!(actual_inspected_accounts, expected_inspected_accounts,); + } } diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 623ab991b5fcbf..c6a5e43149949e 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -437,7 +437,7 @@ impl TransactionBatchProcessor { callbacks.inspect_account( fee_payer_address, AccountState::Alive(&fee_payer_account), - true, + true, // <-- is_writable ); let fee_payer_loaded_rent_epoch = fee_payer_account.rent_epoch(); @@ -1040,6 +1040,9 @@ mod tests { #[derive(Default, Clone)] pub struct MockBankCallback { pub account_shared_data: Arc>>, + #[allow(clippy::type_complexity)] + pub inspected_accounts: + Arc, /* is_writable */ bool)>>>>, } impl TransactionProcessingCallback for MockBankCallback { @@ -1071,6 +1074,24 @@ mod tests { .unwrap() .insert(*program_id, account_data); } + + fn inspect_account( + &self, + address: &Pubkey, + account_state: AccountState, + is_writable: bool, + ) { + let account = match account_state { + AccountState::Dead => None, + AccountState::Alive(account) => Some(account.clone()), + }; + self.inspected_accounts + .write() + .unwrap() + .entry(*address) + .or_default() + .push((account, is_writable)); + } } #[test] @@ -1859,6 +1880,7 @@ mod tests { mock_accounts.insert(*fee_payer_address, fee_payer_account.clone()); let mock_bank = MockBankCallback { account_shared_data: Arc::new(RwLock::new(mock_accounts)), + ..Default::default() }; let mut error_counters = TransactionErrorMetrics::default(); @@ -1936,6 +1958,7 @@ mod tests { mock_accounts.insert(*fee_payer_address, fee_payer_account.clone()); let mock_bank = MockBankCallback { account_shared_data: Arc::new(RwLock::new(mock_accounts)), + ..Default::default() }; let mut error_counters = TransactionErrorMetrics::default(); @@ -2020,6 +2043,7 @@ mod tests { mock_accounts.insert(*fee_payer_address, fee_payer_account.clone()); let mock_bank = MockBankCallback { account_shared_data: Arc::new(RwLock::new(mock_accounts)), + ..Default::default() }; let mut error_counters = TransactionErrorMetrics::default(); @@ -2057,6 +2081,7 @@ mod tests { mock_accounts.insert(*fee_payer_address, fee_payer_account.clone()); let mock_bank = MockBankCallback { account_shared_data: Arc::new(RwLock::new(mock_accounts)), + ..Default::default() }; let mut error_counters = TransactionErrorMetrics::default(); @@ -2092,6 +2117,7 @@ mod tests { mock_accounts.insert(*fee_payer_address, fee_payer_account.clone()); let mock_bank = MockBankCallback { account_shared_data: Arc::new(RwLock::new(mock_accounts)), + ..Default::default() }; let mut error_counters = TransactionErrorMetrics::default(); @@ -2183,6 +2209,7 @@ mod tests { mock_accounts.insert(*fee_payer_address, fee_payer_account.clone()); let mock_bank = MockBankCallback { account_shared_data: Arc::new(RwLock::new(mock_accounts)), + ..Default::default() }; let mut error_counters = TransactionErrorMetrics::default(); @@ -2248,6 +2275,7 @@ mod tests { mock_accounts.insert(*fee_payer_address, fee_payer_account.clone()); let mock_bank = MockBankCallback { account_shared_data: Arc::new(RwLock::new(mock_accounts)), + ..Default::default() }; let mut error_counters = TransactionErrorMetrics::default(); @@ -2300,6 +2328,7 @@ mod tests { let mock_bank = MockBankCallback { account_shared_data: Arc::new(RwLock::new(mock_accounts)), + ..Default::default() }; let mut error_counters = TransactionErrorMetrics::default(); @@ -2324,4 +2353,61 @@ mod tests { result.err() ); } + + // Ensure `TransactionProcessingCallback::inspect_account()` is called when + // validating the fee payer, since that's when the fee payer account is loaded. + #[test] + fn test_inspect_account_fee_payer() { + let fee_payer_address = Pubkey::new_unique(); + let fee_payer_account = AccountSharedData::new_rent_epoch( + 123_000_000_000, + 0, + &Pubkey::default(), + RENT_EXEMPT_RENT_EPOCH, + ); + let mock_bank = MockBankCallback::default(); + mock_bank + .account_shared_data + .write() + .unwrap() + .insert(fee_payer_address, fee_payer_account.clone()); + + let message = new_unchecked_sanitized_message(Message::new_with_blockhash( + &[ + ComputeBudgetInstruction::set_compute_unit_limit(2000u32), + ComputeBudgetInstruction::set_compute_unit_price(1_000_000_000), + ], + Some(&fee_payer_address), + &Hash::new_unique(), + )); + let batch_processor = TransactionBatchProcessor::::default(); + batch_processor + .validate_transaction_fee_payer( + &mock_bank, + None, + &message, + CheckedTransactionDetails { + nonce: None, + lamports_per_signature: 5000, + }, + &FeatureSet::default(), + &FeeStructure::default(), + &RentCollector::default(), + &mut TransactionErrorMetrics::default(), + ) + .unwrap(); + + // ensure the fee payer is an inspected account + let actual_inspected_accounts: Vec<_> = mock_bank + .inspected_accounts + .read() + .unwrap() + .iter() + .map(|(k, v)| (*k, v.clone())) + .collect(); + assert_eq!( + actual_inspected_accounts.as_slice(), + &[(fee_payer_address, vec![(Some(fee_payer_account), true)])], + ); + } } From 2238a6c1c6d9291bd803436934f0b794ce929e7a Mon Sep 17 00:00:00 2001 From: brooks Date: Wed, 28 Aug 2024 15:39:32 -0400 Subject: [PATCH 3/6] pr: add integration test --- svm/tests/integration_test.rs | 197 ++++++++++++++++++++++++++++++++++ svm/tests/mock_bank.rs | 18 +++- 2 files changed, 214 insertions(+), 1 deletion(-) diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 37a4870812c2d8..400fe9588ab8c4 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -655,3 +655,200 @@ fn execute_test_entry(test_entry: SvmTestEntry) { } } } + +#[test] +fn svm_inspect_account() { + let mock_bank = MockBankCallback::default(); + let mut expected_inspected_accounts: HashMap<_, Vec<_>> = HashMap::new(); + + let transfer_program = + 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(); + let system = Pubkey::from([0u8; 32]); + + // Setting up the accounts for the transfer + + // fee payer + let mut account_data = AccountSharedData::default(); + account_data.set_lamports(80_020); + mock_bank + .account_shared_data + .write() + .unwrap() + .insert(fee_payer, account_data.clone()); + expected_inspected_accounts + .entry(fee_payer.clone()) + .or_default() + .push((Some(account_data.clone()), true)); + + // sender + let mut account_data = AccountSharedData::default(); + account_data.set_lamports(11_000_000); + mock_bank + .account_shared_data + .write() + .unwrap() + .insert(sender, account_data.clone()); + expected_inspected_accounts + .entry(sender.clone()) + .or_default() + .push((Some(account_data.clone()), true)); + + // recipient -- initially dead + expected_inspected_accounts + .entry(recipient.clone()) + .or_default() + .push((None, true)); + + let mut transaction_builder = SanitizedTransactionBuilder::default(); + transaction_builder.create_instruction( + transfer_program, + vec![ + AccountMeta { + pubkey: sender, + is_signer: true, + is_writable: true, + }, + AccountMeta { + pubkey: recipient, + is_signer: false, + is_writable: true, + }, + AccountMeta { + pubkey: system, + is_signer: false, + is_writable: false, + }, + ], + HashMap::from([(sender, Signature::new_unique())]), + 1_000_000_u64.to_be_bytes().to_vec(), + ); + let sanitized_transaction = transaction_builder + .build(Hash::default(), (fee_payer, Signature::new_unique()), true) + .unwrap(); + let transaction_check = Ok(CheckedTransactionDetails { + nonce: None, + lamports_per_signature: 20, + }); + + // Load and execute the transaction + + let batch_processor = TransactionBatchProcessor::::new( + EXECUTION_SLOT, + EXECUTION_EPOCH, + HashSet::new(), + ); + + let fork_graph = Arc::new(RwLock::new(MockForkGraph {})); + + create_executable_environment( + fork_graph.clone(), + &mock_bank, + &mut batch_processor.program_cache.write().unwrap(), + ); + + // The sysvars must be put in the cache + batch_processor.fill_missing_sysvar_cache_entries(&mock_bank); + register_builtins(&mock_bank, &batch_processor); + + let _result = batch_processor.load_and_execute_sanitized_transactions( + &mock_bank, + &[sanitized_transaction], + vec![transaction_check], + &TransactionProcessingEnvironment::default(), + &TransactionProcessingConfig::default(), + ); + + // do another transfer; recipient should be alive now + + // fee payer + let mut account_data = AccountSharedData::default(); + account_data.set_lamports(80_000); + mock_bank + .account_shared_data + .write() + .unwrap() + .insert(fee_payer, account_data.clone()); + expected_inspected_accounts + .entry(fee_payer.clone()) + .or_default() + .push((Some(account_data.clone()), true)); + + // sender + let mut account_data = AccountSharedData::default(); + account_data.set_lamports(10_000_000); + mock_bank + .account_shared_data + .write() + .unwrap() + .insert(sender, account_data.clone()); + expected_inspected_accounts + .entry(sender.clone()) + .or_default() + .push((Some(account_data.clone()), true)); + + // recipient -- now alive + let mut account_data = AccountSharedData::default(); + account_data.set_lamports(1_000_000); + mock_bank + .account_shared_data + .write() + .unwrap() + .insert(recipient, account_data.clone()); + expected_inspected_accounts + .entry(recipient.clone()) + .or_default() + .push((Some(account_data.clone()), true)); + + let mut transaction_builder = SanitizedTransactionBuilder::default(); + transaction_builder.create_instruction( + transfer_program, + vec![ + AccountMeta { + pubkey: sender, + is_signer: true, + is_writable: true, + }, + AccountMeta { + pubkey: recipient, + is_signer: false, + is_writable: true, + }, + AccountMeta { + pubkey: system, + is_signer: false, + is_writable: false, + }, + ], + HashMap::from([(sender, Signature::new_unique())]), + 456_u64.to_be_bytes().to_vec(), + ); + let sanitized_transaction = transaction_builder + .build(Hash::default(), (fee_payer, Signature::new_unique()), true) + .unwrap(); + let transaction_check = Ok(CheckedTransactionDetails { + nonce: None, + lamports_per_signature: 20, + }); + + // Load and execute the second transaction + let _result = batch_processor.load_and_execute_sanitized_transactions( + &mock_bank, + &[sanitized_transaction], + vec![transaction_check], + &TransactionProcessingEnvironment::default(), + &TransactionProcessingConfig::default(), + ); + + // Ensure all the expected inspected accounts were inspected + let actual_inspected_accounts = mock_bank.inspected_accounts.read().unwrap().clone(); + for (expected_pubkey, expected_account) in expected_inspected_accounts { + let actual_account = actual_inspected_accounts.get(&expected_pubkey).unwrap(); + assert_eq!( + expected_account, *actual_account, + "pubkey: {expected_pubkey}", + ); + } +} diff --git a/svm/tests/mock_bank.rs b/svm/tests/mock_bank.rs index ee6a6692a7b0fe..6ece85bfcc1385 100644 --- a/svm/tests/mock_bank.rs +++ b/svm/tests/mock_bank.rs @@ -25,7 +25,7 @@ use { sysvar::SysvarId, }, solana_svm::{ - transaction_processing_callback::TransactionProcessingCallback, + transaction_processing_callback::{AccountState, TransactionProcessingCallback}, transaction_processor::TransactionBatchProcessor, }, solana_type_overrides::sync::{Arc, RwLock}, @@ -56,6 +56,9 @@ impl ForkGraph for MockForkGraph { pub struct MockBankCallback { pub feature_set: Arc, pub account_shared_data: Arc>>, + #[allow(clippy::type_complexity)] + pub inspected_accounts: + Arc, /* is_writable */ bool)>>>>, } impl TransactionProcessingCallback for MockBankCallback { @@ -87,6 +90,19 @@ impl TransactionProcessingCallback for MockBankCallback { .unwrap() .insert(*program_id, account_data); } + + fn inspect_account(&self, address: &Pubkey, account_state: AccountState, is_writable: bool) { + let account = match account_state { + AccountState::Dead => None, + AccountState::Alive(account) => Some(account.clone()), + }; + self.inspected_accounts + .write() + .unwrap() + .entry(*address) + .or_default() + .push((account, is_writable)); + } } impl MockBankCallback { From 5ee617ff6a69db781c7e75de1c5571e62e3f34e5 Mon Sep 17 00:00:00 2001 From: brooks Date: Thu, 29 Aug 2024 18:12:49 -0400 Subject: [PATCH 4/6] clippy --- svm/tests/integration_test.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 400fe9588ab8c4..26f283a00a8829 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -679,7 +679,7 @@ fn svm_inspect_account() { .unwrap() .insert(fee_payer, account_data.clone()); expected_inspected_accounts - .entry(fee_payer.clone()) + .entry(fee_payer) .or_default() .push((Some(account_data.clone()), true)); @@ -692,13 +692,13 @@ fn svm_inspect_account() { .unwrap() .insert(sender, account_data.clone()); expected_inspected_accounts - .entry(sender.clone()) + .entry(sender) .or_default() .push((Some(account_data.clone()), true)); // recipient -- initially dead expected_inspected_accounts - .entry(recipient.clone()) + .entry(recipient) .or_default() .push((None, true)); @@ -772,7 +772,7 @@ fn svm_inspect_account() { .unwrap() .insert(fee_payer, account_data.clone()); expected_inspected_accounts - .entry(fee_payer.clone()) + .entry(fee_payer) .or_default() .push((Some(account_data.clone()), true)); @@ -785,7 +785,7 @@ fn svm_inspect_account() { .unwrap() .insert(sender, account_data.clone()); expected_inspected_accounts - .entry(sender.clone()) + .entry(sender) .or_default() .push((Some(account_data.clone()), true)); @@ -798,7 +798,7 @@ fn svm_inspect_account() { .unwrap() .insert(recipient, account_data.clone()); expected_inspected_accounts - .entry(recipient.clone()) + .entry(recipient) .or_default() .push((Some(account_data.clone()), true)); From a2556cc04254728f15515790efb6bb85d2339198 Mon Sep 17 00:00:00 2001 From: brooks Date: Fri, 30 Aug 2024 11:30:50 -0400 Subject: [PATCH 5/6] pr: check non-writable accounts in integration test --- svm/tests/integration_test.rs | 55 +++++++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 3 deletions(-) diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 26f283a00a8829..4371eec8714d0a 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -761,6 +761,19 @@ fn svm_inspect_account() { &TransactionProcessingConfig::default(), ); + // the system account is modified during transaction processing, + // so set the expected inspected account afterwards. + let system_account = mock_bank + .account_shared_data + .read() + .unwrap() + .get(&system) + .cloned(); + expected_inspected_accounts + .entry(system) + .or_default() + .push((system_account, false)); + // do another transfer; recipient should be alive now // fee payer @@ -842,13 +855,49 @@ fn svm_inspect_account() { &TransactionProcessingConfig::default(), ); + // the system account is modified during transaction processing, + // so set the expected inspected account afterwards. + let system_account = mock_bank + .account_shared_data + .read() + .unwrap() + .get(&system) + .cloned(); + expected_inspected_accounts + .entry(system) + .or_default() + .push((system_account, false)); + // Ensure all the expected inspected accounts were inspected let actual_inspected_accounts = mock_bank.inspected_accounts.read().unwrap().clone(); - for (expected_pubkey, expected_account) in expected_inspected_accounts { - let actual_account = actual_inspected_accounts.get(&expected_pubkey).unwrap(); + for (expected_pubkey, expected_account) in &expected_inspected_accounts { + let actual_account = actual_inspected_accounts.get(expected_pubkey).unwrap(); assert_eq!( - expected_account, *actual_account, + expected_account, actual_account, "pubkey: {expected_pubkey}", ); } + + // The transfer program account is also loaded during transaction processing, however the + // account state passed to `inspect_account()` is *not* the same as what is held by + // MockBankCallback::account_shared_data. So we check the transfer program differently. + // + // First ensure we have the correct number of inspected accounts, correctly counting the + // transfer program. + let num_expected_inspected_accounts: usize = + expected_inspected_accounts.values().map(Vec::len).sum(); + let num_actual_inspected_accounts: usize = + actual_inspected_accounts.values().map(Vec::len).sum(); + assert_eq!( + num_expected_inspected_accounts + 2, + num_actual_inspected_accounts, + ); + + // And second, ensure the inspected transfer program accounts are alive and not writable. + let actual_transfer_program_accounts = + actual_inspected_accounts.get(&transfer_program).unwrap(); + for actual_transfer_program_account in actual_transfer_program_accounts { + assert!(actual_transfer_program_account.0.is_some()); + assert!(!actual_transfer_program_account.1); + } } From b19d98cea1902887c78a1bfb162dcd57176759a3 Mon Sep 17 00:00:00 2001 From: brooks Date: Fri, 30 Aug 2024 14:30:58 -0400 Subject: [PATCH 6/6] updates to handle the refactor --- svm/tests/integration_test.rs | 118 +++++++++++++++------------------- 1 file changed, 51 insertions(+), 67 deletions(-) diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 4371eec8714d0a..08f6db09101e04 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -663,38 +663,42 @@ fn svm_inspect_account() { let transfer_program = deploy_program("simple-transfer".to_string(), DEPLOYMENT_SLOT, &mock_bank); - let sender = Pubkey::new_unique(); + + let fee_payer_keypair = Keypair::new(); + let sender_keypair = Keypair::new(); + + let fee_payer = fee_payer_keypair.pubkey(); + let sender = sender_keypair.pubkey(); let recipient = Pubkey::new_unique(); - let fee_payer = Pubkey::new_unique(); - let system = Pubkey::from([0u8; 32]); + let system = system_program::id(); // Setting up the accounts for the transfer // fee payer - let mut account_data = AccountSharedData::default(); - account_data.set_lamports(80_020); + let mut fee_payer_account = AccountSharedData::default(); + fee_payer_account.set_lamports(80_020); mock_bank .account_shared_data .write() .unwrap() - .insert(fee_payer, account_data.clone()); + .insert(fee_payer, fee_payer_account.clone()); expected_inspected_accounts .entry(fee_payer) .or_default() - .push((Some(account_data.clone()), true)); + .push((Some(fee_payer_account.clone()), true)); // sender - let mut account_data = AccountSharedData::default(); - account_data.set_lamports(11_000_000); + let mut sender_account = AccountSharedData::default(); + sender_account.set_lamports(11_000_000); mock_bank .account_shared_data .write() .unwrap() - .insert(sender, account_data.clone()); + .insert(sender, sender_account.clone()); expected_inspected_accounts .entry(sender) .or_default() - .push((Some(account_data.clone()), true)); + .push((Some(sender_account.clone()), true)); // recipient -- initially dead expected_inspected_accounts @@ -702,32 +706,22 @@ fn svm_inspect_account() { .or_default() .push((None, true)); - let mut transaction_builder = SanitizedTransactionBuilder::default(); - transaction_builder.create_instruction( + let instruction = Instruction::new_with_bytes( transfer_program, + &u64::to_be_bytes(1_000_000), vec![ - AccountMeta { - pubkey: sender, - is_signer: true, - is_writable: true, - }, - AccountMeta { - pubkey: recipient, - is_signer: false, - is_writable: true, - }, - AccountMeta { - pubkey: system, - is_signer: false, - is_writable: false, - }, + AccountMeta::new(sender, true), + AccountMeta::new(recipient, false), + AccountMeta::new_readonly(system, false), ], - HashMap::from([(sender, Signature::new_unique())]), - 1_000_000_u64.to_be_bytes().to_vec(), ); - let sanitized_transaction = transaction_builder - .build(Hash::default(), (fee_payer, Signature::new_unique()), true) - .unwrap(); + let transaction = Transaction::new_signed_with_payer( + &[instruction], + Some(&fee_payer), + &[&fee_payer_keypair, &sender_keypair], + Hash::default(), + ); + let sanitized_transaction = SanitizedTransaction::from_transaction_for_tests(transaction); let transaction_check = Ok(CheckedTransactionDetails { nonce: None, lamports_per_signature: 20, @@ -777,70 +771,60 @@ fn svm_inspect_account() { // do another transfer; recipient should be alive now // fee payer - let mut account_data = AccountSharedData::default(); - account_data.set_lamports(80_000); + let mut fee_payer_account = AccountSharedData::default(); + fee_payer_account.set_lamports(80_000); mock_bank .account_shared_data .write() .unwrap() - .insert(fee_payer, account_data.clone()); + .insert(fee_payer, fee_payer_account.clone()); expected_inspected_accounts .entry(fee_payer) .or_default() - .push((Some(account_data.clone()), true)); + .push((Some(fee_payer_account.clone()), true)); // sender - let mut account_data = AccountSharedData::default(); - account_data.set_lamports(10_000_000); + let mut sender_account = AccountSharedData::default(); + sender_account.set_lamports(10_000_000); mock_bank .account_shared_data .write() .unwrap() - .insert(sender, account_data.clone()); + .insert(sender, sender_account.clone()); expected_inspected_accounts .entry(sender) .or_default() - .push((Some(account_data.clone()), true)); + .push((Some(sender_account.clone()), true)); // recipient -- now alive - let mut account_data = AccountSharedData::default(); - account_data.set_lamports(1_000_000); + let mut recipient_account = AccountSharedData::default(); + recipient_account.set_lamports(1_000_000); mock_bank .account_shared_data .write() .unwrap() - .insert(recipient, account_data.clone()); + .insert(recipient, recipient_account.clone()); expected_inspected_accounts .entry(recipient) .or_default() - .push((Some(account_data.clone()), true)); + .push((Some(recipient_account.clone()), true)); - let mut transaction_builder = SanitizedTransactionBuilder::default(); - transaction_builder.create_instruction( + let instruction = Instruction::new_with_bytes( transfer_program, + &u64::to_be_bytes(456), vec![ - AccountMeta { - pubkey: sender, - is_signer: true, - is_writable: true, - }, - AccountMeta { - pubkey: recipient, - is_signer: false, - is_writable: true, - }, - AccountMeta { - pubkey: system, - is_signer: false, - is_writable: false, - }, + AccountMeta::new(sender, true), + AccountMeta::new(recipient, false), + AccountMeta::new_readonly(system, false), ], - HashMap::from([(sender, Signature::new_unique())]), - 456_u64.to_be_bytes().to_vec(), ); - let sanitized_transaction = transaction_builder - .build(Hash::default(), (fee_payer, Signature::new_unique()), true) - .unwrap(); + let transaction = Transaction::new_signed_with_payer( + &[instruction], + Some(&fee_payer), + &[&fee_payer_keypair, &sender_keypair], + Hash::default(), + ); + let sanitized_transaction = SanitizedTransaction::from_transaction_for_tests(transaction); let transaction_check = Ok(CheckedTransactionDetails { nonce: None, lamports_per_signature: 20,