Feature Request: Clearer error when handling Account & ProgramAccount

[cyphersnake]

For Account and ProgramAccount, before checking ownership - do a check for existence. An important difference between transferring a non-existent account (belonging to the system program and with lamports == 0) an account with really incorrect ownership.
It is suggested to add an error that the account was expected to be already initialized:

#[msg("The program expected this account to be already initialized")]
AccountNotInitialized,

This is an important clarification, since ownership is not the most intuitive marker of existence.

[cyphersnake]

@armaniferrante @fanatid
I'm ready to do it myself. I want to do this through a generic trait like:

/// Allows you to check if a given account is initialized by system program call or not
pub trait AccountInitialized<'info>: ToAccountInfo<'info> {
    fn initialized(&self) -> bool {
        let account_info = self.to_account_info();
        account_info.lamports() == 0 && system_program::ID.eq(account_info.owner)
    }
    fn not_initialized(&self) -> bool {
        !self.initialized()
    }
}

Since this will be my first contribution to the project, let me know if this conflicts with any accepted practice.

[fanatid]

Interesting, can you provide an example of where the current implementation will fail or be incorrect?
btw, ProgramAccount is deprecated.

[cyphersnake]

@fanatid I didn't quite understand what kind of incorrect implementation you are talking about 😅.

Just instead of adding two if statements in try_from & try_from_unchecked, I thought to add a trait that will be automatically implemented for all types with: ToAccountInfo.
Simple case:

if account_info.lamports() == 0 && system_program::ID.eq(account_info.owner) {
    return Err(ErrorCode::AccountNotInitialized.into());
}

Slightly harder

if account.not_initialized() {
    return Err(ErrorCode::AccountNotInitialized.into());
}

and add all needed context about solana account initialization with all links in rustdoc of this trait method.

But the first option will also solve the issue and it seems to me it will also improve the code base! So if you prefer not to complicate things and add simple if, then I'll be happy to implement it too.

[fanatid]

I'm just not sure that I understand everything correctly.

Are you saying that right now everything works correctly but we need an additional check for better errors, right? Or without check

account_info.lamports() == 0 && system_program::ID.eq(account_info.owner)

we have security vulnerability?

[cyphersnake]

Since non-initialized accounts do not have allocated memory, we cannot initialize any of them (I have not tried it, I am guided by the documentation, you can correct me if you are wrong). Therefore, I do not see a vulnerability here.

And the original issue is only about the decomposition of the error into two more understandable and intuitive errors.

[fanatid]

Since non-initialized accounts do not have allocated memory, we cannot initialize any of them (I have not tried it, I am guided by the documentation, you can correct me if you are wrong). Therefore, I do not see a vulnerability here.

I do not know the exact behavior. Can you leave links to the documentation?

[cyphersnake]

Accounts that have never been created via the system program can also be passed to programs. When an instruction references an account that hasn't been previously created, the program will be passed an account with no data and zero lamports that is owned by the system program.

I also made the manual check, for an empty account, the following condition is correct:

assert_eq!(ctx.accounts.not_initialized.try_borrow_data().unwrap().len(), 0);