Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

CPI without Account Refs #20034

Merged
Lichtso merged 3 commits into from
Sep 21, 2021
Merged

CPI without Account Refs #20034

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
31e5c1d
Removes search for accounts and unsafe lifetime transmute in InvokeCo…
Lichtso Sep 20, 2021
dbc9918
Replaces accounts by account_indices in verify_and_update() and proce…
Lichtso Sep 20, 2021
7d2d2b7
Fix bounds check of account_indices
Lichtso Sep 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 26 additions & 77 deletions program-runtime/src/instruction_processor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
instruction::{CompiledInstruction, Instruction, InstructionError},
ic_msg,
instruction::{Instruction, InstructionError},
message::Message,
process_instruction::{Executor, InvokeContext, Logger, ProcessInstructionWithContext},
process_instruction::{Executor, InvokeContext, ProcessInstructionWithContext},
pubkey::Pubkey,
rent::Rent,
system_program,
Expand Down Expand Up @@ -516,20 +516,16 @@ impl InstructionProcessor {
caller_write_privileges.push(caller_keyed_accounts[*index].is_writable());
}
};
let accounts = message
.account_keys
.iter()
.map(|account_key| {
invoke_context
.get_account(account_key)
.ok_or(InstructionError::MissingAccount)
.map(|(_account_index, account)| (*account_key, account))
})
.collect::<Result<Vec<_>, InstructionError>>()?;
let account_sizes = accounts
.iter()
.map(|(_key, account)| account.borrow().data().len())
.collect::<Vec<_>>();
let mut account_indices = Vec::with_capacity(message.account_keys.len());
let mut accounts = Vec::with_capacity(message.account_keys.len());
for account_key in message.account_keys.iter() {
let (account_index, account) = invoke_context
.get_account(account_key)
.ok_or(InstructionError::MissingAccount)?;
let account_length = account.borrow().data().len();
account_indices.push(account_index);
accounts.push((account, account_length));
}

// Record the instruction
invoke_context.record_instruction(&instruction);
Expand All @@ -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
Expand All @@ -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> {
Expand All @@ -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() {
Expand All @@ -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)]
Expand Down
102 changes: 50 additions & 52 deletions program-test/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,30 +267,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 {
AccountSharedData::from(Account {
lamports: ai.lamports(),
data: ai.try_borrow_data().unwrap().to_vec(),
owner: *ai.owner,
executable: ai.executable,
rent_epoch: ai.rent_epoch,
})
}
let mut accounts = vec![];
'outer: for key in &message.account_keys {
for account_info in account_infos {
if account_info.unsigned_key() == key {
accounts.push((*key, Rc::new(RefCell::new(ai_to_a(account_info)))));
continue 'outer;
}
let mut account_indices = Vec::with_capacity(message.account_keys.len());
let mut accounts = Vec::with_capacity(message.account_keys.len());
for (i, account_key) in message.account_keys.iter().enumerate() {
let ((account_index, account), account_info) = invoke_context
.get_account(account_key)
.zip(
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);
}
panic!("Account {} wasn't found in account_infos", key);
let account_info = if message.is_writable(i, demote_program_write_locks) {
Some(account_info)
} else {
None
};
account_indices.push(account_index);
accounts.push((account, account_info));
}
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];
Expand Down Expand Up @@ -322,43 +326,37 @@ 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()) {
if !message.is_writable(i, demote_program_write_locks) {
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();
if account_info.owner != account.borrow().owner() {
// TODO Figure out a better way to allow the System Program to set the account owner
#[allow(clippy::transmute_ptr_to_ptr)]
#[allow(mutable_transmutes)]
let account_info_mut =
unsafe { transmute::<&Pubkey, &mut Pubkey>(account_info.owner) };
*account_info_mut = *account.borrow().owner();
}
if data.len() != new_data.len() {
// TODO: Figure out how to allow the System Program to resize the account data
panic!(
"Account data resizing not supported yet: {} -> {}. \
Consider making this test conditional on `#[cfg(feature = \"test-bpf\")]`",
data.len(),
new_data.len()
);
}
data.clone_from_slice(new_data);
for (account, account_info) in accounts.iter() {
if let Some(account_info) = account_info {
**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();
if account_info.owner != account.borrow().owner() {
// TODO Figure out a better way to allow the System Program to set the account owner
#[allow(clippy::transmute_ptr_to_ptr)]
#[allow(mutable_transmutes)]
let account_info_mut =
unsafe { transmute::<&Pubkey, &mut Pubkey>(account_info.owner) };
*account_info_mut = *account.borrow().owner();
}
if data.len() != new_data.len() {
// TODO: Figure out how to allow the System Program to resize the account data
panic!(
"Account data resizing not supported yet: {} -> {}. \
Consider making this test conditional on `#[cfg(feature = \"test-bpf\")]`",
data.len(),
new_data.len()
);
}
data.clone_from_slice(new_data);
}
}

Expand Down
32 changes: 18 additions & 14 deletions programs/bpf_loader/src/syscalls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1513,8 +1513,11 @@ struct AccountReferences<'a> {
rent_epoch: u64,
}
type TranslatedAccounts<'a> = (
Vec<(Pubkey, Rc<RefCell<AccountSharedData>>)>,
Vec<Option<AccountReferences<'a>>>,
Vec<usize>,
Vec<(
Rc<RefCell<AccountSharedData>>,
Option<AccountReferences<'a>>,
)>,
);

/// Implemented by language specific data structure translators
Expand Down Expand Up @@ -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));
refs.push(None);
account_indices.push(account_index);
accounts.push((account, None));
continue;
} else if let Some(account_ref_index) =
account_info_keys.iter().position(|key| *key == account_key)
Expand All @@ -2075,12 +2078,13 @@ where
account.set_executable(account_ref.executable);
account.set_rent_epoch(account_ref.rent_epoch);
}
accounts.push((*account_key, account));
refs.push(if message.is_writable(i, demote_program_write_locks) {
let account_ref = 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;
}
}
Expand All @@ -2092,7 +2096,7 @@ where
return Err(SyscallError::InstructionError(InstructionError::MissingAccount).into());
}

Ok((accounts, refs))
Ok((account_indices, accounts))
}

fn check_instruction_size(
Expand Down Expand Up @@ -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,
Expand All @@ -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() {
Expand Down
Loading