Skip to content

Commit

Permalink
Cleanup - Feature gate of `disable_cpi_setting_executable_and_rent_ep…
Browse files Browse the repository at this point in the history 
…och` (#34086)

Cleans up feature gate of disable_cpi_setting_executable_and_rent_epoch.
  • Loading branch information
@Lichtso
Lichtso authored Nov 16, 2023
1 parent 6fc6a49 commit f349d71
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 155 deletions.
199 changes: 47 additions & 152 deletions programs/bpf_loader/src/syscalls/cpi.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,16 +95,13 @@ struct CallerAccount<'a, 'b> {
// the pointer field and ref_to_len_in_vm points to the length field.
vm_data_addr: u64,
ref_to_len_in_vm: VmValue<'b, 'a, u64>,
executable: bool,
rent_epoch: u64,
}

impl<'a, 'b> CallerAccount<'a, 'b> {
// Create a CallerAccount given an AccountInfo.
fn from_account_info(
invoke_context: &InvokeContext,
memory_mapping: &'b MemoryMapping<'a>,
is_disable_cpi_setting_executable_and_rent_epoch_active: bool,
_vm_addr: u64,
account_info: &AccountInfo,
account_metadata: &SerializedAccountMetadata,
Expand Down Expand Up @@ -235,24 +232,13 @@ impl<'a, 'b> CallerAccount<'a, 'b> {
serialized_data,
vm_data_addr,
ref_to_len_in_vm,
executable: if is_disable_cpi_setting_executable_and_rent_epoch_active {
false
} else {
account_info.executable
},
rent_epoch: if is_disable_cpi_setting_executable_and_rent_epoch_active {
0
} else {
account_info.rent_epoch
},
})
}

// Create a CallerAccount given a SolAccountInfo.
fn from_sol_account_info(
invoke_context: &InvokeContext,
memory_mapping: &'b MemoryMapping<'a>,
is_disable_cpi_setting_executable_and_rent_epoch_active: bool,
vm_addr: u64,
account_info: &SolAccountInfo,
account_metadata: &SerializedAccountMetadata,
Expand Down Expand Up @@ -361,16 +347,6 @@ impl<'a, 'b> CallerAccount<'a, 'b> {
serialized_data,
vm_data_addr: account_info.data_addr,
ref_to_len_in_vm,
executable: if is_disable_cpi_setting_executable_and_rent_epoch_active {
false
} else {
account_info.executable
},
rent_epoch: if is_disable_cpi_setting_executable_and_rent_epoch_active {
0
} else {
account_info.rent_epoch
},
})
}

Expand Down Expand Up @@ -459,29 +435,20 @@ impl SyscallInvokeSigned for SyscallInvokeSignedRust {
ix.accounts.len() as u64,
invoke_context.get_check_aligned(),
)?;
let accounts = if invoke_context
.feature_set
.is_active(&feature_set::disable_cpi_setting_executable_and_rent_epoch::id())
{
let mut accounts = Vec::with_capacity(ix.accounts.len());
#[allow(clippy::needless_range_loop)]
for account_index in 0..ix.accounts.len() {
#[allow(clippy::indexing_slicing)]
let account_meta = &account_metas[account_index];
if unsafe {
std::ptr::read_volatile(&account_meta.is_signer as *const _ as *const u8) > 1
|| std::ptr::read_volatile(
&account_meta.is_writable as *const _ as *const u8,
) > 1
} {
return Err(Box::new(InstructionError::InvalidArgument));
}
accounts.push(account_meta.clone());
let mut accounts = Vec::with_capacity(ix.accounts.len());
#[allow(clippy::needless_range_loop)]
for account_index in 0..ix.accounts.len() {
#[allow(clippy::indexing_slicing)]
let account_meta = &account_metas[account_index];
if unsafe {
std::ptr::read_volatile(&account_meta.is_signer as *const _ as *const u8) > 1
|| std::ptr::read_volatile(&account_meta.is_writable as *const _ as *const u8)
> 1
} {
return Err(Box::new(InstructionError::InvalidArgument));
}
accounts
} else {
account_metas.to_vec()
};
accounts.push(account_meta.clone());
}

let ix_data_len = ix.data.len() as u64;
if invoke_context
Expand Down Expand Up @@ -718,52 +685,29 @@ impl SyscallInvokeSigned for SyscallInvokeSignedC {
)?
.to_vec();

let accounts = if invoke_context
.feature_set
.is_active(&feature_set::disable_cpi_setting_executable_and_rent_epoch::id())
{
let mut accounts = Vec::with_capacity(ix_c.accounts_len as usize);
#[allow(clippy::needless_range_loop)]
for account_index in 0..ix_c.accounts_len as usize {
#[allow(clippy::indexing_slicing)]
let account_meta = &account_metas[account_index];
if unsafe {
std::ptr::read_volatile(&account_meta.is_signer as *const _ as *const u8) > 1
|| std::ptr::read_volatile(
&account_meta.is_writable as *const _ as *const u8,
) > 1
} {
return Err(Box::new(InstructionError::InvalidArgument));
}
let pubkey = translate_type::<Pubkey>(
memory_mapping,
account_meta.pubkey_addr,
invoke_context.get_check_aligned(),
)?;
accounts.push(AccountMeta {
pubkey: *pubkey,
is_signer: account_meta.is_signer,
is_writable: account_meta.is_writable,
});
let mut accounts = Vec::with_capacity(ix_c.accounts_len as usize);
#[allow(clippy::needless_range_loop)]
for account_index in 0..ix_c.accounts_len as usize {
#[allow(clippy::indexing_slicing)]
let account_meta = &account_metas[account_index];
if unsafe {
std::ptr::read_volatile(&account_meta.is_signer as *const _ as *const u8) > 1
|| std::ptr::read_volatile(&account_meta.is_writable as *const _ as *const u8)
> 1
} {
return Err(Box::new(InstructionError::InvalidArgument));
}
accounts
} else {
account_metas
.iter()
.map(|account_meta| {
let pubkey = translate_type::<Pubkey>(
memory_mapping,
account_meta.pubkey_addr,
invoke_context.get_check_aligned(),
)?;
Ok(AccountMeta {
pubkey: *pubkey,
is_signer: account_meta.is_signer,
is_writable: account_meta.is_writable,
})
})
.collect::<Result<Vec<AccountMeta>, Error>>()?
};
let pubkey = translate_type::<Pubkey>(
memory_mapping,
account_meta.pubkey_addr,
invoke_context.get_check_aligned(),
)?;
accounts.push(AccountMeta {
pubkey: *pubkey,
is_signer: account_meta.is_signer,
is_writable: account_meta.is_writable,
});
}

Ok(StableInstruction {
accounts: accounts.into(),
Expand Down Expand Up @@ -869,34 +813,17 @@ where
invoke_context.get_check_aligned(),
)?;
check_account_infos(account_infos.len(), invoke_context)?;
let account_info_keys = if invoke_context
.feature_set
.is_active(&feature_set::disable_cpi_setting_executable_and_rent_epoch::id())
{
let mut account_info_keys = Vec::with_capacity(account_infos_len as usize);
#[allow(clippy::needless_range_loop)]
for account_index in 0..account_infos_len as usize {
#[allow(clippy::indexing_slicing)]
let account_info = &account_infos[account_index];
account_info_keys.push(translate_type::<Pubkey>(
memory_mapping,
key_addr(account_info),
invoke_context.get_check_aligned(),
)?);
}
account_info_keys
} else {
account_infos
.iter()
.map(|account_info| {
translate_type::<Pubkey>(
memory_mapping,
key_addr(account_info),
invoke_context.get_check_aligned(),
)
})
.collect::<Result<Vec<_>, Error>>()?
};
let mut account_info_keys = Vec::with_capacity(account_infos_len as usize);
#[allow(clippy::needless_range_loop)]
for account_index in 0..account_infos_len as usize {
#[allow(clippy::indexing_slicing)]
let account_info = &account_infos[account_index];
account_info_keys.push(translate_type::<Pubkey>(
memory_mapping,
key_addr(account_info),
invoke_context.get_check_aligned(),
)?);
}
Ok((account_infos, account_info_keys))
}

Expand All @@ -917,7 +844,6 @@ where
F: Fn(
&InvokeContext,
&'b MemoryMapping<'a>,
bool,
u64,
&T,
&SerializedAccountMetadata,
Expand All @@ -926,9 +852,6 @@ where
let transaction_context = &invoke_context.transaction_context;
let instruction_context = transaction_context.get_current_instruction_context()?;
let mut accounts = Vec::with_capacity(instruction_accounts.len().saturating_add(1));
let is_disable_cpi_setting_executable_and_rent_epoch_active = invoke_context
.feature_set
.is_active(&disable_cpi_setting_executable_and_rent_epoch::id());

let program_account_index = program_indices
.last()
Expand Down Expand Up @@ -993,7 +916,6 @@ where
do_translate(
invoke_context,
memory_mapping,
is_disable_cpi_setting_executable_and_rent_epoch_active,
account_infos_addr.saturating_add(
caller_account_index.saturating_mul(mem::size_of::<T>()) as u64,
),
Expand Down Expand Up @@ -1256,9 +1178,6 @@ fn update_callee_account(
mut callee_account: BorrowedAccount<'_>,
direct_mapping: bool,
) -> Result<(), Error> {
let is_disable_cpi_setting_executable_and_rent_epoch_active = invoke_context
.feature_set
.is_active(&disable_cpi_setting_executable_and_rent_epoch::id());
if callee_account.get_lamports() != *caller_account.lamports {
callee_account.set_lamports(*caller_account.lamports)?;
}
Expand Down Expand Up @@ -1312,30 +1231,11 @@ fn update_callee_account(
}
}

if !is_disable_cpi_setting_executable_and_rent_epoch_active
&& callee_account.is_executable() != caller_account.executable
{
callee_account.set_executable(caller_account.executable)?;
}

// Change the owner at the end so that we are allowed to change the lamports and data before
if callee_account.get_owner() != caller_account.owner {
callee_account.set_owner(caller_account.owner.as_ref())?;
}

// BorrowedAccount doesn't allow changing the rent epoch. Drop it and use
// AccountSharedData directly.
let index_in_transaction = callee_account.get_index_in_transaction();
drop(callee_account);
let callee_account = invoke_context
.transaction_context
.get_account_at_index(index_in_transaction)?;
if !is_disable_cpi_setting_executable_and_rent_epoch_active
&& callee_account.borrow().rent_epoch() != caller_account.rent_epoch
{
return Err(Box::new(InstructionError::RentEpochModified));
}

Ok(())
}

Expand Down Expand Up @@ -1670,7 +1570,7 @@ mod tests {
ebpf::MM_INPUT_START, memory_region::MemoryRegion, program::SBPFVersion, vm::Config,
},
solana_sdk::{
account::{Account, AccountSharedData},
account::{Account, AccountSharedData, ReadableAccount},
clock::Epoch,
feature_set::bpf_account_data_direct_mapping,
instruction::Instruction,
Expand Down Expand Up @@ -1846,7 +1746,6 @@ mod tests {
let caller_account = CallerAccount::from_account_info(
&invoke_context,
&memory_mapping,
false,
vm_addr,
account_info,
&account_metadata,
Expand All @@ -1860,8 +1759,6 @@ mod tests {
account.data().len()
);
assert_eq!(caller_account.serialized_data, account.data());
assert_eq!(caller_account.executable, account.executable());
assert_eq!(caller_account.rent_epoch, account.rent_epoch());
}

#[test]
Expand Down Expand Up @@ -2758,8 +2655,6 @@ mod tests {
serialized_data: data,
vm_data_addr: self.vm_addr + mem::size_of::<u64>() as u64,
ref_to_len_in_vm: VmValue::Translated(&mut self.len),
executable: false,
rent_epoch: 0,
}
}
}
Expand Down
5 changes: 2 additions & 3 deletions programs/bpf_loader/src/syscalls/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ use {
vm::Config,
},
solana_sdk::{
account::ReadableAccount,
account_info::AccountInfo,
alt_bn128::prelude::{
alt_bn128_addition, alt_bn128_multiplication, alt_bn128_pairing, AltBn128Error,
Expand All @@ -36,8 +35,8 @@ use {
feature_set::FeatureSet,
feature_set::{
self, blake3_syscall_enabled, curve25519_syscall_enabled,
disable_cpi_setting_executable_and_rent_epoch, disable_deploy_of_alloc_free_syscall,
disable_fees_sysvar, enable_alt_bn128_compression_syscall, enable_alt_bn128_syscall,
disable_deploy_of_alloc_free_syscall, disable_fees_sysvar,
enable_alt_bn128_compression_syscall, enable_alt_bn128_syscall,
enable_big_mod_exp_syscall, enable_partitioned_epoch_reward, enable_poseidon_syscall,
error_on_syscall_bpf_function_hash_collisions, last_restart_slot_sysvar,
reject_callx_r10, remaining_compute_units_syscall_enabled,
Expand Down

0 comments on commit f349d71

Please sign in to comment.