0xPolygonMiden / miden-vm

STARK-based virtual machine
MIT License
612 stars 150 forks source link

Implement error codes for the `mtree_verify` instructions #1328

Closed Fumuran closed 1 month ago

Fumuran commented 2 months ago

This PR implements the ability to specify error codes for mtree_verify instruction the same way it is done for assert instructions.

TODO:

Related issue: https://github.com/0xPolygonMiden/miden-vm/issues/1264

Fumuran commented 1 month ago

We can add a Host handler for Merkle tree errors exactly the same as we do it for Assert now, with handler like on_merkle_tree_error. I think it makes sense to use it for all Merkle-tree related instructions: for now we have only mtree_verify, but probably other instructions eventually will have error codes as well.

By the way, I noticed that we call on_assert_failed Host function only for Assert(err) operation, but not for U32assert2(err). Formally it is also an assert operation, although it is used in a slightly different way. Should we leave it as it is?

bobbinth commented 1 month ago

By the way, I noticed that we call on_assert_failed Host function only for Assert(err) operation, but not for U32assert2(err). Formally it is also an assert operation, although it is used in a slightly different way. Should we leave it as it is?

Ah - I didn't realize this was happening. Let's fix this (in a different PR). The one question would be is whether we can re-use the same function on_assert_failed() since now we need to return a different error variant, I believe. Or would we need to add another handler to the host.

In my mind, adding another function to the host is potentially too much because we'd end up with something like on_assert_failed, on_u32_assert_failed, on_merkle_tree_error. Maybe we have a single function in the host and specify the type of the assertion via an additional enum parameter (and this would include regular assert, u32assert2, and mtree_verify).

Let's try to solve this in a follow up PR.