Use BPF serialization ABI for native target in program-test; Fix account data resizing and editing (#26507)

* Use BPF serialization ABI for native target in program-test (PR22754)

* Fixes for the program-test PR22754 rebase on v1.11.1

* Include bug test case in program-test unit tests

* Fix segfault on duped accounts

* Add CPI create_account test

* Revert gitignore addition

Co-authored-by: jozanza <hello@jsavary.com>
Co-authored-by: Jon Cinque <jon.cinque@gmail.com>

program-test/src/lib.rs

@@ -9,6 +9,7 @@ use {
     log::*,
     solana_banks_client::start_client,
     solana_banks_server::banks_server::start_local_server,
+    solana_bpf_loader_program::serialization::serialize_parameters,
     solana_program_runtime::{
         compute_budget::ComputeBudget, ic_msg, invoke_context::ProcessInstructionWithContext,
         stable_log, timings::ExecuteTimings,
@@ -24,7 +25,7 @@ use {
         account::{Account, AccountSharedData, ReadableAccount},
         account_info::AccountInfo,
         clock::Slot,
-        entrypoint::{ProgramResult, SUCCESS},
+        entrypoint::{deserialize, ProgramResult, SUCCESS},
         feature_set::FEATURE_NAMES,
         fee_calculator::{FeeCalculator, FeeRateGovernor, DEFAULT_TARGET_LAMPORTS_PER_SIGNATURE},
         genesis_config::{ClusterType, GenesisConfig},
@@ -41,13 +42,12 @@ use {
     solana_vote_program::vote_state::{VoteState, VoteStateVersions},
     std::{
         cell::RefCell,
-        collections::HashSet,
+        collections::{HashMap, HashSet},
         convert::TryFrom,
         fs::File,
         io::{self, Read},
         mem::transmute,
         path::{Path, PathBuf},
-        rc::Rc,
         sync::{
             atomic::{AtomicBool, Ordering},
             Arc, RwLock,
@@ -112,58 +112,19 @@ pub fn builtin_process_instruction(
     );
 
     // Copy indices_in_instruction into a HashSet to ensure there are no duplicates
-    let deduplicated_indices: HashSet<usize> = instruction_account_indices.clone().collect();
+    let deduplicated_indices: HashSet<usize> = instruction_account_indices.collect();
 
-    // Create copies of the accounts
-    let mut account_copies = deduplicated_indices
-        .iter()
-        .map(|instruction_account_index| {
-            let borrowed_account = instruction_context
-                .try_borrow_instruction_account(transaction_context, *instruction_account_index)?;
-            Ok((
-                *borrowed_account.get_key(),
-                *borrowed_account.get_owner(),
-                borrowed_account.get_lamports(),
-                borrowed_account.get_data().to_vec(),
-            ))
-        })
-        .collect::<Result<Vec<_>, InstructionError>>()?;
-
-    // Create shared references to account_copies
-    let account_refs: Vec<_> = account_copies
-        .iter_mut()
-        .map(|(key, owner, lamports, data)| {
-            (
-                key,
-                owner,
-                Rc::new(RefCell::new(lamports)),
-                Rc::new(RefCell::new(data.as_mut())),
-            )
-        })
-        .collect();
+    // Serialize entrypoint parameters with BPF ABI
+    let (mut parameter_bytes, _account_lengths) = serialize_parameters(
+        invoke_context.transaction_context,
+        invoke_context
+            .transaction_context
+            .get_current_instruction_context()?,
+    )?;
 
-    // Create AccountInfos
-    let account_infos = instruction_account_indices
-        .map(|instruction_account_index| {
-            let account_copy_index = deduplicated_indices
-                .iter()
-                .position(|index| *index == instruction_account_index)
-                .unwrap();
-            let (key, owner, lamports, data) = &account_refs[account_copy_index];
-            let borrowed_account = instruction_context
-                .try_borrow_instruction_account(transaction_context, instruction_account_index)?;
-            Ok(AccountInfo {
-                key,
-                is_signer: borrowed_account.is_signer(),
-                is_writable: borrowed_account.is_writable(),
-                lamports: lamports.clone(),
-                data: data.clone(),
-                owner,
-                executable: borrowed_account.is_executable(),
-                rent_epoch: borrowed_account.get_rent_epoch(),
-            })
-        })
-        .collect::<Result<Vec<AccountInfo>, InstructionError>>()?;
+    // Deserialize data back into instruction params
+    let (program_id, account_infos, _input) =
+        unsafe { deserialize(&mut parameter_bytes.as_slice_mut()[0] as *mut u8) };
 
     // Execute the program
     process_instruction(program_id, &account_infos, instruction_data).map_err(|err| {
@@ -173,27 +134,35 @@ pub fn builtin_process_instruction(
     })?;
     stable_log::program_success(&log_collector, program_id);
 
+    // Lookup table for AccountInfo
+    let account_info_map: HashMap<_, _> = account_infos.into_iter().map(|a| (a.key, a)).collect();
+
+    // Re-fetch the instruction context. The previous reference may have been
+    // invalidated due to the `set_invoke_context` in a CPI.
+    let transaction_context = &invoke_context.transaction_context;
+    let instruction_context = transaction_context.get_current_instruction_context()?;
+
     // Commit AccountInfo changes back into KeyedAccounts
-    for (instruction_account_index, (_key, owner, lamports, data)) in deduplicated_indices
-        .into_iter()
-        .zip(account_copies.into_iter())
-    {
-        let mut borrowed_account = instruction_context
-            .try_borrow_instruction_account(transaction_context, instruction_account_index)?;
-        if borrowed_account.get_lamports() != lamports {
-            borrowed_account.set_lamports(lamports)?;
-        }
-        // The redundant check helps to avoid the expensive data comparison if we can
-        match borrowed_account
-            .can_data_be_resized(data.len())
-            .and_then(|_| borrowed_account.can_data_be_changed())
-        {
-            Ok(()) => borrowed_account.set_data(&data)?,
-            Err(err) if borrowed_account.get_data() != data => return Err(err),
-            _ => {}
-        }
-        if borrowed_account.get_owner() != &owner {
-            borrowed_account.set_owner(owner.as_ref())?;
+    for i in deduplicated_indices.into_iter() {
+        let mut borrowed_account =
+            instruction_context.try_borrow_instruction_account(transaction_context, i)?;
+        if borrowed_account.is_writable() {
+            if let Some(account_info) = account_info_map.get(borrowed_account.get_key()) {
+                if borrowed_account.get_lamports() != account_info.lamports() {
+                    borrowed_account.set_lamports(account_info.lamports())?;
+                }
+
+                if borrowed_account
+                    .can_data_be_resized(account_info.data_len())
+                    .is_ok()
+                    && borrowed_account.can_data_be_changed().is_ok()
+                {
+                    borrowed_account.set_data(&account_info.data.borrow())?;
+                }
+                if borrowed_account.get_owner() != account_info.owner {
+                    borrowed_account.set_owner(account_info.owner.as_ref())?;
+                }
+            }
         }
     }
 
@@ -276,6 +245,7 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs {
             .iter()
             .map(|seeds| Pubkey::create_program_address(seeds, caller).unwrap())
             .collect::<Vec<_>>();
+
         let (instruction_accounts, program_indices) = invoke_context
             .prepare_instruction(instruction, &signers)
             .unwrap();
@@ -363,8 +333,6 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs {
                 .unwrap();
             let account_info = &account_infos[account_info_index];
             **account_info.try_borrow_mut_lamports().unwrap() = borrowed_account.get_lamports();
-            let mut data = account_info.try_borrow_mut_data()?;
-            let new_data = borrowed_account.get_data();
             if account_info.owner != borrowed_account.get_owner() {
                 // TODO Figure out a better way to allow the System Program to set the account owner
                 #[allow(clippy::transmute_ptr_to_ptr)]
@@ -373,14 +341,17 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs {
                     unsafe { transmute::<&Pubkey, &mut Pubkey>(account_info.owner) };
                 *account_info_mut = *borrowed_account.get_owner();
             }
-            // TODO: Figure out how to allow the System Program to resize the account data
-            assert!(
-                data.len() == new_data.len(),
-                "Account data resizing not supported yet: {} -> {}. \
-                    Consider making this test conditional on `#[cfg(feature = \"test-bpf\")]`",
-                data.len(),
-                new_data.len()
-            );
+
+            let new_data = borrowed_account.get_data();
+            let new_len = new_data.len();
+
+            // Resize account_info data (grow-only)
+            if account_info.data_len() < new_len {
+                account_info.realloc(new_len, false)?;
+            }
+
+            // Clone the data
+            let mut data = account_info.try_borrow_mut_data()?;
             data.clone_from_slice(new_data);
         }
 

program-test/tests/cpi.rs

@@ -2,17 +2,22 @@ use {
     solana_program_test::{processor, ProgramTest},
     solana_sdk::{
         account_info::{next_account_info, AccountInfo},
-        entrypoint::ProgramResult,
+        entrypoint::{ProgramResult, MAX_PERMITTED_DATA_INCREASE},
         instruction::{AccountMeta, Instruction},
         msg,
         program::invoke,
         pubkey::Pubkey,
+        rent::Rent,
         signature::Signer,
+        signer::keypair::Keypair,
+        system_instruction, system_program,
+        sysvar::Sysvar,
         transaction::Transaction,
     },
 };
 
 // Process instruction to invoke into another program
+// We pass this specific number of accounts in order to test for a reported error.
 fn invoker_process_instruction(
     _program_id: &Pubkey,
     accounts: &[AccountInfo],
@@ -23,13 +28,50 @@ fn invoker_process_instruction(
     let account_info_iter = &mut accounts.iter();
     let invoked_program_info = next_account_info(account_info_iter)?;
     invoke(
-        &Instruction::new_with_bincode(*invoked_program_info.key, &[0], vec![]),
+        &Instruction::new_with_bincode(
+            *invoked_program_info.key,
+            &[0],
+            vec![AccountMeta::new_readonly(*invoked_program_info.key, false)],
+        ),
         &[invoked_program_info.clone()],
     )?;
     msg!("Processing invoker instruction after CPI");
     Ok(())
 }
 
+// Process instruction to invoke into another program with duplicates
+fn invoker_dupes_process_instruction(
+    _program_id: &Pubkey,
+    accounts: &[AccountInfo],
+    _input: &[u8],
+) -> ProgramResult {
+    // if we can call `msg!` successfully, then InvokeContext exists as required
+    msg!("Processing invoker instruction before CPI");
+    let account_info_iter = &mut accounts.iter();
+    let invoked_program_info = next_account_info(account_info_iter)?;
+    invoke(
+        &Instruction::new_with_bincode(
+            *invoked_program_info.key,
+            &[0],
+            vec![
+                AccountMeta::new_readonly(*invoked_program_info.key, false),
+                AccountMeta::new_readonly(*invoked_program_info.key, false),
+                AccountMeta::new_readonly(*invoked_program_info.key, false),
+                AccountMeta::new_readonly(*invoked_program_info.key, false),
+            ],
+        ),
+        &[
+            invoked_program_info.clone(),
+            invoked_program_info.clone(),
+            invoked_program_info.clone(),
+            invoked_program_info.clone(),
+            invoked_program_info.clone(),
+        ],
+    )?;
+    msg!("Processing invoker instruction after CPI");
+    Ok(())
+}
+
 // Process instruction to be invoked by another program
 #[allow(clippy::unnecessary_wraps)]
 fn invoked_process_instruction(
@@ -42,6 +84,37 @@ fn invoked_process_instruction(
     Ok(())
 }
 
+// Process instruction to invoke into system program to create an account
+fn invoke_create_account(
+    program_id: &Pubkey,
+    accounts: &[AccountInfo],
+    _input: &[u8],
+) -> ProgramResult {
+    msg!("Processing instruction before system program CPI instruction");
+    let account_info_iter = &mut accounts.iter();
+    let payer_info = next_account_info(account_info_iter)?;
+    let create_account_info = next_account_info(account_info_iter)?;
+    let system_program_info = next_account_info(account_info_iter)?;
+    let rent = Rent::get()?;
+    let minimum_balance = rent.minimum_balance(MAX_PERMITTED_DATA_INCREASE);
+    invoke(
+        &system_instruction::create_account(
+            payer_info.key,
+            create_account_info.key,
+            minimum_balance,
+            MAX_PERMITTED_DATA_INCREASE as u64,
+            program_id,
+        ),
+        &[
+            payer_info.clone(),
+            create_account_info.clone(),
+            system_program_info.clone(),
+        ],
+    )?;
+    msg!("Processing instruction after system program CPI");
+    Ok(())
+}
+
 #[tokio::test]
 async fn cpi() {
     let invoker_program_id = Pubkey::new_unique();
@@ -77,3 +150,79 @@ async fn cpi() {
         .await
         .unwrap();
 }
+
+#[tokio::test]
+async fn cpi_dupes() {
+    let invoker_program_id = Pubkey::new_unique();
+    let mut program_test = ProgramTest::new(
+        "invoker",
+        invoker_program_id,
+        processor!(invoker_dupes_process_instruction),
+    );
+    let invoked_program_id = Pubkey::new_unique();
+    program_test.add_program(
+        "invoked",
+        invoked_program_id,
+        processor!(invoked_process_instruction),
+    );
+
+    let mut context = program_test.start_with_context().await;
+    let instructions = vec![Instruction::new_with_bincode(
+        invoker_program_id,
+        &[0],
+        vec![
+            AccountMeta::new_readonly(invoked_program_id, false),
+            AccountMeta::new_readonly(invoked_program_id, false),
+            AccountMeta::new_readonly(invoked_program_id, false),
+            AccountMeta::new_readonly(invoked_program_id, false),
+        ],
+    )];
+
+    let transaction = Transaction::new_signed_with_payer(
+        &instructions,
+        Some(&context.payer.pubkey()),
+        &[&context.payer],
+        context.last_blockhash,
+    );
+
+    context
+        .banks_client
+        .process_transaction(transaction)
+        .await
+        .unwrap();
+}
+
+#[tokio::test]
+async fn cpi_create_account() {
+    let create_account_program_id = Pubkey::new_unique();
+    let program_test = ProgramTest::new(
+        "create_account",
+        create_account_program_id,
+        processor!(invoke_create_account),
+    );
+
+    let create_account_keypair = Keypair::new();
+    let mut context = program_test.start_with_context().await;
+    let instructions = vec![Instruction::new_with_bincode(
+        create_account_program_id,
+        &[0],
+        vec![
+            AccountMeta::new(context.payer.pubkey(), true),
+            AccountMeta::new(create_account_keypair.pubkey(), true),
+            AccountMeta::new_readonly(system_program::id(), false),
+        ],
+    )];
+
+    let transaction = Transaction::new_signed_with_payer(
+        &instructions,
+        Some(&context.payer.pubkey()),
+        &[&context.payer, &create_account_keypair],
+        context.last_blockhash,
+    );
+
+    context
+        .banks_client
+        .process_transaction(transaction)
+        .await
+        .unwrap();
+}