diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.cpp index b845a817bb0..ba6addf953e 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.cpp @@ -35,11 +35,7 @@ template void GoblinUltraCircuitBuilder_::add_gates_to_ensure_ 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]++; + read_calldata(read_idx); // mock a poseidon external gate, with all zeros as input this->blocks.poseidon_external.populate_wires(this->zero_idx, this->zero_idx, this->zero_idx, this->zero_idx); @@ -220,6 +216,33 @@ template void GoblinUltraCircuitBuilder_::set_goblin_ecc_op_co equality_op_idx = this->put_constant_variable(FF(EccOpCode::EQUALITY)); } +/** + * @brief Read from calldata + * @details Creates a calldata lookup gate based on the read data + * + * @tparam FF + * @param read_idx_witness_idx Variable index of the read index + * @return uint32_t Variable index of the result of the read + */ +template uint32_t GoblinUltraCircuitBuilder_::read_calldata(const uint32_t& read_idx_witness_idx) +{ + // Get the raw index into the calldata + const uint32_t read_idx = static_cast(uint256_t(this->get_variable(read_idx_witness_idx))); + + // Ensure that the read index is valid + ASSERT(read_idx < public_calldata.size()); + + // 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) + FF calldata_value = this->get_variable(public_calldata[read_idx]); + uint32_t value_witness_idx = this->add_variable(calldata_value); + + create_calldata_read_gate({ read_idx_witness_idx, value_witness_idx }); + calldata_read_counts[read_idx]++; + + return value_witness_idx; +} + /** * @brief Create a calldata lookup/read gate * @@ -227,7 +250,7 @@ template void GoblinUltraCircuitBuilder_::set_goblin_ecc_op_co * @param databus_lookup_gate_ witness indices corresponding to: calldata index, calldata value */ template -void GoblinUltraCircuitBuilder_::create_calldata_lookup_gate(const databus_lookup_gate_& in) +void GoblinUltraCircuitBuilder_::create_calldata_read_gate(const databus_lookup_gate_& in) { auto& block = this->blocks.busread; block.populate_wires(in.value, in.index, this->zero_idx, this->zero_idx); diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp index 28bda7e6f6e..75227069b2a 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp @@ -42,6 +42,7 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui void populate_ecc_op_wires(const ecc_op_tuple& in); ecc_op_tuple decompose_ecc_operands(uint32_t op, const g1::affine_element& point, const FF& scalar = FF::zero()); void set_goblin_ecc_op_code_constant_variables(); + void create_calldata_read_gate(const databus_lookup_gate_& in); public: GoblinUltraCircuitBuilder_(const size_t size_hint = 0, @@ -136,7 +137,7 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui return index; } - void create_calldata_lookup_gate(const databus_lookup_gate_& in); + uint32_t read_calldata(const uint32_t& read_idx_witness_idx); void create_poseidon2_external_gate(const poseidon2_external_gate_& in); void create_poseidon2_internal_gate(const poseidon2_internal_gate_& in); diff --git a/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp b/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp index 7f04ee8401c..b8119a5f06b 100644 --- a/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp +++ b/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp @@ -48,7 +48,6 @@ void ProverInstance_::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]; } diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp index 72037d8c5dd..6c2452e2057 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp @@ -23,19 +23,6 @@ class DataBusComposerTests : public ::testing::Test { using Curve = curve::BN254; using FF = Curve::ScalarField; - using Point = Curve::AffineElement; - using CommitmentKey = bb::CommitmentKey; - - /** - * @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 and arithmetic gates - GoblinMockCircuits::construct_simple_circuit(builder); - } }; /** @@ -49,39 +36,36 @@ TEST_F(DataBusComposerTests, CallDataRead) { auto op_queue = std::make_shared(); - // Add mock data to op queue to simulate interaction with a previous circuit - GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue); - auto builder = GoblinUltraCircuitBuilder{ op_queue }; - // Create a general test circuit - generate_test_circuit(builder); + // Add some ecc op gates and arithmetic gates + GoblinMockCircuits::construct_simple_circuit(builder); - // Add some values to calldata and store the corresponding witness index - std::array calldata_values = { 7, 10, 3, 12, 1 }; + // Add some values to calldata + std::vector 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 read_index_values = { 1, 4 }; + // Define some raw indices at which to read calldata (these will be ASSERTed to be valid) + std::vector read_indices = { 1, 4 }; - // Create some calldata lookup gates - for (uint32_t& read_index : read_index_values) { + // Create some calldata read gates. (Normally we'd use the result of the read. Example of that is below) + for (uint32_t& read_idx : read_indices) { // 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); + uint32_t read_idx_witness_idx = builder.add_variable(read_idx); - // 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]++; + builder.read_calldata(read_idx_witness_idx); } + // In general we'll want to use the result of a calldata read in another operation. Here's an example using + // an add gate to show that the result of the read is as expected: + uint32_t read_idx = 2; + FF expected_result = calldata_values[read_idx]; + uint32_t read_idx_witness_idx = builder.add_variable(read_idx); + uint32_t result_witness_idx = builder.read_calldata(read_idx_witness_idx); + builder.create_add_gate({ result_witness_idx, builder.zero_idx, builder.zero_idx, 1, 0, 0, -expected_result }); + // Construct and verify Honk proof auto instance = std::make_shared>(builder); // For debugging, use "instance_inspector::print_databus_info(instance)"