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

chore: reorganise translator proving key construction #10853

Merged
merged 5 commits into from
Dec 23, 2024
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 @@ -8,7 +8,6 @@
#include "barretenberg/ecc/curves/bn254/fr.hpp"
#include "barretenberg/ecc/curves/grumpkin/grumpkin.hpp"
#include "barretenberg/honk/proof_system/logderivative_library.hpp"
#include "barretenberg/honk/proof_system/permutation_library.hpp"
#include "barretenberg/polynomials/polynomial.hpp"
#include "barretenberg/relations/relation_parameters.hpp"
#include "barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.hpp"
Expand Down
11 changes: 8 additions & 3 deletions barretenberg/cpp/src/barretenberg/goblin/goblin.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class GoblinProver {
using TranslationEvaluations = ECCVMProver::TranslationEvaluations;
using TranslatorBuilder = bb::TranslatorCircuitBuilder;
using TranslatorProver = bb::TranslatorProver;
using TranslatorProvingKey = bb::TranslatorFlavor::ProvingKey;
using TranslatorProvingKey = bb::TranslatorProvingKey;
maramihali marked this conversation as resolved.
Show resolved Hide resolved
using RecursiveMergeVerifier = bb::stdlib::recursion::goblin::MergeRecursiveVerifier_<MegaCircuitBuilder>;
using PairingPoints = RecursiveMergeVerifier::PairingPoints;
using MergeProver = bb::MergeProver_<MegaFlavor>;
Expand All @@ -60,13 +60,17 @@ class GoblinProver {
bool merge_proof_exists{ false };

std::shared_ptr<ECCVMProvingKey> get_eccvm_proving_key() const { return eccvm_key; }
std::shared_ptr<TranslatorProvingKey> get_translator_proving_key() const { return translator_prover->key; }
std::shared_ptr<TranslatorProvingKey::ProvingKey> get_translator_proving_key() const
{
return translator_key->proving_key;
}

private:
// TODO(https://github.com/AztecProtocol/barretenberg/issues/798) unique_ptr use is a hack
std::unique_ptr<TranslatorProver> translator_prover;
std::unique_ptr<ECCVMProver> eccvm_prover;
std::shared_ptr<ECCVMProvingKey> eccvm_key;
std::shared_ptr<TranslatorProvingKey> translator_key;

GoblinAccumulationOutput accumulator; // Used only for ACIR methods for now

Expand Down Expand Up @@ -211,7 +215,8 @@ class GoblinProver {

auto translator_builder =
std::make_unique<TranslatorBuilder>(translation_batching_challenge_v, evaluation_challenge_x, op_queue);
translator_prover = std::make_unique<TranslatorProver>(*translator_builder, transcript, commitment_key);
translator_key = std::make_shared<TranslatorProvingKey>(*translator_builder, commitment_key);
translator_prover = std::make_unique<TranslatorProver>(translator_key, transcript);
}

{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ template <typename RecursiveFlavor> class TranslatorRecursiveTests : public ::te
public:
using InnerFlavor = typename RecursiveFlavor::NativeFlavor;
using InnerBuilder = typename InnerFlavor::CircuitBuilder;
using InnerProvingKey = TranslatorProvingKey;
using InnerProver = TranslatorProver;
using InnerVerifier = TranslatorVerifier;
using InnerG1 = InnerFlavor::Commitment;
Expand Down Expand Up @@ -68,8 +69,8 @@ template <typename RecursiveFlavor> class TranslatorRecursiveTests : public ::te

auto circuit_builder = InnerBuilder(translation_batching_challenge, translation_evaluation_challenge, op_queue);
EXPECT_TRUE(circuit_builder.check_circuit());

InnerProver prover{ circuit_builder, prover_transcript };
auto proving_key = std::make_shared<TranslatorProvingKey>(circuit_builder);
InnerProver prover{ proving_key, prover_transcript };
auto proof = prover.construct_proof();

OuterBuilder outer_circuit;
Expand All @@ -79,7 +80,7 @@ template <typename RecursiveFlavor> class TranslatorRecursiveTests : public ::te
auto transcript = std::make_shared<typename RecursiveFlavor::Transcript>(stdlib_proof);
transcript->template receive_from_prover<typename RecursiveFlavor::BF>("init");

auto verification_key = std::make_shared<typename InnerFlavor::VerificationKey>(prover.key);
auto verification_key = std::make_shared<typename InnerFlavor::VerificationKey>(prover.key->proving_key);
RecursiveVerifier verifier{ &outer_circuit, verification_key, transcript };
auto pairing_points = verifier.verify_proof(proof);
info("Recursive Verifier: num gates = ", outer_circuit.num_gates);
Expand All @@ -89,7 +90,7 @@ template <typename RecursiveFlavor> class TranslatorRecursiveTests : public ::te

auto native_verifier_transcript = std::make_shared<Transcript>(prover_transcript->proof_data);
native_verifier_transcript->template receive_from_prover<InnerBF>("init");
InnerVerifier native_verifier(prover.key, native_verifier_transcript);
InnerVerifier native_verifier(verification_key, native_verifier_transcript);
bool native_result = native_verifier.verify_proof(proof);
auto recursive_result = native_verifier.key->pcs_verification_key->pairing_check(pairing_points[0].get_value(),
pairing_points[1].get_value());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include "barretenberg/common/thread.hpp"
#include "barretenberg/honk/proof_system/permutation_library.hpp"
#include "barretenberg/plonk_honk_shared/library/grand_product_library.hpp"
#include "barretenberg/translator_vm/translator_flavor.hpp"
#include "barretenberg/translator_vm/translator_proving_key.hpp"

#include <gtest/gtest.h>
#include <unordered_set>
Expand Down Expand Up @@ -45,7 +45,6 @@ TEST_F(TranslatorRelationCorrectnessTests, Permutation)
using Flavor = TranslatorFlavor;
using FF = typename Flavor::FF;
using ProverPolynomials = typename Flavor::ProverPolynomials;
using ProvingKey = typename Flavor::ProvingKey;
using Polynomial = bb::Polynomial<FF>;
auto& engine = numeric::get_debug_randomness();
const size_t mini_circuit_size = 2048;
Expand All @@ -59,8 +58,10 @@ TEST_F(TranslatorRelationCorrectnessTests, Permutation)
params.gamma = gamma;

// Create storage for polynomials
auto proving_key = std::make_shared<ProvingKey>();

auto proving_key = std::make_shared<Flavor::ProvingKey>(full_circuit_size);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please make and use an appropriate constructor.

TranslatorProvingKey key;
key.proving_key = proving_key;
key.mini_circuit_dyadic_size = mini_circuit_size;
ProverPolynomials& prover_polynomials = proving_key->polynomials;
// ensure we can shift these
for (Polynomial& prover_poly : prover_polynomials.get_to_be_shifted()) {
Expand Down Expand Up @@ -149,13 +150,13 @@ TEST_F(TranslatorRelationCorrectnessTests, Permutation)
fill_polynomial_with_random_14_bit_values(prover_polynomials.relation_wide_limbs_range_constraint_3);

// Compute ordered range constraint polynomials that go in the denominator of the grand product polynomial
compute_translator_range_constraint_ordered_polynomials<Flavor>(prover_polynomials, mini_circuit_size);
key.compute_translator_range_constraint_ordered_polynomials();

// Compute the fixed numerator (part of verification key)
proving_key->compute_extra_range_constraint_numerator();
key.compute_extra_range_constraint_numerator();

// Compute concatenated polynomials (4 polynomials produced from other constraint polynomials by concatenation)
compute_concatenated_polynomials<Flavor>(prover_polynomials);
key.compute_concatenated_polynomials();

// Compute the grand product polynomial
compute_grand_product<Flavor, bb::TranslatorPermutationRelation<FF>>(prover_polynomials, params);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,14 @@ TEST_F(TranslatorTests, Basic)

auto circuit_builder = CircuitBuilder(translation_batching_challenge, translation_evaluation_challenge, op_queue);
EXPECT_TRUE(circuit_builder.check_circuit());

TranslatorProver prover{ circuit_builder, prover_transcript };
auto proving_key = std::make_shared<TranslatorProvingKey>(circuit_builder);
TranslatorProver prover{ proving_key, prover_transcript };
auto proof = prover.construct_proof();

auto verifier_transcript = std::make_shared<Transcript>(prover_transcript->proof_data);
verifier_transcript->template receive_from_prover<Fq>("init");
TranslatorVerifier verifier(prover.key, verifier_transcript);
auto verification_key = std::make_shared<TranslatorFlavor::VerificationKey>(proving_key->proving_key);
TranslatorVerifier verifier(verification_key, verifier_transcript);
bool verified = verifier.verify_proof(proof);
EXPECT_TRUE(verified);
}
104 changes: 4 additions & 100 deletions barretenberg/cpp/src/barretenberg/translator_vm/translator_flavor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include "barretenberg/flavor/flavor_macros.hpp"
#include "barretenberg/flavor/relation_definitions.hpp"
#include "barretenberg/flavor/repeated_commitments_data.hpp"
#include "barretenberg/honk/proof_system/permutation_library.hpp"
#include "barretenberg/polynomials/polynomial.hpp"
#include "barretenberg/polynomials/univariate.hpp"
#include "barretenberg/relations/relation_parameters.hpp"
Expand Down Expand Up @@ -544,7 +543,7 @@ class TranslatorFlavor {
public:
DEFINE_COMPOUND_GET_ALL(PrecomputedEntities<DataType>, WitnessEntities<DataType>, ShiftedEntities<DataType>)

auto get_precomputed() { return PrecomputedEntities<DataType>::get_all(); };
auto get_precomputed() const { return PrecomputedEntities<DataType>::get_all(); };

/**
* @brief Get entities concatenated for the permutation relation
Expand Down Expand Up @@ -615,29 +614,6 @@ class TranslatorFlavor {
}
};

public:
static inline size_t compute_total_num_gates(const CircuitBuilder& builder)
{
return std::max(builder.num_gates, MINIMUM_MINI_CIRCUIT_SIZE);
}

static inline size_t compute_dyadic_circuit_size(const CircuitBuilder& builder)
{
const size_t total_num_gates = compute_total_num_gates(builder);

// Next power of 2
const size_t mini_circuit_dyadic_size = builder.get_circuit_subgroup_size(total_num_gates);

// The actual circuit size is several times bigger than the trace in the builder, because we use concatenation
// to bring the degree of relations down, while extending the length.
return mini_circuit_dyadic_size * CONCATENATION_GROUP_SIZE;
}

static inline size_t compute_mini_circuit_dyadic_size(const CircuitBuilder& builder)
{
return builder.get_circuit_subgroup_size(compute_total_num_gates(builder));
}

/**
* @brief A field element for each entity of the flavor. These entities represent the prover polynomials
* evaluated at one point.
Expand Down Expand Up @@ -704,89 +680,17 @@ class TranslatorFlavor {
*/
class ProvingKey : public ProvingKey_<FF, CommitmentKey> {
public:
BF batching_challenge_v = { 0 };
BF evaluation_input_x = { 0 };
ProverPolynomials polynomials; // storage for all polynomials evaluated by the prover

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

ProvingKey() = default;
ProvingKey(const CircuitBuilder& builder)
: Base(compute_dyadic_circuit_size(builder), 0)
, batching_challenge_v(builder.batching_challenge_v)
, evaluation_input_x(builder.evaluation_input_x)
ProvingKey(const size_t dyadic_circuit_size, std::shared_ptr<CommitmentKey> commitment_key = nullptr)
: Base(dyadic_circuit_size, 0, std::move(commitment_key))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why move the commitment key?

Copy link
Contributor

Choose a reason for hiding this comment

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

If a shared_ptr is passed as value to the constructor (as it is here) moving it is the right thing to do. Also, passing by value and moving is the right pattern for smart pointers (clangd-format suggests it).

Note that I'm not judging IF the constructor should take a commitment key or not! That is, whether the addition makes sense.

, polynomials(this->circuit_size)
{
// First and last lagrange polynomials (in the full circuit size)
polynomials.lagrange_first.at(0) = 1;
polynomials.lagrange_last.at(circuit_size - 1) = 1;

// Compute polynomials with odd and even indices set to 1 up to the minicircuit margin + lagrange
// polynomials at second and second to last indices in the minicircuit
compute_lagrange_polynomials(builder);

// Compute the numerator for the permutation argument with several repetitions of steps bridging 0 and
// maximum range constraint compute_extra_range_constraint_numerator();
compute_extra_range_constraint_numerator();
}

inline void compute_lagrange_polynomials(const CircuitBuilder& builder)
{
const size_t mini_circuit_dyadic_size = compute_mini_circuit_dyadic_size(builder);

for (size_t i = 1; i < mini_circuit_dyadic_size - 1; i += 2) {
polynomials.lagrange_odd_in_minicircuit.at(i) = 1;
polynomials.lagrange_even_in_minicircuit.at(i + 1) = 1;
}
polynomials.lagrange_second.at(1) = 1;
polynomials.lagrange_second_to_last_in_minicircuit.at(mini_circuit_dyadic_size - 2) = 1;
}

/**
* @brief Compute the extra numerator for Goblin range constraint argument
*
* @details Goblin proves that several polynomials contain only values in a certain range through 2
* relations: 1) A grand product which ignores positions of elements (TranslatorPermutationRelation) 2) A
* relation enforcing a certain ordering on the elements of the given polynomial
* (TranslatorDeltaRangeConstraintRelation)
*
* We take the values from 4 polynomials, and spread them into 5 polynomials + add all the steps from
* MAX_VALUE to 0. We order these polynomials and use them in the denominator of the grand product, at the
* same time checking that they go from MAX_VALUE to 0. To counteract the added steps we also generate an
* extra range constraint numerator, which contains 5 MAX_VALUE, 5 (MAX_VALUE-STEP),... values
*
*/
inline void compute_extra_range_constraint_numerator()
{
auto& extra_range_constraint_numerator = polynomials.ordered_extra_range_constraints_numerator;

static constexpr uint32_t MAX_VALUE = (1 << MICRO_LIMB_BITS) - 1;

// Calculate how many elements there are in the sequence MAX_VALUE, MAX_VALUE - 3,...,0
size_t sorted_elements_count = (MAX_VALUE / SORT_STEP) + 1 + (MAX_VALUE % SORT_STEP == 0 ? 0 : 1);

// Check that we can fit every element in the polynomial
ASSERT((NUM_CONCATENATED_WIRES + 1) * sorted_elements_count < extra_range_constraint_numerator.size());

std::vector<size_t> sorted_elements(sorted_elements_count);

// Calculate the sequence in integers
sorted_elements[0] = MAX_VALUE;
for (size_t i = 1; i < sorted_elements_count; i++) {
sorted_elements[i] = (sorted_elements_count - 1 - i) * SORT_STEP;
}

// TODO(#756): can be parallelized further. This will use at most 5 threads
auto fill_with_shift = [&](size_t shift) {
for (size_t i = 0; i < sorted_elements_count; i++) {
extra_range_constraint_numerator.at(shift + i * (NUM_CONCATENATED_WIRES + 1)) = sorted_elements[i];
}
};
// Fill polynomials with a sequence, where each element is repeated NUM_CONCATENATED_WIRES+1 times
parallel_for(NUM_CONCATENATED_WIRES + 1, fill_with_shift);
}
{}
};

/**
Expand Down
Loading