feat!: remove avm fallback and constrain public inputs validation (#18957)

Remove the AVM fallback and constrain AVM public inputs validation in
recursive verifier.

Author: dbanks12

barretenberg/cpp/src/barretenberg/vm2/common/aztec_constants.hpp

@@ -175,7 +175,7 @@
 #define AVM_PUBLIC_INPUTS_COLUMNS_MAX_LENGTH 4685
 #define AVM_NUM_PUBLIC_INPUT_COLUMNS 4
 #define AVM_PUBLIC_INPUTS_COLUMNS_COMBINED_LENGTH 18740
-#define AVM_V2_PROOF_LENGTH_IN_FIELDS_PADDED 20000
+#define AVM_V2_PROOF_LENGTH_IN_FIELDS_PADDED 16200
 #define AVM_V2_VERIFICATION_KEY_LENGTH_IN_FIELDS_PADDED 1000
 #define AVM_MAX_PROCESSABLE_L2_GAS 6000000
 #define AVM_PC_SIZE_IN_BITS 32

barretenberg/cpp/src/barretenberg/vm2/constraining/prover.cpp

@@ -219,9 +219,8 @@ HonkProof AvmProver::construct_proof()
     // Add circuit size public input size and public inputs to transcript.
     execute_preamble_round();
 
-    // TODO(https://github.com/AztecProtocol/aztec-packages/pull/17045): make the protocols secure at some point
-    // // Add public inputs to transcript.
-    // AVM_TRACK_TIME("prove/public_inputs_round", execute_public_inputs_round());
+    // Add public inputs to transcript.
+    AVM_TRACK_TIME("prove/public_inputs_round", execute_public_inputs_round());
 
     // Compute wire commitments.
     AVM_TRACK_TIME("prove/wire_commitments_round", execute_wire_commitments_round());

barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_verifier.cpp

@@ -9,7 +9,6 @@
 #include "barretenberg/honk/proof_system/types/proof.hpp"
 #include "barretenberg/polynomials/polynomial.hpp"
 #include "barretenberg/polynomials/shared_shifted_virtual_zeroes_array.hpp"
-#include "barretenberg/stdlib/primitives/bool/bool.hpp"
 #include "barretenberg/stdlib/primitives/field/field.hpp"
 #include "barretenberg/stdlib/primitives/padding_indicator_array/padding_indicator_array.hpp"
 #include "barretenberg/transcript/transcript.hpp"
@@ -70,9 +69,8 @@ AvmRecursiveVerifier::PairingPoints AvmRecursiveVerifier::verify_proof(
 }
 
 // TODO(#991): (see https://github.com/AztecProtocol/barretenberg/issues/991)
-// TODO(#14234)[Unconditional PIs validation]: rename stdlib_proof_with_pi_flag to stdlib_proof
 AvmRecursiveVerifier::PairingPoints AvmRecursiveVerifier::verify_proof(
-    const stdlib::Proof<Builder>& stdlib_proof_with_pi_flag, const std::vector<std::vector<FF>>& public_inputs)
+    const stdlib::Proof<Builder>& stdlib_proof, const std::vector<std::vector<FF>>& public_inputs)
 {
     using Curve = typename Flavor::Curve;
     using PCS = typename Flavor::PCS;
@@ -81,16 +79,6 @@ AvmRecursiveVerifier::PairingPoints AvmRecursiveVerifier::verify_proof(
     using Shplemini = ShpleminiVerifier_<Curve, Flavor::HasZK>;
     using ClaimBatcher = ClaimBatcher_<Curve>;
     using ClaimBatch = ClaimBatcher::Batch;
-    using stdlib::bool_t;
-
-    // TODO(#14234)[Unconditional PIs validation]: Remove the next 3 lines
-    StdlibProof stdlib_proof = stdlib_proof_with_pi_flag;
-    bool_t<Builder> pi_validation = !bool_t<Builder>(stdlib_proof.at(0));
-    // TODO(https://github.com/AztecProtocol/aztec-packages/issues/16716) Origin Tag security mechanism is screaming
-    // that there is a free witness affecting proof verificaton. Because it is and this bool allows completely disabling
-    // public input logic. So this has to be removed in the future.
-    pi_validation.unset_free_witness_tag();
-    stdlib_proof.erase(stdlib_proof.begin());
 
     if (public_inputs.size() != AVM_NUM_PUBLIC_INPUT_COLUMNS) {
         throw_or_abort("AvmRecursiveVerifier::verify_proof: public inputs size mismatch");
@@ -110,21 +98,11 @@ AvmRecursiveVerifier::PairingPoints AvmRecursiveVerifier::verify_proof(
     RelationParams relation_parameters;
     VerifierCommitments commitments{ key };
 
-    // TODO(https://github.com/AztecProtocol/aztec-packages/pull/17045): make the protocols secure at some point
-    // // Add public inputs to transcript
-    // for (size_t i = 0; i < AVM_NUM_PUBLIC_INPUT_COLUMNS; i++) {
-    //     for (size_t j = 0; j < public_inputs[i].size(); j++) {
-    //         transcript->add_to_hash_buffer("public_input_" + std::to_string(i) + "_" + std::to_string(j),
-    //                                        public_inputs[i][j]);
-    //     }
-    // }
-
+    // Add public inputs to transcript for Fiat-Shamir
     for (size_t i = 0; i < AVM_NUM_PUBLIC_INPUT_COLUMNS; i++) {
         for (size_t j = 0; j < public_inputs[i].size(); j++) {
-            // TODO(https://github.com/AztecProtocol/aztec-packages/pull/17045): make the protocols secure at some point
-            // transcript->add_to_hash_buffer("public_input_" + std::to_string(i) + "_" + std::to_string(j),
-            //                               public_inputs[i][j]);
-            public_inputs[i][j].unset_free_witness_tag();
+            transcript->add_to_hash_buffer("public_input_" + std::to_string(i) + "_" + std::to_string(j),
+                                           public_inputs[i][j]);
         }
     }
     // Get commitments to VM wires
@@ -168,12 +146,11 @@ AvmRecursiveVerifier::PairingPoints AvmRecursiveVerifier::verify_proof(
         output.claimed_evaluations.get(C::public_inputs_cols_3_),
     };
 
-    // TODO(#14234)[Unconditional PIs validation]: Inside of loop, replace pi_validation.must_imply() by
-    // public_input_evaluation.assert_equal(claimed_evaluations[i]
+    // Validate public inputs match the claimed evaluations
     for (size_t i = 0; i < AVM_NUM_PUBLIC_INPUT_COLUMNS; i++) {
         FF public_input_evaluation = evaluate_public_input_column(public_inputs[i], output.challenge);
-        pi_validation.must_imply(public_input_evaluation == claimed_evaluations[i],
-                                 format("public_input_evaluation failed at column ", i));
+        public_input_evaluation.assert_equal(claimed_evaluations[i],
+                                             format("public_input_evaluation failed at column ", i));
     }
 
     // Batch commitments and evaluations using short scalars to reduce ECCVM circuit size

barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_verifier.hpp

@@ -29,9 +29,7 @@ class AvmRecursiveVerifier {
     [[nodiscard("IPA claim and Pairing points should be accumulated")]] PairingPoints verify_proof(
         const HonkProof& proof, const std::vector<std::vector<fr>>& public_inputs_vec_nt);
     [[nodiscard("IPA claim and Pairing points should be accumulated")]] PairingPoints verify_proof(
-        const StdlibProof& stdlib_proof_with_pi_flag, // TODO(#14234)[Unconditional PIs validation]: rename
-                                                      // stdlib_proof_with_pi_flag to stdlib_proof
-        const std::vector<std::vector<typename Flavor::FF>>& public_inputs);
+        const StdlibProof& stdlib_proof, const std::vector<std::vector<typename Flavor::FF>>& public_inputs);
 
     Builder& builder;
     std::shared_ptr<VerificationKey> key;

barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_verifier.test.cpp

@@ -6,6 +6,7 @@
 #include "barretenberg/ultra_honk/prover_instance.hpp"
 #include "barretenberg/ultra_honk/ultra_prover.hpp"
 #include "barretenberg/ultra_honk/ultra_verifier.hpp"
+#include "barretenberg/vm2/common/aztec_constants.hpp"
 #include "barretenberg/vm2/constraining/prover.hpp"
 #include "barretenberg/vm2/constraining/recursion/goblin_avm_recursive_verifier.hpp"
 #include "barretenberg/vm2/constraining/recursion/recursive_flavor.hpp"
@@ -57,19 +58,26 @@ class AvmRecursiveTests : public ::testing::Test {
     }
 };
 
+// Parameterized test class for testing with and without proof padding
+class AvmRecursiveTestsParameterized : public AvmRecursiveTests, public ::testing::WithParamInterface<bool> {};
+
 /**
  * @brief A test of the Goblinized AVM recursive verifier.
  * @details Constructs a simple AVM circuit for which a proof is verified using the Goblinized AVM recursive verifier. A
  * proof is constructed and verified for the outer (Ultra) circuit produced by this algorithm. See the documentation in
  * AvmGoblinRecursiveVerifier for details of the recursive verification algorithm.
  *
+ * When pad_proof=true (Padded variant), the proof is padded to AVM_V2_PROOF_LENGTH_IN_FIELDS_PADDED to match production
+ * behavior where TypeScript pads the proof before passing it to noir circuits.
  */
-TEST_F(AvmRecursiveTests, GoblinRecursion)
+TEST_P(AvmRecursiveTestsParameterized, GoblinRecursion)
 {
     if (testing::skip_slow_tests()) {
         GTEST_SKIP() << "Skipping slow test";
     }
 
+    const bool pad_proof = GetParam();
+
     // Type aliases specific to GoblinRecursion test
     using AvmRecursiveVerifier = AvmGoblinRecursiveVerifier;
     using OuterBuilder = typename UltraRollupFlavor::CircuitBuilder;
@@ -86,7 +94,14 @@ TEST_F(AvmRecursiveTests, GoblinRecursion)
               << "s" << std::endl;
 
     auto [proof, public_inputs_cols] = proof_result;
-    proof.insert(proof.begin(), 0); // TODO(#14234)[Unconditional PIs validation]: remove this
+
+    // Optionally pad the proof to match production behavior
+    if (pad_proof) {
+        std::cout << "Padding proof from " << proof.size() << " to " << AVM_V2_PROOF_LENGTH_IN_FIELDS_PADDED
+                  << " fields" << std::endl;
+        ASSERT_LE(proof.size(), AVM_V2_PROOF_LENGTH_IN_FIELDS_PADDED) << "Proof exceeds padded length";
+        proof.resize(AVM_V2_PROOF_LENGTH_IN_FIELDS_PADDED, 0);
+    }
 
     // Construct stdlib representations of the proof, public inputs and verification key
     OuterBuilder outer_circuit;
@@ -106,7 +121,8 @@ TEST_F(AvmRecursiveTests, GoblinRecursion)
     // Construct the AVM recursive verifier and verify the proof
     // Scoped to free memory of AvmRecursiveVerifier.
     auto verifier_output = [&]() {
-        std::cout << "Constructing AvmRecursiveVerifier and verifying proof..." << std::endl;
+        std::cout << "Constructing AvmRecursiveVerifier and verifying " << (pad_proof ? "padded " : "") << "proof..."
+                  << std::endl;
         std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now();
         AvmRecursiveVerifier avm_rec_verifier(outer_circuit);
         auto result = avm_rec_verifier.verify_proof(stdlib_proof, public_inputs_ct);
@@ -129,7 +145,10 @@ TEST_F(AvmRecursiveTests, GoblinRecursion)
     ASSERT_TRUE(agg_output_valid) << "Pairing points (aggregation state) are not valid.";
     ASSERT_FALSE(outer_circuit.failed()) << "Outer circuit has failed.";
 
-    vinfo("Recursive verifier: finalized num gates = ", outer_circuit.num_gates());
+    vinfo("Recursive verifier",
+          (pad_proof ? " (padded proof)" : ""),
+          ": finalized num gates = ",
+          outer_circuit.num_gates());
 
     // Construct and verify an Ultra Rollup proof of the AVM recursive verifier circuit. This proof carries an IPA claim
     // from ECCVM recursive verification in its public inputs that will be verified as part of the UltraRollupVerifier.
@@ -153,99 +172,10 @@ TEST_F(AvmRecursiveTests, GoblinRecursion)
     EXPECT_TRUE(result);
 }
 
-// Similar to GoblinRecursion, but with PI validation disabled and garbage PIs in the public inputs.
-// This is important as long as we use a fallback mechanism for the AVM proofs.
-TEST_F(AvmRecursiveTests, GoblinRecursionWithoutPIValidation)
-{
-    if (testing::skip_slow_tests()) {
-        GTEST_SKIP() << "Skipping slow test";
-    }
-
-    // Type aliases specific to GoblinRecursion test
-    using AvmRecursiveVerifier = AvmGoblinRecursiveVerifier;
-    using OuterBuilder = typename UltraRollupFlavor::CircuitBuilder;
-    using UltraFF = UltraRecursiveFlavor_<OuterBuilder>::FF;
-    using UltraRollupProver = UltraProver_<UltraRollupFlavor>;
-    using NativeVerifierCommitmentKey = typename AvmFlavor::VerifierCommitmentKey;
-
-    NativeProofResult proof_result;
-    std::cout << "Creating and verifying native proof..." << std::endl;
-    std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now();
-    ASSERT_NO_FATAL_FAILURE({ create_and_verify_native_proof(proof_result); });
-    std::chrono::steady_clock::time_point end = std::chrono::steady_clock::now();
-    std::cout << "Time taken (native proof): " << std::chrono::duration_cast<std::chrono::seconds>(end - start).count()
-              << "s" << std::endl;
-
-    auto [proof, public_inputs_cols] = proof_result;
-    // Set fallback / disable PI validation
-    proof.insert(proof.begin(),
-                 1); // TODO(#14234)[Unconditional PIs validation]: PI validation is disabled for this test.
-
-    // Construct stdlib representations of the proof, public inputs and verification key
-    OuterBuilder outer_circuit;
-    stdlib::Proof<OuterBuilder> stdlib_proof(outer_circuit, proof);
-
-    std::vector<std::vector<UltraFF>> public_inputs_ct;
-    public_inputs_ct.reserve(public_inputs_cols.size());
-    // Use GARBAGE in public inputs and confirm that PI validation is disabled!
-    for (const auto& vec : public_inputs_cols) {
-        std::vector<UltraFF> vec_ct;
-        vec_ct.reserve(vec.size());
-        for (const auto& _ : vec) {
-            vec_ct.push_back(UltraFF::from_witness(&outer_circuit, FF::random_element()));
-        }
-        public_inputs_ct.push_back(vec_ct);
-    }
-
-    // Construct the AVM recursive verifier and verify the proof
-    // Scoped to free memory of AvmRecursiveVerifier.
-    auto verifier_output = [&]() {
-        std::cout << "Constructing AvmRecursiveVerifier and verifying proof..." << std::endl;
-        std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now();
-        AvmRecursiveVerifier avm_rec_verifier(outer_circuit);
-        auto result = avm_rec_verifier.verify_proof(stdlib_proof, public_inputs_ct);
-        std::chrono::steady_clock::time_point end = std::chrono::steady_clock::now();
-        std::cout << "Time taken (recursive verification): "
-                  << std::chrono::duration_cast<std::chrono::seconds>(end - start).count() << "s" << std::endl;
-        return result;
-    }();
-
-    stdlib::recursion::honk::RollupIO inputs;
-    inputs.pairing_inputs = verifier_output.points_accumulator;
-    inputs.ipa_claim = verifier_output.ipa_claim;
-    inputs.set_public();
-    outer_circuit.ipa_proof = verifier_output.ipa_proof.get_value();
-
-    // Ensure that the pairing check is satisfied on the outputs of the recursive verifier
-    NativeVerifierCommitmentKey pcs_vkey{};
-    bool agg_output_valid = pcs_vkey.pairing_check(verifier_output.points_accumulator.P0.get_value(),
-                                                   verifier_output.points_accumulator.P1.get_value());
-    ASSERT_TRUE(agg_output_valid) << "Pairing points (aggregation state) are not valid.";
-    ASSERT_FALSE(outer_circuit.failed()) << "Outer circuit has failed.";
-
-    vinfo("Recursive verifier: finalized num gates = ", outer_circuit.num_gates());
-
-    // Construct and verify an Ultra Rollup proof of the AVM recursive verifier circuit. This proof carries an IPA claim
-    // from ECCVM recursive verification in its public inputs that will be verified as part of the UltraRollupVerifier.
-    auto outer_proving_key = std::make_shared<ProverInstance_<UltraRollupFlavor>>(outer_circuit);
-
-    // Scoped to free memory of UltraRollupProver.
-    auto outer_proof = [&]() {
-        auto verification_key =
-            std::make_shared<UltraRollupFlavor::VerificationKey>(outer_proving_key->get_precomputed());
-        UltraRollupProver outer_prover(outer_proving_key, verification_key);
-        return outer_prover.construct_proof();
-    }();
-
-    // Verify the proof of the Ultra circuit that verified the AVM recursive verifier circuit
-    auto outer_verification_key =
-        std::make_shared<UltraRollupFlavor::VerificationKey>(outer_proving_key->get_precomputed());
-    auto outer_vk_and_hash = std::make_shared<UltraRollupFlavor::VKAndHash>(outer_verification_key);
-    UltraRollupVerifier final_verifier(outer_vk_and_hash);
-
-    bool result = final_verifier.verify_proof(outer_proof).result;
-    EXPECT_TRUE(result);
-}
+INSTANTIATE_TEST_SUITE_P(PaddingVariants,
+                         AvmRecursiveTestsParameterized,
+                         ::testing::Values(false, true),
+                         [](const auto& info) { return info.param ? "Padded" : "Unpadded"; });
 
 // Ensures that the recursive verifier fails with wrong PIs.
 TEST_F(AvmRecursiveTests, GoblinRecursionFailsWithWrongPIs)
@@ -268,8 +198,6 @@ TEST_F(AvmRecursiveTests, GoblinRecursionFailsWithWrongPIs)
               << "s" << std::endl;
 
     auto [proof, public_inputs_cols] = proof_result;
-    // PI validation is enabled.
-    proof.insert(proof.begin(), 0); // TODO(#14234)[Unconditional PIs validation]: remove this
 
     // Construct stdlib representations of the proof, public inputs and verification key
     OuterBuilder outer_circuit;
@@ -285,7 +213,7 @@ TEST_F(AvmRecursiveTests, GoblinRecursionFailsWithWrongPIs)
         }
         public_inputs_ct.push_back(vec_ct);
     }
-    // Mutate some PI entry so that we can confirm that PI validation is enabled and fails!
+    // Mutate a PI entry to verify that validation correctly fails with incorrect public inputs
     public_inputs_ct[1][5] += 1;
 
     // Construct the AVM recursive verifier and verify the proof

barretenberg/cpp/src/barretenberg/vm2/constraining/verifier.cpp

@@ -71,11 +71,10 @@ bool AvmVerifier::verify_proof(const HonkProof& proof, const std::vector<std::ve
             vinfo("Public input size mismatch");
             return false;
         }
-        // TODO(https://github.com/AztecProtocol/aztec-packages/pull/17045): make the protocols secure at some point
-        // for (size_t j = 0; j < public_inputs[i].size(); j++) {
-        //     transcript->add_to_hash_buffer("public_input_" + std::to_string(i) + "_" + std::to_string(j),
-        //                                    public_inputs[i][j]);
-        // }
+        for (size_t j = 0; j < public_inputs[i].size(); j++) {
+            transcript->add_to_hash_buffer("public_input_" + std::to_string(i) + "_" + std::to_string(j),
+                                           public_inputs[i][j]);
+        }
     }
     VerifierCommitments commitments{ key };
     // Get commitments to VM wires

barretenberg/cpp/src/barretenberg/vm2/dsl/avm2_recursion_constraint.cpp

@@ -67,12 +67,8 @@ void create_dummy_proof(Builder& builder, [[maybe_unused]] size_t proof_size, co
         offset++;
     };
 
-    size_t offset = 0;
-
     // This routine is adding some placeholders for avm proof and avm vk in the case where witnesses are not present.
-    // TODO(#14234)[Unconditional PIs validation]: Remove next line and use offset == 0 for subsequent line.
-    builder.set_variable(proof_fields[0].get_witness_index(), 1);
-    offset = 1; // TODO(#14234)[Unconditional PIs validation]: reset offset = 1
+    size_t offset = 0;
 
     // Witness Commitments
     for (size_t i = 0; i < Flavor::NUM_WITNESS_ENTITIES; i++) {

barretenberg/cpp/src/barretenberg/vm2/dsl/avm2_recursion_constraint.test.cpp

@@ -54,8 +54,6 @@ class AcirAvm2RecursionConstraint : public ::testing::Test {
 
         const auto public_inputs_flat = PublicInputs::columns_to_flat(public_inputs.to_columns());
 
-        // TODO(#14234)[Unconditional PIs validation]: Remove next line
-        proof.insert(proof.begin(), 0);
         return { proof, public_inputs_flat };
     }