-
Notifications
You must be signed in to change notification settings - Fork 298
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Initial trazability of ACIR #1701
Changes from 10 commits
102e661
9095bc6
0ef69c6
66c2eba
3740fd0
5b7483b
95315ae
dc71b3a
de0906d
3a1f442
d18cc8c
2255584
2cfafbe
c567ea5
0b963a6
3915334
7070367
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,8 @@ import { | |
executeCircuitWithBlackBoxSolver, | ||
} from 'acvm_js'; | ||
|
||
import { FunctionDebugMetadata } from '../index.js'; | ||
|
||
/** | ||
* The format for fields on the ACVM. | ||
*/ | ||
|
@@ -71,6 +73,74 @@ export interface ACIRExecutionResult { | |
partialWitness: ACVMWitness; | ||
} | ||
|
||
/** | ||
* Extracts the opcode location from an ACVM error string. | ||
*/ | ||
function extractOpcodeLocationFromError(err: string): string | undefined { | ||
const match = err.match(/^Cannot satisfy constraint (?<opcodeLocation>[0-9]+(?:\.[0-9]+)?)/); | ||
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[] { | ||
const { debugSymbols, files } = debug; | ||
|
||
const callStack = debugSymbols.locations[opcodeLocation] || []; | ||
return callStack.map(call => { | ||
const { file: fileId, span } = call; | ||
|
||
const { path, source } = files[fileId]; | ||
|
||
const assertionText = 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, | ||
}; | ||
}); | ||
} | ||
|
||
/** | ||
* Creates a formatted string for an error stack | ||
* @param callStack - The error stack | ||
* @returns - The formatted string | ||
*/ | ||
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'); | ||
} | ||
|
||
/** | ||
* The function call that executes an ACIR. | ||
*/ | ||
|
@@ -79,6 +149,7 @@ export async function acvm( | |
acir: Buffer, | ||
initialWitness: ACVMWitness, | ||
callback: ACIRCallback, | ||
debug?: FunctionDebugMetadata, | ||
): Promise<ACIRExecutionResult> { | ||
const logger = createDebugLogger('aztec:simulator:acvm'); | ||
const partialWitness = await executeCircuitWithBlackBoxSolver( | ||
|
@@ -100,7 +171,25 @@ export async function acvm( | |
throw err; | ||
} | ||
}, | ||
); | ||
).catch(err => { | ||
// ACVM_js throws raw string errors | ||
if (typeof err !== 'string') { | ||
throw err; | ||
} | ||
|
||
const opcodeLocation = extractOpcodeLocationFromError(err); | ||
if (!opcodeLocation || !debug) { | ||
throw err; | ||
} | ||
|
||
const callStack = getCallStackFromOpcodeLocation(opcodeLocation, debug); | ||
logger(printErrorStack(callStack)); | ||
|
||
// The ACVM only lets string errors pass through so we need to throw a string at the execution level. | ||
// We should probably update the ACVM to let proper errors through. | ||
throw `Assertion failed: '${callStack.pop()?.assertionText ?? 'Unknown'}'`; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But the first There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem is that the acvm js only lets error strings through, so if we emitted new Error(str), if the execution of this ACVM was child of another ACVM, when passing through it'd get transformed to Error in oracle call: unknown or something like that ): Maybe we can ignore the ACVM error completely if there was an error in a nested call... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I might try that so we have proper errors from the simulator executions |
||
}); | ||
|
||
return Promise.resolve({ partialWitness }); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the dependency from 0.21 + init pedersen to 0.22 + init pedersen. Init pedersen will eventually be released as part of 0.23, let's hope that lands soon!