Skip to content

Commit

Permalink
fix: Unrevert "feat: trace AVM side effects per enqueued call"" (#9095)
Browse files Browse the repository at this point in the history
Reverts #9058

Revives the original PR with the bug fixed
  • Loading branch information
dbanks12 authored Oct 8, 2024
1 parent 757ccef commit 72e4867
Show file tree
Hide file tree
Showing 14 changed files with 1,370 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ export class CombinedConstantData {
);
}

clone(): CombinedConstantData {
return CombinedConstantData.fromBuffer(this.toBuffer());
}

getSize() {
return (
this.historicalHeader.getSize() +
Expand Down
4 changes: 4 additions & 0 deletions yarn-project/circuits.js/src/structs/l2_to_l1_message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ export class L2ToL1Message {
isEmpty(): boolean {
return this.recipient.isZero() && this.content.isZero() && !this.counter;
}

scope(contractAddress: AztecAddress) {
return new ScopedL2ToL1Message(this, contractAddress);
}
}

export class ScopedL2ToL1Message {
Expand Down
4 changes: 4 additions & 0 deletions yarn-project/circuits.js/src/structs/log_hash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ export class LogHash implements Ordered {
return `value=${this.value} counter=${this.counter} length=${this.length}`;
}

scope(contractAddress: AztecAddress) {
return new ScopedLogHash(this, contractAddress);
}

[inspect.custom](): string {
return `LogHash { ${this.toString()} }`;
}
Expand Down
13 changes: 10 additions & 3 deletions yarn-project/prover-client/src/mocks/test_context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,14 @@ import {
type Tx,
type TxValidator,
} from '@aztec/circuit-types';
import { type Gas, type GlobalVariables, Header, type Nullifier, type TxContext } from '@aztec/circuits.js';
import {
type CombinedConstantData,
type Gas,
type GlobalVariables,
Header,
type Nullifier,
type TxContext,
} 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 Down Expand Up @@ -151,7 +158,7 @@ export class TestContext {
) {
const defaultExecutorImplementation = (
execution: PublicExecutionRequest,
_globalVariables: GlobalVariables,
_constants: CombinedConstantData,
availableGas: Gas,
_txContext: TxContext,
_pendingNullifiers: Nullifier[],
Expand Down Expand Up @@ -191,7 +198,7 @@ export class TestContext {
txValidator?: TxValidator<ProcessedTx>,
executorMock?: (
execution: PublicExecutionRequest,
globalVariables: GlobalVariables,
constants: CombinedConstantData,
availableGas: Gas,
txContext: TxContext,
pendingNullifiers: Nullifier[],
Expand Down
75 changes: 74 additions & 1 deletion yarn-project/simulator/src/avm/avm_simulator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { type Fieldable } from '@aztec/foundation/serialize';
import { randomInt } from 'crypto';
import { mock } from 'jest-mock-extended';

import { PublicEnqueuedCallSideEffectTrace } from '../public/enqueued_call_side_effect_trace.js';
import { type WorldStateDB } from '../public/public_db_sources.js';
import { type PublicSideEffectTraceInterface } from '../public/side_effect_trace_interface.js';
import { type AvmContext } from './avm_context.js';
Expand All @@ -30,7 +31,23 @@ import {
resolveAvmTestContractAssertionMessage,
} from './fixtures/index.js';
import { type AvmPersistableStateManager } from './journal/journal.js';
import { Add, CalldataCopy, Return, Set } from './opcodes/index.js';
import {
Add,
CalldataCopy,
EmitNoteHash,
EmitNullifier,
EmitUnencryptedLog,
type Instruction,
Jump,
L1ToL2MessageExists,
NoteHashExists,
NullifierExists,
Return,
SLoad,
SStore,
SendL2ToL1Message,
Set,
} from './opcodes/index.js';
import { encodeToBytecode } from './serialization/bytecode_serialization.js';
import { Opcode } from './serialization/instruction_serialization.js';
import {
Expand Down Expand Up @@ -955,6 +972,62 @@ describe('AVM simulator: transpiled Noir contracts', () => {
);
});
});

describe('Side effect trace errors on overflow', () => {
const trace = new PublicEnqueuedCallSideEffectTrace();
const persistableState = initPersistableStateManager({ worldStateDB, trace });

it.each([
['Public storage writes', () => new SStore(/*indirect=*/ 0, /*srcOffset=*/ 0, /*slotOffset=*/ 0)],
['Public storage reads', () => new SLoad(/*indirect=*/ 0, /*slotOffset=*/ 0, /*dstOffset=*/ 0)],
[
'Note hash checks',
() => new NoteHashExists(/*indirect=*/ 0, /*noteHashOffset=*/ 0, /*leafIndexOffest=*/ 0, /*existsOffset=*/ 1),
],
['New note hashes', () => new EmitNoteHash(/*indirect=*/ 0, /*noteHashOffset=*/ 0)],
[
'Nullifier checks',
() => new NullifierExists(/*indirect=*/ 0, /*nullifierOffset=*/ 0, /*addressOffest=*/ 0, /*existsOffset=*/ 1),
],
['New nullifiers', () => new EmitNullifier(/*indirect=*/ 0, /*noteHashOffset=*/ 0)],
[
'L1 to L2 message checks',
() =>
new L1ToL2MessageExists(
/*indirect=*/ 0,
/*msgHashOffset=*/ 0,
/*msgLeafIndexOffest=*/ 0,
/*existsOffset=*/ 1,
),
],
['New unencrypted logs', () => new EmitUnencryptedLog(/*indirect=*/ 0, /*logOffset=*/ 0, /*logSizeOffest=*/ 1)],
[
'New L1 to L2 messages',
() => new SendL2ToL1Message(/*indirect=*/ 0, /*recipientOffset=*/ 0, /*contentOffest=*/ 0),
],
])(`Overrun of %s`, async (_sideEffectType: string, createInstr: () => Instruction) => {
const bytecode = encodeToBytecode([
new Set(/*indirect*/ 0, TypeTag.FIELD, /*value*/ 0, /*dstOffset*/ 0).as(Opcode.SET_8, Set.wireFormat8),
new Set(/*indirect*/ 0, TypeTag.FIELD, /*value*/ 100, /*dstOffset*/ 100).as(Opcode.SET_8, Set.wireFormat8),
new Set(/*indirect*/ 0, TypeTag.UINT32, /*value*/ 1, /*dstOffset*/ 1).as(Opcode.SET_8, Set.wireFormat8),
createInstr(),
// change value at memory offset 0 so each instr operates on a different value (important for nullifier emission)
new Add(/*indirect=*/ 0, TypeTag.FIELD, /*aOffset=*/ 0, /*bOffset=*/ 100, /*dstOffset=*/ 0).as(
Opcode.ADD_8,
Add.wireFormat8,
),
// infinitely loop back to the tested instruction
// infinite loop should break on side effect overrun error,
// but otherwise will run out of gas
new Jump(/*jumpOffset*/ 2),
]);
const context = initContext({ persistableState });
const results = await new AvmSimulator(context).executeBytecode(markBytecodeAsAvm(bytecode));
expect(results.reverted).toBe(true);
expect(results.output).toEqual([]);
expect(results.revertReason?.message).toMatch('Reached the limit');
});
});
});
});

Expand Down
3 changes: 2 additions & 1 deletion yarn-project/simulator/src/avm/avm_simulator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { type DebugLogger, createDebugLogger } from '@aztec/foundation/log';

import { strict as assert } from 'assert';

import { SideEffectLimitReachedError } from '../public/side_effect_errors.js';
import type { AvmContext } from './avm_context.js';
import { AvmContractCallResult } from './avm_contract_call_result.js';
import { isAvmBytecode } from './bytecode_utils.js';
Expand Down Expand Up @@ -98,7 +99,7 @@ export class AvmSimulator {
return results;
} catch (err: any) {
this.log.verbose('Exceptional halt (revert by something other than REVERT opcode)');
if (!(err instanceof AvmExecutionError)) {
if (!(err instanceof AvmExecutionError || err instanceof SideEffectLimitReachedError)) {
this.log.verbose(`Unknown error thrown by AVM: ${err}`);
throw err;
}
Expand Down
4 changes: 2 additions & 2 deletions yarn-project/simulator/src/avm/journal/journal.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { AztecAddress, type FunctionSelector, type Gas } from '@aztec/circuits.js';
import { Fr } from '@aztec/foundation/fields';
import { type DebugLogger, createDebugLogger } from '@aztec/foundation/log';
import { createDebugLogger } from '@aztec/foundation/log';
import { SerializableContractInstance } from '@aztec/types/contracts';

import { getPublicFunctionDebugName } from '../../common/debug_fn_name.js';
Expand All @@ -22,7 +22,7 @@ import { PublicStorage } from './public_storage.js';
* Manages merging of successful/reverted child state into current state.
*/
export class AvmPersistableStateManager {
private readonly log: DebugLogger = createDebugLogger('aztec:avm_simulator:state_manager');
private readonly log = createDebugLogger('aztec:avm_simulator:state_manager');

constructor(
/** Reference to node storage */
Expand Down
169 changes: 169 additions & 0 deletions yarn-project/simulator/src/public/dual_side_effect_trace.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
import { type CombinedConstantData, type Gas, type VMCircuitPublicInputs } from '@aztec/circuits.js';
import { type Fr } from '@aztec/foundation/fields';
import { type ContractInstanceWithAddress } from '@aztec/types/contracts';

import { assert } from 'console';

import { type AvmContractCallResult } from '../avm/avm_contract_call_result.js';
import { type AvmExecutionEnvironment } from '../avm/avm_execution_environment.js';
import { type PublicEnqueuedCallSideEffectTrace } from './enqueued_call_side_effect_trace.js';
import { type PublicExecutionResult } from './execution.js';
import { type PublicSideEffectTrace } from './side_effect_trace.js';
import { type PublicSideEffectTraceInterface } from './side_effect_trace_interface.js';

export type TracedContractInstance = { exists: boolean } & ContractInstanceWithAddress;

export class DualSideEffectTrace implements PublicSideEffectTraceInterface {
constructor(
public readonly innerCallTrace: PublicSideEffectTrace,
public readonly enqueuedCallTrace: PublicEnqueuedCallSideEffectTrace,
) {}

public fork() {
return new DualSideEffectTrace(this.innerCallTrace.fork(), this.enqueuedCallTrace.fork());
}

public getCounter() {
assert(this.innerCallTrace.getCounter() == this.enqueuedCallTrace.getCounter());
return this.innerCallTrace.getCounter();
}

public tracePublicStorageRead(storageAddress: Fr, slot: Fr, value: Fr, exists: boolean, cached: boolean) {
this.innerCallTrace.tracePublicStorageRead(storageAddress, slot, value, exists, cached);
this.enqueuedCallTrace.tracePublicStorageRead(storageAddress, slot, value, exists, cached);
}

public tracePublicStorageWrite(storageAddress: Fr, slot: Fr, value: Fr) {
this.innerCallTrace.tracePublicStorageWrite(storageAddress, slot, value);
this.enqueuedCallTrace.tracePublicStorageWrite(storageAddress, slot, value);
}

// TODO(8287): _exists can be removed once we have the vm properly handling the equality check
public traceNoteHashCheck(_storageAddress: Fr, noteHash: Fr, leafIndex: Fr, exists: boolean) {
this.innerCallTrace.traceNoteHashCheck(_storageAddress, noteHash, leafIndex, exists);
this.enqueuedCallTrace.traceNoteHashCheck(_storageAddress, noteHash, leafIndex, exists);
}

public traceNewNoteHash(_storageAddress: Fr, noteHash: Fr) {
this.innerCallTrace.traceNewNoteHash(_storageAddress, noteHash);
this.enqueuedCallTrace.traceNewNoteHash(_storageAddress, noteHash);
}

public traceNullifierCheck(storageAddress: Fr, nullifier: Fr, leafIndex: Fr, exists: boolean, isPending: boolean) {
this.innerCallTrace.traceNullifierCheck(storageAddress, nullifier, leafIndex, exists, isPending);
this.enqueuedCallTrace.traceNullifierCheck(storageAddress, nullifier, leafIndex, exists, isPending);
}

public traceNewNullifier(storageAddress: Fr, nullifier: Fr) {
this.innerCallTrace.traceNewNullifier(storageAddress, nullifier);
this.enqueuedCallTrace.traceNewNullifier(storageAddress, nullifier);
}

public traceL1ToL2MessageCheck(contractAddress: Fr, msgHash: Fr, msgLeafIndex: Fr, exists: boolean) {
this.innerCallTrace.traceL1ToL2MessageCheck(contractAddress, msgHash, msgLeafIndex, exists);
this.enqueuedCallTrace.traceL1ToL2MessageCheck(contractAddress, msgHash, msgLeafIndex, exists);
}

public traceNewL2ToL1Message(contractAddress: Fr, recipient: Fr, content: Fr) {
this.innerCallTrace.traceNewL2ToL1Message(contractAddress, recipient, content);
this.enqueuedCallTrace.traceNewL2ToL1Message(contractAddress, recipient, content);
}

public traceUnencryptedLog(contractAddress: Fr, log: Fr[]) {
this.innerCallTrace.traceUnencryptedLog(contractAddress, log);
this.enqueuedCallTrace.traceUnencryptedLog(contractAddress, log);
}

public traceGetContractInstance(instance: TracedContractInstance) {
this.innerCallTrace.traceGetContractInstance(instance);
this.enqueuedCallTrace.traceGetContractInstance(instance);
}

/**
* Trace a nested call.
* Accept some results from a finished nested call's trace into this one.
*/
public traceNestedCall(
/** The trace of the nested call. */
nestedCallTrace: this,
/** The execution environment of the nested call. */
nestedEnvironment: AvmExecutionEnvironment,
/** How much gas was available for this public execution. */
startGasLeft: Gas,
/** How much gas was left after this public execution. */
endGasLeft: Gas,
/** Bytecode used for this execution. */
bytecode: Buffer,
/** The call's results */
avmCallResults: AvmContractCallResult,
/** Function name for logging */
functionName: string = 'unknown',
) {
this.innerCallTrace.traceNestedCall(
nestedCallTrace.innerCallTrace,
nestedEnvironment,
startGasLeft,
endGasLeft,
bytecode,
avmCallResults,
functionName,
);
this.enqueuedCallTrace.traceNestedCall(
nestedCallTrace.enqueuedCallTrace,
nestedEnvironment,
startGasLeft,
endGasLeft,
bytecode,
avmCallResults,
functionName,
);
}

/**
* Convert this trace to a PublicExecutionResult for use externally to the simulator.
*/
public toPublicExecutionResult(
/** The execution environment of the nested call. */
avmEnvironment: AvmExecutionEnvironment,
/** How much gas was available for this public execution. */
startGasLeft: Gas,
/** How much gas was left after this public execution. */
endGasLeft: Gas,
/** Bytecode used for this execution. */
bytecode: Buffer,
/** The call's results */
avmCallResults: AvmContractCallResult,
/** Function name for logging */
functionName: string = 'unknown',
): PublicExecutionResult {
return this.innerCallTrace.toPublicExecutionResult(
avmEnvironment,
startGasLeft,
endGasLeft,
bytecode,
avmCallResults,
functionName,
);
}

public toVMCircuitPublicInputs(
/** Constants */
constants: CombinedConstantData,
/** The execution environment of the nested call. */
avmEnvironment: AvmExecutionEnvironment,
/** How much gas was available for this public execution. */
startGasLeft: Gas,
/** How much gas was left after this public execution. */
endGasLeft: Gas,
/** The call's results */
avmCallResults: AvmContractCallResult,
): VMCircuitPublicInputs {
return this.enqueuedCallTrace.toVMCircuitPublicInputs(
constants,
avmEnvironment,
startGasLeft,
endGasLeft,
avmCallResults,
);
}
}
Loading

0 comments on commit 72e4867

Please sign in to comment.