-
Notifications
You must be signed in to change notification settings - Fork 268
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
feat: compile noir contracts with noir_wasm #2737
Conversation
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
Running the compiler on this branch is not going to work until a newer version of |
yarn-project/noir-compiler/src/compile/noir/noir-wasm-compiler.ts
Outdated
Show resolved
Hide resolved
yarn-project/noir-compiler/src/compile/noir/noir-wasm-compiler.ts
Outdated
Show resolved
Hide resolved
yarn-project/noir-compiler/src/compile/noir/noir-wasm-compiler.ts
Outdated
Show resolved
Hide resolved
87c8d75
to
31835f1
Compare
@@ -20,6 +25,7 @@ export function compileContract(program: Command, name = 'contract', log: LogFn | |||
.option('-o, --outdir <path>', 'Output folder for the binary artifacts, relative to the project path', 'target') | |||
.option('-ts, --typescript <path>', 'Optional output folder for generating typescript wrappers', undefined) | |||
.option('-i, --interface <path>', 'Optional output folder for generating an Aztec.nr contract interface', undefined) | |||
.option('--compiler <compiler>', 'Compiler to use (nargo|wasm)', 'nargo') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the default is nargo, does this mean the aztec-cli will use nargo? I can't see a change in the aztec-cli.
515ea33
to
80aff83
Compare
d8a0e9e
to
ff3c8ed
Compare
ff3c8ed
to
6d0fba2
Compare
I'm investigating why 4 end to end tests are currently failing with |
My last commit undid a bunch of changes:
I've done this because noir_wasm doesn't output debug information yet so it would make fixing issues in contracts difficult. The wasm compiler can still be used if needed (will be used in dockerized aztec-cli, see #3031). Plan going forward:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work! Left a bunch of comments.
|
||
async #recursivelyResolveDependencies(packageName: string, noirPackage: NoirPackage): Promise<void> { | ||
for (const [name, config] of Object.entries(noirPackage.getDependencies())) { | ||
// TODO what happens if more than one package has the same name but different versions? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd blow up loudly, rather than silently pick either of them. Though you'd probably need to use semver to collect all version restrictions and get their intersection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I thought about it but noir libs don't have a version number in Nargo.toml
and because of this is makes comparing git dependencies to relative-path dependencies difficult. (e.g. contract depends on aztec and authwit via git, authwit depends on aztec via relative path)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we then just use the git revision, and if we have a relative path, we "inherit" the git revision of the parent that included it? That should let us blow up if we see two different git revisions requested.
yarn-project/noir-compiler/src/compile/noir/dependencies/github-dependency-resolver.ts
Outdated
Show resolved
Hide resolved
yarn-project/noir-compiler/src/compile/noir/dependencies/github-dependency-resolver.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that nargo has a concept of workspace (see here). Is this something we'd need to manually support from wasm as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nargo does support workspaces but this wrapper around it didn't advertise that functionality. The wasm compiler doesn't support workspaces but we could do it in node.js but it would be pretty slow: we'd have to compile each contract serially because the wasm code uses a global function to resolve file contents (or worker threads? 👀)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's open a new issue to track "workspace support" then, given it's not a current feature that we lose because of the switch to wasm, but a new thing to implement.
…b-dependency-resolver.ts Co-authored-by: Santiago Palladino <santiago@aztecprotocol.com>
…b-dependency-resolver.ts Co-authored-by: Santiago Palladino <santiago@aztecprotocol.com>
Co-authored-by: Santiago Palladino <santiago@aztecprotocol.com>
This PR dockerizes aztec-cli. It requires #2737 to be merged in first. Fix #2875 # Checklist: Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge. - [ ] If the pull request requires a cryptography review (e.g. cryptographic algorithm implementations) I have added the 'crypto' tag. - [x] I have reviewed my diff in github, line by line and removed unexpected formatting changes, testing logs, or commented-out code. - [x] Every change is related to the PR description. - [x] I have [linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue) this pull request to relevant issues (if any exist).
🤖 I have created a release *beep* *boop* --- <details><summary>aztec-packages: 0.13.0</summary> ## [0.13.0](aztec-packages-v0.12.0...aztec-packages-v0.13.0) (2023-10-31) ### ⚠ BREAKING CHANGES * PXE.getNotes(...) + refactor of note types ([#3051](#3051)) ### Features * `FieldNote` ([#3037](#3037)) ([3d1ffd0](3d1ffd0)) * Add Aztec Boxes page to docs ([#2569](#2569)) ([997c15c](997c15c)) * Adding structure to Transcript ([#2937](#2937)) ([db67aa1](db67aa1)) * Compile noir contracts with noir_wasm ([#2737](#2737)) ([524cecf](524cecf)) * Dockerize aztec-cli ([#3031](#3031)) ([ec2e3c2](ec2e3c2)) * Efficient ZM quotient computation ([#3016](#3016)) ([ebda5fc](ebda5fc)) * **feature_branch:** Private Kernel Circuit ([#2740](#2740)) ([f800a36](f800a36)) * Measure plonk rounds ([#3065](#3065)) ([c8e1d8b](c8e1d8b)) * Migrate the init kernel CPP tests to noir ([#3091](#3091)) ([906429f](906429f)) * New script to output table of benchmarks for README pasting. ([#2780](#2780)) ([6c20b45](6c20b45)) * Pedersen in typescript. ([#3111](#3111)) ([933f1b2](933f1b2)) * Protogalaxy folding of challenges ([#2935](#2935)) ([7ed30e8](7ed30e8)) * PXE.getNotes(...) + refactor of note types ([#3051](#3051)) ([16abb5a](16abb5a)) * Zeromorph with concatenation (Goblin Translator part 10) ([#3006](#3006)) ([70b0f17](70b0f17)) ### Bug Fixes * Bad contract txs publishing contract data ([#2673](#2673)) ([ccd4611](ccd4611)) * Better error message for compute_note_hash_and_nullifier. ([#3097](#3097)) ([57bec53](57bec53)) * Broken `FieldNote` test ([#3135](#3135)) ([fe78ecf](fe78ecf)) * Docker-compose up, rather than run. ([#3081](#3081)) ([242f780](242f780)) * Formatting ([#3070](#3070)) ([e1633d3](e1633d3)) * Minor stale naming fix ([#3117](#3117)) ([a6786ae](a6786ae)) * Push cli docker image to docker hub ([#3120](#3120)) ([ccad50f](ccad50f)) * Remove duplicate terraform resource definition ([#3066](#3066)) ([d5abadb](d5abadb)) * Retry request spot ([#3116](#3116)) ([82de5f1](82de5f1)) ### Miscellaneous * Add stdlib tests for pedersen commitment ([#3075](#3075)) ([87fa621](87fa621)) * Automatic c_binds for commit should return a point instead of an Fr element ([#3072](#3072)) ([2e289a5](2e289a5)) * Cleanup remaining mentions of `compress` with pedersen in cpp and ts ([#3074](#3074)) ([52cf383](52cf383)) * E2e on spots [ci rebuild] ([#3068](#3068)) ([15db6bf](15db6bf)) * Fix dapp_testing e2e race condition ([#3094](#3094)) ([89e7c21](89e7c21)) * Remove docs mirror ([#3122](#3122)) ([3fa51e2](3fa51e2)) * Remove endomorphism coefficient from ecc_add_gate ([#3115](#3115)) ([d294987](d294987)) * Remove unecessary calls to `pedersen__init` ([#3079](#3079)) ([84f8db2](84f8db2)) * Remove unused pedersen c_binds ([#3058](#3058)) ([e71e5f9](e71e5f9)) * Removes pedersen commit native pairs method ([#3073](#3073)) ([69a34c7](69a34c7)) * Rename private-kernel subpackage to protocol-circuits ([#3134](#3134)) ([3e07104](3e07104)) ### Documentation * Initial keys spec ([#3035](#3035)) ([4b24c58](4b24c58)) </details> <details><summary>barretenberg.js: 0.13.0</summary> ## [0.13.0](barretenberg.js-v0.12.0...barretenberg.js-v0.13.0) (2023-10-31) ### Features * New script to output table of benchmarks for README pasting. ([#2780](#2780)) ([6c20b45](6c20b45)) ### Miscellaneous * Automatic c_binds for commit should return a point instead of an Fr element ([#3072](#3072)) ([2e289a5](2e289a5)) * Remove unecessary calls to `pedersen__init` ([#3079](#3079)) ([84f8db2](84f8db2)) * Remove unused pedersen c_binds ([#3058](#3058)) ([e71e5f9](e71e5f9)) </details> <details><summary>barretenberg: 0.13.0</summary> ## [0.13.0](barretenberg-v0.12.0...barretenberg-v0.13.0) (2023-10-31) ### Features * Adding structure to Transcript ([#2937](#2937)) ([db67aa1](db67aa1)) * Efficient ZM quotient computation ([#3016](#3016)) ([ebda5fc](ebda5fc)) * Measure plonk rounds ([#3065](#3065)) ([c8e1d8b](c8e1d8b)) * New script to output table of benchmarks for README pasting. ([#2780](#2780)) ([6c20b45](6c20b45)) * Pedersen in typescript. ([#3111](#3111)) ([933f1b2](933f1b2)) * Protogalaxy folding of challenges ([#2935](#2935)) ([7ed30e8](7ed30e8)) * Zeromorph with concatenation (Goblin Translator part 10) ([#3006](#3006)) ([70b0f17](70b0f17)) ### Miscellaneous * Add stdlib tests for pedersen commitment ([#3075](#3075)) ([87fa621](87fa621)) * Automatic c_binds for commit should return a point instead of an Fr element ([#3072](#3072)) ([2e289a5](2e289a5)) * Cleanup remaining mentions of `compress` with pedersen in cpp and ts ([#3074](#3074)) ([52cf383](52cf383)) * Remove endomorphism coefficient from ecc_add_gate ([#3115](#3115)) ([d294987](d294987)) * Remove unecessary calls to `pedersen__init` ([#3079](#3079)) ([84f8db2](84f8db2)) * Remove unused pedersen c_binds ([#3058](#3058)) ([e71e5f9](e71e5f9)) * Removes pedersen commit native pairs method ([#3073](#3073)) ([69a34c7](69a34c7)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
In #2737 noir_wasm was added back for contract compilation but kept as a secondary option to Nargo because at the time noir_wasm did not output debug info. This PR switches the default to noir_wasm because it now supports everything we need for contract compilation # Checklist: Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge. - [ ] If the pull request requires a cryptography review (e.g. cryptographic algorithm implementations) I have added the 'crypto' tag. - [x] I have reviewed my diff in github, line by line and removed unexpected formatting changes, testing logs, or commented-out code. - [x] Every change is related to the PR description. - [ ] I have [linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue) this pull request to relevant issues (if any exist).
🤖 I have created a release *beep* *boop* --- <details><summary>aztec-packages: 0.13.0</summary> ## [0.13.0](AztecProtocol/aztec-packages@aztec-packages-v0.12.0...aztec-packages-v0.13.0) (2023-10-31) ### ⚠ BREAKING CHANGES * PXE.getNotes(...) + refactor of note types ([#3051](AztecProtocol/aztec-packages#3051)) ### Features * `FieldNote` ([#3037](AztecProtocol/aztec-packages#3037)) ([3d1ffd0](AztecProtocol/aztec-packages@3d1ffd0)) * Add Aztec Boxes page to docs ([#2569](AztecProtocol/aztec-packages#2569)) ([997c15c](AztecProtocol/aztec-packages@997c15c)) * Adding structure to Transcript ([#2937](AztecProtocol/aztec-packages#2937)) ([db67aa1](AztecProtocol/aztec-packages@db67aa1)) * Compile noir contracts with noir_wasm ([#2737](AztecProtocol/aztec-packages#2737)) ([524cecf](AztecProtocol/aztec-packages@524cecf)) * Dockerize aztec-cli ([#3031](AztecProtocol/aztec-packages#3031)) ([ec2e3c2](AztecProtocol/aztec-packages@ec2e3c2)) * Efficient ZM quotient computation ([#3016](AztecProtocol/aztec-packages#3016)) ([ebda5fc](AztecProtocol/aztec-packages@ebda5fc)) * **feature_branch:** Private Kernel Circuit ([#2740](AztecProtocol/aztec-packages#2740)) ([f800a36](AztecProtocol/aztec-packages@f800a36)) * Measure plonk rounds ([#3065](AztecProtocol/aztec-packages#3065)) ([c8e1d8b](AztecProtocol/aztec-packages@c8e1d8b)) * Migrate the init kernel CPP tests to noir ([#3091](AztecProtocol/aztec-packages#3091)) ([906429f](AztecProtocol/aztec-packages@906429f)) * New script to output table of benchmarks for README pasting. ([#2780](AztecProtocol/aztec-packages#2780)) ([6c20b45](AztecProtocol/aztec-packages@6c20b45)) * Pedersen in typescript. ([#3111](AztecProtocol/aztec-packages#3111)) ([933f1b2](AztecProtocol/aztec-packages@933f1b2)) * Protogalaxy folding of challenges ([#2935](AztecProtocol/aztec-packages#2935)) ([7ed30e8](AztecProtocol/aztec-packages@7ed30e8)) * PXE.getNotes(...) + refactor of note types ([#3051](AztecProtocol/aztec-packages#3051)) ([16abb5a](AztecProtocol/aztec-packages@16abb5a)) * Zeromorph with concatenation (Goblin Translator part 10) ([#3006](AztecProtocol/aztec-packages#3006)) ([70b0f17](AztecProtocol/aztec-packages@70b0f17)) ### Bug Fixes * Bad contract txs publishing contract data ([#2673](AztecProtocol/aztec-packages#2673)) ([ccd4611](AztecProtocol/aztec-packages@ccd4611)) * Better error message for compute_note_hash_and_nullifier. ([#3097](AztecProtocol/aztec-packages#3097)) ([57bec53](AztecProtocol/aztec-packages@57bec53)) * Broken `FieldNote` test ([#3135](AztecProtocol/aztec-packages#3135)) ([fe78ecf](AztecProtocol/aztec-packages@fe78ecf)) * Docker-compose up, rather than run. ([#3081](AztecProtocol/aztec-packages#3081)) ([242f780](AztecProtocol/aztec-packages@242f780)) * Formatting ([#3070](AztecProtocol/aztec-packages#3070)) ([e1633d3](AztecProtocol/aztec-packages@e1633d3)) * Minor stale naming fix ([#3117](AztecProtocol/aztec-packages#3117)) ([a6786ae](AztecProtocol/aztec-packages@a6786ae)) * Push cli docker image to docker hub ([#3120](AztecProtocol/aztec-packages#3120)) ([ccad50f](AztecProtocol/aztec-packages@ccad50f)) * Remove duplicate terraform resource definition ([#3066](AztecProtocol/aztec-packages#3066)) ([d5abadb](AztecProtocol/aztec-packages@d5abadb)) * Retry request spot ([#3116](AztecProtocol/aztec-packages#3116)) ([82de5f1](AztecProtocol/aztec-packages@82de5f1)) ### Miscellaneous * Add stdlib tests for pedersen commitment ([#3075](AztecProtocol/aztec-packages#3075)) ([87fa621](AztecProtocol/aztec-packages@87fa621)) * Automatic c_binds for commit should return a point instead of an Fr element ([#3072](AztecProtocol/aztec-packages#3072)) ([2e289a5](AztecProtocol/aztec-packages@2e289a5)) * Cleanup remaining mentions of `compress` with pedersen in cpp and ts ([#3074](AztecProtocol/aztec-packages#3074)) ([52cf383](AztecProtocol/aztec-packages@52cf383)) * E2e on spots [ci rebuild] ([#3068](AztecProtocol/aztec-packages#3068)) ([15db6bf](AztecProtocol/aztec-packages@15db6bf)) * Fix dapp_testing e2e race condition ([#3094](AztecProtocol/aztec-packages#3094)) ([89e7c21](AztecProtocol/aztec-packages@89e7c21)) * Remove docs mirror ([#3122](AztecProtocol/aztec-packages#3122)) ([3fa51e2](AztecProtocol/aztec-packages@3fa51e2)) * Remove endomorphism coefficient from ecc_add_gate ([#3115](AztecProtocol/aztec-packages#3115)) ([d294987](AztecProtocol/aztec-packages@d294987)) * Remove unecessary calls to `pedersen__init` ([#3079](AztecProtocol/aztec-packages#3079)) ([84f8db2](AztecProtocol/aztec-packages@84f8db2)) * Remove unused pedersen c_binds ([#3058](AztecProtocol/aztec-packages#3058)) ([e71e5f9](AztecProtocol/aztec-packages@e71e5f9)) * Removes pedersen commit native pairs method ([#3073](AztecProtocol/aztec-packages#3073)) ([69a34c7](AztecProtocol/aztec-packages@69a34c7)) * Rename private-kernel subpackage to protocol-circuits ([#3134](AztecProtocol/aztec-packages#3134)) ([3e07104](AztecProtocol/aztec-packages@3e07104)) ### Documentation * Initial keys spec ([#3035](AztecProtocol/aztec-packages#3035)) ([4b24c58](AztecProtocol/aztec-packages@4b24c58)) </details> <details><summary>barretenberg.js: 0.13.0</summary> ## [0.13.0](AztecProtocol/aztec-packages@barretenberg.js-v0.12.0...barretenberg.js-v0.13.0) (2023-10-31) ### Features * New script to output table of benchmarks for README pasting. ([#2780](AztecProtocol/aztec-packages#2780)) ([6c20b45](AztecProtocol/aztec-packages@6c20b45)) ### Miscellaneous * Automatic c_binds for commit should return a point instead of an Fr element ([#3072](AztecProtocol/aztec-packages#3072)) ([2e289a5](AztecProtocol/aztec-packages@2e289a5)) * Remove unecessary calls to `pedersen__init` ([#3079](AztecProtocol/aztec-packages#3079)) ([84f8db2](AztecProtocol/aztec-packages@84f8db2)) * Remove unused pedersen c_binds ([#3058](AztecProtocol/aztec-packages#3058)) ([e71e5f9](AztecProtocol/aztec-packages@e71e5f9)) </details> <details><summary>barretenberg: 0.13.0</summary> ## [0.13.0](AztecProtocol/aztec-packages@barretenberg-v0.12.0...barretenberg-v0.13.0) (2023-10-31) ### Features * Adding structure to Transcript ([#2937](AztecProtocol/aztec-packages#2937)) ([db67aa1](AztecProtocol/aztec-packages@db67aa1)) * Efficient ZM quotient computation ([#3016](AztecProtocol/aztec-packages#3016)) ([ebda5fc](AztecProtocol/aztec-packages@ebda5fc)) * Measure plonk rounds ([#3065](AztecProtocol/aztec-packages#3065)) ([c8e1d8b](AztecProtocol/aztec-packages@c8e1d8b)) * New script to output table of benchmarks for README pasting. ([#2780](AztecProtocol/aztec-packages#2780)) ([6c20b45](AztecProtocol/aztec-packages@6c20b45)) * Pedersen in typescript. ([#3111](AztecProtocol/aztec-packages#3111)) ([933f1b2](AztecProtocol/aztec-packages@933f1b2)) * Protogalaxy folding of challenges ([#2935](AztecProtocol/aztec-packages#2935)) ([7ed30e8](AztecProtocol/aztec-packages@7ed30e8)) * Zeromorph with concatenation (Goblin Translator part 10) ([#3006](AztecProtocol/aztec-packages#3006)) ([70b0f17](AztecProtocol/aztec-packages@70b0f17)) ### Miscellaneous * Add stdlib tests for pedersen commitment ([#3075](AztecProtocol/aztec-packages#3075)) ([87fa621](AztecProtocol/aztec-packages@87fa621)) * Automatic c_binds for commit should return a point instead of an Fr element ([#3072](AztecProtocol/aztec-packages#3072)) ([2e289a5](AztecProtocol/aztec-packages@2e289a5)) * Cleanup remaining mentions of `compress` with pedersen in cpp and ts ([#3074](AztecProtocol/aztec-packages#3074)) ([52cf383](AztecProtocol/aztec-packages@52cf383)) * Remove endomorphism coefficient from ecc_add_gate ([#3115](AztecProtocol/aztec-packages#3115)) ([d294987](AztecProtocol/aztec-packages@d294987)) * Remove unecessary calls to `pedersen__init` ([#3079](AztecProtocol/aztec-packages#3079)) ([84f8db2](AztecProtocol/aztec-packages@84f8db2)) * Remove unused pedersen c_binds ([#3058](AztecProtocol/aztec-packages#3058)) ([e71e5f9](AztecProtocol/aztec-packages@e71e5f9)) * Removes pedersen commit native pairs method ([#3073](AztecProtocol/aztec-packages#3073)) ([69a34c7](AztecProtocol/aztec-packages@69a34c7)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This PR adds support for compiling contracts with
noir_wasm
instead ofnargo
.Fix #2644
Requires:
pub
modifier to grumpkin functions noir-lang/noir#3036noir-contracts
to addpub
modifiers to return types (TBD)e.g.
getThisAddress() -> pub Field
hereaztec-packages/yarn-project/noir-contracts/src/contracts/test_contract/src/main.nr
Line 45 in 4881414
noir_wasm
withaztec
feature enabled (based on feat: publish aztec build of noir_wasm noir-lang/noir#3049)Checklist:
Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge.