Fix dispatch for multisig and other cases

[JuanCoRo]

This PR is the product of a joint investigation with @dkcumming.
The inner_process_instruction function(s) incorrectly dispatch the cases for the owner being an account vs. a multisig.

In the way it was done before we had:

1. If the data field of the owner had Account::LEN length, then it would dispatch it assuming cheatcode_is_account(owner)
2. If the data field of the owner had Multisig::LEN length, then it would dispatch it assuming cheatcode_is_multisig(owner)
3. Otherwise it would panic because of malformed data.

Turns out, this is not correct on how ownership works. Anything able to cast a signature can be an owner. This means that an account with no data can also be an owner. Upon closer inspection to the validate owner function, the only data requirement is for the multisig case.

Therefore, the correct approach seems to be:

1. Check if the owner has Multisig::LEN length and is owned by P-Token TOKEN_PROGRAM_ID. Dispatch it to the multisig case if so, assuming cheatcode_is_multisig(owner)
2. If the above fails, forward the old "account" case, this time without assuming cheatcode_is_account(owner). If the owner is to fail, it should do so in the test, not in the dispatcher.

The problem with dispatching through the multisig case when the account's data length is Multisig::LEN but it's not owned by P-Token is that it might just be a coincidence that the data length matches. This particular case makes the concrete tests crash.

Testing: The changes introduced in this PR have successfully passed the concrete test suites for P-Token and SPL. Refer to this branch for the modifications needed to run the concrete tests.

[jberthold]

Makes sense - if we don't use the accounts[2] in the non-multisig case, we should not even set it up. However this will cause trouble in the inner_test_validate_owner function which will explore the "multisig" branch.
We may have to use a different owner validation function for the "account" case which avoids accessing data which is not present.

[jberthold]

The accounts[2] is passed as owner_account_info into inner_test_validate_owner. Inside that function, there is a branch for the multisig case which accesses the account data.
So if we aren't setting up any data via cheat code, this branch will be explored in the "account" case, too, because it may well be that the length matches the "multisig", the data_len and the owner key are just variables. Without setting up data, that branch will access data that is not set up (undefined behaviour).

[dkcumming]

I see what you mean, I think that means we need some other cheatcode for the authority to remove this case, cheatcode_is_not_multisig since that is the only restriction on that AccountInfo

[dkcumming]

Is that as reasonable as the cheatcode_is_multisig?

[dkcumming]

Although it might be easier to just separate the Multisig block as you said when validating

[JuanCoRo]

An easy fix for this could be this.
maybe_multisig_is_initialised is always None for non-multisig cases. So I've added that to the path condition to check for the multisig case.

[dkcumming]

Oh that fix is much better than what I did, I like that.

[dkcumming]

@JuanCoRo I tested it concretely and it passed both p-token and program test suites (with the expected malformed test removed) - see on this branch

[JuanCoRo]

Just tested this concretely with the latest changes and it's passing both test suites. See this branch for the modifications.

[dkcumming]

Nice I think this is good to go