anchor icon indicating copy to clipboard operation
anchor copied to clipboard

Optional Accounts in Instruction Context

Open crispheaney opened this issue 3 years ago • 22 comments

Sometimes an instruction has "optional accounts" which aren't guaranteed to be present. For example, on an exchange, when a user opens a position, they may optionally pass in a referrer account.

I made this work by doing the following:

  1. Add an instruction data argument which specifies the optional accounts that are present
  2. Pull the optional account from the remaining_accounts

The biggest downside here is that I couldn't leverage all of anchor's security checks. I'm wondering if other people have a similar use for optional accounts and if we can work it into anchor.

crispheaney avatar Nov 14 '21 23:11 crispheaney

I don't think there's a way around requiring some type of option is_some() map in the instruction data.

armaniferrante avatar Nov 15 '21 01:11 armaniferrante

@armaniferrante could you show me an example code?

rocalex avatar Jul 14 '22 18:07 rocalex

@armaniferrante what do you think about the option for an account to potentially be None and have the constraints only checked if it's not none. Not sure how this would be implemented but on the account side you could mark an account as optional and if the program ID is passed in at that index the account would be treated as None. I'd be interested in spending some time trying to get this to work. Before that, a longer discussion on whether what I'm proposing would be the best way to make this work should probably take place.

stegaBOB avatar Jul 27 '22 20:07 stegaBOB

Passing in the programId to represent None is very clever and could be coupled with a new macro, e.g., #[account(option)]. Unfortunately, the tradeoff here is that it can potentially introduce a foot gun, since the user might actually want to allow the current program there.

Another option is to determine optionality based on a boolean expression, e.g., #[account(option = <expression>)], which would allow one to use optional accounts based on instruction or account data. We could do something similar for a Vec of accounts as well.

armaniferrante avatar Jul 27 '22 21:07 armaniferrante

Would you also potentially run into issues if you're trying to pass in multiple accounts of the same type? Or would that be coupled with something like a bitmask in the ix data?

stegaBOB avatar Jul 27 '22 21:07 stegaBOB

Not that I can see. It would be up to the developer to ensure uniqueness between accounts with sufficient constraints--which is another consideration here, i.e., how do constraints work on optional accounts.

armaniferrante avatar Jul 27 '22 21:07 armaniferrante

I feel like that might make it easier to shoot oneself in the foot with using deserialization and constraints alone to determine whether an account should be optional. I'd argue that if someone wanted to pass in the program Id in an optional account that frankly they're most certainly doing something wrong. I think constraints with the program Id method would just be ignored if the account is none. I think the constraits for other accounts would have to treat the option accounts as an option (which is what I think would make the most sense to treat them as such).

stegaBOB avatar Jul 27 '22 21:07 stegaBOB

Using constraints that may not apply to determine the order of accounts that may not exist sounds like a scary problem that I'd prefer not to work on haha.

stegaBOB avatar Jul 27 '22 21:07 stegaBOB

I think there are multiple use cases to having multiple trailing accounts:

  1. A few optional accounts

Supporting a referral account like the Serum 20% host fee cut feature.

So if we had an amm with that feature

#[derive(Accounts)]
struct Swap<'info> {
    pool: Account<'info, Pool>,
    ...
    referrer: Option<Account<'info, TokenAccount>>,
}
  1. An actual need for a vec of tuples to perform action sequentially or a mix of that and the above

A program transferring multiple tokens at once into escrow, the length being unknown, thus requiring to iterate over the remaining accounts and validate each "pair" of source/destination.

I think that one has too many edge cases so it is to remain being handled wildly.

Unfortunately, the tradeoff here is that it can potentially introduce a foot gun, since the user might actually want to allow the current program there.

It might not make sense anyway to provide the program id account in an optional way as it takes a single byte in the tx to refer to it.

Arrowana avatar Jul 27 '22 23:07 Arrowana

That code example is exactly the thing I was imagining. I think a bit more thought still has to be put in in terms of how constraints are handled but I'm quite partial to using the program ID. Should save space and make it easier to implement I think.

stegaBOB avatar Jul 27 '22 23:07 stegaBOB

For one of my projects I made a fork of anchor but finally I created a copy from scratch of it to handle all deserialisation cases, including enums, options, etc.

I handled options in two ways:

  1. Option<Account<'info, AccountType>>: tries to deserialise the AccountType and if it fails, it returns a None. This includes checking the programId and the deserialisation of AccountType, so it skips that if it fails. This option implies you can SKIP to include the account in the TX.

  2. OptionalAccount<'info, AccountType>>: this is a special enumeration that in the end mimics an Option. This approach verifies whether the account passed is the default one (1111...1111). In that case it results in the None equivalent variant, otherwise it tries to deserialise AccountType failing if it can't be deserialised. With this approach you HAVE to always pass the actual account or the default one (1111...1111)

juliotpaez avatar Jul 28 '22 05:07 juliotpaez

For one of my projects I made a fork of anchor but finally I created a copy from scratch of it to handle all deserialisation cases, including enums, options, etc.

I handled options in two ways:

  1. Option<Account<'info, AccountType>>: tries to deserialise the AccountType and if it fails, it returns a None. This includes checking the programId and the deserialisation of AccountType, so it skips that if it fails. This option implies you can SKIP to include the account in the TX.

  2. OptionalAccount<'info, AccountType>>: this is a special enumeration that in the end mimics an Option. This approach verifies whether the account passed is the default one (1111...1111). In that case it results in the None equivalent variant, otherwise it tries to deserialise AccountType failing if it can't be deserialised. With this approach you HAVE to always pass the actual account or the default one (1111...1111)

Can you share the link to this anchor fork of yours? The #2 OptionalAccount struct is exactly what Im imagining.

stegaBOB avatar Jul 28 '22 18:07 stegaBOB

Agree with option 2 by @juliotpaez. Each optional account needs to be represented or can't provide guarantees on the accounts. As for constraints they should be applied when the account isn't 111...1111. A good real world example to run the proposal through is CM mint instruction. It makes heavy use of remaining accounts.

https://github.com/metaplex-foundation/metaplex-program-library/blob/master/candy-machine/program/src/processor/mint.rs#L77

kespinola avatar Jul 28 '22 18:07 kespinola

I think the program ID should be used instead of the default key. That should save 32 extra bytes in the transaction.

stegaBOB avatar Jul 28 '22 18:07 stegaBOB

Hey @stegaBOB for sure, here it is the link of how I I decode my OptionalAccount. As you can see, the project looks similar to Anchor but I use another approach based on traits to reduce the code in the macros. Also I don't generate TS code yet.

@kespinola I also included in my version deserialising the list of accounts as an Enum, so a single instruction can have more than one representation (I consider that it should have only slightly different changes, but removing the possibility is not a goal).

Finally, regarding using the programId as None variant, I think that can save 32 bytes but can complicate things. Think about you are calling CPIs. Each program would have a different value for its None. Moreover, if you pass many Nones for whatever reason you would need, you will end wasting more that what you saved, so I think keeping the default account as None is the best option, because no matter what you would always use 32 bytes for all Nones.

juliotpaez avatar Jul 28 '22 19:07 juliotpaez

Moreover, if you pass many Nones for whatever reason you would need, you will end wasting more that what you saved, so I think keeping the default account as None is the best option, because no matter what you would always use 32 bytes for all Nones.

I'm not sure if I follow this. Using the programId shouldn't actually use any bytes regardless of how many CPIs there are since all the programs that you will end up using, either in CPIs or in additional instructions, will always be required to be included in the accounts list for the transaction. The fact that it may be a bit more complicated is a fair argument, although I think adding solid support for cpi builders and transaction builders should abstract all the complexities away. Unless you plan to build the instructions manually, in which case it might make it a bit more difficult to see whats actually going on.

stegaBOB avatar Jul 29 '22 20:07 stegaBOB

@armaniferrante we're going to start working on this addition to hopefully get a working proof of concept out that can be fleshed out fully after. Once we have something of value we'll open a draft PR.

stegaBOB avatar Jul 29 '22 20:07 stegaBOB

Each program would have a different value for its None.

@juliotpaez Now that I'm working on implementing this, I see how this is an issue. Implementing the ToAccountInfos and ToAccountMetas for the Option doesn't have access to the program Id. (obviously! Its just a None!). Do you think an asymmetric implementation would make sense? It should be possible to implement the Accounts trait (with the try_accounts) to check for both the default keypair and the program id, while the to_account_infos and to_account_metas only return the default keypair? I don't think this would be an issue, and it may make it possible to save those 32 bytes again?

stegaBOB avatar Jul 30 '22 01:07 stegaBOB

Hi @stegaBOB, I think you have access in the macro using crate::ID, because the programId is generated as a static for every crate. What I doubt here is that you need to pass all the programIds your program is going to call by CPI. For example, in my library, I use CPI passing the accounts and arguments for the instruction but I don't pass any AccountInfo<'info> of the program itself. Anchor does the same. So you're not really saving anything here if you pass each None as a programId if the transaction is not going to call it directly. That's why I still think using the default one is the best option.

Let me know if I'm wrong with this.

PD: I'm speaking about CPI, not using instructions for different programs in the same transaction.

juliotpaez avatar Jul 30 '22 08:07 juliotpaez

Ah the crate::ID may work! I'll need to look into that. In terms of the program ID not showing up in CPIs, it should really be there since it's needed to run the CPI. Regardless, the peogramID has to have been passed in in the txn at some point so it's not adding more data. Are you in the anchor discord? I made a forum post about this and it may be easier to discuss there.

stegaBOB avatar Jul 30 '22 14:07 stegaBOB

Also @armaniferrante can you assign me to this issue?

stegaBOB avatar Jul 30 '22 17:07 stegaBOB

#2094 is the current WIP for this feature

stegaBOB avatar Jul 31 '22 19:07 stegaBOB

@Henry-E looks like this can be closed!

stegaBOB avatar Jan 03 '23 09:01 stegaBOB