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

feat(compiler): adds Aztec noir compiler #438

Merged
merged 20 commits into from
May 5, 2023
Merged

feat(compiler): adds Aztec noir compiler #438

merged 20 commits into from
May 5, 2023

Conversation

sirasistant
Copy link
Contributor

@sirasistant sirasistant commented May 3, 2023 •
edited
Loading

Description

Closes #392

Added a noir compiler CLI tool that allows to compile noir contracts using master noir_wasm and outputs aztec formatted ABIs.

Currently, this noir compiler uses noir master branch. It's not compatible yet with the test contracts for Aztec that are in the noir-contracts package, that need to be built following its README.md instructions.

Checklist:

  • I have reviewed my diff in github, line by line.
  • Every change is related to the PR description.
  • I have linked this pull request to the issue(s) that it resolves.
  • There are no unexpected formatting changes, superfluous debug logs, or commented-out code.
  • The branch has been merged or rebased against the head of its merge target.
  • I'm happy for the PR to be merged at the reviewer's next convenience.

@sirasistant sirasistant changed the title feat(compiler): adds Aztec noir compiler feat(compiler): adds Aztec noir compiler (WIP) May 3, 2023
@sirasistant
Copy link
Contributor Author

noir-compiler tests won't pass until noir-lang/noir#1281 is merged and I update the noir_wasm

@sirasistant sirasistant marked this pull request as ready for review May 5, 2023 07:56
@sirasistant sirasistant changed the title feat(compiler): adds Aztec noir compiler (WIP) feat(compiler): adds Aztec noir compiler May 5, 2023
spalladino
spalladino approved these changes May 5, 2023
View reviewed changes
Copy link
Collaborator

@spalladino spalladino left a comment

Choose a reason for hiding this comment

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

LGTM! I left a bunch of comments, none of them a dealbreaker.

Currently, this noir compiler uses noir master branch. It's not compatible yet with the test contracts for Aztec that are in the noir-contracts package, that need to be built following its README.md instructions.

What would it take to have this compiler use a different branch..?

yarn-project/noir-compiler/src/cli.ts Outdated Show resolved Hide resolved
yarn-project/noir-compiler/src/compile.ts Outdated Show resolved Hide resolved
yarn-project/noir-compiler/src/compile.ts Show resolved Hide resolved
yarn-project/noir-compiler/src/compile.ts Outdated Show resolved Hide resolved
yarn-project/noir-compiler/src/compile.ts
Comment on lines +100 to +103
return compile({
contracts: true,
optional_dependencies_set: Object.keys(dependenciesMap), // eslint-disable-line camelcase
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hold on, is the noirResolver a global object, and not an instance that we pass onto the compiler? Can we change that in noir? Global objects are always a pain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm afraid it is ): I'll check if it's possible to do in a different way on noir side, because it interacts with autogenerated code from wasm-bindgen

yarn-project/noir-compiler/src/compile.ts Show resolved Hide resolved
yarn-project/noir-compiler/src/compile.ts Show resolved Hide resolved
@LeilaWang LeilaWang merged commit 7542d85 into master May 5, 2023
@LeilaWang LeilaWang deleted the arv/typescript-compiler branch May 5, 2023 16:27
ludamad pushed a commit that referenced this pull request Jul 14, 2023
@kevaundray @zac-williamson
DSL: Add valid dummy data for ecdsa constraints when verifier is crea…
59fa8a1 
…ting circuit (#438)

* Add way to make verifiers data valid by replacing zeroes with valid public keys and signatures

Co-authored-by: Zachary James Williamson <zac-williamson@users.noreply.github.com>

* Update cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp

* replace templates with concrete methods

* add comment

* PR review

* add comments

* change to use boolean flag, so dummy_ecdsa method lives in ecdsa

* ad true as default

---------

Co-authored-by: Zachary James Williamson <zac-williamson@users.noreply.github.com>
This was referenced Jul 25, 2023
Closed
Closed
codygunton pushed a commit that referenced this pull request Jan 23, 2024
@kevaundray @zac-williamson
DSL: Add valid dummy data for ecdsa constraints when verifier is crea…
223b9de 
…ting circuit (#438)

* Add way to make verifiers data valid by replacing zeroes with valid public keys and signatures

Co-authored-by: Zachary James Williamson <zac-williamson@users.noreply.github.com>

* Update cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp

* replace templates with concrete methods

* add comment

* PR review

* add comments

* change to use boolean flag, so dummy_ecdsa method lives in ecdsa

* ad true as default

---------

Co-authored-by: Zachary James Williamson <zac-williamson@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spalladino spalladino spalladino approved these changes

@LeilaWang LeilaWang LeilaWang approved these changes

@ludamad ludamad Awaiting requested review from ludamad

@kevaundray kevaundray Awaiting requested review from kevaundray

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Compile Noir contract using noir_wasm
3 participants
@sirasistant @spalladino @LeilaWang