Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ class GoblinBench : public benchmark::Fixture {
// Construct and accumulate the mock kernel circuit
// Note: in first round, kernel_accum is empty since there is no previous kernel to recursively verify
GoblinUltraCircuitBuilder circuit_builder{ goblin.op_queue };
GoblinMockCircuits::construct_mock_recursion_kernel_circuit(circuit_builder, function_accum, kernel_accum);
GoblinMockCircuits::construct_mock_recursion_kernel_circuit(
circuit_builder,
{ function_accum.proof, function_accum.verification_key },
{ kernel_accum.proof, kernel_accum.verification_key });
kernel_accum = goblin.accumulate(circuit_builder);
}
}
Expand All @@ -62,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 @@ -84,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 @@ -101,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 @@ -121,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();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a bit weird to me. I know it's not being introduced in this PR but is there some reason why we don't simply instantiate a new goblin instance for use within the scope of this function?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added to do to aim to move vk precomputaion out of client ivc

}

} // 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); }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the appropriate TODO with issue here?

/**
* @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 All @@ -58,7 +55,9 @@ TEST_F(GoblinRecursionTests, Vanilla)

// Construct and accumulate the mock kernel circuit (no kernel accum in first round)
GoblinUltraCircuitBuilder kernel_circuit{ goblin.op_queue };
GoblinMockCircuits::construct_mock_kernel_small(kernel_circuit, function_accum, kernel_accum);
GoblinMockCircuits::construct_mock_kernel_small(kernel_circuit,
{ function_accum.proof, function_accum.verification_key },
{ kernel_accum.proof, kernel_accum.verification_key });
info("kernel accum");
goblin.merge(kernel_circuit);
kernel_accum = construct_accumulator(kernel_circuit);
Expand Down
12 changes: 6 additions & 6 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 All @@ -30,13 +29,17 @@ class GoblinMockCircuits {
using Flavor = bb::GoblinUltraFlavor;
using RecursiveFlavor = bb::GoblinUltraRecursiveFlavor_<GoblinUltraBuilder>;
using RecursiveVerifier = bb::stdlib::recursion::honk::UltraRecursiveVerifier_<RecursiveFlavor>;
using KernelInput = Goblin::AccumulationOutput;
using VerifierInstance = bb::VerifierInstance_<Flavor>;
using RecursiveVerifierInstance = ::bb::stdlib::recursion::honk::RecursiveVerifierInstance_<RecursiveFlavor>;
using RecursiveVerifierAccumulator = std::shared_ptr<RecursiveVerifierInstance>;
using VerificationKey = Flavor::VerificationKey;
static constexpr size_t NUM_OP_QUEUE_COLUMNS = Flavor::NUM_WIRES;

struct KernelInput {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To mock inside Goblin I had to break the dependency between MockCircuits and Goblin (to avoid a circular dependency)

HonkProof proof;
std::shared_ptr<Flavor::VerificationKey> verification_key;
};

/**
* @brief Information required by the verifier to verify a folding round besides the previous accumulator.
*/
Expand Down Expand Up @@ -163,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,13 +39,15 @@ 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);
auto function_accum = goblin.accumulate(app_circuit);
GoblinUltraCircuitBuilder kernel_circuit{ goblin.op_queue };
GoblinMockCircuits::construct_mock_recursion_kernel_circuit(kernel_circuit, function_accum, kernel_accum);
GoblinMockCircuits::construct_mock_recursion_kernel_circuit(
kernel_circuit,
{ function_accum.proof, function_accum.verification_key },
{ kernel_accum.proof, kernel_accum.verification_key });

auto instance = std::make_shared<ProverInstance>(kernel_circuit);
if (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()

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved this at the end of the file as a private function

{
// 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