From 655c322ab0e71074b3f747c95bfafbd6b7008217 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Mon, 23 Oct 2023 10:17:13 +0100 Subject: [PATCH] fix: Pending commitments contract using the wrong number of arguments (#2959) --- yarn-project/acir-simulator/src/acvm/acvm.ts | 6 +++- .../src/client/client_execution_context.ts | 15 ++++++++-- .../src/client/private_execution.ts | 2 +- yarn-project/foundation/src/abi/encoder.ts | 28 +++++++++++++++++++ .../pending_commitments_contract/src/main.nr | 25 +++++------------ 5 files changed, 53 insertions(+), 23 deletions(-) diff --git a/yarn-project/acir-simulator/src/acvm/acvm.ts b/yarn-project/acir-simulator/src/acvm/acvm.ts index 85287d2a1c3..68a91c61fac 100644 --- a/yarn-project/acir-simulator/src/acvm/acvm.ts +++ b/yarn-project/acir-simulator/src/acvm/acvm.ts @@ -158,5 +158,9 @@ export function extractCallStack( return callStack; } - return resolveOpcodeLocations(callStack, debug); + try { + return resolveOpcodeLocations(callStack, debug); + } catch (err) { + return callStack; + } } diff --git a/yarn-project/acir-simulator/src/client/client_execution_context.ts b/yarn-project/acir-simulator/src/client/client_execution_context.ts index 156f528e15e..0b64bcfd332 100644 --- a/yarn-project/acir-simulator/src/client/client_execution_context.ts +++ b/yarn-project/acir-simulator/src/client/client_execution_context.ts @@ -11,7 +11,7 @@ import { } from '@aztec/circuits.js'; import { computeUniqueCommitment, siloCommitment } from '@aztec/circuits.js/abis'; import { Grumpkin } from '@aztec/circuits.js/barretenberg'; -import { FunctionArtifact } from '@aztec/foundation/abi'; +import { FunctionAbi, FunctionArtifact, countArgumentsSize } from '@aztec/foundation/abi'; import { AztecAddress } from '@aztec/foundation/aztec-address'; import { Fr, Point } from '@aztec/foundation/fields'; import { createDebugLogger } from '@aztec/foundation/log'; @@ -83,11 +83,20 @@ export class ClientExecutionContext extends ViewDataOracle { // TODO When that is sorted out on noir side, we can use instead the utilities in serialize.ts /** * Writes the function inputs to the initial witness. + * @param abi - The function ABI. * @returns The initial witness. */ - public getInitialWitness() { + public getInitialWitness(abi: FunctionAbi) { const contractDeploymentData = this.txContext.contractDeploymentData; + const argumentsSize = countArgumentsSize(abi); + + const args = this.packedArgsCache.unpack(this.argsHash); + + if (args.length !== argumentsSize) { + throw new Error('Invalid arguments size'); + } + const fields = [ ...toACVMCallContext(this.callContext), ...toACVMHistoricBlockData(this.historicBlockData), @@ -96,7 +105,7 @@ export class ClientExecutionContext extends ViewDataOracle { this.txContext.chainId, this.txContext.version, - ...this.packedArgsCache.unpack(this.argsHash), + ...args, ]; return toACVMWitness(1, fields); diff --git a/yarn-project/acir-simulator/src/client/private_execution.ts b/yarn-project/acir-simulator/src/client/private_execution.ts index c6e8483641a..d2d43ca534d 100644 --- a/yarn-project/acir-simulator/src/client/private_execution.ts +++ b/yarn-project/acir-simulator/src/client/private_execution.ts @@ -27,7 +27,7 @@ export async function executePrivateFunction( log(`Executing external function ${contractAddress}:${functionSelector}`); const acir = Buffer.from(artifact.bytecode, 'base64'); - const initialWitness = context.getInitialWitness(); + const initialWitness = context.getInitialWitness(artifact); const acvmCallback = new Oracle(context); const { partialWitness } = await acvm(await AcirSimulator.getSolver(), acir, initialWitness, acvmCallback).catch( (err: Error) => { diff --git a/yarn-project/foundation/src/abi/encoder.ts b/yarn-project/foundation/src/abi/encoder.ts index 8fccb8b3838..7ebf8535554 100644 --- a/yarn-project/foundation/src/abi/encoder.ts +++ b/yarn-project/foundation/src/abi/encoder.ts @@ -10,6 +10,25 @@ class ArgumentEncoder { constructor(private abi: FunctionAbi, private args: any[]) {} + static typeSize(abiType: ABIType): number { + switch (abiType.kind) { + case 'field': + case 'boolean': + case 'integer': + return 1; + case 'string': + return abiType.length; + case 'array': + return abiType.length * ArgumentEncoder.typeSize(abiType.type); + case 'struct': + return abiType.fields.reduce((acc, field) => acc + ArgumentEncoder.typeSize(field.type), 0); + default: { + const exhaustiveCheck: never = abiType; + throw new Error(`Unhandled abi type: ${exhaustiveCheck}`); + } + } + } + /** * Encodes a single argument from the given type to field. * @param abiType - The abi type of the argument. @@ -89,3 +108,12 @@ class ArgumentEncoder { export function encodeArguments(abi: FunctionAbi, args: any[]) { return new ArgumentEncoder(abi, args).encode(); } + +/** + * Returns the size of the arguments for a function ABI. + * @param abi - The function ABI entry. + * @returns The size of the arguments. + */ +export function countArgumentsSize(abi: FunctionAbi) { + return abi.parameters.reduce((acc, parameter) => acc + ArgumentEncoder.typeSize(parameter.type), 0); +} diff --git a/yarn-project/noir-contracts/src/contracts/pending_commitments_contract/src/main.nr b/yarn-project/noir-contracts/src/contracts/pending_commitments_contract/src/main.nr index 7254f5cdd4b..fcd738e8a67 100644 --- a/yarn-project/noir-contracts/src/contracts/pending_commitments_contract/src/main.nr +++ b/yarn-project/noir-contracts/src/contracts/pending_commitments_contract/src/main.nr @@ -11,8 +11,8 @@ contract PendingCommitments { value_note::{VALUE_NOTE_LEN, ValueNote, ValueNoteMethods}, }; use dep::aztec::{ - constants_gen::ARGS_LENGTH, - context::Context, + context::{PrivateContext, PublicContext, Context}, + log::emit_encrypted_log, note::{ note_getter::NoteGetterOptions, note_header::NoteHeader, @@ -167,15 +167,10 @@ contract PendingCommitments { get_then_nullify_fn_selector: Field, get_note_zero_fn_selector: Field, ) { - // args for nested calls - let mut args = [0; ARGS_LENGTH]; - args[0] = amount; - args[1] = owner; - // nested call to create/insert note - let _callStackItem1 = context.call_private_function(inputs.call_context.storage_contract_address, insert_fn_selector, args); + let _callStackItem1 = context.call_private_function(inputs.call_context.storage_contract_address, insert_fn_selector, [amount, owner]); // nested call to read and nullify that note - let _callStackItem2 = context.call_private_function(inputs.call_context.storage_contract_address, get_then_nullify_fn_selector, args); + let _callStackItem2 = context.call_private_function(inputs.call_context.storage_contract_address, get_then_nullify_fn_selector, [amount, owner]); // nested call to confirm that balance is zero let _callStackItem3 = context.call_private_function(inputs.call_context.storage_contract_address, get_note_zero_fn_selector, [owner]); } @@ -189,9 +184,7 @@ contract PendingCommitments { get_then_nullify_fn_selector: Field, ) { // args for nested calls - let mut args = [0; ARGS_LENGTH]; - args[0] = amount; - args[1] = owner; + let args = [amount, owner]; // nested call to create/insert note let _callStackItem1 = context.call_private_function(inputs.call_context.storage_contract_address, insert_fn_selector, args); @@ -213,9 +206,7 @@ contract PendingCommitments { get_then_nullify_fn_selector: Field, ) { // args for nested calls - let mut args = [0; ARGS_LENGTH]; - args[0] = amount; - args[1] = owner; + let args = [amount, owner]; // nested call to create/insert note let _callStackItem1 = context.call_private_function(inputs.call_context.storage_contract_address, insert_fn_selector, args); @@ -236,9 +227,7 @@ contract PendingCommitments { get_note_zero_fn_selector: Field, ) { // args for nested calls - let mut args = [0; ARGS_LENGTH]; - args[0] = amount; - args[1] = owner; + let args = [amount, owner]; // nested call to create/insert note let _callStackItem1 = context.call_private_function(inputs.call_context.storage_contract_address, insert_fn_selector, args);