Using Account<T> with owner constraint ignores owner constraint

[nvsriram]

Summary

When using Account<T> alongside owner constraint, the Account owner check on deserialization makes the owner constraint mutually exclusive (or redundant at best).

Playground Example

In this playground (inspired by this post's solution *), I try to init the pool_account and set the owner to be the user (signer).
On taking a look at the expanded code, I see that it does assign the owner as user correctly but when deserializing, it hits the AccountOwnedByWrongProgram error - it expects the owner to be the calling program when the owner is actually the user.

* I know this is not a very practical use case as is but this is purely to showcase the error.

Discrepancy in Docs

The 0.30.1 anchor lang docs show this example for using Account<T> alongside the owner constraint:

MyData is probably defined in the calling program (and not token_program) and so we have a case where T::owner() is different from the owner constraint. And the expected behaviour suggests that data/data_two are owned by token_program and deserialized into MyData without any error.

Potential Solutions

1. lang: make parser error when owner is used on owner checking types #1749 could be implemented to make it clear that Account<T> and owner constraint should not be used alongside each other.
2. Docs can be updated to show working examples. Or it be explicit in how MyData should be defined for this to work.
   eg:- The owner trait on MyData is overriden to token_program ID, only then would this work.
3. Allow the owner constraint to be used alongside Account<T> and give the owner constraint priority
   i.e., override the owner check on Account deserialization to behave differently when owner constraint is used - either skip OR check for owner constraint owner.

[acheroncrypto]

Thanks for creating the issue. I don't think overriding the owner via constraints should be allowed when the account type already has an owner, so the first solution looks to be the best one. Instead of overriding the owner of the account via constraints, we can also potentially support overriding it when annoting the account with the #[account] macro in order to make it easier (rather than implementing all traits manually), for example:

#[account(owner = <PROGRAM_ID>)]
pub struct MyAccount {}

When using Account<T> alongside owner constraint, the Account owner check on deserialization makes the owner constraint mutually exclusive (or redundant at best).

Correct — the documentation should also be updated.

[evan-gray]

@acheroncrypto would that account annotation support a flexible owner check? I would expect a possible use case would be to reference a field on another account in the instruction and want to enforce that value to be the owner. I also don't necessarily like the idea of overriding the owner via the constraints, but perhaps there could be another Account Type like Account but explicitly doesn't enforce the owner so that the owner constraint could be effectively used? 🙏 All the other checks are useful in this case, so it would be nice to avoid going full UncheckedAccount.

I certainly prefer an explicit unchecked over an implicit override.

[kcsongor]

I'd like to support @evan-gray's suggestion here, basically something along the lines of InterfaceAccount but instead of the owner having to belong to a predefined set of owners, it would instead be required to add an explicit owner constraint.

[acheroncrypto]

@acheroncrypto would that account annotation support a flexible owner check?

It could, but I'm not sure if it should.

I'd like to support @evan-gray's suggestion here, basically something along the lines of InterfaceAccount but instead of the owner having to belong to a predefined set of owners, it would instead be required to add an explicit owner constraint.

Something like the following?

#[derive(Accounts)]
struct MyIx<'info> {
    #[account(owner = other_account.owner)]
    generic_owner: GenericOwnerAccount<'info, MyAccount>,
    other: Account<'info, OtherAccount>,
}

#[account]
struct MyAccount {
    field: u64,
}

#[account]
struct OtherAccount {
    owner: Pubkey,
}

If so, how do you make sure this is going to be safe?

[kcsongor]

Right, something like that!

If so, how do you make sure this is going to be safe?

Not sure I understand -- safety will be achieved through a combination of the owner and other constraints depending on the application (for example a constraint that checks that the owner has to be "registered" via a corresponding PDA or somethint like that).
This would essentially generalise InterfaceAccount into a more dynamic modular system (and I believe that in addition to generalisation, it would also improve it in some ways, because currently multiple InterfaceAccounts are only implicitly checked to be owned by the same owner if they happen to be passed into a CPI instruction that performs those checks, so in many cases the actual ownersip relaions are not manifest in the caller program's IDL)

[acheroncrypto]

If so, how do you make sure this is going to be safe?

Not sure I understand -- safety will be achieved through a combination of the owner and other constraints depending on the application (for example a constraint that checks that the owner has to be "registered" via a corresponding PDA or somethint like that).

Malicious actors can change the account data to whatever they want when an account is owned by an external program (assuming they have the program authority), making the account type unsafe by default, and it would be up to the developer to ensure its safety.