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: Pending commitments contract using the wrong number of arguments #2959

Merged
merged 3 commits into from
Oct 23, 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
6 changes: 5 additions & 1 deletion yarn-project/acir-simulator/src/acvm/acvm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,5 +158,9 @@ export function extractCallStack(
return callStack;
}

return resolveOpcodeLocations(callStack, debug);
try {
return resolveOpcodeLocations(callStack, debug);
} catch (err) {
return callStack;
}
}
15 changes: 12 additions & 3 deletions yarn-project/acir-simulator/src/client/client_execution_context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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),
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
28 changes: 28 additions & 0 deletions yarn-project/foundation/src/abi/encoder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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]);
}
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down