From f60c680ddb9bf79b03a062bf4f1e0ccc1f57f0f0 Mon Sep 17 00:00:00 2001 From: febo Date: Sat, 11 Jan 2025 22:38:31 +0000 Subject: [PATCH 1/4] Use mollusk on process instruction --- program/tests/processor.rs | 286 +++++++++++++++++++------------------ 1 file changed, 147 insertions(+), 139 deletions(-) diff --git a/program/tests/processor.rs b/program/tests/processor.rs index 2d173155..c7099ca6 100644 --- a/program/tests/processor.rs +++ b/program/tests/processor.rs @@ -1,21 +1,21 @@ #![cfg(feature = "test-sbf")] +//! Program state processor tests + use { + mollusk_svm::{result::ProgramResult as MolluskResult, Mollusk}, serial_test::serial, - solana_program::{ - account_info::IntoAccountInfo, instruction::Instruction, program_error, sysvar::rent, - }, solana_sdk::{ - account::{ - create_account_for_test, create_is_signer_account_infos, Account as SolanaAccount, - }, - account_info::AccountInfo, + account::{create_account_for_test, Account as SolanaAccount, AccountSharedData}, + account_info::{AccountInfo, IntoAccountInfo}, entrypoint::ProgramResult, + instruction::Instruction, program_error::ProgramError, program_option::COption, program_pack::Pack, pubkey::Pubkey, rent::Rent, + sysvar::rent, }, spl_token::{ error::TokenError, @@ -27,87 +27,114 @@ use { set_authority, sync_native, thaw_account, transfer, transfer_checked, ui_amount_to_amount, AuthorityType, MAX_SIGNERS, }, - processor::Processor, state::{Account, AccountState, Mint, Multisig}, }, - std::sync::{Arc, RwLock}, + std::{ + collections::HashMap, + sync::{Arc, RwLock}, + }, }; lazy_static::lazy_static! { static ref EXPECTED_DATA: Arc>> = Arc::new(RwLock::new(Vec::new())); } -fn set_expected_data(expected_data: Vec) { - *EXPECTED_DATA.write().unwrap() = expected_data; -} - -struct SyscallStubs {} -impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs { - fn sol_log(&self, _message: &str) {} - - fn sol_invoke_signed( - &self, - _instruction: &Instruction, - _account_infos: &[AccountInfo], - _signers_seeds: &[&[&[u8]]], - ) -> ProgramResult { - Err(ProgramError::Custom(42)) // Not supported - } - - fn sol_get_clock_sysvar(&self, _var_addr: *mut u8) -> u64 { - program_error::UNSUPPORTED_SYSVAR - } +fn do_process_instruction( + instruction: Instruction, + mut accounts: Vec<&mut SolanaAccount>, +) -> ProgramResult { + let mut instruction_accounts = Vec::new(); - fn sol_get_epoch_schedule_sysvar(&self, _var_addr: *mut u8) -> u64 { - program_error::UNSUPPORTED_SYSVAR - } + // Prepare accounts for mollusk. + instruction + .accounts + .iter() + .zip(&accounts) + .map(|(account_meta, account)| { + ( + account_meta.pubkey, + AccountSharedData::from((*account).clone()), + ) + }) + .for_each(|(pubkey, account)| { + instruction_accounts.push((pubkey, account)); + }); - #[allow(deprecated)] - fn sol_get_fees_sysvar(&self, _var_addr: *mut u8) -> u64 { - program_error::UNSUPPORTED_SYSVAR - } + let mollusk = Mollusk::new(&spl_token::ID, "spl_token"); + let result = + mollusk.process_and_validate_instruction_chain(&[instruction], &instruction_accounts, &[]); - fn sol_get_rent_sysvar(&self, var_addr: *mut u8) -> u64 { - unsafe { - *(var_addr as *mut _ as *mut Rent) = Rent::default(); - } - solana_program::entrypoint::SUCCESS + // Update accounts after the instruction is processed. + for (original, (_, updated)) in accounts.iter_mut().zip(result.resulting_accounts.iter()) { + let account = SolanaAccount::from(updated.clone()); + original.data = account.data; + original.lamports = account.lamports; + original.owner = account.owner; } - fn sol_set_return_data(&self, data: &[u8]) { - assert_eq!(&*EXPECTED_DATA.write().unwrap(), data) + match result.program_result { + MolluskResult::Success => Ok(()), + MolluskResult::Failure(err) => Err(err), + MolluskResult::UnknownError(err) => panic!("Unknown error: {:?}", err), } } -fn do_process_instruction( +fn do_process_instruction_dups( instruction: Instruction, - accounts: Vec<&mut SolanaAccount>, + account_infos: Vec, ) -> ProgramResult { - { - use std::sync::Once; - static ONCE: Once = Once::new(); + let mut cached_accounts = HashMap::new(); + let mut dedup_accounts = Vec::new(); + + // Dedup accounts for mollusk. + account_infos.iter().for_each(|account_info| { + if !cached_accounts.contains_key(account_info.key) { + let account = SolanaAccount { + lamports: account_info.lamports(), + data: account_info.try_borrow_data().unwrap().to_vec(), + owner: *account_info.owner, + executable: account_info.executable, + rent_epoch: account_info.rent_epoch, + }; + dedup_accounts.push((*account_info.key, AccountSharedData::from(account))); + cached_accounts.insert(account_info.key, account_info); + } + }); - ONCE.call_once(|| { - solana_sdk::program_stubs::set_syscall_stubs(Box::new(SyscallStubs {})); - }); - } + let mollusk = Mollusk::new(&spl_token::ID, "spl_token"); + let result = + mollusk.process_and_validate_instruction_chain(&[instruction], &dedup_accounts, &[]); - let mut meta = instruction - .accounts + // Update accounts after the instruction is processed. + result + .resulting_accounts .iter() - .zip(accounts) - .map(|(account_meta, account)| (&account_meta.pubkey, account_meta.is_signer, account)) - .collect::>(); + .for_each(|(pubkey, account)| { + let account = SolanaAccount::from(account.clone()); + let account_info = cached_accounts.get(pubkey).unwrap(); + if account.data.is_empty() { + // When the account is closed, the tests expect the data to + // be zeroed. + account_info.try_borrow_mut_data().unwrap().fill(0); + } else { + account_info + .try_borrow_mut_data() + .unwrap() + .copy_from_slice(&account.data); + } + **account_info.try_borrow_mut_lamports().unwrap() = account.lamports; + account_info.assign(&account.owner); + }); - let account_infos = create_is_signer_account_infos(&mut meta); - Processor::process(&instruction.program_id, &account_infos, &instruction.data) + match result.program_result { + MolluskResult::Success => Ok(()), + MolluskResult::Failure(err) => Err(err), + MolluskResult::UnknownError(err) => panic!("Unknown error: {:?}", err), + } } -fn do_process_instruction_dups( - instruction: Instruction, - account_infos: Vec, -) -> ProgramResult { - Processor::process(&instruction.program_id, &account_infos, &instruction.data) +fn set_expected_data(expected_data: Vec) { + *EXPECTED_DATA.write().unwrap() = expected_data; } fn rent_sysvar() -> SolanaAccount { @@ -1217,14 +1244,13 @@ fn test_self_transfer() { .unwrap(); assert_eq!( Ok(()), - Processor::process( - &instruction.program_id, - &[ + do_process_instruction_dups( + instruction, + vec![ account_info.clone(), account_info.clone(), owner_info.clone(), ], - &instruction.data, ) ); // no balance change... @@ -1245,15 +1271,14 @@ fn test_self_transfer() { .unwrap(); assert_eq!( Ok(()), - Processor::process( - &instruction.program_id, - &[ + do_process_instruction_dups( + instruction, + vec![ account_info.clone(), mint_info.clone(), account_info.clone(), owner_info.clone(), ], - &instruction.data, ) ); // no balance change... @@ -1275,14 +1300,13 @@ fn test_self_transfer() { owner_no_sign_info.is_signer = false; assert_eq!( Err(ProgramError::MissingRequiredSignature), - Processor::process( - &instruction.program_id, - &[ + do_process_instruction_dups( + instruction, + vec![ account_info.clone(), account_info.clone(), owner_no_sign_info.clone(), ], - &instruction.data, ) ); @@ -1301,15 +1325,14 @@ fn test_self_transfer() { instruction.accounts[3].is_signer = false; assert_eq!( Err(ProgramError::MissingRequiredSignature), - Processor::process( - &instruction.program_id, - &[ + do_process_instruction_dups( + instruction, + vec![ account_info.clone(), mint_info.clone(), account_info.clone(), owner_no_sign_info, ], - &instruction.data, ) ); @@ -1325,14 +1348,13 @@ fn test_self_transfer() { .unwrap(); assert_eq!( Err(TokenError::OwnerMismatch.into()), - Processor::process( - &instruction.program_id, - &[ + do_process_instruction_dups( + instruction, + vec![ account_info.clone(), account_info.clone(), owner2_info.clone(), ], - &instruction.data, ) ); @@ -1350,15 +1372,14 @@ fn test_self_transfer() { .unwrap(); assert_eq!( Err(TokenError::OwnerMismatch.into()), - Processor::process( - &instruction.program_id, - &[ + do_process_instruction_dups( + instruction, + vec![ account_info.clone(), mint_info.clone(), account_info.clone(), owner2_info.clone(), ], - &instruction.data, ) ); @@ -1374,14 +1395,13 @@ fn test_self_transfer() { .unwrap(); assert_eq!( Err(TokenError::InsufficientFunds.into()), - Processor::process( - &instruction.program_id, - &[ + do_process_instruction_dups( + instruction, + vec![ account_info.clone(), account_info.clone(), owner_info.clone(), ], - &instruction.data, ) ); @@ -1399,15 +1419,14 @@ fn test_self_transfer() { .unwrap(); assert_eq!( Err(TokenError::InsufficientFunds.into()), - Processor::process( - &instruction.program_id, - &[ + do_process_instruction_dups( + instruction, + vec![ account_info.clone(), mint_info.clone(), account_info.clone(), owner_info.clone(), ], - &instruction.data, ) ); @@ -1425,15 +1444,14 @@ fn test_self_transfer() { .unwrap(); assert_eq!( Err(TokenError::MintDecimalsMismatch.into()), - Processor::process( - &instruction.program_id, - &[ + do_process_instruction_dups( + instruction, + vec![ account_info.clone(), mint_info.clone(), account_info.clone(), owner_info.clone(), ], - &instruction.data, ) ); @@ -1451,15 +1469,14 @@ fn test_self_transfer() { .unwrap(); assert_eq!( Err(TokenError::MintMismatch.into()), - Processor::process( - &instruction.program_id, - &[ + do_process_instruction_dups( + instruction, + vec![ account_info.clone(), account3_info.clone(), // <-- incorrect mint account_info.clone(), owner_info.clone(), ], - &instruction.data, ) ); @@ -1473,14 +1490,13 @@ fn test_self_transfer() { 100, ) .unwrap(); - Processor::process( - &instruction.program_id, - &[ + do_process_instruction_dups( + instruction, + vec![ account_info.clone(), delegate_info.clone(), owner_info.clone(), ], - &instruction.data, ) .unwrap(); @@ -1496,14 +1512,13 @@ fn test_self_transfer() { .unwrap(); assert_eq!( Ok(()), - Processor::process( - &instruction.program_id, - &[ + do_process_instruction_dups( + instruction, + vec![ account_info.clone(), account_info.clone(), delegate_info.clone(), ], - &instruction.data, ) ); // no balance change... @@ -1525,15 +1540,14 @@ fn test_self_transfer() { .unwrap(); assert_eq!( Ok(()), - Processor::process( - &instruction.program_id, - &[ + do_process_instruction_dups( + instruction, + vec![ account_info.clone(), mint_info.clone(), account_info.clone(), delegate_info.clone(), ], - &instruction.data, ) ); // no balance change... @@ -1553,14 +1567,13 @@ fn test_self_transfer() { .unwrap(); assert_eq!( Err(TokenError::InsufficientFunds.into()), - Processor::process( - &instruction.program_id, - &[ + do_process_instruction_dups( + instruction, + vec![ account_info.clone(), account_info.clone(), delegate_info.clone(), ], - &instruction.data, ) ); @@ -1578,15 +1591,14 @@ fn test_self_transfer() { .unwrap(); assert_eq!( Err(TokenError::InsufficientFunds.into()), - Processor::process( - &instruction.program_id, - &[ + do_process_instruction_dups( + instruction, + vec![ account_info.clone(), mint_info.clone(), account_info.clone(), delegate_info.clone(), ], - &instruction.data, ) ); @@ -1602,14 +1614,13 @@ fn test_self_transfer() { .unwrap(); assert_eq!( Ok(()), - Processor::process( - &instruction.program_id, - &[ + do_process_instruction_dups( + instruction, + vec![ account_info.clone(), account_info.clone(), owner_info.clone(), ], - &instruction.data, ) ); // no balance change... @@ -1630,15 +1641,14 @@ fn test_self_transfer() { .unwrap(); assert_eq!( Ok(()), - Processor::process( - &instruction.program_id, - &[ + do_process_instruction_dups( + instruction, + vec![ account_info.clone(), mint_info.clone(), account_info.clone(), owner_info.clone(), ], - &instruction.data, ) ); // no balance change... @@ -3020,8 +3030,8 @@ fn test_burn_dups() { do_process_instruction_dups( burn( &program_id, - &mint_key, &account1_key, + &mint_key, &account1_key, &[], 500, @@ -4406,10 +4416,9 @@ fn test_close_account() { ], ) .unwrap(); + assert!(account_account.data.is_empty()); assert_eq!(account_account.lamports, 0); assert_eq!(account3_account.lamports, 2 * account_minimum_balance()); - let account = Account::unpack_unchecked(&account_account.data).unwrap(); - assert_eq!(account.amount, 0); // fund and initialize new non-native account to test close authority let account_key = Pubkey::new_unique(); @@ -4473,10 +4482,9 @@ fn test_close_account() { ], ) .unwrap(); + assert!(account_account.data.is_empty()); assert_eq!(account_account.lamports, 0); assert_eq!(account3_account.lamports, 2 * account_minimum_balance() + 2); - let account = Account::unpack_unchecked(&account_account.data).unwrap(); - assert_eq!(account.amount, 0); // close native account do_process_instruction( @@ -4488,7 +4496,7 @@ fn test_close_account() { ], ) .unwrap(); - assert_eq!(account2_account.data, [0u8; Account::LEN]); + assert!(account2_account.data.is_empty()); assert_eq!( account3_account.lamports, 3 * account_minimum_balance() + 2 + 42 @@ -4706,7 +4714,7 @@ fn test_native_token() { .unwrap(); assert_eq!(account_account.lamports, 0); assert_eq!(account3_account.lamports, 2 * account_minimum_balance()); - assert_eq!(account_account.data, [0u8; Account::LEN]); + assert!(account_account.data.is_empty()); } #[test] From db6c1ebafd3127415bbee068733357b2b41f9dae Mon Sep 17 00:00:00 2001 From: febo Date: Sat, 11 Jan 2025 22:47:13 +0000 Subject: [PATCH 2/4] Enable workflow --- .github/workflows/main.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 46166f3a..1a90eed6 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -4,7 +4,6 @@ on: push: branches: [main] pull_request: - branches: [main] jobs: format_and_lint_client_js: From e49de7a265af3a3f2763c2318c2b4278cf4733c8 Mon Sep 17 00:00:00 2001 From: febo Date: Sun, 12 Jan 2025 02:02:43 +0000 Subject: [PATCH 3/4] Use non-chain process instruction --- program/tests/processor.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/program/tests/processor.rs b/program/tests/processor.rs index c7099ca6..d4aa2a1a 100644 --- a/program/tests/processor.rs +++ b/program/tests/processor.rs @@ -61,8 +61,7 @@ fn do_process_instruction( }); let mollusk = Mollusk::new(&spl_token::ID, "spl_token"); - let result = - mollusk.process_and_validate_instruction_chain(&[instruction], &instruction_accounts, &[]); + let result = mollusk.process_and_validate_instruction(&instruction, &instruction_accounts, &[]); // Update accounts after the instruction is processed. for (original, (_, updated)) in accounts.iter_mut().zip(result.resulting_accounts.iter()) { @@ -102,8 +101,7 @@ fn do_process_instruction_dups( }); let mollusk = Mollusk::new(&spl_token::ID, "spl_token"); - let result = - mollusk.process_and_validate_instruction_chain(&[instruction], &dedup_accounts, &[]); + let result = mollusk.process_and_validate_instruction(&instruction, &dedup_accounts, &[]); // Update accounts after the instruction is processed. result From b8d47b77620e5ce6139325c7e8a1244e3bbbdfbd Mon Sep 17 00:00:00 2001 From: febo Date: Wed, 15 Jan 2025 10:32:27 +0000 Subject: [PATCH 4/4] Address review comments --- program/tests/processor.rs | 59 +++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 32 deletions(-) diff --git a/program/tests/processor.rs b/program/tests/processor.rs index d4aa2a1a..32e0779f 100644 --- a/program/tests/processor.rs +++ b/program/tests/processor.rs @@ -3,10 +3,12 @@ //! Program state processor tests use { - mollusk_svm::{result::ProgramResult as MolluskResult, Mollusk}, + mollusk_svm::Mollusk, serial_test::serial, solana_sdk::{ - account::{create_account_for_test, Account as SolanaAccount, AccountSharedData}, + account::{ + create_account_for_test, Account as SolanaAccount, AccountSharedData, ReadableAccount, + }, account_info::{AccountInfo, IntoAccountInfo}, entrypoint::ProgramResult, instruction::Instruction, @@ -43,10 +45,8 @@ fn do_process_instruction( instruction: Instruction, mut accounts: Vec<&mut SolanaAccount>, ) -> ProgramResult { - let mut instruction_accounts = Vec::new(); - // Prepare accounts for mollusk. - instruction + let instruction_accounts: Vec<(Pubkey, AccountSharedData)> = instruction .accounts .iter() .zip(&accounts) @@ -56,26 +56,24 @@ fn do_process_instruction( AccountSharedData::from((*account).clone()), ) }) - .for_each(|(pubkey, account)| { - instruction_accounts.push((pubkey, account)); - }); + .collect(); let mollusk = Mollusk::new(&spl_token::ID, "spl_token"); - let result = mollusk.process_and_validate_instruction(&instruction, &instruction_accounts, &[]); + let result = mollusk.process_instruction(&instruction, &instruction_accounts); // Update accounts after the instruction is processed. - for (original, (_, updated)) in accounts.iter_mut().zip(result.resulting_accounts.iter()) { - let account = SolanaAccount::from(updated.clone()); - original.data = account.data; - original.lamports = account.lamports; - original.owner = account.owner; + for (original, (_, updated)) in accounts + .iter_mut() + .zip(result.resulting_accounts.into_iter()) + { + original.data = updated.data().to_vec(); + original.lamports = updated.lamports(); + original.owner = *updated.owner(); } - match result.program_result { - MolluskResult::Success => Ok(()), - MolluskResult::Failure(err) => Err(err), - MolluskResult::UnknownError(err) => panic!("Unknown error: {:?}", err), - } + result + .raw_result + .map_err(|e| ProgramError::try_from(e).unwrap()) } fn do_process_instruction_dups( @@ -101,16 +99,15 @@ fn do_process_instruction_dups( }); let mollusk = Mollusk::new(&spl_token::ID, "spl_token"); - let result = mollusk.process_and_validate_instruction(&instruction, &dedup_accounts, &[]); + let result = mollusk.process_instruction(&instruction, &dedup_accounts); // Update accounts after the instruction is processed. result .resulting_accounts - .iter() + .into_iter() .for_each(|(pubkey, account)| { - let account = SolanaAccount::from(account.clone()); - let account_info = cached_accounts.get(pubkey).unwrap(); - if account.data.is_empty() { + let account_info = cached_accounts.get(&pubkey).unwrap(); + if account.data().is_empty() { // When the account is closed, the tests expect the data to // be zeroed. account_info.try_borrow_mut_data().unwrap().fill(0); @@ -118,17 +115,15 @@ fn do_process_instruction_dups( account_info .try_borrow_mut_data() .unwrap() - .copy_from_slice(&account.data); + .copy_from_slice(account.data()); } - **account_info.try_borrow_mut_lamports().unwrap() = account.lamports; - account_info.assign(&account.owner); + **account_info.try_borrow_mut_lamports().unwrap() = account.lamports(); + account_info.assign(account.owner()); }); - match result.program_result { - MolluskResult::Success => Ok(()), - MolluskResult::Failure(err) => Err(err), - MolluskResult::UnknownError(err) => panic!("Unknown error: {:?}", err), - } + result + .raw_result + .map_err(|e| ProgramError::try_from(e).unwrap()) } fn set_expected_data(expected_data: Vec) {