Skip to content
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
a509429
feat: first working version of simulate pub
sirasistant Aug 28, 2023
408c83c
feat: allow parsing nested public exec errors
sirasistant Aug 28, 2023
1ff4bd8
feat: sim errors with fn and noir call stacks
sirasistant Aug 29, 2023
3d14c40
Merge branch 'master' into arv/simulate_public
sirasistant Aug 29, 2023
8233a82
fix: cleanup
sirasistant Aug 29, 2023
7c09e7a
Merge branch 'arv/simulate_public' of github.com:AztecProtocol/aztec-…
sirasistant Aug 29, 2023
7eda8a4
test: fix test
sirasistant Aug 29, 2023
aba3018
fix: avoid simulations modifying state and txs
sirasistant Aug 29, 2023
4894747
fix: clone inside processor to avoid the mutation
sirasistant Aug 29, 2023
e706da1
Merge branch 'master' into arv/simulate_public
sirasistant Aug 29, 2023
213e465
refactor: improve printing of simulation errors
sirasistant Aug 29, 2023
7e9fad9
Merge branch 'arv/simulate_public' of github.com:AztecProtocol/aztec-…
sirasistant Aug 29, 2023
21e7a0b
feat: initial approach of transactional merkle db
sirasistant Aug 29, 2023
87b53b2
feat: option skip public sim & refactor trees txs
sirasistant Aug 29, 2023
ff46ce9
Merge branch 'master' into arv/simulate_public
sirasistant Aug 29, 2023
204386f
docs: added comment pointing to the created issue
sirasistant Aug 29, 2023
c90822a
Merge branch 'master' into arv/simulate_public
sirasistant Aug 29, 2023
98f2690
refactor: improvements after peer review
sirasistant Aug 30, 2023
c11a01f
Merge branch 'master' into arv/simulate_public
sirasistant Aug 30, 2023
e0db983
fix: improve enrichment
sirasistant Aug 30, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 44 additions & 43 deletions yarn-project/acir-simulator/src/acvm/acvm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
*/
assertionText: 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] || [];
Expand All @@ -111,30 +90,47 @@ 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;

return {
filePath: path,
line,
fileSource: source,
assertionText,
locationText,
};
});
}

/**
* Creates a formatted string for an error stack
* @param callStack - The error stack
* @returns - The formatted string
* 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.
*/
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}'`),
].join('\n');
export function processAcvmError(errMessage: string, debug: FunctionDebugMetadata): NoirCallStack | undefined {
const opcodeLocation = extractOpcodeLocationFromError(errMessage);
if (!opcodeLocation) {
return undefined;
}

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);
}
}

/**
Expand Down Expand Up @@ -174,22 +170,27 @@ 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;
}
},
).catch((acvmError: string) => {
).catch((acvmErrorString: string) => {
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(acvmErrorString, debug);

if (callStack) {
throw new ACVMError(
`Assertion failed: '${callStack[callStack.length - 1]?.locationText ?? 'Unknown'}'`,
callStack,
);
}
}
// If we cannot find a callstack, throw the original error.
throw new ACVMError(acvmErrorString);
});

return Promise.resolve({ partialWitness });
Expand Down
17 changes: 13 additions & 4 deletions yarn-project/acir-simulator/src/client/private_execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ 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 {
ACVMError,
ZERO_ACVM_FIELD,
acvm,
convertACVMFieldToBuffer,
Expand Down Expand Up @@ -55,8 +56,8 @@ export class PrivateFunctionExecution {
* @returns The execution result.
*/
public async run(): Promise<ExecutionResult> {
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();
Expand Down Expand Up @@ -197,7 +198,15 @@ 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,
});
});

const publicInputs = extractPrivateCircuitPublicInputs(partialWitness, acir);

Expand Down
26 changes: 20 additions & 6 deletions yarn-project/acir-simulator/src/client/unconstrained_execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -33,9 +41,7 @@ export class UnconstrainedFunctionExecution {
* @returns The return values of the executed function.
*/
public async run(aztecNode?: AztecNode): Promise<DecodedReturn> {
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);
Expand Down Expand Up @@ -105,7 +111,15 @@ 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,
});
});

const returnValues: ACVMField[] = extractReturnWitness(acir, partialWitness);

Expand Down
12 changes: 10 additions & 2 deletions yarn-project/acir-simulator/src/public/executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ 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 {
ACVMError,
ZERO_ACVM_FIELD,
acvm,
convertACVMFieldToBuffer,
Expand Down Expand Up @@ -67,7 +68,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)));
Expand Down Expand Up @@ -140,6 +140,14 @@ export class PublicExecutor {
(await this.contractsDb.getPortalContractAddress(contractAddress)) ?? EthAddress.ZERO;
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,
});
Comment thread
sirasistant marked this conversation as resolved.
Outdated
});

const {
Expand Down
35 changes: 34 additions & 1 deletion yarn-project/aztec-node/src/aztec-node/http-node.test.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -10,6 +10,7 @@ import {
LogType,
MerkleTreeId,
SiblingPath,
SimulationError,
TxHash,
mockTx,
} from '@aztec/types';
Expand Down Expand Up @@ -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 = new SimulationError('Failing function', {
contractAddress: AztecAddress.ZERO,
functionSelector: FunctionSelector.empty(),
});
const response = {
simulationError: simulationError.toJSON(),
};
setFetchMock(response);

await expect(httpNode.simulatePublicPart(tx)).rejects.toThrow(simulationError);
});
});
});
16 changes: 16 additions & 0 deletions yarn-project/aztec-node/src/aztec-node/http-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
LogType,
MerkleTreeId,
SiblingPath,
SimulationError,
Tx,
TxHash,
} from '@aztec/types';
Expand Down Expand Up @@ -370,4 +371,19 @@ 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) {
Comment thread
sirasistant marked this conversation as resolved.
Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Aside from the name, does it make sense to send a full tx object? Shouldn't we be just sending the enqueuedFunctionCalls, plus any side effects from private execution that may affect the public simulation (such as new contract deployment)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think it's better to send the whole TX for some reasons:

  • This method IMO will evolve to be the gas estimator of a TX. In ethereum, gas estimation takes TX objects https://www.quicknode.com/docs/ethereum/eth_estimateGas
  • This also runs the public kernel simulator, which I think makes this method need most of the fields in the TX
  • It just feels like a cleaner interface to me. Create your TX, call the node to see if it will succeed / to estimate fees on it!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fair point. And I know this is probably a bigger discussion, but how does this impact privacy? Should we enable the RPC server to run its own public simulations and gas estimations, by just requiring the state it needs from the node? Or would that leak information as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I believe it's ok, since you send to the node for simulation the same data you'd send the node for submission of the TX. I think in this case it's similar to ethereum, if you don't want the node to track which transaction came from which IP, you can run the node yourself and rely on the gossip p2p networking.

const url = new URL(`${this.baseUrl}/simulate-tx`);
const init: RequestInit = {};
init['method'] = 'POST';
init['body'] = tx.toBuffer();
const response = await (await fetch(url, init)).json();
if (response.simulationError) {
throw SimulationError.fromJSON(response.simulationError);
}
}
}
Loading