Lde/gemini shplonk in prover #67

ledwards2225 · 2023-01-18T17:00:20Z

No description provided.

ledwards2225 · 2023-01-19T19:27:43Z

cpp/src/aztec/transcript/transcript.cpp

 {
+    (void)circuit_size;


…anscript using poly manifest

…ess polynomials

… raw pointer for commitment key

…ent key

codygunton

Looks good, mostly just asking questions in the comments.

codygunton · 2023-01-19T20:47:48Z

cpp/src/aztec/honk/pcs/claim.hpp

@@ -127,6 +127,11 @@ template <typename Params> struct MLEOpeningClaim {
    using Commitment = typename Params::Commitment;
    using Fr = typename Params::Fr;

+    MLEOpeningClaim(auto commitment, auto evaluation)


You explained that adding this is necessary to work around an optimization targeting the recursive case.

Yeah this was so I could construct MLEOpeningClaims in place with emplace_back in the prover. Need a constructor for that and there was previously no need for one.

codygunton · 2023-01-19T20:48:10Z

cpp/src/aztec/honk/pcs/commitment_key.test.hpp

@@ -23,19 +24,20 @@ namespace {
 constexpr std::string_view kzg_srs_path = "../srs_db/ignition";
 }

-template <class CK> inline CK* CreateCommitmentKey();
+template <class CK> inline std::shared_ptr<CK> CreateCommitmentKey();


IOU on what the right data structure is here, but we should just use this for now.

codygunton · 2023-01-19T20:49:36Z

cpp/src/aztec/honk/proof_system/prover.cpp

@@ -157,7 +162,7 @@ template <typename settings> void Prover<settings>::compute_grand_product_polyno

    // Construct permutation polynomial 'z_perm' in lagrange form as:
    // z_perm = [1 numererator_accum[0][0] numererator_accum[0][1] ... numererator_accum[0][n-2]]
-    polynomial z_perm(proving_key->n, proving_key->n);
+    Polynomial z_perm(proving_key->n, proving_key->n);


polynomial ~> Polynomial is you renaming?

Yeah they both alias the same thing, just trying to make it consistent throughout the prover and I prefer Polynomial

Aha, thanks

codygunton · 2023-01-19T20:51:16Z

cpp/src/aztec/honk/proof_system/prover.cpp

@@ -280,7 +285,7 @@ template <typename settings> void Prover<settings>::execute_relation_check_round
 {
    // queue.flush_queue(); // NOTE: Don't remove; we may reinstate the queue

-    using Multivariates = sumcheck::Multivariates<barretenberg::fr, waffle::STANDARD_HONK_MANIFEST_SIZE>;
+    using Multivariates = sumcheck::Multivariates<Fr, waffle::TOTAL_NUM_POLYNOMIALS>;


This should contain the words HONK and STANDARD in it somewhere.

Good call, done.

codygunton · 2023-01-19T20:53:16Z

cpp/src/aztec/honk/proof_system/prover.cpp

+    std::vector<Fr> opening_point;
+    std::vector<MLEOpeningClaim> opening_claims;
+    std::vector<MLEOpeningClaim> opening_claims_shifted;
+    std::vector<Polynomial*> multivariate_polynomials;


This is idiomatic to Gemini -- raw pointers are in use there?

Yes, just matching what's done in Gemini. Should probably change in the future

codygunton · 2023-01-19T20:55:00Z

cpp/src/aztec/honk/proof_system/prover.cpp

-    for (size_t round_idx = 0; round_idx < proving_key->log_n; round_idx++) {
-        transcript.add_element("a_" + std::to_string(round_idx), barretenberg::fr(round_idx + 1000).to_buffer());
-    }
+    // TODO(luke): This functionality is performed within Gemini::reduce_prove(), called in the previous round. In the


Right, we discussed this a week or two ago.

codygunton · 2023-01-19T20:56:47Z

cpp/src/aztec/honk/sumcheck/polynomials/multivariates.hpp

@@ -98,8 +108,10 @@ template <class FF_, size_t num_polys> class Multivariates {
    }

    explicit Multivariates(transcript::StandardTranscript transcript)
-        : multivariate_n(
-              static_cast<size_t>(transcript.get_field_element("circuit_size").from_montgomery_form().data[0]))
+        : multivariate_n([](std::vector<uint8_t> buffer) {


This is ugly, but it was done in a hurry by me, and I think it's fine for now.

codygunton · 2023-01-19T20:58:20Z

cpp/src/aztec/honk/sumcheck/sumcheck.test.cpp

-
-    auto transcript = Transcript(transcript::Manifest(manifest_rounds));
+    // Mock prover-transcript interactions prior to Sumcheck
+    auto transcript = Transcript(StandardHonk::create_unrolled_manifest(0, multivariate_d));


I was a little unsure of this (test more likely to break if it depends on external stuff), but I did this too and I think it's fine for now.

I actually think this is the right thing to do. If we change Sumcheck-related Manifest entries, we must change Sumcheck accordingly. If this test is using some fake Manifest, we then also have to go in and fix that. But this is a Sumcheck test, not a fake Manifest test.

codygunton · 2023-01-19T20:59:01Z

cpp/src/aztec/transcript/transcript.cpp

 {
+    (void)circuit_size;


codygunton · 2023-01-19T21:01:42Z

cpp/src/aztec/transcript/transcript.cpp

@@ -150,6 +164,10 @@ void Transcript::apply_fiat_shamir(const std::string& challenge_name /*, const b
    // TODO(Cody): Coupling: this line insists that the challenges in the manifest
    // are encountered in the order that matches the order of the proof construction functions.
    // Future architecture should specify this data in a single place (?).
+    // info("apply_fiat_shamir: challenge name match");


Leaving a lot of logging statements in comments. Intended? OK if so.

Yeah I wasn't planning on leaving them in the final PR but I'd actually like to while were in major debugging mode. I wind up putting these statements in every branch for debugging anyway.

* update multivariates constructor to handle shifts and add evals to transcript using poly manifest * computing claims in prover; still using mock commitments for non witness polynomials * calling gemini reduce prove in prover; all tests passing; still using raw pointer for commitment key * Updating PCS to use shared pointer instead of raw pointer for commitment key * Codys sumcheck round size fix * fixing multivariates constructor * gemini, shplonk, and kzg all running in prover

ledwards2225 commented Jan 19, 2023

View reviewed changes

ledwards2225 added 13 commits January 19, 2023 19:48

update multivariates constructor to handle shifts and add evals to tr…

0834845

…anscript using poly manifest

WiP; claim construction

7fd32cc

computing claims in prover; still using mock commitments for non witn…

00a2976

…ess polynomials

calling gemini reduce prove in prover; all tests passing; still using…

08c8a9b

… raw pointer for commitment key

Updating PCS to use shared pointer instead of raw pointer for commitm…

8a7ec37

…ent key

adding debug outputs for CCI

c89c353

more debug prints

6413d31

Codys sumcheck round size fix

105591a

testing change in full_polynomials initialization

1fcc2d0

fixing multivariates constructor

3cf8fab

gemini, shplonk, and kzg all running in prover

b91588b

fix Sumcheck tests that broke after last update

15d7194

cleanup for PR

d990b38

ledwards2225 force-pushed the lde/gemini_shplonk_in_prover branch from 08befef to d990b38 Compare January 19, 2023 19:51

codygunton requested changes Jan 19, 2023

View reviewed changes

Adding STANDARD_HONK tag to constant and deleting debug void line

2e51fcb

ledwards2225 marked this pull request as ready for review January 19, 2023 21:31

codygunton approved these changes Jan 19, 2023

View reviewed changes

codygunton merged commit 72fd093 into master Jan 19, 2023

codygunton deleted the lde/gemini_shplonk_in_prover branch January 19, 2023 22:09

codygunton assigned Rumata888 and ledwards2225 and unassigned Rumata888 Jan 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lde/gemini shplonk in prover #67

Lde/gemini shplonk in prover #67

ledwards2225 commented Jan 18, 2023

ledwards2225 Jan 19, 2023

codygunton Jan 19, 2023

ledwards2225 Jan 19, 2023

codygunton left a comment

codygunton Jan 19, 2023

ledwards2225 Jan 19, 2023

codygunton Jan 19, 2023

codygunton Jan 19, 2023

ledwards2225 Jan 19, 2023

codygunton Jan 19, 2023

codygunton Jan 19, 2023

ledwards2225 Jan 19, 2023

codygunton Jan 19, 2023

ledwards2225 Jan 19, 2023

codygunton Jan 19, 2023

codygunton Jan 19, 2023

codygunton Jan 19, 2023

ledwards2225 Jan 19, 2023

codygunton Jan 19, 2023

codygunton Jan 19, 2023

codygunton Jan 19, 2023

ledwards2225 Jan 19, 2023

Lde/gemini shplonk in prover #67

Lde/gemini shplonk in prover #67

Conversation

ledwards2225 commented Jan 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codygunton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment