From b143973dc81496070b62ca936dce485a855fe370 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 5 Nov 2024 09:34:19 +0000 Subject: [PATCH 1/6] fix: Have an AVM simulation error with revert data --- .../circuit-types/src/tx/processed_tx.ts | 5 ++- .../src/tx/public_simulation_output.ts | 36 +++++++++++++++++- .../src/e2e_crowdfunding_and_claim.test.ts | 2 +- .../pxe/src/pxe_service/error_enriching.ts | 7 ++-- .../pxe/src/pxe_service/pxe_service.ts | 26 +++++++------ yarn-project/simulator/src/mocks/fixtures.ts | 10 ++--- .../src/public/enqueued_call_simulator.ts | 4 +- .../src/public/enqueued_calls_processor.ts | 12 +++--- .../simulator/src/public/execution.ts | 4 +- .../src/public/public_processor.test.ts | 11 +++--- .../simulator/src/public/side_effect_trace.ts | 38 +++++++++++++++++-- yarn-project/txe/src/oracle/txe_oracle.ts | 1 - .../txe/src/txe_service/txe_service.ts | 2 - 13 files changed, 112 insertions(+), 46 deletions(-) diff --git a/yarn-project/circuit-types/src/tx/processed_tx.ts b/yarn-project/circuit-types/src/tx/processed_tx.ts index 63345772338f..504f6c8b9118 100644 --- a/yarn-project/circuit-types/src/tx/processed_tx.ts +++ b/yarn-project/circuit-types/src/tx/processed_tx.ts @@ -1,5 +1,6 @@ import { type AvmProvingRequest, + type AvmSimulationError, EncryptedNoteTxL2Logs, EncryptedTxL2Logs, PublicDataWrite, @@ -52,7 +53,7 @@ export type ProcessedTx = Pick fr.toString()), + }; + } + + static override fromJSON(obj: ReturnType) { + return new AvmSimulationError( + obj.originalMessage, + obj.functionErrorStack, + obj.revertData.map(serializedFr => Fr.fromString(serializedFr)), + obj.noirErrorStack, + ); + } +} + /** * Outputs of processing the public component of a transaction. */ @@ -44,7 +76,7 @@ export class PublicSimulationOutput { constructor( public encryptedLogs: EncryptedTxL2Logs, public unencryptedLogs: UnencryptedTxL2Logs, - public revertReason: SimulationError | undefined, + public revertReason: AvmSimulationError | undefined, public constants: CombinedConstantData, public end: CombinedAccumulatedData, public publicReturnValues: NestedProcessReturnValues[], diff --git a/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts b/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts index 96aad82e8c07..08a49bced6d6 100644 --- a/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts +++ b/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts @@ -346,7 +346,7 @@ describe('e2e_crowdfunding_and_claim', () => { donorWallets[1].setScopes([donorWallets[1].getAddress(), crowdfundingContract.address]); await expect(donorWallets[1].simulateTx(request, true, operatorWallet.getAddress())).rejects.toThrow( - 'Assertion failed: Users cannot set msg_sender in first call', + 'Circuit execution failed: Users cannot set msg_sender in first call', ); }); diff --git a/yarn-project/pxe/src/pxe_service/error_enriching.ts b/yarn-project/pxe/src/pxe_service/error_enriching.ts index 3cc946bfe3c3..bc284fe57d61 100644 --- a/yarn-project/pxe/src/pxe_service/error_enriching.ts +++ b/yarn-project/pxe/src/pxe_service/error_enriching.ts @@ -1,4 +1,4 @@ -import { type SimulationError, isNoirCallStackUnresolved } from '@aztec/circuit-types'; +import { type AvmSimulationError, type SimulationError, isNoirCallStackUnresolved } from '@aztec/circuit-types'; import { AztecAddress, Fr, FunctionSelector, PUBLIC_DISPATCH_SELECTOR } from '@aztec/circuits.js'; import { type DebugLogger } from '@aztec/foundation/log'; import { resolveAssertionMessageFromRevertData, resolveOpcodeLocations } from '@aztec/simulator'; @@ -53,8 +53,7 @@ export async function enrichSimulationError(err: SimulationError, db: PxeDatabas } export async function enrichPublicSimulationError( - err: SimulationError, - revertData: Fr[], + err: AvmSimulationError, contractDataOracle: ContractDataOracle, db: PxeDatabase, logger: DebugLogger, @@ -70,7 +69,7 @@ export async function enrichPublicSimulationError( originalFailingFunction.contractAddress, FunctionSelector.fromField(new Fr(PUBLIC_DISPATCH_SELECTOR)), ); - const assertionMessage = resolveAssertionMessageFromRevertData(revertData, artifact); + const assertionMessage = resolveAssertionMessageFromRevertData(err.revertData, artifact); if (assertionMessage) { err.setOriginalMessage(err.getOriginalMessage() + `${assertionMessage}`); } diff --git a/yarn-project/pxe/src/pxe_service/pxe_service.ts b/yarn-project/pxe/src/pxe_service/pxe_service.ts index 8f4db4709062..19a9b939d775 100644 --- a/yarn-project/pxe/src/pxe_service/pxe_service.ts +++ b/yarn-project/pxe/src/pxe_service/pxe_service.ts @@ -1,5 +1,6 @@ import { type AuthWitness, + AvmSimulationError, type AztecNode, EventMetadata, EventType, @@ -781,18 +782,21 @@ export class PXEService implements PXE { * @param tx - The transaction to be simulated. */ async #simulatePublicCalls(tx: Tx) { - const result = await this.node.simulatePublicCalls(tx); - if (result.revertReason) { - await enrichPublicSimulationError( - result.revertReason, - result.publicReturnValues[0].values || [], - this.contractDataOracle, - this.db, - this.log, - ); - throw result.revertReason; + // Simulating public calls can throw if the TX fails in a phase that doesn't allow reverts (setup) + // Or return as reverted if it fails in a phase that allows reverts (app logic, teardown) + try { + const result = await this.node.simulatePublicCalls(tx); + if (result.revertReason) { + await enrichPublicSimulationError(result.revertReason, this.contractDataOracle, this.db, this.log); + throw result.revertReason; + } + return result; + } catch (err) { + if (err instanceof AvmSimulationError) { + await enrichPublicSimulationError(err, this.contractDataOracle, this.db, this.log); + } + throw err; } - return result; } async #profileKernelProver( diff --git a/yarn-project/simulator/src/mocks/fixtures.ts b/yarn-project/simulator/src/mocks/fixtures.ts index ccba4e58f8d8..839d963ca75c 100644 --- a/yarn-project/simulator/src/mocks/fixtures.ts +++ b/yarn-project/simulator/src/mocks/fixtures.ts @@ -1,7 +1,7 @@ import { + type AvmSimulationError, type FunctionCall, PublicExecutionRequest, - type SimulationError, UnencryptedFunctionL2Logs, } from '@aztec/circuit-types'; import { @@ -27,7 +27,7 @@ export class PublicExecutionResultBuilder { private _contractStorageReads: ContractStorageRead[] = []; private _returnValues: Fr[] = []; private _reverted = false; - private _revertReason: SimulationError | undefined = undefined; + private _revertReason: AvmSimulationError | undefined = undefined; constructor(executionRequest: PublicExecutionRequest) { this._executionRequest = executionRequest; @@ -46,7 +46,7 @@ export class PublicExecutionResultBuilder { nestedExecutions?: PublicExecutionResult[]; contractStorageUpdateRequests?: ContractStorageUpdateRequest[]; contractStorageReads?: ContractStorageRead[]; - revertReason?: SimulationError; + revertReason?: AvmSimulationError; }): PublicExecutionResultBuilder { const builder = new PublicExecutionResultBuilder(request); @@ -76,7 +76,7 @@ export class PublicExecutionResultBuilder { nestedExecutions?: PublicExecutionResult[]; contractStorageUpdateRequests?: ContractStorageUpdateRequest[]; contractStorageReads?: ContractStorageRead[]; - revertReason?: SimulationError; + revertReason?: AvmSimulationError; }) { const builder = new PublicExecutionResultBuilder( new PublicExecutionRequest(new CallContext(from, tx.to, tx.selector, false), tx.args), @@ -113,7 +113,7 @@ export class PublicExecutionResultBuilder { return this; } - withReverted(reason: SimulationError): PublicExecutionResultBuilder { + withReverted(reason: AvmSimulationError): PublicExecutionResultBuilder { this._reverted = true; this._revertReason = reason; return this; diff --git a/yarn-project/simulator/src/public/enqueued_call_simulator.ts b/yarn-project/simulator/src/public/enqueued_call_simulator.ts index c5c501ef551e..d79bcc5cf5fb 100644 --- a/yarn-project/simulator/src/public/enqueued_call_simulator.ts +++ b/yarn-project/simulator/src/public/enqueued_call_simulator.ts @@ -1,11 +1,11 @@ import { type AvmProvingRequest, + type AvmSimulationError, MerkleTreeId, NestedProcessReturnValues, ProvingRequestType, type PublicExecutionRequest, PublicKernelPhase, - type SimulationError, type Tx, UnencryptedFunctionL2Logs, } from '@aztec/circuit-types'; @@ -84,7 +84,7 @@ export type EnqueuedCallResult = { /** Gas used during the execution this enqueued call */ gasUsed: Gas; /** Revert reason, if any */ - revertReason?: SimulationError; + revertReason?: AvmSimulationError; }; export class EnqueuedCallSimulator { diff --git a/yarn-project/simulator/src/public/enqueued_calls_processor.ts b/yarn-project/simulator/src/public/enqueued_calls_processor.ts index 6d0d1616d358..ad89150b1bb6 100644 --- a/yarn-project/simulator/src/public/enqueued_calls_processor.ts +++ b/yarn-project/simulator/src/public/enqueued_calls_processor.ts @@ -1,11 +1,11 @@ import { type AvmProvingRequest, + type AvmSimulationError, type MerkleTreeReadOperations, type NestedProcessReturnValues, type ProcessedTx, type PublicExecutionRequest, PublicKernelPhase, - type SimulationError, type Tx, } from '@aztec/circuit-types'; import { @@ -54,13 +54,13 @@ type PublicPhaseResult = { /** Time spent for the execution this phase */ durationMs: number; /** Revert reason, if any */ - revertReason?: SimulationError; + revertReason?: AvmSimulationError; }; export type ProcessedPhase = { phase: PublicKernelPhase; durationMs: number; - revertReason?: SimulationError; + revertReason?: AvmSimulationError; }; export type TxPublicCallsResult = { @@ -72,7 +72,7 @@ export type TxPublicCallsResult = { /** Gas used during the execution this tx */ gasUsed: ProcessedTx['gasUsed']; /** Revert reason, if any */ - revertReason?: SimulationError; + revertReason?: AvmSimulationError; processedPhases: ProcessedPhase[]; }; @@ -160,7 +160,7 @@ export class EnqueuedCallsProcessor { let publicKernelOutput = tx.data.toPublicKernelCircuitPublicInputs(); let isFromPrivate = true; let returnValues: NestedProcessReturnValues[] = []; - let revertReason: SimulationError | undefined; + let revertReason: AvmSimulationError | undefined; for (let i = 0; i < phases.length; i++) { const phase = phases[i]; const callRequests = EnqueuedCallsProcessor.getCallRequestsByPhase(tx, phase); @@ -233,7 +233,7 @@ export class EnqueuedCallsProcessor { let avmProvingRequest: AvmProvingRequest; let publicKernelOutput = previousPublicKernelOutput; let gasUsed = Gas.empty(); - let revertReason: SimulationError | undefined; + let revertReason: AvmSimulationError | undefined; for (let i = callRequests.length - 1; i >= 0 && !revertReason; i--) { const callRequest = callRequests[i]; const executionRequest = executionRequests[i]; diff --git a/yarn-project/simulator/src/public/execution.ts b/yarn-project/simulator/src/public/execution.ts index 94fcc6106afa..26b37cdf9a8a 100644 --- a/yarn-project/simulator/src/public/execution.ts +++ b/yarn-project/simulator/src/public/execution.ts @@ -1,7 +1,7 @@ import { + type AvmSimulationError, NestedProcessReturnValues, type PublicExecutionRequest, - type SimulationError, type UnencryptedFunctionL2Logs, } from '@aztec/circuit-types'; import { @@ -49,7 +49,7 @@ export interface PublicExecutionResult { /** Whether the execution reverted. */ reverted: boolean; /** The revert reason if the execution reverted. */ - revertReason?: SimulationError; + revertReason?: AvmSimulationError; /** The contract storage reads performed by the function. */ contractStorageReads: ContractStorageRead[]; diff --git a/yarn-project/simulator/src/public/public_processor.test.ts b/yarn-project/simulator/src/public/public_processor.test.ts index acede1589750..e6dc1e1eeacd 100644 --- a/yarn-project/simulator/src/public/public_processor.test.ts +++ b/yarn-project/simulator/src/public/public_processor.test.ts @@ -1,4 +1,5 @@ import { + AvmSimulationError, type MerkleTreeWriteOperations, type ProcessedTx, type ProcessedTxHandler, @@ -329,7 +330,7 @@ describe('public_processor', () => { PublicExecutionResultBuilder.fromFunctionCall({ from: revertibleRequests[0].callContext.contractAddress, tx: makeFunctionCall('', nestedContractAddress, makeSelector(5)), - revertReason: new SimulationError('Simulation Failed', []), + revertReason: new AvmSimulationError('Simulation Failed', [], []), }).build(), ], }).build(), @@ -434,7 +435,7 @@ describe('public_processor', () => { PublicExecutionResultBuilder.fromFunctionCall({ from: nonRevertibleRequests[0].callContext.contractAddress, tx: makeFunctionCall('', nestedContractAddress, makeSelector(5)), - revertReason: new SimulationError('Simulation Failed', []), + revertReason: new AvmSimulationError('Simulation Failed', [], []), }).build(), ], }).build(), @@ -546,7 +547,7 @@ describe('public_processor', () => { from: teardownRequest.callContext.contractAddress, tx: makeFunctionCall('', nestedContractAddress, makeSelector(5)), contractStorageUpdateRequests: [new ContractStorageUpdateRequest(contractSlotC, fr(0x202), 17)], - revertReason: new SimulationError('Simulation Failed', []), + revertReason: new AvmSimulationError('Simulation Failed', [], []), }).build(teardownResultSettings), ], }).build(teardownResultSettings), @@ -644,7 +645,7 @@ describe('public_processor', () => { new ContractStorageUpdateRequest(contractSlotB, fr(0x152), 14), new ContractStorageUpdateRequest(contractSlotC, fr(0x201), 15), ], - revertReason: new SimulationError('Simulation Failed', []), + revertReason: new AvmSimulationError('Simulation Failed', [], []), }).build(), // Teardown @@ -660,7 +661,7 @@ describe('public_processor', () => { from: teardownRequest.callContext.contractAddress, tx: makeFunctionCall('', nestedContractAddress, makeSelector(5)), contractStorageUpdateRequests: [new ContractStorageUpdateRequest(contractSlotC, fr(0x202), 16)], - revertReason: new SimulationError('Simulation Failed', []), + revertReason: new AvmSimulationError('Simulation Failed', [], []), }).build(teardownResultSettings), ], }).build(teardownResultSettings), diff --git a/yarn-project/simulator/src/public/side_effect_trace.ts b/yarn-project/simulator/src/public/side_effect_trace.ts index 9116e5660942..703edf80f2d9 100644 --- a/yarn-project/simulator/src/public/side_effect_trace.ts +++ b/yarn-project/simulator/src/public/side_effect_trace.ts @@ -1,4 +1,11 @@ -import { PublicExecutionRequest, UnencryptedFunctionL2Logs, UnencryptedL2Log } from '@aztec/circuit-types'; +import { + AvmSimulationError, + type FailingFunction, + type NoirCallStack, + PublicExecutionRequest, + UnencryptedFunctionL2Logs, + UnencryptedL2Log, +} from '@aztec/circuit-types'; import { AvmContractBytecodeHints, AvmContractInstanceHint, @@ -37,7 +44,7 @@ import { createDebugLogger } from '@aztec/foundation/log'; import { type AvmContractCallResult } from '../avm/avm_contract_call_result.js'; import { type AvmExecutionEnvironment } from '../avm/avm_execution_environment.js'; -import { createSimulationError } from '../common/errors.js'; +import { ExecutionError, traverseCauseChain } from '../common/errors.js'; import { type PublicExecutionResult, resultToPublicCallRequest } from './execution.js'; import { SideEffectLimitReachedError } from './side_effect_errors.js'; import { type PublicSideEffectTraceInterface } from './side_effect_trace_interface.js'; @@ -361,7 +368,9 @@ export class PublicSideEffectTrace implements PublicSideEffectTraceInterface { calldata: avmEnvironment.calldata, returnValues: avmCallResults.output, reverted: avmCallResults.reverted, - revertReason: avmCallResults.revertReason ? createSimulationError(avmCallResults.revertReason) : undefined, + revertReason: avmCallResults.revertReason + ? createAvmSimulationError(avmCallResults.revertReason, avmCallResults.output) + : undefined, contractStorageReads: this.contractStorageReads, contractStorageUpdateRequests: this.contractStorageUpdateRequests, @@ -420,3 +429,26 @@ function createPublicExecutionRequest(avmEnvironment: AvmExecutionEnvironment): }); return new PublicExecutionRequest(callContext, avmEnvironment.calldata); } + +/** + * Creates an AVM simulation error from an error chain generated during the execution of public functions. + * @param error - The error thrown during execution. + * @returns - An AVM simulation error. + */ +function createAvmSimulationError(error: Error, revertData: Fr[]): AvmSimulationError { + let rootCause = error; + let noirCallStack: NoirCallStack | undefined = undefined; + const aztecCallStack: FailingFunction[] = []; + + traverseCauseChain(error, cause => { + rootCause = cause; + if (cause instanceof ExecutionError) { + aztecCallStack.push(cause.failingFunction); + if (cause.noirCallStack) { + noirCallStack = cause.noirCallStack; + } + } + }); + + return new AvmSimulationError(rootCause.message, aztecCallStack, revertData, noirCallStack, { cause: rootCause }); +} diff --git a/yarn-project/txe/src/oracle/txe_oracle.ts b/yarn-project/txe/src/oracle/txe_oracle.ts index b73adb1bb328..95442df89437 100644 --- a/yarn-project/txe/src/oracle/txe_oracle.ts +++ b/yarn-project/txe/src/oracle/txe_oracle.ts @@ -695,7 +695,6 @@ export class TXE implements TypedOracle { if (executionResult.revertReason && executionResult.revertReason instanceof SimulationError) { await enrichPublicSimulationError( executionResult.revertReason, - executionResult.returnValues, this.contractDataOracle, this.txeDatabase, this.logger, diff --git a/yarn-project/txe/src/txe_service/txe_service.ts b/yarn-project/txe/src/txe_service/txe_service.ts index c5d36e100f38..52ceb74f06f5 100644 --- a/yarn-project/txe/src/txe_service/txe_service.ts +++ b/yarn-project/txe/src/txe_service/txe_service.ts @@ -743,7 +743,6 @@ export class TXEService { if (result.revertReason && result.revertReason instanceof SimulationError) { await enrichPublicSimulationError( result.revertReason, - result.returnValues, (this.typedOracle as TXE).getContractDataOracle(), (this.typedOracle as TXE).getTXEDatabase(), this.logger, @@ -774,7 +773,6 @@ export class TXEService { if (result.revertReason && result.revertReason instanceof SimulationError) { await enrichPublicSimulationError( result.revertReason, - result.returnValues, (this.typedOracle as TXE).getContractDataOracle(), (this.typedOracle as TXE).getTXEDatabase(), this.logger, From 5e95da78d04eb1c9e33137f8b01176a111d1a34a Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 5 Nov 2024 09:36:57 +0000 Subject: [PATCH 2/6] reenable failing tests --- scripts/ci/get_e2e_jobs.sh | 3 +++ yarn-project/end-to-end/scripts/e2e_test_config.yml | 13 +++++-------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/scripts/ci/get_e2e_jobs.sh b/scripts/ci/get_e2e_jobs.sh index 772c2c6b53ac..c7e9d26b55f2 100755 --- a/scripts/ci/get_e2e_jobs.sh +++ b/scripts/ci/get_e2e_jobs.sh @@ -23,10 +23,13 @@ allow_list=( "e2e_avm_simulator" "e2e_block_building" "e2e_cross_chain_messaging" + "e2e_crowdfunding_and_claim" "e2e_deploy_contract" "e2e_fees" + "e2e_fees_failures" "e2e_fees_gas_estimation" "e2e_fees_private_payments" + "e2e_fees_private_refunds" "e2e_max_block_number" "e2e_nested_contract" "e2e_ordering" diff --git a/yarn-project/end-to-end/scripts/e2e_test_config.yml b/yarn-project/end-to-end/scripts/e2e_test_config.yml index 76059945180c..4ed71e04d03e 100644 --- a/yarn-project/end-to-end/scripts/e2e_test_config.yml +++ b/yarn-project/end-to-end/scripts/e2e_test_config.yml @@ -30,8 +30,7 @@ tests: e2e_card_game: {} e2e_cheat_codes: {} e2e_cross_chain_messaging: {} - # TODO reenable in https://github.com/AztecProtocol/aztec-packages/pull/9727 - # e2e_crowdfunding_and_claim: {} + e2e_crowdfunding_and_claim: {} e2e_deploy_contract: {} e2e_devnet_smoke: {} docs_examples: @@ -42,18 +41,16 @@ tests: # TODO(https://github.com/AztecProtocol/aztec-packages/issues/9488): reenable # e2e_fees_dapp_subscription: # test_path: "e2e_fees/dapp_subscription.test.ts" - # TODO reenable in https://github.com/AztecProtocol/aztec-packages/pull/9727 - # e2e_fees_failures: - # test_path: 'e2e_fees/failures.test.ts' + e2e_fees_failures: + test_path: 'e2e_fees/failures.test.ts' e2e_fees_fee_juice_payments: test_path: 'e2e_fees/fee_juice_payments.test.ts' e2e_fees_gas_estimation: test_path: 'e2e_fees/gas_estimation.test.ts' e2e_fees_private_payments: test_path: 'e2e_fees/private_payments.test.ts' - # TODO reenable in https://github.com/AztecProtocol/aztec-packages/pull/9727 - # e2e_fees_private_refunds: - # test_path: 'e2e_fees/private_refunds.test.ts' + e2e_fees_private_refunds: + test_path: 'e2e_fees/private_refunds.test.ts' e2e_keys: {} e2e_l1_with_wall_time: {} e2e_lending_contract: {} From 638416f424d9c676b2213bbeff92beea59aeb297 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 5 Nov 2024 10:03:36 +0000 Subject: [PATCH 3/6] fix --- yarn-project/pxe/src/pxe_service/pxe_service.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/yarn-project/pxe/src/pxe_service/pxe_service.ts b/yarn-project/pxe/src/pxe_service/pxe_service.ts index 19a9b939d775..3572cb92075e 100644 --- a/yarn-project/pxe/src/pxe_service/pxe_service.ts +++ b/yarn-project/pxe/src/pxe_service/pxe_service.ts @@ -784,19 +784,20 @@ export class PXEService implements PXE { async #simulatePublicCalls(tx: Tx) { // Simulating public calls can throw if the TX fails in a phase that doesn't allow reverts (setup) // Or return as reverted if it fails in a phase that allows reverts (app logic, teardown) + let result; try { - const result = await this.node.simulatePublicCalls(tx); - if (result.revertReason) { - await enrichPublicSimulationError(result.revertReason, this.contractDataOracle, this.db, this.log); - throw result.revertReason; - } - return result; + result = await this.node.simulatePublicCalls(tx); } catch (err) { if (err instanceof AvmSimulationError) { await enrichPublicSimulationError(err, this.contractDataOracle, this.db, this.log); } throw err; } + if (result.revertReason) { + await enrichPublicSimulationError(result.revertReason, this.contractDataOracle, this.db, this.log); + throw result.revertReason; + } + return result; } async #profileKernelProver( From 24beac0bc278612f2d0d41824702c232ffa17ded Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 5 Nov 2024 10:07:07 +0000 Subject: [PATCH 4/6] refactor --- yarn-project/pxe/src/pxe_service/pxe_service.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/yarn-project/pxe/src/pxe_service/pxe_service.ts b/yarn-project/pxe/src/pxe_service/pxe_service.ts index 3572cb92075e..bbdfa52364f4 100644 --- a/yarn-project/pxe/src/pxe_service/pxe_service.ts +++ b/yarn-project/pxe/src/pxe_service/pxe_service.ts @@ -784,20 +784,18 @@ export class PXEService implements PXE { async #simulatePublicCalls(tx: Tx) { // Simulating public calls can throw if the TX fails in a phase that doesn't allow reverts (setup) // Or return as reverted if it fails in a phase that allows reverts (app logic, teardown) - let result; try { - result = await this.node.simulatePublicCalls(tx); + const result = await this.node.simulatePublicCalls(tx); + if (result.revertReason) { + throw result.revertReason; + } + return result; } catch (err) { if (err instanceof AvmSimulationError) { await enrichPublicSimulationError(err, this.contractDataOracle, this.db, this.log); } throw err; } - if (result.revertReason) { - await enrichPublicSimulationError(result.revertReason, this.contractDataOracle, this.db, this.log); - throw result.revertReason; - } - return result; } async #profileKernelProver( From 76c3d5ac652d444327c1ce7850ab5e4905c8c61d Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 5 Nov 2024 11:36:16 +0000 Subject: [PATCH 5/6] refactor: simulation error with revertdata --- .../circuit-types/src/simulation_error.ts | 11 ++++-- .../circuit-types/src/tx/processed_tx.ts | 5 ++- .../src/tx/public_simulation_output.ts | 36 ++----------------- .../pxe/src/pxe_service/error_enriching.ts | 4 +-- .../pxe/src/pxe_service/pxe_service.ts | 3 +- yarn-project/simulator/src/common/errors.ts | 5 +-- yarn-project/simulator/src/mocks/fixtures.ts | 10 +++--- .../src/public/enqueued_call_simulator.ts | 4 +-- .../src/public/enqueued_calls_processor.ts | 12 +++---- .../simulator/src/public/execution.ts | 4 +-- .../src/public/public_processor.test.ts | 11 +++--- .../simulator/src/public/side_effect_trace.ts | 29 ++------------- 12 files changed, 42 insertions(+), 92 deletions(-) diff --git a/yarn-project/circuit-types/src/simulation_error.ts b/yarn-project/circuit-types/src/simulation_error.ts index 2d88c1e68086..6b0a695c5194 100644 --- a/yarn-project/circuit-types/src/simulation_error.ts +++ b/yarn-project/circuit-types/src/simulation_error.ts @@ -1,4 +1,4 @@ -import { type AztecAddress, type FunctionSelector } from '@aztec/circuits.js'; +import { type AztecAddress, Fr, type FunctionSelector } from '@aztec/circuits.js'; import { type OpcodeLocation } from '@aztec/foundation/abi'; /** @@ -68,6 +68,7 @@ export class SimulationError extends Error { constructor( private originalMessage: string, private functionErrorStack: FailingFunction[], + public revertData: Fr[] = [], private noirErrorStack?: NoirCallStack, options?: ErrorOptions, ) { @@ -202,10 +203,16 @@ export class SimulationError extends Error { originalMessage: this.originalMessage, functionErrorStack: this.functionErrorStack, noirErrorStack: this.noirErrorStack, + revertData: this.revertData.map(fr => fr.toString()), }; } static fromJSON(obj: ReturnType) { - return new SimulationError(obj.originalMessage, obj.functionErrorStack, obj.noirErrorStack); + return new SimulationError( + obj.originalMessage, + obj.functionErrorStack, + obj.revertData.map(serializedFr => Fr.fromString(serializedFr)), + obj.noirErrorStack, + ); } } diff --git a/yarn-project/circuit-types/src/tx/processed_tx.ts b/yarn-project/circuit-types/src/tx/processed_tx.ts index 504f6c8b9118..63345772338f 100644 --- a/yarn-project/circuit-types/src/tx/processed_tx.ts +++ b/yarn-project/circuit-types/src/tx/processed_tx.ts @@ -1,6 +1,5 @@ import { type AvmProvingRequest, - type AvmSimulationError, EncryptedNoteTxL2Logs, EncryptedTxL2Logs, PublicDataWrite, @@ -53,7 +52,7 @@ export type ProcessedTx = Pick fr.toString()), - }; - } - - static override fromJSON(obj: ReturnType) { - return new AvmSimulationError( - obj.originalMessage, - obj.functionErrorStack, - obj.revertData.map(serializedFr => Fr.fromString(serializedFr)), - obj.noirErrorStack, - ); - } -} - /** * Outputs of processing the public component of a transaction. */ @@ -76,7 +44,7 @@ export class PublicSimulationOutput { constructor( public encryptedLogs: EncryptedTxL2Logs, public unencryptedLogs: UnencryptedTxL2Logs, - public revertReason: AvmSimulationError | undefined, + public revertReason: SimulationError | undefined, public constants: CombinedConstantData, public end: CombinedAccumulatedData, public publicReturnValues: NestedProcessReturnValues[], diff --git a/yarn-project/pxe/src/pxe_service/error_enriching.ts b/yarn-project/pxe/src/pxe_service/error_enriching.ts index bc284fe57d61..938d391ada77 100644 --- a/yarn-project/pxe/src/pxe_service/error_enriching.ts +++ b/yarn-project/pxe/src/pxe_service/error_enriching.ts @@ -1,4 +1,4 @@ -import { type AvmSimulationError, type SimulationError, isNoirCallStackUnresolved } from '@aztec/circuit-types'; +import { type SimulationError, isNoirCallStackUnresolved } from '@aztec/circuit-types'; import { AztecAddress, Fr, FunctionSelector, PUBLIC_DISPATCH_SELECTOR } from '@aztec/circuits.js'; import { type DebugLogger } from '@aztec/foundation/log'; import { resolveAssertionMessageFromRevertData, resolveOpcodeLocations } from '@aztec/simulator'; @@ -53,7 +53,7 @@ export async function enrichSimulationError(err: SimulationError, db: PxeDatabas } export async function enrichPublicSimulationError( - err: AvmSimulationError, + err: SimulationError, contractDataOracle: ContractDataOracle, db: PxeDatabase, logger: DebugLogger, diff --git a/yarn-project/pxe/src/pxe_service/pxe_service.ts b/yarn-project/pxe/src/pxe_service/pxe_service.ts index bbdfa52364f4..bbf50bf3e4df 100644 --- a/yarn-project/pxe/src/pxe_service/pxe_service.ts +++ b/yarn-project/pxe/src/pxe_service/pxe_service.ts @@ -1,6 +1,5 @@ import { type AuthWitness, - AvmSimulationError, type AztecNode, EventMetadata, EventType, @@ -791,7 +790,7 @@ export class PXEService implements PXE { } return result; } catch (err) { - if (err instanceof AvmSimulationError) { + if (err instanceof SimulationError) { await enrichPublicSimulationError(err, this.contractDataOracle, this.db, this.log); } throw err; diff --git a/yarn-project/simulator/src/common/errors.ts b/yarn-project/simulator/src/common/errors.ts index b90a7714d208..c02ec606a8de 100644 --- a/yarn-project/simulator/src/common/errors.ts +++ b/yarn-project/simulator/src/common/errors.ts @@ -1,4 +1,5 @@ import { type FailingFunction, type NoirCallStack, SimulationError } from '@aztec/circuit-types'; +import { type Fr } from '@aztec/circuits.js'; /** * An error that occurred during the execution of a function. @@ -47,7 +48,7 @@ export function traverseCauseChain(error: Error, callback: (error: Error) => voi * @param error - The error thrown during execution. * @returns - A simulation error. */ -export function createSimulationError(error: Error): SimulationError { +export function createSimulationError(error: Error, revertData?: Fr[]): SimulationError { let rootCause = error; let noirCallStack: NoirCallStack | undefined = undefined; const aztecCallStack: FailingFunction[] = []; @@ -62,5 +63,5 @@ export function createSimulationError(error: Error): SimulationError { } }); - return new SimulationError(rootCause.message, aztecCallStack, noirCallStack, { cause: rootCause }); + return new SimulationError(rootCause.message, aztecCallStack, revertData, noirCallStack, { cause: rootCause }); } diff --git a/yarn-project/simulator/src/mocks/fixtures.ts b/yarn-project/simulator/src/mocks/fixtures.ts index 839d963ca75c..ccba4e58f8d8 100644 --- a/yarn-project/simulator/src/mocks/fixtures.ts +++ b/yarn-project/simulator/src/mocks/fixtures.ts @@ -1,7 +1,7 @@ import { - type AvmSimulationError, type FunctionCall, PublicExecutionRequest, + type SimulationError, UnencryptedFunctionL2Logs, } from '@aztec/circuit-types'; import { @@ -27,7 +27,7 @@ export class PublicExecutionResultBuilder { private _contractStorageReads: ContractStorageRead[] = []; private _returnValues: Fr[] = []; private _reverted = false; - private _revertReason: AvmSimulationError | undefined = undefined; + private _revertReason: SimulationError | undefined = undefined; constructor(executionRequest: PublicExecutionRequest) { this._executionRequest = executionRequest; @@ -46,7 +46,7 @@ export class PublicExecutionResultBuilder { nestedExecutions?: PublicExecutionResult[]; contractStorageUpdateRequests?: ContractStorageUpdateRequest[]; contractStorageReads?: ContractStorageRead[]; - revertReason?: AvmSimulationError; + revertReason?: SimulationError; }): PublicExecutionResultBuilder { const builder = new PublicExecutionResultBuilder(request); @@ -76,7 +76,7 @@ export class PublicExecutionResultBuilder { nestedExecutions?: PublicExecutionResult[]; contractStorageUpdateRequests?: ContractStorageUpdateRequest[]; contractStorageReads?: ContractStorageRead[]; - revertReason?: AvmSimulationError; + revertReason?: SimulationError; }) { const builder = new PublicExecutionResultBuilder( new PublicExecutionRequest(new CallContext(from, tx.to, tx.selector, false), tx.args), @@ -113,7 +113,7 @@ export class PublicExecutionResultBuilder { return this; } - withReverted(reason: AvmSimulationError): PublicExecutionResultBuilder { + withReverted(reason: SimulationError): PublicExecutionResultBuilder { this._reverted = true; this._revertReason = reason; return this; diff --git a/yarn-project/simulator/src/public/enqueued_call_simulator.ts b/yarn-project/simulator/src/public/enqueued_call_simulator.ts index d79bcc5cf5fb..c5c501ef551e 100644 --- a/yarn-project/simulator/src/public/enqueued_call_simulator.ts +++ b/yarn-project/simulator/src/public/enqueued_call_simulator.ts @@ -1,11 +1,11 @@ import { type AvmProvingRequest, - type AvmSimulationError, MerkleTreeId, NestedProcessReturnValues, ProvingRequestType, type PublicExecutionRequest, PublicKernelPhase, + type SimulationError, type Tx, UnencryptedFunctionL2Logs, } from '@aztec/circuit-types'; @@ -84,7 +84,7 @@ export type EnqueuedCallResult = { /** Gas used during the execution this enqueued call */ gasUsed: Gas; /** Revert reason, if any */ - revertReason?: AvmSimulationError; + revertReason?: SimulationError; }; export class EnqueuedCallSimulator { diff --git a/yarn-project/simulator/src/public/enqueued_calls_processor.ts b/yarn-project/simulator/src/public/enqueued_calls_processor.ts index ad89150b1bb6..6d0d1616d358 100644 --- a/yarn-project/simulator/src/public/enqueued_calls_processor.ts +++ b/yarn-project/simulator/src/public/enqueued_calls_processor.ts @@ -1,11 +1,11 @@ import { type AvmProvingRequest, - type AvmSimulationError, type MerkleTreeReadOperations, type NestedProcessReturnValues, type ProcessedTx, type PublicExecutionRequest, PublicKernelPhase, + type SimulationError, type Tx, } from '@aztec/circuit-types'; import { @@ -54,13 +54,13 @@ type PublicPhaseResult = { /** Time spent for the execution this phase */ durationMs: number; /** Revert reason, if any */ - revertReason?: AvmSimulationError; + revertReason?: SimulationError; }; export type ProcessedPhase = { phase: PublicKernelPhase; durationMs: number; - revertReason?: AvmSimulationError; + revertReason?: SimulationError; }; export type TxPublicCallsResult = { @@ -72,7 +72,7 @@ export type TxPublicCallsResult = { /** Gas used during the execution this tx */ gasUsed: ProcessedTx['gasUsed']; /** Revert reason, if any */ - revertReason?: AvmSimulationError; + revertReason?: SimulationError; processedPhases: ProcessedPhase[]; }; @@ -160,7 +160,7 @@ export class EnqueuedCallsProcessor { let publicKernelOutput = tx.data.toPublicKernelCircuitPublicInputs(); let isFromPrivate = true; let returnValues: NestedProcessReturnValues[] = []; - let revertReason: AvmSimulationError | undefined; + let revertReason: SimulationError | undefined; for (let i = 0; i < phases.length; i++) { const phase = phases[i]; const callRequests = EnqueuedCallsProcessor.getCallRequestsByPhase(tx, phase); @@ -233,7 +233,7 @@ export class EnqueuedCallsProcessor { let avmProvingRequest: AvmProvingRequest; let publicKernelOutput = previousPublicKernelOutput; let gasUsed = Gas.empty(); - let revertReason: AvmSimulationError | undefined; + let revertReason: SimulationError | undefined; for (let i = callRequests.length - 1; i >= 0 && !revertReason; i--) { const callRequest = callRequests[i]; const executionRequest = executionRequests[i]; diff --git a/yarn-project/simulator/src/public/execution.ts b/yarn-project/simulator/src/public/execution.ts index 26b37cdf9a8a..94fcc6106afa 100644 --- a/yarn-project/simulator/src/public/execution.ts +++ b/yarn-project/simulator/src/public/execution.ts @@ -1,7 +1,7 @@ import { - type AvmSimulationError, NestedProcessReturnValues, type PublicExecutionRequest, + type SimulationError, type UnencryptedFunctionL2Logs, } from '@aztec/circuit-types'; import { @@ -49,7 +49,7 @@ export interface PublicExecutionResult { /** Whether the execution reverted. */ reverted: boolean; /** The revert reason if the execution reverted. */ - revertReason?: AvmSimulationError; + revertReason?: SimulationError; /** The contract storage reads performed by the function. */ contractStorageReads: ContractStorageRead[]; diff --git a/yarn-project/simulator/src/public/public_processor.test.ts b/yarn-project/simulator/src/public/public_processor.test.ts index e6dc1e1eeacd..acede1589750 100644 --- a/yarn-project/simulator/src/public/public_processor.test.ts +++ b/yarn-project/simulator/src/public/public_processor.test.ts @@ -1,5 +1,4 @@ import { - AvmSimulationError, type MerkleTreeWriteOperations, type ProcessedTx, type ProcessedTxHandler, @@ -330,7 +329,7 @@ describe('public_processor', () => { PublicExecutionResultBuilder.fromFunctionCall({ from: revertibleRequests[0].callContext.contractAddress, tx: makeFunctionCall('', nestedContractAddress, makeSelector(5)), - revertReason: new AvmSimulationError('Simulation Failed', [], []), + revertReason: new SimulationError('Simulation Failed', []), }).build(), ], }).build(), @@ -435,7 +434,7 @@ describe('public_processor', () => { PublicExecutionResultBuilder.fromFunctionCall({ from: nonRevertibleRequests[0].callContext.contractAddress, tx: makeFunctionCall('', nestedContractAddress, makeSelector(5)), - revertReason: new AvmSimulationError('Simulation Failed', [], []), + revertReason: new SimulationError('Simulation Failed', []), }).build(), ], }).build(), @@ -547,7 +546,7 @@ describe('public_processor', () => { from: teardownRequest.callContext.contractAddress, tx: makeFunctionCall('', nestedContractAddress, makeSelector(5)), contractStorageUpdateRequests: [new ContractStorageUpdateRequest(contractSlotC, fr(0x202), 17)], - revertReason: new AvmSimulationError('Simulation Failed', [], []), + revertReason: new SimulationError('Simulation Failed', []), }).build(teardownResultSettings), ], }).build(teardownResultSettings), @@ -645,7 +644,7 @@ describe('public_processor', () => { new ContractStorageUpdateRequest(contractSlotB, fr(0x152), 14), new ContractStorageUpdateRequest(contractSlotC, fr(0x201), 15), ], - revertReason: new AvmSimulationError('Simulation Failed', [], []), + revertReason: new SimulationError('Simulation Failed', []), }).build(), // Teardown @@ -661,7 +660,7 @@ describe('public_processor', () => { from: teardownRequest.callContext.contractAddress, tx: makeFunctionCall('', nestedContractAddress, makeSelector(5)), contractStorageUpdateRequests: [new ContractStorageUpdateRequest(contractSlotC, fr(0x202), 16)], - revertReason: new AvmSimulationError('Simulation Failed', [], []), + revertReason: new SimulationError('Simulation Failed', []), }).build(teardownResultSettings), ], }).build(teardownResultSettings), diff --git a/yarn-project/simulator/src/public/side_effect_trace.ts b/yarn-project/simulator/src/public/side_effect_trace.ts index 703edf80f2d9..c68bf45a38fe 100644 --- a/yarn-project/simulator/src/public/side_effect_trace.ts +++ b/yarn-project/simulator/src/public/side_effect_trace.ts @@ -1,8 +1,8 @@ import { - AvmSimulationError, type FailingFunction, type NoirCallStack, PublicExecutionRequest, + SimulationError, UnencryptedFunctionL2Logs, UnencryptedL2Log, } from '@aztec/circuit-types'; @@ -44,7 +44,7 @@ import { createDebugLogger } from '@aztec/foundation/log'; import { type AvmContractCallResult } from '../avm/avm_contract_call_result.js'; import { type AvmExecutionEnvironment } from '../avm/avm_execution_environment.js'; -import { ExecutionError, traverseCauseChain } from '../common/errors.js'; +import { ExecutionError, createSimulationError, traverseCauseChain } from '../common/errors.js'; import { type PublicExecutionResult, resultToPublicCallRequest } from './execution.js'; import { SideEffectLimitReachedError } from './side_effect_errors.js'; import { type PublicSideEffectTraceInterface } from './side_effect_trace_interface.js'; @@ -369,7 +369,7 @@ export class PublicSideEffectTrace implements PublicSideEffectTraceInterface { returnValues: avmCallResults.output, reverted: avmCallResults.reverted, revertReason: avmCallResults.revertReason - ? createAvmSimulationError(avmCallResults.revertReason, avmCallResults.output) + ? createSimulationError(avmCallResults.revertReason, avmCallResults.output) : undefined, contractStorageReads: this.contractStorageReads, @@ -429,26 +429,3 @@ function createPublicExecutionRequest(avmEnvironment: AvmExecutionEnvironment): }); return new PublicExecutionRequest(callContext, avmEnvironment.calldata); } - -/** - * Creates an AVM simulation error from an error chain generated during the execution of public functions. - * @param error - The error thrown during execution. - * @returns - An AVM simulation error. - */ -function createAvmSimulationError(error: Error, revertData: Fr[]): AvmSimulationError { - let rootCause = error; - let noirCallStack: NoirCallStack | undefined = undefined; - const aztecCallStack: FailingFunction[] = []; - - traverseCauseChain(error, cause => { - rootCause = cause; - if (cause instanceof ExecutionError) { - aztecCallStack.push(cause.failingFunction); - if (cause.noirCallStack) { - noirCallStack = cause.noirCallStack; - } - } - }); - - return new AvmSimulationError(rootCause.message, aztecCallStack, revertData, noirCallStack, { cause: rootCause }); -} From 4eda7781010a4b9b5338182ac5a9d4fc9256da0a Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 5 Nov 2024 11:39:02 +0000 Subject: [PATCH 6/6] lint --- .../simulator/src/public/side_effect_trace.ts | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/yarn-project/simulator/src/public/side_effect_trace.ts b/yarn-project/simulator/src/public/side_effect_trace.ts index c68bf45a38fe..bc92d05c8ec3 100644 --- a/yarn-project/simulator/src/public/side_effect_trace.ts +++ b/yarn-project/simulator/src/public/side_effect_trace.ts @@ -1,11 +1,4 @@ -import { - type FailingFunction, - type NoirCallStack, - PublicExecutionRequest, - SimulationError, - UnencryptedFunctionL2Logs, - UnencryptedL2Log, -} from '@aztec/circuit-types'; +import { PublicExecutionRequest, UnencryptedFunctionL2Logs, UnencryptedL2Log } from '@aztec/circuit-types'; import { AvmContractBytecodeHints, AvmContractInstanceHint, @@ -44,7 +37,7 @@ import { createDebugLogger } from '@aztec/foundation/log'; import { type AvmContractCallResult } from '../avm/avm_contract_call_result.js'; import { type AvmExecutionEnvironment } from '../avm/avm_execution_environment.js'; -import { ExecutionError, createSimulationError, traverseCauseChain } from '../common/errors.js'; +import { createSimulationError } from '../common/errors.js'; import { type PublicExecutionResult, resultToPublicCallRequest } from './execution.js'; import { SideEffectLimitReachedError } from './side_effect_errors.js'; import { type PublicSideEffectTraceInterface } from './side_effect_trace_interface.js';