Skip to content

Commit

Permalink
fix(cli): Support initializers not named constructor in cli
Browse files Browse the repository at this point in the history
  • Loading branch information
spalladino committed Mar 22, 2024
1 parent 6a7ccca commit 10ad46f
Show file tree
Hide file tree
Showing 6 changed files with 217 additions and 47 deletions.
12 changes: 3 additions & 9 deletions yarn-project/aztec.js/src/contract/deploy_method.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
getContractClassFromArtifact,
getContractInstanceFromDeployParams,
} from '@aztec/circuits.js';
import { ContractArtifact, FunctionArtifact, getDefaultInitializer } from '@aztec/foundation/abi';
import { ContractArtifact, FunctionArtifact, getInitializer } from '@aztec/foundation/abi';
import { EthAddress } from '@aztec/foundation/eth-address';
import { Fr } from '@aztec/foundation/fields';
import { createDebugLogger } from '@aztec/foundation/log';
Expand Down Expand Up @@ -63,16 +63,10 @@ export class DeployMethod<TContract extends ContractBase = Contract> extends Bas
private artifact: ContractArtifact,
private postDeployCtor: (address: AztecAddress, wallet: Wallet) => Promise<TContract>,
private args: any[] = [],
constructorName?: string,
constructorNameOrArtifact?: string | FunctionArtifact,
) {
super(wallet);
this.constructorArtifact = constructorName
? artifact.functions.find(f => f.name === constructorName)
: getDefaultInitializer(artifact);

if (constructorName && !this.constructorArtifact) {
throw new Error(`Constructor method ${constructorName} not found in contract artifact`);
}
this.constructorArtifact = getInitializer(artifact, constructorNameOrArtifact);
}

/**
Expand Down
23 changes: 13 additions & 10 deletions yarn-project/cli/src/cmds/deploy.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { getSchnorrAccount } from '@aztec/accounts/schnorr';
import { ContractDeployer, EthAddress, Fq, Fr, Point } from '@aztec/aztec.js';
import { getInitializer } from '@aztec/foundation/abi';
import { DebugLogger, LogFn } from '@aztec/foundation/log';

import { createCompatibleClient } from '../client.js';
import { encodeArgs } from '../encoding.js';
import { GITHUB_TAG_PREFIX } from '../github.js';
import { getContractArtifact, getFunctionArtifact } from '../utils.js';
import { getContractArtifact } from '../utils.js';

export async function deploy(
artifactPath: string,
Expand All @@ -16,13 +17,14 @@ export async function deploy(
portalAddress: EthAddress,
salt: Fr,
privateKey: Fq,
initializer: string | undefined,
wait: boolean,
debugLogger: DebugLogger,
log: LogFn,
logJson: (output: any) => void,
) {
const contractArtifact = await getContractArtifact(artifactPath, log);
const constructorArtifact = contractArtifact.functions.find(({ name }) => name === 'constructor');
const constructorArtifact = getInitializer(contractArtifact, initializer);

const client = await createCompatibleClient(rpcUrl, debugLogger);
const nodeInfo = await client.getNodeInfo();
Expand All @@ -34,17 +36,18 @@ export async function deploy(
}

const wallet = await getSchnorrAccount(client, privateKey, privateKey, Fr.ZERO).getWallet();
const deployer = new ContractDeployer(contractArtifact, wallet, publicKey);
const deployer = new ContractDeployer(contractArtifact, wallet, publicKey, initializer);

const constructor = getFunctionArtifact(contractArtifact, 'constructor');
if (!constructor) {
throw new Error(`Constructor not found in contract ABI`);
let args = [];
if (rawArgs.length > 0) {
if (!constructorArtifact) {
throw new Error(`Cannot process constructor arguments as no constructor was found`);
}
debugLogger(`Input arguments: ${rawArgs.map((x: any) => `"${x}"`).join(', ')}`);
args = encodeArgs(rawArgs, constructorArtifact!.parameters);
debugLogger(`Encoded arguments: ${args.join(', ')}`);
}

debugLogger(`Input arguments: ${rawArgs.map((x: any) => `"${x}"`).join(', ')}`);
const args = encodeArgs(rawArgs, constructorArtifact!.parameters);
debugLogger(`Encoded arguments: ${args.join(', ')}`);

const deploy = deployer.deploy(...args);

await deploy.create({ contractAddressSalt: salt, portalContract: portalAddress });
Expand Down
39 changes: 23 additions & 16 deletions yarn-project/cli/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ export function getProgram(log: LogFn, debugLogger: DebugLogger): Command {
'<artifact>',
"A compiled Aztec.nr contract's artifact in JSON format or name of a contract artifact exported by @aztec/noir-contracts.js",
)
.option('--initializer <string>', 'The contract initializer function to call')
.option('-a, --args <constructorArgs...>', 'Contract constructor arguments', [])
.addOption(pxeOption)
.option(
Expand All @@ -178,23 +179,29 @@ export function getProgram(log: LogFn, debugLogger: DebugLogger): Command {
// `options.wait` is default true. Passing `--no-wait` will set it to false.
// https://github.com/tj/commander.js#other-option-types-negatable-boolean-and-booleanvalue
.option('--no-wait', 'Skip waiting for the contract to be deployed. Print the hash of deployment transaction')
.action(async (artifactPath, { json, rpcUrl, publicKey, args: rawArgs, portalAddress, salt, wait, privateKey }) => {
const { deploy } = await import('./cmds/deploy.js');
await deploy(
.action(
async (
artifactPath,
json,
rpcUrl,
publicKey,
rawArgs,
portalAddress,
salt,
privateKey,
wait,
debugLogger,
log,
logJson,
);
});
{ json, rpcUrl, publicKey, args: rawArgs, portalAddress, salt, wait, privateKey, initializer },
) => {
const { deploy } = await import('./cmds/deploy.js');
await deploy(
artifactPath,
json,
rpcUrl,
publicKey,
rawArgs,
portalAddress,
salt,
privateKey,
initializer,
wait,
debugLogger,
log,
logJson,
);
},
);

program
.command('check-deploy')
Expand Down
43 changes: 33 additions & 10 deletions yarn-project/end-to-end/src/shared/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,7 @@ const TRANSFER_BALANCE = 3000;

export const cliTestSuite = (
name: string,
setup: () => Promise<{
/**
* The PXE instance.
*/
pxe: PXE;
/**
* The URL of the PXE RPC server.
*/
rpcURL: string;
}>,
setup: () => Promise<{ pxe: PXE; rpcURL: string }>,
cleanup: () => Promise<void>,
debug: DebugLogger,
) =>
Expand Down Expand Up @@ -119,6 +110,38 @@ export const cliTestSuite = (
expect(fetchedAddress).toEqual(newCompleteAddress.publicKey.toString());
});

// Regression test for deploy cmd with a constructor not named "constructor"
it('deploys a contract using a public initializer', async () => {
debug('Create an account using a private key');
await run('generate-private-key', false);
const privKey = findInLogs(/Private\sKey:\s+0x(?<privKey>[a-fA-F0-9]+)/)?.groups?.privKey;
expect(privKey).toHaveLength(64);
await run(`create-account --private-key ${privKey}`);
const foundAddress = findInLogs(/Address:\s+(?<address>0x[a-fA-F0-9]+)/)?.groups?.address;
expect(foundAddress).toBeDefined();
const ownerAddress = AztecAddress.fromString(foundAddress!);
const salt = 42;

debug('Deploy StatefulTestContract with public_constructor using created account.');
await run(
`deploy StatefulTestContractArtifact --private-key ${privKey} --salt ${salt} --initializer public_constructor --args ${ownerAddress} 100`,
);
const loggedAddress = findInLogs(/Contract\sdeployed\sat\s+(?<address>0x[a-fA-F0-9]+)/)?.groups?.address;
expect(loggedAddress).toBeDefined();
contractAddress = AztecAddress.fromString(loggedAddress!);

const deployedContract = await pxe.getContractInstance(contractAddress);
expect(deployedContract?.address).toEqual(contractAddress);

clearLogs();
await run(
`call get_public_value --args ${ownerAddress} --contract-artifact StatefulTestContractArtifact --contract-address ${contractAddress.toString()}`,
);

const balance = findInLogs(/View\sresult:\s+(?<data>\S+)/)?.groups?.data;
expect(balance!).toEqual(`${BigInt(100).toString()}n`);
});

it.each([
['an example Token contract', 'TokenContractArtifact', '0'],
['a Nargo artifact', '../noir-contracts.js/artifacts/token_contract-Token.json', '1'],
Expand Down
114 changes: 114 additions & 0 deletions yarn-project/foundation/src/abi/abi.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import { ContractArtifact, FunctionArtifact, FunctionType, getDefaultInitializer, getInitializer } from './abi.js';

describe('abi', () => {
describe('getDefaultInitializer', () => {
it('does not return non initializer functions', () => {
const contract = { functions: [{ isInitializer: false }] } as ContractArtifact;
expect(getDefaultInitializer(contract)).toBeUndefined();
});

it('returns the single initializer in a contract', () => {
const contract = {
functions: [
{ name: 'non-init', isInitializer: false },
{ name: 'init', isInitializer: true },
],
} as ContractArtifact;
expect(getDefaultInitializer(contract)?.name).toEqual('init');
});

it('prefers functions based on name', () => {
const contract = {
functions: [
{ name: 'foo', isInitializer: true },
{ name: 'constructor', isInitializer: true },
],
} as ContractArtifact;
expect(getDefaultInitializer(contract)?.name).toEqual('constructor');
});

it('prefers functions based on parameter length', () => {
const contract = {
functions: [
{ name: 'foo', parameters: [{}], isInitializer: true },
{ name: 'bar', parameters: [], isInitializer: true },
],
} as ContractArtifact;
expect(getDefaultInitializer(contract)?.name).toEqual('bar');
});

it('prefers functions based on type', () => {
const contract = {
functions: [
{ name: 'foo', isInitializer: true, functionType: FunctionType.OPEN },
{ name: 'bar', isInitializer: true, functionType: FunctionType.SECRET },
],
} as ContractArtifact;
expect(getDefaultInitializer(contract)?.name).toEqual('bar');
});

it('returns an initializer if there is any', () => {
const contract = {
functions: [
{ name: 'foo', isInitializer: true },
{ name: 'bar', isInitializer: true },
],
} as ContractArtifact;
expect(getDefaultInitializer(contract)?.name).toBeDefined();
});
});

describe('getInitializer', () => {
it('returns initializer based on name', () => {
const contract = {
functions: [
{ name: 'foo', isInitializer: true },
{ name: 'bar', isInitializer: true },
],
} as ContractArtifact;
expect(getInitializer(contract, 'bar')?.name).toEqual('bar');
});

it('fails if named initializer not found', () => {
const contract = {
functions: [
{ name: 'foo', isInitializer: true },
{ name: 'bar', isInitializer: true },
],
} as ContractArtifact;
expect(() => getInitializer(contract, 'baz')).toThrow();
});

it('fails if named initializer not an initializer', () => {
const contract = {
functions: [
{ name: 'foo', isInitializer: true },
{ name: 'bar', isInitializer: false },
],
} as ContractArtifact;
expect(() => getInitializer(contract, 'bar')).toThrow();
});

it('falls back to default initializer on undefined argument', () => {
const contract = {
functions: [
{ name: 'foo', isInitializer: true },
{ name: 'initializer', isInitializer: true },
],
} as ContractArtifact;
expect(getInitializer(contract, undefined)?.name).toEqual('initializer');
});

it('passes artifact through', () => {
const contract = {} as ContractArtifact;
const artifact = { name: 'foo', isInitializer: true } as FunctionArtifact;
expect(getInitializer(contract, artifact)?.name).toEqual('foo');
});

it('validates artifact is initializer', () => {
const contract = {} as ContractArtifact;
const artifact = { name: 'foo', isInitializer: false } as FunctionArtifact;
expect(() => getInitializer(contract, artifact)).toThrow();
});
});
});
33 changes: 31 additions & 2 deletions yarn-project/foundation/src/abi/abi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,16 +342,45 @@ export function getFunctionDebugMetadata(

/**
* Returns an initializer from the contract, assuming there is at least one. If there are multiple initializers,
* it returns the one named "constructor"; if there is none with that name, it returns the first private initializer
* it finds.
* it returns the one named "constructor" or "initializer"; if there is none with that name, it returns the first
* initializer it finds, prioritizing initializers with no arguments and then private ones.
* @param contractArtifact - The contract artifact.
* @returns An initializer function, or none if there are no functions flagged as initializers in the contract.
*/
export function getDefaultInitializer(contractArtifact: ContractArtifact): FunctionArtifact | undefined {
const initializers = contractArtifact.functions.filter(f => f.isInitializer);
return initializers.length > 1
? initializers.find(f => f.name === 'constructor') ??
initializers.find(f => f.name === 'initializer') ??
initializers.find(f => f.parameters?.length === 0) ??
initializers.find(f => f.functionType === FunctionType.SECRET) ??
initializers[0]
: initializers[0];
}

/**
* Returns an initializer from the contract.
* @param initalizerNameOrArtifact - The name of the constructor, or the artifact of the constructor, or undefined
* to pick the default initializer.
*/
export function getInitializer(
contract: ContractArtifact,
initalizerNameOrArtifact: string | undefined | FunctionArtifact,
): FunctionArtifact | undefined {
if (typeof initalizerNameOrArtifact === 'string') {
const found = contract.functions.find(f => f.name === initalizerNameOrArtifact);
if (!found) {
throw new Error(`Constructor method ${initalizerNameOrArtifact} not found in contract artifact`);
} else if (!found.isInitializer) {
throw new Error(`Method ${initalizerNameOrArtifact} is not an initializer`);
}
return found;
} else if (initalizerNameOrArtifact === undefined) {
return getDefaultInitializer(contract);
} else {
if (!initalizerNameOrArtifact.isInitializer) {
throw new Error(`Method ${initalizerNameOrArtifact.name} is not an initializer`);
}
return initalizerNameOrArtifact;
}
}

0 comments on commit 10ad46f

Please sign in to comment.