fix: Fix block constraint key divergence bug. (#3256)

* Fixes the "bad witness key divergence bug" in RAM block constraint by
creating new witness and asserting it's equal to the original witness.
* Adds a new `prove_then_verify` test flow, which is used to always
exercise the good/bad witness data paths to ensure the resulting keys
are the same. This is faster than `all_cmds` which isn't needed for
every test. It's slower than `prove_and_verify` which uses the single
built in command, but that's less safe as it only exercised the bad
witness path for circuit construction.
* Limit hardware concurrency to 16 cores as performance can actually be
worse with higher core counts.

barretenberg/acir_tests/Dockerfile.bb

@@ -1,11 +1,12 @@
 FROM 278380418400.dkr.ecr.eu-west-2.amazonaws.com/barretenberg-x86_64-linux-clang-assert
 
 FROM node:18-alpine
-RUN apk update && apk add git bash curl jq
+RUN apk update && apk add git bash curl jq coreutils
 COPY --from=0 /usr/src/barretenberg/cpp/build /usr/src/barretenberg/cpp/build
 WORKDIR /usr/src/barretenberg/acir_tests
 COPY . .
-# Run every acir test through native bb build "prove_and_verify".
-RUN FLOW=all_cmds ./run_acir_tests.sh
+# Run every acir test through native bb build prove_then_verify flow.
+# This ensures we test independent pk construction through real/garbage witness data paths.
+RUN FLOW=prove_then_verify ./run_acir_tests.sh
 # Run 1_mul through native bb build, all_cmds flow, to test all cli args.
 RUN VERBOSE=1 FLOW=all_cmds ./run_acir_tests.sh 1_mul

barretenberg/acir_tests/Dockerfile.bb.js

@@ -11,7 +11,7 @@ RUN (cd browser-test-app && yarn && yarn build) && (cd headless-test && yarn &&
 COPY . .
 ENV VERBOSE=1
 # Run double_verify_proof through bb.js on node to check 512k support.
-RUN BIN=../ts/dest/node/main.js ./run_acir_tests.sh double_verify_proof
+RUN BIN=../ts/dest/node/main.js FLOW=prove_then_verify ./run_acir_tests.sh double_verify_proof
 # Run 1_mul through bb.js build, all_cmds flow, to test all cli args.
 RUN BIN=../ts/dest/node/main.js FLOW=all_cmds ./run_acir_tests.sh 1_mul
 # Run double_verify_proof through bb.js on chrome testing multi-threaded browser support.

barretenberg/acir_tests/browser-test-app/src/index.ts

@@ -56,6 +56,8 @@ function base64ToUint8Array(base64: string) {
 }
 
 // This is the 1_mul test, for triggering via the button click.
+// Will likely rot as acir changes.
+// Update by extracting from ../acir_tests/1_mul/target/* as needed.
 const acir = inflate(
   base64ToUint8Array(
     "H4sIAAAAAAAA/+2W3W6CQBCFB7AqVak/VdM0TXmAXuzyo3BXH6Wm+P6P0G5cZCS2N5whkrCJmWUjh505zPKFRPRB5+H8/lwbQ3bt1q49e82HY+OnjbHaJUmxjwod6y8V5ccsVUl63GU602mWfkdZHBdZku3zY75XuU7iQp/SPD6p8xg014qslvbY/v7bs2o29ACnpfh+H9h8YKPL1jwbhwI5Ue059ToGN9agD5cw6UFAd0i4l18q7yHeI8UkRWuqGg6PqkaR2Gt5OErWF6StBbUvz+C1GNk4Zmu+jeUHxowh86b0yry3B3afw6LDNA7snlv/cf7Q8dlaeX9AMr0icEAr0QO4fKmNgSFVBDCmigCkGglNFC8k05QeZp8XWhkBcx4DfZGqH9pnH+hFW+To47SuyPGRzXtybKjp24KidSd03+Ro8p7gPRIlxwlwn9LkaA7psXB9Qdqtk+PUxhlb68kRo9kKORoDQ6rIcUZy5Fg2EpooXkmmKdHkOAXmPAP6IlU/tM8BdY8cA5Ihxyc278mxoWZgC4rWndN9k6PJe473SJQc59QdcjSH9Ey4viDt1slxYeOSrfXkiNFshRyNgSFV5LgkOXIsGwlNFG8k05RoclwAc14CfZGqH9rnFXWPHFckQ47PbN6TY0PNlS0oWndN902OJu813iNRclxTd8jRHNJL4fqCtFsnx42NW7bWkyNGsxVyNAaGVJHjluTIsWwkNFG8k0xToslxA8x5C/RFqn4u2GcPmDOwfoofTi5df4zq4we8wQQCRCoAAA=="

barretenberg/acir_tests/flows/all_cmds.sh

@@ -1,12 +1,7 @@
 #!/bin/sh
 set -eu
 
-if [ -n "${VERBOSE:-}" ]; then
-  VFLAG="-v"
-else
-  VFLAG=""
-fi
-
+VFLAG=${VERBOSE:+-v}
 BFLAG="-b ./target/acir.gz"
 FLAGS="-c $CRS_PATH $VFLAG"
 

barretenberg/acir_tests/flows/prove_and_verify.sh

@@ -1,8 +1,8 @@
 #!/bin/sh
 set -eu
 
-if [ -n "$VERBOSE" ]; then
-  $BIN prove_and_verify -v -c $CRS_PATH -b ./target/acir.gz
-else
-  $BIN prove_and_verify -c $CRS_PATH -b ./target/acir.gz
-fi
+VFLAG=${VERBOSE:+-v}
+
+# This is the fastest flow, because it only generates pk/vk once, gate count once, etc.
+# It may not catch all class of bugs.
+$BIN prove_and_verify $VFLAG -c $CRS_PATH -b ./target/acir.gz

barretenberg/acir_tests/flows/prove_then_verify.sh

@@ -0,0 +1,12 @@
+#!/bin/sh
+set -eu
+
+VFLAG=${VERBOSE:+-v}
+BFLAG="-b ./target/acir.gz"
+FLAGS="-c $CRS_PATH $VFLAG"
+
+# Test we can perform the proof/verify flow.
+# This ensures we test independent pk construction through real/garbage witness data paths.
+$BIN prove -o proof $FLAGS $BFLAG
+$BIN write_vk -o vk $FLAGS $BFLAG
+$BIN verify -k vk -p proof $FLAGS

barretenberg/acir_tests/run_acir_tests.sh

@@ -16,6 +16,8 @@ CRS_PATH=~/.bb-crs
 BRANCH=master
 VERBOSE=${VERBOSE:-}
 TEST_NAMES=("$@")
+# We get little performance benefit over 16 cores (in fact it can be worse).
+HARDWARE_CONCURRENCY=${HARDWARE_CONCURRENCY:-16}
 
 FLOW_SCRIPT=$(realpath ./flows/${FLOW}.sh)
 

barretenberg/cpp/src/barretenberg/bb/main.cpp

@@ -117,7 +117,6 @@ void prove(const std::string& bytecodePath,
     auto constraint_system = get_constraint_system(bytecodePath);
     auto witness = get_witness(witnessPath);
     auto acir_composer = init(constraint_system);
-    acir_composer.init_proving_key(constraint_system);
     auto proof = acir_composer.create_proof(constraint_system, witness, recursive);
 
     if (outputPath == "-") {

barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp

@@ -54,9 +54,13 @@ void create_block_constraints(Builder& builder, const BlockConstraint constraint
         for (auto& op : constraint.trace) {
             field_ct value = poly_to_field_ct(op.value, builder);
             field_ct index = poly_to_field_ct(op.index, builder);
-            if (has_valid_witness_assignments == false) {
-                index = field_ct::from_witness(&builder, 0);
-            }
+
+            // We create a new witness w to avoid issues with non-valid witness assignements.
+            // If witness are not assigned, then index will be zero and table[index] won't hit bounds check.
+            fr index_value = has_valid_witness_assignments ? index.get_value() : 0;
+            // Create new witness and ensure equal to index.
+            field_ct::from_witness(&builder, index_value).assert_equal(index);
+
             if (op.access_type == 0) {
                 value.assert_equal(table.read(index));
             } else {

barretenberg/ts/src/main.ts

@@ -124,7 +124,6 @@ export async function prove(
     debug(`creating proof...`);
     const bytecode = getBytecode(bytecodePath);
     const witness = getWitness(witnessPath);
-    await api.acirInitProvingKey(acirComposer, bytecode);
     const proof = await api.acirCreateProof(acirComposer, bytecode, witness, isRecursive);
     debug(`done.`);