Skip to content

Commit

Permalink
fix(cli): Support initializers not named constructor in cli (#5397)
Browse files Browse the repository at this point in the history
Adds a new `--initializer` option to CLI deploy for specifying the name
of the initializer function, otherwise falls back to fetching the
default initializer (based on name, visibility, param length, etc).

Fixes issue reported by @signorecello.
  • Loading branch information
spalladino authored Mar 26, 2024
1 parent 1381064 commit 85f14c5
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`);
}, 60_000);

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 85f14c5

Please sign in to comment.