CPI without Account Refs (#20034)

* Removes search for accounts and unsafe lifetime transmute in InvokeContext::push().

* Replaces accounts by account_indices in verify_and_update() and process_cross_program_instruction().

Co-authored-by: Justin Starry <justin.m.starry@gmail.com>

program-runtime/src/instruction_processor.rs

@@ -5,10 +5,10 @@ use solana_sdk::{
     account_utils::StateMut,
     bpf_loader_upgradeable::{self, UpgradeableLoaderState},
     feature_set::{demote_program_write_locks, fix_write_privs},
-    ic_logger_msg, ic_msg,
+    ic_msg,
-    instruction::{CompiledInstruction, Instruction, InstructionError},
+    instruction::{Instruction, InstructionError},
     message::Message,
-    process_instruction::{Executor, InvokeContext, Logger, ProcessInstructionWithContext},
+    process_instruction::{Executor, InvokeContext, ProcessInstructionWithContext},
     pubkey::Pubkey,
     rent::Rent,
     system_program,
@@ -516,20 +516,16 @@ impl InstructionProcessor {
                 caller_write_privileges.push(caller_keyed_accounts[*index].is_writable());
             }
         };
-        let accounts = message
+        let mut account_indices = Vec::with_capacity(message.account_keys.len());
-            .account_keys
+        let mut accounts = Vec::with_capacity(message.account_keys.len());
-            .iter()
+        for account_key in message.account_keys.iter() {
-            .map(|account_key| {
+            let (account_index, account) = invoke_context
-                invoke_context
                 .get_account(account_key)
-                    .ok_or(InstructionError::MissingAccount)
+                .ok_or(InstructionError::MissingAccount)?;
-                    .map(|(_account_index, account)| (*account_key, account))
+            let account_length = account.borrow().data().len();
-            })
+            account_indices.push(account_index);
-            .collect::<Result<Vec<_>, InstructionError>>()?;
+            accounts.push((account, account_length));
-        let account_sizes = accounts
+        }
-            .iter()
-            .map(|(_key, account)| account.borrow().data().len())
-            .collect::<Vec<_>>();
 
         // Record the instruction
         invoke_context.record_instruction(&instruction);
@@ -538,13 +534,13 @@ impl InstructionProcessor {
         InstructionProcessor::process_cross_program_instruction(
             &message,
             &program_indices,
-            &accounts,
+            &account_indices,
             &caller_write_privileges,
             *invoke_context,
         )?;
 
         // Verify the called program has not misbehaved
-        for ((_key, account), prev_size) in accounts.iter().zip(account_sizes.iter()) {
+        for (account, prev_size) in accounts.iter() {
             if *prev_size != account.borrow().data().len() && *prev_size != 0 {
                 // Only support for `CreateAccount` at this time.
                 // Need a way to limit total realloc size across multiple CPI calls
@@ -564,7 +560,7 @@ impl InstructionProcessor {
     pub fn process_cross_program_instruction(
         message: &Message,
         program_indices: &[usize],
-        accounts: &[(Pubkey, Rc<RefCell<AccountSharedData>>)],
+        account_indices: &[usize],
         caller_write_privileges: &[bool],
         invoke_context: &mut dyn InvokeContext,
     ) -> Result<(), InstructionError> {
@@ -577,13 +573,19 @@ impl InstructionProcessor {
         let program_id = instruction.program_id(&message.account_keys);
 
         // Verify the calling program hasn't misbehaved
-        invoke_context.verify_and_update(instruction, accounts, caller_write_privileges)?;
+        invoke_context.verify_and_update(instruction, account_indices, caller_write_privileges)?;
 
         // clear the return data
         invoke_context.set_return_data(None);
 
         // Invoke callee
-        invoke_context.push(program_id, message, instruction, program_indices, accounts)?;
+        invoke_context.push(
+            program_id,
+            message,
+            instruction,
+            program_indices,
+            account_indices,
+        )?;
 
         let mut instruction_processor = InstructionProcessor::default();
         for (program_id, process_instruction) in invoke_context.get_programs().iter() {
@@ -602,67 +604,14 @@ impl InstructionProcessor {
             let write_privileges: Vec<bool> = (0..message.account_keys.len())
                 .map(|i| message.is_writable(i, demote_program_write_locks))
                 .collect();
-            result = invoke_context.verify_and_update(instruction, accounts, &write_privileges);
+            result =
+                invoke_context.verify_and_update(instruction, account_indices, &write_privileges);
         }
 
         // Restore previous state
         invoke_context.pop();
         result
     }
-
-    /// Verify the results of a cross-program instruction
-    #[allow(clippy::too_many_arguments)]
-    pub fn verify_and_update(
-        instruction: &CompiledInstruction,
-        pre_accounts: &mut [PreAccount],
-        accounts: &[(Pubkey, Rc<RefCell<AccountSharedData>>)],
-        program_id: &Pubkey,
-        rent: &Rent,
-        write_privileges: &[bool],
-        timings: &mut ExecuteDetailsTimings,
-        logger: Rc<RefCell<dyn Logger>>,
-    ) -> Result<(), InstructionError> {
-        // Verify the per-account instruction results
-        let (mut pre_sum, mut post_sum) = (0_u128, 0_u128);
-        let mut work = |_unique_index: usize, account_index: usize| {
-            if account_index < write_privileges.len() && account_index < accounts.len() {
-                let (key, account) = &accounts[account_index];
-                let is_writable = write_privileges[account_index];
-                // Find the matching PreAccount
-                for pre_account in pre_accounts.iter_mut() {
-                    if key == pre_account.key() {
-                        {
-                            // Verify account has no outstanding references
-                            let _ = account
-                                .try_borrow_mut()
-                                .map_err(|_| InstructionError::AccountBorrowOutstanding)?;
-                        }
-                        let account = account.borrow();
-                        pre_account
-                            .verify(program_id, is_writable, rent, &account, timings, false)
-                            .map_err(|err| {
-                                ic_logger_msg!(logger, "failed to verify account {}: {}", key, err);
-                                err
-                            })?;
-                        pre_sum += u128::from(pre_account.lamports());
-                        post_sum += u128::from(account.lamports());
-                        if is_writable && !pre_account.executable() {
-                            pre_account.update(&account);
-                        }
-                        return Ok(());
-                    }
-                }
-            }
-            Err(InstructionError::MissingAccount)
-        };
-        instruction.visit_each_account(&mut work)?;
-
-        // Verify that the total sum of all the lamports did not change
-        if pre_sum != post_sum {
-            return Err(InstructionError::UnbalancedInstruction);
-        }
-        Ok(())
-    }
 }
 
 #[cfg(test)]

program-test/src/lib.rs

@@ -275,30 +275,34 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs {
         stable_log::program_invoke(&logger, &program_id, invoke_context.invoke_depth());
 
         // Convert AccountInfos into Accounts
-        fn ai_to_a(ai: &AccountInfo) -> AccountSharedData {
+        let mut account_indices = Vec::with_capacity(message.account_keys.len());
-            AccountSharedData::from(Account {
+        let mut accounts = Vec::with_capacity(message.account_keys.len());
-                lamports: ai.lamports(),
+        for (i, account_key) in message.account_keys.iter().enumerate() {
-                data: ai.try_borrow_data().unwrap().to_vec(),
+            let ((account_index, account), account_info) = invoke_context
-                owner: *ai.owner,
+                .get_account(account_key)
-                executable: ai.executable,
+                .zip(
-                rent_epoch: ai.rent_epoch,
+                    account_infos
-            })
+                        .iter()
+                        .find(|account_info| account_info.unsigned_key() == account_key),
+                )
+                .ok_or(InstructionError::MissingAccount)
+                .unwrap();
+            {
+                let mut account = account.borrow_mut();
+                account.copy_into_owner_from_slice(account_info.owner.as_ref());
+                account.set_data_from_slice(&account_info.try_borrow_data().unwrap());
+                account.set_lamports(account_info.lamports());
+                account.set_executable(account_info.executable);
+                account.set_rent_epoch(account_info.rent_epoch);
             }
-        let mut accounts = vec![];
+            let account_info = if message.is_writable(i, demote_program_write_locks) {
-        'outer: for key in &message.account_keys {
+                Some(account_info)
-            for account_info in account_infos {
+            } else {
-                if account_info.unsigned_key() == key {
+                None
-                    accounts.push((*key, Rc::new(RefCell::new(ai_to_a(account_info)))));
+            };
-                    continue 'outer;
+            account_indices.push(account_index);
+            accounts.push((account, account_info));
         }
-            }
-            panic!("Account {} wasn't found in account_infos", key);
-        }
-        assert_eq!(
-            accounts.len(),
-            message.account_keys.len(),
-            "Missing or not enough accounts passed to invoke"
-        );
         let (program_account_index, _program_account) =
             invoke_context.get_account(&program_id).unwrap();
         let program_indices = vec![program_account_index];
@@ -330,21 +334,16 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs {
         InstructionProcessor::process_cross_program_instruction(
             &message,
             &program_indices,
-            &accounts,
+            &account_indices,
             &caller_privileges,
             invoke_context,
         )
         .map_err(|err| ProgramError::try_from(err).unwrap_or_else(|err| panic!("{}", err)))?;
 
         // Copy writeable account modifications back into the caller's AccountInfos
-        for (i, (pubkey, account)) in accounts.iter().enumerate().take(message.account_keys.len()) {
+        for (account, account_info) in accounts.iter() {
-            if !message.is_writable(i, demote_program_write_locks) {
+            if let Some(account_info) = account_info {
-                continue;
-            }
-            for account_info in account_infos {
-                if account_info.unsigned_key() == pubkey {
                 **account_info.try_borrow_mut_lamports().unwrap() = account.borrow().lamports();
-
                 let mut data = account_info.try_borrow_mut_data()?;
                 let account_borrow = account.borrow();
                 let new_data = account_borrow.data();
@@ -368,7 +367,6 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs {
                 data.clone_from_slice(new_data);
             }
         }
-        }
 
         stable_log::program_success(&logger, &program_id);
         Ok(())

programs/bpf_loader/src/syscalls.rs

@@ -1513,8 +1513,11 @@ struct AccountReferences<'a> {
     rent_epoch: u64,
 }
 type TranslatedAccounts<'a> = (
-    Vec<(Pubkey, Rc<RefCell<AccountSharedData>>)>,
+    Vec<usize>,
-    Vec<Option<AccountReferences<'a>>>,
+    Vec<(
+        Rc<RefCell<AccountSharedData>>,
+        Option<AccountReferences<'a>>,
+    )>,
 );
 
 /// Implemented by language specific data structure translators
@@ -2052,16 +2055,16 @@ where
 {
     let demote_program_write_locks =
         invoke_context.is_feature_active(&demote_program_write_locks::id());
+    let mut account_indices = Vec::with_capacity(message.account_keys.len());
     let mut accounts = Vec::with_capacity(message.account_keys.len());
-    let mut refs = Vec::with_capacity(message.account_keys.len());
     for (i, account_key) in message.account_keys.iter().enumerate() {
-        if let Some((_account_index, account)) = invoke_context.get_account(account_key) {
+        if let Some((account_index, account)) = invoke_context.get_account(account_key) {
             if i == message.instructions[0].program_id_index as usize
                 || account.borrow().executable()
             {
                 // Use the known account
-                accounts.push((*account_key, account));
+                account_indices.push(account_index);
-                refs.push(None);
+                accounts.push((account, None));
                 continue;
             } else if let Some(account_ref_index) =
                 account_info_keys.iter().position(|key| *key == account_key)
@@ -2075,12 +2078,13 @@ where
                     account.set_executable(account_ref.executable);
                     account.set_rent_epoch(account_ref.rent_epoch);
                 }
-                accounts.push((*account_key, account));
+                let account_ref = if message.is_writable(i, demote_program_write_locks) {
-                refs.push(if message.is_writable(i, demote_program_write_locks) {
                     Some(account_ref)
                 } else {
                     None
-                });
+                };
+                account_indices.push(account_index);
+                accounts.push((account, account_ref));
                 continue;
             }
         }
@@ -2092,7 +2096,7 @@ where
         return Err(SyscallError::InstructionError(InstructionError::MissingAccount).into());
     }
 
-    Ok((accounts, refs))
+    Ok((account_indices, accounts))
 }
 
 fn check_instruction_size(
@@ -2176,7 +2180,7 @@ fn call<'a>(
         &instruction.data,
         invoke_context.is_feature_active(&close_upgradeable_program_accounts::id()),
     )?;
-    let (accounts, account_refs) = syscall.translate_accounts(
+    let (account_indices, mut accounts) = syscall.translate_accounts(
         &message,
         account_infos_addr,
         account_infos_len,
@@ -2191,16 +2195,16 @@ fn call<'a>(
     InstructionProcessor::process_cross_program_instruction(
         &message,
         &program_indices,
-        &accounts,
+        &account_indices,
         &caller_write_privileges,
         *invoke_context,
     )
     .map_err(SyscallError::InstructionError)?;
 
     // Copy results back to caller
-    for ((_key, account), account_ref) in accounts.iter().zip(account_refs) {
+    for (account, account_ref) in accounts.iter_mut() {
         let account = account.borrow();
-        if let Some(mut account_ref) = account_ref {
+        if let Some(account_ref) = account_ref {
             *account_ref.lamports = account.lamports();
             *account_ref.owner = *account.owner();
             if account_ref.data.len() != account.data().len() {

runtime/src/message_processor.rs

@@ -119,7 +119,14 @@ impl<'a> ThisInvokeContext<'a> {
             fee_calculator,
             return_data: None,
         };
-        invoke_context.push(program_id, message, instruction, program_indices, accounts)?;
+        let account_indices = (0..accounts.len()).collect::<Vec<usize>>();
+        invoke_context.push(
+            program_id,
+            message,
+            instruction,
+            program_indices,
+            &account_indices,
+        )?;
         Ok(invoke_context)
     }
 }
@@ -130,7 +137,7 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> {
         message: &Message,
         instruction: &CompiledInstruction,
         program_indices: &[usize],
-        instruction_accounts: &[(Pubkey, Rc<RefCell<AccountSharedData>>)],
+        account_indices: &[usize],
     ) -> Result<(), InstructionError> {
         if self.invoke_stack.len() > self.compute_budget.max_invoke_depth {
             return Err(InstructionError::CallDepth);
@@ -161,44 +168,17 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> {
                     &self.accounts[*account_index].1 as &RefCell<AccountSharedData>,
                 )
             })
-            .chain(instruction.accounts.iter().map(|index| {
+            .chain(instruction.accounts.iter().map(|index_in_instruction| {
-                let index = *index as usize;
+                let index_in_instruction = *index_in_instruction as usize;
+                let account_index = account_indices[index_in_instruction];
                 (
-                    message.is_signer(index),
+                    message.is_signer(index_in_instruction),
-                    message.is_writable(index, demote_program_write_locks),
+                    message.is_writable(index_in_instruction, demote_program_write_locks),
-                    &instruction_accounts[index].0,
+                    &self.accounts[account_index].0,
-                    &instruction_accounts[index].1 as &RefCell<AccountSharedData>,
+                    &self.accounts[account_index].1 as &RefCell<AccountSharedData>,
                 )
             }))
             .collect::<Vec<_>>();
-
-        // Alias the keys and account references in the provided keyed_accounts
-        // with the ones already existing in self, so that the lifetime 'a matches.
-        fn transmute_lifetime<'a, 'b, T: Sized>(value: &'a T) -> &'b T {
-            unsafe { std::mem::transmute(value) }
-        }
-        let keyed_accounts = keyed_accounts
-            .iter()
-            .map(|(is_signer, is_writable, search_key, account)| {
-                self.accounts
-                    .iter()
-                    .position(|(key, _account)| key == *search_key)
-                    .map(|index| {
-                        // TODO
-                        // Currently we are constructing new accounts on the stack
-                        // before calling MessageProcessor::process_cross_program_instruction
-                        // Ideally we would recycle the existing accounts here.
-                        (
-                            *is_signer,
-                            *is_writable,
-                            &self.accounts[index].0,
-                            // &self.accounts[index] as &RefCell<AccountSharedData>
-                            transmute_lifetime(*account),
-                        )
-                    })
-            })
-            .collect::<Option<Vec<_>>>()
-            .ok_or(InstructionError::InvalidArgument)?;
         self.invoke_stack.push(InvokeContextStackFrame::new(
             *key,
             create_keyed_accounts_unified(keyed_accounts.as_slice()),
@@ -214,24 +194,63 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> {
     fn verify_and_update(
         &mut self,
         instruction: &CompiledInstruction,
-        accounts: &[(Pubkey, Rc<RefCell<AccountSharedData>>)],
+        account_indices: &[usize],
         write_privileges: &[bool],
     ) -> Result<(), InstructionError> {
         let stack_frame = self
             .invoke_stack
             .last()
             .ok_or(InstructionError::CallDepth)?;
+        let program_id = &stack_frame.key;
+        let rent = &self.rent;
         let logger = self.get_logger();
-        InstructionProcessor::verify_and_update(
+        let accounts = &self.accounts;
-            instruction,
+        let pre_accounts = &mut self.pre_accounts;
-            &mut self.pre_accounts,
+        let timings = &mut self.timings;
-            accounts,
+
-            &stack_frame.key,
+        // Verify the per-account instruction results
-            &self.rent,
+        let (mut pre_sum, mut post_sum) = (0_u128, 0_u128);
-            write_privileges,
+        let mut work = |_unique_index: usize, index_in_instruction: usize| {
-            &mut self.timings,
+            if index_in_instruction < write_privileges.len()
-            logger,
+                && index_in_instruction < account_indices.len()
-        )
+            {
+                let account_index = account_indices[index_in_instruction];
+                let (key, account) = &accounts[account_index];
+                let is_writable = write_privileges[index_in_instruction];
+                // Find the matching PreAccount
+                for pre_account in pre_accounts.iter_mut() {
+                    if key == pre_account.key() {
+                        {
+                            // Verify account has no outstanding references
+                            let _ = account
+                                .try_borrow_mut()
+                                .map_err(|_| InstructionError::AccountBorrowOutstanding)?;
+                        }
+                        let account = account.borrow();
+                        pre_account
+                            .verify(program_id, is_writable, rent, &account, timings, false)
+                            .map_err(|err| {
+                                ic_logger_msg!(logger, "failed to verify account {}: {}", key, err);
+                                err
+                            })?;
+                        pre_sum += u128::from(pre_account.lamports());
+                        post_sum += u128::from(account.lamports());
+                        if is_writable && !pre_account.executable() {
+                            pre_account.update(&account);
+                        }
+                        return Ok(());
+                    }
+                }
+            }
+            Err(InstructionError::MissingAccount)
+        };
+        instruction.visit_each_account(&mut work)?;
+
+        // Verify that the total sum of all the lamports did not change
+        if pre_sum != post_sum {
+            return Err(InstructionError::UnbalancedInstruction);
+        }
+        Ok(())
     }
     fn get_caller(&self) -> Result<&Pubkey, InstructionError> {
         self.invoke_stack
@@ -671,6 +690,7 @@ mod tests {
             ));
             metas.push(AccountMeta::new(*program_id, false));
         }
+        let account_indices = (0..accounts.len()).collect::<Vec<usize>>();
 
         let message = Message::new(
             &[Instruction::new_with_bytes(invoke_stack[0], &[0], metas)],
@@ -709,7 +729,7 @@ mod tests {
                     &message,
                     &message.instructions[0],
                     &[],
-                    &accounts,
+                    &account_indices,
                 )
             {
                 break;
@@ -734,24 +754,19 @@ mod tests {
                 )],
                 None,
             );
+            let write_privileges: Vec<bool> = (0..message.account_keys.len())
+                .map(|i| message.is_writable(i, /*demote_program_write_locks=*/ true))
+                .collect();
 
             // modify account owned by the program
             accounts[owned_index].1.borrow_mut().data_as_mut_slice()[0] =
                 (MAX_DEPTH + owned_index) as u8;
-            let mut these_accounts = accounts[not_owned_index..owned_index + 1].to_vec();
-            these_accounts.push((
-                message.account_keys[2],
-                Rc::new(RefCell::new(AccountSharedData::new(
-                    1,
-                    1,
-                    &solana_sdk::pubkey::Pubkey::default(),
-                ))),
-            ));
-            let write_privileges: Vec<bool> = (0..message.account_keys.len())
-                .map(|i| message.is_writable(i, /*demote_program_write_locks=*/ true))
-                .collect();
             invoke_context
-                .verify_and_update(&message.instructions[0], &these_accounts, &write_privileges)
+                .verify_and_update(
+                    &message.instructions[0],
+                    &account_indices[not_owned_index..owned_index + 1],
+                    &write_privileges,
+                )
                 .unwrap();
             assert_eq!(
                 invoke_context.pre_accounts[owned_index].data()[0],
@@ -765,7 +780,7 @@ mod tests {
             assert_eq!(
                 invoke_context.verify_and_update(
                     &message.instructions[0],
-                    &accounts[not_owned_index..owned_index + 1],
+                    &account_indices[not_owned_index..owned_index + 1],
                     &write_privileges,
                 ),
                 Err(InstructionError::ExternalAccountDataModified)
@@ -1217,6 +1232,7 @@ mod tests {
             ),
             (callee_program_id, Rc::new(RefCell::new(program_account))),
         ];
+        let account_indices = [0, 1, 2];
         let program_indices = vec![3];
 
         let programs: Vec<(_, ProcessInstructionWithContext)> =
@@ -1274,7 +1290,7 @@ mod tests {
             InstructionProcessor::process_cross_program_instruction(
                 &message,
                 &program_indices,
-                &accounts,
+                &account_indices,
                 &caller_write_privileges,
                 &mut invoke_context,
             ),
@@ -1288,7 +1304,7 @@ mod tests {
             InstructionProcessor::process_cross_program_instruction(
                 &message,
                 &program_indices,
-                &accounts,
+                &account_indices,
                 &caller_write_privileges,
                 &mut invoke_context,
             ),
@@ -1348,7 +1364,7 @@ mod tests {
                 InstructionProcessor::process_cross_program_instruction(
                     &message,
                     &program_indices,
-                    &accounts,
+                    &account_indices,
                     &caller_write_privileges,
                     &mut invoke_context,
                 ),

sdk/src/process_instruction.rs

@@ -51,19 +51,15 @@ impl<'a> InvokeContextStackFrame<'a> {
 /// Invocation context passed to loaders
 pub trait InvokeContext {
     /// Push a stack frame onto the invocation stack
-    ///
-    /// Used in MessageProcessor::process_cross_program_instruction
     fn push(
         &mut self,
         key: &Pubkey,
         message: &Message,
         instruction: &CompiledInstruction,
         program_indices: &[usize],
-        instruction_accounts: &[(Pubkey, Rc<RefCell<AccountSharedData>>)],
+        account_indices: &[usize],
     ) -> Result<(), InstructionError>;
     /// Pop a stack frame from the invocation stack
-    ///
-    /// Used in MessageProcessor::process_cross_program_instruction
     fn pop(&mut self);
     /// Current depth of the invocation stake
     fn invoke_depth(&self) -> usize;
@@ -71,7 +67,7 @@ pub trait InvokeContext {
     fn verify_and_update(
         &mut self,
         instruction: &CompiledInstruction,
-        accounts: &[(Pubkey, Rc<RefCell<AccountSharedData>>)],
+        account_indices: &[usize],
         write_privileges: &[bool],
     ) -> Result<(), InstructionError>;
     /// Get the program ID of the currently executing program
@@ -478,7 +474,7 @@ impl<'a> InvokeContext for MockInvokeContext<'a> {
         _message: &Message,
         _instruction: &CompiledInstruction,
         _program_indices: &[usize],
-        _instruction_accounts: &[(Pubkey, Rc<RefCell<AccountSharedData>>)],
+        _account_indices: &[usize],
     ) -> Result<(), InstructionError> {
         self.invoke_stack.push(InvokeContextStackFrame::new(
             *_key,
@@ -495,7 +491,7 @@ impl<'a> InvokeContext for MockInvokeContext<'a> {
     fn verify_and_update(
         &mut self,
         _instruction: &CompiledInstruction,
-        _accounts: &[(Pubkey, Rc<RefCell<AccountSharedData>>)],
+        _account_indices: &[usize],
         _write_pivileges: &[bool],
     ) -> Result<(), InstructionError> {
         Ok(())