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

fix(cli): Support initializers not named constructor in cli #5397

Merged
merged 2 commits into from
Mar 26, 2024
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
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;
}
}
Loading