Skip to content

Commit

Permalink
fix: add handling to noir_wasm for projects without dependencies (#…
Browse files Browse the repository at this point in the history
…4344)

# Description

## Problem\*

Resolves #4338

## Summary\*

This PR returns an empty dependencies map rather than undefined if the
package being compiled doesn't have any dependencies.

I've also updated the test suite so it also compiles more than just a
contract

## 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.

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>
  • Loading branch information
TomAFrench and kevaundray authored Feb 22, 2024
1 parent 292a972 commit 4982251
Show file tree
Hide file tree
Showing 9 changed files with 197 additions and 77 deletions.
2 changes: 1 addition & 1 deletion compiler/wasm/src/noir/package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export class Package {
* Gets this package's dependencies.
*/
public getDependencies(): Record<string, DependencyConfig> {
return this.#config.dependencies;
return this.#config.dependencies ?? {};
}

/**
Expand Down
2 changes: 2 additions & 0 deletions compiler/wasm/src/types/noir_artifact.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ export interface ContractArtifact {
* The compilation result of an Noir contract.
*/
export interface ProgramArtifact {
/** Version of noir used for the build. */
noir_version: string;
/** The hash of the circuit. */
hash?: number;
/** * The ABI of the function. */
Expand Down
2 changes: 1 addition & 1 deletion compiler/wasm/src/types/noir_package_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type NoirPackageConfigSchema = {
backend?: string;
license?: string;
};
dependencies: Record<string, GitDependencyConfig | LocalDependencyConfig>;
dependencies?: Record<string, GitDependencyConfig | LocalDependencyConfig>;
};

/**
Expand Down
79 changes: 79 additions & 0 deletions compiler/wasm/test/compiler/browser/compile.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/* eslint-disable @typescript-eslint/ban-ts-comment */
import { getPaths } from '../../shared';
import { expect } from '@esm-bundle/chai';
import { compile, createFileManager } from '@noir-lang/noir_wasm';
import { ContractArtifact, ProgramArtifact } from '../../../src/types/noir_artifact';
import { shouldCompileContractIdentically, shouldCompileProgramIdentically } from '../shared/compile.test';

const paths = getPaths('.');

async function getFile(path: string) {
// @ts-ignore
const basePath = new URL('./../../', import.meta.url).toString().replace(/\/$/g, '');
const url = `${basePath}${path.replace('.', '')}`;
const response = await fetch(url);
return response;
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
async function getPrecompiledSource(path: string): Promise<any> {
const response = await getFile(path);
const compiledData = await response.text();
return JSON.parse(compiledData);
}

describe('noir-compiler/browser', () => {
shouldCompileProgramIdentically(
async () => {
const { simpleScriptExpectedArtifact } = paths;
const fm = createFileManager('/');
const files = Object.values(paths).filter((fileOrDir) => /^\.?\/.*\..*$/.test(fileOrDir));
for (const path of files) {
console.log(path);
await fm.writeFile(path, (await getFile(path)).body as ReadableStream<Uint8Array>);
}
const nargoArtifact = (await getPrecompiledSource(simpleScriptExpectedArtifact)) as ProgramArtifact;
const noirWasmArtifact = await compile(fm, '/fixtures/simple');

return { nargoArtifact, noirWasmArtifact };
},
expect,
60 * 20e3,
);

shouldCompileProgramIdentically(
async () => {
const { depsScriptExpectedArtifact } = paths;
const fm = createFileManager('/');
const files = Object.values(paths).filter((fileOrDir) => /^\.?\/.*\..*$/.test(fileOrDir));
for (const path of files) {
console.log(path);
await fm.writeFile(path, (await getFile(path)).body as ReadableStream<Uint8Array>);
}
const nargoArtifact = (await getPrecompiledSource(depsScriptExpectedArtifact)) as ProgramArtifact;
const noirWasmArtifact = await compile(fm, '/fixtures/with-deps');

return { nargoArtifact, noirWasmArtifact };
},
expect,
60 * 20e3,
);

shouldCompileContractIdentically(
async () => {
const { contractExpectedArtifact } = paths;
const fm = createFileManager('/');
const files = Object.values(paths).filter((fileOrDir) => /^\.?\/.*\..*$/.test(fileOrDir));
for (const path of files) {
console.log(path);
await fm.writeFile(path, (await getFile(path)).body as ReadableStream<Uint8Array>);
}
const nargoArtifact = (await getPrecompiledSource(contractExpectedArtifact)) as ContractArtifact;
const noirWasmArtifact = await compile(fm, '/fixtures/noir-contract');

return { nargoArtifact, noirWasmArtifact };
},
expect,
60 * 20e3,
);
});
43 changes: 0 additions & 43 deletions compiler/wasm/test/compiler/browser/compile_with_deps.test.ts

This file was deleted.

39 changes: 39 additions & 0 deletions compiler/wasm/test/compiler/node/compile.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { join, resolve } from 'path';
import { getPaths } from '../../shared';

import { expect } from 'chai';
import { compile, createFileManager } from '@noir-lang/noir_wasm';
import { readFile } from 'fs/promises';
import { ContractArtifact, ProgramArtifact } from '../../../src/types/noir_artifact';
import { shouldCompileContractIdentically, shouldCompileProgramIdentically } from '../shared/compile.test';

const basePath = resolve(join(__dirname, '../../'));

describe('noir-compiler/node', () => {
shouldCompileProgramIdentically(async () => {
const { simpleScriptProjectPath, simpleScriptExpectedArtifact } = getPaths(basePath);

const fm = createFileManager(simpleScriptProjectPath);
const nargoArtifact = JSON.parse((await readFile(simpleScriptExpectedArtifact)).toString()) as ProgramArtifact;
const noirWasmArtifact = await compile(fm);
return { nargoArtifact, noirWasmArtifact };
}, expect);

shouldCompileProgramIdentically(async () => {
const { depsScriptProjectPath, depsScriptExpectedArtifact } = getPaths(basePath);

const fm = createFileManager(depsScriptProjectPath);
const nargoArtifact = JSON.parse((await readFile(depsScriptExpectedArtifact)).toString()) as ProgramArtifact;
const noirWasmArtifact = await compile(fm);
return { nargoArtifact, noirWasmArtifact };
}, expect);

shouldCompileContractIdentically(async () => {
const { contractProjectPath, contractExpectedArtifact } = getPaths(basePath);

const fm = createFileManager(contractProjectPath);
const nargoArtifact = JSON.parse((await readFile(contractExpectedArtifact)).toString()) as ContractArtifact;
const noirWasmArtifact = await compile(fm);
return { nargoArtifact, noirWasmArtifact };
}, expect);
});
20 changes: 0 additions & 20 deletions compiler/wasm/test/compiler/node/compile_with_deps.test.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,47 @@ import {
DebugFileMap,
DebugInfo,
NoirFunctionEntry,
ProgramArtifact,
ProgramCompilationArtifacts,
} from '../../../src/types/noir_artifact';

export function shouldCompileIdentically(
export function shouldCompileProgramIdentically(
compileFn: () => Promise<{ nargoArtifact: ProgramArtifact; noirWasmArtifact: CompilationResult }>,
expect: typeof Expect,
timeout = 5000,
) {
it('both nargo and noir_wasm should compile identically', async () => {
// Compile!
const { nargoArtifact, noirWasmArtifact } = await compileFn();

// Prepare nargo artifact
const [_nargoDebugInfos, nargoFileMap] = deleteProgramDebugMetadata(nargoArtifact);
normalizeVersion(nargoArtifact);

// Prepare noir-wasm artifact
const noirWasmProgram = (noirWasmArtifact as unknown as ProgramCompilationArtifacts).program;
expect(noirWasmProgram).not.to.be.undefined;
const [_noirWasmDebugInfos, norWasmFileMap] = deleteProgramDebugMetadata(noirWasmProgram);
normalizeVersion(noirWasmProgram);

// We first compare both contracts without considering debug info
delete (noirWasmProgram as Partial<ProgramArtifact>).hash;
delete (nargoArtifact as Partial<ProgramArtifact>).hash;
expect(nargoArtifact).to.deep.eq(noirWasmProgram);

// Compare the file maps, ignoring keys, since those depend in the order in which files are visited,
// which may change depending on the file manager implementation. Also ignores paths, since the base
// path is reported differently between nargo and noir-wasm.
expect(getSources(nargoFileMap)).to.have.members(getSources(norWasmFileMap));

// Compare the debug symbol information, ignoring the actual ids used for file identifiers.
// Debug symbol info looks like the following, what we need is to ignore the 'file' identifiers
// {"locations":{"0":[{"span":{"start":141,"end":156},"file":39},{"span":{"start":38,"end":76},"file":38},{"span":{"start":824,"end":862},"file":23}]}}
// expect(nargoDebugInfos).to.deep.eq(noirWasmDebugInfos);
}).timeout(timeout);
}

export function shouldCompileContractIdentically(
compileFn: () => Promise<{ nargoArtifact: ContractArtifact; noirWasmArtifact: CompilationResult }>,
expect: typeof Expect,
timeout = 5000,
Expand All @@ -18,13 +56,13 @@ export function shouldCompileIdentically(
const { nargoArtifact, noirWasmArtifact } = await compileFn();

// Prepare nargo artifact
const [nargoDebugInfos, nargoFileMap] = deleteDebugMetadata(nargoArtifact);
const [nargoDebugInfos, nargoFileMap] = deleteContractDebugMetadata(nargoArtifact);
normalizeVersion(nargoArtifact);

// Prepare noir-wasm artifact
const noirWasmContract = (noirWasmArtifact as ContractCompilationArtifacts).contract;
const noirWasmContract = (noirWasmArtifact as unknown as ContractCompilationArtifacts).contract;
expect(noirWasmContract).not.to.be.undefined;
const [noirWasmDebugInfos, norWasmFileMap] = deleteDebugMetadata(noirWasmContract);
const [noirWasmDebugInfos, norWasmFileMap] = deleteContractDebugMetadata(noirWasmContract);
normalizeVersion(noirWasmContract);

// We first compare both contracts without considering debug info
Expand All @@ -43,7 +81,7 @@ export function shouldCompileIdentically(
}

/** Remove commit identifier from version, which may not match depending on cached nargo and noir-wasm */
function normalizeVersion(contract: ContractArtifact) {
function normalizeVersion(contract: ProgramArtifact | ContractArtifact) {
contract.noir_version = contract.noir_version.replace(/\+.+$/, '');
}

Expand All @@ -57,8 +95,18 @@ function extractDebugInfos(fns: NoirFunctionEntry[]) {
});
}

/** Deletes all debug info from a program and returns it. */
function deleteProgramDebugMetadata(program: ProgramArtifact) {
const debugSymbols = inflateDebugSymbols(program.debug_symbols);
const fileMap = program.file_map;

delete (program as Partial<ProgramArtifact>).debug_symbols;
delete (program as Partial<ProgramArtifact>).file_map;
return [debugSymbols, fileMap];
}

/** Deletes all debug info from a contract and returns it. */
function deleteDebugMetadata(contract: ContractArtifact) {
function deleteContractDebugMetadata(contract: ContractArtifact) {
contract.functions.sort((a, b) => a.name.localeCompare(b.name));
const fileMap = contract.file_map;
delete (contract as Partial<ContractArtifact>).file_map;
Expand Down
27 changes: 21 additions & 6 deletions compiler/wasm/test/shared.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
export function getPaths(basePath: string) {
const fixtures = `${basePath}/fixtures`;

const simpleScriptSourcePath = `${fixtures}/simple/src/main.nr`;
const simpleScriptExpectedArtifact = `${fixtures}/simple/target/noir_wasm_testing.json`;
const simpleScriptProjectPath = `${fixtures}/simple`;
const simpleScriptSourcePath = `${simpleScriptProjectPath}/src/main.nr`;
const simpleScriptTOMLPath = `${simpleScriptProjectPath}/Nargo.toml`;
const simpleScriptExpectedArtifact = `${simpleScriptProjectPath}/target/noir_wasm_testing.json`;

const depsScriptSourcePath = `${fixtures}/with-deps/src/main.nr`;
const depsScriptExpectedArtifact = `${fixtures}/with-deps/target/noir_wasm_testing.json`;
const depsScriptProjectPath = `${fixtures}/with-deps`;
const depsScriptSourcePath = `${depsScriptProjectPath}/src/main.nr`;
const depsScriptTOMLPath = `${depsScriptProjectPath}/Nargo.toml`;
const depsScriptExpectedArtifact = `${depsScriptProjectPath}/target/noir_wasm_testing.json`;

const libASourcePath = `${fixtures}/deps/lib-a/src/lib.nr`;
const libBSourcePath = `${fixtures}/deps/lib-b/src/lib.nr`;
const libAProjectPath = `${fixtures}/deps/lib-a`;
const libASourcePath = `${libAProjectPath}/src/lib.nr`;
const libATOMLPath = `${libAProjectPath}/Nargo.toml`;

const libBProjectPath = `${fixtures}/deps/lib-b`;
const libBSourcePath = `${libBProjectPath}/src/lib.nr`;
const libBTOMLPath = `${libBProjectPath}/Nargo.toml`;

const contractProjectPath = `${fixtures}/noir-contract`;
const contractSourcePath = `${contractProjectPath}/src/main.nr`;
Expand All @@ -22,12 +31,18 @@ export function getPaths(basePath: string) {
const libCTOMLPath = `${libCProjectPath}/Nargo.toml`;

return {
simpleScriptProjectPath,
simpleScriptSourcePath,
simpleScriptTOMLPath,
simpleScriptExpectedArtifact,
depsScriptProjectPath,
depsScriptSourcePath,
depsScriptTOMLPath,
depsScriptExpectedArtifact,
libASourcePath,
libATOMLPath,
libBSourcePath,
libBTOMLPath,
contractProjectPath,
contractSourcePath,
contractTOMLPath,
Expand Down

0 comments on commit 4982251

Please sign in to comment.