-
Notifications
You must be signed in to change notification settings - Fork 265
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: bye bye shared ptrs for ultra/goblin ultra proving_keys :) #5407
Changes from 23 commits
d132a10
eec17c6
45c5ee2
810d369
e76c972
6078832
b0855d5
f4aa979
1f4e307
3fe93cc
2798eaf
4030744
e11bb7e
b4c4354
2b67229
abd0bd7
8923c8c
c425e3d
2b6aae9
2d019cd
994c767
de50495
c7192bc
7e27f9c
70ff641
6397673
d1a4965
03238e8
35ae332
7b4a20e
7f79def
bf9a7f7
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 |
---|---|---|
|
@@ -408,8 +408,27 @@ class GoblinUltraFlavor { | |
* circuits. | ||
* @todo TODO(https://github.com/AztecProtocol/barretenberg/issues/876) | ||
*/ | ||
using VerificationKey = VerificationKey_<PrecomputedEntities<Commitment>, VerifierCommitmentKey>; | ||
// using VerificationKey = VerificationKey_<PrecomputedEntities<Commitment>, VerifierCommitmentKey>; | ||
class VerificationKey : public VerificationKey_<PrecomputedEntities<Commitment>, VerifierCommitmentKey> { | ||
public: | ||
VerificationKey() = default; | ||
VerificationKey(const size_t circuit_size, const size_t num_public_inputs) | ||
: VerificationKey_(circuit_size, num_public_inputs) | ||
{} | ||
|
||
VerificationKey(ProvingKey& proving_key) | ||
{ | ||
this->pcs_verification_key = std::make_shared<VerifierCommitmentKey>(); | ||
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->pub_inputs_offset = proving_key.pub_inputs_offset; | ||
|
||
for (auto [polynomial, commitment] : zip_view(proving_key.get_precomputed_polynomials(), this->get_all())) { | ||
commitment = proving_key.commitment_key->commit(polynomial); | ||
} | ||
} | ||
}; | ||
/** | ||
* @brief A container for storing the partially evaluated multivariates produced by sumcheck. | ||
*/ | ||
|
@@ -452,6 +471,7 @@ class GoblinUltraFlavor { | |
*/ | ||
class ProverPolynomials : public AllEntities<Polynomial> { | ||
public: | ||
// TODO(https://github.com/AztecProtocol/barretenberg/issues/925), proving_key could be const ref | ||
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. extra todo added from prev PR |
||
ProverPolynomials(ProvingKey& proving_key) | ||
{ | ||
for (auto [prover_poly, key_poly] : zip_view(this->get_unshifted(), proving_key.get_all())) { | ||
|
@@ -463,17 +483,6 @@ class GoblinUltraFlavor { | |
prover_poly = key_poly.shifted(); | ||
} | ||
} | ||
ProverPolynomials(std::shared_ptr<ProvingKey>& proving_key) | ||
{ | ||
for (auto [prover_poly, key_poly] : zip_view(this->get_unshifted(), proving_key->get_all())) { | ||
ASSERT(flavor_get_label(*this, prover_poly) == flavor_get_label(*proving_key, key_poly)); | ||
prover_poly = key_poly.share(); | ||
} | ||
for (auto [prover_poly, key_poly] : zip_view(this->get_shifted(), proving_key->get_to_be_shifted())) { | ||
ASSERT(flavor_get_label(*this, prover_poly) == (flavor_get_label(*proving_key, key_poly) + "_shift")); | ||
prover_poly = key_poly.shifted(); | ||
} | ||
} | ||
// Define all operations as default, except move construction/assignment | ||
ProverPolynomials() = default; | ||
ProverPolynomials& operator=(const ProverPolynomials&) = delete; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,7 +41,7 @@ std::shared_ptr<plonk::proving_key> StandardComposer::compute_proving_key(Circui | |
subgroup_size, circuit_constructor.public_inputs.size(), crs, CircuitType::STANDARD); | ||
|
||
// Construct and add to proving key the wire, selector and copy constraint polynomials | ||
Trace::populate(circuit_constructor, circuit_proving_key); | ||
Trace::populate(circuit_constructor, *circuit_proving_key); | ||
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 guess it would have spiraled here to handle not through a pointer here? Seems reasonable to leave as a pointer in that case since Plonk is on its way out. 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. indeed |
||
|
||
// Make all selectors nonzero | ||
enforce_nonzero_selector_polynomials(circuit_constructor, circuit_proving_key.get()); | ||
|
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.
this function with a provingkeyptr template is pretty bad
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.
Instead, I put them in the specific flavor classes