feat: Initial trazability of ACIR #1701

sirasistant · 2023-08-21T15:33:41Z

Working towards #1224

This PR adds trazability of errors for ACIR opcodes when the debug data is available. Future work will enable this in brillig, and also will add assertion messages embedded in the bytecode itself.

Checklist:

Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge.

If the pull request requires a cryptography review (e.g. cryptographic algorithm implementations) I have added the 'crypto' tag.
I have reviewed my diff in github, line by line and removed unexpected formatting changes, testing logs, or commented-out code.
Every change is related to the PR description.
I have linked this pull request to relevant issues (if any exist).

sirasistant · 2023-08-21T15:34:41Z

yarn-project/acir-simulator/package.json

@@ -35,7 +35,7 @@
    "@aztec/circuits.js": "workspace:^",
    "@aztec/foundation": "workspace:^",
    "@aztec/types": "workspace:^",
-    "acvm_js": "github:noir-lang/acvm-js-wasm#db/init-sim-backend",
+    "acvm_js": "github:noir-lang/acvm-js-wasm#arv/0.22+init-pedersen",


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!

sirasistant · 2023-08-21T15:36:43Z

yarn-project/noir-compiler/src/contract-interface-gen/typescript.ts

@@ -145,7 +145,7 @@ function generateAbiGetter(name: string) {
 function generateAbiStatement(name: string, abiImportPath: string) {
  const stmts = [
    `import ${name}ContractAbiJson from '${abiImportPath}' assert { type: 'json' };`,
-    `export const ${name}ContractAbi = ${name}ContractAbiJson as ContractAbi;`,
+    `export const ${name}ContractAbi = ${name}ContractAbiJson as unknown as ContractAbi;`,


This is needed because having a Record<> inside breaks automatic inference of the type from the JSON

yarn-project/acir-simulator/src/client/private_execution.test.ts

sirasistant · 2023-08-21T16:13:45Z

Seems like the test was expecting the old format of error messages

yarn-project/acir-simulator/src/acvm/acvm.ts

LeilaWang · 2023-08-22T09:13:40Z

yarn-project/acir-simulator/src/acvm/acvm.ts

+
+    // 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'}'`;


But the first if throws the error when it's not a string. Should we convert that to a string or is it actually ok to throw new Error(str)?

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...

I might try that so we have proper errors from the simulator executions

LeilaWang

Amazing work!! So excited we can display better errors now 😃

yarn-project/end-to-end/src/e2e_private_token_contract.test.ts

yarn-project/aztec-rpc/src/contract_data_oracle/index.ts

* Attempt to fix production deployment (#1683) * Add stefan to mainframe. Deployment publishes bb.js and blockchain to npm. * Fix block scanning (#1692) * Attempt to fix deploy. * Fix dai-permit issues. Providers did not offset `v` properly (#1698) * fix: perform offset to `sig.v if 0 or 1 for `signTypedData` * fix: remove git-submodules * Force to -O2 with binaryen installed to prevent aes bug. * Updated fork block number (#1685) * Updated fork block number * Updated fork block and element test tranche * Removed no longer used env variable * Updated Faucet index page amounts * [zk-money] Jcf/aave (#1642) * Configure aave defi cards * copy-paste error * Increase test timeout Co-authored-by: PhilWindle <[email protected]> * fix aave token addresses (#1701) * Fuzzer-found bugs that are not related to bigfield (#1605) * Added a regression test to detect original parallelized asm bugs * FIxed parallelized SQR asm bugs * Added comment that Emerson wanted * Fix for create_range_constraint in the constant case * Various safe_uint bug fixes having to do with not handling constant cases * Fixed the comparison issue for the point at infinity * Added regression tests and fixed 3 more missing flags in optimized SQR * Fixed tabulation in asm_macros.hpp * Fixed Montgomery Issue that Adrian Found * Fixed issue with negative zero found by Guido * FIxed same self_neg bug found by Guido * Renamed 1 test * Fixed one more SQR bug * Added extra regression tests * Slightly moved one test * Added TODO comments into field_impl for potential optimisations * Addressed Zac's comments Co-authored-by: zac-williamson <[email protected]> * Fixing errors in addition and subtraction of field elements with moduli > 254 bits (#1702) * Added test * Added bugfixes * Added comments * Use 400k gas bridge for aave (#1705) * Add versioning to falafel status endpoint. Refresh zk-money on version mismatch. (#1674) * all get/post reqs from sdk to falafel now include header * falafel expects all endpoint requests to be given a 'version' header. SDK now gives all reqs a version. If falafel sees a req with version that does not match falafel version, it responds with error 402. ServerRollupProvider now emits versionMismatch whenever one occurs. zk-money promotes this event and forces refresh of browser. * cleanup new FALAFEL_VERSION constant in falafel - using configurator everywhere instead of using constant directly * fix prettier errors * prettier server.ts * sdk version to 0 * version is now a str like 2.1.0 in falafel and sdk. moved getRollupProviderStatus into SDK so that it doesn't need to accept sdk_version as arg * prettier sdk index.ts * zk-money alerts on version mismatch. Falafel allows requests without version header, but if version header is present it enforces a match. * server rollup provider and block source shouldn't NEED a to interact with falafel. * prettier rollup provider ts * block source / provider fix version default undefined changed to '' Co-authored-by: joss-aztec <[email protected]> Co-authored-by: Charlie Lye <[email protected]> Co-authored-by: Lasse Herskind <[email protected]> Co-authored-by: Innokentii Sennovskii <[email protected]> Co-authored-by: zac-williamson <[email protected]> Co-authored-by: David Banks <[email protected]>

sirasistant added 4 commits August 18, 2023 09:15

feat: initial version of trazability

102e661

Merge branch 'master' into arv/trazability

9095bc6

feat: first version of trazability for acir

0ef69c6

test: remove test assertion

66c2eba

sirasistant commented Aug 21, 2023

View reviewed changes

Update yarn-project/acir-simulator/src/client/private_execution.test.ts

3740fd0

sirasistant changed the title ~~WIP: Initial trazability of ACIR~~ feat: Initial trazability of ACIR Aug 21, 2023

sirasistant requested review from spalladino and LeilaWang August 21, 2023 15:56

sirasistant marked this pull request as ready for review August 21, 2023 16:13

sirasistant added 3 commits August 22, 2023 07:34

fix: increased json limit in body parser

5b7483b

style: fix styles

95315ae

Merge branch 'master' into arv/trazability

dc71b3a

sirasistant commented Aug 22, 2023

View reviewed changes

yarn-project/acir-simulator/src/acvm/acvm.ts Outdated Show resolved Hide resolved

sirasistant added 2 commits August 22, 2023 10:15

Update yarn-project/acir-simulator/src/acvm/acvm.ts

de0906d

Merge branch 'master' into arv/trazability

3a1f442

LeilaWang reviewed Aug 22, 2023

View reviewed changes

sirasistant added 2 commits August 22, 2023 10:14

fix: workaround acvm string throws

d18cc8c

Merge branch 'master' into arv/trazability

2255584

sirasistant requested a review from LeilaWang August 22, 2023 10:20

test: fix test that relied in old obscure error

2cfafbe

LeilaWang approved these changes Aug 22, 2023

View reviewed changes

yarn-project/end-to-end/src/e2e_private_token_contract.test.ts Outdated Show resolved Hide resolved

yarn-project/aztec-rpc/src/contract_data_oracle/index.ts Show resolved Hide resolved

sirasistant added 4 commits August 22, 2023 10:39

test: remove extraneous only

c567ea5

refactor: extract getting debug to foundation

0b963a6

Merge branch 'master' into arv/trazability

3915334

Merge branch 'master' into arv/trazability

7070367

sirasistant merged commit 89e4e1a into master Aug 22, 2023

sirasistant deleted the arv/trazability branch August 22, 2023 12:34

sirasistant mentioned this pull request Aug 25, 2023

Show failing constraints in private & unconstrained functions (done) #1812

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Initial trazability of ACIR #1701

feat: Initial trazability of ACIR #1701

sirasistant commented Aug 21, 2023 •

edited

Loading

sirasistant Aug 21, 2023

sirasistant Aug 21, 2023

sirasistant commented Aug 21, 2023

LeilaWang Aug 22, 2023

sirasistant Aug 22, 2023

sirasistant Aug 22, 2023

LeilaWang left a comment

feat: Initial trazability of ACIR #1701

feat: Initial trazability of ACIR #1701

Conversation

sirasistant commented Aug 21, 2023 • edited Loading

Checklist:

sirasistant Aug 21, 2023

Choose a reason for hiding this comment

sirasistant Aug 21, 2023

Choose a reason for hiding this comment

sirasistant commented Aug 21, 2023

LeilaWang Aug 22, 2023

Choose a reason for hiding this comment

sirasistant Aug 22, 2023

Choose a reason for hiding this comment

sirasistant Aug 22, 2023

Choose a reason for hiding this comment

LeilaWang left a comment

Choose a reason for hiding this comment

sirasistant commented Aug 21, 2023 •

edited

Loading