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 remaining data out of Honk UltraComposer #4848

Merged
merged 20 commits into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from 18 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
22 changes: 11 additions & 11 deletions barretenberg/cpp/scripts/bb-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,18 @@ IMAGE_URI=$(calculate_image_uri $REPOSITORY)
retry docker pull $IMAGE_URI

TESTS=(
client_ivc_tests
flavor_tests
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did this so that when we run CI first we run the less complex tests first, so if we segfault or fail an assertion we can see the failure farther upstream.

relations_tests
transcript_tests
commitment_schemes_tests
sumcheck_tests
eccvm_tests
translator_vm_tests
protogalaxy_tests
ultra_honk_tests
goblin_tests
client_ivc_tests
dsl_tests
crypto_aes128_tests
crypto_blake2s_tests
crypto_blake3s_tests
Expand All @@ -22,23 +32,13 @@ TESTS=(
crypto_poseidon2_tests
crypto_schnorr_tests
crypto_sha256_tests
dsl_tests
ecc_tests
eccvm_tests
flavor_tests
goblin_tests
join_split_example_proofs_inner_proof_data_tests
join_split_example_proofs_notes_tests
numeric_tests
plonk_tests
polynomials_tests
protogalaxy_tests
relations_tests
srs_tests
sumcheck_tests
transcript_tests
translator_vm_tests
ultra_honk_tests
vm_tests
)
TESTS_STR="${TESTS[@]}"
Expand Down
22 changes: 22 additions & 0 deletions barretenberg/cpp/scripts/ultra_honk_tests.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
set -eu
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simple script that's useful for running tests locally. Will delete in a future PR.


# Move above script dir.
cd $(dirname $0)/..

cmake --preset clang16
cmake --build --preset clang16

cd build/

./bin/flavor_tests
./bin/relations_tests
./bin/transcript_tests
./bin/commitment_schemes_tests
./bin/sumcheck_tests
./bin/eccvm_tests
./bin/translator_vm_tests
./bin/protogalaxy_tests
./bin/ultra_honk_tests
./bin/goblin_tests
./bin/client_ivc_tests
./bin/stdlib_recursion_tests
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,9 @@ template <>
inline std::shared_ptr<VerifierCommitmentKey<curve::BN254>> CreateVerifierCommitmentKey<
VerifierCommitmentKey<curve::BN254>>()
{
constexpr size_t n = 4096;
std::shared_ptr<bb::srs::factories::CrsFactory<curve::BN254>> crs_factory(
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a point to this line anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, idk why the compiler didn't complain.

new bb::srs::factories::FileCrsFactory<curve::BN254>("../srs_db/ignition", 4096));
return std::make_shared<VerifierCommitmentKey<curve::BN254>>(n, crs_factory);
return std::make_shared<VerifierCommitmentKey<curve::BN254>>();
}
// For IPA
template <>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@
#include "barretenberg/numeric/bitop/pow.hpp"
#include "barretenberg/polynomials/polynomial.hpp"
#include "barretenberg/polynomials/polynomial_arithmetic.hpp"
#include "barretenberg/srs/factories/crs_factory.hpp"
#include "barretenberg/srs/factories/file_crs_factory.hpp"
#include "barretenberg/srs/global_crs.hpp"

#include <cstddef>
#include <memory>
Expand All @@ -35,19 +34,13 @@ template <> class VerifierCommitmentKey<curve::BN254> {
using Commitment = typename Curve::AffineElement;

public:
VerifierCommitmentKey() = delete;
std::shared_ptr<bb::srs::factories::VerifierCrs<Curve>> srs;

/**
* @brief Construct a new Kate Verification Key object from existing SRS
*
* @param num_points
* @param srs verifier G2 point
*/
VerifierCommitmentKey(
[[maybe_unused]] size_t num_points, // TODO(https://github.com/AztecProtocol/barretenberg/issues/874)
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 PR doesn't resolve all of 874.

Copy link
Contributor

Choose a reason for hiding this comment

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

leave it as a TODO then?

std::shared_ptr<bb::srs::factories::CrsFactory<Curve>> crs_factory)
: srs(crs_factory->get_verifier_crs())
{}
VerifierCommitmentKey()
{
srs::init_crs_factory("../srs_db/ignition");
srs = srs::get_crs_factory<Curve>()->get_verifier_crs();
};

/**
* @brief verifies a pairing equation over 2 points using the verifier SRS
Expand All @@ -65,8 +58,6 @@ template <> class VerifierCommitmentKey<curve::BN254> {

return (result == Curve::TargetField::one());
}

std::shared_ptr<bb::srs::factories::VerifierCrs<Curve>> srs;
};

/**
Expand Down
2 changes: 1 addition & 1 deletion barretenberg/cpp/src/barretenberg/flavor/ecc_vm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ template <typename CycleGroup_T, typename Curve_T, typename PCS_T> class ECCVMBa
* resolve that, and split out separate PrecomputedPolynomials/Commitments data for clarity but also for
* portability of our circuits.
*/
using VerificationKey = VerificationKey_<PrecomputedEntities<Commitment>>;
using VerificationKey = VerificationKey_<PrecomputedEntities<Commitment>, VerifierCommitmentKey>;

/**
* @brief A container for polynomials produced after the first round of sumcheck.
Expand Down
8 changes: 7 additions & 1 deletion barretenberg/cpp/src/barretenberg/flavor/flavor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
#include "barretenberg/polynomials/univariate.hpp"
#include "barretenberg/proof_system/types/circuit_type.hpp"
#include <array>
#include <barretenberg/srs/global_crs.hpp>
#include <concepts>
#include <vector>

Expand Down Expand Up @@ -142,20 +143,25 @@ class ProvingKey_ : public PrecomputedPolynomials, public WitnessPolynomials {
*
* @tparam PrecomputedEntities An instance of PrecomputedEntities_ with affine_element data type and handle type.
*/
template <typename PrecomputedCommitments> class VerificationKey_ : public PrecomputedCommitments {
template <typename PrecomputedCommitments, typename VerifierCommitmentKey>
class VerificationKey_ : public PrecomputedCommitments {
public:
std::shared_ptr<VerifierCommitmentKey> pcs_verification_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.

Core change


VerificationKey_() = default;
VerificationKey_(const size_t circuit_size, const size_t num_public_inputs)
{
this->circuit_size = circuit_size;
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;
this->pcs_verification_key = std::make_shared<VerifierCommitmentKey>();

for (auto [polynomial, commitment] : zip_view(proving_key->get_precomputed_polynomials(), this->get_all())) {
commitment = proving_key->commitment_key->commit(polynomial);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ class AvmFlavor {
RefArray<Polynomial, 0> get_table_column_wires() { return {}; };
};

using VerificationKey = VerificationKey_<PrecomputedEntities<Commitment>>;
using VerificationKey = VerificationKey_<PrecomputedEntities<Commitment>, VerifierCommitmentKey>;

using FoldedPolynomials = AllEntities<std::vector<FF>>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ class ToyFlavor {
RefArray<Polynomial, 0> get_table_column_wires() { return {}; };
};

using VerificationKey = VerificationKey_<PrecomputedEntities<Commitment>>;
using VerificationKey = VerificationKey_<PrecomputedEntities<Commitment>, VerifierCommitmentKey>;

using FoldedPolynomials = AllEntities<std::vector<FF>>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -934,7 +934,7 @@ class GoblinTranslatorFlavor {
* resolve that, and split out separate PrecomputedPolynomials/Commitments data for clarity but also for
* portability of our circuits.
*/
using VerificationKey = VerificationKey_<PrecomputedEntities<Commitment>>;
using VerificationKey = VerificationKey_<PrecomputedEntities<Commitment>, VerifierCommitmentKey>;

/**
* @brief A field element for each entity of the flavor. These entities represent the prover polynomials
Expand Down
2 changes: 1 addition & 1 deletion barretenberg/cpp/src/barretenberg/flavor/goblin_ultra.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ class GoblinUltraFlavor {
* circuits.
* @todo TODO(https://github.com/AztecProtocol/barretenberg/issues/876)
*/
using VerificationKey = VerificationKey_<PrecomputedEntities<Commitment>>;
using VerificationKey = VerificationKey_<PrecomputedEntities<Commitment>, VerifierCommitmentKey>;

/**
* @brief A container for storing the partially evaluated multivariates produced by sumcheck.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,6 @@
#include "barretenberg/polynomials/polynomial.hpp"
#include "barretenberg/polynomials/univariate.hpp"
#include "barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp"
#include "barretenberg/srs/factories/crs_factory.hpp"
#include <array>
#include <concepts>
#include <span>
#include <string>
#include <type_traits>
#include <vector>

#include "barretenberg/stdlib/primitives/curves/bn254.hpp"
#include "barretenberg/stdlib/primitives/field/field.hpp"
#include "barretenberg/stdlib/recursion/honk/transcript/transcript.hpp"
Expand Down Expand Up @@ -104,7 +96,8 @@ template <typename BuilderType> class GoblinUltraRecursiveFlavor_ {
* circuits.
* This differs from GoblinUltra in how we construct the commitments.
*/
class VerificationKey : public VerificationKey_<GoblinUltraFlavor::PrecomputedEntities<Commitment>> {
class VerificationKey
: public VerificationKey_<GoblinUltraFlavor::PrecomputedEntities<Commitment>, VerifierCommitmentKey> {
public:
VerificationKey(const size_t circuit_size, const size_t num_public_inputs)
{
Expand Down
2 changes: 1 addition & 1 deletion barretenberg/cpp/src/barretenberg/flavor/ultra.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ class UltraFlavor {
* that, and split out separate PrecomputedPolynomials/Commitments data for clarity but also for portability of our
* circuits.
*/
using VerificationKey = VerificationKey_<PrecomputedEntities<Commitment>>;
using VerificationKey = VerificationKey_<PrecomputedEntities<Commitment>, VerifierCommitmentKey>;

/**
* @brief A field element for each entity of the flavor. These entities represent the prover polynomials
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ template <typename BuilderType> class UltraRecursiveFlavor_ {
* that, and split out separate PrecomputedPolynomials/Commitments data for clarity but also for portability of our
* circuits.
*/
class VerificationKey : public VerificationKey_<PrecomputedEntities<Commitment>> {
class VerificationKey : public VerificationKey_<PrecomputedEntities<Commitment>, VerifierCommitmentKey> {
public:
VerificationKey(const size_t circuit_size, const size_t num_public_inputs)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,10 @@ namespace bb {
* */
template <IsUltraFlavor Flavor>
DeciderProver_<Flavor>::DeciderProver_(const std::shared_ptr<Instance>& inst,
const std::shared_ptr<CommitmentKey>& commitment_key,
const std::shared_ptr<Transcript>& transcript)
: accumulator(std::move(inst))
, transcript(transcript)
, commitment_key(commitment_key)
, commitment_key(inst->commitment_key)
{}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ template <IsUltraFlavor Flavor> class DeciderProver_ {

public:
explicit DeciderProver_(const std::shared_ptr<Instance>&,
const std::shared_ptr<CommitmentKey>&,
const std::shared_ptr<Transcript>& transcript = std::make_shared<Transcript>());

BB_PROFILE void execute_relation_check_rounds();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ template <typename Flavor>
DeciderVerifier_<Flavor>::DeciderVerifier_(const std::shared_ptr<Transcript>& transcript,
const std::shared_ptr<VerifierInstance>& accumulator)
: accumulator(accumulator)
, pcs_verification_key(accumulator->verification_key->pcs_verification_key)
, transcript(transcript)
{}

template <typename Flavor>
DeciderVerifier_<Flavor>::DeciderVerifier_()
: pcs_verification_key(std::make_unique<VerifierCommitmentKey>(0, bb::srs::get_bn254_crs_factory()))
: pcs_verification_key(std::make_unique<VerifierCommitmentKey>())
, transcript(std::make_shared<Transcript>())
{}

Expand Down Expand Up @@ -52,7 +54,7 @@ template <typename Flavor> bool DeciderVerifier_<Flavor>::verify_proof(const Hon
multivariate_challenge,
transcript);

auto verified = pcs_verification_key->pairing_check(pairing_points[0], pairing_points[1]);
auto verified = accumulator->pcs_verification_key->pairing_check(pairing_points[0], pairing_points[1]);

return sumcheck_verified.value() && verified;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ namespace bb {
template <typename Flavor> class DeciderVerifier_ {
using FF = typename Flavor::FF;
using Commitment = typename Flavor::Commitment;
using CommitmentKey = typename Flavor::CommitmentKey;
Copy link
Contributor

Choose a reason for hiding this comment

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

seems useless?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed

using VerificationKey = typename Flavor::VerificationKey;
using VerifierCommitmentKey = typename Flavor::VerifierCommitmentKey;
using Transcript = typename Flavor::Transcript;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,12 @@ std::shared_ptr<typename ProverInstances::Instance> ProtoGalaxyProver_<ProverIns
auto vanishing_polynomial_at_challenge = challenge * (challenge - FF(1));
std::vector<FF> lagranges{ FF(1) - challenge, challenge };

// TODO(https://github.com/AztecProtocol/barretenberg/issues/881): bad pattern
auto next_accumulator = std::make_shared<Instance>();
next_accumulator->is_accumulator = true;
next_accumulator->instance_size = instances[0]->instance_size;
next_accumulator->log_instance_size = instances[0]->log_instance_size;
next_accumulator->commitment_key = instances[0]->commitment_key;

// Compute the next target sum and send the next folding parameters to the verifier
FF next_target_sum =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
namespace bb {
template <class ProverInstances_> struct ProtogalaxyProofConstructionState {
using FF = typename ProverInstances_::FF;
using Instance = typename ProverInstances_::Instance;
using ProverInstance = typename ProverInstances_::Instance;
Copy link
Contributor

Choose a reason for hiding this comment

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

why keep both ProverInstance and Instance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Trying to keep foot print small esp since all of this is going to get updated, but changed, it was easy.

using Instance = ProverInstance;

std::shared_ptr<Instance> accumulator;
Polynomial<FF> perturbator;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,8 @@ template <typename BuilderType> class GoblinRecursiveVerifierTest : public testi
// verifier and check that the result agrees.
auto native_verifier = inner_composer.create_verifier(instance->verification_key);
auto native_result = native_verifier.verify_proof(inner_proof);
auto recursive_result = native_verifier.pcs_verification_key->pairing_check(pairing_points[0].get_value(),
pairing_points[1].get_value());
auto recursive_result = native_verifier.key->pcs_verification_key->pairing_check(pairing_points[0].get_value(),
pairing_points[1].get_value());
EXPECT_EQ(recursive_result, native_result);

// Check 2: Ensure that the underlying native and recursive verification algorithms agree by ensuring
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ class RecursiveMergeVerifierTest : public testing::Test {
GoblinMockCircuits::construct_simple_initial_circuit(sample_circuit);

// Generate a proof over the inner circuit
InnerComposer inner_composer;
MergeProver merge_prover{ op_queue };
auto merge_proof = merge_prover.construct_proof();

Expand All @@ -65,7 +64,7 @@ class RecursiveMergeVerifierTest : public testing::Test {
// verifier and check that the result agrees.
MergeVerifier native_verifier;
bool verified_native = native_verifier.verify_proof(merge_proof);
VerifierCommitmentKey pcs_verification_key(0, srs::get_bn254_crs_factory());
VerifierCommitmentKey pcs_verification_key;
auto verified_recursive =
pcs_verification_key.pairing_check(pairing_points[0].get_value(), pairing_points[1].get_value());
EXPECT_EQ(verified_native, verified_recursive);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ template <typename RecursiveFlavor> class ProtoGalaxyRecursiveTests : public tes
// decider verifier and check that the result agrees.
DeciderVerifier native_decider_verifier = composer.create_decider_verifier(verifier_accumulator);
auto native_result = native_decider_verifier.verify_proof(decider_proof);
auto recursive_result = native_decider_verifier.pcs_verification_key->pairing_check(
auto recursive_result = native_decider_verifier.accumulator->pcs_verification_key->pairing_check(
pairing_points[0].get_value(), pairing_points[1].get_value());
EXPECT_EQ(native_result, recursive_result);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,8 @@ template <typename BuilderType> class RecursiveVerifierTest : public testing::Te
// verifier and check that the result agrees.
auto native_verifier = inner_composer.create_verifier(instance->verification_key);
auto native_result = native_verifier.verify_proof(inner_proof);
auto recursive_result = native_verifier.pcs_verification_key->pairing_check(pairing_points[0].get_value(),
pairing_points[1].get_value());
auto recursive_result = native_verifier.key->pcs_verification_key->pairing_check(pairing_points[0].get_value(),
pairing_points[1].get_value());
EXPECT_EQ(recursive_result, native_result);

// Check 2: Ensure that the underlying native and recursive verification algorithms agree by ensuring
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

namespace bb {

template <typename Flavor_, size_t NUM_> struct ProverInstances_ {
template <typename Flavor_, size_t NUM_ = 2> struct ProverInstances_ {
public:
static_assert(NUM_ > 0, "Must have at least one prover instance");
static_assert(NUM_ > 1, "Must have at least two prover instances");
using Flavor = Flavor_;
using FF = typename Flavor::FF;
static constexpr size_t NUM = NUM_;
Expand Down Expand Up @@ -84,7 +84,8 @@ template <typename Flavor_, size_t NUM_> struct ProverInstances_ {
}
};

template <typename Flavor_, size_t NUM_> struct VerifierInstances_ {
template <typename Flavor_, size_t NUM_ = 2> struct VerifierInstances_ {
static_assert(NUM_ > 1, "Must have at least two prover instances");
using Flavor = Flavor_;
using VerificationKey = typename Flavor::VerificationKey;
using Instance = VerifierInstance_<Flavor>;
Expand Down
Loading
Loading