-
Notifications
You must be signed in to change notification settings - Fork 607
refactor: share decider with ultra_prover #5467
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
Changes from 4 commits
9cc7b7a
29f9a03
5ab3f4a
b1ddafb
cb9757b
bd964d1
885605d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,7 @@ DeciderProver_<Flavor>::DeciderProver_(const std::shared_ptr<Instance>& inst, | |
| const std::shared_ptr<Transcript>& transcript) | ||
| : accumulator(std::move(inst)) | ||
| , transcript(transcript) | ||
| , commitment_key(inst->proving_key->commitment_key) | ||
| , commitment_key(accumulator->proving_key->commitment_key) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably not meaningful, but I wasn't sure
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually, shouldn't inst be nullified by the std::move on line 18? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. std::move doesn't nullify the object being moved it just facilities moving rather than copying resources, your change makes sense but the previous code wasn't incorrect |
||
| {} | ||
|
|
||
| /** | ||
|
|
@@ -49,13 +49,13 @@ template <IsUltraFlavor Flavor> void DeciderProver_<Flavor>::execute_zeromorph_r | |
| transcript); | ||
| } | ||
|
|
||
| template <IsUltraFlavor Flavor> HonkProof& DeciderProver_<Flavor>::export_proof() | ||
| template <IsUltraFlavor Flavor> HonkProof DeciderProver_<Flavor>::export_proof() | ||
| { | ||
| proof = transcript->proof_data; | ||
| return proof; | ||
| } | ||
|
|
||
| template <IsUltraFlavor Flavor> HonkProof& DeciderProver_<Flavor>::construct_proof() | ||
| template <IsUltraFlavor Flavor> HonkProof DeciderProver_<Flavor>::construct_proof() | ||
| { | ||
| BB_OP_COUNT_TIME_NAME("Decider::construct_proof"); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,8 +25,12 @@ template <IsUltraFlavor Flavor> OinkProverOutput<Flavor> OinkProver<Flavor>::pro | |
| // Compute grand product(s) and commitments. | ||
| execute_grand_product_computation_round(); | ||
|
|
||
| // Generate relation separators alphas for sumcheck | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could probably make a separate function for this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. they are not only for sumcheck, they are for batching all subrelations into the big U(G)H relation either in sumcheck or combiner computation, cna you make the comment more specific? |
||
| RelationSeparator alphas = generate_alphas(); | ||
|
|
||
| return OinkProverOutput<Flavor>{ | ||
| .relation_parameters = std::move(relation_parameters), | ||
| .alphas = std::move(alphas), | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -149,6 +153,14 @@ template <IsUltraFlavor Flavor> void OinkProver<Flavor>::execute_grand_product_c | |
| transcript->send_to_verifier(domain_separator + commitment_labels.z_lookup, witness_commitments.z_lookup); | ||
| } | ||
|
|
||
| template <IsUltraFlavor Flavor> typename Flavor::RelationSeparator OinkProver<Flavor>::generate_alphas() | ||
| { | ||
| RelationSeparator alphas; | ||
| for (size_t idx = 0; idx < alphas.size(); idx++) { | ||
| alphas[idx] = transcript->template get_challenge<FF>("Sumcheck:alpha_" + std::to_string(idx)); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is not necessarily for sumcheck (and neither are the gate challenges) so maybe it should be made explicit? |
||
| } | ||
| return alphas; | ||
| } | ||
| template class OinkProver<UltraFlavor>; | ||
| template class OinkProver<GoblinUltraFlavor>; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| #include "ultra_prover.hpp" | ||
| #include "barretenberg/sumcheck/sumcheck.hpp" | ||
| #include "barretenberg/ultra_honk/decider_prover.hpp" | ||
|
|
||
| namespace bb { | ||
|
|
||
|
|
@@ -15,7 +16,7 @@ UltraProver_<Flavor>::UltraProver_(const std::shared_ptr<Instance>& inst, const | |
| : instance(std::move(inst)) | ||
| , transcript(transcript) | ||
| , commitment_key(instance->proving_key->commitment_key) | ||
| , oink_prover(inst->proving_key, commitment_key, transcript, "") | ||
| , oink_prover(instance->proving_key, commitment_key, transcript, "") | ||
| {} | ||
|
|
||
| /** | ||
|
|
@@ -33,65 +34,31 @@ UltraProver_<Flavor>::UltraProver_(Builder& circuit) | |
| , oink_prover(instance->proving_key, commitment_key, transcript, "") | ||
| {} | ||
|
|
||
| /** | ||
| * @brief Run Sumcheck resulting in u = (u_1,...,u_d) challenges and all evaluations at u being calculated. | ||
| * | ||
| */ | ||
| template <IsUltraFlavor Flavor> void UltraProver_<Flavor>::execute_relation_check_rounds() | ||
| template <IsUltraFlavor Flavor> HonkProof UltraProver_<Flavor>::export_proof() | ||
| { | ||
| using Sumcheck = SumcheckProver<Flavor>; | ||
| auto circuit_size = instance->proving_key->circuit_size; | ||
| auto sumcheck = Sumcheck(circuit_size, transcript); | ||
| RelationSeparator alphas; | ||
| for (size_t idx = 0; idx < alphas.size(); idx++) { | ||
| alphas[idx] = transcript->template get_challenge<FF>("Sumcheck:alpha_" + std::to_string(idx)); | ||
| } | ||
| instance->alphas = alphas; | ||
| std::vector<FF> gate_challenges(numeric::get_msb(circuit_size)); | ||
| proof = transcript->proof_data; | ||
| return proof; | ||
| } | ||
| template <IsUltraFlavor Flavor> void UltraProver_<Flavor>::generate_gate_challenges() | ||
| { | ||
| std::vector<FF> gate_challenges(numeric::get_msb(instance->proving_key->circuit_size)); | ||
| for (size_t idx = 0; idx < gate_challenges.size(); idx++) { | ||
| gate_challenges[idx] = transcript->template get_challenge<FF>("Sumcheck:gate_challenge_" + std::to_string(idx)); | ||
| } | ||
| instance->gate_challenges = gate_challenges; | ||
| sumcheck_output = sumcheck.prove(instance); | ||
| } | ||
|
|
||
| /** | ||
| * @brief Execute the ZeroMorph protocol to prove the multilinear evaluations produced by Sumcheck | ||
| * @details See https://hackmd.io/dlf9xEwhTQyE3hiGbq4FsA?view for a complete description of the unrolled protocol. | ||
| * | ||
| * */ | ||
| template <IsUltraFlavor Flavor> void UltraProver_<Flavor>::execute_zeromorph_rounds() | ||
| { | ||
| ZeroMorph::prove(instance->prover_polynomials.get_unshifted(), | ||
| instance->prover_polynomials.get_to_be_shifted(), | ||
| sumcheck_output.claimed_evaluations.get_unshifted(), | ||
| sumcheck_output.claimed_evaluations.get_shifted(), | ||
| sumcheck_output.challenge, | ||
| commitment_key, | ||
| transcript); | ||
| } | ||
|
|
||
| template <IsUltraFlavor Flavor> HonkProof& UltraProver_<Flavor>::export_proof() | ||
| template <IsUltraFlavor Flavor> HonkProof UltraProver_<Flavor>::construct_proof() | ||
| { | ||
| proof = transcript->proof_data; | ||
| return proof; | ||
| } | ||
|
|
||
| template <IsUltraFlavor Flavor> HonkProof& UltraProver_<Flavor>::construct_proof() | ||
| { | ||
| auto [relation_params] = oink_prover.prove(); | ||
| auto [relation_params, alphas] = oink_prover.prove(); | ||
| instance->relation_parameters = std::move(relation_params); | ||
| instance->alphas = alphas; | ||
| instance->prover_polynomials = ProverPolynomials(instance->proving_key); | ||
|
|
||
| // Fiat-Shamir: alpha | ||
| // Run sumcheck subprotocol. | ||
| execute_relation_check_rounds(); | ||
|
|
||
| // Fiat-Shamir: rho, y, x, z | ||
| // Execute Zeromorph multilinear PCS | ||
| execute_zeromorph_rounds(); | ||
| generate_gate_challenges(); | ||
|
|
||
| return export_proof(); | ||
| DeciderProver_<Flavor> decider_prover(instance, transcript); | ||
| return decider_prover.construct_proof(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's quite odd that the decider_prover is being called here since the ultra prover should not care about folding, this is one of those place where round abstraction would be awesome :-? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but until then I would kinda leave the sumcheck and zeromorph rounds unshared between the decider and ultra prover just to not create this dependence |
||
| } | ||
|
|
||
| template class UltraProver_<UltraFlavor>; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't it more efficient to pass by reference?