diff --git a/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh b/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh index 4b823425c7b2..70acf0e9afc6 100755 --- a/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh +++ b/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh @@ -8,7 +8,7 @@ cd .. # IF A VK CHANGE IS EXPECTED - we need to redo this: # - Generate inputs: $root/yarn-project/end-to-end/bootstrap.sh generate_example_app_ivc_inputs # - Upload the compressed results: aws s3 cp bb-civc-inputs-[version].tar.gz s3://aztec-ci-artifacts/protocol/bb-civc-inputs-[version].tar.gz -pinned_civc_inputs_url="https://aztec-ci-artifacts.s3.us-east-2.amazonaws.com/protocol/bb-civc-inputs-v7.tar.gz" +pinned_civc_inputs_url="https://aztec-ci-artifacts.s3.us-east-2.amazonaws.com/protocol/bb-civc-inputs-7f48a235.tar.gz" export inputs_tmp_dir=$(mktemp -d) trap 'rm -rf "$inputs_tmp_dir"' EXIT SIGINT diff --git a/barretenberg/cpp/src/barretenberg/benchmark/ultra_bench/mock_circuits.hpp b/barretenberg/cpp/src/barretenberg/benchmark/ultra_bench/mock_circuits.hpp index 78e2e5774692..85f2c0d7946b 100644 --- a/barretenberg/cpp/src/barretenberg/benchmark/ultra_bench/mock_circuits.hpp +++ b/barretenberg/cpp/src/barretenberg/benchmark/ultra_bench/mock_circuits.hpp @@ -18,9 +18,6 @@ namespace bb::mock_circuits { */ template void generate_basic_arithmetic_circuit(Builder& builder, size_t log2_num_gates) { - // Add default pairing points as its required, but this causes gates to be created... - // TODO(https://github.com/AztecProtocol/barretenberg/issues/984): Get rid of gates when creating default - // pairing points. stdlib::recursion::PairingPoints::add_default_to_public_inputs(builder); stdlib::field_t a(stdlib::witness_t(&builder, fr::random_element())); diff --git a/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description_bigfield.test.cpp b/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description_bigfield.test.cpp index ed3c4689da64..16f847f67e83 100644 --- a/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description_bigfield.test.cpp +++ b/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description_bigfield.test.cpp @@ -41,7 +41,7 @@ using witness_ct = bn254::witness_ct; */ void fix_bigfield_element(const fq_ct& element) { - for (int i = 0; i < 4; i++) { + for (size_t i = 0; i < 4; i++) { element.binary_basis_limbs[i].element.fix_witness(); } element.prime_basis_limb.fix_witness(); diff --git a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp index b603b3109a71..f78d25912872 100644 --- a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp +++ b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp @@ -404,7 +404,6 @@ HonkProof ClientIVC::decider_prove() const vinfo("prove decider..."); fold_output.accumulator->proving_key.commitment_key = bn254_commitment_key; MegaDeciderProver decider_prover(fold_output.accumulator); - vinfo("finished decider proving."); return decider_prover.construct_proof(); } diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/pairing_points.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/pairing_points.hpp index b26108af92de..8a1a0b08db33 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/pairing_points.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/pairing_points.hpp @@ -78,6 +78,11 @@ class PairingPoints { */ void aggregate(const PairingPoints& other) { + if (P0 == Point::infinity() || P1 == Point::infinity() || other.P0 == Point::infinity() || + other.P1 == Point::infinity()) { + throw_or_abort("WARNING: Shouldn't be aggregating with Point at infinity! The pairing points are probably " + "uninitialized."); + } Fr aggregation_separator = Fr::random_element(); P0 = P0 + other.P0 * aggregation_separator; P1 = P1 + other.P1 * aggregation_separator; diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp index 885b107e7562..34c26d1fc383 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp @@ -94,16 +94,17 @@ void create_dummy_vkey_and_proof(typename Flavor::CircuitBuilder& builder, builder.assert_equal(builder.add_variable(fr::random_element()), proof_fields[offset].witness_index); offset++; } - // TODO(https://github.com/AztecProtocol/barretenberg/issues/1352): Using SMALL_DUMMY_VALUE might resolve this - // issue. - fr SMALL_DUMMY_VALUE(2); // arbtirary small value that shouldn't cause builder problems. - // The aggregation object + + // Get some values for a valid aggregation object and use them here to avoid divide by 0 or other issues. + std::array::PUBLIC_INPUTS_SIZE> dummy_pairing_points_values = + PairingPoints::construct_dummy(); for (size_t i = 0; i < PairingPoints::PUBLIC_INPUTS_SIZE; i++) { - builder.assert_equal(builder.add_variable(SMALL_DUMMY_VALUE), proof_fields[offset].witness_index); + builder.assert_equal(builder.add_variable(dummy_pairing_points_values[i]), proof_fields[offset].witness_index); offset++; } // IPA claim + // TODO(https://github.com/AztecProtocol/barretenberg/issues/1392): Don't use random elements here. if constexpr (HasIPAAccumulator) { for (size_t i = 0; i < bb::IPA_CLAIM_SIZE; i++) { builder.assert_equal(builder.add_variable(fr::random_element()), proof_fields[offset].witness_index); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ivc_recursion_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ivc_recursion_constraint.cpp index ec2bb48070f9..60b20c2a9572 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ivc_recursion_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ivc_recursion_constraint.cpp @@ -145,11 +145,32 @@ std::vector create_mock_oink_proof(const size_t num_public_inputs std::vector proof; // Populate mock public inputs - FF MAGIC_PUBLIC_INPUT = 2; // arbitrary small non-zero value to avoid errors - for (size_t i = 0; i < num_public_inputs; ++i) { - proof.emplace_back(MAGIC_PUBLIC_INPUT); + // Get some values for a valid aggregation object and use them here to avoid divide by 0 or other issues. + std::array::PUBLIC_INPUTS_SIZE> + dummy_pairing_points_values = stdlib::recursion::PairingPoints::construct_dummy(); + size_t public_input_count = 0; + for (size_t i = 0; i < stdlib::recursion::PairingPoints::PUBLIC_INPUTS_SIZE; i++) { + proof.emplace_back(dummy_pairing_points_values[i]); + public_input_count++; } + if (public_input_count < num_public_inputs) { + // Databus commitments if necessary + for (size_t i = 0; i < NUM_DATABUS_COMMITMENTS; ++i) { + // We represent commitments in the public inputs as biggroup elements. + using BigGroup = stdlib::element_default::element, + stdlib::field_t, + curve::BN254::Group>; + auto pub_input_comm_vals = BigGroup::construct_dummy(); + for (const fr& comm_fr : pub_input_comm_vals) { + proof.emplace_back(comm_fr); + public_input_count++; + } + } + } + BB_ASSERT_EQ(public_input_count, num_public_inputs, "Mock oink proof has the wrong number of public inputs."); + // Populate mock witness polynomial commitments auto mock_commitment = curve::BN254::AffineElement::one(); std::vector mock_commitment_frs = field_conversion::convert_to_bn254_frs(mock_commitment); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.cpp b/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.cpp index d0ed8993e242..2bbbde00dec0 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.cpp @@ -138,10 +138,6 @@ UltraRecursiveVerifier_::Output UltraRecursiveVerifier_::verify_ sumcheck_output.claimed_libra_evaluation); auto pairing_points = PCS::reduce_verify_batch_opening_claim(opening_claim, transcript); - - // TODO(https://github.com/AztecProtocol/barretenberg/issues/1352): Investigate if normalize() calls are needed. - pairing_points[0] = pairing_points[0].normalize(); - pairing_points[1] = pairing_points[1].normalize(); output.points_accumulator.aggregate(pairing_points); // Extract the IPA claim from the public inputs diff --git a/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.test.cpp index cabb4b964785..a20d14763f7b 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.test.cpp @@ -249,7 +249,7 @@ template class RecursiveVerifierTest : public testing } // Check the size of the recursive verifier if constexpr (std::same_as>) { - uint32_t NUM_GATES_EXPECTED = 871733; + uint32_t NUM_GATES_EXPECTED = 871531; BB_ASSERT_EQ(static_cast(outer_circuit.get_num_finalized_gates()), NUM_GATES_EXPECTED, "MegaZKHonk Recursive verifier changed in Ultra gate count! Update this value if you " diff --git a/barretenberg/cpp/src/barretenberg/stdlib/pairing_points.hpp b/barretenberg/cpp/src/barretenberg/stdlib/pairing_points.hpp index df2cdc9a1b2c..f494eae65366 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/pairing_points.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/pairing_points.hpp @@ -119,13 +119,36 @@ template struct PairingPoints { Group P1 = Group::reconstruct_from_public(P1_limbs); return { P0, P1 }; } + + static std::array construct_dummy() + { + // We just biggroup here instead of Group (which is either biggroup or biggroup_goblin) because this is the most + // efficient way of setting the default pairing points. If we use biggroup_goblin elements, we have to convert + // them back to biggroup elements anyway to add them to the public inputs... + using BigGroup = element_default:: + element, field_t, curve::BN254::Group>; + std::array dummy_pairing_points_values; + size_t idx = 0; + for (size_t i = 0; i < 2; i++) { + std::array element_vals = BigGroup::construct_dummy(); + for (auto& val : element_vals) { + dummy_pairing_points_values[idx++] = val; + } + } + + return dummy_pairing_points_values; + } + /** - * @brief Constructs an arbitrary but valid aggregation state from a valid set of pairing inputs. + * @brief Adds default public inputs to the builder. + * @details This should cost exactly 20 gates because there's 4 bigfield elements and each have 5 total + * witnesses including the prime limb. * * @param builder - * @return PairingPoints */ - static PairingPoints construct_default(typename Curve::Builder& builder) + // TODO(https://github.com/AztecProtocol/barretenberg/issues/984): Check how many gates this costs and if they're + // necessary. + static void add_default_to_public_inputs(Builder& builder) { using BaseField = typename Curve::BaseField; // TODO(https://github.com/AztecProtocol/barretenberg/issues/911): These are pairing points extracted from a @@ -138,15 +161,7 @@ template struct PairingPoints { BaseField y0 = BaseField::from_witness(&builder, y0_val); BaseField x1 = BaseField::from_witness(&builder, x1_val); BaseField y1 = BaseField::from_witness(&builder, y1_val); - // PairingPoints points_accumulator{ Group(x0, y0), Group(x1, y1) }; - return { Group(x0, y0), Group(x1, y1) }; - } - - // TODO(https://github.com/AztecProtocol/barretenberg/issues/984): Check how many gates this costs and if they're - // necessary. - static void add_default_to_public_inputs(Builder& builder) - { - PairingPoints points_accumulator = construct_default(builder); + PairingPoints points_accumulator{ Group(x0, y0), Group(x1, y1) }; points_accumulator.set_public(); } }; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/pairing_points.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/pairing_points.test.cpp new file mode 100644 index 000000000000..227b6fbb90a8 --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/stdlib/pairing_points.test.cpp @@ -0,0 +1,25 @@ +#include "barretenberg/stdlib/pairing_points.hpp" +#include "barretenberg/srs/global_crs.hpp" +#include + +namespace bb::stdlib::recursion { + +template class PairingPointsTests : public testing::Test { + public: + static void SetUpTestSuite() { bb::srs::init_file_crs_factory(bb::srs::bb_crs_path()); } +}; + +using Builders = testing::Types; +TYPED_TEST_SUITE(PairingPointsTests, Builders); + +TYPED_TEST(PairingPointsTests, ConstructDefault) +{ + TypeParam builder; + info("Num gates: ", builder.num_gates); + PairingPoints::add_default_to_public_inputs(builder); + info("Num gates after add_default_to_public_inputs: ", builder.num_gates); + builder.finalize_circuit(/*ensure_nonzero=*/true); + info("Num gates: ", builder.num_gates); + EXPECT_TRUE(CircuitChecker::check(builder)); +} +} // namespace bb::stdlib::recursion diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp index 8388519c7b68..a8c00bba4d7e 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp @@ -62,7 +62,7 @@ template class bigfield { static constexpr size_t NUM_LIMBS = 4; Builder* context; - mutable Limb binary_basis_limbs[NUM_LIMBS]; + mutable std::array binary_basis_limbs; mutable field_t prime_basis_limb; bigfield(const field_t& low_bits, diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp index 6fbae5e28ded..92a3cbb35927 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp @@ -48,6 +48,21 @@ template class element { element(const element& other); element(element&& other) noexcept; + static std::array construct_dummy() + { + const typename NativeGroup::affine_element& native_val = NativeGroup::affine_element::one(); + element val(native_val); + size_t idx = 0; + std::array limb_vals; + for (auto& limb : val.x.binary_basis_limbs) { + limb_vals[idx++] = limb.element.get_value(); + } + for (auto& limb : val.y.binary_basis_limbs) { + limb_vals[idx++] = limb.element.get_value(); + } + BB_ASSERT_EQ(idx, PUBLIC_INPUTS_SIZE); + return limb_vals; + } /** * @brief Set the witness indices for the x and y coordinates to public * diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp index 9881bea6ff64..288cce0e3ebf 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp @@ -13,8 +13,9 @@ namespace bb { // We assume all kernels have space for two return data commitments on their public inputs +constexpr uint32_t NUM_DATABUS_COMMITMENTS = 2; constexpr uint32_t PROPAGATED_DATABUS_COMMITMENT_SIZE = 8; -constexpr uint32_t PROPAGATED_DATABUS_COMMITMENTS_SIZE = PROPAGATED_DATABUS_COMMITMENT_SIZE * 2; // Two databus comms +constexpr uint32_t PROPAGATED_DATABUS_COMMITMENTS_SIZE = PROPAGATED_DATABUS_COMMITMENT_SIZE * NUM_DATABUS_COMMITMENTS; /** * @brief A DataBus column diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/decider_prover.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/decider_prover.cpp index 7edd30479b0a..7ab670d1a035 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/decider_prover.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/decider_prover.cpp @@ -115,7 +115,7 @@ template HonkProof DeciderProver_::construct_ // Fiat-Shamir: rho, y, x, z // Execute Shplemini PCS execute_pcs_rounds(); - + vinfo("finished decider proving."); return export_proof(); } diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/decider_proving_key.hpp b/barretenberg/cpp/src/barretenberg/ultra_honk/decider_proving_key.hpp index 0d4ad6986a24..7b23931bb7cd 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/decider_proving_key.hpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/decider_proving_key.hpp @@ -173,8 +173,8 @@ template class DeciderProvingKey_ { // Set the pairing point accumulator indices. This should exist for all flavors. ASSERT(circuit.pairing_inputs_public_input_key.is_set() && - "Honk circuit must output a pairing point accumulator. If this is a test, you might need to add a " - "default one through a method in PairingPoints."); + "Honk circuit must output a pairing point accumulator. If this is a test, you might need to add a \ + default one through a method in PairingPoints."); proving_key.pairing_inputs_public_input_key = circuit.pairing_inputs_public_input_key; if constexpr (HasIPAAccumulator) { // Set the IPA claim indices diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/decider_verifier.hpp b/barretenberg/cpp/src/barretenberg/ultra_honk/decider_verifier.hpp index 4e6d4a1c1b4f..2863b0d12c23 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/decider_verifier.hpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/decider_verifier.hpp @@ -27,7 +27,14 @@ template class DeciderVerifier_ { bool libra_evals_verified; PairingPoints pairing_points; - bool check() { return sumcheck_verified && libra_evals_verified && pairing_points.check(); } + bool check() + { + bool pairing_check_verified = pairing_points.check(); + vinfo("sumcheck_verified: ", sumcheck_verified); + vinfo("libra_evals_verified: ", libra_evals_verified); + vinfo("pairing_check_verified: ", pairing_check_verified); + return sumcheck_verified && libra_evals_verified && pairing_check_verified; + } }; public: diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_verifier.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_verifier.cpp index 84e5c1268a24..ec67c0ecfc24 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_verifier.cpp @@ -75,6 +75,14 @@ template bool UltraVerifier_::verify_proof(const HonkP DeciderVerifier decider_verifier{ verification_key, transcript }; auto decider_output = decider_verifier.verify(); + if (!decider_output.sumcheck_verified) { + info("Sumcheck failed!"); + return false; + } + if (!decider_output.libra_evals_verified) { + info("Libra evals failed!"); + return false; + } // Extract nested pairing points from the proof // TODO(https://github.com/AztecProtocol/barretenberg/issues/1094): Handle pairing points in keccak flavors. diff --git a/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_verifier.cpp b/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_verifier.cpp index 39b4b255d2b1..5c75f5791a7a 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_verifier.cpp @@ -155,10 +155,6 @@ AvmRecursiveVerifier_::PairingPoints AvmRecursiveVerifier_::veri padding_indicator_array, claim_batcher, output.challenge, Commitment::one(&builder), transcript); auto pairing_points = PCS::reduce_verify_batch_opening_claim(opening_claim, transcript); - - // TODO(https://github.com/AztecProtocol/barretenberg/issues/1352): Investigate if normalize() calls are needed. - pairing_points[0] = pairing_points[0].normalize(); - pairing_points[1] = pairing_points[1].normalize(); return pairing_points; }