Skip to content

Commit

Permalink
refactor: remove PublicExecutor (#10028)
Browse files Browse the repository at this point in the history
- Pulls what the PublicExecutor did into an internal function of the
PublicTxSimulator
- Modifies the TXE to craft a carrier TX for public calls and just call
`publicTxSimulator.simulate(tx)` so that we only need to maintain that
interface for external use of the simulator
- Adds `gasLeft` to `AvmContractCallResult`, removes gasLeft as an extra
param to functions that accept call result.
- Introduces `AvmFinalizedCallResult` which is just like
`AvmContractCallResult` but is friendlier for use externally to the AVM.
Uses standard Gas type, and uses `SimulationError` type for
`revertReason`.
- Note that we might want to only have one gas type eventually, but even
just having this dedicated result type with revertReason of type
SimulationError led to some cleanup
- Removes bytecode size metering that was done in PublicExector (it
wasn't useful anymore)
- Removes unused/stale SimulationProvider paramters
  • Loading branch information
dbanks12 authored Nov 19, 2024
1 parent e1ef998 commit 9643dcd
Show file tree
Hide file tree
Showing 37 changed files with 620 additions and 510 deletions.
Original file line number Diff line number Diff line change
@@ -1,20 +1,30 @@
use crate::{test::utils, Token};
use aztec::oracle::random::random;
use aztec::protocol_types::abis::gas::Gas;
use aztec::protocol_types::abis::gas_fees::GasFees;

use dep::authwit::cheatcodes as authwit_cheatcodes;
use std::test::OracleMock;

#[test]
unconstrained fn setup_refund_success() {
// Gas used to compute transaction fee
// TXE oracle uses gas_used = Gas(1,1) when crafting TX
let txe_expected_gas_used = Gas::new(1, 1);
// TXE oracle uses default gas fees
let txe_gas_fees = GasFees::default();
let expected_tx_fee = txe_expected_gas_used.compute_fee(txe_gas_fees);

// Fund account with enough to cover tx fee plus some
let funded_amount = 1_000 + expected_tx_fee;

let (env, token_contract_address, owner, recipient, mint_amount) =
utils::setup_and_mint_to_private(true);
utils::setup_and_mint_amount_to_private(true, funded_amount);

// Renaming owner and recipient to match naming in Token
let user = owner;
let fee_payer = recipient;

let funded_amount = 1_000;

// We use the same randomness for both the fee payer, the user and the nonce as we currently don't have
// `OracleMock::mock_once()`
let fee_payer_randomness = 123;
Expand Down Expand Up @@ -44,19 +54,19 @@ unconstrained fn setup_refund_success() {
env,
token_contract_address,
fee_payer,
1,
expected_tx_fee,
fee_payer_randomness,
);
utils::add_token_note(
env,
token_contract_address,
user,
funded_amount - 1,
funded_amount - expected_tx_fee,
user_randomness,
);

utils::check_private_balance(token_contract_address, user, mint_amount - 1);
utils::check_private_balance(token_contract_address, fee_payer, 1)
utils::check_private_balance(token_contract_address, user, mint_amount - expected_tx_fee);
utils::check_private_balance(token_contract_address, fee_payer, expected_tx_fee)
}

// This test should be reworked when final support for partial notes is in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,19 +56,25 @@ pub unconstrained fn setup_and_mint_to_public(
(env, token_contract_address, owner, recipient, mint_amount)
}

pub unconstrained fn setup_and_mint_to_private(
pub unconstrained fn setup_and_mint_amount_to_private(
with_account_contracts: bool,
mint_amount: Field,
) -> (&mut TestEnvironment, AztecAddress, AztecAddress, AztecAddress, Field) {
// Setup the tokens and mint public balance
let (env, token_contract_address, owner, recipient) = setup(with_account_contracts);

// Mint some tokens
let mint_amount = 10000;
mint_to_private(env, token_contract_address, owner, mint_amount);

(env, token_contract_address, owner, recipient, mint_amount)
}

pub unconstrained fn setup_and_mint_to_private(
with_account_contracts: bool,
) -> (&mut TestEnvironment, AztecAddress, AztecAddress, AztecAddress, Field) {
setup_and_mint_amount_to_private(with_account_contracts, 10000)
}

pub unconstrained fn mint_to_private(
env: &mut TestEnvironment,
token_contract_address: AztecAddress,
Expand Down
5 changes: 1 addition & 4 deletions yarn-project/aztec-node/src/aztec-node/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ import {
} from '@aztec/p2p';
import { ProtocolContractAddress } from '@aztec/protocol-contracts';
import { GlobalVariableBuilder, SequencerClient } from '@aztec/sequencer-client';
import { PublicProcessorFactory, createSimulationProvider } from '@aztec/simulator';
import { PublicProcessorFactory } from '@aztec/simulator';
import { type TelemetryClient } from '@aztec/telemetry-client';
import { NoopTelemetryClient } from '@aztec/telemetry-client/noop';
import { createValidatorClient } from '@aztec/validator-client';
Expand Down Expand Up @@ -160,8 +160,6 @@ export class AztecNodeService implements AztecNode {
// start both and wait for them to sync from the block source
await Promise.all([p2pClient.start(), worldStateSynchronizer.start()]);

const simulationProvider = await createSimulationProvider(config, log);

const validatorClient = createValidatorClient(config, p2pClient, telemetry);

// now create the sequencer
Expand All @@ -175,7 +173,6 @@ export class AztecNodeService implements AztecNode {
archiver,
archiver,
archiver,
simulationProvider,
telemetry,
);

Expand Down
3 changes: 1 addition & 2 deletions yarn-project/bb-prover/src/avm_proving.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,8 @@ const proveAndVerifyAvmTestContract = async (
const pxResult = trace.toPublicFunctionCallResult(
environment,
startGas,
/*endGasLeft=*/ Gas.from(context.machineState.gasLeft),
/*bytecode=*/ simulator.getBytecode()!,
avmResult,
avmResult.finalize(),
functionName,
);

Expand Down
14 changes: 12 additions & 2 deletions yarn-project/circuit-types/src/public_execution_request.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { CallContext, type PublicCallRequest, Vector } from '@aztec/circuits.js';
import { CallContext, PublicCallRequest, Vector } from '@aztec/circuits.js';
import { computeVarArgsHash } from '@aztec/circuits.js/hash';
import { Fr } from '@aztec/foundation/fields';
import { schemas } from '@aztec/foundation/schemas';
Expand Down Expand Up @@ -82,9 +82,19 @@ export class PublicExecutionRequest {
);
}

toCallRequest(): PublicCallRequest {
return new PublicCallRequest(
this.callContext.msgSender,
this.callContext.contractAddress,
this.callContext.functionSelector,
this.callContext.isStaticCall,
computeVarArgsHash(this.args),
);
}

[inspect.custom]() {
return `PublicExecutionRequest {
callContext: ${this.callContext}
callContext: ${inspect(this.callContext)}
args: ${this.args}
}`;
}
Expand Down
6 changes: 0 additions & 6 deletions yarn-project/circuit-types/src/stats/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,6 @@ export const Metrics = [
description: 'Time to simulate an AVM program.',
events: ['avm-simulation'],
},
{
name: 'avm_simulation_bytecode_size_in_bytes',
groupBy: 'app-circuit-name',
description: 'Uncompressed bytecode size for an AVM program.',
events: ['avm-simulation'],
},
{
name: 'proof_construction_time_sha256_ms',
groupBy: 'threads',
Expand Down
2 changes: 0 additions & 2 deletions yarn-project/circuit-types/src/stats/stats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,6 @@ export type AvmSimulationStats = {
appCircuitName: string;
/** Duration in ms. */
duration: number;
/** Uncompressed bytecode size. */
bytecodeSize: number;
};

/** Stats for witness generation. */
Expand Down
16 changes: 16 additions & 0 deletions yarn-project/circuit-types/src/tx/tx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,22 @@ export class Tx extends Gossipable {
);
}

static newWithTxData(
data: PrivateKernelTailCircuitPublicInputs,
publicTeardownExecutionRequest?: PublicExecutionRequest,
) {
return new Tx(
data,
ClientIvcProof.empty(),
EncryptedNoteTxL2Logs.empty(),
EncryptedTxL2Logs.empty(),
UnencryptedTxL2Logs.empty(),
ContractClassTxL2Logs.empty(),
[],
publicTeardownExecutionRequest ? publicTeardownExecutionRequest : PublicExecutionRequest.empty(),
);
}

/**
* Serializes the Tx object into a Buffer.
* @returns Buffer representation of the Tx object.
Expand Down
10 changes: 10 additions & 0 deletions yarn-project/circuits.js/src/structs/call_context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { schemas } from '@aztec/foundation/schemas';
import { BufferReader, FieldReader, serializeToBuffer, serializeToFields } from '@aztec/foundation/serialize';
import { type FieldsOf } from '@aztec/foundation/types';

import { inspect } from 'util';
import { z } from 'zod';

import { CALL_CONTEXT_LENGTH } from '../constants.gen.js';
Expand Down Expand Up @@ -125,4 +126,13 @@ export class CallContext {
callContext.isStaticCall === this.isStaticCall
);
}

[inspect.custom]() {
return `CallContext {
msgSender: ${this.msgSender}
contractAddress: ${this.contractAddress}
functionSelector: ${this.functionSelector}
isStaticCall: ${this.isStaticCall}
}`;
}
}
3 changes: 1 addition & 2 deletions yarn-project/ivc-integration/src/avm_integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,8 @@ const proveAvmTestContract = async (
const pxResult = trace.toPublicFunctionCallResult(
environment,
startGas,
/*endGasLeft=*/ Gas.from(context.machineState.gasLeft),
/*bytecode=*/ simulator.getBytecode()!,
avmResult,
avmResult.finalize(),
functionName,
);

Expand Down
60 changes: 37 additions & 23 deletions yarn-project/prover-client/src/mocks/test_context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@ import { type Fr } from '@aztec/foundation/fields';
import { type DebugLogger } from '@aztec/foundation/log';
import { openTmpStore } from '@aztec/kv-store/utils';
import {
type PublicExecutionResult,
PublicExecutionResultBuilder,
type PublicExecutor,
PublicProcessor,
PublicTxSimulator,
type SimulationProvider,
WASMSimulator,
type WorldStateDB,
Expand All @@ -25,10 +23,12 @@ import { NoopTelemetryClient } from '@aztec/telemetry-client/noop';
import { MerkleTrees } from '@aztec/world-state';
import { NativeWorldStateService } from '@aztec/world-state/native';

import { jest } from '@jest/globals';
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 { AvmFinalizedCallResult } from '../../../simulator/src/avm/avm_contract_call_result.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';
Expand All @@ -37,7 +37,7 @@ import { getEnvironmentConfig, getSimulationProvider, makeGlobals } from './fixt

export class TestContext {
constructor(
public publicExecutor: MockProxy<PublicExecutor>,
public publicTxSimulator: PublicTxSimulator,
public worldStateDB: MockProxy<WorldStateDB>,
public publicProcessor: PublicProcessor,
public simulationProvider: SimulationProvider,
Expand Down Expand Up @@ -66,7 +66,6 @@ export class TestContext {
const directoriesToCleanup: string[] = [];
const globalVariables = makeGlobals(blockNumber);

const publicExecutor = mock<PublicExecutor>();
const worldStateDB = mock<WorldStateDB>();
const telemetry = new NoopTelemetryClient();

Expand All @@ -84,12 +83,13 @@ export class TestContext {
proverDb = await ws.getLatest();
}

const processor = PublicProcessor.create(
const publicTxSimulator = new PublicTxSimulator(publicDb, worldStateDB, telemetry, globalVariables);
const processor = new PublicProcessor(
publicDb,
publicExecutor,
globalVariables,
Header.empty(),
worldStateDB,
publicTxSimulator,
telemetry,
);

Expand Down Expand Up @@ -124,7 +124,7 @@ export class TestContext {
agent.start(queue);

return new this(
publicExecutor,
publicTxSimulator,
worldStateDB,
processor,
simulationProvider,
Expand Down Expand Up @@ -154,25 +154,24 @@ export class TestContext {
) {
const defaultExecutorImplementation = (
_stateManager: AvmPersistableStateManager,
execution: PublicExecutionRequest,
_globalVariables: GlobalVariables,
executionRequest: PublicExecutionRequest,
allocatedGas: Gas,
_transactionFee?: Fr,
_transactionFee: Fr,
_fnName: string,
) => {
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.empty().build({
endGasLeft: allocatedGas,
});
return Promise.resolve(result);
if (executionRequest.callContext.equals(request.callContext)) {
return Promise.resolve(
new AvmFinalizedCallResult(/*reverted=*/ false, /*output=*/ [], /*gasLeft=*/ allocatedGas),
);
}
}
}
throw new Error(`Unexpected execution request: ${execution}`);
throw new Error(`Unexpected execution request: ${executionRequest}`);
};
return await this.processPublicFunctionsWithMockExecutorImplementation(
txs,
Expand All @@ -183,21 +182,36 @@ export class TestContext {
);
}

public async processPublicFunctionsWithMockExecutorImplementation(
private async processPublicFunctionsWithMockExecutorImplementation(
txs: Tx[],
maxTransactions: number,
txHandler?: ProcessedTxHandler,
txValidator?: TxValidator<ProcessedTx>,
executorMock?: (
stateManager: AvmPersistableStateManager,
execution: PublicExecutionRequest,
globalVariables: GlobalVariables,
executionRequest: PublicExecutionRequest,
allocatedGas: Gas,
transactionFee?: Fr,
) => Promise<PublicExecutionResult>,
transactionFee: Fr,
fnName: string,
) => Promise<AvmFinalizedCallResult>,
) {
// Mock the internal private function. Borrowed from https://stackoverflow.com/a/71033167
const simulateInternal: jest.SpiedFunction<
(
stateManager: AvmPersistableStateManager,
executionResult: any,
allocatedGas: Gas,
transactionFee: any,
fnName: any,
) => Promise<AvmFinalizedCallResult>
> = jest.spyOn(
this.publicTxSimulator as unknown as {
simulateEnqueuedCallInternal: PublicTxSimulator['simulateEnqueuedCallInternal'];
},
'simulateEnqueuedCallInternal',
);
if (executorMock) {
this.publicExecutor.simulate.mockImplementation(executorMock);
simulateInternal.mockImplementation(executorMock);
}
return await this.publicProcessor.process(txs, maxTransactions, txHandler, txValidator);
}
Expand Down
4 changes: 0 additions & 4 deletions yarn-project/prover-node/src/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { type DataStoreConfig } from '@aztec/kv-store/config';
import { RollupAbi } from '@aztec/l1-artifacts';
import { createProverClient } from '@aztec/prover-client';
import { L1Publisher } from '@aztec/sequencer-client';
import { createSimulationProvider } from '@aztec/simulator';
import { type TelemetryClient } from '@aztec/telemetry-client';
import { NoopTelemetryClient } from '@aztec/telemetry-client/noop';
import { createWorldStateSynchronizer } from '@aztec/world-state';
Expand Down Expand Up @@ -43,8 +42,6 @@ export async function createProverNode(
const worldStateSynchronizer = await createWorldStateSynchronizer(worldStateConfig, archiver, telemetry);
await worldStateSynchronizer.start();

const simulationProvider = await createSimulationProvider(config, log);

const prover = await createProverClient(config, telemetry);

// REFACTOR: Move publisher out of sequencer package and into an L1-related package
Expand Down Expand Up @@ -82,7 +79,6 @@ export async function createProverNode(
archiver,
worldStateSynchronizer,
proverCoordination,
simulationProvider,
quoteProvider,
quoteSigner,
claimsMonitor,
Expand Down
Loading

0 comments on commit 9643dcd

Please sign in to comment.