Skip to content

Commit

Permalink
refactor: Move vk computation out of Honk Ultra composer (#4811)
Browse files Browse the repository at this point in the history
This is the first step in a series of PRs to get rid of the Honk
composer classes. The goal is to delete the `compute_verification_key`
functions from the Honk composers. Since this function uses the
commitment key, we must also change how that is initialized. We aim to
move logic into the proving key and other flavor classes.
  • Loading branch information
codygunton authored Feb 28, 2024
1 parent 29b1ea3 commit f354e89
Show file tree
Hide file tree
Showing 47 changed files with 114 additions and 167 deletions.
2 changes: 1 addition & 1 deletion barretenberg/cpp/scripts/analyze_client_ivc_bench.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
to_keep = [
"construct_mock_function_circuit(t)",
"construct_mock_folding_kernel(t)",
"UltraComposer::create_prover_instance(t)",
"ProverInstance(Circuit&)(t)",
"ProtogalaxyProver::fold_instances(t)",
"Decider::construct_proof(t)",
"ECCVMComposer::create_prover(t)",
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
barretenberg_module(ipa_bench ultra_honk)
barretenberg_module(ipa_bench commitment_schemes)
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ constexpr size_t MAX_POLYNOMIAL_DEGREE_LOG2 = 16;
std::shared_ptr<bb::srs::factories::CrsFactory<curve::Grumpkin>> crs_factory(
new bb::srs::factories::FileCrsFactory<curve::Grumpkin>("../srs_db/grumpkin", 1 << 16));

auto ck = std::make_shared<CommitmentKey<Curve>>(1 << MAX_POLYNOMIAL_DEGREE_LOG2, crs_factory);
auto ck = std::make_shared<CommitmentKey<Curve>>(1 << MAX_POLYNOMIAL_DEGREE_LOG2);
auto vk = std::make_shared<VerifierCommitmentKey<Curve>>(1 << MAX_POLYNOMIAL_DEGREE_LOG2, crs_factory);

std::vector<std::shared_ptr<NativeTranscript>> prover_transcripts(MAX_POLYNOMIAL_DEGREE_LOG2 -
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,7 @@ namespace bb {
template <typename Curve> std::shared_ptr<CommitmentKey<Curve>> create_commitment_key(const size_t num_points)
{
std::string srs_path;
if constexpr (std::same_as<Curve, curve::BN254>) {
srs_path = "../srs_db/ignition";
} else {
static_assert(std::same_as<Curve, curve::Grumpkin>);
srs_path = "../srs_db/grumpkin";
}
auto crs_factory = std::make_shared<bb::srs::factories::FileCrsFactory<Curve>>(srs_path, num_points);
return std::make_shared<CommitmentKey<Curve>>(num_points, crs_factory);
return std::make_shared<CommitmentKey<Curve>>(num_points);
}

constexpr size_t MAX_LOG_NUM_POINTS = 24;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ template <class Curve> class CommitmentKey {
using Commitment = typename Curve::AffineElement;

public:
scalar_multiplication::pippenger_runtime_state<Curve> pippenger_runtime_state;
std::shared_ptr<srs::factories::CrsFactory<Curve>> crs_factory;
std::shared_ptr<srs::factories::ProverCrs<Curve>> srs;

CommitmentKey() = delete;

/**
Expand All @@ -45,14 +49,14 @@ template <class Curve> class CommitmentKey {
* @param path
*
*/
CommitmentKey(const size_t num_points,
std::shared_ptr<bb::srs::factories::CrsFactory<Curve>> crs_factory = bb::srs::get_crs_factory())
CommitmentKey(const size_t num_points)
: pippenger_runtime_state(num_points)
, crs_factory(srs::get_crs_factory<Curve>())
, srs(crs_factory->get_prover_crs(num_points))
{}

// Note: This constructor is used only by Plonk; For Honk the srs is extracted by the CommitmentKey
CommitmentKey(const size_t num_points, std::shared_ptr<bb::srs::factories::ProverCrs<Curve>> prover_crs)
// Note: This constructor is to be used only by Plonk; For Honk the srs lives in the CommitmentKey
CommitmentKey(const size_t num_points, std::shared_ptr<srs::factories::ProverCrs<Curve>> prover_crs)
: pippenger_runtime_state(num_points)
, srs(prover_crs)
{}
Expand All @@ -68,12 +72,9 @@ template <class Curve> class CommitmentKey {
BB_OP_COUNT_TIME();
const size_t degree = polynomial.size();
ASSERT(degree <= srs->get_monomial_size());
return bb::scalar_multiplication::pippenger_unsafe<Curve>(
return scalar_multiplication::pippenger_unsafe<Curve>(
const_cast<Fr*>(polynomial.data()), srs->get_monomial_points(), degree, pippenger_runtime_state);
};

bb::scalar_multiplication::pippenger_runtime_state<Curve> pippenger_runtime_state;
std::shared_ptr<bb::srs::factories::ProverCrs<Curve>> srs;
};

} // namespace bb
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@
#include "barretenberg/polynomials/polynomial.hpp"
#include "barretenberg/srs/factories/file_crs_factory.hpp"
#include "claim.hpp"
#include <algorithm>
#include <concepts>
#include <memory>
#include <string_view>

#include <gtest/gtest.h>

Expand All @@ -19,18 +15,16 @@ template <class CK> inline std::shared_ptr<CK> CreateCommitmentKey();

template <> inline std::shared_ptr<CommitmentKey<curve::BN254>> CreateCommitmentKey<CommitmentKey<curve::BN254>>()
{
srs::init_crs_factory("../srs_db/ignition");
constexpr size_t n = 4096;
std::shared_ptr<bb::srs::factories::CrsFactory<curve::BN254>> crs_factory(
new bb::srs::factories::FileCrsFactory<curve::BN254>("../srs_db/ignition", 4096));
return std::make_shared<CommitmentKey<curve::BN254>>(n, crs_factory);
return std::make_shared<CommitmentKey<curve::BN254>>(n);
}
// For IPA
template <> inline std::shared_ptr<CommitmentKey<curve::Grumpkin>> CreateCommitmentKey<CommitmentKey<curve::Grumpkin>>()
{
srs::init_grumpkin_crs_factory("../srs_db/grumpkin");
constexpr size_t n = 4096;
std::shared_ptr<bb::srs::factories::CrsFactory<curve::Grumpkin>> crs_factory(
new bb::srs::factories::FileCrsFactory<curve::Grumpkin>("../srs_db/grumpkin", 4096));
return std::make_shared<CommitmentKey<curve::Grumpkin>>(n, crs_factory);
return std::make_shared<CommitmentKey<curve::Grumpkin>>(n);
}

template <typename CK> inline std::shared_ptr<CK> CreateCommitmentKey()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ template <> class VerifierCommitmentKey<curve::BN254> {
* @param num_points
* @param srs verifier G2 point
*/
VerifierCommitmentKey([[maybe_unused]] size_t num_points,
std::shared_ptr<bb::srs::factories::CrsFactory<Curve>> crs_factory)
VerifierCommitmentKey(
[[maybe_unused]] size_t num_points, // TODO(https://github.com/AztecProtocol/barretenberg/issues/874)
std::shared_ptr<bb::srs::factories::CrsFactory<Curve>> crs_factory)
: srs(crs_factory->get_verifier_crs())
{}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ std::shared_ptr<bb::plonk::verification_key> AcirComposer::init_verification_key

void AcirComposer::load_verification_key(bb::plonk::verification_key_data&& data)
{
verification_key_ =
std::make_shared<bb::plonk::verification_key>(std::move(data), srs::get_crs_factory()->get_verifier_crs());
verification_key_ = std::make_shared<bb::plonk::verification_key>(std::move(data),
srs::get_bn254_crs_factory()->get_verifier_crs());
}

bool AcirComposer::verify_proof(std::vector<uint8_t> const& proof)
Expand Down
2 changes: 1 addition & 1 deletion barretenberg/cpp/src/barretenberg/eccvm/eccvm_composer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ template <IsECCVMFlavor Flavor> class ECCVMComposer_ {

void compute_commitment_key(size_t circuit_size)
{
commitment_key = std::make_shared<CommitmentKey>(circuit_size, crs_factory_);
commitment_key = std::make_shared<CommitmentKey>(circuit_size);
};
};

Expand Down
4 changes: 2 additions & 2 deletions barretenberg/cpp/src/barretenberg/flavor/ecc_vm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -310,10 +310,10 @@ template <typename CycleGroup_T, typename Curve_T, typename PCS_T> class ECCVMBa
* @note TODO(Cody): Maybe multiple inheritance is the right thing here. In that case, nothing should eve
* inherit from ProvingKey.
*/
class ProvingKey : public ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>> {
class ProvingKey : public ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>, CommitmentKey> {
public:
// Expose constructors on the base class
using Base = ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>>;
using Base = ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>, CommitmentKey>;
using Base::Base;

auto get_to_be_shifted() { return ECCVMBase::get_to_be_shifted<Polynomial>(*this); }
Expand Down
17 changes: 15 additions & 2 deletions barretenberg/cpp/src/barretenberg/flavor/flavor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,15 +97,17 @@ class PrecomputedEntitiesBase {
* @tparam FF The scalar field on which we will encode our polynomial data. When instantiating, this may be extractable
* from the other template paramter.
*/
template <typename PrecomputedPolynomials, typename WitnessPolynomials>
template <typename PrecomputedPolynomials, typename WitnessPolynomials, typename CommitmentKey_>
class ProvingKey_ : public PrecomputedPolynomials, public WitnessPolynomials {
public:
using Polynomial = typename PrecomputedPolynomials::DataType;
using FF = typename Polynomial::FF;

size_t circuit_size;
bool contains_recursive_proof;
std::vector<uint32_t> recursive_proof_public_input_indices;
bb::EvaluationDomain<FF> evaluation_domain;
std::shared_ptr<CommitmentKey_> commitment_key;

std::vector<std::string> get_labels() const
{
Expand All @@ -119,8 +121,9 @@ class ProvingKey_ : public PrecomputedPolynomials, public WitnessPolynomials {
ProvingKey_() = default;
ProvingKey_(const size_t circuit_size, const size_t num_public_inputs)
{
this->commitment_key = std::make_shared<CommitmentKey_>(circuit_size + 1);
this->evaluation_domain = bb::EvaluationDomain<FF>(circuit_size, circuit_size);
PrecomputedPolynomials::circuit_size = circuit_size;
this->circuit_size = circuit_size;
this->log_circuit_size = numeric::get_msb(circuit_size);
this->num_public_inputs = num_public_inputs;
// Allocate memory for precomputed polynomials
Expand Down Expand Up @@ -148,6 +151,16 @@ template <typename PrecomputedCommitments> class VerificationKey_ : public Preco
this->log_circuit_size = numeric::get_msb(circuit_size);
this->num_public_inputs = num_public_inputs;
};
template <typename ProvingKeyPtr> VerificationKey_(const ProvingKeyPtr& proving_key)
{
this->circuit_size = proving_key->circuit_size;
this->log_circuit_size = numeric::get_msb(this->circuit_size);
this->num_public_inputs = proving_key->num_public_inputs;

for (auto [polynomial, commitment] : zip_view(proving_key->get_precomputed_polynomials(), this->get_all())) {
commitment = proving_key->commitment_key->commit(polynomial);
}
}
};

// Because of how Gemini is written, is importat to put the polynomials out in this order.
Expand Down
1 change: 1 addition & 0 deletions barretenberg/cpp/src/barretenberg/flavor/flavor.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ using namespace bb;

TEST(Flavor, Getters)
{
srs::init_crs_factory("../srs_db/ignition");
using Flavor = UltraFlavor;
using FF = Flavor::FF;
using ProvingKey = typename Flavor::ProvingKey;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -504,10 +504,10 @@ class AvmFlavor {
};

public:
class ProvingKey : public ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>> {
class ProvingKey : public ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>, CommitmentKey> {
public:
// Expose constructors on the base class
using Base = ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>>;
using Base = ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>, CommitmentKey>;
using Base::Base;

auto get_to_be_shifted()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,10 @@ class ToyFlavor {
};

public:
class ProvingKey : public ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>> {
class ProvingKey : public ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>, CommitmentKey> {
public:
// Expose constructors on the base class
using Base = ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>>;
using Base = ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>, CommitmentKey>;
using Base::Base;

auto get_to_be_shifted() { return RefArray<DataType, 0>{}; };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -895,14 +895,14 @@ class GoblinTranslatorFlavor {
* @note TODO(Cody): Maybe multiple inheritance is the right thing here. In that case, nothing should eve
* inherit from ProvingKey.
*/
class ProvingKey : public ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>> {
class ProvingKey : public ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>, CommitmentKey> {
public:
BF batching_challenge_v = { 0 };
BF evaluation_input_x = { 0 };
ProvingKey() = default;

// Expose constructors on the base class
using Base = ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>>;
using Base = ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>, CommitmentKey>;
using Base::Base;

// TODO(https://github.com/AztecProtocol/barretenberg/issues/810): get around this by properly having
Expand All @@ -919,7 +919,7 @@ class GoblinTranslatorFlavor {
}

ProvingKey(const size_t circuit_size)
: ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>>(circuit_size, 0)
: ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>, CommitmentKey>(circuit_size, 0)

, batching_challenge_v(0)
, evaluation_input_x(0)
Expand Down
5 changes: 3 additions & 2 deletions barretenberg/cpp/src/barretenberg/flavor/goblin_ultra.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 +258,10 @@ class GoblinUltraFlavor {
* @note TODO(Cody): Maybe multiple inheritance is the right thing here. In that case, nothing should eve inherit
* from ProvingKey.
*/
class ProvingKey : public ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>> {
class ProvingKey : public ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>, CommitmentKey> {
public:
// Expose constructors on the base class
using Base = ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>>;
using Base = ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>, CommitmentKey>;
using Base::Base;

std::vector<uint32_t> memory_read_records;
Expand All @@ -285,6 +285,7 @@ class GoblinUltraFlavor {
* @note Note the discrepancy with what sort of data is stored here vs in the proving key. We may want to resolve
* that, and split out separate PrecomputedPolynomials/Commitments data for clarity but also for portability of our
* circuits.
* @todo TODO(https://github.com/AztecProtocol/barretenberg/issues/876)
*/
using VerificationKey = VerificationKey_<PrecomputedEntities<Commitment>>;

Expand Down
4 changes: 2 additions & 2 deletions barretenberg/cpp/src/barretenberg/flavor/ultra.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -264,10 +264,10 @@ class UltraFlavor {
* @note TODO(Cody): Maybe multiple inheritance is the right thing here. In that case, nothing should eve inherit
* from ProvingKey.
*/
class ProvingKey : public ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>> {
class ProvingKey : public ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>, CommitmentKey> {
public:
// Expose constructors on the base class
using Base = ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>>;
using Base = ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>, CommitmentKey>;
using Base::Base;

std::vector<uint32_t> memory_read_records;
Expand Down
3 changes: 1 addition & 2 deletions barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,7 @@ class GoblinMockCircuits {
op_queue->set_size_data();

// Manually compute the op queue transcript commitments (which would normally be done by the merge prover)
auto crs_factory_ = bb::srs::get_crs_factory();
auto commitment_key = CommitmentKey(op_queue->get_current_size(), crs_factory_);
auto commitment_key = CommitmentKey(op_queue->get_current_size());
std::array<Point, Flavor::NUM_WIRES> op_queue_commitments;
size_t idx = 0;
for (auto& entry : op_queue->get_aggregate_transcript()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ void init_verification_key()
}

verification_key =
bb::plonk::compute_verification_key_common(proving_key, srs::get_crs_factory()->get_verifier_crs());
bb::plonk::compute_verification_key_common(proving_key, srs::get_bn254_crs_factory()->get_verifier_crs());
}

Prover new_join_split_prover(join_split_tx const& tx, bool mock)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ std::shared_ptr<plonk::proving_key> StandardComposer::compute_proving_key(Circui
circuit_constructor.num_gates + circuit_constructor.public_inputs.size() + NUM_RESERVED_GATES;
const size_t subgroup_size = circuit_constructor.get_circuit_subgroup_size(total_num_gates); // next power of 2

auto crs = srs::get_crs_factory()->get_prover_crs(subgroup_size + 1);
auto crs = srs::get_bn254_crs_factory()->get_prover_crs(subgroup_size + 1);
// TODO(https://github.com/AztecProtocol/barretenberg/issues/392): Composer type
circuit_proving_key = std::make_shared<plonk::proving_key>(
subgroup_size, circuit_constructor.public_inputs.size(), crs, CircuitType::STANDARD);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class StandardComposer {

bool computed_witness = false;

StandardComposer() { crs_factory_ = bb::srs::get_crs_factory(); }
StandardComposer() { crs_factory_ = bb::srs::get_bn254_crs_factory(); }
StandardComposer(std::shared_ptr<bb::srs::factories::CrsFactory<curve::BN254>> crs_factory)
: crs_factory_(std::move(crs_factory))
{}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ std::shared_ptr<proving_key> UltraComposer::compute_proving_key(CircuitBuilder&

const size_t subgroup_size = compute_dyadic_circuit_size(circuit);

auto crs = srs::get_crs_factory()->get_prover_crs(subgroup_size + 1);
auto crs = srs::get_bn254_crs_factory()->get_prover_crs(subgroup_size + 1);
// TODO(https://github.com/AztecProtocol/barretenberg/issues/392): Composer type
circuit_proving_key =
std::make_shared<plonk::proving_key>(subgroup_size, circuit.public_inputs.size(), crs, CircuitType::ULTRA);
Expand Down Expand Up @@ -209,7 +209,7 @@ std::shared_ptr<plonk::verification_key> UltraComposer::compute_verification_key
compute_proving_key(circuit_constructor);
}
circuit_verification_key =
compute_verification_key_common(circuit_proving_key, srs::get_crs_factory()->get_verifier_crs());
compute_verification_key_common(circuit_proving_key, srs::get_bn254_crs_factory()->get_verifier_crs());

circuit_verification_key->circuit_type = CircuitType::ULTRA;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ struct verification_key {
void msgpack_unpack(auto obj)
{
verification_key_data data = obj;
*this = verification_key{ std::move(data), bb::srs::get_crs_factory()->get_verifier_crs() };
*this = verification_key{ std::move(data), bb::srs::get_bn254_crs_factory()->get_verifier_crs() };
}
// Alias verification_key as verification_key_data in the schema
void msgpack_schema(auto& packer) const { packer.pack_schema(bb::plonk::verification_key_data{}); }
Expand All @@ -131,15 +131,15 @@ template <typename B> inline void read(B& buf, verification_key& key)
using serialize::read;
verification_key_data vk_data;
read(buf, vk_data);
key = verification_key{ std::move(vk_data), bb::srs::get_crs_factory()->get_verifier_crs() };
key = verification_key{ std::move(vk_data), bb::srs::get_bn254_crs_factory()->get_verifier_crs() };
}

template <typename B> inline void read(B& buf, std::shared_ptr<verification_key>& key)
{
using serialize::read;
verification_key_data vk_data;
read(buf, vk_data);
key = std::make_shared<verification_key>(std::move(vk_data), bb::srs::get_crs_factory()->get_verifier_crs());
key = std::make_shared<verification_key>(std::move(vk_data), bb::srs::get_bn254_crs_factory()->get_verifier_crs());
}

template <typename B> inline void write(B& buf, verification_key const& key)
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
barretenberg_module(proof_system relations crypto_pedersen_commitment crypto_pedersen_hash)
barretenberg_module(proof_system relations crypto_pedersen_hash srs)
Loading

0 comments on commit f354e89

Please sign in to comment.