Skip to content
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

fix: Fix block constraint key divergence bug. #3256

Merged
merged 5 commits into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
7 changes: 4 additions & 3 deletions barretenberg/acir_tests/Dockerfile.bb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion barretenberg/acir_tests/Dockerfile.bb.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions barretenberg/acir_tests/browser-test-app/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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=="
Expand Down
7 changes: 1 addition & 6 deletions barretenberg/acir_tests/flows/all_cmds.sh
Original file line number Diff line number Diff line change
@@ -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"

Expand Down
10 changes: 5 additions & 5 deletions barretenberg/acir_tests/flows/prove_and_verify.sh
Original file line number Diff line number Diff line change
@@ -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
12 changes: 12 additions & 0 deletions barretenberg/acir_tests/flows/prove_then_verify.sh
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions barretenberg/acir_tests/run_acir_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,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)

Expand Down
1 change: 0 additions & 1 deletion barretenberg/cpp/src/barretenberg/bb/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,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);
Copy link
Member

Choose a reason for hiding this comment

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

After a master merge this will have to be removed in main.ts for bb.js

auto proof = acir_composer.create_proof(constraint_system, witness, recursive);

if (outputPath == "-") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down