chore: minor fixes pt. 3 (#22181)

triaging relations findings

Author: iakovenkos

barretenberg/cpp/src/barretenberg/bbapi/bbapi.test.cpp

@@ -1,4 +1,6 @@
 #include "barretenberg/bbapi/bbapi.hpp"
+#include "barretenberg/api/file_io.hpp"
+#include "barretenberg/bbapi/bbapi_shared.hpp"
 #include "barretenberg/common/serialize.hpp"
 #include "barretenberg/common/utils.hpp"
 #include "barretenberg/serialize/test_helper.hpp"
@@ -42,3 +44,72 @@ TYPED_TEST(BBApiMsgpack, DefaultConstructorRoundtrip)
     EXPECT_EQ(actual_response, expected_response);
     std::cout << msgpack_schema_to_string(command) << " " << msgpack_schema_to_string(response) << std::endl;
 }
+
+// Regression tests for input validation at API boundaries.
+// These ensure non-canonical field encodings and trailing bytes are rejected.
+
+TEST(BBApiInputValidation, NonCanonicalPublicInputRejected)
+{
+    using Flavor = UltraFlavor;
+    // A value >= BN254 scalar field modulus should be rejected
+    uint256_t non_canonical = fr::modulus + 1;
+    std::vector<uint256_t> bad_public_inputs = { non_canonical };
+    std::vector<uint256_t> proof = { uint256_t(0) };
+
+    EXPECT_THROW(bbapi::concatenate_proof<Flavor>(bad_public_inputs, proof), std::runtime_error);
+}
+
+TEST(BBApiInputValidation, NonCanonicalProofElementRejected)
+{
+    using Flavor = UltraFlavor;
+    // The modulus itself is non-canonical (valid range is [0, modulus))
+    uint256_t non_canonical = fr::modulus;
+    std::vector<uint256_t> public_inputs = { uint256_t(42) };
+    std::vector<uint256_t> bad_proof = { non_canonical };
+
+    EXPECT_THROW(bbapi::concatenate_proof<Flavor>(public_inputs, bad_proof), std::runtime_error);
+}
+
+TEST(BBApiInputValidation, CanonicalValuesAccepted)
+{
+    using Flavor = UltraFlavor;
+    // modulus - 1 is the largest canonical value
+    uint256_t max_canonical = fr::modulus - 1;
+    std::vector<uint256_t> public_inputs = { uint256_t(0), max_canonical };
+    std::vector<uint256_t> proof = { uint256_t(1) };
+
+    EXPECT_NO_THROW(bbapi::concatenate_proof<Flavor>(public_inputs, proof));
+}
+
+TEST(BBApiInputValidation, TrailingBytesInBinaryInputRejected)
+{
+    // A buffer that is not a multiple of 32 bytes should be rejected
+    std::vector<uint8_t> buf(32 + 1, 0); // 33 bytes = 1 field element + 1 trailing byte
+    EXPECT_THROW(many_from_buffer_exact<uint256_t>(buf, "test input"), std::runtime_error);
+}
+
+TEST(BBApiInputValidation, ExactBinaryInputAccepted)
+{
+    // A buffer that is exactly 2 field elements should parse fine
+    std::vector<uint8_t> buf(64, 0);
+    EXPECT_NO_THROW(many_from_buffer_exact<uint256_t>(buf, "test input"));
+    auto result = many_from_buffer_exact<uint256_t>(buf, "test input");
+    EXPECT_EQ(result.size(), 2UL);
+}
+
+TEST(BBApiInputValidation, VkWithTrailingBytesRejectedOnProveSide)
+{
+    using VK = UltraFlavor::VerificationKey;
+    const size_t expected_size = VK::calc_num_data_types() * sizeof(bb::fr);
+    // One extra byte beyond the expected VK size
+    std::vector<uint8_t> bad_vk(expected_size + 1, 0);
+    EXPECT_THROW(bbapi::validate_vk_size<VK>(bad_vk), std::runtime_error);
+}
+
+TEST(BBApiInputValidation, VkWithCorrectSizeAccepted)
+{
+    using VK = UltraFlavor::VerificationKey;
+    const size_t expected_size = VK::calc_num_data_types() * sizeof(bb::fr);
+    std::vector<uint8_t> good_vk(expected_size, 0);
+    EXPECT_NO_THROW(bbapi::validate_vk_size<VK>(good_vk));
+}

barretenberg/cpp/src/barretenberg/bbapi/bbapi_ultra_honk.cpp

@@ -72,6 +72,7 @@ CircuitProve::Response _prove(std::vector<uint8_t>&& bytecode,
         info("WARNING: computing verification key while proving. Pass in a precomputed vk for better performance.");
         vk = std::make_shared<VerificationKey>(prover_instance->get_precomputed());
     } else {
+        validate_vk_size<VerificationKey>(vk_bytes);
         vk = std::make_shared<VerificationKey>(from_buffer<VerificationKey>(vk_bytes));
     }
 

barretenberg/cpp/src/barretenberg/chonk/chonk.cpp

@@ -25,7 +25,7 @@ namespace bb {
 Chonk::Chonk(size_t num_circuits)
     : num_circuits(num_circuits)
 {
-    BB_ASSERT_GT(num_circuits, 0UL, "Number of circuits must be specified and greater than 0.");
+    BB_ASSERT_GTE(num_circuits, 4UL, "Number of circuits must be at least 4 (get_queue_type uses num_circuits - 3).");
 }
 
 /**

barretenberg/cpp/src/barretenberg/chonk/chonk_proof.cpp

@@ -43,7 +43,9 @@ ChonkProof_<IsRecursive> ChonkProof_<IsRecursive>::from_field_elements(const std
 
     // MegaZK Oink proof size = total - all other fixed-size components.
     // This correctly accounts for any ACIR public inputs prepended to the oink portion.
-    const size_t mega_zk_oink_length = fields.size() - merge_size - eccvm_size - ipa_size - joint_size;
+    constexpr size_t fixed_total = merge_size + eccvm_size + ipa_size + joint_size;
+    BB_ASSERT_GTE(fields.size(), fixed_total, "ChonkProof::from_field_elements: proof too short");
+    const size_t mega_zk_oink_length = fields.size() - fixed_total;
 
     auto it = fields.begin();
 

barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder_elliptic.test.cpp

@@ -276,3 +276,30 @@ TEST_F(UltraCircuitBuilderElliptic, ChainedOperationsDoubleFailure)
     // Should fail because the middle operation (doubling) has an invalid result
     EXPECT_FALSE(CircuitChecker::check(builder));
 }
+
+// Demonstrates that a doubling gate with off-curve input (0,0) does not constrain the output.
+// The elliptic relation substitutes x1^3 = y1^2 - b, which is invalid for off-curve points.
+// When (x1,y1) = (0,0), both the x- and y-coordinate subrelations become 0 = 0 for any output.
+// Production code (cycle_group::dbl) prevents this by substituting y1 = 1 for points at infinity.
+// See CycleGroupTest.TestDblWitnessPoints (cycle_group.test.cpp) for the complementary test
+// that verifies the mitigation works correctly.
+TEST_F(UltraCircuitBuilderElliptic, DoubleOffCurveOriginUnconstrainedOutput)
+{
+    // (0,0) is not on grumpkin (y^2 = x^3 - 17), so this is an off-curve input
+    auto x1_val = bb::fr(0);
+    auto y1_val = bb::fr(0);
+    // Claim an arbitrary output point
+    auto x3_val = bb::fr::random_element();
+    auto y3_val = bb::fr::random_element();
+
+    UltraCircuitBuilder builder;
+    auto x1 = builder.add_variable(x1_val);
+    auto y1 = builder.add_variable(y1_val);
+    auto x3 = builder.add_variable(x3_val);
+    auto y3 = builder.add_variable(y3_val);
+    builder.create_ecc_dbl_gate({ x1, y1, x3, y3 });
+
+    // The circuit checker passes because the relation is trivially satisfied for (0,0) input.
+    // This is NOT a bug — production code never lets (0,0) reach a doubling gate.
+    EXPECT_TRUE(CircuitChecker::check(builder));
+}

barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_checker.cpp

@@ -48,6 +48,10 @@ MegaCircuitBuilder_<bb::fr> UltraCircuitChecker::prepare_circuit<MegaCircuitBuil
 
 template <typename Builder> bool UltraCircuitChecker::check(const Builder& builder_in)
 {
+    if (builder_in.failed()) {
+        info("CircuitChecker: circuit contains invalid witnesses: ", builder_in.err());
+    }
+
     Builder builder = UltraCircuitChecker::prepare_circuit(builder_in);
 
     // Construct a hash table for lookup table entries to efficiently determine if a lookup gate is valid

barretenberg/cpp/src/barretenberg/flavor/mega_flavor.hpp

@@ -199,24 +199,6 @@ class MegaFlavor {
         auto get_to_be_shifted() { return RefArray{ z_perm }; };
     };
 
-    /**
-     * @brief ZK-specific entities (only used when HasZK = true)
-     * @details Contains the Gemini masking polynomial used for zero-knowledge
-     */
-    template <typename DataType, bool HasZK_ = false> class MaskingEntities {
-      public:
-        // When ZK is disabled, this class is empty
-        auto get_all() { return RefArray<DataType, 0>{}; }
-        auto get_all() const { return RefArray<const DataType, 0>{}; }
-        static auto get_labels() { return std::vector<std::string>{}; }
-    };
-
-    // Specialization for when ZK is enabled
-    template <typename DataType> class MaskingEntities<DataType, true> {
-      public:
-        DEFINE_FLAVOR_MEMBERS(DataType, gemini_masking_poly)
-    };
-
     /**
      * @brief Container for all witness polynomials used/constructed by the prover.
      * @details Shifts are not included here since they do not occupy their own memory.
@@ -324,24 +306,19 @@ class MegaFlavor {
      * @details Used to build containers for: the prover's polynomial during sumcheck; the sumcheck's folded
      * polynomials; the univariates constructed during sumcheck; the evaluations produced by sumcheck.
      *
-     * Symbolically we have: AllEntities = MaskingEntities + PrecomputedEntities + WitnessEntities + ShiftedEntities.
+     * Symbolically we have: AllEntities = PrecomputedEntities + WitnessEntities + ShiftedEntities.
+     * Note: Mega has no MaskingEntities — ZK masking is provided by the Translator in Chonk.
      */
-    template <typename DataType, bool HasZK_ = HasZK>
-    class AllEntities_ : public MaskingEntities<DataType, HasZK_>,
-                         public PrecomputedEntities<DataType>,
+    template <typename DataType>
+    class AllEntities_ : public PrecomputedEntities<DataType>,
                          public WitnessEntities_<DataType>,
                          public ShiftedEntities<DataType> {
       public:
-        DEFINE_COMPOUND_GET_ALL(MaskingEntities<DataType, HasZK_>,
-                                PrecomputedEntities<DataType>,
-                                WitnessEntities_<DataType>,
-                                ShiftedEntities<DataType>)
+        DEFINE_COMPOUND_GET_ALL(PrecomputedEntities<DataType>, WitnessEntities_<DataType>, ShiftedEntities<DataType>)
 
         auto get_unshifted()
         {
-            return concatenate(MaskingEntities<DataType, HasZK_>::get_all(),
-                               PrecomputedEntities<DataType>::get_all(),
-                               WitnessEntities_<DataType>::get_all());
+            return concatenate(PrecomputedEntities<DataType>::get_all(), WitnessEntities_<DataType>::get_all());
         };
         auto get_precomputed() { return PrecomputedEntities<DataType>::get_all(); }
         auto get_witness() { return WitnessEntities_<DataType>::get_all(); };
@@ -350,8 +327,7 @@ class MegaFlavor {
         auto get_shifted() const { return ShiftedEntities<DataType>::get_all(); };
     };
 
-    // Default AllEntities alias (no ZK)
-    template <typename DataType> using AllEntities = AllEntities_<DataType, HasZK>;
+    template <typename DataType> using AllEntities = AllEntities_<DataType>;
 
     // Derive entity counts from the actual struct definitions
     static constexpr size_t NUM_PRECOMPUTED_ENTITIES = PrecomputedEntities<FF>::_members_size;
@@ -375,21 +351,12 @@ class MegaFlavor {
      * @brief A field element for each entity of the flavor. These entities represent the prover polynomials evaluated
      * at one point.
      */
-    template <bool HasZK_ = HasZK> class AllValues_ : public AllEntities_<FF, HasZK_> {
-      public:
-        using Base = AllEntities_<FF, HasZK_>;
-        using Base::Base;
-    };
-
-    using AllValues = AllValues_<HasZK>;
+    using AllValues = AllEntities_<FF>;
 
     /**
      * @brief A container for the prover polynomials handles.
      */
-    template <bool HasZK_ = HasZK>
-    using ProverPolynomials_ = ProverPolynomialsBase<AllEntities_<Polynomial, HasZK_>, AllValues_<HasZK_>, Polynomial>;
-
-    using ProverPolynomials = ProverPolynomials_<HasZK>;
+    using ProverPolynomials = ProverPolynomialsBase<AllEntities_<Polynomial>, AllValues, Polynomial>;
 
     using PrecomputedData = PrecomputedData_<Polynomial, NUM_PRECOMPUTED_ENTITIES>;
 
@@ -404,11 +371,8 @@ class MegaFlavor {
     /**
      * @brief A container for storing the partially evaluated multivariates produced by sumcheck.
      */
-    template <bool HasZK_ = HasZK>
-    using PartiallyEvaluatedMultivariates_ =
-        PartiallyEvaluatedMultivariatesBase<AllEntities_<Polynomial, HasZK_>, ProverPolynomials_<HasZK_>, Polynomial>;
-
-    using PartiallyEvaluatedMultivariates = PartiallyEvaluatedMultivariates_<HasZK>;
+    using PartiallyEvaluatedMultivariates =
+        PartiallyEvaluatedMultivariatesBase<AllEntities_<Polynomial>, ProverPolynomials, Polynomial>;
 
     /**
      * @brief A container for univariates used in sumcheck.
@@ -497,8 +461,8 @@ class MegaFlavor {
     /**
      * Note: Made generic for use in MegaRecursive.
      **/
-    template <typename Commitment, typename VerificationKey, bool HasZK_ = HasZK>
-    class VerifierCommitments_ : public AllEntities_<Commitment, HasZK_> {
+    template <typename Commitment, typename VerificationKey>
+    class VerifierCommitments_ : public AllEntities_<Commitment> {
       public:
         VerifierCommitments_(const std::shared_ptr<VerificationKey>& verification_key,
                              const std::optional<WitnessEntities<Commitment>>& witness_commitments = std::nullopt)
@@ -525,7 +489,7 @@ class MegaFlavor {
         }
     };
     // Specialize for Mega (general case used in MegaRecursive).
-    using VerifierCommitments = VerifierCommitments_<Commitment, VerificationKey, HasZK>;
+    using VerifierCommitments = VerifierCommitments_<Commitment, VerificationKey>;
 };
 
 } // namespace bb

barretenberg/cpp/src/barretenberg/honk/composer/composer_lib.hpp

@@ -34,6 +34,8 @@ void construct_lookup_table_polynomials(const RefArray<typename Flavor::Polynomi
             offset++;
         }
     }
+    BB_ASSERT(offset <= table_polynomials[0].end_index(),
+              "construct_lookup_table_polynomials: total lookup table entries exceed polynomial size");
 }
 
 /**

barretenberg/cpp/src/barretenberg/honk/proof_length.hpp

@@ -109,9 +109,11 @@ template <typename Flavor> struct Honk {
      * @param proof_size Total proof size in field elements
      * @param log_n Log of circuit size (VIRTUAL_LOG_N for padded, vk->log_circuit_size for non-padded)
      */
-    static constexpr size_t derive_num_public_inputs(size_t proof_size, size_t log_n)
+    static size_t derive_num_public_inputs(size_t proof_size, size_t log_n)
     {
-        return proof_size - LENGTH_WITHOUT_PUB_INPUTS(log_n);
+        const size_t overhead = LENGTH_WITHOUT_PUB_INPUTS(log_n);
+        BB_ASSERT_GTE(proof_size, overhead, "Honk proof too short to derive num_public_inputs");
+        return proof_size - overhead;
     }
 
     /**
@@ -141,9 +143,11 @@ template <typename Flavor> struct HypernovaInstanceToAccum {
         return Oink<Flavor>::LENGTH_WITHOUT_PUB_INPUTS + Sumcheck<Flavor>::LENGTH(log_n);
     }
 
-    static constexpr size_t derive_num_public_inputs(size_t proof_size, size_t log_n)
+    static size_t derive_num_public_inputs(size_t proof_size, size_t log_n)
     {
-        return proof_size - LENGTH_WITHOUT_PUB_INPUTS(log_n);
+        const size_t overhead = LENGTH_WITHOUT_PUB_INPUTS(log_n);
+        BB_ASSERT_GTE(proof_size, overhead, "HypernovaInstanceToAccum proof too short to derive num_public_inputs");
+        return proof_size - overhead;
     }
 };
 
@@ -179,9 +183,11 @@ template <typename Flavor, typename BatchingFlavor> struct HypernovaFolding {
                MultilinearBatching<BatchingFlavor>::LENGTH;
     }
 
-    static constexpr size_t derive_num_public_inputs(size_t proof_size, size_t log_n)
+    static size_t derive_num_public_inputs(size_t proof_size, size_t log_n)
     {
-        return proof_size - LENGTH_WITHOUT_PUB_INPUTS(log_n);
+        const size_t overhead = LENGTH_WITHOUT_PUB_INPUTS(log_n);
+        BB_ASSERT_GTE(proof_size, overhead, "HypernovaFolding proof too short to derive num_public_inputs");
+        return proof_size - overhead;
     }
 };
 

barretenberg/cpp/src/barretenberg/relations/logderiv_lookup_relation.hpp

@@ -75,9 +75,14 @@ namespace bb {
  * \text{read_tag} \cdot \text{read_tag} - \text{read_tag} = 0
  * \f]
  *
- * Further constraining of read_tags and read_counts is not required, since by tampering read_tags a malicious
- * prover can only skip a table_term. This is disadvantageous for the cheating prover as it reduces the size of the
- * lookup table. Hence, a malicious prover cannot abuse this to prove an incorrect lookup.
+ * Further constraining of read_tags and read_counts is not required:
+ * - read_tags: tampering can only skip a table_term, which is disadvantageous for the cheating prover as it
+ *   reduces the size of the lookup table.
+ * - read_counts: soundness follows from the partial fraction decomposition of the lookup identity. After
+ *   substituting random challenges (β, γ), each table_term and lookup_term evaluates to a distinct field element
+ *   (Schwartz-Zippel). A lookup of a value not in the table creates a pole in the LHS (Σ 1/lookup_term) that no
+ *   choice of read_counts can cancel on the RHS (Σ read_count/table_term), since the denominators are distinct.
+ *   See Haböck (https://eprint.iacr.org/2022/1530.pdf), Section 4.
  *
  * @note Subrelation (2) is "linearly dependent" in the sense that it establishes that a sum across all rows of the
  * execution trace is zero, rather than that some expression holds independently at each row. Accordingly, this