Conversation
| // blinded rows | ||
| std::array<FF, multivariate_n> skipping_disabler = { 0, 0, 0, 0, 0, 1, 1, 1 }; | ||
| full_polynomials.z_perm = bb::Polynomial<FF>(z_perm); | ||
| full_polynomials.lookup_inverses = bb::Polynomial<FF>(lookup_inverses); |
There was a problem hiding this comment.
the current skipping condition in ecc_op_queue_relation conflicts with the masking mechanism. for now, I un-masked the ecc op wire, but we'll have to find a way to make it more robust (when merge protocol is completed)
|
@dbanks12 @fcarreiro As I understand it, this is a log standing bug originally identified by Facundo. Basically it was allowing the verifier to utilize skipping in relations, which is never valid. Resolving it initially led to a failure in some of our internal tests (fixes for this have gone in separately) but it looks like maybe it's triggering a real failure in the AVM? FYI The failure we had was due to a skip condition in a particular relation that was virtually always true, thus the verifier was not identifying an invalid witness. Once we corrected the skip condition, the tests passed with skipping correctly turned OFF for the verifier. Would you guys mind taking a look at the failures on this PR to evaluate? |
I'll take a look tomorrow! It might be that one of our skippable conditions was not right, but was not failing verification due to the verifier skipping that :) |
Should be fixed once #12099 is merged. I tried it on top of this branch. |
There used to be a long standing BB bug where the verifier was (wrongly) skipping. The branch with the fix (#10598) fails with an AVM2 test. I tracked it down to the skippable condition in sha256. I'm disabling it and also updating the lookups skippable to a simpler condition that Kesha had suggested.
* master: (264 commits) chore(p2p): log if rate limit was peer or global (#12116) chore: @aztec/stdlib pt1 -> cleanup circuits js (#12039) chore(tests): shorten block times in e2e p2p tests (#12073) fix: darwin properly erroring (#12113) chore: add missing import (#12111) fix: yarn remake-constants (#12109) chore: fix error in oracle definition (#12090) fix: Don't consider skipping (#10598) fix: Use gas billed in block header building (#12101) fix(avm): disable wrong sha skippable (#12099) chore: Provide defaults for bb and acvm in release image (#12105) fix(avm): break TS dependency cycle (#12103) feat: IVC gates command in WASM (#11792) fix: SharedMutable compilation warnings (#12098) git subrepo push --branch=master noir-projects/aztec-nr git_subrepo.sh: Fix parent in .gitrepo file. [skip ci] chore: replace relative paths to noir-protocol-circuits git subrepo push --branch=master barretenberg feat: Sync from noir (#12064) chore: Fix unbound CI variable on release image bootstrap (#12095) ...
* master: (264 commits) chore(p2p): log if rate limit was peer or global (#12116) chore: @aztec/stdlib pt1 -> cleanup circuits js (#12039) chore(tests): shorten block times in e2e p2p tests (#12073) fix: darwin properly erroring (#12113) chore: add missing import (#12111) fix: yarn remake-constants (#12109) chore: fix error in oracle definition (#12090) fix: Don't consider skipping (#10598) fix: Use gas billed in block header building (#12101) fix(avm): disable wrong sha skippable (#12099) chore: Provide defaults for bb and acvm in release image (#12105) fix(avm): break TS dependency cycle (#12103) feat: IVC gates command in WASM (#11792) fix: SharedMutable compilation warnings (#12098) git subrepo push --branch=master noir-projects/aztec-nr git_subrepo.sh: Fix parent in .gitrepo file. [skip ci] chore: replace relative paths to noir-protocol-circuits git subrepo push --branch=master barretenberg feat: Sync from noir (#12064) chore: Fix unbound CI variable on release image bootstrap (#12095) ...
There used to be a long standing BB bug where the verifier was (wrongly) skipping. The branch with the fix (AztecProtocol/aztec-packages#10598) fails with an AVM2 test. I tracked it down to the skippable condition in sha256. I'm disabling it and also updating the lookups skippable to a simpler condition that Kesha had suggested.
4 participants
The relation accumulation method
accumulate_relation_evaluations_without_skippingused by the verifier has been erroneously allowing the verifier to use the skip mechanism via/*consider_skipping=*/true.Fixing this uncovered an issue with the
EccOpQueueRelationskip condition which has been resolved.