miden-vm icon indicating copy to clipboard operation
miden-vm copied to clipboard

mtree_verify should also accept err

Open hackaugusto opened this issue 11 months ago • 3 comments

The mtree_verify instruction, like the assert instructions, will fail the execution if its criteria fails. This should also support an error code.

hackaugusto avatar Mar 01 '24 15:03 hackaugusto

I'm not sure I understand the purpose of adding an error codes to the mtree_verify. We decided to add error codes to the assert instructions because they were returning just a FailedAssertion error and a cycle number without additional data, so it was difficult to find out what exactly caused an error. But in case of mtree_verify we have a specific error which contains necessary data about Merkle Tree and a value being checked. I think I just missing something, @hackaugusto can you describe in more detail why this is necessary?

Fumuran avatar Mar 06 '24 17:03 Fumuran

I think the main rationale is that mtree_verify acts kind of like an assert instruction. That is, if a Merkle path cannot be verified, the execution fails. These failures can happen in different contexts (e.g., asset is not present in account vault) and it would be helpful to let users specify their own error codes.

bobbinth avatar Mar 06 '24 18:03 bobbinth

The motivation for the issue was the usage of mtree_verify in miden-base. There are two uses and having an error will make it clearer which one caused an issue (also for other errors)

In general, I think every instruction that can fail should have an error to make debugging easier. My ideal situation would be to have error code to every instruction that can fail. So that when I get a failure in a test, I can just search for the error code and find exactly the instruction that failed.

hackaugusto avatar Mar 07 '24 14:03 hackaugusto

Closed by #1328.

bobbinth avatar Jun 06 '24 22:06 bobbinth