forked from solana-program/token
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit b453429
authored
Fix dispatch for multisig and other cases (#92)
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](https://github.com/runtimeverification/solana-token/blob/proofs/p-token/src/processor/mod.rs#L106-#L107),
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](https://github.com/runtimeverification/solana-token/tree/dc/concrete-test-fix-dispatch-multisig)
for the modifications needed to run the concrete tests.1 parent 19467e1 commit b453429Copy full SHA for b453429
File tree
Expand file treeCollapse file tree
1 file changed
+87
-104
lines changedOpen diff view settings
Filter options
- p-token/src
Expand file treeCollapse file tree
1 file changed
+87
-104
lines changedOpen diff view settings
0 commit comments