Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect behavior of caller instruction for dynamic calls #1478

Open
bobbinth opened this issue Sep 4, 2024 · 0 comments
Open

Incorrect behavior of caller instruction for dynamic calls #1478

bobbinth opened this issue Sep 4, 2024 · 0 comments
Labels
air Related to Miden AIR and constraints bug Something isn't working processor Related to Miden VM processor

Comments

@bobbinth
Copy link
Contributor

bobbinth commented Sep 4, 2024

Currently, dyncall instruction translates into two operations in the VM: call dyn. This works mostly fine, but it does result in caller instruction returning an unintuitive (and probably wrong) value.

Specifically, the caller instruction should return the MAST root of the context we are currently in. So, if we do something like:

proc.foo
    caller
    # => [MAST_ROOT_OF_FOO, ...]
end

begin
    call.foo
end

But, if we use dyncall instruction, the result will be different:

proc.foo
    caller
    # => [HASH_DYN_NODE, ...]
end

begin
    procref.foo
    dyncall
end

This is because when we execute the call instruction, we set the MAST root of the new context to the hash of the callee. In case of dyncall, the callee is just a single dyn node - so, that's what gets used to set the MAST root.

Off the top of my head, there are two ways to fix this:

  1. Update the semantics of the call operation to check if the next operation is dyn, and if so, use the top of the stack to set the MAST root. This may result in pretty complicated constraints - so, not sure if it will be worth it.
  2. Introduce a new dedicate operation for dyncall. This will behave basically like call operation except it will use the top of the stack to set the MAST root of the context. I think dyncall will need to be a "very high degree" operation, and so we can do this only after we free up one of such operation slots (i.e., after #1457).
@bobbinth bobbinth added bug Something isn't working processor Related to Miden VM processor air Related to Miden AIR and constraints labels Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
air Related to Miden AIR and constraints bug Something isn't working processor Related to Miden VM processor
Projects
None yet
Development

No branches or pull requests

1 participant