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 client with return values #20

Closed
armaniferrante opened this issue Jan 13, 2021 · 13 comments · Fixed by #1598
Closed

CPI client with return values #20

armaniferrante opened this issue Jan 13, 2021 · 13 comments · Fixed by #1598
Labels
help wanted Extra attention is needed lang v1.0.0-blocker

Comments

@armaniferrante
Copy link
Member

armaniferrante commented Jan 13, 2021

The CPI client should transparently use "retbuf" accounts to allow returning values from CPI.

@armaniferrante
Copy link
Member Author

armaniferrante commented Feb 2, 2021

One advantage of using CPI instead of reading accounts directly is that the program can perform access control that the caller doesn't need to worry about.

@armaniferrante
Copy link
Member Author

Another advantage is that sometimes you want to make a call to an opaque address that implements some interface. So you need to make a CPI instead of reading accounts.

@armaniferrante
Copy link
Member Author

armaniferrante commented Mar 19, 2021

The "Ephemeral Accounts" proposed here will allow this to be implemented without the shared memory program, which is the last thing blocking the implementation of this https://github.com/solana-labs/solana/blob/1b81a6323ff1c0c2a9510804f71f07bbde3719a7/docs/src/proposals/interest-bearing-tokens.md#ephemeral-accounts.

@armaniferrante
Copy link
Member Author

New proposal for return data here solana-labs/solana#19318.

@armaniferrante armaniferrante added the help wanted Extra attention is needed label Feb 16, 2022
@paul-schaaf
Copy link
Contributor

@armaniferrante
Copy link
Member Author

armaniferrante commented Feb 16, 2022

To clarify the desired behavior here.

One should be able to define an instruction handler with a return value, e.g.

#[program]
mod program {
  fn my_instruction(ctx: Context<MyInstruction) -> Result<u64> {
    Ok(2)
  }
}

Such that the cpi client in crate::cpi::my_program can be invoked, returning Result<u64> without ever having to worry about the return data syscalls or serialization. This also means the return data will have to implement AnchorSerialize, AnchorDeserialize.

@paul-schaaf
Copy link
Contributor

We should consider an initial explicit approach instead of a Result<T> return value imo.

I can imagine that devs want to toggle return data via ix input to save compute for callers that dont need it.

And we could write wrappers around the syscall with T: Anchor(De)Serialize that would make them pleasant to use.

Explicit functions also feel like a reasonable stepping stone from which we could see whether we should add cpi return values as handler return values

@armaniferrante
Copy link
Member Author

Explicit functions also feel like a reasonable stepping stone from which we could see whether we should add cpi return values as handler return values

Explicit functions sound good as an additional feature, though I still think we should add the return functions as described above so that we can have a more intuitive programming model that favors composability. This feature should be used for pluggable components that typically don't store state and just return a value. Programs that care about compute can simply not return a value.

@paul-schaaf
Copy link
Contributor

I agree that it would be more intuitive although I dont see how it's more composable. its the same functionality, just a different api.

That said, I like it with functions that only compute and return values and do nothing else although Im not sure those are really necessary to be called via cpi. you can save one context switch in compute by not cpi'ing. It seems to me that return values are most useful when something else is done in the cpi anyway.

In a world where compute doesnt matter this is fine to do via cpi but I dont think that world is real. We might not have to do eth like optimizations but compute-intensive tx are hurting the network and may cost more in the future.

And I think that all programs should care about compute because theyre not independent of other programs (and also compute will probably cost more in the future). there will be a tx wide compute budget. this means that program optimization becomes more important cause badly optimized programs take away compute from other ix.

@armaniferrante
Copy link
Member Author

The feature is opt in. So there's no downside of adding it since programs that don't need it can opt out.

@tomlinton
Copy link
Contributor

I'll tackle this if no one is working on it.

vadorovsky pushed a commit to vadorovsky/anchor that referenced this issue Jun 21, 2023
vadorovsky added a commit to vadorovsky/anchor that referenced this issue Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed lang v1.0.0-blocker
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants