Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 9 additions & 1 deletion yarn-project/circuit-types/src/simulation_error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ export function isNoirCallStackUnresolved(callStack: NoirCallStack): callStack i
* An error during the simulation of a function call.
*/
export class SimulationError extends Error {
private aztecContext: string = '';

constructor(
private originalMessage: string,
private functionErrorStack: FailingFunction[],
Expand Down Expand Up @@ -124,7 +126,9 @@ export class SimulationError extends Error {

getMessage() {
if (this.noirErrorStack && !isNoirCallStackUnresolved(this.noirErrorStack) && this.noirErrorStack.length) {
return `${this.originalMessage} '${this.noirErrorStack[this.noirErrorStack.length - 1].locationText}'`;
return `${this.originalMessage} '${this.noirErrorStack[this.noirErrorStack.length - 1].locationText}'${
this.aztecContext
}`;
}
return this.originalMessage;
}
Expand Down Expand Up @@ -212,6 +216,10 @@ export class SimulationError extends Error {
this.noirErrorStack = callStack;
}

setAztecContext(context: string) {
this.aztecContext = context;
}

toJSON() {
return {
originalMessage: this.originalMessage,
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/foundation/src/abi/event_selector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export class EventSelector extends Selector {
static fromString(selector: string) {
const buf = fromHex(selector);
if (buf.length !== Selector.SIZE) {
throw new Error(`Invalid Selector length ${buf.length} (expected ${Selector.SIZE}).`);
throw new Error(`Invalid EventSelector length ${buf.length} (expected ${Selector.SIZE}).`);
}
return EventSelector.fromBuffer(buf);
}
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/foundation/src/abi/function_selector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export class FunctionSelector extends Selector {
static fromString(selector: string) {
const buf = fromHex(selector);
if (buf.length !== Selector.SIZE) {
throw new Error(`Invalid Selector length ${buf.length} (expected ${Selector.SIZE}).`);
throw new Error(`Invalid FunctionSelector length ${buf.length} (expected ${Selector.SIZE}).`);
}
return FunctionSelector.fromBuffer(buf);
}
Expand Down
14 changes: 7 additions & 7 deletions yarn-project/pxe/src/pxe_service/error_enriching.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,26 @@ import { type ContractDataOracle, type PxeDatabase } from '../index.js';
* @param err - The error to enrich.
*/
export async function enrichSimulationError(err: SimulationError, db: PxeDatabase, logger: Logger) {
// Maps contract addresses to the set of functions selectors that were in error.
// Maps contract addresses to the set of function names that were in error.
// Map and Set do reference equality for their keys instead of value equality, so we store the string
// representation to get e.g. different contract address objects with the same address value to match.
const mentionedFunctions: Map<string, Set<string>> = new Map();

err.getCallStack().forEach(({ contractAddress, functionSelector }) => {
err.getCallStack().forEach(({ contractAddress, functionName }) => {
if (!mentionedFunctions.has(contractAddress.toString())) {
mentionedFunctions.set(contractAddress.toString(), new Set());
}
mentionedFunctions.get(contractAddress.toString())!.add(functionSelector?.toString() ?? '');
mentionedFunctions.get(contractAddress.toString())!.add(functionName?.toString() ?? '');
Copy link
Contributor

@benesjan benesjan Mar 4, 2025

Choose a reason for hiding this comment

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

I think this might have broke error enriching for private functions as I am now struggling to debug a test because I don't get a useful error message:
image

I have some questions:

  1. Is it know what was the crux of the original issue that this change needed to be done?
  2. @sirasistant is there some fundamental reasons why so many fields in FailingFunction are optional? This just seems to reduce the interface to the point of being useless. Also just defaulting on the line above to '' when functionName is undefined (or originally functionSelector) just seems weird. Why not just print a warning that the artifact has not been found for whatever reason? This is all so brittle.

});

await Promise.all(
[...mentionedFunctions.entries()].map(async ([contractAddress, selectors]) => {
[...mentionedFunctions.entries()].map(async ([contractAddress, fnNames]) => {
const parsedContractAddress = AztecAddress.fromString(contractAddress);
const contract = await db.getContract(parsedContractAddress);
if (contract) {
err.enrichWithContractName(parsedContractAddress, contract.name);
selectors.forEach(selector => {
const functionArtifact = contract.functions.find(f => FunctionSelector.fromString(selector).equals(f));
fnNames.forEach(fnName => {
const functionArtifact = contract.functions.find(f => fnName === f.name);
if (functionArtifact) {
err.enrichWithFunctionName(
parsedContractAddress,
Expand All @@ -39,7 +39,7 @@ export async function enrichSimulationError(err: SimulationError, db: PxeDatabas
);
} else {
logger.warn(
`Could not function artifact in contract ${contract.name} for selector ${selector} when enriching error callstack`,
`Could not function artifact in contract ${contract.name} for function '${fnName}' when enriching error callstack`,
);
}
});
Expand Down
13 changes: 9 additions & 4 deletions yarn-project/pxe/src/pxe_service/pxe_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1004,10 +1004,15 @@ export class PXEService implements PXE {
}

private contextualizeError(err: Error, ...context: string[]): Error {
this.log.error(err.name, err);
this.log.debug('Context:');
for (const c of context) {
this.log.debug(c);
let contextStr = '';
if (context.length > 0) {
contextStr = `\nContext:\n${context.join('\n')}`;
}
if (err instanceof SimulationError) {
err.setAztecContext(contextStr);
} else {
this.log.error(err.name, err);
this.log.debug(contextStr);
}
return err;
}
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/simulator/src/common/debug_fn_name.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ export async function getPublicFunctionDebugName(
calldata[0] !== undefined
? await db.getDebugFunctionName(contractAddress, FunctionSelector.fromField(calldata[0]))
: `<calldata[0] undefined> (Contract Address: ${contractAddress})`;
return `${targetFunction} (via dispatch)`;
return `${targetFunction}`;
}