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

Honk proofs are structs(?) #656

Closed
codygunton opened this issue Aug 14, 2023 · 7 comments · Fixed by AztecProtocol/aztec-packages#2937
Closed

Honk proofs are structs(?) #656

codygunton opened this issue Aug 14, 2023 · 7 comments · Fixed by AztecProtocol/aztec-packages#2937
Assignees
Labels
good first issue Good for newcomers

Comments

@codygunton
Copy link
Collaborator

codygunton commented Aug 14, 2023

This seems much better--you can read what's in the proof and you can explicitly see how thing serialize because there's a function that shows the serialization.

Luke: Also, this would allow for more explicit testing of invalid proofs in the recursive verification setting. E.g. this would make it easy to supply an incorrect multilinear evaluation or an bad wire commitment. See the honk recursive verifier tests for an example.

@codygunton codygunton added the good first issue Good for newcomers label Aug 14, 2023
@itsajay1029
Copy link

Can you assign this issue @codygunton and also please ellaborate on the issue a little

@codygunton
Copy link
Collaborator Author

Hi @itsajay1029, sorry for the delay, I must have missed a notification.

To elaborate: here

plonk::proof& construct_proof();

and here

we see that the Honk provers output a plonk::proof (this is already weird, since Plonk and Honk are different proving systems). Elsewhere you will see that plonk::proof is simply an alias for a std::vector<uint8_t>. My idea is that, for Honk, we, should have more informative structures. Namely, each "flavor" (e.g., Ultra and Standard, but there are others, and more to come) should have a custom struct defined as a container for the proof. Each should have an explicit serialization method (say, serialize) that creates a std::vector or std::array of uint8_ts. This will make it very easy to see the exact structure of a proof, which currently has to be read by looking at the appropriate construct_proof function for calls to transcript.send_to_verifier. I like the explicitness of this struct solution, the fact that it localizes the proof information making it very easy to answer "what's in the proof", and believe that it will also help with debugging serialization issues (which I expect to be a major pain point farther down the line).

Does that make sense? Are you saying you'd like to have a crack at this?

@codygunton
Copy link
Collaborator Author

Btw @ludamad if our proofs were structs, would we want this serialization method, or do we have some magic with msgpack that would let us deserialize to structs? I feel like the potential to catch bugs here is huge.

@maramihali
Copy link
Contributor

Also folding "proofs" when implemented

@codygunton
Copy link
Collaborator Author

To be closed by AztecProtocol/aztec-packages#2668

@lucasxia01
Copy link
Contributor

lucasxia01 commented Oct 25, 2023

Also folding "proofs" when implemented

@maramihali What do you mean by this? That we need to structure these proofs, or that it would help with implementing the folding scheme?

@maramihali
Copy link
Contributor

Not entirely sure how you ended up structuring the transcript, happy to discuss more concretely once you have the PR ready. The point is that, ideally, folding proofs (which are not exactly proofs, just data to construct an accumulator) should be structured as well rather than std::vector<uint8_t> which is what you extract from the current transcript atm.

lucasxia01 added a commit to AztecProtocol/aztec-packages that referenced this issue Oct 31, 2023
Adding structuring feature to transcript based on the flavor and
resolves AztecProtocol/barretenberg#656.

The main two functions added are `deserialize_full_transcript()` and
`serialize_full_transcript()`, which are defined for each flavored
transcript derived class. These classes are defined in the flavor
classes, and primarily declare all of the transcript objects as member
variables.

`deserialize_full_transcript()` will take a full proof generated by the
flavored prover, and put all those bytes into the member variables.
Then, one can now view and modify the objects by just using the member
variables. In order to actually push the modification, one has to call
`serialize_full_transcript()` in order to regenerate the proof from the
(modified) member variables. The tests in `(flavor)_transcript.test.cpp`
illustrate how to use these functions in more depth. I also modified the
tests in `stdlib/recursion/honk/verifier/goblin_verifier.test.cpp` and
in `stdlib/recursion/honk/verifier/verifier.test.cpp`.

Got rid of the ProverTranscript and VerifierTranscript and merging them
into the BaseTranscript. This was done to avoid having to create these
for every flavor derived class and because they did not provide much on
top of the BaseTranscript.

Created transcript tests for flavors besides Ultra.

Transcript breakdown for all flavors:
https://docs.google.com/spreadsheets/d/1TpkkPj9o5ZXr_kWT7WsquXSEZY10C1Lt3BWeCA7Qe9U/edit#gid=0.

Some design considerations:
https://hackmd.io/o8HbTiujTGa4sLccAHQaxw?both.
AztecBot pushed a commit that referenced this issue Nov 7, 2023
Adding structuring feature to transcript based on the flavor and
resolves #656.

The main two functions added are `deserialize_full_transcript()` and
`serialize_full_transcript()`, which are defined for each flavored
transcript derived class. These classes are defined in the flavor
classes, and primarily declare all of the transcript objects as member
variables.

`deserialize_full_transcript()` will take a full proof generated by the
flavored prover, and put all those bytes into the member variables.
Then, one can now view and modify the objects by just using the member
variables. In order to actually push the modification, one has to call
`serialize_full_transcript()` in order to regenerate the proof from the
(modified) member variables. The tests in `(flavor)_transcript.test.cpp`
illustrate how to use these functions in more depth. I also modified the
tests in `stdlib/recursion/honk/verifier/goblin_verifier.test.cpp` and
in `stdlib/recursion/honk/verifier/verifier.test.cpp`.

Got rid of the ProverTranscript and VerifierTranscript and merging them
into the BaseTranscript. This was done to avoid having to create these
for every flavor derived class and because they did not provide much on
top of the BaseTranscript.

Created transcript tests for flavors besides Ultra.

Transcript breakdown for all flavors:
https://docs.google.com/spreadsheets/d/1TpkkPj9o5ZXr_kWT7WsquXSEZY10C1Lt3BWeCA7Qe9U/edit#gid=0.

Some design considerations:
https://hackmd.io/o8HbTiujTGa4sLccAHQaxw?both.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants