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.

Sign up for GitHub

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

Jump to bottom

Remove duplication of Flavor entity elements in Prover constructors #2213

Closed
zac-williamson opened this issue Sep 11, 2023 · 0 comments · Fixed by #3560
Closed

Remove duplication of Flavor entity elements in Prover constructors #2213

zac-williamson opened this issue Sep 11, 2023 · 0 comments · Fixed by #3560
Assignees
@ludamad
Labels
crypto cryptography

Comments

@zac-williamson
Copy link
Contributor

In every Prover class (e.g. barretenberg/honk/proof_system/prover.cpp), in the constructor we copy every Flavor entity element from the proving key into the prover_polynomials member variable. The error surface of this step is large as one mis-copied member will break the prover and is difficult to spot + debug.

Ideally we can eithe remove or automate this copying step
@zac-williamson zac-williamson self-assigned this Sep 11, 2023
@github-project-automation github-project-automation bot moved this to Todo in A3 Sep 11, 2023
@zac-williamson zac-williamson added the crypto cryptography label Sep 11, 2023
@zac-williamson zac-williamson mentioned this issue Sep 11, 2023
Merged
6 tasks
@codygunton codygunton assigned ludamad and unassigned zac-williamson Nov 17, 2023
@ludamad ludamad mentioned this issue Dec 11, 2023
Merged
@github-project-automation github-project-automation bot moved this from Todo to Done in A3 Dec 14, 2023
AztecBot pushed a commit to AztecProtocol/barretenberg that referenced this issue Dec 15, 2023
@ludamad @ludamad0 @AztecBot
feat: ProverPolynomials owns its memory (#3560)
416f680 
Makes ProverPolynomials no longer use a span
- Polynomial class already used shared memory, now allowing shifted
polynomials to be represented by the same class
- Fixes AztecProtocol/aztec-packages#2213 by
using ranges defined in entity classes
- For safety, deletes the full-copy assignment and constructor of
ProverPolynomials, so that we correctly std::move it around. This can
possibly be removed, but wanted to catch any now-different-semantics
usage and avoid extra copies
- Makes explicit share() calls wherever semantics were to share before
due to std::span usage. shifted() is similar, shares underlying memory,
shifted
- Remove cruft needed with a separate polynomial storages variable, some
might remain

---------

Co-authored-by: ludamad <adam@aztecprotocol.com>
michaelelliot pushed a commit to Swoir/noir_rs that referenced this issue Feb 28, 2024
@ludamad @ludamad0
feat: ProverPolynomials owns its memory (AztecProtocol#3560)
24258de 
Makes ProverPolynomials no longer use a span
- Polynomial class already used shared memory, now allowing shifted
polynomials to be represented by the same class
- Fixes AztecProtocol#2213 by
using ranges defined in entity classes
- For safety, deletes the full-copy assignment and constructor of
ProverPolynomials, so that we correctly std::move it around. This can
possibly be removed, but wanted to catch any now-different-semantics
usage and avoid extra copies
- Makes explicit share() calls wherever semantics were to share before
due to std::span usage. shifted() is similar, shares underlying memory,
shifted
- Remove cruft needed with a separate polynomial storages variable, some
might remain

---------

Co-authored-by: ludamad <adam@aztecprotocol.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ludamad ludamad

Labels
crypto cryptography
Projects
A3
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: ProverPolynomials owns its memory AztecProtocol/aztec-packages
2 participants
@ludamad @zac-williamson