Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Move vk computation out of Honk Ultra composer #4811

Merged
merged 18 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
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);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

crs_factory now lives in commitment key.

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>) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This logic was repeated

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;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just moved these up bc data members go at the top now.

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;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Establishing that this is where this should live.


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())) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Verified by counting members that this is accomplishing the same as the particular flavor functions it replaces.

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
Loading