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

Generated CPI Instructions Are Stack Size Inefficient #390

Closed
bonedaddy opened this issue Jun 16, 2021 · 2 comments · Fixed by #824
Closed

Generated CPI Instructions Are Stack Size Inefficient #390

bonedaddy opened this issue Jun 16, 2021 · 2 comments · Fixed by #824

Comments

@bonedaddy
Copy link
Contributor

Overview

Generated CPI instructions are stack size inefficient and can lead to blowing the stack size for programs which make heavy use of CPI. The reason is that the CPI instructions expect the same account types as those defined in the program itself, which can lead to excessive deserialization causing the stack size limit to be reached.

Consider the following program which will be called by another program:

pub mod programA {
   pub fn do_something(ctx: Context<SomeInstruction>, some_value: u64) -> Result<()> {
      ctx.some_large_account.some_field =some_value;
     Ok(())
   }
}
#[derive(Accounts)]
pub struct SomeInstruction<'info> {
 #[account(mut)]
 pub some_large_account: ProgramAccount<'Info, BigData>,
}

The other program needs to import the BigData account as a CpiAccount which means that deserialization and cloning takes place because one needs to call ctx.accounts.some_large_account.clone().into() which can potentially blow the stack size, or lead to a very large stack size which then gets fully consumed within programA.

pub mod programB {
   pub fn call_do_something(...) -> Result<()> {
     programA::cpi::do_something(...)
   }
#[derive(Accounts)]
pub struct SomeInstructionB<'info> {
  #[account(mut)}
   pub some_large_account: CpiAccount<'info, BigData>
}

Solution(s)

1 - Increase Stack Size Limit To 8KB

While I think this is probably the best option in the long run, without actually addressing the deserialization concern it's entirely possible for that stack size to be reached as well (that aside, anchor isnt the place to discuss such a thing).

2 - Default To AccountInfo Types 🎉

By avoiding the need to have explicit account types and defaulting to AccountInfo no extra deserialization is needed. Therefore the generated CPI instructions can default to AccountInfo instead of whatever account type is used when the instruction is declared within the main program (programA in this example). For callers that may want access to the account data there currently exists methods to obtain such data (CpiContext::try_from, etc....). This solution provides the best of both worlds allowing people to have access to the account data if they need it, without impacting those who dont.

For context i had a CPI instruction that was blowing the stack size by 700 bytes, copying & pasting the generated CPI instructions but changing them to use AccountInfo instead resolved the stack size issue.

@armaniferrante
Copy link
Member

In short, the task here is to change all all account structs used in the generated cpi module (and CpiContext) to use AccountInfo.

@armaniferrante
Copy link
Member

Priority 1 since this is needed for those composing with Psyoptions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants