program: use counter to access accounts list

[buffalojoec]

Problem

When initializing a config account, the config account is expected to be a signer. However, if it's also included in the signer list as a required signer, the loop where signer keys are checked will fail, telling the user that the account is missing. This failure case also arises when updating a config account, and attempting to declare that the config account must be a signer for updates.

The account isn't actually missing, it's just the fact that the if signer != config_account_key is being evaluated after the counter is advanced, so the rest of the loop is off by one.
https://github.com/anza-xyz/agave/blob/d9c24ed5837407aa411fbb7cc881c4a1a9757f5e/programs/config/src/config_processor.rs#L61-L72

In the BPF version, we properly handle this case by only advancing the accounts_iter when the list of accounts is actually accessed.

In my opinion, this seems like a bug with the builtin implementation, not the BPF version. It's possible that maybe the program isn't designed to allow you to include the config account as a required signer, but that seems like an unnecessary constraint.

[buffalojoec]

@joncinque I'm leaving this as a draft for now, but let me know what you think about my findings detailed above. I've also included a test with comments to demonstrate what I'm talking about.

[joncinque]

Nice catch! For the first BPF version of these programs, I think we should stick with replicating all the behavior of the native program, unless it's an obvious bug. We can change it in future work

[buffalojoec]

OK, I rebased with real commits this time, so it can come out of draft, and ready for another look.

Side note: the compute unit profiling actually helped me catch a bug! The ok_or method is eagerly evaluated, so it was printing the log message every time and pumping up compute usage. Instead, I went with ok_or_else, which is lazily evaluated, and solves the issue. CU reduction!

[joncinque]

The ok_or method is eagerly evaluated, so it was printing the log message every time and pumping up compute usage. Instead, I went with ok_or_else, which is lazily evaluated, and solves the issue. CU reduction!

That's very interesting! We should definitely keep that in mind for optimized programs