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

chore: share state manager and side effect tracer for all enqueued calls in tx #9565

Merged
merged 2 commits into from
Nov 6, 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
2 changes: 1 addition & 1 deletion l1-contracts/src/core/libraries/ConstantsGen.sol
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ library Constants {
uint256 internal constant DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE =
14061769416655647708490531650437236735160113654556896985372298487345;
uint256 internal constant DEFAULT_GAS_LIMIT = 1000000000;
uint256 internal constant DEFAULT_TEARDOWN_GAS_LIMIT = 100000000;
uint256 internal constant DEFAULT_TEARDOWN_GAS_LIMIT = 12000000;
uint256 internal constant MAX_L2_GAS_PER_ENQUEUED_CALL = 12000000;
uint256 internal constant DEFAULT_MAX_FEE_PER_GAS = 10;
uint256 internal constant DEFAULT_INCLUSION_FEE = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ impl EnqueuedCallDataValidator {
) {
self.validate_global_variables(previous_kernel);
self.validate_against_call_request(previous_kernel);
self.validate_counters(previous_kernel);
self.validate_start_gas(previous_kernel);
self.validate_transaction_fee(previous_kernel);
self.validate_array_lengths(
Expand Down Expand Up @@ -86,14 +85,6 @@ impl EnqueuedCallDataValidator {
);
}

fn validate_counters(self, previous_kernel: PublicKernelCircuitPublicInputs) {
assert_eq(
self.enqueued_call.data.start_side_effect_counter,
previous_kernel.end_side_effect_counter + 1,
"enqueued call must start from the end counter of the previous call",
);
}

// Validates that the start gas injected into the vm circuit matches the remaining gas.
Comment on lines 87 to 88
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This constraint was failing because when an enqueued call had no side effects, it would start with start == prev.end. I was trying to figure out the proper way to modify this constraint or modify the AVM/tracing when I realized.... We're going to delete this anyway since side effect counters shouldn't be a critical part of the protocol with everything in the AVM.

fn validate_start_gas(self, previous_kernel: PublicKernelCircuitPublicInputs) {
let enqueued_call_start_gas = self.enqueued_call.data.start_gas_left;
Expand Down Expand Up @@ -165,7 +156,7 @@ impl EnqueuedCallDataValidator {
assert_eq(
self.enqueued_call.data.previous_accumulated_data_array_lengths,
prev_lengths,
"mismatch previoius_accumulated_data_array_lengths",
"mismatch previous_accumulated_data_array_lengths",
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ global DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE =

// GAS DEFAULTS
global DEFAULT_GAS_LIMIT: u32 = 1_000_000_000;
global DEFAULT_TEARDOWN_GAS_LIMIT: u32 = 100_000_000;
global DEFAULT_TEARDOWN_GAS_LIMIT: u32 = 12_000_000;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no reason the teardown limit should be higher than the enqueued call limit

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It maybe doesn't make sense to change this here, but I did so while debugging a test failure and thought... why change it back?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then maybe you can move the line past MAX_L2_GAS_PER_ENQUEUED_CALL and do

global DEFAULT_TEARDOWN_GAS_LIMIT  = MAX_L2_GAS_PER_ENQUEUED_CALL;

?

global MAX_L2_GAS_PER_ENQUEUED_CALL: u32 = 12_000_000;
global DEFAULT_MAX_FEE_PER_GAS: Field = 10;
global DEFAULT_INCLUSION_FEE: Field = 0;
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/bb-prover/src/avm_proving.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ const proveAndVerifyAvmTestContract = async (
);
}

const pxResult = trace.toPublicExecutionResult(
const pxResult = trace.toPublicFunctionCallResult(
environment,
startGas,
/*endGasLeft=*/ Gas.from(context.machineState.gasLeft),
Expand Down
4 changes: 2 additions & 2 deletions yarn-project/bb-prover/src/test/test_avm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ import {
} from '@aztec/circuits.js';
import { computeVarArgsHash } from '@aztec/circuits.js/hash';
import { padArrayEnd } from '@aztec/foundation/collection';
import { type PublicExecutionResult } from '@aztec/simulator';
import { type PublicFunctionCallResult } from '@aztec/simulator';

// TODO: pub somewhere more usable - copied from abstract phase manager
export function getPublicInputs(result: PublicExecutionResult): PublicCircuitPublicInputs {
export function getPublicInputs(result: PublicFunctionCallResult): PublicCircuitPublicInputs {
return PublicCircuitPublicInputs.from({
callContext: result.executionRequest.callContext,
proverAddress: AztecAddress.ZERO,
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/circuits.js/src/constants.gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export const REGISTERER_UNCONSTRAINED_FUNCTION_BROADCASTED_MAGIC_VALUE =
export const DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE =
14061769416655647708490531650437236735160113654556896985372298487345n;
export const DEFAULT_GAS_LIMIT = 1000000000;
export const DEFAULT_TEARDOWN_GAS_LIMIT = 100000000;
export const DEFAULT_TEARDOWN_GAS_LIMIT = 12000000;
export const MAX_L2_GAS_PER_ENQUEUED_CALL = 12000000;
export const DEFAULT_MAX_FEE_PER_GAS = 10;
export const DEFAULT_INCLUSION_FEE = 0;
Expand Down
89 changes: 86 additions & 3 deletions yarn-project/circuits.js/src/structs/avm/avm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,76 @@ import { Gas } from '../gas.js';
import { PublicCircuitPublicInputs } from '../public_circuit_public_inputs.js';
import { Vector } from '../shared.js';

export class AvmEnqueuedCallHint {
public readonly contractAddress: Fr;
public readonly calldata: Vector<Fr>;

constructor(contractAddress: Fr, calldata: Fr[]) {
this.contractAddress = contractAddress;
this.calldata = new Vector(calldata);
}

/* Serializes the inputs to a buffer.
* @returns - The inputs serialized to a buffer.
*/
toBuffer() {
return serializeToBuffer(...AvmEnqueuedCallHint.getFields(this));
}

/**
* Serializes the inputs to a hex string.
* @returns The instance serialized to a hex string.
*/
toString() {
return this.toBuffer().toString('hex');
}

/**
* Is the struct empty?
* @returns whether all members are empty.
*/
isEmpty(): boolean {
return this.contractAddress.isEmpty() && this.calldata.items.length == 0;
}

/**
* Creates a new instance from fields.
* @param fields - Fields to create the instance from.
* @returns A new AvmExecutionHints instance.
*/
static from(fields: FieldsOf<AvmEnqueuedCallHint>): AvmEnqueuedCallHint {
return new AvmEnqueuedCallHint(fields.contractAddress, fields.calldata.items);
}

/**
* Extracts fields from an instance.
* @param fields - Fields to create the instance from.
* @returns An array of fields.
*/
static getFields(fields: FieldsOf<AvmEnqueuedCallHint>) {
return [fields.contractAddress, fields.calldata] as const;
}

/**
* Deserializes from a buffer or reader.
* @param buffer - Buffer or reader to read from.
* @returns The deserialized instance.
*/
static fromBuffer(buff: Buffer | BufferReader) {
const reader = BufferReader.asReader(buff);
return new AvmEnqueuedCallHint(Fr.fromBuffer(reader), reader.readVector(Fr));
}

/**
* Deserializes from a hex string.
* @param str - Hex string to read from.
* @returns The deserialized instance.
*/
static fromString(str: string): AvmEnqueuedCallHint {
return AvmEnqueuedCallHint.fromBuffer(Buffer.from(str, 'hex'));
}
}

// TODO: Consider just using Tuple.
export class AvmKeyValueHint {
constructor(public readonly key: Fr, public readonly value: Fr) {}
Expand Down Expand Up @@ -364,8 +434,8 @@ export class AvmContractBytecodeHints {
}
}

// TODO(dbanks12): rename AvmCircuitHints
export class AvmExecutionHints {
public readonly enqueuedCalls: Vector<AvmEnqueuedCallHint>;
public readonly storageValues: Vector<AvmKeyValueHint>;
public readonly noteHashExists: Vector<AvmKeyValueHint>;
public readonly nullifierExists: Vector<AvmKeyValueHint>;
Expand All @@ -375,6 +445,7 @@ export class AvmExecutionHints {
public readonly contractBytecodeHints: Vector<AvmContractBytecodeHints>;

constructor(
enqueuedCalls: AvmEnqueuedCallHint[],
storageValues: AvmKeyValueHint[],
noteHashExists: AvmKeyValueHint[],
nullifierExists: AvmKeyValueHint[],
Expand All @@ -383,6 +454,7 @@ export class AvmExecutionHints {
contractInstances: AvmContractInstanceHint[],
contractBytecodeHints: AvmContractBytecodeHints[],
) {
this.enqueuedCalls = new Vector(enqueuedCalls);
this.storageValues = new Vector(storageValues);
this.noteHashExists = new Vector(noteHashExists);
this.nullifierExists = new Vector(nullifierExists);
Expand All @@ -397,7 +469,7 @@ export class AvmExecutionHints {
* @returns an empty instance.
*/
empty() {
return new AvmExecutionHints([], [], [], [], [], [], []);
return new AvmExecutionHints([], [], [], [], [], [], [], []);
}

/**
Expand All @@ -422,6 +494,7 @@ export class AvmExecutionHints {
*/
isEmpty(): boolean {
return (
this.enqueuedCalls.items.length == 0 &&
this.storageValues.items.length == 0 &&
this.noteHashExists.items.length == 0 &&
this.nullifierExists.items.length == 0 &&
Expand All @@ -439,6 +512,8 @@ export class AvmExecutionHints {
*/
static from(fields: FieldsOf<AvmExecutionHints>): AvmExecutionHints {
return new AvmExecutionHints(
// omit enqueued call hints until they're implemented in C++
new Array<AvmEnqueuedCallHint>(),
fields.storageValues.items,
fields.noteHashExists.items,
fields.nullifierExists.items,
Expand All @@ -456,6 +531,8 @@ export class AvmExecutionHints {
*/
static getFields(fields: FieldsOf<AvmExecutionHints>) {
return [
// omit enqueued call hints until they're implemented in C++
//fields.enqueuedCalls,
fields.storageValues,
fields.noteHashExists,
fields.nullifierExists,
Expand All @@ -474,6 +551,8 @@ export class AvmExecutionHints {
static fromBuffer(buff: Buffer | BufferReader): AvmExecutionHints {
const reader = BufferReader.asReader(buff);
return new AvmExecutionHints(
// omit enqueued call hints until they're implemented in C++
new Array<AvmEnqueuedCallHint>(),
reader.readVector(AvmKeyValueHint),
reader.readVector(AvmKeyValueHint),
reader.readVector(AvmKeyValueHint),
Expand All @@ -498,7 +577,7 @@ export class AvmExecutionHints {
* @returns The empty instance.
*/
static empty() {
return new AvmExecutionHints([], [], [], [], [], [], []);
return new AvmExecutionHints([], [], [], [], [], [], [], []);
}
}

Expand Down Expand Up @@ -547,6 +626,10 @@ export class AvmCircuitInputs {
);
}

static empty(): AvmCircuitInputs {
return new AvmCircuitInputs('', [], PublicCircuitPublicInputs.empty(), AvmExecutionHints.empty());
}

/**
* Creates a new instance from fields.
* @param fields - Fields to create the instance from.
Expand Down
23 changes: 16 additions & 7 deletions yarn-project/circuits.js/src/tests/factories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ import { GlobalVariables } from '../structs/global_variables.js';
import { Header } from '../structs/header.js';
import {
AvmContractBytecodeHints,
AvmEnqueuedCallHint,
AvmProofData,
BaseRollupHints,
EnqueuedCallData,
Expand Down Expand Up @@ -1490,6 +1491,13 @@ export function makeAvmContractInstanceHint(seed = 0): AvmContractInstanceHint {
);
}

export function makeAvmEnqueuedCallHint(seed = 0): AvmEnqueuedCallHint {
return AvmEnqueuedCallHint.from({
contractAddress: new Fr(seed),
calldata: makeVector((seed % 20) + 4, i => new Fr(i), seed + 0x1000),
});
}

/**
* Creates arbitrary AvmExecutionHints.
* @param seed - The seed to use for generating the hints.
Expand All @@ -1504,13 +1512,14 @@ export function makeAvmExecutionHints(
const baseLength = lengthOffset + (seed % lengthSeedMod);

return AvmExecutionHints.from({
storageValues: makeVector(baseLength, makeAvmKeyValueHint, seed + 0x4200),
noteHashExists: makeVector(baseLength + 1, makeAvmKeyValueHint, seed + 0x4300),
nullifierExists: makeVector(baseLength + 2, makeAvmKeyValueHint, seed + 0x4400),
l1ToL2MessageExists: makeVector(baseLength + 3, makeAvmKeyValueHint, seed + 0x4500),
externalCalls: makeVector(baseLength + 4, makeAvmExternalCallHint, seed + 0x4600),
contractInstances: makeVector(baseLength + 5, makeAvmContractInstanceHint, seed + 0x4700),
contractBytecodeHints: makeVector(baseLength + 6, makeAvmBytecodeHints, seed + 0x4800),
enqueuedCalls: makeVector(baseLength, makeAvmEnqueuedCallHint, seed + 0x4100),
storageValues: makeVector(baseLength + 1, makeAvmKeyValueHint, seed + 0x4200),
noteHashExists: makeVector(baseLength + 2, makeAvmKeyValueHint, seed + 0x4300),
nullifierExists: makeVector(baseLength + 3, makeAvmKeyValueHint, seed + 0x4400),
l1ToL2MessageExists: makeVector(baseLength + 4, makeAvmKeyValueHint, seed + 0x4500),
externalCalls: makeVector(baseLength + 5, makeAvmExternalCallHint, seed + 0x4600),
contractInstances: makeVector(baseLength + 6, makeAvmContractInstanceHint, seed + 0x4700),
contractBytecodeHints: makeVector(baseLength + 7, makeAvmBytecodeHints, seed + 0x4800),
...overrides,
});
}
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/ivc-integration/src/avm_integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ const proveAvmTestContract = async (
);
}

const pxResult = trace.toPublicExecutionResult(
const pxResult = trace.toPublicFunctionCallResult(
environment,
startGas,
/*endGasLeft=*/ Gas.from(context.machineState.gasLeft),
Expand Down
34 changes: 11 additions & 23 deletions yarn-project/prover-client/src/mocks/test_context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,7 @@ import {
type Tx,
type TxValidator,
} from '@aztec/circuit-types';
import {
type CombinedConstantData,
type Gas,
type GlobalVariables,
Header,
type Nullifier,
type TxContext,
} from '@aztec/circuits.js';
import { type Gas, type GlobalVariables, Header } from '@aztec/circuits.js';
import { type Fr } from '@aztec/foundation/fields';
import { type DebugLogger } from '@aztec/foundation/log';
import { openTmpStore } from '@aztec/kv-store/utils';
Expand All @@ -37,6 +30,7 @@ import * as fs from 'fs/promises';
import { type MockProxy, mock } from 'jest-mock-extended';

import { TestCircuitProver } from '../../../bb-prover/src/test/test_circuit_prover.js';
import { type AvmPersistableStateManager } from '../../../simulator/src/avm/journal/journal.js';
import { ProvingOrchestrator } from '../orchestrator/index.js';
import { MemoryProvingQueue } from '../prover-agent/memory-proving-queue.js';
import { ProverAgent } from '../prover-agent/prover-agent.js';
Expand Down Expand Up @@ -162,24 +156,20 @@ export class TestContext {
txValidator?: TxValidator<ProcessedTx>,
) {
const defaultExecutorImplementation = (
_stateManager: AvmPersistableStateManager,
execution: PublicExecutionRequest,
_constants: CombinedConstantData,
availableGas: Gas,
_txContext: TxContext,
_pendingNullifiers: Nullifier[],
transactionFee?: Fr,
_sideEffectCounter?: number,
_globalVariables: GlobalVariables,
allocatedGas: Gas,
_transactionFee?: Fr,
) => {
for (const tx of txs) {
const allCalls = tx.publicTeardownFunctionCall.isEmpty()
? tx.enqueuedPublicFunctionCalls
: [...tx.enqueuedPublicFunctionCalls, tx.publicTeardownFunctionCall];
for (const request of allCalls) {
if (execution.callContext.equals(request.callContext)) {
const result = PublicExecutionResultBuilder.fromPublicExecutionRequest({ request }).build({
startGasLeft: availableGas,
endGasLeft: availableGas,
transactionFee,
const result = PublicExecutionResultBuilder.empty().build({
endGasLeft: allocatedGas,
});
return Promise.resolve(result);
}
Expand All @@ -202,13 +192,11 @@ export class TestContext {
txHandler?: ProcessedTxHandler,
txValidator?: TxValidator<ProcessedTx>,
executorMock?: (
stateManager: AvmPersistableStateManager,
execution: PublicExecutionRequest,
constants: CombinedConstantData,
availableGas: Gas,
txContext: TxContext,
pendingNullifiers: Nullifier[],
globalVariables: GlobalVariables,
allocatedGas: Gas,
transactionFee?: Fr,
sideEffectCounter?: number,
) => Promise<PublicExecutionResult>,
) {
if (executorMock) {
Expand Down
Loading
Loading