Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CPI size restrictions are too strict #26639

Closed
jstarry opened this issue Jul 15, 2022 · 0 comments · Fixed by #26653
Closed

CPI size restrictions are too strict #26639

jstarry opened this issue Jul 15, 2022 · 0 comments · Fixed by #26653

Comments

@jstarry
Copy link
Member

jstarry commented Jul 15, 2022

Problem

Currently, the restriction on the serialized size of cpi instructions is too strict. The current CPI size restriction is useful for preventing programs from invoking overly large instructions that wouldn't otherwise be possible to invoked via serialized transaction instructions. However, with the introduction of address lookup tables, it will soon be possible to invoke transaction instructions with many more accounts and so this restriction needs to be adjusted to allow programs to have the same ability to invoke instructions with more accounts.

There are two different checks imposed on CPI's:

  1. check_instruction_size which enforces that 34 * ix.accounts.len() + ix.data.len() <= 1280

    fn check_instruction_size(
    num_accounts: usize,
    data_len: usize,
    invoke_context: &mut InvokeContext,
    ) -> Result<(), EbpfError<BpfError>> {
    let size = num_accounts
    .saturating_mul(size_of::<AccountMeta>())
    .saturating_add(data_len);
    let max_size = invoke_context.get_compute_budget().max_cpi_instruction_size;
    if size > max_size {
    return Err(SyscallError::InstructionTooLarge(size, max_size).into());
    }
    Ok(())
    }

  2. check_account_infos which enforces that 32 * account_infos.len() <= 1280

    fn check_account_infos(
    len: usize,
    invoke_context: &mut InvokeContext,
    ) -> Result<(), EbpfError<BpfError>> {
    let adjusted_len = if invoke_context
    .feature_set
    .is_active(&syscall_saturated_math::id())
    {
    len.saturating_mul(size_of::<Pubkey>())
    } else {
    #[allow(clippy::integer_arithmetic)]
    {
    len * size_of::<Pubkey>()
    }
    };
    if adjusted_len > invoke_context.get_compute_budget().max_cpi_instruction_size {
    // Cap the number of account_infos a caller can pass to approximate
    // maximum that accounts that could be passed in an instruction
    return Err(SyscallError::TooManyAccounts.into());
    };
    Ok(())

Proposed Solution

  • Restrict the number of instruction accounts in CPI instruction to 255 (matches max bpf program ix accounts)
  • Restrict the number of account info's in CPI instruction to 64 (matches max tx account locks)
  • Restrict the size of instruction data to 1280 (continue using max_cpi_instruction_size)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant