Don't verify proofs for failed zkApp commands #12550
Conversation
|
!ci-build-me |
|
!ci-build-me |
| User_command.to_verifiable ~ledger ~get ~location_of_account cmd | ||
| |> Or_error.ok_exn | ||
| in | ||
| { With_status.data = cmd; status } ) ) |
There was a problem hiding this comment.
This won't be enough in the case of failed transactions where vk hashes in account updates don't match with what's in the ledger or in the same transaction, right? Zkapp_command.Verifiable.create (called by to_verifiable here) will fail to find vk hashes, cause a failure here and the block will be rejected
There was a problem hiding this comment.
Good catch. I've added an optional allow_missing_vk argument to create that should prevent this from triggering.
Sounds like we probably also need a test; I'll put something together later.
|
!ci-build-me |
|
!ci-build-me |
src/lib/mina_base/zkapp_command.ml
Outdated
| @@ -1185,34 +1191,43 @@ end = struct | |||
| return err | |||
| in | |||
| if Control.(Tag.equal Tag.Proof (Control.tag p.authorization)) | |||
| then ( | |||
| then | |||
There was a problem hiding this comment.
Would it be better Applied|Failed check is performed before entering this block? That way none of VKs are included. Currently there could be some VKs included and some not and that might cause confusion.
…-verification-when-failed
|
!ci-build-me |
…-verification-when-failed
|
!ci-build-me |
|
!ci-build-me |
|
!ci-build-me |
This PR builds upon #12490, skipping the verification of proofs inside failed zkApp commands.
Checklist: