feat!: Unify ABIs between nargo and yarn-project #4035

spalladino · 2024-01-12T17:42:57Z

Subrepo push of AztecProtocol/aztec-packages#3989 to a branch in order to trigger Noir CI

Should be merged as part of a proper git subrepo push

TomAFrench · 2024-01-12T18:02:04Z

compiler/wasm/src/compile.rs

+    Contract { contract: ContractArtifact, warnings: Vec<String> },
+    Program { program: ProgramArtifact, warnings: Vec<String> },


I realise now that we're actually dropping all of the warnings from the noirc_frontend. Shall we drop these (warnings from noirc_evaluator) as well for now and then we can add a warnings field which contains both of these?

On third thoughts, I'm fairly sure that the types as emitted from the wasm are now completely ignored. There's types in the TS code which are what are meaningful now.

Ideally thought we'd still surface these types from the wasm into the TS. Seems like the lazy loading is throwing a spanner in the works however.

I'm leaving the warnings in for now if that's ok, I think we should make a bigger refactor around those ts types. I'll open another conversation for it.

See AztecProtocol/aztec-packages#3989 (comment)

socket-security · 2024-01-12T22:21:03Z

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages	Version	New capabilities	Transitives	Size	Publisher
@types/pako	2.0.3	None	`+0`	7.8 kB	types
pako	2.1.0	None	`+0`	1.64 MB	vitaly

spalladino · 2024-01-15T19:44:34Z

The error firing in the codegen-verify of the integration test seems to happen because of a wrong bb version being used. I could reproduce locally, then re-built bb and pointed NARGO_BACKEND_PATH to it, and code generation worked fine. Still, I did get an unreachable error running the recursion test:

test/browser/recursion.test.ts:

 🚧 Browser logs:
      Compiling at /src/main.nr
      Dependencies: 
      Adding source /src/main.nr
      Compiling at /src/main.nr
      Dependencies: 
      Adding source /src/main.nr

 ❌ It compiles noir program code, receiving circuit bytes and abi object. > Should generate valid inner proof for correct input, then verify proof within a proof
      RuntimeError: unreachable
        at wasm://wasm/017523ee:wasm-function[14038]:0x588099
        at wasm://wasm/017523ee:wasm-function[29]:0x1e831
        at wasm://wasm/017523ee:wasm-function[3889]:0x1aa7f3
        at wasm://wasm/017523ee:wasm-function[3887]:0x1aa4f9
        at wasm://wasm/017523ee:wasm-function[3885]:0x1aa470
        at wasm://wasm/017523ee:wasm-function[3884]:0x1aa402
        at wasm://wasm/017523ee:wasm-function[3844]:0x1a9830
        at wasm://wasm/017523ee:wasm-function[3666]:0x1a8162
        at wasm://wasm/017523ee:wasm-function[3652]:0x1a7c11
        at wasm://wasm/017523ee:wasm-function[3643]:0x1a795d

Not sure how to unblock this one.

TomAFrench

Looks fine to me. The JS types are still outdated and referring to internal compiler structs however.

TomAFrench · 2024-01-16T15:26:55Z

compiler/wasm/src/types/noir_artifact.ts

@@ -57,12 +59,16 @@ export interface NoirFunctionEntry {
 export interface CompiledContract {


Suggested change

export interface CompiledContract {

export interface ContractArtifact {

TomAFrench · 2024-01-16T15:27:03Z

compiler/wasm/src/types/noir_artifact.ts

  /** Compilation backend. */
  backend: string;


Suggested change

/** Compilation backend. */

backend: string;

TomAFrench · 2024-01-16T15:27:25Z

compiler/wasm/src/types/noir_artifact.ts

@@ -73,12 +79,14 @@ export interface CompiledCircuit {
  hash?: number;


This should be a ProgramArtifact

TomAFrench · 2024-01-16T15:27:36Z

compiler/wasm/src/types/noir_artifact.ts

  /** Compilation backend. */
  backend: string;


Suggested change

/** Compilation backend. */

backend: string;

Changes the output of `nargo` and `noir-wasm` to generate a single artifact that includes debug information. Compiled contracts now have a `file_map` at the root of the artifact, and a `debug_symbols` within each function. For consistency, compiled programs also have `file_map` and `debug_symbols`. Debug symbols are compressed and base64-encoded, as they were by the former `noir-compiler` postprocessing. The `debug` json artifact is no longer emitted. Removes all code related to generating `yarn-project`-specific ABIs from noir compiler. Instead, `types` now exposes a `loadContractArtifact` that renames fields as needed (and use camel instead of snake case), and is required when loading a noir contract artifact. Autogenerated types run this automatically. Since we are no longer post-processing artifacts, the `noir-contracts` project shape is changed. JSON files live in the `target` folder where nargo outputs them (this is not configurable), and are loaded by the autogenerated typescript interfaces from there. As part of the refactor, moves functions for getting functions debug info out of `acir-simulator` and into `types`. **Breaking change**: This removes the need to run `codegen` for using a compiled contract. However, when creating a new contract object from aztec.js, a dev needs to call `loadContractArtifact`. **Future changes**: Type information for a compilation artifact is now spread all over the place, and duplicated in more than one place. There are types defined within rust code in Noir as `[wasm_bindgen(typescript_custom_section)]`, others defined within typescript code in the `noir_wasm` package, others in `foundation`, and others in `types`. We should unify it in a single place. Fixes #3812 Supersedes #3906 Noir subrepo has been pushed to noir-lang/noir#4035 to run the Noir CI

Changes the output of `nargo` and `noir-wasm` to generate a single artifact that includes debug information. Compiled contracts now have a `file_map` at the root of the artifact, and a `debug_symbols` within each function. For consistency, compiled programs also have `file_map` and `debug_symbols`. Debug symbols are compressed and base64-encoded, as they were by the former `noir-compiler` postprocessing. The `debug` json artifact is no longer emitted. Removes all code related to generating `yarn-project`-specific ABIs from noir compiler. Instead, `types` now exposes a `loadContractArtifact` that renames fields as needed (and use camel instead of snake case), and is required when loading a noir contract artifact. Autogenerated types run this automatically. Since we are no longer post-processing artifacts, the `noir-contracts` project shape is changed. JSON files live in the `target` folder where nargo outputs them (this is not configurable), and are loaded by the autogenerated typescript interfaces from there. As part of the refactor, moves functions for getting functions debug info out of `acir-simulator` and into `types`. **Breaking change**: This removes the need to run `codegen` for using a compiled contract. However, when creating a new contract object from aztec.js, a dev needs to call `loadContractArtifact`. **Future changes**: Type information for a compilation artifact is now spread all over the place, and duplicated in more than one place. There are types defined within rust code in Noir as `[wasm_bindgen(typescript_custom_section)]`, others defined within typescript code in the `noir_wasm` package, others in `foundation`, and others in `types`. We should unify it in a single place. Fixes AztecProtocol/aztec-packages#3812 Supersedes #3906 Noir subrepo has been pushed to #4035 to run the Noir CI

TomAFrench · 2024-01-22T16:00:21Z

Closing as this got merged in aztec-packages.

Changes the output of `nargo` and `noir-wasm` to generate a single artifact that includes debug information. Compiled contracts now have a `file_map` at the root of the artifact, and a `debug_symbols` within each function. For consistency, compiled programs also have `file_map` and `debug_symbols`. Debug symbols are compressed and base64-encoded, as they were by the former `noir-compiler` postprocessing. The `debug` json artifact is no longer emitted. Removes all code related to generating `yarn-project`-specific ABIs from noir compiler. Instead, `types` now exposes a `loadContractArtifact` that renames fields as needed (and use camel instead of snake case), and is required when loading a noir contract artifact. Autogenerated types run this automatically. Since we are no longer post-processing artifacts, the `noir-contracts` project shape is changed. JSON files live in the `target` folder where nargo outputs them (this is not configurable), and are loaded by the autogenerated typescript interfaces from there. As part of the refactor, moves functions for getting functions debug info out of `acir-simulator` and into `types`. **Breaking change**: This removes the need to run `codegen` for using a compiled contract. However, when creating a new contract object from aztec.js, a dev needs to call `loadContractArtifact`. **Future changes**: Type information for a compilation artifact is now spread all over the place, and duplicated in more than one place. There are types defined within rust code in Noir as `[wasm_bindgen(typescript_custom_section)]`, others defined within typescript code in the `noir_wasm` package, others in `foundation`, and others in `types`. We should unify it in a single place. Fixes AztecProtocol#3812 Supersedes AztecProtocol#3906 Noir subrepo has been pushed to noir-lang/noir#4035 to run the Noir CI

TomAFrench reviewed Jan 12, 2024

View reviewed changes

spalladino force-pushed the palla/unify-abis-2 branch from 5ecbcbb to ec8f7f1 Compare January 12, 2024 22:14

spalladino force-pushed the palla/unify-abis-2 branch from ec8f7f1 to d34f6c2 Compare January 15, 2024 14:52

spalladino added 3 commits January 15, 2024 13:55

Update nargo and noir-wasm

9d1cf69

Make cargo clippy happy

9e5e880

Inflate as helper in noir wasm and fix browser test

08c9b74

spalladino force-pushed the palla/unify-abis-2 branch from d34f6c2 to 08c9b74 Compare January 15, 2024 16:55

spalladino mentioned this pull request Jan 15, 2024

feat!: Unify ABIs between nargo and yarn-project AztecProtocol/aztec-packages#3989

Merged

TomAFrench reviewed Jan 16, 2024

View reviewed changes

TomAFrench closed this Jan 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: Unify ABIs between nargo and yarn-project #4035

feat!: Unify ABIs between nargo and yarn-project #4035

spalladino commented Jan 12, 2024

TomAFrench Jan 12, 2024

TomAFrench Jan 12, 2024

TomAFrench Jan 12, 2024

spalladino Jan 15, 2024

spalladino Jan 15, 2024

socket-security bot commented Jan 12, 2024

spalladino commented Jan 15, 2024

TomAFrench left a comment

TomAFrench Jan 16, 2024

TomAFrench Jan 16, 2024

TomAFrench Jan 16, 2024

TomAFrench Jan 16, 2024

TomAFrench commented Jan 22, 2024

		Contract { contract: ContractArtifact, warnings: Vec<String> },
		Program { program: ProgramArtifact, warnings: Vec<String> },

		@@ -57,12 +59,16 @@ export interface NoirFunctionEntry {
		export interface CompiledContract {

	export interface CompiledContract {
	export interface ContractArtifact {

		@@ -73,12 +79,14 @@ export interface CompiledCircuit {
		hash?: number;

feat!: Unify ABIs between nargo and yarn-project #4035

feat!: Unify ABIs between nargo and yarn-project #4035

Conversation

spalladino commented Jan 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

socket-security bot commented Jan 12, 2024

spalladino commented Jan 15, 2024

TomAFrench left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomAFrench commented Jan 22, 2024