Skip to content

Commit

Permalink
ensure first op queue mocking is internal to the Goblin and not done …
Browse files Browse the repository at this point in the history
…in benchmarks; make it unique function
  • Loading branch information
maramihali committed Mar 11, 2024
1 parent 9d67a9a commit 82b6428
Show file tree
Hide file tree
Showing 12 changed files with 30 additions and 104 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,6 @@ BENCHMARK_DEFINE_F(GoblinBench, GoblinFull)(benchmark::State& state)
{
Goblin goblin;

// TODO(https://github.com/AztecProtocol/barretenberg/issues/723): Simply populate the OpQueue with some data
// and corresponding commitments so the merge protocol has "prev" data into which it can accumulate
GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(goblin.op_queue);

for (auto _ : state) {
BB_REPORT_OP_COUNT_IN_BENCH(state);
// Perform a specified number of iterations of function/kernel accumulation
Expand All @@ -87,9 +83,6 @@ BENCHMARK_DEFINE_F(GoblinBench, GoblinAccumulate)(benchmark::State& state)
{
Goblin goblin;

// TODO(https://github.com/AztecProtocol/barretenberg/issues/723)
GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(goblin.op_queue);

// Perform a specified number of iterations of function/kernel accumulation
for (auto _ : state) {
perform_goblin_accumulation_rounds(state, goblin);
Expand All @@ -104,9 +97,6 @@ BENCHMARK_DEFINE_F(GoblinBench, GoblinECCVMProve)(benchmark::State& state)
{
Goblin goblin;

// TODO(https://github.com/AztecProtocol/barretenberg/issues/723)
GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(goblin.op_queue);

// Perform a specified number of iterations of function/kernel accumulation
perform_goblin_accumulation_rounds(state, goblin);

Expand All @@ -124,9 +114,6 @@ BENCHMARK_DEFINE_F(GoblinBench, GoblinTranslatorProve)(benchmark::State& state)
{
Goblin goblin;

// TODO(https://github.com/AztecProtocol/barretenberg/issues/723)
GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(goblin.op_queue);

// Perform a specified number of iterations of function/kernel accumulation
perform_goblin_accumulation_rounds(state, goblin);

Expand Down
12 changes: 2 additions & 10 deletions barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,6 @@

namespace bb {

ClientIVC::ClientIVC()
{
// TODO(https://github.com/AztecProtocol/barretenberg/issues/723):
GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(goblin.op_queue);
}

/**
* @brief Initialize the IVC with a first circuit
* @details Initializes the accumulator and performs the initial goblin merge
Expand Down Expand Up @@ -125,10 +119,8 @@ void ClientIVC::precompute_folding_verification_keys()

vks.kernel_vk = std::make_shared<VerificationKey>(prover_instance->proving_key);

// Clean the ivc state
goblin.op_queue = std::make_shared<Goblin::OpQueue>();
goblin.merge_proof_exists = false;
GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(goblin.op_queue);
// Clean the Goblin state (reinitialise op_queue with mocking and clear merge proofs)
goblin = Goblin();
}

} // namespace bb
2 changes: 0 additions & 2 deletions barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ class ClientIVC {
// be needed in the real IVC as they are provided as inputs
std::shared_ptr<ProverInstance> prover_instance;

ClientIVC();

void initialize(ClientCircuit& circuit);

FoldProof accumulate(ClientCircuit& circuit);
Expand Down
2 changes: 2 additions & 0 deletions barretenberg/cpp/src/barretenberg/goblin/goblin.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include "barretenberg/eccvm/eccvm_composer.hpp"
#include "barretenberg/flavor/goblin_ultra.hpp"
#include "barretenberg/goblin/mock_circuits.hpp"
#include "barretenberg/proof_system/circuit_builder/eccvm/eccvm_circuit_builder.hpp"
#include "barretenberg/proof_system/circuit_builder/goblin_translator_circuit_builder.hpp"
#include "barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp"
Expand Down Expand Up @@ -88,6 +89,7 @@ class Goblin {
AccumulationOutput accumulator; // Used only for ACIR methods for now

public:
Goblin() { GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue); }
/**
* @brief Construct a GUH proof and a merge proof for the present circuit.
* @details If there is a previous merge proof, recursively verify it.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,6 @@ TEST_F(GoblinRecursionTests, Vanilla)

Goblin::AccumulationOutput kernel_accum;

// TODO(https://github.com/AztecProtocol/barretenberg/issues/723):
GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(goblin.op_queue);

size_t NUM_CIRCUITS = 2;
for (size_t circuit_idx = 0; circuit_idx < NUM_CIRCUITS; ++circuit_idx) {

Expand Down
6 changes: 1 addition & 5 deletions barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include "barretenberg/crypto/merkle_tree/memory_store.hpp"
#include "barretenberg/crypto/merkle_tree/merkle_tree.hpp"
#include "barretenberg/flavor/goblin_ultra.hpp"
#include "barretenberg/goblin/goblin.hpp"
#include "barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp"
#include "barretenberg/srs/global_crs.hpp"
#include "barretenberg/stdlib/encryption/ecdsa/ecdsa.hpp"
Expand Down Expand Up @@ -167,11 +166,8 @@ class GoblinMockCircuits {
*
* @param builder
*/
static void construct_simple_initial_circuit(GoblinUltraBuilder& builder)
static void construct_simple_circuit(GoblinUltraBuilder& builder)
{
// TODO(https://github.com/AztecProtocol/barretenberg/issues/800) Testing cleanup
perform_op_queue_interactions_for_mock_first_circuit(builder.op_queue);

// Add some arbitrary ecc op gates
for (size_t i = 0; i < 3; ++i) {
auto point = Point::random_element();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ TEST_F(MockCircuits, PinRecursionKernelSizes)
const auto run_test = [](bool large) {
{
Goblin goblin;
GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(goblin.op_queue);
Goblin::AccumulationOutput kernel_accum;
GoblinUltraCircuitBuilder app_circuit{ goblin.op_queue };
GoblinMockCircuits::construct_mock_function_circuit(app_circuit, large);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,30 +166,6 @@ class ECCOpQueue {
return result;
}

/**
* @brief TESTING PURPOSES ONLY: Populate ECC op queue with mock data as stand in for "previous circuit" in tests
* @details TODO(#723): We currently cannot support Goblin proofs (specifically, transcript aggregation) if there
* is not existing data in the ECC op queue (since this leads to zero-commitment issues). This method populates the
* op queue with mock data so that the prover of an arbitrary 'first' circuit can behave as if it were not the
* prover over the first circuit in the stack. This method should be removed entirely once this is resolved.
*
* @param op_queue
*/
void populate_with_mock_initital_data()
{
// Add a single row of data to the op queue and commit to each column as [1] * FF(data)
std::array<Point, 4> mock_op_queue_commitments;
for (size_t idx = 0; idx < 4; idx++) {
auto mock_data = Fr::random_element();
this->ultra_ops[idx].emplace_back(mock_data);
mock_op_queue_commitments[idx] = Point::one() * mock_data;
}
// Set some internal data based on the size of the op queue data
this->set_size_data();
// Add the commitments to the op queue data for use by the next circuit
this->set_commitment_data(mock_op_queue_commitments);
}

/**
* @brief Write point addition op to queue and natively perform addition
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,13 @@ template <typename OuterFlavor> class GoblinRecursiveVerifierTest : public testi

// Instantiate ECC op queue and add mock data to simulate interaction with a previous circuit
auto op_queue = std::make_shared<ECCOpQueue>();
op_queue->populate_with_mock_initital_data();

InnerBuilder builder(op_queue);
// Add a mul accum op and an equality op
auto p = point::one() * fr::random_element();
auto scalar = fr::random_element();
builder.queue_ecc_mul_accum(p, scalar);
builder.queue_ecc_eq();

// Create 2^log_n many add gates based on input log num gates
const size_t num_gates = 1 << log_num_gates;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
#include "barretenberg/ultra_honk/merge_verifier.hpp"
#include "barretenberg/common/test.hpp"
#include "barretenberg/goblin/mock_circuits.hpp"
#include "barretenberg/stdlib/primitives/curves/bn254.hpp"
#include "barretenberg/stdlib/recursion/honk/verifier/merge_recursive_verifier.hpp"
#include "barretenberg/ultra_honk/merge_prover.hpp"
#include "barretenberg/ultra_honk/ultra_prover.hpp"
#include "barretenberg/ultra_honk/ultra_verifier.hpp"

namespace bb::stdlib::recursion::goblin {

Expand Down Expand Up @@ -42,9 +46,11 @@ class RecursiveMergeVerifierTest : public testing::Test {
static void test_recursive_merge_verification()
{
auto op_queue = std::make_shared<ECCOpQueue>();
// TODO(https://github.com/AztecProtocol/barretenberg/issues/800) Testing cleanup
GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue);

InnerBuilder sample_circuit{ op_queue };
GoblinMockCircuits::construct_simple_initial_circuit(sample_circuit);
GoblinMockCircuits::construct_simple_circuit(sample_circuit);

// Generate a proof over the inner circuit
MergeProver merge_prover{ op_queue };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include "barretenberg/proof_system/instance_inspector.hpp"

#include "barretenberg/ultra_honk/ultra_prover.hpp"
#include "barretenberg/ultra_honk/ultra_verifier.hpp"

using namespace bb;

namespace {
Expand Down Expand Up @@ -49,7 +51,7 @@ TEST_F(DataBusComposerTests, CallDataRead)
auto op_queue = std::make_shared<bb::ECCOpQueue>();

// Add mock data to op queue to simulate interaction with a previous circuit
op_queue->populate_with_mock_initital_data();
GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue);

auto builder = GoblinUltraCircuitBuilder{ op_queue };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#include <gtest/gtest.h>

#include "barretenberg/common/log.hpp"
#include "barretenberg/goblin/mock_circuits.hpp"
#include "barretenberg/proof_system/circuit_builder/eccvm/eccvm_circuit_builder.hpp"
#include "barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp"
#include "barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp"
#include "barretenberg/ultra_honk/merge_prover.hpp"
Expand All @@ -26,36 +28,6 @@ class GoblinUltraHonkComposerTests : public ::testing::Test {
using MergeProver = MergeProver_<GoblinUltraFlavor>;
using MergeVerifier = MergeVerifier_<GoblinUltraFlavor>;

/**
* @brief Generate a simple test circuit with some ECC op gates and conventional arithmetic 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
*
Expand Down Expand Up @@ -99,12 +71,10 @@ TEST_F(GoblinUltraHonkComposerTests, SingleCircuit)
{
auto op_queue = std::make_shared<bb::ECCOpQueue>();

// Add mock data to op queue to simulate interaction with a previous circuit
op_queue->populate_with_mock_initital_data();

GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue);
auto builder = GoblinUltraCircuitBuilder{ op_queue };

generate_test_circuit(builder);
GoblinMockCircuits::construct_simple_circuit(builder);

// Construct and verify Honk proof
bool honk_verified = construct_and_verify_honk_proof(builder);
Expand All @@ -125,15 +95,14 @@ TEST_F(GoblinUltraHonkComposerTests, MultipleCircuitsMergeOnly)
// Instantiate EccOpQueue. This will be shared across all circuits in the series
auto op_queue = std::make_shared<bb::ECCOpQueue>();

// Add mock data to op queue to simulate interaction with a previous circuit
op_queue->populate_with_mock_initital_data();
GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue);

// Construct multiple test circuits that share an ECC op queue. Generate and verify a proof for each.
size_t NUM_CIRCUITS = 3;
for (size_t i = 0; i < NUM_CIRCUITS; ++i) {
auto builder = GoblinUltraCircuitBuilder{ op_queue };

generate_test_circuit(builder);
GoblinMockCircuits::construct_simple_circuit(builder);

// Construct and verify Goblin ECC op queue Merge proof
auto merge_verified = construct_and_verify_merge_proof(op_queue);
Expand All @@ -151,15 +120,14 @@ TEST_F(GoblinUltraHonkComposerTests, MultipleCircuitsHonkOnly)
// Instantiate EccOpQueue. This will be shared across all circuits in the series
auto op_queue = std::make_shared<bb::ECCOpQueue>();

// Add mock data to op queue to simulate interaction with a previous circuit
op_queue->populate_with_mock_initital_data();
GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue);

// Construct multiple test circuits that share an ECC op queue. Generate and verify a proof for each.
size_t NUM_CIRCUITS = 3;
for (size_t i = 0; i < NUM_CIRCUITS; ++i) {
auto builder = GoblinUltraCircuitBuilder{ op_queue };

generate_test_circuit(builder);
GoblinMockCircuits::construct_simple_circuit(builder);

// Construct and verify Honk proof
bool honk_verified = construct_and_verify_honk_proof(builder);
Expand All @@ -177,15 +145,14 @@ TEST_F(GoblinUltraHonkComposerTests, MultipleCircuitsHonkAndMerge)
// Instantiate EccOpQueue. This will be shared across all circuits in the series
auto op_queue = std::make_shared<bb::ECCOpQueue>();

// Add mock data to op queue to simulate interaction with a previous circuit
op_queue->populate_with_mock_initital_data();
GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue);

// Construct multiple test circuits that share an ECC op queue. Generate and verify a proof for each.
size_t NUM_CIRCUITS = 3;
for (size_t i = 0; i < NUM_CIRCUITS; ++i) {
auto builder = GoblinUltraCircuitBuilder{ op_queue };

generate_test_circuit(builder);
GoblinMockCircuits::construct_simple_circuit(builder);

// Construct and verify Honk proof
bool honk_verified = construct_and_verify_honk_proof(builder);
Expand Down

0 comments on commit 82b6428

Please sign in to comment.