feat: Standalone calldata test (AztecProtocol#3842)

Adds a basic gate construction method for a calldata lookup and a standalone test to leave the DataBus PoC work in a slightly better state while I'm distracted with other things.

barretenberg/cpp/src/barretenberg/proof_system/arithmetization/gate_data.hpp

@@ -133,6 +133,11 @@ template <typename FF> struct ecc_dbl_gate_ {
     uint32_t y3;
 };
 
+template <typename FF> struct databus_lookup_gate_ {
+    uint32_t index;
+    uint32_t value;
+};
+
 template <typename FF> struct poseidon2_external_gate_ {
     uint32_t a;
     uint32_t b;

barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.cpp

@@ -31,42 +31,15 @@ template <typename FF> void GoblinUltraCircuitBuilder_<FF>::add_gates_to_ensure_
     // All that remains is to handle databus related and poseidon2 related polynomials. In what follows we populate the
     // calldata with some mock data then constuct a single calldata read gate
 
-    // Populate the calldata with some data
-    public_calldata.emplace_back(this->add_variable(FF(5)));
-    public_calldata.emplace_back(this->add_variable(FF(7)));
-    public_calldata.emplace_back(this->add_variable(FF(9)));
-
-    // Construct read counts with length of calldata
-    calldata_read_counts.resize(public_calldata.size());
-    for (auto& val : calldata_read_counts) {
-        val = 0;
-    }
-
-    // Construct gate corresponding to a single calldata read
-    size_t read_idx = 1;                                        // index into calldata array at which we want to read
-    this->w_l().emplace_back(public_calldata[read_idx]);        // populate with value of calldata at read index
-    this->w_r().emplace_back(this->add_variable(FF(read_idx))); // populate with read index as witness
-    calldata_read_counts[read_idx]++;                           // increment read count at read index
-    q_busread().emplace_back(1);                                // read selector on
-
-    // populate all other components with zero
-    this->w_o().emplace_back(this->zero_idx);
-    this->w_4().emplace_back(this->zero_idx);
-    this->q_m().emplace_back(0);
-    this->q_1().emplace_back(0);
-    this->q_2().emplace_back(0);
-    this->q_3().emplace_back(0);
-    this->q_c().emplace_back(0);
-    this->q_sort().emplace_back(0);
-    this->q_arith().emplace_back(0);
-    this->q_4().emplace_back(0);
-    this->q_lookup_type().emplace_back(0);
-    this->q_elliptic().emplace_back(0);
-    this->q_aux().emplace_back(0);
-    this->q_poseidon2_external().emplace_back(0);
-    this->q_poseidon2_internal().emplace_back(0);
-
-    ++this->num_gates;
+    // Create an arbitrary calldata read gate
+    add_public_calldata(FF(25)); // ensure there is at least one entry in calldata
+    uint32_t raw_read_idx = 0;   // read first entry in calldata
+    auto read_idx = this->add_variable(raw_read_idx);
+    FF calldata_value = this->get_variable(public_calldata[raw_read_idx]);
+    auto value_idx = this->add_variable(calldata_value);
+    create_calldata_lookup_gate({ read_idx, value_idx });
+    // TODO(https://github.com/AztecProtocol/barretenberg/issues/821): automate updating of read counts
+    calldata_read_counts[raw_read_idx]++;
 
     // mock gates that use poseidon selectors, with all zeros as input
     this->w_l().emplace_back(this->zero_idx);
@@ -254,6 +227,39 @@ template <typename FF> void GoblinUltraCircuitBuilder_<FF>::set_goblin_ecc_op_co
     equality_op_idx = this->put_constant_variable(FF(EccOpCode::EQUALITY));
 }
 
+/**
+ * @brief Create a calldata lookup/read gate
+ *
+ * @tparam FF
+ * @param databus_lookup_gate_ witness indices corresponding to: calldata index, calldata value
+ */
+template <typename FF>
+void GoblinUltraCircuitBuilder_<FF>::create_calldata_lookup_gate(const databus_lookup_gate_<FF>& in)
+{
+    this->w_l().emplace_back(in.value);
+    this->w_r().emplace_back(in.index);
+    q_busread().emplace_back(1);
+
+    // populate all other components with zero
+    this->w_o().emplace_back(this->zero_idx);
+    this->w_4().emplace_back(this->zero_idx);
+    this->q_m().emplace_back(0);
+    this->q_1().emplace_back(0);
+    this->q_2().emplace_back(0);
+    this->q_3().emplace_back(0);
+    this->q_c().emplace_back(0);
+    this->q_sort().emplace_back(0);
+    this->q_arith().emplace_back(0);
+    this->q_4().emplace_back(0);
+    this->q_lookup_type().emplace_back(0);
+    this->q_elliptic().emplace_back(0);
+    this->q_aux().emplace_back(0);
+    this->q_poseidon2_external().emplace_back(0);
+    this->q_poseidon2_internal().emplace_back(0);
+
+    ++this->num_gates;
+}
+
 template <typename FF>
 void GoblinUltraCircuitBuilder_<FF>::create_poseidon2_external_gate(const poseidon2_external_gate_<FF>& in)
 {

barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp

@@ -155,22 +155,21 @@ template <typename FF> class GoblinUltraCircuitBuilder_ : public UltraCircuitBui
     }
 
     /**
-     * Make a witness variable a member of the public calldata.
+     * @brief Add a witness variable to the public calldata.
      *
-     * @param witness_index The index of the witness.
+     * @param in Value to be added to calldata.
      * */
-    void set_public_calldata(const uint32_t witness_index)
+    uint32_t add_public_calldata(const FF& in)
     {
-        for (const uint32_t calldata : public_calldata) {
-            if (calldata == witness_index) {
-                if (!this->failed()) {
-                    this->failure("Attempted to redundantly set a public calldata!");
-                }
-                return;
-            }
-        }
-        public_calldata.emplace_back(witness_index);
+        const uint32_t index = this->add_variable(in);
+        public_calldata.emplace_back(index);
+        // Note: this is a bit inefficent to do every time but for safety these need to be coupled
+        calldata_read_counts.resize(public_calldata.size());
+        return index;
     }
+
+    void create_calldata_lookup_gate(const databus_lookup_gate_<FF>& in);
+
     void create_poseidon2_external_gate(const poseidon2_external_gate_<FF>& in);
     void create_poseidon2_internal_gate(const poseidon2_internal_gate_<FF>& in);
 

barretenberg/cpp/src/barretenberg/proof_system/instance_inspector.hpp

@@ -48,19 +48,19 @@ void inspect_instance(auto& prover_instance)
 void print_databus_info(auto& prover_instance)
 {
     info("\nInstance Inspector: Printing databus gate info.");
-    auto& prover_polys = prover_instance->prover_polynomials;
+    auto& key = prover_instance->proving_key;
     for (size_t idx = 0; idx < prover_instance->proving_key->circuit_size; ++idx) {
-        if (prover_polys.q_busread[idx] == 1) {
+        if (key->q_busread[idx] == 1) {
             info("idx = ", idx);
-            info("q_busread = ", prover_polys.q_busread[idx]);
-            info("w_l = ", prover_polys.w_l[idx]);
-            info("w_r = ", prover_polys.w_r[idx]);
+            info("q_busread = ", key->q_busread[idx]);
+            info("w_l = ", key->w_l[idx]);
+            info("w_r = ", key->w_r[idx]);
         }
-        if (prover_polys.calldata_read_counts[idx] > 0) {
+        if (key->calldata_read_counts[idx] > 0) {
             info("idx = ", idx);
-            info("read_counts = ", prover_polys.calldata_read_counts[idx]);
-            info("calldata = ", prover_polys.calldata[idx]);
-            info("databus_id = ", prover_polys.databus_id[idx]);
+            info("read_counts = ", key->calldata_read_counts[idx]);
+            info("calldata = ", key->calldata[idx]);
+            info("databus_id = ", key->databus_id[idx]);
         }
     }
     info();

barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp

@@ -195,6 +195,7 @@ void ProverInstance_<Flavor>::construct_databus_polynomials(Circuit& circuit)
     // Note: We do not utilize a zero row for databus columns
     for (size_t idx = 0; idx < circuit.public_calldata.size(); ++idx) {
         public_calldata[idx] = circuit.get_variable(circuit.public_calldata[idx]);
+        // TODO(https://github.com/AztecProtocol/barretenberg/issues/821): automate updating of read counts
         calldata_read_counts[idx] = circuit.calldata_read_counts[idx];
     }
 

barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp

@@ -3,8 +3,10 @@
 #include <gtest/gtest.h>
 
 #include "barretenberg/common/log.hpp"
+#include "barretenberg/goblin/mock_circuits.hpp"
 #include "barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp"
 #include "barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp"
+#include "barretenberg/proof_system/instance_inspector.hpp"
 #include "barretenberg/ultra_honk/ultra_composer.hpp"
 #include "barretenberg/ultra_honk/ultra_prover.hpp"
 
@@ -26,59 +28,26 @@ class DataBusComposerTests : public ::testing::Test {
     using CommitmentKey = pcs::CommitmentKey<Curve>;
 
     /**
-     * @brief Generate a simple test circuit with some ECC op gates and conventional arithmetic gates
+     * @brief Generate a simple test circuit that includes arithmetic and goblin ecc op gates
      *
      * @param builder
      */
     void generate_test_circuit(auto& builder)
     {
-        // Add some ecc op gates
-        for (size_t i = 0; i < 3; ++i) {
-            auto point = Point::one() * FF::random_element();
-            auto scalar = FF::random_element();
-            builder.queue_ecc_mul_accum(point, scalar);
-        }
-        builder.queue_ecc_eq();
-
-        // Add some conventional gates that utilize public inputs
-        for (size_t i = 0; i < 10; ++i) {
-            FF a = FF::random_element();
-            FF b = FF::random_element();
-            FF c = FF::random_element();
-            FF d = a + b + c;
-            uint32_t a_idx = builder.add_public_variable(a);
-            uint32_t b_idx = builder.add_variable(b);
-            uint32_t c_idx = builder.add_variable(c);
-            uint32_t d_idx = builder.add_variable(d);
-
-            builder.create_big_add_gate({ a_idx, b_idx, c_idx, d_idx, FF(1), FF(1), FF(1), FF(-1), FF(0) });
-        }
-    }
-
-    /**
-     * @brief Construct and a verify a Honk proof
-     *
-     */
-    bool construct_and_verify_honk_proof(auto& composer, auto& builder)
-    {
-        auto instance = composer.create_instance(builder);
-        auto prover = composer.create_prover(instance);
-        auto verifier = composer.create_verifier(instance);
-        auto proof = prover.construct_proof();
-        bool verified = verifier.verify_proof(proof);
-
-        return verified;
+        // Add some ecc op gates and arithmetic gates
+        GoblinMockCircuits::construct_goblin_ecc_op_circuit(builder);
+        GoblinMockCircuits::construct_arithmetic_circuit(builder);
     }
 };
 
 /**
- * @brief Test proof construction/verification for a circuit with ECC op gates, public inputs, and basic arithmetic
+ * @brief Test proof construction/verification for a circuit with calldata lookup gates
  * gates
  * @note We simulate op queue interactions with a previous circuit so the actual circuit under test utilizes an op queue
  * with non-empty 'previous' data. This avoid complications with zero-commitments etc.
  *
  */
-TEST_F(DataBusComposerTests, SingleCircuit)
+TEST_F(DataBusComposerTests, CallDataRead)
 {
     auto op_queue = std::make_shared<proof_system::ECCOpQueue>();
 
@@ -87,13 +56,44 @@ TEST_F(DataBusComposerTests, SingleCircuit)
 
     auto builder = proof_system::GoblinUltraCircuitBuilder{ op_queue };
 
+    // Create a general test circuit
     generate_test_circuit(builder);
 
+    // Add some values to calldata and store the corresponding witness index
+    std::array<FF, 5> calldata_values = { 7, 10, 3, 12, 1 };
+    for (auto& val : calldata_values) {
+        builder.add_public_calldata(val);
+    }
+
+    // Define some indices at which to read calldata
+    std::array<uint32_t, 2> read_index_values = { 1, 4 };
+
+    // Create some calldata lookup gates
+    for (uint32_t& read_index : read_index_values) {
+        // Create a variable corresponding to the index at which we want to read into calldata
+        uint32_t read_idx_witness_idx = builder.add_variable(read_index);
+
+        // Create a variable corresponding to the result of the read. Note that we do not in general connect reads from
+        // calldata via copy constraints (i.e. we create a unique variable for the result of each read)
+        ASSERT_LT(read_index, builder.public_calldata.size());
+        FF calldata_value = builder.get_variable(builder.public_calldata[read_index]);
+        uint32_t value_witness_idx = builder.add_variable(calldata_value);
+
+        builder.create_calldata_lookup_gate({ read_idx_witness_idx, value_witness_idx });
+        // TODO(https://github.com/AztecProtocol/barretenberg/issues/821): automate updating of read counts
+        builder.calldata_read_counts[read_index]++;
+    }
+
     auto composer = GoblinUltraComposer();
 
     // Construct and verify Honk proof
-    auto honk_verified = construct_and_verify_honk_proof(composer, builder);
-    EXPECT_TRUE(honk_verified);
+    auto instance = composer.create_instance(builder);
+    // For debugging, use "instance_inspector::print_databus_info(instance)"
+    auto prover = composer.create_prover(instance);
+    auto verifier = composer.create_verifier(instance);
+    auto proof = prover.construct_proof();
+    bool verified = verifier.verify_proof(proof);
+    EXPECT_TRUE(verified);
 }
 
 } // namespace test_ultra_honk_composer