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

feat: prove openings of masking polynomials in ECCVM and Translator #9726

Merged
merged 25 commits into from
Nov 7, 2024

Conversation

iakovenkos
Copy link
Contributor

@iakovenkos iakovenkos commented Nov 4, 2024

As a part of ZK-fication of Honk, we have to mask the evaluations of round univariates that the prover sends to the verifier. The evaluations were masked in Sumcheck in PR #7517. However, the logic for proving evaluations of Libra masking polynomials was missing. This PR fixes this issue and enables efficient batch opening of these polynomials.

  • Added necessary logic to Shplonk Prover, Shplemini Prover, and Shplemini Verifer
  • Better handling of the ZKSumcheckData
  • Removed methods and reverted changes that became obsolete because of the new ZK strategy
  • Enabled the opening of Libra masking univariates in ECCVM and Translator

{
const size_t num_opening_claims = opening_claims.size();

// {ẑⱼ(r)}ⱼ , where ẑⱼ(r) = 1/zⱼ(r) = 1/(r - xⱼ)
// {ẑⱼ(z)}ⱼ , where ẑⱼ(r) = 1/zⱼ(z) = 1/(z - xⱼ)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

typo

@@ -41,6 +41,42 @@ TYPED_TEST(KZGTest, single)
EXPECT_EQ(this->vk()->pairing_check(pairing_points[0], pairing_points[1]), true);
}

/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We usually commit to and open univariate polynomials represented in the monomial basis. Here is a correct flow for the univariates given in the Lagrange basis.

@@ -439,5 +466,70 @@ template <typename Curve> class ShpleminiVerifier_ {
commitments.emplace_back(std::move(fold_commitments[j]));
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

main feature of this PR - the method enabling efficient opening of claimed evaluations of Libra masking univariates. no extra challenges or batch mul calls required

@@ -113,6 +134,22 @@ template <typename Curve> class ShplonkProver_ {
idx++;
}

// Take into account the constant proof size in Gemini
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's just a usual shplonk stuff taking into account that we have const proof size in gemini

@@ -68,7 +68,7 @@ class ECCVMFlavor {
using Relations = Relations_<FF>;
using LookupRelation = ECCVMLookupRelation<FF>;

static constexpr size_t MAX_PARTIAL_RELATION_LENGTH = compute_max_partial_relation_length<Relations, HasZK>();
static constexpr size_t MAX_PARTIAL_RELATION_LENGTH = compute_max_partial_relation_length<Relations>();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't need this anymore as we are switching to a different evaluation masking technique. same in translator


// Default constructor
Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a constructor here, moved a bunch of method from sumcheck.hpp

@@ -190,13 +188,11 @@ template <typename Flavor> class SumcheckProver {
SumcheckOutput<Flavor> prove(ProverPolynomials& full_polynomials,
const bb::RelationParameters<FF>& relation_parameters,
const RelationSeparator alpha,
const std::vector<FF>& gate_challenges)
const std::vector<FF>& gate_challenges,
ZKSumcheckData<Flavor> zk_sumcheck_data = ZKSumcheckData<Flavor>())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

decoupled the creation of a ZKSumcheckData object from the prove method. now it's taken care by a prover that uses a constructor to create these data and then feeds it to the prove method

if constexpr (Flavor::HasZK) {
output += full_libra_purported_value.value();
if constexpr (IsECCVMRecursiveFlavor<Flavor>) {
output.self_reduce();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we have to self_reduce here

Copy link
Contributor

Choose a reason for hiding this comment

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

this is only necessary for the ECCVM? is it clear why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's only the ECCVMRecursiveFlavor, because it's operating with the scalars in bigfield

@iakovenkos iakovenkos self-assigned this Nov 5, 2024
Copy link
Contributor

@ledwards2225 ledwards2225 left a comment

Choose a reason for hiding this comment

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

Looks great, just some minor suggestions about style/comments. Also, is there any more cleanup that can be done in the Flavors as a result of this? I'm thinking about the erroneous NUM_ALL_WITNESS_ENTITIES in the MegaFlavor for example..

OpeningClaim batched_claim = ShplonkProver::prove(commitment_key, opening_claims, transcript);
// Create opening claims for Libra masking univariates
std::vector<OpeningClaim> libra_opening_claims;
if (!libra_univariates.empty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

its a little redundant to have this check prior to the loop. If it's empty, the loop will just do nothing right?

@@ -254,6 +271,16 @@ template <typename Curve> class ShpleminiVerifier_ {
commitments.emplace_back(g1_identity);
scalars.emplace_back(constant_term_accumulator);

if (!libra_univariate_evaluations.empty()) {
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 more clarifying condition that could be used here? e.g is this always empty for non-zk flavors and always non-empty for zk flavors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

exactly, but it's handled somewhat manually because the PCS doesn't know anything about the Flavor. added a clarifying comment

Copy link
Contributor

Choose a reason for hiding this comment

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

this is probably an indication that the PCS now needs to be templated on Flavor instead of just Curve

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agree. I think I'll stop adding stuff to Shplemini quite soon and then we'll have to refactor it, because its interface is getting quite messy

size_t num_libra_univariates = libra_univariate_commitments.size();

// compute Shplonk denominators and invert them
for (size_t idx = 0; idx < num_libra_univariates; idx++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the number of libra univariates different from the size of the multivariate challenge? If not it might be better to use a range loop here on multivariate_challenge

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, the size of the challenge is CONST_PROOF_SIZE_LOG_N, and the number of libra univariates if the honest log circuit size. prob we'll need to constify it at some point, but I decided to avoid it here

}

// needed to keep track of the constant term contribution
const size_t idx_of_constant_term = commitments.size() - 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

another option here is to simply define contant_term or constant_scalar as a ref then update it below. E.g. Fr& constant_scalar = scalars.back(); then constant_scalar += constant_term;. just personal preference I suppose

for (size_t idx = 0; idx < log_n; idx++) {
// generate random polynomial
Polynomial libra_polynomial = Polynomial::random(LIBRA_UNIVARIATE_LENGTH);
// create a univariate with the same coefficients (to store an array intead of a vector)
Copy link
Contributor

Choose a reason for hiding this comment

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

typo

Polynomial libra_polynomial = Polynomial::random(LIBRA_UNIVARIATE_LENGTH);
// create a univariate with the same coefficients (to store an array intead of a vector)
bb::Univariate<Fr, LIBRA_UNIVARIATE_LENGTH> libra_univariate;
for (size_t i = 0; i < LIBRA_UNIVARIATE_LENGTH; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI it looks like Univariate has it's own random_element() method - might be cleaner

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks! it's ugly here: Univariates are in the Lagrange basis, Polynomials are in the monomial, which means we can't commit to a Univariate without computing its coefficients in the monomial basis or computing the Lagrange basis using SRS + I want to store the masking data as a std::vector<Univariate<FF, 12>> (could/will be turned into std::array) instead of std::vector

auto eval3_shift = poly3.evaluate_mle(mle_opening_point, true);

// Collect multilinear evaluations for input to prover
// std::vector<Fr> multilinear_evaluations = { eval1, eval2, eval3, eval4, eval2_shift, eval3_shift };
Copy link
Contributor

Choose a reason for hiding this comment

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

unused?

@@ -61,7 +62,20 @@ template <typename Curve> class ShplonkProver_ {
Q.add_scaled(tmp, current_nu);
current_nu *= nu;
}
for (size_t idx = opening_claims.size(); idx < CONST_PROOF_SIZE_LOG_N + 2; idx++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

would be nice to add a comment explaining the significance of CONST_PROOF_SIZE_LOG_N + 2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a comment

@@ -82,15 +82,18 @@ polynomials,
using View = typename Accumulator::View;

if constexpr (read_index == 0) {

Copy link
Contributor

Choose a reason for hiding this comment

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

are these spaces intentional?

if constexpr (Flavor::HasZK) {
output += full_libra_purported_value.value();
if constexpr (IsECCVMRecursiveFlavor<Flavor>) {
output.self_reduce();
Copy link
Contributor

Choose a reason for hiding this comment

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

this is only necessary for the ECCVM? is it clear why?

@iakovenkos iakovenkos added e2e-prover-full CI: Enables this CI job. crypto cryptography labels Nov 6, 2024
@ludamad ludamad added e2e-prover-full CI: Enables this CI job. e2e Enables e2e jobs in BB change PRs. and removed e2e-prover-full CI: Enables this CI job. labels Nov 6, 2024
@iakovenkos iakovenkos removed e2e-prover-full CI: Enables this CI job. e2e Enables e2e jobs in BB change PRs. labels Nov 7, 2024
@iakovenkos iakovenkos merged commit f1cdc2d into master Nov 7, 2024
47 checks passed
@iakovenkos iakovenkos deleted the si/zk-sumcheck-plus-shplemini branch November 7, 2024 12:49
ludamad pushed a commit that referenced this pull request Nov 11, 2024
…9726)

As a part of ZK-fication of Honk, we have to mask the evaluations of
round univariates that the prover sends to the verifier. The evaluations
were masked in Sumcheck in PR #7517. However, the logic for proving
evaluations of Libra masking polynomials was missing. This PR fixes this
issue and enables efficient batch opening of these polynomials.
* Added necessary logic to Shplonk Prover, Shplemini Prover, and
Shplemini Verifer
* Better handling of the ZKSumcheckData
* Removed methods and reverted changes that became obsolete because of
the new ZK strategy
* Enabled the opening of Libra masking univariates in ECCVM and
Translator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto cryptography
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants