From a509429bab52ee0743eed377441400b327d8778a Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 28 Aug 2023 11:46:54 +0000 Subject: [PATCH 01/13] feat: first working version of simulate pub --- yarn-project/acir-simulator/src/acvm/acvm.ts | 33 ++++++++++++----- .../acir-simulator/src/public/executor.ts | 5 ++- .../aztec-node/src/aztec-node/http-node.ts | 12 +++++++ .../aztec-node/src/aztec-node/server.ts | 36 ++++++++++++++++++- .../src/aztec_rpc_server/aztec_rpc_server.ts | 36 +++++++++++++++++-- yarn-project/rollup-provider/src/app.ts | 7 ++++ .../src/global_variable_builder/index.ts | 1 + yarn-project/sequencer-client/src/index.ts | 4 +++ .../src/sequencer/processed_tx.ts | 14 ++++++++ .../src/sequencer/public_processor.ts | 14 ++++---- .../src/sequencer/sequencer.ts | 5 +-- .../sequencer-client/src/simulator/index.ts | 2 ++ .../types/src/interfaces/aztec-node.ts | 7 ++++ 13 files changed, 156 insertions(+), 20 deletions(-) diff --git a/yarn-project/acir-simulator/src/acvm/acvm.ts b/yarn-project/acir-simulator/src/acvm/acvm.ts index 5b07aca3abbf..f64e3d970c18 100644 --- a/yarn-project/acir-simulator/src/acvm/acvm.ts +++ b/yarn-project/acir-simulator/src/acvm/acvm.ts @@ -129,7 +129,7 @@ function getCallStackFromOpcodeLocation(opcodeLocation: string, debug: FunctionD * @param callStack - The error stack * @returns - The formatted string */ -function printErrorStack(callStack: SourceCodeLocation[]): string { +export function printErrorStack(callStack: SourceCodeLocation[]): string { // TODO experiment with formats of reporting this for better error reporting return [ 'Error: Assertion failed', @@ -137,6 +137,21 @@ function printErrorStack(callStack: SourceCodeLocation[]): string { ].join('\n'); } +/** + * Extracts source code locations from an ACVM error if possible. + * @param errMessage - The ACVM error. + * @param debug - The debug metadata of the function. + * @returns The source code locations or undefined if they couldn't be extracted from the error. + */ +export function processAcvmError(errMessage: string, debug: FunctionDebugMetadata): SourceCodeLocation[] | undefined { + const opcodeLocation = extractOpcodeLocationFromError(errMessage); + if (!opcodeLocation) { + return undefined; + } + + return getCallStackFromOpcodeLocation(opcodeLocation, debug); +} + /** * The function call that executes an ACIR. */ @@ -182,14 +197,16 @@ export async function acvm( if (oracleError) { throw oracleError; } - const opcodeLocation = extractOpcodeLocationFromError(acvmError); - if (!opcodeLocation || !debug) { - throw new Error(acvmError); - } - const callStack = getCallStackFromOpcodeLocation(opcodeLocation, debug); - logger(printErrorStack(callStack)); - throw new Error(`Assertion failed: '${callStack.pop()?.assertionText ?? 'Unknown'}'`); + if (debug) { + const callStack = processAcvmError(acvmError, debug); + if (callStack) { + logger(printErrorStack(callStack)); + throw new Error(`Assertion failed: '${callStack.pop()?.assertionText ?? 'Unknown'}'`); + } + } + // If we cannot find a callstack, throw the original error. + throw new Error(acvmError); }); return Promise.resolve({ partialWitness }); diff --git a/yarn-project/acir-simulator/src/public/executor.ts b/yarn-project/acir-simulator/src/public/executor.ts index 2b0081508a9a..27ffcdb83648 100644 --- a/yarn-project/acir-simulator/src/public/executor.ts +++ b/yarn-project/acir-simulator/src/public/executor.ts @@ -67,7 +67,6 @@ export class PublicExecutor { // Functions can request to pack arguments before calling other functions. // We use this cache to hold the packed arguments. const packedArgs = await PackedArgsCache.create([]); - const { partialWitness } = await acvm(await AcirSimulator.getSolver(), acir, initialWitness, { packArguments: async args => { return toACVMField(await packedArgs.pack(args.map(fromACVMField))); @@ -140,6 +139,10 @@ export class PublicExecutor { (await this.contractsDb.getPortalContractAddress(contractAddress)) ?? EthAddress.ZERO; return Promise.resolve(toACVMField(portalContactAddress)); }, + }).catch((err: Error) => { + throw new Error( + `Error executing public function ${execution.contractAddress.toString()}:${selector}: ${err.message}`, + ); }); const { diff --git a/yarn-project/aztec-node/src/aztec-node/http-node.ts b/yarn-project/aztec-node/src/aztec-node/http-node.ts index 363f15301a6c..bd57ee99f0bb 100644 --- a/yarn-project/aztec-node/src/aztec-node/http-node.ts +++ b/yarn-project/aztec-node/src/aztec-node/http-node.ts @@ -370,4 +370,16 @@ export class HttpNode implements AztecNode { const response = await (await fetch(url.toString())).json(); return response.blockData; } + + /** + * Simulates the public part of a transaction with the current state. + * @param tx - The transaction to simulate. + **/ + public async simulatePublicPart(tx: Tx) { + const url = new URL(`${this.baseUrl}/simulate-tx`); + const init: RequestInit = {}; + init['method'] = 'POST'; + init['body'] = tx.toBuffer(); + await fetch(url.toString(), init); + } } diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index 09b03ab0d2f4..4c39db479a60 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -4,6 +4,7 @@ import { CircuitsWasm, EthAddress, Fr, + GlobalVariables, HistoricBlockData, L1_TO_L2_MSG_TREE_HEIGHT, PRIVATE_DATA_TREE_HEIGHT, @@ -11,7 +12,12 @@ import { import { AztecAddress } from '@aztec/foundation/aztec-address'; import { createDebugLogger } from '@aztec/foundation/log'; import { InMemoryTxPool, P2P, createP2PClient } from '@aztec/p2p'; -import { SequencerClient } from '@aztec/sequencer-client'; +import { + GlobalVariableBuilder, + PublicProcessorFactory, + SequencerClient, + getGlobalVariableBuilder, +} from '@aztec/sequencer-client'; import { AztecNode, ContractData, @@ -61,6 +67,7 @@ export class AztecNodeService implements AztecNode { protected sequencer: SequencerClient, protected chainId: number, protected version: number, + protected globalVariableBuilder: GlobalVariableBuilder, private log = createDebugLogger('aztec:node'), ) {} @@ -108,6 +115,7 @@ export class AztecNodeService implements AztecNode { sequencer, config.chainId, config.version, + getGlobalVariableBuilder(config), ); } @@ -368,6 +376,32 @@ export class AztecNodeService implements AztecNode { ); } + /** + * Simulates the public part of a transaction with the current state. + * @param tx - The transaction to simulate. + **/ + public async simulatePublicPart(tx: Tx) { + this.log.info(`Simulating tx ${await tx.getTxHash()}`); + + const publicProcessorFactory = new PublicProcessorFactory( + this.worldStateSynchroniser.getLatest(), + this.contractDataSource, + this.l1ToL2MessageSource, + ); + const blockNumber = (await this.blockSource.getBlockNumber()) + 1; + const newGlobalVariables = await this.globalVariableBuilder.buildGlobalVariables(new Fr(blockNumber)); + const prevGlobalVariables = (await this.blockSource.getL2Block(-1))?.globalVariables ?? GlobalVariables.empty(); + + // Process txs and drop the ones that fail processing + // We create a fresh processor each time to reset any cached state (eg storage writes) + const processor = await publicProcessorFactory.create(prevGlobalVariables, newGlobalVariables); + const [, failedTxs] = await processor.process([tx]); + if (failedTxs.length > 0) { + throw failedTxs[0].error; + } + this.log.info(`Simulated tx ${await tx.getTxHash()} succeeds`); + } + /** * Returns an instance of MerkleTreeOperations having first ensured the world state is fully synched * @returns An instance of a committed MerkleTreeOperations diff --git a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts index acb4f54ad19f..604bc655d9e5 100644 --- a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts +++ b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts @@ -3,6 +3,8 @@ import { collectEncryptedLogs, collectEnqueuedPublicFunctionCalls, collectUnencryptedLogs, + printErrorStack, + processAcvmError, } from '@aztec/acir-simulator'; import { AztecAddress, @@ -14,7 +16,7 @@ import { PrivateKey, PublicCallRequest, } from '@aztec/circuits.js'; -import { encodeArguments } from '@aztec/foundation/abi'; +import { FunctionSelector, encodeArguments } from '@aztec/foundation/abi'; import { padArrayEnd } from '@aztec/foundation/collection'; import { Fr } from '@aztec/foundation/fields'; import { DebugLogger, createDebugLogger } from '@aztec/foundation/log'; @@ -395,7 +397,7 @@ export class AztecRPCServer implements AztecRPC { // TODO(#757): Enforce proper ordering of enqueued public calls await this.patchPublicCallStackOrdering(publicInputs, enqueuedPublicFunctions); - return new Tx( + const tx = new Tx( publicInputs, proof, encryptedLogs, @@ -403,6 +405,36 @@ export class AztecRPCServer implements AztecRPC { newContractPublicFunctions, enqueuedPublicFunctions, ); + + // Simulate public part of the transaction to catch public execution errors before submitting + // This can be used for estimating gas in the future. + try { + await this.node.simulatePublicPart(tx); + } catch (err) { + if (err instanceof Error) { + const acvmErrorMatch = err.message.match( + /Error executing public function (?0x[0-9a-f]+):(?[0-9a-f]+): (?.*)/, + ); + if (acvmErrorMatch) { + const { contractAddress, selector, acvmError } = acvmErrorMatch.groups!; + const contractDataOracle = new ContractDataOracle(this.db, this.node); + const debugInfo = await contractDataOracle.getFunctionDebugMetadata( + AztecAddress.fromString(contractAddress), + FunctionSelector.fromBuffer(Buffer.from(selector, 'hex')), + ); + if (debugInfo) { + const callStack = processAcvmError(acvmError, debugInfo); + if (callStack) { + this.log(printErrorStack(callStack)); + throw new Error(`Assertion failed in public execution: ${callStack.pop()?.assertionText ?? 'Unknown'}`); + } + } + } + } + throw err; + } + + return tx; } // HACK(#1639): this is a hack to fix ordering of public calls enqueued in the call stack. Since the private kernel diff --git a/yarn-project/rollup-provider/src/app.ts b/yarn-project/rollup-provider/src/app.ts index bdd223e19947..76df95ae1611 100644 --- a/yarn-project/rollup-provider/src/app.ts +++ b/yarn-project/rollup-provider/src/app.ts @@ -269,6 +269,13 @@ export function appFactory(node: AztecNode, prefix: string) { ctx.status = 200; }); + router.post('/tx-simulate', checkReady, async (ctx: Koa.Context) => { + const stream = new PromiseReadable(ctx.req); + const postData = (await stream.readAll()) as Buffer; + const tx = Tx.fromBuffer(postData); + await node.simulatePublicPart(tx); + }); + const app = new Koa(); app.on('error', error => { logger.error(`KOA app-level error. ${JSON.stringify({ error })}`); diff --git a/yarn-project/sequencer-client/src/global_variable_builder/index.ts b/yarn-project/sequencer-client/src/global_variable_builder/index.ts index 8de0248afcc2..91ab6caecb8f 100644 --- a/yarn-project/sequencer-client/src/global_variable_builder/index.ts +++ b/yarn-project/sequencer-client/src/global_variable_builder/index.ts @@ -4,6 +4,7 @@ import { ViemReader } from './viem-reader.js'; export { SimpleTestGlobalVariableBuilder as SimpleGlobalVariableBuilder } from './global_builder.js'; export { GlobalReaderConfig } from './config.js'; +export { GlobalVariableBuilder } from './global_builder.js'; /** * Returns a new instance of the global variable builder. diff --git a/yarn-project/sequencer-client/src/index.ts b/yarn-project/sequencer-client/src/index.ts index fb6a33a566d7..f95eed987417 100644 --- a/yarn-project/sequencer-client/src/index.ts +++ b/yarn-project/sequencer-client/src/index.ts @@ -4,6 +4,10 @@ export * from './publisher/index.js'; export * from './client/index.js'; export * from './mocks/verification_keys.js'; +// Used by the node to simulate public parts of transactions. Should these be moved to a shared library? +export * from './sequencer/public_processor.js'; +export * from './global_variable_builder/index.js'; + // Used by publisher test in e2e export { WasmRollupCircuitSimulator } from './simulator/rollup.js'; export { EmptyRollupProver } from './prover/empty.js'; diff --git a/yarn-project/sequencer-client/src/sequencer/processed_tx.ts b/yarn-project/sequencer-client/src/sequencer/processed_tx.ts index fbf80ff5f039..7c4ce3e64daa 100644 --- a/yarn-project/sequencer-client/src/sequencer/processed_tx.ts +++ b/yarn-project/sequencer-client/src/sequencer/processed_tx.ts @@ -16,6 +16,20 @@ export type ProcessedTx = Pick { + public async process(txs: Tx[]): Promise<[ProcessedTx[], FailedTx[]]> { const result: ProcessedTx[] = []; - const failed: Tx[] = []; + const failed: FailedTx[] = []; for (const tx of txs) { this.log(`Processing tx ${await tx.getTxHash()}`); @@ -117,7 +116,10 @@ export class PublicProcessor { result.push(await this.processTx(tx)); } catch (err) { this.log.error(`Error processing tx ${await tx.getTxHash()}: ${err}`); - failed.push(tx); + failed.push({ + tx, + error: err instanceof Error ? err : new Error('Unknown error'), + }); } } return [result, failed]; diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.ts index b3a6e3b75401..35748885af42 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.ts @@ -149,8 +149,9 @@ export class Sequencer { const processor = await this.publicProcessorFactory.create(prevGlobalVariables, newGlobalVariables); const [processedTxs, failedTxs] = await processor.process(validTxs); if (failedTxs.length > 0) { - this.log(`Dropping failed txs ${(await Tx.getHashes(failedTxs)).join(', ')}`); - await this.p2pClient.deleteTxs(await Tx.getHashes(failedTxs)); + const failedTxData = failedTxs.map(fail => fail.tx); + this.log(`Dropping failed txs ${(await Tx.getHashes(failedTxData)).join(', ')}`); + await this.p2pClient.deleteTxs(await Tx.getHashes(failedTxData)); } if (processedTxs.length === 0) { diff --git a/yarn-project/sequencer-client/src/simulator/index.ts b/yarn-project/sequencer-client/src/simulator/index.ts index a9ede5663766..0cc49389219f 100644 --- a/yarn-project/sequencer-client/src/simulator/index.ts +++ b/yarn-project/sequencer-client/src/simulator/index.ts @@ -8,6 +8,8 @@ import { RootRollupPublicInputs, } from '@aztec/circuits.js'; +export { getPublicExecutor } from './public_executor.js'; + /** * Circuit simulator for the rollup circuits. */ diff --git a/yarn-project/types/src/interfaces/aztec-node.ts b/yarn-project/types/src/interfaces/aztec-node.ts index 4ae7e4f65c3c..ac192ede1732 100644 --- a/yarn-project/types/src/interfaces/aztec-node.ts +++ b/yarn-project/types/src/interfaces/aztec-node.ts @@ -138,4 +138,11 @@ export interface AztecNode extends DataCommitmentProvider, L1ToL2MessageProvider * @returns The current committed block data. */ getHistoricBlockData(): Promise; + + /** + * Simulates the public part of a transaction with the current state. + * This currently just checks that the transaction execution succeeds. + * @param tx - The transaction to simulate. + **/ + simulatePublicPart(tx: Tx): Promise; } From 408c83cc3f01bc1bce27815fbdefe5bbaec0e596 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 28 Aug 2023 14:38:59 +0000 Subject: [PATCH 02/13] feat: allow parsing nested public exec errors --- yarn-project/acir-simulator/src/acvm/acvm.ts | 10 +-- .../src/client/private_execution.ts | 14 +++-- .../acir-simulator/src/public/executor.ts | 8 ++- .../aztec-node/src/aztec-node/http-node.ts | 6 +- .../src/aztec_rpc_server/aztec_rpc_server.ts | 61 ++++++++++--------- yarn-project/rollup-provider/src/app.ts | 14 ++++- yarn-project/types/src/index.ts | 1 + yarn-project/types/src/simulation_error.ts | 47 ++++++++++++++ 8 files changed, 118 insertions(+), 43 deletions(-) create mode 100644 yarn-project/types/src/simulation_error.ts diff --git a/yarn-project/acir-simulator/src/acvm/acvm.ts b/yarn-project/acir-simulator/src/acvm/acvm.ts index f64e3d970c18..57273430ec22 100644 --- a/yarn-project/acir-simulator/src/acvm/acvm.ts +++ b/yarn-project/acir-simulator/src/acvm/acvm.ts @@ -96,7 +96,7 @@ interface SourceCodeLocation { /** * The source code text of the failed constraint. */ - assertionText: string; + locationText: string; } /** @@ -111,7 +111,7 @@ function getCallStackFromOpcodeLocation(opcodeLocation: string, debug: FunctionD const { path, source } = files[fileId]; - const assertionText = source.substring(span.start, span.end + 1); + const locationText = source.substring(span.start, span.end + 1); const precedingText = source.substring(0, span.start); const line = precedingText.split('\n').length; @@ -119,7 +119,7 @@ function getCallStackFromOpcodeLocation(opcodeLocation: string, debug: FunctionD filePath: path, line, fileSource: source, - assertionText, + locationText, }; }); } @@ -133,7 +133,7 @@ export function printErrorStack(callStack: SourceCodeLocation[]): string { // TODO experiment with formats of reporting this for better error reporting return [ 'Error: Assertion failed', - callStack.map(call => ` at ${call.filePath}:${call.line} '${call.assertionText}'`), + callStack.map(call => ` at ${call.filePath}:${call.line} '${call.locationText}'`), ].join('\n'); } @@ -202,7 +202,7 @@ export async function acvm( const callStack = processAcvmError(acvmError, debug); if (callStack) { logger(printErrorStack(callStack)); - throw new Error(`Assertion failed: '${callStack.pop()?.assertionText ?? 'Unknown'}'`); + throw new Error(`Assertion failed: '${callStack.pop()?.locationText ?? 'Unknown'}'`); } } // If we cannot find a callstack, throw the original error. diff --git a/yarn-project/acir-simulator/src/client/private_execution.ts b/yarn-project/acir-simulator/src/client/private_execution.ts index 93425ffb9553..07356de27d24 100644 --- a/yarn-project/acir-simulator/src/client/private_execution.ts +++ b/yarn-project/acir-simulator/src/client/private_execution.ts @@ -11,7 +11,7 @@ import { AztecAddress } from '@aztec/foundation/aztec-address'; import { Fr, Point } from '@aztec/foundation/fields'; import { createDebugLogger } from '@aztec/foundation/log'; import { to2Fields } from '@aztec/foundation/serialize'; -import { FunctionL2Logs, NotePreimage, NoteSpendingInfo } from '@aztec/types'; +import { FunctionL2Logs, NotePreimage, NoteSpendingInfo, SimulationError } from '@aztec/types'; import { extractPrivateCircuitPublicInputs, frToAztecAddress } from '../acvm/deserialize.js'; import { @@ -55,8 +55,8 @@ export class PrivateFunctionExecution { * @returns The execution result. */ public async run(): Promise { - const selector = this.functionData.selector.toString(); - this.log(`Executing external function ${this.contractAddress.toString()}:${selector}`); + const selector = this.functionData.selector; + this.log(`Executing external function ${this.contractAddress}:${selector}`); const acir = Buffer.from(this.abi.bytecode, 'base64'); const initialWitness = this.getInitialWitness(); @@ -197,7 +197,13 @@ export class PrivateFunctionExecution { }, }, this.abi.debug, - ); + ).catch((err: Error) => { + throw new SimulationError( + err.message, + { contractAddress: this.contractAddress, functionSelector: selector }, + err instanceof SimulationError ? err.getCallStack() : undefined, + ); + }); const publicInputs = extractPrivateCircuitPublicInputs(partialWitness, acir); diff --git a/yarn-project/acir-simulator/src/public/executor.ts b/yarn-project/acir-simulator/src/public/executor.ts index 27ffcdb83648..e2c9a346cc80 100644 --- a/yarn-project/acir-simulator/src/public/executor.ts +++ b/yarn-project/acir-simulator/src/public/executor.ts @@ -10,7 +10,7 @@ import { } from '@aztec/circuits.js'; import { padArrayEnd } from '@aztec/foundation/collection'; import { createDebugLogger } from '@aztec/foundation/log'; -import { FunctionL2Logs } from '@aztec/types'; +import { FunctionL2Logs, SimulationError } from '@aztec/types'; import { ZERO_ACVM_FIELD, @@ -140,8 +140,10 @@ export class PublicExecutor { return Promise.resolve(toACVMField(portalContactAddress)); }, }).catch((err: Error) => { - throw new Error( - `Error executing public function ${execution.contractAddress.toString()}:${selector}: ${err.message}`, + throw new SimulationError( + err.message, + { contractAddress: execution.contractAddress, functionSelector: selector }, + err instanceof SimulationError ? err.getCallStack() : undefined, ); }); diff --git a/yarn-project/aztec-node/src/aztec-node/http-node.ts b/yarn-project/aztec-node/src/aztec-node/http-node.ts index bd57ee99f0bb..8497a246facb 100644 --- a/yarn-project/aztec-node/src/aztec-node/http-node.ts +++ b/yarn-project/aztec-node/src/aztec-node/http-node.ts @@ -20,6 +20,7 @@ import { LogType, MerkleTreeId, SiblingPath, + SimulationError, Tx, TxHash, } from '@aztec/types'; @@ -380,6 +381,9 @@ export class HttpNode implements AztecNode { const init: RequestInit = {}; init['method'] = 'POST'; init['body'] = tx.toBuffer(); - await fetch(url.toString(), init); + const response = await (await fetch(url.toString(), init)).json(); + if (response.simulationError) { + throw SimulationError.fromJSON(response.simulationError); + } } } diff --git a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts index 604bc655d9e5..b91ff7ee5272 100644 --- a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts +++ b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts @@ -16,7 +16,7 @@ import { PrivateKey, PublicCallRequest, } from '@aztec/circuits.js'; -import { FunctionSelector, encodeArguments } from '@aztec/foundation/abi'; +import { encodeArguments } from '@aztec/foundation/abi'; import { padArrayEnd } from '@aztec/foundation/collection'; import { Fr } from '@aztec/foundation/fields'; import { DebugLogger, createDebugLogger } from '@aztec/foundation/log'; @@ -34,6 +34,7 @@ import { L2BlockL2Logs, LogType, NodeInfo, + SimulationError, Tx, TxExecutionRequest, TxHash, @@ -362,6 +363,36 @@ export class AztecRPCServer implements AztecRPC { return result; } + /** + * Simulate the public part of a transaction. + * This allows to catch public execution errors before submitting the transaction. + * It can also be used for estimating gas in the future. + * @param tx - The transaction to be simulated. + */ + async #simulatePublicPart(tx: Tx) { + try { + await this.node.simulatePublicPart(tx); + } catch (err) { + if (err instanceof SimulationError) { + const originalFailingFunction = err.getCallStack().pop()!; + const contractDataOracle = new ContractDataOracle(this.db, this.node); + const debugInfo = await contractDataOracle.getFunctionDebugMetadata( + originalFailingFunction.contractAddress, + originalFailingFunction.functionSelector, + ); + if (debugInfo) { + const callStack = processAcvmError(err.message, debugInfo); + if (callStack) { + this.log(printErrorStack(callStack)); + err.message = `Assertion failed in public execution: ${callStack.pop()?.locationText ?? 'Unknown'}`; + } + } + } + + throw err; + } + } + /** * Simulate a transaction, generate a kernel proof, and create a private transaction object. * The function takes in a transaction request and an ECDSA signature. It simulates the transaction, @@ -406,33 +437,7 @@ export class AztecRPCServer implements AztecRPC { enqueuedPublicFunctions, ); - // Simulate public part of the transaction to catch public execution errors before submitting - // This can be used for estimating gas in the future. - try { - await this.node.simulatePublicPart(tx); - } catch (err) { - if (err instanceof Error) { - const acvmErrorMatch = err.message.match( - /Error executing public function (?0x[0-9a-f]+):(?[0-9a-f]+): (?.*)/, - ); - if (acvmErrorMatch) { - const { contractAddress, selector, acvmError } = acvmErrorMatch.groups!; - const contractDataOracle = new ContractDataOracle(this.db, this.node); - const debugInfo = await contractDataOracle.getFunctionDebugMetadata( - AztecAddress.fromString(contractAddress), - FunctionSelector.fromBuffer(Buffer.from(selector, 'hex')), - ); - if (debugInfo) { - const callStack = processAcvmError(acvmError, debugInfo); - if (callStack) { - this.log(printErrorStack(callStack)); - throw new Error(`Assertion failed in public execution: ${callStack.pop()?.assertionText ?? 'Unknown'}`); - } - } - } - } - throw err; - } + await this.#simulatePublicPart(tx); return tx; } diff --git a/yarn-project/rollup-provider/src/app.ts b/yarn-project/rollup-provider/src/app.ts index 76df95ae1611..1f7f20399190 100644 --- a/yarn-project/rollup-provider/src/app.ts +++ b/yarn-project/rollup-provider/src/app.ts @@ -1,7 +1,7 @@ import { Fr, HistoricBlockData } from '@aztec/circuits.js'; import { AztecAddress } from '@aztec/foundation/aztec-address'; import { createDebugLogger } from '@aztec/foundation/log'; -import { AztecNode, MerkleTreeId, Tx, TxHash } from '@aztec/types'; +import { AztecNode, MerkleTreeId, SimulationError, Tx, TxHash } from '@aztec/types'; import Koa, { Context, DefaultState } from 'koa'; import Router from 'koa-router'; @@ -273,7 +273,17 @@ export function appFactory(node: AztecNode, prefix: string) { const stream = new PromiseReadable(ctx.req); const postData = (await stream.readAll()) as Buffer; const tx = Tx.fromBuffer(postData); - await node.simulatePublicPart(tx); + try { + await node.simulatePublicPart(tx); + } catch (err) { + if (err instanceof SimulationError) { + ctx.body = { + simulationError: err.toJSON(), + }; + } else { + throw err; + } + } }); const app = new Koa(); diff --git a/yarn-project/types/src/index.ts b/yarn-project/types/src/index.ts index 2dc0f8bddd90..9d0c7f314ea5 100644 --- a/yarn-project/types/src/index.ts +++ b/yarn-project/types/src/index.ts @@ -15,6 +15,7 @@ export * from './logs/index.js'; export * from './merkle_tree_id.js'; export * from './mocks.js'; export * from './public_data_write.js'; +export * from './simulation_error.js'; export * from './tx/index.js'; export * from './tx_execution_request.js'; export * from './packed_arguments.js'; diff --git a/yarn-project/types/src/simulation_error.ts b/yarn-project/types/src/simulation_error.ts new file mode 100644 index 000000000000..24d50bbbcd3e --- /dev/null +++ b/yarn-project/types/src/simulation_error.ts @@ -0,0 +1,47 @@ +import { AztecAddress, FunctionSelector } from '@aztec/circuits.js'; + +/** + * Address and selector of a function that failed during simulation. + */ +export interface FailingFunction { + /** + * The address of the contract that failed. + */ + contractAddress: AztecAddress; + /** + * The selector of the function that failed. + */ + functionSelector: FunctionSelector; +} + +/** + * An error during the simulation of a function call. + */ +export class SimulationError extends Error { + constructor( + message: string, + private failingFunction: FailingFunction, + private previousErrorStack?: FailingFunction[], + ) { + super(message); + } + + /** + * The call stack of the failing simulation. + */ + getCallStack(): FailingFunction[] { + return [this.failingFunction].concat(this.previousErrorStack || []); + } + + toJSON() { + return { + message: this.message, + failingFunction: this.failingFunction, + previousErrorStack: this.previousErrorStack, + }; + } + + static fromJSON(obj: any) { + return new SimulationError(obj.message, obj.failingFunction, obj.previousErrorStack); + } +} From 1ff4bd87a4e6e3d5bb1fd9d003f6f8545b8dabab Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 29 Aug 2023 08:44:52 +0000 Subject: [PATCH 03/13] feat: sim errors with fn and noir call stacks --- yarn-project/acir-simulator/src/acvm/acvm.ts | 64 ++++++--------- .../src/client/private_execution.ts | 11 +-- .../src/client/unconstrained_execution.ts | 24 ++++-- .../acir-simulator/src/public/executor.ts | 11 +-- .../src/aztec-node/http-node.test.ts | 35 +++++++- .../aztec-node/src/aztec-node/http-node.ts | 2 +- .../src/aztec_rpc_server/aztec_rpc_server.ts | 79 ++++++++++++++----- .../src/e2e_nested_contract.test.ts | 16 ++++ .../src/contracts/child_contract/src/main.nr | 29 +++++++ yarn-project/types/src/simulation_error.ts | 70 ++++++++++++++-- 10 files changed, 257 insertions(+), 84 deletions(-) diff --git a/yarn-project/acir-simulator/src/acvm/acvm.ts b/yarn-project/acir-simulator/src/acvm/acvm.ts index 57273430ec22..3aaac4130917 100644 --- a/yarn-project/acir-simulator/src/acvm/acvm.ts +++ b/yarn-project/acir-simulator/src/acvm/acvm.ts @@ -3,6 +3,7 @@ import { AztecAddress } from '@aztec/foundation/aztec-address'; import { EthAddress } from '@aztec/foundation/eth-address'; import { Fr } from '@aztec/foundation/fields'; import { createDebugLogger } from '@aztec/foundation/log'; +import { NoirCallStack } from '@aztec/types'; import { ForeignCallInput, @@ -77,32 +78,10 @@ function extractOpcodeLocationFromError(err: string): string | undefined { return match?.groups?.opcodeLocation; } -/** - * The data for a call in the call stack. - */ -interface SourceCodeLocation { - /** - * The path to the source file. - */ - filePath: string; - /** - * The line number of the call. - */ - line: number; - /** - * The source code of the file. - */ - fileSource: string; - /** - * The source code text of the failed constraint. - */ - locationText: string; -} - /** * Extracts the call stack from the location of a failing opcode and the debug metadata. */ -function getCallStackFromOpcodeLocation(opcodeLocation: string, debug: FunctionDebugMetadata): SourceCodeLocation[] { +function getCallStackFromOpcodeLocation(opcodeLocation: string, debug: FunctionDebugMetadata): NoirCallStack { const { debugSymbols, files } = debug; const callStack = debugSymbols.locations[opcodeLocation] || []; @@ -124,26 +103,13 @@ function getCallStackFromOpcodeLocation(opcodeLocation: string, debug: FunctionD }); } -/** - * Creates a formatted string for an error stack - * @param callStack - The error stack - * @returns - The formatted string - */ -export function printErrorStack(callStack: SourceCodeLocation[]): string { - // TODO experiment with formats of reporting this for better error reporting - return [ - 'Error: Assertion failed', - callStack.map(call => ` at ${call.filePath}:${call.line} '${call.locationText}'`), - ].join('\n'); -} - /** * Extracts source code locations from an ACVM error if possible. * @param errMessage - The ACVM error. * @param debug - The debug metadata of the function. * @returns The source code locations or undefined if they couldn't be extracted from the error. */ -export function processAcvmError(errMessage: string, debug: FunctionDebugMetadata): SourceCodeLocation[] | undefined { +export function processAcvmError(errMessage: string, debug: FunctionDebugMetadata): NoirCallStack | undefined { const opcodeLocation = extractOpcodeLocationFromError(errMessage); if (!opcodeLocation) { return undefined; @@ -152,6 +118,21 @@ export function processAcvmError(errMessage: string, debug: FunctionDebugMetadat return getCallStackFromOpcodeLocation(opcodeLocation, debug); } +/** + * An error thrown by the ACVM during simulation. Optionally contains a noir call stack. + */ +export class ACVMError extends Error { + constructor( + message: string, + /** + * The noir call stack of the error, if it could be extracted. + */ + public callStack?: NoirCallStack, + ) { + super(message); + } +} + /** * The function call that executes an ACIR. */ @@ -200,13 +181,16 @@ export async function acvm( if (debug) { const callStack = processAcvmError(acvmError, debug); + if (callStack) { - logger(printErrorStack(callStack)); - throw new Error(`Assertion failed: '${callStack.pop()?.locationText ?? 'Unknown'}'`); + throw new ACVMError( + `Assertion failed: '${callStack[callStack.length - 1]?.locationText ?? 'Unknown'}'`, + callStack, + ); } } // If we cannot find a callstack, throw the original error. - throw new Error(acvmError); + throw new ACVMError(acvmError); }); return Promise.resolve({ partialWitness }); diff --git a/yarn-project/acir-simulator/src/client/private_execution.ts b/yarn-project/acir-simulator/src/client/private_execution.ts index 07356de27d24..e8f2785fb073 100644 --- a/yarn-project/acir-simulator/src/client/private_execution.ts +++ b/yarn-project/acir-simulator/src/client/private_execution.ts @@ -15,6 +15,7 @@ import { FunctionL2Logs, NotePreimage, NoteSpendingInfo, SimulationError } from import { extractPrivateCircuitPublicInputs, frToAztecAddress } from '../acvm/deserialize.js'; import { + ACVMError, ZERO_ACVM_FIELD, acvm, convertACVMFieldToBuffer, @@ -198,11 +199,11 @@ export class PrivateFunctionExecution { }, this.abi.debug, ).catch((err: Error) => { - throw new SimulationError( - err.message, - { contractAddress: this.contractAddress, functionSelector: selector }, - err instanceof SimulationError ? err.getCallStack() : undefined, - ); + const failingFunction = { contractAddress: this.contractAddress, functionSelector: selector }; + if (err instanceof SimulationError) { + throw SimulationError.fromPreviousSimulationError(failingFunction, err); + } + throw SimulationError.new(err.message, failingFunction, err instanceof ACVMError ? err.callStack : undefined); }); const publicInputs = extractPrivateCircuitPublicInputs(partialWitness, acir); diff --git a/yarn-project/acir-simulator/src/client/unconstrained_execution.ts b/yarn-project/acir-simulator/src/client/unconstrained_execution.ts index a370240baddd..a7f58c792d25 100644 --- a/yarn-project/acir-simulator/src/client/unconstrained_execution.ts +++ b/yarn-project/acir-simulator/src/client/unconstrained_execution.ts @@ -3,10 +3,18 @@ import { DecodedReturn, decodeReturnValues } from '@aztec/foundation/abi'; import { AztecAddress } from '@aztec/foundation/aztec-address'; import { Fr } from '@aztec/foundation/fields'; import { createDebugLogger } from '@aztec/foundation/log'; -import { AztecNode } from '@aztec/types'; +import { AztecNode, SimulationError } from '@aztec/types'; import { extractReturnWitness, frToAztecAddress } from '../acvm/deserialize.js'; -import { ACVMField, ZERO_ACVM_FIELD, acvm, fromACVMField, toACVMField, toACVMWitness } from '../acvm/index.js'; +import { + ACVMError, + ACVMField, + ZERO_ACVM_FIELD, + acvm, + fromACVMField, + toACVMField, + toACVMWitness, +} from '../acvm/index.js'; import { AcirSimulator } from '../index.js'; import { ClientTxExecutionContext } from './client_execution_context.js'; import { FunctionAbiWithDebugMetadata } from './db_oracle.js'; @@ -33,9 +41,7 @@ export class UnconstrainedFunctionExecution { * @returns The return values of the executed function. */ public async run(aztecNode?: AztecNode): Promise { - this.log( - `Executing unconstrained function ${this.contractAddress.toShortString()}:${this.functionData.selector.toString()}`, - ); + this.log(`Executing unconstrained function ${this.contractAddress.toShortString()}:${this.functionData.selector}`); const acir = Buffer.from(this.abi.bytecode, 'base64'); const initialWitness = toACVMWitness(1, this.args); @@ -105,7 +111,13 @@ export class UnconstrainedFunctionExecution { }, }, this.abi.debug, - ); + ).catch((err: Error) => { + const failingFunction = { contractAddress: this.contractAddress, functionSelector: this.functionData.selector }; + if (err instanceof SimulationError) { + throw SimulationError.fromPreviousSimulationError(failingFunction, err); + } + throw SimulationError.new(err.message, failingFunction, err instanceof ACVMError ? err.callStack : undefined); + }); const returnValues: ACVMField[] = extractReturnWitness(acir, partialWitness); diff --git a/yarn-project/acir-simulator/src/public/executor.ts b/yarn-project/acir-simulator/src/public/executor.ts index e2c9a346cc80..c19f624f38e8 100644 --- a/yarn-project/acir-simulator/src/public/executor.ts +++ b/yarn-project/acir-simulator/src/public/executor.ts @@ -13,6 +13,7 @@ import { createDebugLogger } from '@aztec/foundation/log'; import { FunctionL2Logs, SimulationError } from '@aztec/types'; import { + ACVMError, ZERO_ACVM_FIELD, acvm, convertACVMFieldToBuffer, @@ -140,11 +141,11 @@ export class PublicExecutor { return Promise.resolve(toACVMField(portalContactAddress)); }, }).catch((err: Error) => { - throw new SimulationError( - err.message, - { contractAddress: execution.contractAddress, functionSelector: selector }, - err instanceof SimulationError ? err.getCallStack() : undefined, - ); + const failingFunction = { contractAddress: execution.contractAddress, functionSelector: selector }; + if (err instanceof SimulationError) { + throw SimulationError.fromPreviousSimulationError(failingFunction, err); + } + throw SimulationError.new(err.message, failingFunction, err instanceof ACVMError ? err.callStack : undefined); }); const { diff --git a/yarn-project/aztec-node/src/aztec-node/http-node.test.ts b/yarn-project/aztec-node/src/aztec-node/http-node.test.ts index 86faa5950c85..59aff00de986 100644 --- a/yarn-project/aztec-node/src/aztec-node/http-node.test.ts +++ b/yarn-project/aztec-node/src/aztec-node/http-node.test.ts @@ -1,4 +1,4 @@ -import { AztecAddress, CircuitsWasm, EthAddress, Fr, HistoricBlockData } from '@aztec/circuits.js'; +import { AztecAddress, CircuitsWasm, EthAddress, Fr, FunctionSelector, HistoricBlockData } from '@aztec/circuits.js'; import { randomBytes } from '@aztec/foundation/crypto'; import { Pedersen } from '@aztec/merkle-tree'; import { @@ -10,6 +10,7 @@ import { LogType, MerkleTreeId, SiblingPath, + SimulationError, TxHash, mockTx, } from '@aztec/types'; @@ -482,4 +483,36 @@ describe('HttpNode', () => { expect(result).toEqual(blockData); }); }); + + describe('simulatePublicPart', () => { + it('should fetch a successful simulation response', async () => { + const tx = mockTx(); + const response = {}; + setFetchMock(response); + + await httpNode.simulatePublicPart(tx); + + const init: RequestInit = { + method: 'POST', + body: tx.toBuffer(), + }; + const call = (fetch as jest.Mock).mock.calls[0] as any[]; + expect(call[0].href).toBe(`${TEST_URL}simulate-tx`); + expect(call[1]).toStrictEqual(init); + }); + + it('should fetch a simulation error', async () => { + const tx = mockTx(); + const simulationError = SimulationError.new('Failing function', { + contractAddress: AztecAddress.ZERO, + functionSelector: FunctionSelector.empty(), + }); + const response = { + simulationError: simulationError.toJSON(), + }; + setFetchMock(response); + + await expect(httpNode.simulatePublicPart(tx)).rejects.toThrow(simulationError); + }); + }); }); diff --git a/yarn-project/aztec-node/src/aztec-node/http-node.ts b/yarn-project/aztec-node/src/aztec-node/http-node.ts index 8497a246facb..e7367075fc16 100644 --- a/yarn-project/aztec-node/src/aztec-node/http-node.ts +++ b/yarn-project/aztec-node/src/aztec-node/http-node.ts @@ -381,7 +381,7 @@ export class HttpNode implements AztecNode { const init: RequestInit = {}; init['method'] = 'POST'; init['body'] = tx.toBuffer(); - const response = await (await fetch(url.toString(), init)).json(); + const response = await (await fetch(url, init)).json(); if (response.simulationError) { throw SimulationError.fromJSON(response.simulationError); } diff --git a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts index b91ff7ee5272..f2c9306ecaad 100644 --- a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts +++ b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts @@ -3,7 +3,6 @@ import { collectEncryptedLogs, collectEnqueuedPublicFunctionCalls, collectUnencryptedLogs, - printErrorStack, processAcvmError, } from '@aztec/acir-simulator'; import { @@ -189,6 +188,7 @@ export class AztecRPCServer implements AztecRPC { const newContract = deployedContractAddress ? await this.db.getContract(deployedContractAddress) : undefined; const tx = await this.#simulateAndProve(txRequest, newContract); + await this.#simulatePublicPart(tx); this.log.info(`Executed local simulation for ${await tx.getTxHash()}`); return tx; @@ -324,10 +324,16 @@ export class AztecRPCServer implements AztecRPC { const simulator = getAcirSimulator(this.db, this.node, this.node, this.node, this.keyStore, contractDataOracle); this.log('Executing simulator...'); - const result = await simulator.run(txRequest, functionAbi, contractAddress, portalContract); - this.log('Simulation completed!'); - - return result; + try { + const result = await simulator.run(txRequest, functionAbi, contractAddress, portalContract); + this.log('Simulation completed!'); + return result; + } catch (err) { + if (err instanceof SimulationError) { + await this.#debugLogSimulationError(err); + } + throw err; + } } /** @@ -349,18 +355,25 @@ export class AztecRPCServer implements AztecRPC { const simulator = getAcirSimulator(this.db, this.node, this.node, this.node, this.keyStore, contractDataOracle); - this.log('Executing unconstrained simulator...'); - const result = await simulator.runUnconstrained( - execRequest, - from ?? AztecAddress.ZERO, - functionAbi, - contractAddress, - portalContract, - this.node, - ); - this.log('Unconstrained simulation completed!'); + try { + this.log('Executing unconstrained simulator...'); + const result = await simulator.runUnconstrained( + execRequest, + from ?? AztecAddress.ZERO, + functionAbi, + contractAddress, + portalContract, + this.node, + ); + this.log('Unconstrained simulation completed!'); - return result; + return result; + } catch (err) { + if (err instanceof SimulationError) { + await this.#debugLogSimulationError(err); + } + throw err; + } } /** @@ -373,6 +386,7 @@ export class AztecRPCServer implements AztecRPC { try { await this.node.simulatePublicPart(tx); } catch (err) { + // Try to fill in the noir call stack since the RPC server may have access to the debug metadata if (err instanceof SimulationError) { const originalFailingFunction = err.getCallStack().pop()!; const contractDataOracle = new ContractDataOracle(this.db, this.node); @@ -383,10 +397,13 @@ export class AztecRPCServer implements AztecRPC { if (debugInfo) { const callStack = processAcvmError(err.message, debugInfo); if (callStack) { - this.log(printErrorStack(callStack)); - err.message = `Assertion failed in public execution: ${callStack.pop()?.locationText ?? 'Unknown'}`; + err.setNoirCallStack(callStack); + err.message = `Assertion failed in public execution: '${ + callStack[callStack.length - 1]?.locationText ?? 'Unknown' + }'`; } } + await this.#debugLogSimulationError(err); } throw err; @@ -437,11 +454,33 @@ export class AztecRPCServer implements AztecRPC { enqueuedPublicFunctions, ); - await this.#simulatePublicPart(tx); - return tx; } + async #debugLogSimulationError(err: SimulationError) { + const functionCallStack = err.getCallStack(); + const noirCallStack = err.getNoirCallStack(); + + // Try to resolve the contract and function names of the stack of failing functions. + const stackLines: string[] = [ + ...(await Promise.all( + functionCallStack.map(async ({ contractAddress, functionSelector }) => { + const contract = await this.db.getContract(contractAddress); + const functionAbi = contract?.functions.find(f => f.selector.equals(functionSelector)); + return ` at ${contract?.name ?? contractAddress.toString()}.${ + functionAbi?.name ?? functionSelector.toString() + }`; + }), + )), + ...noirCallStack.map( + sourceCodeLocation => + ` at ${sourceCodeLocation.filePath}:${sourceCodeLocation.line} '${sourceCodeLocation.locationText}'`, + ), + ]; + + this.log([`Simulation error: ${err.message}`, ...stackLines.reverse()].join('\n')); + } + // HACK(#1639): this is a hack to fix ordering of public calls enqueued in the call stack. Since the private kernel // cannot keep track of side effects that happen after or before a nested call, we override the public call stack // it emits with whatever we got from the simulator collected enqueued calls. As a sanity check, we at least verify diff --git a/yarn-project/end-to-end/src/e2e_nested_contract.test.ts b/yarn-project/end-to-end/src/e2e_nested_contract.test.ts index 73429762f952..a2eb9fe420fe 100644 --- a/yarn-project/end-to-end/src/e2e_nested_contract.test.ts +++ b/yarn-project/end-to-end/src/e2e_nested_contract.test.ts @@ -71,6 +71,14 @@ describe('e2e_nested_contract', () => { expect(receipt.status).toBe(TxStatus.MINED); }, 100_000); + it('fails simulation if calling a function not allowed to be called externally', async () => { + await expect( + parentContract.methods + .entryPoint(childContract.address, childContract.methods.valueInternal.selector.toField()) + .simulate(), + ).rejects.toThrowError(/Assertion failed: '.*'/); + }, 100_000); + it('performs public nested calls', async () => { const tx = parentContract.methods .pubEntryPoint(childContract.address, childContract.methods.pubGetValue.selector.toField(), 42n) @@ -94,6 +102,14 @@ describe('e2e_nested_contract', () => { expect(await getChildStoredValue(childContract)).toEqual(42n); }, 100_000); + it('fails simulation if calling a public function not allowed to be called externally', async () => { + await expect( + parentContract.methods + .enqueueCallToChild(childContract.address, childContract.methods.pubIncValueInternal.selector.toField(), 42n) + .simulate(), + ).rejects.toThrowError(/Assertion failed in public execution: '.*'/); + }, 100_000); + // Fails with "solver opcode resolution error: cannot solve opcode: expression has too many unknowns %EXPR [ 0 ]%" // See https://github.com/noir-lang/noir/issues/1347 // Task to repair this test: https://github.com/AztecProtocol/aztec-packages/issues/1587 diff --git a/yarn-project/noir-contracts/src/contracts/child_contract/src/main.nr b/yarn-project/noir-contracts/src/contracts/child_contract/src/main.nr index 222090acc6c5..9462d7ce67c1 100644 --- a/yarn-project/noir-contracts/src/contracts/child_contract/src/main.nr +++ b/yarn-project/noir-contracts/src/contracts/child_contract/src/main.nr @@ -24,6 +24,22 @@ contract Child { inputs: PrivateContextInputs, input: Field, ) -> distinct pub abi::PrivateCircuitPublicInputs { + assert(inputs.call_context.msg_sender == 0); + let mut context = PrivateContext::new(inputs, abi::hash_args([input])); + + context.return_values.push(input + context.chain_id() + context.version()); + + // Return private circuit public inputs. All private functions need to return this as it is part of the input of the private kernel. + context.finish() + } + + // Returns a sum of the input and the chain id and version of the contract in private circuit public input's return_values. + // Can only be called from this contract. + fn valueInternal( + inputs: PrivateContextInputs, + input: Field, + ) -> distinct pub abi::PrivateCircuitPublicInputs { + assert(inputs.call_context.msg_sender == inputs.call_context.storage_contract_address); let mut context = PrivateContext::new(inputs, abi::hash_args([input])); context.return_values.push(input + context.chain_id() + context.version()); @@ -65,6 +81,19 @@ contract Child { context.finish() } + // Increments `current_value` by `new_value`. Can only be called from this contract. + open fn pubIncValueInternal(inputs: PublicContextInputs, new_value: Field) -> pub abi::PublicCircuitPublicInputs { + assert(inputs.call_context.msg_sender == inputs.call_context.storage_contract_address); + let mut context = PublicContext::new(inputs, abi::hash_args([new_value])); + + let storage = Storage::init(); + let old_value = storage.current_value.read(); + storage.current_value.write(old_value + new_value); + let _hash = emit_unencrypted_log(new_value); + context.return_values.push(new_value); + context.finish() + } + open fn setValueTwiceWithNestedFirst( inputs: PublicContextInputs, ) -> pub abi::PublicCircuitPublicInputs { diff --git a/yarn-project/types/src/simulation_error.ts b/yarn-project/types/src/simulation_error.ts index 24d50bbbcd3e..a959b70e41b4 100644 --- a/yarn-project/types/src/simulation_error.ts +++ b/yarn-project/types/src/simulation_error.ts @@ -14,34 +14,92 @@ export interface FailingFunction { functionSelector: FunctionSelector; } +/** + * A pointer to a failing section of the noir source code. + */ +export interface SourceCodeLocation { + /** + * The path to the source file. + */ + filePath: string; + /** + * The line number of the call. + */ + line: number; + /** + * The source code of the file. + */ + fileSource: string; + /** + * The source code text of the failed constraint. + */ + locationText: string; +} + +/** + * A stack of noir source code locations. + */ +export type NoirCallStack = SourceCodeLocation[]; + /** * An error during the simulation of a function call. */ export class SimulationError extends Error { - constructor( + private constructor( message: string, private failingFunction: FailingFunction, - private previousErrorStack?: FailingFunction[], + private noirErrorStack?: NoirCallStack, + private functionErrorStack?: FailingFunction[], ) { super(message); } + static new(message: string, failingFunction: FailingFunction, noirErrorStack?: NoirCallStack) { + return new SimulationError(message, failingFunction, noirErrorStack); + } + + static fromPreviousSimulationError(failingFunction: FailingFunction, previousError: SimulationError) { + return new SimulationError( + previousError.message, + failingFunction, + previousError.noirErrorStack, + previousError.getCallStack(), + ); + } + /** - * The call stack of the failing simulation. + * The aztec function stack that failed during simulation. */ getCallStack(): FailingFunction[] { - return [this.failingFunction].concat(this.previousErrorStack || []); + return [this.failingFunction].concat(this.functionErrorStack || []); + } + + /** + * Returns the noir call stack inside the first function that failed during simulation. + * @returns The noir call stack. + */ + getNoirCallStack(): NoirCallStack { + return this.noirErrorStack || []; + } + + /** + * Sets the noir call stack. + * @param callStack - The noir call stack. + */ + setNoirCallStack(callStack: NoirCallStack) { + this.noirErrorStack = callStack; } toJSON() { return { message: this.message, failingFunction: this.failingFunction, - previousErrorStack: this.previousErrorStack, + noirErrorStack: this.noirErrorStack, + previousErrorStack: this.functionErrorStack, }; } static fromJSON(obj: any) { - return new SimulationError(obj.message, obj.failingFunction, obj.previousErrorStack); + return new SimulationError(obj.message, obj.failingFunction, obj.noirErrorStack, obj.previousErrorStack); } } From 8233a8207427a6d42986c225fd8c373935c8e3ea Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 29 Aug 2023 08:51:34 +0000 Subject: [PATCH 04/13] fix: cleanup --- yarn-project/acir-simulator/src/acvm/acvm.ts | 6 +++--- yarn-project/aztec-node/src/aztec-node/server.ts | 4 +--- .../aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts | 6 ++---- .../noir-contracts/src/contracts/child_contract/src/main.nr | 1 - 4 files changed, 6 insertions(+), 11 deletions(-) diff --git a/yarn-project/acir-simulator/src/acvm/acvm.ts b/yarn-project/acir-simulator/src/acvm/acvm.ts index 3aaac4130917..2b81f55650df 100644 --- a/yarn-project/acir-simulator/src/acvm/acvm.ts +++ b/yarn-project/acir-simulator/src/acvm/acvm.ts @@ -174,13 +174,13 @@ export async function acvm( throw typedError; } }, - ).catch((acvmError: string) => { + ).catch((acvmErrorString: string) => { if (oracleError) { throw oracleError; } if (debug) { - const callStack = processAcvmError(acvmError, debug); + const callStack = processAcvmError(acvmErrorString, debug); if (callStack) { throw new ACVMError( @@ -190,7 +190,7 @@ export async function acvm( } } // If we cannot find a callstack, throw the original error. - throw new ACVMError(acvmError); + throw new ACVMError(acvmErrorString); }); return Promise.resolve({ partialWitness }); diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index 4c39db479a60..29ecf74d8e12 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -392,11 +392,9 @@ export class AztecNodeService implements AztecNode { const newGlobalVariables = await this.globalVariableBuilder.buildGlobalVariables(new Fr(blockNumber)); const prevGlobalVariables = (await this.blockSource.getL2Block(-1))?.globalVariables ?? GlobalVariables.empty(); - // Process txs and drop the ones that fail processing - // We create a fresh processor each time to reset any cached state (eg storage writes) const processor = await publicProcessorFactory.create(prevGlobalVariables, newGlobalVariables); const [, failedTxs] = await processor.process([tx]); - if (failedTxs.length > 0) { + if (failedTxs.length) { throw failedTxs[0].error; } this.log.info(`Simulated tx ${await tx.getTxHash()} succeeds`); diff --git a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts index f2c9306ecaad..88fc5dcde21f 100644 --- a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts +++ b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts @@ -355,8 +355,8 @@ export class AztecRPCServer implements AztecRPC { const simulator = getAcirSimulator(this.db, this.node, this.node, this.node, this.keyStore, contractDataOracle); + this.log('Executing unconstrained simulator...'); try { - this.log('Executing unconstrained simulator...'); const result = await simulator.runUnconstrained( execRequest, from ?? AztecAddress.ZERO, @@ -445,7 +445,7 @@ export class AztecRPCServer implements AztecRPC { // TODO(#757): Enforce proper ordering of enqueued public calls await this.patchPublicCallStackOrdering(publicInputs, enqueuedPublicFunctions); - const tx = new Tx( + return new Tx( publicInputs, proof, encryptedLogs, @@ -453,8 +453,6 @@ export class AztecRPCServer implements AztecRPC { newContractPublicFunctions, enqueuedPublicFunctions, ); - - return tx; } async #debugLogSimulationError(err: SimulationError) { diff --git a/yarn-project/noir-contracts/src/contracts/child_contract/src/main.nr b/yarn-project/noir-contracts/src/contracts/child_contract/src/main.nr index 9462d7ce67c1..7112173160f4 100644 --- a/yarn-project/noir-contracts/src/contracts/child_contract/src/main.nr +++ b/yarn-project/noir-contracts/src/contracts/child_contract/src/main.nr @@ -24,7 +24,6 @@ contract Child { inputs: PrivateContextInputs, input: Field, ) -> distinct pub abi::PrivateCircuitPublicInputs { - assert(inputs.call_context.msg_sender == 0); let mut context = PrivateContext::new(inputs, abi::hash_args([input])); context.return_values.push(input + context.chain_id() + context.version()); From 7eda8a4e4c43fe66f9834de1e388b5e99c7a6332 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 29 Aug 2023 09:05:19 +0000 Subject: [PATCH 05/13] test: fix test --- .../sequencer-client/src/sequencer/public_processor.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn-project/sequencer-client/src/sequencer/public_processor.test.ts b/yarn-project/sequencer-client/src/sequencer/public_processor.test.ts index 2f2fa204759e..95a4fc37e043 100644 --- a/yarn-project/sequencer-client/src/sequencer/public_processor.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/public_processor.test.ts @@ -113,7 +113,7 @@ describe('public_processor', () => { const [processed, failed] = await processor.process([tx]); expect(processed).toEqual([]); - expect(failed).toEqual([tx]); + expect(failed[0].tx).toEqual(tx); }); }); From aba3018a8aaffe8dbac435aa3791cf615a389a7c Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 29 Aug 2023 10:31:47 +0000 Subject: [PATCH 06/13] fix: avoid simulations modifying state and txs --- yarn-project/aztec-node/src/aztec-node/server.ts | 11 ++++++++--- .../sequencer-client/src/sequencer/sequencer.ts | 3 ++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index 29ecf74d8e12..5cbfcfab8d13 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -88,7 +88,7 @@ export class AztecNodeService implements AztecNode { const p2pClient = await createP2PClient(config, new InMemoryTxPool(), archiver); // now create the merkle trees and the world state syncher - const merkleTreeDB = await MerkleTrees.new(levelup(createMemDown()), await CircuitsWasm.get()); + const merkleTreeDB = await AztecNodeService.createMerkleTreeDB(); const worldStateConfig: WorldStateConfig = getWorldStateConfig(); const worldStateSynchroniser = new ServerWorldStateSynchroniser(merkleTreeDB, archiver, worldStateConfig); @@ -119,6 +119,10 @@ export class AztecNodeService implements AztecNode { ); } + static async createMerkleTreeDB() { + return MerkleTrees.new(levelup(createMemDown()), await CircuitsWasm.get()); + } + /** * Method to determine if the node is ready to accept transactions. * @returns - Flag indicating the readiness for tx submission. @@ -382,16 +386,17 @@ export class AztecNodeService implements AztecNode { **/ public async simulatePublicPart(tx: Tx) { this.log.info(`Simulating tx ${await tx.getTxHash()}`); + // Instantiate merkle tree db so uncommited updates by this simulation are local to it. + const merkleTreeDb = await AztecNodeService.createMerkleTreeDB(); const publicProcessorFactory = new PublicProcessorFactory( - this.worldStateSynchroniser.getLatest(), + merkleTreeDb.asLatest(), this.contractDataSource, this.l1ToL2MessageSource, ); const blockNumber = (await this.blockSource.getBlockNumber()) + 1; const newGlobalVariables = await this.globalVariableBuilder.buildGlobalVariables(new Fr(blockNumber)); const prevGlobalVariables = (await this.blockSource.getL2Block(-1))?.globalVariables ?? GlobalVariables.empty(); - const processor = await publicProcessorFactory.create(prevGlobalVariables, newGlobalVariables); const [, failedTxs] = await processor.process([tx]); if (failedTxs.length) { diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.ts index 35748885af42..eb7dc60bf765 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.ts @@ -147,7 +147,8 @@ export class Sequencer { // Process txs and drop the ones that fail processing // We create a fresh processor each time to reset any cached state (eg storage writes) const processor = await this.publicProcessorFactory.create(prevGlobalVariables, newGlobalVariables); - const [processedTxs, failedTxs] = await processor.process(validTxs); + // The processor modifies the tx objects in place, so we need to clone them. + const [processedTxs, failedTxs] = await processor.process(validTxs.map(tx => Tx.fromJSON(tx.toJSON()))); if (failedTxs.length > 0) { const failedTxData = failedTxs.map(fail => fail.tx); this.log(`Dropping failed txs ${(await Tx.getHashes(failedTxData)).join(', ')}`); From 489474785e5540550340fc2157d83e2d9684a615 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 29 Aug 2023 10:34:50 +0000 Subject: [PATCH 07/13] fix: clone inside processor to avoid the mutation --- .../sequencer-client/src/sequencer/public_processor.ts | 2 ++ yarn-project/sequencer-client/src/sequencer/sequencer.ts | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/yarn-project/sequencer-client/src/sequencer/public_processor.ts b/yarn-project/sequencer-client/src/sequencer/public_processor.ts index 37e9fcef7856..6ba99647bb4e 100644 --- a/yarn-project/sequencer-client/src/sequencer/public_processor.ts +++ b/yarn-project/sequencer-client/src/sequencer/public_processor.ts @@ -107,6 +107,8 @@ export class PublicProcessor { * @returns The list of processed txs with their circuit simulation outputs. */ public async process(txs: Tx[]): Promise<[ProcessedTx[], FailedTx[]]> { + // The processor modifies the tx objects in place, so we need to clone them. + txs = txs.map(tx => Tx.fromJSON(tx.toJSON())); const result: ProcessedTx[] = []; const failed: FailedTx[] = []; diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.ts index eb7dc60bf765..35748885af42 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.ts @@ -147,8 +147,7 @@ export class Sequencer { // Process txs and drop the ones that fail processing // We create a fresh processor each time to reset any cached state (eg storage writes) const processor = await this.publicProcessorFactory.create(prevGlobalVariables, newGlobalVariables); - // The processor modifies the tx objects in place, so we need to clone them. - const [processedTxs, failedTxs] = await processor.process(validTxs.map(tx => Tx.fromJSON(tx.toJSON()))); + const [processedTxs, failedTxs] = await processor.process(validTxs); if (failedTxs.length > 0) { const failedTxData = failedTxs.map(fail => fail.tx); this.log(`Dropping failed txs ${(await Tx.getHashes(failedTxData)).join(', ')}`); From 213e465d6a1a3cb944fefffa5ac2e50b39716abf Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 29 Aug 2023 11:04:33 +0000 Subject: [PATCH 08/13] refactor: improve printing of simulation errors --- .../src/client/private_execution.ts | 4 +-- .../src/client/unconstrained_execution.ts | 4 +-- .../acir-simulator/src/public/executor.ts | 4 +-- .../src/aztec-node/http-node.test.ts | 2 +- yarn-project/types/src/simulation_error.ts | 34 ++++++++----------- 5 files changed, 22 insertions(+), 26 deletions(-) diff --git a/yarn-project/acir-simulator/src/client/private_execution.ts b/yarn-project/acir-simulator/src/client/private_execution.ts index e8f2785fb073..67daf8ee3484 100644 --- a/yarn-project/acir-simulator/src/client/private_execution.ts +++ b/yarn-project/acir-simulator/src/client/private_execution.ts @@ -201,9 +201,9 @@ export class PrivateFunctionExecution { ).catch((err: Error) => { const failingFunction = { contractAddress: this.contractAddress, functionSelector: selector }; if (err instanceof SimulationError) { - throw SimulationError.fromPreviousSimulationError(failingFunction, err); + throw SimulationError.extendPreviousSimulationError(failingFunction, err); } - throw SimulationError.new(err.message, failingFunction, err instanceof ACVMError ? err.callStack : undefined); + throw new SimulationError(err.message, failingFunction, err instanceof ACVMError ? err.callStack : undefined); }); const publicInputs = extractPrivateCircuitPublicInputs(partialWitness, acir); diff --git a/yarn-project/acir-simulator/src/client/unconstrained_execution.ts b/yarn-project/acir-simulator/src/client/unconstrained_execution.ts index a7f58c792d25..ac930dac7073 100644 --- a/yarn-project/acir-simulator/src/client/unconstrained_execution.ts +++ b/yarn-project/acir-simulator/src/client/unconstrained_execution.ts @@ -114,9 +114,9 @@ export class UnconstrainedFunctionExecution { ).catch((err: Error) => { const failingFunction = { contractAddress: this.contractAddress, functionSelector: this.functionData.selector }; if (err instanceof SimulationError) { - throw SimulationError.fromPreviousSimulationError(failingFunction, err); + throw SimulationError.extendPreviousSimulationError(failingFunction, err); } - throw SimulationError.new(err.message, failingFunction, err instanceof ACVMError ? err.callStack : undefined); + throw new SimulationError(err.message, failingFunction, err instanceof ACVMError ? err.callStack : undefined); }); const returnValues: ACVMField[] = extractReturnWitness(acir, partialWitness); diff --git a/yarn-project/acir-simulator/src/public/executor.ts b/yarn-project/acir-simulator/src/public/executor.ts index c19f624f38e8..97c4d14e01ac 100644 --- a/yarn-project/acir-simulator/src/public/executor.ts +++ b/yarn-project/acir-simulator/src/public/executor.ts @@ -143,9 +143,9 @@ export class PublicExecutor { }).catch((err: Error) => { const failingFunction = { contractAddress: execution.contractAddress, functionSelector: selector }; if (err instanceof SimulationError) { - throw SimulationError.fromPreviousSimulationError(failingFunction, err); + throw SimulationError.extendPreviousSimulationError(failingFunction, err); } - throw SimulationError.new(err.message, failingFunction, err instanceof ACVMError ? err.callStack : undefined); + throw new SimulationError(err.message, failingFunction, err instanceof ACVMError ? err.callStack : undefined); }); const { diff --git a/yarn-project/aztec-node/src/aztec-node/http-node.test.ts b/yarn-project/aztec-node/src/aztec-node/http-node.test.ts index 59aff00de986..f022f9b08f15 100644 --- a/yarn-project/aztec-node/src/aztec-node/http-node.test.ts +++ b/yarn-project/aztec-node/src/aztec-node/http-node.test.ts @@ -503,7 +503,7 @@ describe('HttpNode', () => { it('should fetch a simulation error', async () => { const tx = mockTx(); - const simulationError = SimulationError.new('Failing function', { + const simulationError = new SimulationError('Failing function', { contractAddress: AztecAddress.ZERO, functionSelector: FunctionSelector.empty(), }); diff --git a/yarn-project/types/src/simulation_error.ts b/yarn-project/types/src/simulation_error.ts index a959b70e41b4..dade1f6449bc 100644 --- a/yarn-project/types/src/simulation_error.ts +++ b/yarn-project/types/src/simulation_error.ts @@ -45,33 +45,28 @@ export type NoirCallStack = SourceCodeLocation[]; * An error during the simulation of a function call. */ export class SimulationError extends Error { - private constructor( - message: string, - private failingFunction: FailingFunction, - private noirErrorStack?: NoirCallStack, - private functionErrorStack?: FailingFunction[], - ) { + private functionErrorStack: FailingFunction[]; + + // We want to maintain a public constructor for proper printing. + constructor(message: string, failingFunction: FailingFunction, private noirErrorStack?: NoirCallStack) { super(message); + this.functionErrorStack = [failingFunction]; } - static new(message: string, failingFunction: FailingFunction, noirErrorStack?: NoirCallStack) { - return new SimulationError(message, failingFunction, noirErrorStack); + private addCaller(failingFunction: FailingFunction) { + this.functionErrorStack.unshift(failingFunction); } - static fromPreviousSimulationError(failingFunction: FailingFunction, previousError: SimulationError) { - return new SimulationError( - previousError.message, - failingFunction, - previousError.noirErrorStack, - previousError.getCallStack(), - ); + static extendPreviousSimulationError(failingFunction: FailingFunction, previousError: SimulationError) { + previousError.addCaller(failingFunction); + return previousError; } /** * The aztec function stack that failed during simulation. */ getCallStack(): FailingFunction[] { - return [this.failingFunction].concat(this.functionErrorStack || []); + return this.functionErrorStack; } /** @@ -93,13 +88,14 @@ export class SimulationError extends Error { toJSON() { return { message: this.message, - failingFunction: this.failingFunction, + functionErrorStack: this.functionErrorStack, noirErrorStack: this.noirErrorStack, - previousErrorStack: this.functionErrorStack, }; } static fromJSON(obj: any) { - return new SimulationError(obj.message, obj.failingFunction, obj.noirErrorStack, obj.previousErrorStack); + const error = new SimulationError(obj.message, obj.functionErrorStack[0], obj.noirErrorStack); + error.functionErrorStack = obj.functionErrorStack; + return error; } } From 21e7a0b66f8c8e1df7f73fd6db62769504b2ae84 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 29 Aug 2023 12:43:04 +0000 Subject: [PATCH 09/13] feat: initial approach of transactional merkle db --- yarn-project/acir-simulator/src/acvm/acvm.ts | 2 +- .../src/client/private_execution.ts | 4 +- .../src/client/unconstrained_execution.ts | 4 +- .../acir-simulator/src/public/executor.ts | 4 +- .../aztec-node/src/aztec-node/server.ts | 10 ++-- yarn-project/types/src/simulation_error.ts | 9 ++- .../src/world-state-db/merkle_trees.ts | 56 +++++++++++++++---- 7 files changed, 66 insertions(+), 23 deletions(-) diff --git a/yarn-project/acir-simulator/src/acvm/acvm.ts b/yarn-project/acir-simulator/src/acvm/acvm.ts index 2b81f55650df..7be30ea0e821 100644 --- a/yarn-project/acir-simulator/src/acvm/acvm.ts +++ b/yarn-project/acir-simulator/src/acvm/acvm.ts @@ -170,7 +170,7 @@ export async function acvm( typedError = new Error(`Error in oracle callback ${err}`); } oracleError = typedError; - logger.error(`Error in oracle callback ${name}: ${typedError.message}`); + logger.error(`Error in oracle callback ${name}:`, typedError); throw typedError; } }, diff --git a/yarn-project/acir-simulator/src/client/private_execution.ts b/yarn-project/acir-simulator/src/client/private_execution.ts index 67daf8ee3484..d9cec52106cb 100644 --- a/yarn-project/acir-simulator/src/client/private_execution.ts +++ b/yarn-project/acir-simulator/src/client/private_execution.ts @@ -203,7 +203,9 @@ export class PrivateFunctionExecution { if (err instanceof SimulationError) { throw SimulationError.extendPreviousSimulationError(failingFunction, err); } - throw new SimulationError(err.message, failingFunction, err instanceof ACVMError ? err.callStack : undefined); + throw new SimulationError(err.message, failingFunction, err instanceof ACVMError ? err.callStack : undefined, { + cause: err, + }); }); const publicInputs = extractPrivateCircuitPublicInputs(partialWitness, acir); diff --git a/yarn-project/acir-simulator/src/client/unconstrained_execution.ts b/yarn-project/acir-simulator/src/client/unconstrained_execution.ts index ac930dac7073..081d8f14c4dd 100644 --- a/yarn-project/acir-simulator/src/client/unconstrained_execution.ts +++ b/yarn-project/acir-simulator/src/client/unconstrained_execution.ts @@ -116,7 +116,9 @@ export class UnconstrainedFunctionExecution { if (err instanceof SimulationError) { throw SimulationError.extendPreviousSimulationError(failingFunction, err); } - throw new SimulationError(err.message, failingFunction, err instanceof ACVMError ? err.callStack : undefined); + throw new SimulationError(err.message, failingFunction, err instanceof ACVMError ? err.callStack : undefined, { + cause: err, + }); }); const returnValues: ACVMField[] = extractReturnWitness(acir, partialWitness); diff --git a/yarn-project/acir-simulator/src/public/executor.ts b/yarn-project/acir-simulator/src/public/executor.ts index 97c4d14e01ac..152533c99ad9 100644 --- a/yarn-project/acir-simulator/src/public/executor.ts +++ b/yarn-project/acir-simulator/src/public/executor.ts @@ -145,7 +145,9 @@ export class PublicExecutor { if (err instanceof SimulationError) { throw SimulationError.extendPreviousSimulationError(failingFunction, err); } - throw new SimulationError(err.message, failingFunction, err instanceof ACVMError ? err.callStack : undefined); + throw new SimulationError(err.message, failingFunction, err instanceof ACVMError ? err.callStack : undefined, { + cause: err, + }); }); const { diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index 5cbfcfab8d13..5ecfaa2881e7 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -68,6 +68,7 @@ export class AztecNodeService implements AztecNode { protected chainId: number, protected version: number, protected globalVariableBuilder: GlobalVariableBuilder, + protected merkleTreeDb: MerkleTrees, private log = createDebugLogger('aztec:node'), ) {} @@ -88,7 +89,7 @@ export class AztecNodeService implements AztecNode { const p2pClient = await createP2PClient(config, new InMemoryTxPool(), archiver); // now create the merkle trees and the world state syncher - const merkleTreeDB = await AztecNodeService.createMerkleTreeDB(); + const merkleTreeDB = await MerkleTrees.new(levelup(createMemDown()), await CircuitsWasm.get()); const worldStateConfig: WorldStateConfig = getWorldStateConfig(); const worldStateSynchroniser = new ServerWorldStateSynchroniser(merkleTreeDB, archiver, worldStateConfig); @@ -116,13 +117,10 @@ export class AztecNodeService implements AztecNode { config.chainId, config.version, getGlobalVariableBuilder(config), + merkleTreeDB, ); } - static async createMerkleTreeDB() { - return MerkleTrees.new(levelup(createMemDown()), await CircuitsWasm.get()); - } - /** * Method to determine if the node is ready to accept transactions. * @returns - Flag indicating the readiness for tx submission. @@ -387,7 +385,7 @@ export class AztecNodeService implements AztecNode { public async simulatePublicPart(tx: Tx) { this.log.info(`Simulating tx ${await tx.getTxHash()}`); // Instantiate merkle tree db so uncommited updates by this simulation are local to it. - const merkleTreeDb = await AztecNodeService.createMerkleTreeDB(); + const merkleTreeDb = await this.merkleTreeDb.forTransaction(); const publicProcessorFactory = new PublicProcessorFactory( merkleTreeDb.asLatest(), diff --git a/yarn-project/types/src/simulation_error.ts b/yarn-project/types/src/simulation_error.ts index dade1f6449bc..0af9bda87ecd 100644 --- a/yarn-project/types/src/simulation_error.ts +++ b/yarn-project/types/src/simulation_error.ts @@ -48,8 +48,13 @@ export class SimulationError extends Error { private functionErrorStack: FailingFunction[]; // We want to maintain a public constructor for proper printing. - constructor(message: string, failingFunction: FailingFunction, private noirErrorStack?: NoirCallStack) { - super(message); + constructor( + message: string, + failingFunction: FailingFunction, + private noirErrorStack?: NoirCallStack, + options?: ErrorOptions, + ) { + super(message, options); this.functionErrorStack = [failingFunction]; } diff --git a/yarn-project/world-state/src/world-state-db/merkle_trees.ts b/yarn-project/world-state/src/world-state-db/merkle_trees.ts index 1b502f5ba02b..fab7d9c1fa87 100644 --- a/yarn-project/world-state/src/world-state-db/merkle_trees.ts +++ b/yarn-project/world-state/src/world-state-db/merkle_trees.ts @@ -25,6 +25,7 @@ import { StandardIndexedTree, StandardTree, UpdateOnlyTree, + loadTree, newTree, } from '@aztec/merkle-tree'; import { L2Block, MerkleTreeId, SiblingPath, merkleTreeIds } from '@aztec/types'; @@ -43,6 +44,16 @@ import { TreeInfo, } from './index.js'; +/** + * Data necessary to reinitialise the merkle trees from Db. + */ +interface FromDbOptions { + /** + * The global variables from the last block. + */ + globalVariables: GlobalVariables; +} + /** * A convenience class for managing multiple merkle trees. */ @@ -58,18 +69,22 @@ export class MerkleTrees implements MerkleTreeDb { /** * Initialises the collection of Merkle Trees. * @param optionalWasm - WASM instance to use for hashing (if not provided PrimitivesWasm will be used). + * @param fromDbOptions - Options to initialise the trees from the database. */ - public async init(optionalWasm?: IWasmModule) { + public async init(optionalWasm?: IWasmModule, fromDbOptions?: FromDbOptions) { + const fromDb = fromDbOptions !== undefined; + const initialiseTree = fromDb ? loadTree : newTree; + const wasm = optionalWasm ?? (await CircuitsWasm.get()); const hasher = new Pedersen(wasm); - const contractTree: AppendOnlyTree = await newTree( + const contractTree: AppendOnlyTree = await initialiseTree( StandardTree, this.db, hasher, `${MerkleTreeId[MerkleTreeId.CONTRACT_TREE]}`, CONTRACT_TREE_HEIGHT, ); - const nullifierTree = await newTree( + const nullifierTree = await initialiseTree( StandardIndexedTree, this.db, hasher, @@ -77,28 +92,28 @@ export class MerkleTrees implements MerkleTreeDb { NULLIFIER_TREE_HEIGHT, INITIAL_NULLIFIER_TREE_SIZE, ); - const privateDataTree: AppendOnlyTree = await newTree( + const privateDataTree: AppendOnlyTree = await initialiseTree( StandardTree, this.db, hasher, `${MerkleTreeId[MerkleTreeId.PRIVATE_DATA_TREE]}`, PRIVATE_DATA_TREE_HEIGHT, ); - const publicDataTree: UpdateOnlyTree = await newTree( + const publicDataTree: UpdateOnlyTree = await initialiseTree( SparseTree, this.db, hasher, `${MerkleTreeId[MerkleTreeId.PUBLIC_DATA_TREE]}`, PUBLIC_DATA_TREE_HEIGHT, ); - const l1Tol2MessagesTree: AppendOnlyTree = await newTree( + const l1Tol2MessagesTree: AppendOnlyTree = await initialiseTree( StandardTree, this.db, hasher, `${MerkleTreeId[MerkleTreeId.L1_TO_L2_MESSAGES_TREE]}`, L1_TO_L2_MSG_TREE_HEIGHT, ); - const historicBlocksTree: AppendOnlyTree = await newTree( + const historicBlocksTree: AppendOnlyTree = await initialiseTree( StandardTree, this.db, hasher, @@ -110,10 +125,14 @@ export class MerkleTrees implements MerkleTreeDb { this.jobQueue.start(); // The first leaf in the blocks tree contains the empty roots of the other trees and empty global variables. - const initialGlobalVariablesHash = await computeGlobalVariablesHash(GlobalVariables.empty()); - await this._updateLatestGlobalVariablesHash(initialGlobalVariablesHash); - await this._updateHistoricBlocksTree(initialGlobalVariablesHash, true); - await this._commit(); + if (!fromDb) { + const initialGlobalVariablesHash = await computeGlobalVariablesHash(GlobalVariables.empty()); + await this._updateLatestGlobalVariablesHash(initialGlobalVariablesHash); + await this._updateHistoricBlocksTree(initialGlobalVariablesHash, true); + await this._commit(); + } else { + await this.updateLatestGlobalVariablesHash(await computeGlobalVariablesHash(fromDbOptions.globalVariables)); + } } /** @@ -392,6 +411,21 @@ export class MerkleTrees implements MerkleTreeDb { return await this.synchronise(() => tree.batchInsert(leaves, subtreeHeight)); } + /** + * Returns a new merkle trees instance with an independent cache. + * @param fromDbOptions - Options to initialise the trees from the database. + * @param optionalWasm - WASM instance to use for hashing (if not provided PrimitivesWasm will be used). + * @returns A new merkle trees instance. + */ + public async forTransaction(optionalWasm?: IWasmModule) { + const newMerkleTrees = new MerkleTrees(this.db, this.log); + await newMerkleTrees.init(optionalWasm, { + globalVariables: GlobalVariables.empty(), + }); + await newMerkleTrees.updateLatestGlobalVariablesHash(this.latestGlobalVariablesHash.get()); + return newMerkleTrees; + } + /** * Waits for all jobs to finish before executing the given function. * @param fn - The function to execute. From 87b53b233b1c1c488a6aaa17bfdd6d3f2b4c5a37 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 29 Aug 2023 14:07:01 +0000 Subject: [PATCH 10/13] feat: option skip public sim & refactor trees txs --- .../aztec-node/src/aztec-node/server.ts | 25 +++++++++++-------- .../src/aztec_rpc_server/aztec_rpc_server.ts | 21 +++++++++------- .../test/aztec_rpc_test_suite.ts | 2 +- .../aztec.js/src/aztec_rpc_client/wallet.ts | 4 +-- .../src/contract/base_contract_interaction.ts | 6 ++++- .../src/e2e_lending_contract.test.ts | 6 +++-- .../types/src/interfaces/aztec_rpc.ts | 4 +-- yarn-project/types/src/simulation_error.ts | 12 +++++++++ .../src/world-state-db/merkle_trees.ts | 17 +------------ 9 files changed, 54 insertions(+), 43 deletions(-) diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index 5ecfaa2881e7..0d5bbf69c9dd 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -68,7 +68,7 @@ export class AztecNodeService implements AztecNode { protected chainId: number, protected version: number, protected globalVariableBuilder: GlobalVariableBuilder, - protected merkleTreeDb: MerkleTrees, + protected merkleTreesDb: levelup.LevelUp, private log = createDebugLogger('aztec:node'), ) {} @@ -89,9 +89,10 @@ export class AztecNodeService implements AztecNode { const p2pClient = await createP2PClient(config, new InMemoryTxPool(), archiver); // now create the merkle trees and the world state syncher - const merkleTreeDB = await MerkleTrees.new(levelup(createMemDown()), await CircuitsWasm.get()); + const merkleTreesDb = levelup(createMemDown()); + const merkleTrees = await MerkleTrees.new(merkleTreesDb, await CircuitsWasm.get()); const worldStateConfig: WorldStateConfig = getWorldStateConfig(); - const worldStateSynchroniser = new ServerWorldStateSynchroniser(merkleTreeDB, archiver, worldStateConfig); + const worldStateSynchroniser = new ServerWorldStateSynchroniser(merkleTrees, archiver, worldStateConfig); // start both and wait for them to sync from the block source await Promise.all([p2pClient.start(), worldStateSynchroniser.start()]); @@ -117,7 +118,7 @@ export class AztecNodeService implements AztecNode { config.chainId, config.version, getGlobalVariableBuilder(config), - merkleTreeDB, + merkleTreesDb, ); } @@ -384,17 +385,21 @@ export class AztecNodeService implements AztecNode { **/ public async simulatePublicPart(tx: Tx) { this.log.info(`Simulating tx ${await tx.getTxHash()}`); - // Instantiate merkle tree db so uncommited updates by this simulation are local to it. - const merkleTreeDb = await this.merkleTreeDb.forTransaction(); + const blockNumber = (await this.blockSource.getBlockNumber()) + 1; + const newGlobalVariables = await this.globalVariableBuilder.buildGlobalVariables(new Fr(blockNumber)); + const prevGlobalVariables = (await this.blockSource.getL2Block(-1))?.globalVariables ?? GlobalVariables.empty(); + + // Instantiate merkle trees so uncommited updates by this simulation are local to it. + const merkleTrees = new MerkleTrees(this.merkleTreesDb, this.log); + await merkleTrees.init(await CircuitsWasm.get(), { + globalVariables: prevGlobalVariables, + }); const publicProcessorFactory = new PublicProcessorFactory( - merkleTreeDb.asLatest(), + merkleTrees.asLatest(), this.contractDataSource, this.l1ToL2MessageSource, ); - const blockNumber = (await this.blockSource.getBlockNumber()) + 1; - const newGlobalVariables = await this.globalVariableBuilder.buildGlobalVariables(new Fr(blockNumber)); - const prevGlobalVariables = (await this.blockSource.getL2Block(-1))?.globalVariables ?? GlobalVariables.empty(); const processor = await publicProcessorFactory.create(prevGlobalVariables, newGlobalVariables); const [, failedTxs] = await processor.process([tx]); if (failedTxs.length) { diff --git a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts index c01992496683..924c7cf9d769 100644 --- a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts +++ b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts @@ -178,7 +178,7 @@ export class AztecRPCServer implements AztecRPC { return await this.node.getBlock(blockNumber); } - public async simulateTx(txRequest: TxExecutionRequest) { + public async simulateTx(txRequest: TxExecutionRequest, simulatePublic: boolean) { if (!txRequest.functionData.isPrivate) { throw new Error(`Public entrypoints are not allowed`); } @@ -192,7 +192,7 @@ export class AztecRPCServer implements AztecRPC { const newContract = deployedContractAddress ? await this.db.getContract(deployedContractAddress) : undefined; const tx = await this.#simulateAndProve(txRequest, newContract); - await this.#simulatePublicPart(tx); + if (simulatePublic) await this.#simulatePublicPart(tx); this.log.info(`Executed local simulation for ${await tx.getTxHash()}`); return tx; @@ -393,19 +393,22 @@ export class AztecRPCServer implements AztecRPC { } catch (err) { // Try to fill in the noir call stack since the RPC server may have access to the debug metadata if (err instanceof SimulationError) { - const originalFailingFunction = err.getCallStack().pop()!; + const callStack = err.getCallStack(); + const originalFailingFunction = callStack[callStack.length - 1]; const contractDataOracle = new ContractDataOracle(this.db, this.node); const debugInfo = await contractDataOracle.getFunctionDebugMetadata( originalFailingFunction.contractAddress, originalFailingFunction.functionSelector, ); if (debugInfo) { - const callStack = processAcvmError(err.message, debugInfo); - if (callStack) { - err.setNoirCallStack(callStack); - err.message = `Assertion failed in public execution: '${ - callStack[callStack.length - 1]?.locationText ?? 'Unknown' - }'`; + const noirCallStack = processAcvmError(err.message, debugInfo); + if (noirCallStack) { + err.setNoirCallStack(noirCallStack); + err.updateMessage( + `Assertion failed in public execution: '${ + noirCallStack[noirCallStack.length - 1]?.locationText ?? 'Unknown' + }'`, + ); } } await this.#debugLogSimulationError(err); diff --git a/yarn-project/aztec-rpc/src/aztec_rpc_server/test/aztec_rpc_test_suite.ts b/yarn-project/aztec-rpc/src/aztec_rpc_server/test/aztec_rpc_test_suite.ts index 0747e1149efa..501b9240f1d7 100644 --- a/yarn-project/aztec-rpc/src/aztec_rpc_server/test/aztec_rpc_test_suite.ts +++ b/yarn-project/aztec-rpc/src/aztec_rpc_server/test/aztec_rpc_test_suite.ts @@ -107,7 +107,7 @@ export const aztecRpcTestSuite = (testName: string, aztecRpcSetup: () => Promise [], ); - await expect(async () => await rpc.simulateTx(txExecutionRequest)).rejects.toThrow( + await expect(async () => await rpc.simulateTx(txExecutionRequest, false)).rejects.toThrow( 'Public entrypoints are not allowed', ); }); diff --git a/yarn-project/aztec.js/src/aztec_rpc_client/wallet.ts b/yarn-project/aztec.js/src/aztec_rpc_client/wallet.ts index 12c0fce9e4cd..53e74796500b 100644 --- a/yarn-project/aztec.js/src/aztec_rpc_client/wallet.ts +++ b/yarn-project/aztec.js/src/aztec_rpc_client/wallet.ts @@ -55,8 +55,8 @@ export abstract class BaseWallet implements Wallet { getContracts(): Promise { return this.rpc.getContracts(); } - simulateTx(txRequest: TxExecutionRequest): Promise { - return this.rpc.simulateTx(txRequest); + simulateTx(txRequest: TxExecutionRequest, simulatePublic: boolean): Promise { + return this.rpc.simulateTx(txRequest, simulatePublic); } sendTx(tx: Tx): Promise { return this.rpc.sendTx(tx); diff --git a/yarn-project/aztec.js/src/contract/base_contract_interaction.ts b/yarn-project/aztec.js/src/contract/base_contract_interaction.ts index 07db8847daa2..2412401f5dfa 100644 --- a/yarn-project/aztec.js/src/contract/base_contract_interaction.ts +++ b/yarn-project/aztec.js/src/contract/base_contract_interaction.ts @@ -12,6 +12,10 @@ export interface SendMethodOptions { * Sender's address initiating the transaction. */ origin?: AztecAddress; + /** + * Wether to skip the simulation of the public part of the transaction. + */ + skipPublicSimulation?: boolean; } /** @@ -38,7 +42,7 @@ export abstract class BaseContractInteraction { */ public async simulate(options: SendMethodOptions = {}): Promise { const txRequest = this.txRequest ?? (await this.create(options)); - this.tx = await this.rpc.simulateTx(txRequest); + this.tx = await this.rpc.simulateTx(txRequest, !options.skipPublicSimulation); return this.tx; } diff --git a/yarn-project/end-to-end/src/e2e_lending_contract.test.ts b/yarn-project/end-to-end/src/e2e_lending_contract.test.ts index 1e305ba7688e..2eee79c81eeb 100644 --- a/yarn-project/end-to-end/src/e2e_lending_contract.test.ts +++ b/yarn-project/end-to-end/src/e2e_lending_contract.test.ts @@ -535,7 +535,9 @@ describe('e2e_lending_contract', () => { { // Withdraw more than possible to test the revert. logger('Withdraw: trying to withdraw more than possible'); - const tx = lendingContract.methods.withdraw_public(recipient, 10n ** 9n).send({ origin: recipient }); + const tx = lendingContract.methods + .withdraw_public(recipient, 10n ** 9n) + .send({ origin: recipient, skipPublicSimulation: true }); await tx.isMined({ interval: 0.1 }); const receipt = await tx.getReceipt(); expect(receipt.status).toBe(TxStatus.DROPPED); @@ -601,7 +603,7 @@ describe('e2e_lending_contract', () => { const tx = lendingContract.methods ._deposit(recipient.toField(), 42n, collateralAsset.address) - .send({ origin: recipient }); + .send({ origin: recipient, skipPublicSimulation: true }); await tx.isMined({ interval: 0.1 }); const receipt = await tx.getReceipt(); expect(receipt.status).toBe(TxStatus.DROPPED); diff --git a/yarn-project/types/src/interfaces/aztec_rpc.ts b/yarn-project/types/src/interfaces/aztec_rpc.ts index 3c4ce90c0e87..549725d42c42 100644 --- a/yarn-project/types/src/interfaces/aztec_rpc.ts +++ b/yarn-project/types/src/interfaces/aztec_rpc.ts @@ -139,10 +139,10 @@ export interface AztecRPC { * Throws an error if the contract or function is unknown. * * @param txRequest - An authenticated tx request ready for simulation - * @param optionalFromAddress - The address to simulate from + * @param simulatePublic - Whether to simulate the public part of the transaction. * @returns A Tx ready to send to the p2p pool for execution. */ - simulateTx(txRequest: TxExecutionRequest): Promise; + simulateTx(txRequest: TxExecutionRequest, simulatePublic: boolean): Promise; /** * Send a transaction. diff --git a/yarn-project/types/src/simulation_error.ts b/yarn-project/types/src/simulation_error.ts index 0af9bda87ecd..9eece2c769f7 100644 --- a/yarn-project/types/src/simulation_error.ts +++ b/yarn-project/types/src/simulation_error.ts @@ -67,6 +67,18 @@ export class SimulationError extends Error { return previousError; } + /** + * Updates the error message. This is needed because in some engines the stack also contains the message. + * @param newMessage - The new message of this error. + */ + updateMessage(newMessage: string) { + const oldMessage = this.message; + this.message = newMessage; + if (this.stack?.startsWith(`Error: ${oldMessage}`)) { + this.stack = this.stack?.replace(`Error: ${oldMessage}`, `Error: ${newMessage}`); + } + } + /** * The aztec function stack that failed during simulation. */ diff --git a/yarn-project/world-state/src/world-state-db/merkle_trees.ts b/yarn-project/world-state/src/world-state-db/merkle_trees.ts index fab7d9c1fa87..23166d507872 100644 --- a/yarn-project/world-state/src/world-state-db/merkle_trees.ts +++ b/yarn-project/world-state/src/world-state-db/merkle_trees.ts @@ -131,7 +131,7 @@ export class MerkleTrees implements MerkleTreeDb { await this._updateHistoricBlocksTree(initialGlobalVariablesHash, true); await this._commit(); } else { - await this.updateLatestGlobalVariablesHash(await computeGlobalVariablesHash(fromDbOptions.globalVariables)); + await this._updateLatestGlobalVariablesHash(await computeGlobalVariablesHash(fromDbOptions.globalVariables)); } } @@ -411,21 +411,6 @@ export class MerkleTrees implements MerkleTreeDb { return await this.synchronise(() => tree.batchInsert(leaves, subtreeHeight)); } - /** - * Returns a new merkle trees instance with an independent cache. - * @param fromDbOptions - Options to initialise the trees from the database. - * @param optionalWasm - WASM instance to use for hashing (if not provided PrimitivesWasm will be used). - * @returns A new merkle trees instance. - */ - public async forTransaction(optionalWasm?: IWasmModule) { - const newMerkleTrees = new MerkleTrees(this.db, this.log); - await newMerkleTrees.init(optionalWasm, { - globalVariables: GlobalVariables.empty(), - }); - await newMerkleTrees.updateLatestGlobalVariablesHash(this.latestGlobalVariablesHash.get()); - return newMerkleTrees; - } - /** * Waits for all jobs to finish before executing the given function. * @param fn - The function to execute. From 204386f21ceda1c89670b7936ed9d69fdf8a95c4 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 29 Aug 2023 15:44:14 +0000 Subject: [PATCH 11/13] docs: added comment pointing to the created issue --- yarn-project/aztec-node/src/aztec-node/server.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index 0d5bbf69c9dd..34143dd85480 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -390,6 +390,8 @@ export class AztecNodeService implements AztecNode { const prevGlobalVariables = (await this.blockSource.getL2Block(-1))?.globalVariables ?? GlobalVariables.empty(); // Instantiate merkle trees so uncommited updates by this simulation are local to it. + // TODO we should be able to remove this after https://github.com/AztecProtocol/aztec-packages/issues/1869 + // So simulation of public functions doesn't affect the merkle trees. const merkleTrees = new MerkleTrees(this.merkleTreesDb, this.log); await merkleTrees.init(await CircuitsWasm.get(), { globalVariables: prevGlobalVariables, From 98f26903d9412f5a019895ff852b3fb6ff8fc9ef Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 30 Aug 2023 08:15:54 +0000 Subject: [PATCH 12/13] refactor: improvements after peer review --- .../src/client/private_execution.ts | 9 +-- .../src/client/unconstrained_execution.ts | 18 +---- .../acir-simulator/src/public/executor.ts | 9 +-- .../src/aztec-node/http-node.test.ts | 6 +- .../aztec-node/src/aztec-node/http-node.ts | 2 +- .../aztec-node/src/aztec-node/server.ts | 2 +- .../src/aztec_rpc_server/aztec_rpc_server.ts | 68 +++++++++------- yarn-project/rollup-provider/src/app.ts | 3 +- .../src/sequencer/public_processor.ts | 2 +- .../types/src/interfaces/aztec-node.ts | 2 +- yarn-project/types/src/simulation_error.ts | 81 +++++++++++++++++++ 11 files changed, 134 insertions(+), 68 deletions(-) diff --git a/yarn-project/acir-simulator/src/client/private_execution.ts b/yarn-project/acir-simulator/src/client/private_execution.ts index d9cec52106cb..ef002781d71c 100644 --- a/yarn-project/acir-simulator/src/client/private_execution.ts +++ b/yarn-project/acir-simulator/src/client/private_execution.ts @@ -15,7 +15,6 @@ import { FunctionL2Logs, NotePreimage, NoteSpendingInfo, SimulationError } from import { extractPrivateCircuitPublicInputs, frToAztecAddress } from '../acvm/deserialize.js'; import { - ACVMError, ZERO_ACVM_FIELD, acvm, convertACVMFieldToBuffer, @@ -199,13 +198,7 @@ export class PrivateFunctionExecution { }, this.abi.debug, ).catch((err: Error) => { - const failingFunction = { contractAddress: this.contractAddress, functionSelector: selector }; - if (err instanceof SimulationError) { - throw SimulationError.extendPreviousSimulationError(failingFunction, err); - } - throw new SimulationError(err.message, failingFunction, err instanceof ACVMError ? err.callStack : undefined, { - cause: err, - }); + throw SimulationError.fromError(this.contractAddress, selector, err); }); const publicInputs = extractPrivateCircuitPublicInputs(partialWitness, acir); diff --git a/yarn-project/acir-simulator/src/client/unconstrained_execution.ts b/yarn-project/acir-simulator/src/client/unconstrained_execution.ts index 081d8f14c4dd..2bba8bea4034 100644 --- a/yarn-project/acir-simulator/src/client/unconstrained_execution.ts +++ b/yarn-project/acir-simulator/src/client/unconstrained_execution.ts @@ -6,15 +6,7 @@ import { createDebugLogger } from '@aztec/foundation/log'; import { AztecNode, SimulationError } from '@aztec/types'; import { extractReturnWitness, frToAztecAddress } from '../acvm/deserialize.js'; -import { - ACVMError, - ACVMField, - ZERO_ACVM_FIELD, - acvm, - fromACVMField, - toACVMField, - toACVMWitness, -} from '../acvm/index.js'; +import { ACVMField, ZERO_ACVM_FIELD, acvm, fromACVMField, toACVMField, toACVMWitness } from '../acvm/index.js'; import { AcirSimulator } from '../index.js'; import { ClientTxExecutionContext } from './client_execution_context.js'; import { FunctionAbiWithDebugMetadata } from './db_oracle.js'; @@ -112,13 +104,7 @@ export class UnconstrainedFunctionExecution { }, this.abi.debug, ).catch((err: Error) => { - const failingFunction = { contractAddress: this.contractAddress, functionSelector: this.functionData.selector }; - if (err instanceof SimulationError) { - throw SimulationError.extendPreviousSimulationError(failingFunction, err); - } - throw new SimulationError(err.message, failingFunction, err instanceof ACVMError ? err.callStack : undefined, { - cause: err, - }); + throw SimulationError.fromError(this.contractAddress, this.functionData.selector, err); }); const returnValues: ACVMField[] = extractReturnWitness(acir, partialWitness); diff --git a/yarn-project/acir-simulator/src/public/executor.ts b/yarn-project/acir-simulator/src/public/executor.ts index 152533c99ad9..5ed691cef2a9 100644 --- a/yarn-project/acir-simulator/src/public/executor.ts +++ b/yarn-project/acir-simulator/src/public/executor.ts @@ -13,7 +13,6 @@ import { createDebugLogger } from '@aztec/foundation/log'; import { FunctionL2Logs, SimulationError } from '@aztec/types'; import { - ACVMError, ZERO_ACVM_FIELD, acvm, convertACVMFieldToBuffer, @@ -141,13 +140,7 @@ export class PublicExecutor { return Promise.resolve(toACVMField(portalContactAddress)); }, }).catch((err: Error) => { - const failingFunction = { contractAddress: execution.contractAddress, functionSelector: selector }; - if (err instanceof SimulationError) { - throw SimulationError.extendPreviousSimulationError(failingFunction, err); - } - throw new SimulationError(err.message, failingFunction, err instanceof ACVMError ? err.callStack : undefined, { - cause: err, - }); + throw SimulationError.fromError(execution.contractAddress, selector, err); }); const { diff --git a/yarn-project/aztec-node/src/aztec-node/http-node.test.ts b/yarn-project/aztec-node/src/aztec-node/http-node.test.ts index f022f9b08f15..8b82fb7f364f 100644 --- a/yarn-project/aztec-node/src/aztec-node/http-node.test.ts +++ b/yarn-project/aztec-node/src/aztec-node/http-node.test.ts @@ -484,13 +484,13 @@ describe('HttpNode', () => { }); }); - describe('simulatePublicPart', () => { + describe('simulatePublicCalls', () => { it('should fetch a successful simulation response', async () => { const tx = mockTx(); const response = {}; setFetchMock(response); - await httpNode.simulatePublicPart(tx); + await httpNode.simulatePublicCalls(tx); const init: RequestInit = { method: 'POST', @@ -512,7 +512,7 @@ describe('HttpNode', () => { }; setFetchMock(response); - await expect(httpNode.simulatePublicPart(tx)).rejects.toThrow(simulationError); + await expect(httpNode.simulatePublicCalls(tx)).rejects.toThrow(simulationError); }); }); }); diff --git a/yarn-project/aztec-node/src/aztec-node/http-node.ts b/yarn-project/aztec-node/src/aztec-node/http-node.ts index e7367075fc16..f1da8adf8705 100644 --- a/yarn-project/aztec-node/src/aztec-node/http-node.ts +++ b/yarn-project/aztec-node/src/aztec-node/http-node.ts @@ -376,7 +376,7 @@ export class HttpNode implements AztecNode { * Simulates the public part of a transaction with the current state. * @param tx - The transaction to simulate. **/ - public async simulatePublicPart(tx: Tx) { + public async simulatePublicCalls(tx: Tx) { const url = new URL(`${this.baseUrl}/simulate-tx`); const init: RequestInit = {}; init['method'] = 'POST'; diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index 34143dd85480..373c3a1447e1 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -383,7 +383,7 @@ export class AztecNodeService implements AztecNode { * Simulates the public part of a transaction with the current state. * @param tx - The transaction to simulate. **/ - public async simulatePublicPart(tx: Tx) { + public async simulatePublicCalls(tx: Tx) { this.log.info(`Simulating tx ${await tx.getTxHash()}`); const blockNumber = (await this.blockSource.getBlockNumber()) + 1; const newGlobalVariables = await this.globalVariableBuilder.buildGlobalVariables(new Fr(blockNumber)); diff --git a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts index 924c7cf9d769..ba6dab44acc1 100644 --- a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts +++ b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts @@ -192,7 +192,7 @@ export class AztecRPCServer implements AztecRPC { const newContract = deployedContractAddress ? await this.db.getContract(deployedContractAddress) : undefined; const tx = await this.#simulateAndProve(txRequest, newContract); - if (simulatePublic) await this.#simulatePublicPart(tx); + if (simulatePublic) await this.#simulatePublicCalls(tx); this.log.info(`Executed local simulation for ${await tx.getTxHash()}`); return tx; @@ -335,7 +335,8 @@ export class AztecRPCServer implements AztecRPC { return result; } catch (err) { if (err instanceof SimulationError) { - await this.#debugLogSimulationError(err); + await this.#enrichSimulationError(err); + this.log(err.toString()); } throw err; } @@ -375,7 +376,8 @@ export class AztecRPCServer implements AztecRPC { return result; } catch (err) { if (err instanceof SimulationError) { - await this.#debugLogSimulationError(err); + await this.#enrichSimulationError(err); + this.log(err.toString()); } throw err; } @@ -387,9 +389,9 @@ export class AztecRPCServer implements AztecRPC { * It can also be used for estimating gas in the future. * @param tx - The transaction to be simulated. */ - async #simulatePublicPart(tx: Tx) { + async #simulatePublicCalls(tx: Tx) { try { - await this.node.simulatePublicPart(tx); + await this.node.simulatePublicCalls(tx); } catch (err) { // Try to fill in the noir call stack since the RPC server may have access to the debug metadata if (err instanceof SimulationError) { @@ -411,7 +413,8 @@ export class AztecRPCServer implements AztecRPC { ); } } - await this.#debugLogSimulationError(err); + await this.#enrichSimulationError(err); + this.log(err.toString()); } throw err; @@ -463,28 +466,37 @@ export class AztecRPCServer implements AztecRPC { ); } - async #debugLogSimulationError(err: SimulationError) { - const functionCallStack = err.getCallStack(); - const noirCallStack = err.getNoirCallStack(); - - // Try to resolve the contract and function names of the stack of failing functions. - const stackLines: string[] = [ - ...(await Promise.all( - functionCallStack.map(async ({ contractAddress, functionSelector }) => { - const contract = await this.db.getContract(contractAddress); - const functionAbi = contract?.functions.find(f => f.selector.equals(functionSelector)); - return ` at ${contract?.name ?? contractAddress.toString()}.${ - functionAbi?.name ?? functionSelector.toString() - }`; - }), - )), - ...noirCallStack.map( - sourceCodeLocation => - ` at ${sourceCodeLocation.filePath}:${sourceCodeLocation.line} '${sourceCodeLocation.locationText}'`, - ), - ]; - - this.log([`Simulation error: ${err.message}`, ...stackLines.reverse()].join('\n')); + /** + * Adds contract and function names to a simulation error. + * @param err - The error to enrich. + */ + async #enrichSimulationError(err: SimulationError) { + // Maps contract addresses to the set of functions selectors that were in error. + // Using strings because map and set doesn't use .equals() + const mentionedFunctions: Map> = new Map(); + + err.getCallStack().forEach(({ contractAddress, functionSelector }) => { + if (!mentionedFunctions.has(contractAddress.toString())) { + mentionedFunctions.set(contractAddress.toString(), new Set()); + } + mentionedFunctions.get(contractAddress.toString())!.add(functionSelector.toString()); + }); + + await Promise.all( + Object.keys(mentionedFunctions).map(async contractAddress => { + const selectors = mentionedFunctions.get(contractAddress)!; + const contract = await this.db.getContract(AztecAddress.fromString(contractAddress)); + if (contract) { + err.enrichWithContractName(contract.address, contract.name); + selectors.forEach(selector => { + const functionAbi = contract.functions.find(f => f.selector.toString() === selector); + if (functionAbi) { + err.enrichWithFunctionName(contract.address, functionAbi.selector, functionAbi.name); + } + }); + } + }), + ); } // HACK(#1639): this is a hack to fix ordering of public calls enqueued in the call stack. Since the private kernel diff --git a/yarn-project/rollup-provider/src/app.ts b/yarn-project/rollup-provider/src/app.ts index 1f7f20399190..e56ce78abcfb 100644 --- a/yarn-project/rollup-provider/src/app.ts +++ b/yarn-project/rollup-provider/src/app.ts @@ -274,12 +274,13 @@ export function appFactory(node: AztecNode, prefix: string) { const postData = (await stream.readAll()) as Buffer; const tx = Tx.fromBuffer(postData); try { - await node.simulatePublicPart(tx); + await node.simulatePublicCalls(tx); } catch (err) { if (err instanceof SimulationError) { ctx.body = { simulationError: err.toJSON(), }; + ctx.status = 400; } else { throw err; } diff --git a/yarn-project/sequencer-client/src/sequencer/public_processor.ts b/yarn-project/sequencer-client/src/sequencer/public_processor.ts index 6ba99647bb4e..8b00cfa592cc 100644 --- a/yarn-project/sequencer-client/src/sequencer/public_processor.ts +++ b/yarn-project/sequencer-client/src/sequencer/public_processor.ts @@ -117,7 +117,7 @@ export class PublicProcessor { try { result.push(await this.processTx(tx)); } catch (err) { - this.log.error(`Error processing tx ${await tx.getTxHash()}: ${err}`); + this.log.warn(`Error processing tx ${await tx.getTxHash()}: ${err}`); failed.push({ tx, error: err instanceof Error ? err : new Error('Unknown error'), diff --git a/yarn-project/types/src/interfaces/aztec-node.ts b/yarn-project/types/src/interfaces/aztec-node.ts index ac192ede1732..af20817faf32 100644 --- a/yarn-project/types/src/interfaces/aztec-node.ts +++ b/yarn-project/types/src/interfaces/aztec-node.ts @@ -144,5 +144,5 @@ export interface AztecNode extends DataCommitmentProvider, L1ToL2MessageProvider * This currently just checks that the transaction execution succeeds. * @param tx - The transaction to simulate. **/ - simulatePublicPart(tx: Tx): Promise; + simulatePublicCalls(tx: Tx): Promise; } diff --git a/yarn-project/types/src/simulation_error.ts b/yarn-project/types/src/simulation_error.ts index 9eece2c769f7..9477e758984f 100644 --- a/yarn-project/types/src/simulation_error.ts +++ b/yarn-project/types/src/simulation_error.ts @@ -8,10 +8,18 @@ export interface FailingFunction { * The address of the contract that failed. */ contractAddress: AztecAddress; + /** + * The name of the contract that failed. + */ + contractName?: string; /** * The selector of the function that failed. */ functionSelector: FunctionSelector; + /** + * The name of the function that failed. + */ + functionName?: string; } /** @@ -62,11 +70,84 @@ export class SimulationError extends Error { this.functionErrorStack.unshift(failingFunction); } + static fromError( + failingContract: AztecAddress, + failingselector: FunctionSelector, + err: Error & { + /** + * The noir call stack. + */ + callStack?: NoirCallStack; + }, + ) { + const failingFunction = { contractAddress: failingContract, functionSelector: failingselector }; + if (err instanceof SimulationError) { + return SimulationError.extendPreviousSimulationError(failingFunction, err); + } + return new SimulationError(err.message, failingFunction, err?.callStack, { + cause: err, + }); + } + static extendPreviousSimulationError(failingFunction: FailingFunction, previousError: SimulationError) { previousError.addCaller(failingFunction); return previousError; } + /** + * Enriches the error with the name of a contract that failed. + * @param contractAddress - The address of the contract + * @param contractName - The corresponding name + */ + enrichWithContractName(contractAddress: AztecAddress, contractName: string) { + this.functionErrorStack.forEach(failingFunction => { + if (failingFunction.contractAddress.equals(contractAddress)) { + failingFunction.contractName = contractName; + } + }); + } + + /** + * Enriches the error with the name of a function that failed. + * @param contractAddress - The address of the contract + * @param functionSelector - The selector of the function + * @param functionName - The corresponding name + */ + enrichWithFunctionName(contractAddress: AztecAddress, functionSelector: FunctionSelector, functionName: string) { + this.functionErrorStack.forEach(failingFunction => { + if ( + failingFunction.contractAddress.equals(contractAddress) && + failingFunction.functionSelector.equals(functionSelector) + ) { + failingFunction.functionName = functionName; + } + }); + } + + /** + * Returns a string representation of the error. + * @returns The string. + */ + toString() { + const functionCallStack = this.getCallStack(); + const noirCallStack = this.getNoirCallStack(); + + // Try to resolve the contract and function names of the stack of failing functions. + const stackLines: string[] = [ + ...functionCallStack.map(failingFunction => { + return ` at ${failingFunction.contractName ?? failingFunction.contractAddress.toString()}.${ + failingFunction.functionName ?? failingFunction.functionSelector.toString() + }`; + }), + ...noirCallStack.map( + sourceCodeLocation => + ` at ${sourceCodeLocation.filePath}:${sourceCodeLocation.line} '${sourceCodeLocation.locationText}'`, + ), + ]; + + return [`Simulation error: ${this.message}`, ...stackLines.reverse()].join('\n'); + } + /** * Updates the error message. This is needed because in some engines the stack also contains the message. * @param newMessage - The new message of this error. From e0db98362452736124a045f0d6b12d4b49291a76 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 30 Aug 2023 09:02:22 +0000 Subject: [PATCH 13/13] fix: improve enrichment --- yarn-project/acir-simulator/src/acvm/acvm.ts | 2 +- .../src/aztec_rpc_server/aztec_rpc_server.ts | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/yarn-project/acir-simulator/src/acvm/acvm.ts b/yarn-project/acir-simulator/src/acvm/acvm.ts index 7be30ea0e821..adcbdb736a65 100644 --- a/yarn-project/acir-simulator/src/acvm/acvm.ts +++ b/yarn-project/acir-simulator/src/acvm/acvm.ts @@ -170,7 +170,7 @@ export async function acvm( typedError = new Error(`Error in oracle callback ${err}`); } oracleError = typedError; - logger.error(`Error in oracle callback ${name}:`, typedError); + logger.error(`Error in oracle callback ${name}:`, typedError.message, typedError.stack); throw typedError; } }, diff --git a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts index 12155a93e9f2..6cdeba9a74b4 100644 --- a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts +++ b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts @@ -474,7 +474,7 @@ export class AztecRPCServer implements AztecRPC { */ async #enrichSimulationError(err: SimulationError) { // Maps contract addresses to the set of functions selectors that were in error. - // Using strings because map and set doesn't use .equals() + // Using strings because map and set don't use .equals() const mentionedFunctions: Map> = new Map(); err.getCallStack().forEach(({ contractAddress, functionSelector }) => { @@ -485,15 +485,15 @@ export class AztecRPCServer implements AztecRPC { }); await Promise.all( - Object.keys(mentionedFunctions).map(async contractAddress => { - const selectors = mentionedFunctions.get(contractAddress)!; - const contract = await this.db.getContract(AztecAddress.fromString(contractAddress)); + [...mentionedFunctions.entries()].map(async ([contractAddress, selectors]) => { + const parsedContractAddress = AztecAddress.fromString(contractAddress); + const contract = await this.db.getContract(parsedContractAddress); if (contract) { - err.enrichWithContractName(contract.address, contract.name); + err.enrichWithContractName(parsedContractAddress, contract.name); selectors.forEach(selector => { const functionAbi = contract.functions.find(f => f.selector.toString() === selector); if (functionAbi) { - err.enrichWithFunctionName(contract.address, functionAbi.selector, functionAbi.name); + err.enrichWithFunctionName(parsedContractAddress, functionAbi.selector, functionAbi.name); } }); }