feat: New stdlib Transcript #1219
Conversation
✅ Deploy Preview for preeminent-bienenstitch-606ad0 canceled.
|
|
FYI was looking into that l1-contracts issue, it's known at least, not sure what's causing it though |
ledwards2225
force-pushed
the
lde/stdlib_transcript
branch
from
66776da to
c63dbe2
Compare
ledwards2225
force-pushed
the
lde/stdlib_transcript
branch
from
4fbd067 to
8b7e7b1
Compare
| using VerifierTranscript = proof_system::honk::VerifierTranscript<FF>; | ||
| template <size_t LENGTH> using Univariate = proof_system::honk::sumcheck::Univariate<FF, LENGTH>; | ||
|
|
||
| static constexpr size_t HASH_OUTPUT_SIZE = 32; // WORKTODO: Duplicated from native transcript |
There was a problem hiding this comment.
resolved, thanks
| native_challenges = native_transcript.get_challenges(labels...); | ||
|
|
||
| /* | ||
| * TODO(luke): Do stdlib hashing here. E.g., for the current pedersen/blake setup, we could write data into a |
There was a problem hiding this comment.
Could you make an issue int he aztec-packages repo and put in the appropriate slot in the project board and link here?
circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/transcript/trancript.hpp
Outdated
Show resolved
Hide resolved
| * @param element | ||
| * @return field_pt | ||
| */ | ||
| field_pt stdlib_type_from_witness(uint32_t native_element) |
There was a problem hiding this comment.
Could you use existing functionality? for this and similar functions below? For instance, we have a function field_t<Builder>::from_witness. These things are generally useful and therefore should be constructors or member functions on the appropriate types.
There was a problem hiding this comment.
I'm not sure I follow. Do you just mean e.g. change this from witness_pt(builder, native_element); to field_pt::from_witness(builder, native_element)?
There was a problem hiding this comment.
I mean rather than defining these little utility functions in the class, you should use existing functions or put the utility functions in the appropriate classes -- if you're interested in having these constructor-like functions, then somebody else might be. If you're hesitant to put them there then maybe they shouldn't exist at all?
There was a problem hiding this comment.
I'm having a hard time finding a clean solution along the lines of what you're describing. I've taken a half-measure for now and moved the "from witness" functions into a utility class in the stdlib so at least they are separated from the Transcript.
It's possible I'm not understanding your suggestion well but an example of why it seems difficult: The template type for receive_from_prover() is currently the native type to be extracted and there are no existing methods that map native types to stdlib types, as far as I know. In theory I could add a function to_field_t() or something to field, but as far as I can tell the native types don't currently know anything about stdlib types so that feels like a major departure. Then there are types like uint32_t (which I cant add methods to unless I create a wrapper for the c++ std library type) and std::array<field> which is not a first class type to which I could add methods.
We do have from_witness methods implemented on the stdlib field/group types but I still need to connect the stdlib type to the native type that I'm trying to extract from the native transcript. So for example if I changed the template parameter in receive_from_prover() to a stdlib type, I'm not sure how I would distinguish between extracting a uint32_t or fr from the native transcript.
Anyway, I'm very open to making this better so if you have suggestions I'm happy to keep trying. Otherwise I'd request that we get this in as is and I can make a proper issue for it in case we want to improve it in the future. Either way I'd say it's a substantial improvement over what the existing recursive transcript does.
|
It's a small thing but also: do you mind not using the alians |
ledwards2225
force-pushed
the
lde/stdlib_transcript
branch
from
8b7e7b1 to
776a37a
Compare
ledwards2225
force-pushed
the
lde/stdlib_transcript
branch
from
b185256 to
8cdcb5a
Compare
Description
Basic structure for a new stdlib Transcript corresponding to the new native transcript used by the Honk proving systems. Implements all functions required with the CAVEAT that no constraints are imposed for hashing in
get_challenge. May be useful to do this properly at some point but avoiding it for now since it was non-trivial and the hash will change anyway.Checklist: