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.

Sign up for GitHub

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

Jump to bottom

Debugger: Handle BrilligCalls in the Debugger #4823

Closed
vezenovm opened this issue Apr 16, 2024 · 1 comment · Fixed by #4897
Closed

Debugger: Handle BrilligCalls in the Debugger #4823

vezenovm opened this issue Apr 16, 2024 · 1 comment · Fixed by #4897
Labels
debugger enhancement New feature or request

Comments

@vezenovm
Copy link
Contributor

vezenovm commented Apr 16, 2024 •
edited
Loading

Problem

After this PR AztecProtocol/aztec-packages#5737 which resolves #3907 we now support execution of a BrilligCall opcode that contains an ID to a Brillig function rather than inlining the Brillig bytecode into the opcode itself. This requires some execution changes that also affect the debugger.

cc @ggiraldez @mverzilli

Happy Case

The debugger should support BrilligCall as it supports working with Brillig right now. Once we fully remove Brillig the debugger should still work well. A specific location that comes to mind is the step_into_brillig_opcode will need to be updated similarly to how solve_brillig_call_opcode was updated from the solve_brillig_opcode method. For the most part the execution changes are quite minor, but just that we need to fetch the appropriate Brillig bytecode rather than having it as part of the ACIR opcode itself.

Project Impact

Nice-to-have

Impact Context

The debugger will be broken on compiler versions that have switched to BrilligCall execution.

Workaround

None

Workaround Description

No response

Additional Context

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response
@vezenovm vezenovm added enhancement New feature or request debugger labels Apr 16, 2024
@mverzilli
Copy link
Contributor

Thanks for the context @vezenovm, we're looking into it!

@ggiraldez ggiraldez mentioned this issue Apr 23, 2024
Merged
5 tasks
@vezenovm vezenovm mentioned this issue Apr 24, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Apr 24, 2024
@ggiraldez @vezenovm
feat: Handle BrilligCall opcodes in the debugger (#4897)
b380dc4 
# Description

## Problem\*

Resolves #4823 

With the introduction of the `BrilligCall` opcode to deduplicate the
Brillig bytecode instead of inlining it into `Brillig` opcodes, the
debugger was not being able to correctly step into the execution of
unconstrained functions.

## Summary\*

This PR adds support to the Noir debugger to handle the new
`BrilligCall` opcodes and allows stepping into unconstrained functions
by retrieving the opcodes from the deduplicated Brillig functions. The
changes impact both the REPL and the DAP debuggers.

## Additional Context

In DAP mode, the disassembly request is still not working properly. It
seems that we are not responding to the command in an appropriate
manner. We will tackle that problem separately.

## Documentation\*

Check one:
- [X] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [X] I have tested the changes locally.
- [X] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
debugger enhancement New feature or request
Projects
Noir
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: Handle BrilligCall opcodes in the debugger manastech/noir
2 participants
@mverzilli @vezenovm