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

lang: Remove 3 lifetime definitions from Context #3340

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

acheroncrypto
Copy link
Collaborator

Problem

Context struct definition includes 4 lifetimes:

pub struct Context<'a, 'b, 'c, 'info, T: Bumps> {

While this is technically correct, and it's also what Rust does by default, it results in a poor developer experience because the lifetimes leak to the Anchor users in various cases. For example, remaining accounts usage requires annotating the instruction handler with lifetimes, which is quite difficult to figure out for people who're less experienced with lifetimes (not to mention this is completely unnecessary).

Summary of changes

Remove 3 (out of 4) lifetimes from the Context struct. In other words, make all references have the same lifetime. This makes it so much easier for Anchor users to to handle places that Context is used. For example, remaining accounts usage:

pub fn test_remaining_accounts<'c: 'info, 'info>(
ctx: Context<'_, '_, 'c, 'info, TestRemainingAccounts>,
) -> Result<()> {

simply becomes:

pub fn test_remaining_accounts(ctx: Context<TestRemainingAccounts>) -> Result<()> {

Copy link

vercel bot commented Nov 1, 2024

@acheroncrypto is attempting to deploy a commit to the coral-xyz Team on Vercel.

A member of the Team first needs to authorize it.

@juchiast
Copy link

juchiast commented Nov 2, 2024

How does this interact with the issues in #2770 and #3341
Basically, the type &'info AccountInfo<'info> is not very usable, AccountInfo<'info> is invariant because it contains RefCell, meaning we cannot cast the lifetime AccountInfo<'info> into a struct with shorter lifetime AccountInfo<'a>.

@acheroncrypto
Copy link
Collaborator Author

How does this interact with the issues in #2770 and #3341

It basically fixes the problem. It still requires a lifetime annotation, but we should be able to handle this automically in the program macro.

Basically, the type &'info AccountInfo<'info> is not very usable

Yeah, but the alternative is worse (declaring another lifetime as explained in the "Details" section of #2770).

AccountInfo<'info> is invariant because it contains RefCell, meaning we cannot cast the lifetime AccountInfo<'info> into a struct with shorter lifetime AccountInfo<'a>.

Does that even matter here? I don't think the Rust compiler would allow you to overwrite the data with a reference declared inside the instruction handler.

@juchiast
Copy link

juchiast commented Nov 2, 2024

Does that even matter here? I don't think the Rust compiler would allow you to overwrite the data with a reference declared inside the instruction handler.

With a normal struct, it is possible to change the lifetime to a smaller one by re-borrowing

struct X<'a> {
    s: &'a str,
}

fn cast<'a, 'b>(x: X<'a>) -> X<'b> where 'a: 'b {
    X { s: &*x.s }
   // or just `return x;`, rust can cast automatically
}

but we can't do this with AccountInfo<'a>. It's impossible to make &'a AccountInfo<'a> reference, if you need to use &'a AccountInfo<'a> reference, you are stuck with that lifetime forever.

spl-candy-guard case: they use UncheckedAccount<'info>, do some check manually then call Account::try_from. Fortunately we can still recover the original &'info AccountInfo<'info> inside UncheckedAccount, if the lifetime is lost (by cloning, for example), it is impossible to create &'info AccountInfo<'info> again.

@acheroncrypto
Copy link
Collaborator Author

Does that even matter here? I don't think the Rust compiler would allow you to overwrite the data with a reference declared inside the instruction handler.

With a normal struct, it is possible to change the lifetime to a smaller one by re-borrowing

struct X<'a> {
    s: &'a str,
}

fn cast<'a, 'b>(x: X<'a>) -> X<'b> where 'a: 'b {
    X { s: &*x.s }
   // or just `return x;`, rust can cast automatically
}

but we can't do this with AccountInfo<'a>. It's impossible to make &'a AccountInfo<'a> reference, if you need to use &'a AccountInfo<'a> reference, you are stuck with that lifetime forever.

Yeah, I'm aware of that, but I'm not sure how that answers my initial comment about the Rust compiler not allowing you to overwrite the data with a reference declared inside the instruction handler.

spl-candy-guard case: they use UncheckedAccount<'info>, do some check manually then call Account::try_from. Fortunately we can still recover the original &'info AccountInfo<'info> inside UncheckedAccount, if the lifetime is lost (by cloning, for example), it is impossible to create &'info AccountInfo<'info> again.

Since you're using "they" when referring to the Metaplex team, I'm assuming you're not from the Metaplex team, and you just want to interact with the mpl-candy-guard program. I'm also assuming you're using Anchor v0.29 and Solana v1.18, since that's what you used in metaplex-foundation/mpl-candy-machine#76.

If my assumptions are correct, then trying to fully upgrade that program is completely unnecessary for your use case, because you don't need the program's internal logic to be able to interact with a program (you only need implementation signatures). If you want to use the program's crate as a CPI client or an off-chain client, you can safely remove all its instruction handler logic, including the parts where it uses Account::try_from.

As a side note, the mpl-candy-guard's usage of Account::try_from in order to implement some sort of an optional account check also seems redundant in newer Anchor versions because Anchor now supports optional accounts in accounts structs with Option<T>.

Furthermore, you don't even need to upgrade anything to interact with older programs if you're using the latest version (v0.30.1). In fact, you don't even need to add programs as a dependency. Here are some useful links:

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

Successfully merging this pull request may close these issues.

2 participants