-
Notifications
You must be signed in to change notification settings - Fork 586
Description
The zkApp transaction logic has a fallback in the verification key permission handling for when the transaction version is lower than the current version. But, it doesn’t have a fallback for the access level. Florian suggested that the access should be set to signature as well in this situation.
This is the access check, with no apparent fallback related to the transaction version:
mina/src/lib/transaction_logic/zkapp_command_logic.ml
Lines 1549 to 1556 in 11388aa
| (* Check that the account can be accessed with the given authorization. *) | |
| let local_state = | |
| let has_permission = | |
| Controller.check ~proof_verifies ~signature_verifies | |
| (Account.Permissions.access a) | |
| in | |
| Local_state.add_check local_state Update_not_permitted_access | |
| has_permission |
and this is the verification key permission check, with the transaction version fallback.
mina/src/lib/transaction_logic/zkapp_command_logic.ml
Lines 1617 to 1628 in 11388aa
| let older_than_current_version = | |
| Txn_version.older_than_current | |
| (Account.Permissions.set_verification_key_txn_version a) | |
| in | |
| let original_auth = Account.Permissions.set_verification_key_auth a in | |
| let auth = | |
| Controller.if_ older_than_current_version | |
| ~then_: | |
| (Controller | |
| .verification_key_perm_fallback_to_signature_with_older_version | |
| original_auth ) | |
| ~else_:original_auth |
Deepthi believes that the logic in question can be modified (in advance of the hard fork, I think?) so that we apply an access fallback in the same older_than_current_version circumstances.
We should probably write a test to cover this situation first, to confirm that we don't do the right thing here - the one for the verification key fallback appears to be in the hard fork test executive. That can be merged at the same time (or before, if we make it soft fail) as the fix for this behaviour.
Metadata
Metadata
Assignees
Labels
Type
Projects
Status