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.

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: tag artifacts with the compiler version #3220

Merged
merged 3 commits into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion yarn-project/aztec-sandbox/src/bin/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { createAztecNodeRpcServer } from '@aztec/aztec-node';
import { deployInitialSandboxAccounts } from '@aztec/aztec.js';
import { createDebugLogger } from '@aztec/foundation/log';
import { fileURLToPath } from '@aztec/foundation/url';
import { NoirWasmVersion } from '@aztec/noir-compiler/noir-version';
import { NoirWasmVersion } from '@aztec/noir-compiler/versions';
import { createPXERpcServer } from '@aztec/pxe';

import { readFileSync } from 'fs';
Expand Down
9 changes: 9 additions & 0 deletions yarn-project/cli/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { mnemonicToAccount } from 'viem/accounts';

import { createCompatibleClient } from './client.js';
import { encodeArgs, parseStructString } from './encoding.js';
import { GITHUB_TAG_PREFIX } from './github.js';
import { unboxContract } from './unbox.js';
import { update } from './update/update.js';
import {
Expand Down Expand Up @@ -247,6 +248,14 @@ export function getProgram(log: LogFn, debugLogger: DebugLogger): Command {
const constructorArtifact = contractArtifact.functions.find(({ name }) => name === 'constructor');

const client = await createCompatibleClient(rpcUrl, debugLogger);
const nodeInfo = await client.getNodeInfo();
const expectedAztecNrVersion = `${GITHUB_TAG_PREFIX}-v${nodeInfo.sandboxVersion}`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Might be a good idea to bake in this version in getNodeInfo.

if (contractArtifact.aztecNrVersion && contractArtifact.aztecNrVersion !== expectedAztecNrVersion) {
log(
`\nWarning: Contract was compiled with a different version of Aztec.nr: ${contractArtifact.aztecNrVersion}. Consider updating Aztec.nr to ${expectedAztecNrVersion}\n`,
);
}

const deployer = new ContractDeployer(contractArtifact, client, publicKey);

const constructor = getFunctionArtifact(contractArtifact, 'constructor');
Expand Down
6 changes: 6 additions & 0 deletions yarn-project/foundation/src/abi/abi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,12 @@ export interface ContractArtifact {
* The name of the contract.
*/
name: string;

/**
* The version of compiler used to create this artifact
*/
aztecNrVersion?: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

does this need to be optional? Shouldn't every contract have this?

Copy link
Contributor Author

@alexghr alexghr Nov 6, 2023

Choose a reason for hiding this comment

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

I made it optional because:

  • we still have to guard against contracts compiled with an old compiler that didn't do this
  • our own contracts inside yarn-project/noir-contracts currently don't get this field set because they use relative paths to import Aztec.nr rather than git tags. Once this lands we should be able to fix this feat: Add package version to Nargo.toml metadata noir-lang/noir#3427


/**
* The functions of the contract.
*/
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/noir-compiler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"exports": {
".": "./dest/index.js",
"./cli": "./dest/cli/index.js",
"./noir-version": "./dest/noir-version.js"
"./versions": "./dest/versions.js"
},
"typedocOptions": {
"entryPoints": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
exports[`noir-compiler using nargo compiles the test contract 1`] = `
[
{
"aztecNrVersion": undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't determine the aztec.nr version when imported via relative path. Will be fixed with noir-lang/noir#3427.
As for why it ends up being undefined in the snapshot.. just jest things 😆

"events": [],
"functions": [
{
Expand Down Expand Up @@ -195,6 +196,7 @@ export class TestContractContract extends ContractBase {
exports[`noir-compiler using wasm binary compiles the test contract 1`] = `
[
{
"aztecNrVersion": undefined,
"events": [],
"functions": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { NoirDependencyConfig } from '@aztec/foundation/noir';

import { NoirPackage } from '../package.js';
import { NoirDependencyManager } from './dependency-manager.js';
import { DependencyResolver } from './dependency-resolver.js';
import { NoirDependency, NoirDependencyResolver } from './dependency-resolver.js';

describe('DependencyManager', () => {
let manager: NoirDependencyManager;
Expand Down Expand Up @@ -49,44 +49,53 @@ describe('DependencyManager', () => {
});
});

class TestDependencyResolver implements DependencyResolver {
class TestDependencyResolver implements NoirDependencyResolver {
// eslint-disable-next-line require-await
public async resolveDependency(pkg: NoirPackage, dep: NoirDependencyConfig): Promise<NoirPackage | null> {
public async resolveDependency(pkg: NoirPackage, dep: NoirDependencyConfig): Promise<NoirDependency | null> {
if (!('path' in dep)) {
return null;
}

switch (dep.path) {
case '/lib1':
return new NoirPackage('/lib1', '/lib1/src', {
dependencies: {},
package: {
name: 'lib1',
type: 'lib',
},
});
return {
version: '',
package: new NoirPackage('/lib1', '/lib1/src', {
dependencies: {},
package: {
name: 'lib1',
type: 'lib',
},
}),
};

case '/lib2':
return new NoirPackage('/lib2', '/lib2/src', {
dependencies: {
lib3: {
path: '/lib3',
return {
version: '',
package: new NoirPackage('/lib2', '/lib2/src', {
dependencies: {
lib3: {
path: '/lib3',
},
},
},
package: {
name: 'lib2',
type: 'lib',
},
});
package: {
name: 'lib2',
type: 'lib',
},
}),
};

case '/lib3':
return new NoirPackage('/lib3', '/lib3/src', {
dependencies: {},
package: {
name: 'lib3',
type: 'lib',
},
});
return {
version: '',
package: new NoirPackage('/lib3', '/lib3/src', {
dependencies: {},
package: {
name: 'lib3',
type: 'lib',
},
}),
};

default:
throw new Error();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,24 @@ import { NoirDependencyConfig } from '@aztec/foundation/noir';
import { join } from 'node:path';

import { NoirPackage } from '../package.js';
import { DependencyResolver } from './dependency-resolver.js';
import { NoirDependency, NoirDependencyResolver } from './dependency-resolver.js';

/**
* Noir Dependency Resolver
*/
export class NoirDependencyManager {
#entryPoint: NoirPackage;
#libraries = new Map<string, NoirPackage>();
#libraries = new Map<string, NoirDependency>();
#dependencies = new Map<string, string[]>();
#log: LogFn;
#resolvers: readonly DependencyResolver[];
#resolvers: readonly NoirDependencyResolver[];

/**
* Creates a new dependency resolver
* @param resolvers - A list of dependency resolvers to use
* @param entryPoint - The entry point of the project
*/
constructor(resolvers: readonly DependencyResolver[] = [], entryPoint: NoirPackage) {
constructor(resolvers: readonly NoirDependencyResolver[] = [], entryPoint: NoirPackage) {
this.#log = createDebugOnlyLogger('noir:dependency-resolver');
this.#resolvers = resolvers;
this.#entryPoint = entryPoint;
Expand Down Expand Up @@ -49,6 +49,16 @@ export class NoirDependencyManager {
await this.#recursivelyResolveDependencies('', this.#entryPoint);
}

/**
* Gets the version of a dependency in the dependency tree
* @param name - Dependency name
* @returns The dependency's version
*/
public getVersionOf(name: string): string | undefined {
const dep = this.#libraries.get(name);
return dep?.version;
}

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?
Expand All @@ -60,20 +70,20 @@ export class NoirDependencyManager {
}

const dependency = await this.#resolveDependency(noirPackage, config);
if (dependency.getType() !== 'lib') {
if (dependency.package.getType() !== 'lib') {
this.#log(`Non-library package ${name}`, config);
throw new Error(`Dependency ${name} is not a library`);
}

this.#libraries.set(name, dependency);
this.#dependencies.set(packageName, [...(this.#dependencies.get(packageName) ?? []), name]);

await this.#recursivelyResolveDependencies(name, dependency);
await this.#recursivelyResolveDependencies(name, dependency.package);
}
}

async #resolveDependency(pkg: NoirPackage, config: NoirDependencyConfig) {
let dependency: NoirPackage | null = null;
async #resolveDependency(pkg: NoirPackage, config: NoirDependencyConfig): Promise<NoirDependency> {
let dependency: NoirDependency | null = null;
for (const resolver of this.#resolvers) {
dependency = await resolver.resolveDependency(pkg, config);
if (dependency) {
Expand Down Expand Up @@ -102,9 +112,9 @@ export class NoirDependencyManager {
*/
public findFile(sourceId: string): string | null {
const [lib, ...path] = sourceId.split('/').filter(x => x);
const pkg = this.#libraries.get(lib);
if (pkg) {
return join(pkg.getSrcPath(), ...path);
const dep = this.#libraries.get(lib);
if (dep) {
return join(dep.package.getSrcPath(), ...path);
} else {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,24 @@ import { NoirDependencyConfig } from '@aztec/foundation/noir';

import { NoirPackage } from '../package.js';

/**
* A Noir dependency
*/
export type NoirDependency = {
/** version string as determined by the resolver */
version?: string;
/** the actual package source code */
package: NoirPackage;
};

/**
* Resolves a dependency for a package.
*/
export interface DependencyResolver {
export interface NoirDependencyResolver {
/**
* Resolve a dependency for a package.
* @param pkg - The package to resolve dependencies for
* @param dep - The dependency config to resolve
*/
resolveDependency(pkg: NoirPackage, dep: NoirDependencyConfig): Promise<NoirPackage | null>;
resolveDependency(pkg: NoirPackage, dep: NoirDependencyConfig): Promise<NoirDependency | null>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ import { dirname, join } from 'node:path';
import { FileManager } from '../file-manager/file-manager.js';
import { createMemFSFileManager } from '../file-manager/memfs-file-manager.js';
import { NoirPackage } from '../package.js';
import { DependencyResolver } from './dependency-resolver.js';
import { NoirDependencyResolver } from './dependency-resolver.js';
import { GithubDependencyResolver, resolveGithubCodeArchive, safeFilename } from './github-dependency-resolver.js';

const fixtures = join(dirname(fileURLToPath(import.meta.url)), '../../../fixtures');

describe('GithubDependencyResolver', () => {
let resolver: DependencyResolver;
let resolver: NoirDependencyResolver;
let fm: FileManager;
let pkg: NoirPackage;
let libDependency: NoirGitDependencyConfig;
Expand Down Expand Up @@ -62,9 +62,10 @@ describe('GithubDependencyResolver', () => {

it('resolves Github dependency', async () => {
fetchMock.mockResolvedValueOnce(new Response(await readFile(join(fixtures, 'test_lib.zip')), { status: 200 }));
const libPkg = await resolver.resolveDependency(pkg, libDependency);
expect(libPkg).toBeDefined();
expect(fm.hasFileSync(libPkg!.getEntryPointPath())).toBe(true);
const lib = await resolver.resolveDependency(pkg, libDependency);
expect(lib).toBeDefined();
expect(lib!.version).toEqual(libDependency.tag);
expect(fm.hasFileSync(lib!.package.getEntryPointPath())).toBe(true);
});

it.each<[NoirGitDependencyConfig, 'zip' | 'tar', string]>([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ import { unzip } from 'unzipit';

import { FileManager } from '../file-manager/file-manager.js';
import { NoirPackage } from '../package.js';
import { DependencyResolver } from './dependency-resolver.js';
import { NoirDependency, NoirDependencyResolver } from './dependency-resolver.js';

/**
* Downloads dependencies from github
*/
export class GithubDependencyResolver implements DependencyResolver {
export class GithubDependencyResolver implements NoirDependencyResolver {
#fm: FileManager;
#log = createDebugOnlyLogger('aztec:compile:github-dependency-resolver');

Expand All @@ -25,7 +25,7 @@ export class GithubDependencyResolver implements DependencyResolver {
* @param dependency - The dependency configuration
* @returns asd
*/
async resolveDependency(_pkg: NoirPackage, dependency: NoirDependencyConfig): Promise<NoirPackage | null> {
async resolveDependency(_pkg: NoirPackage, dependency: NoirDependencyConfig): Promise<NoirDependency | null> {
// TODO accept ssh urls?
// TODO github authentication?
if (!('git' in dependency) || !dependency.git.startsWith('https://github.com')) {
Expand All @@ -34,7 +34,10 @@ export class GithubDependencyResolver implements DependencyResolver {

const archivePath = await this.#fetchZipFromGithub(dependency);
const libPath = await this.#extractZip(dependency, archivePath);
return NoirPackage.open(libPath, this.#fm);
return {
version: dependency.tag,
package: NoirPackage.open(libPath, this.#fm),
};
}

async #fetchZipFromGithub(dependency: Pick<NoirGitDependencyConfig, 'git' | 'tag'>): Promise<string> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ import { dirname, join } from 'node:path';
import { FileManager } from '../file-manager/file-manager.js';
import { createMemFSFileManager } from '../file-manager/memfs-file-manager.js';
import { NoirPackage } from '../package.js';
import { DependencyResolver } from './dependency-resolver.js';
import { NoirDependencyResolver } from './dependency-resolver.js';
import { LocalDependencyResolver } from './local-dependency-resolver.js';

describe('DependencyResolver', () => {
let resolver: DependencyResolver;
let resolver: NoirDependencyResolver;
let fm: FileManager;
let pkg: NoirPackage;

Expand Down Expand Up @@ -43,10 +43,11 @@ describe('DependencyResolver', () => {
});

it.each(['../test_contract', '/test_contract'])('resolves a known dependency', async path => {
const libPkg = await resolver.resolveDependency(pkg, {
const lib = await resolver.resolveDependency(pkg, {
path,
});
expect(libPkg).toBeDefined();
expect(fm.hasFileSync(libPkg!.getEntryPointPath())).toBe(true);
expect(lib).toBeDefined();
expect(lib!.version).toBeUndefined();
expect(fm.hasFileSync(lib!.package.getEntryPointPath())).toBe(true);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,25 @@ import { resolve } from 'path';

import { FileManager } from '../file-manager/file-manager.js';
import { NoirPackage } from '../package.js';
import { DependencyResolver } from './dependency-resolver.js';
import { NoirDependency, NoirDependencyResolver } from './dependency-resolver.js';

/**
* Resolves dependencies on-disk, relative to current package
*/
export class LocalDependencyResolver implements DependencyResolver {
export class LocalDependencyResolver implements NoirDependencyResolver {
#fm: FileManager;

constructor(fm: FileManager) {
this.#fm = fm;
}

resolveDependency(pkg: NoirPackage, config: NoirDependencyConfig): Promise<NoirPackage | null> {
resolveDependency(pkg: NoirPackage, config: NoirDependencyConfig): Promise<NoirDependency | null> {
if ('path' in config) {
return Promise.resolve(NoirPackage.open(resolve(pkg.getPackagePath(), config.path), this.#fm));
return Promise.resolve({
// unknown version, Nargo.toml doesn't have a version field
version: undefined,
package: NoirPackage.open(resolve(pkg.getPackagePath(), config.path), this.#fm),
});
} else {
return Promise.resolve(null);
}
Expand Down
Loading