SDK: update error variants in Feature::from_account_info

[buffalojoec]

Problem

Feature::from_account_info(..) checks that the owner field matches Feature111111111111111111111111111111111111 but it throws ProgramError::InvalidArgument.

It also throws ProgramError::InvalidArgument on deserialization failure.

Summary of Changes

Update these errors to be more clear.

[codecov]

Codecov Report

Merging #33750 (32d5322) into master (fbded92) will decrease coverage by 0.1%.
Report is 1147 commits behind head on master.
The diff coverage is 88.5%.

@@            Coverage Diff            @@
##           master   #33750     +/-   ##
=========================================
- Coverage    81.8%    81.8%   -0.1%     
=========================================
  Files         766      807     +41     
  Lines      209094   217936   +8842     
=========================================
+ Hits       171138   178324   +7186     
- Misses      37956    39612   +1656     

[CriesofCarrots]

@t-nelson , can you please sanity check us on this change? I don't see any callers of Feature::from_account_info() in the monorepo or SPL.

[CriesofCarrots]

Another way we could address this is by duplicating/moving the struct Feature definition into spl-feature-gate. But maybe it's better to leave it so that updating the interface is painful.

[t-nelson]

it's not clear to me why this method returns a ProgramError at all. doesn't seem to be used in runtime so making this change w/o a feature gate shouldn't break consensus. *::IncorrectProgramId is very much my least favorite of this legacy error codes. it misleads basically 100% of devs/users upon first encounter

[CriesofCarrots]

it's not clear to me why this method returns a ProgramError at all.

I'm not sure why it was written this way originally, especially as there's no caller, but we're about to start using it in a bpf program.

*::IncorrectProgramId is very much my least favorite of this legacy error codes

What variant do you recommend instead? It seems to be the most correct when an account isn't owned by the expected program.

[buffalojoec]

What variant do you recommend instead? It seems to be the most correct when an account isn't owned by the expected program.

*::IllegalOwner?

[t-nelson]

IllegalOwner should really be IllegalWithSeedOwner. that error was added to return when system_instruction::Instruction::CreateWithSeed addresses would collide with PDAs. there's an InstructionError::InvalidAccountOwner that's most appropriate and has been used for failed ownership checks elsewhere. there's no ProgramError analog today, but maybe that's the problem. afaik ProgramError is a somewhat dubious clone of InstructionError. not having InvalidAccountOwner represented there means the TryFrom<InstructionError> impl returns Err, which is probably a bug. probably add that in a separate pr, then use it here

[buffalojoec]

there's an InstructionError::InvalidAccountOwner that's most appropriate and has been used for failed ownership checks elsewhere. there's no ProgramError analog today, but maybe that's the problem. afaik ProgramError is a somewhat dubious clone of InstructionError. not having InvalidAccountOwner represented there means the TryFrom<InstructionError> impl returns Err, which is probably a bug. probably add that in a separate pr, then use it here

#33766

[buffalojoec]

Swapped *::IncorrectProgramId for *::InvalidAccountOwner!