Skip to content

feat: Implement DAP protocol in Nargo#3627

Merged
TomAFrench merged 22 commits intonoir-lang:masterfrom
manastech:dap
Dec 19, 2023
Merged

feat: Implement DAP protocol in Nargo#3627
TomAFrench merged 22 commits intonoir-lang:masterfrom
manastech:dap

Conversation

@ggiraldez
Copy link
Contributor

@ggiraldez ggiraldez commented Nov 29, 2023

Description

Implements the DAP protocol to allow a tool such as VS.Code to drive the debugger in a Noir binary project. A separate PR for the VS.Code Noir extension will be submitted later.

Problem

Part of #3015

The beginning of this implementation was heavily inspired by @dmvict implementation of an alternative debugger for Noir programs.

Summary

This PR implements a new nargo subcommand dap. This starts a DAP server in single session mode through stdin/stdout and waits for a launch request. Using the arguments in the launch request (projectFolder, and optionally package and proverName) it compiles the Noir binary package and starts it in debug mode. Through DAP requests, a tool can then step through the program, set breakpoints and list the generated opcodes using a disassemble request.

Additional Context

Documentation

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [Exceptional Case] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@ggiraldez
feat: empty skeleton for dap command
3693eb6
@ggiraldez
feat: map errors from Dap into CliErrors and start basic server loop
01ad804
@ggiraldez
WIP
422d13f
@ggiraldez
WIP: next, continue and disassembly view
fcf43c1
@ggiraldez
feat: rewrite offset_opcode_location and add unit tests
cff989e
@ggiraldez
feat: render the circuit and Brillig opcodes for disassembled view
e1da132
@ggiraldez
chore: refactor dap_cmd for readability
5a1877c
@ggiraldez
chore: refactor, move DAP implementation into a new file
e3f3d4f
@ggiraldez
chore: extract stack trace and disassemble handlers
2fc41cb
@ggiraldez
chore: revert backend-interface crate rename
b0be45b
@ggiraldez
chore: remove unneeded dependency
9b1d251
@ggiraldez
feat: implement instruction and source breakpoints
a650368
@ggiraldez
feat: handle breakpoints correctly and emit events when they are reached
27cb858
@ggiraldez
feat: handle stepping granularity
588c681
@ggiraldez
chore: cargo fmt & clippy
8aa9327
@ggiraldez ggiraldez mentioned this pull request Nov 29, 2023
Merged
2 tasks
@TomAFrench TomAFrench mentioned this pull request Dec 12, 2023
Merged
5 tasks
TomAFrench
TomAFrench reviewed Dec 12, 2023
View reviewed changes
Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing functionality atm, raising a few surface things from an initial look at the code.

ggiraldez and others added 2 commits December 12, 2023 11:30
@ggiraldez @TomAFrench
Apply suggestions from code review
5315992 
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
@ggiraldez
chore: fix compilation errors and improve some comments
6a52ab2
github-merge-queue bot pushed a commit that referenced this pull request Dec 12, 2023
@TomAFrench
chore: move debugger tests in submodule (#3780)
ce57038 
# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

This PR just does some simple refactoring to make #3627's diff smaller.
Test functions are placed inside a submodule which then holds all of the
imports instead of them being in the functions themselves.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** 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.
@mverzilli mverzilli mentioned this pull request Dec 18, 2023
Closed
TomAFrench
TomAFrench approved these changes Dec 19, 2023
View reviewed changes
Copy link
Member

@TomAFrench TomAFrench left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested this and got it working in VSCode now. There's a couple of things we could probably handle better, e.g.

  • stepping into a function results in the debugger sending me to the relevant line in main rather than the deepest function which is what I would expect.
  • better automatic finding of the project root

These can get handled later however. Thanks for this and apologies for the delay!

@TomAFrench TomAFrench added this pull request to the merge queue Dec 19, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 19, 2023
@TomAFrench
Copy link
Member

TomAFrench commented Dec 19, 2023

Argh, sorry looks like there are some merge conflicts that have arisen.

@ggiraldez
Copy link
Contributor Author

ggiraldez commented Dec 19, 2023

Argh, sorry looks like there are some merge conflicts that have arisen.

No worries, I'll update to latest master.

@TomAFrench TomAFrench added this pull request to the merge queue Dec 19, 2023
Merged via the queue into noir-lang:master with commit 13834d4 Dec 19, 2023
@kevaundray kevaundray mentioned this pull request Dec 19, 2023
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jan 22, 2024
@kevaundray @TomAFrench
chore: Release Noir(0.23.0) (#3862)
5be9f9d 
🤖 I have created a release *beep* *boop*
---


<details><summary>0.23.0</summary>

## [0.23.0](v0.22.0...v0.23.0)
(2024-01-22)


### ⚠ BREAKING CHANGES

* Ban nested slices
([#4018](#4018))
* Breaking changes from aztec-packages
([#3955](#3955))
* Rename Arithmetic opcode to AssertZero
([#3840](#3840))
* remove circuit methods from noir_wasm
([#3869](#3869))

### Features

* Add `assert_max_bit_size` method to `Field`
([#4016](#4016))
([bc9a44f](bc9a44f))
* Add `noir-compiler` checks to `aztec_macros`
([#4031](#4031))
([420a5c7](420a5c7))
* Add a `--force` flag to force a full recompile
([#4054](#4054))
([27a8e68](27a8e68))
* Add dependency resolver for `noir_wasm` and implement `FileManager`
for consistency with native interface
([#3891](#3891))
([c29c7d7](c29c7d7))
* Add foreign call support to `noir_codegen` functions
([#3933](#3933))
([e5e52a8](e5e52a8))
* Add MVP `nargo export` command
([#3870](#3870))
([fbb51ed](fbb51ed))
* Add support for codegenning multiple functions which use the same
structs in their interface
([#3868](#3868))
([1dcfcc5](1dcfcc5))
* Added efficient field comparisons for bn254
([#4042](#4042))
([1f9cad0](1f9cad0))
* Assert maximum bit size when creating a U128 from an integer
([#4024](#4024))
([8f9c7e4](8f9c7e4))
* Avoid unnecessary range checks by inspecting instructions for casts
([#4039](#4039))
([378c18e](378c18e))
* Breaking changes from aztec-packages
([#3955](#3955))
([5be049e](5be049e))
* Bubble up `Instruction::Constrain`s to be applied as early as
possible. ([#4065](#4065))
([66f5cdd](66f5cdd))
* Cached LSP parsing
([#4083](#4083))
([b4f724e](b4f724e))
* Comparison for signed integers
([#3873](#3873))
([bcbd49b](bcbd49b))
* Decompose `Instruction::Cast` to have an explicit truncation
instruction ([#3946](#3946))
([35f18ef](35f18ef))
* Decompose `Instruction::Constrain` into multiple more basic
constraints ([#3892](#3892))
([51cf9d3](51cf9d3))
* Docker testing flow
([#3895](#3895))
([179c90d](179c90d))
* Extract parsing to its own pass and do it in parallel
([#4063](#4063))
([569cbbc](569cbbc))
* Implement `Eq` trait on curve points
([#3944](#3944))
([abf751a](abf751a))
* Implement DAP protocol in Nargo
([#3627](#3627))
([13834d4](13834d4))
* Implement generic traits
([#4000](#4000))
([916fd15](916fd15))
* Implement Operator Overloading
([#3931](#3931))
([4b16090](4b16090))
* **lsp:** Cache definitions for goto requests
([#3930](#3930))
([4a2140f](4a2140f))
* **lsp:** Goto global
([#4043](#4043))
([15237b3](15237b3))
* **lsp:** Goto struct member inside Impl method
([#3918](#3918))
([99c2c5a](99c2c5a))
* **lsp:** Goto trait from trait impl
([#3956](#3956))
([eb566e2](eb566e2))
* **lsp:** Goto trait method declaration
([#3991](#3991))
([eb79166](eb79166))
* **lsp:** Goto type alias
([#4061](#4061))
([dc83385](dc83385))
* **lsp:** Goto type definition
([#4029](#4029))
([8bb4ddf](8bb4ddf))
* **lsp:** Re-add code lens feature with improved performance
([#3829](#3829))
([8f5cd6c](8f5cd6c))
* Optimize array ops for arrays of structs
([#4027](#4027))
([c9ec0d8](c9ec0d8))
* Optimize logic gate ACIR-gen
([#3897](#3897))
([926460a](926460a))
* Prefer `AcirContext`-native methods for performing logic operations
([#3898](#3898))
([0ec39b8](0ec39b8))
* Remove range constraints from witnesses which are constrained to be
constants ([#3928](#3928))
([afe9c7a](afe9c7a))
* Remove truncation from brillig casts
([#3997](#3997))
([857ff97](857ff97))
* Remove truncations which can be seen to be noops using type
information ([#3953](#3953))
([cc3c2c2](cc3c2c2))
* Remove unnecessary predicate from `Lt` instruction
([#3922](#3922))
([a63433f](a63433f))
* Simplify chains of casts to be all in terms of the original `ValueId`
([#3984](#3984))
([2384d3e](2384d3e))
* Simplify multiplications by `0` or `1` in ACIR gen
([#3924](#3924))
([e58844d](e58844d))
* Support for u128
([#3913](#3913))
([b4911dc](b4911dc))
* Support printing more types
([#4071](#4071))
([f5c4632](f5c4632))
* Sync `aztec-packages`
([#4011](#4011))
([fee2452](fee2452))
* Sync commits from `aztec-packages`
([#4068](#4068))
([7a8f3a3](7a8f3a3))
* Use singleton `WasmBlackBoxFunctionSolver` in `noir_js`
([#3966](#3966))
([10b28de](10b28de))


### Bug Fixes

* Acir gen doesn't panic on unsupported BB function
([#3866](#3866))
([34fd978](34fd978))
* Allow abi encoding arrays of structs from JS
([#3867](#3867))
([9b713f8](9b713f8))
* Allow abi encoding tuples from JS
([#3894](#3894))
([f7fa181](f7fa181))
* Allow ast when macro errors
([#4005](#4005))
([efccec3](efccec3))
* Allow lsp to run inside of a docker container
([#3876](#3876))
([2529977](2529977))
* Bit-shifts for signed integers
([#3890](#3890))
([6ddd98a](6ddd98a))
* Checks for cyclic dependencies
([#3699](#3699))
([642011a](642011a))
* **debugger:** Crash when stepping through locations spanning multiple
lines ([#3920](#3920))
([223e860](223e860))
* Don't fail if no tests and the user didn't provide a pattern
([#3864](#3864))
([decbd0f](decbd0f))
* Fix advisory issue in cargo-deny
([#4077](#4077))
([19baea0](19baea0))
* Fixing dark mode background on the CTA button
([#3882](#3882))
([57eae42](57eae42))
* Fixup exports from `noir_wasm`
([#4022](#4022))
([358cdd2](358cdd2))
* Handle multiple imports in the same file
([#3903](#3903))
([219423e](219423e))
* Hoist constraints on inputs to top of program
([#4076](#4076))
([447aa34](447aa34))
* Implement missing codegen for `BlackBoxFunc::EcdsaSecp256r1` in
brillig ([#3943](#3943))
([2c5eceb](2c5eceb))
* Improve `nargo test` output
([#3973](#3973))
([3ab5ff4](3ab5ff4))
* Make `constant_to_radix` emit a slice instead of an array
([#4049](#4049))
([5cdb1d0](5cdb1d0))
* Operator overloading & static trait method references resolving to
generic impls ([#3967](#3967))
([f1de8fa](f1de8fa))
* Preserve brillig entrypoint functions without arguments
([#3951](#3951))
([1111465](1111465))
* Prevent `Instruction::Constrain`s for non-primitive types
([#3916](#3916))
([467948f](467948f))
* Remove panic for adding an invalid crate name in wasm compiler
([#3977](#3977))
([7a1baa5](7a1baa5))
* Return error rather instead of panicking on invalid circuit
([#3976](#3976))
([67201bf](67201bf))
* Search all levels of struct nesting before codegenning primitive types
([#3970](#3970))
([13ae014](13ae014))
* Update generics docs to mention we have traits now
([#3980](#3980))
([c2acdf1](c2acdf1))


### Miscellaneous Chores

* Ban nested slices
([#4018](#4018))
([f8a1fb7](f8a1fb7))
* Remove circuit methods from noir_wasm
([#3869](#3869))
([12d884e](12d884e))
* Rename Arithmetic opcode to AssertZero
([#3840](#3840))
([836f171](836f171))
</details>

<details><summary>0.39.0</summary>

## [0.39.0](v0.38.0...v0.39.0)
(2024-01-22)


### ⚠ BREAKING CHANGES

* Breaking changes from aztec-packages
([#3955](#3955))
* Rename Arithmetic opcode to AssertZero
([#3840](#3840))
* Remove unused methods on ACIR opcodes
([#3841](#3841))
* Remove partial backend feature
([#3805](#3805))

### Features

* Aztec-packages
([#3754](#3754))
([c043265](c043265))
* Breaking changes from aztec-packages
([#3955](#3955))
([5be049e](5be049e))
* Remove range constraints from witnesses which are constrained to be
constants ([#3928](#3928))
([afe9c7a](afe9c7a))
* Speed up transformation of debug messages
([#3815](#3815))
([2a8af1e](2a8af1e))
* Sync `aztec-packages`
([#4011](#4011))
([fee2452](fee2452))
* Sync commits from `aztec-packages`
([#4068](#4068))
([7a8f3a3](7a8f3a3))


### Bug Fixes

* Deserialize odd length hex literals
([#3747](#3747))
([4000fb2](4000fb2))
* Return error rather instead of panicking on invalid circuit
([#3976](#3976))
([67201bf](67201bf))


### Miscellaneous Chores

* Remove partial backend feature
([#3805](#3805))
([0383100](0383100))
* Remove unused methods on ACIR opcodes
([#3841](#3841))
([9e5d0e8](9e5d0e8))
* Rename Arithmetic opcode to AssertZero
([#3840](#3840))
([836f171](836f171))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: TomAFrench <tom@tomfren.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TomAFrench TomAFrench TomAFrench approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ggiraldez @TomAFrench