Feature/sumcheck #40
Feature/sumcheck #40
Conversation
|
hypernova working with poseidon based transcript here |
|
Is this ready for review @dmpierre ? |
Yes thanks! Forgot to assign, so just added Arnau as well |
|
Added a raw sum-check implementation in the meantime, might help to get rid of |
|
@arnaucube pinged me about code duplication. So I'll save time and review once the changes have been addressed! Could you ping once done @dmpierre ?? |
I think we are getting closer! We eventually go for implementing a more generic trait over Keeping our own sum-check implementation for later since it will require additional work on top of it if we want to use it - things like removing the dependency to |
| .unwrap(); | ||
| let gamma_scalar = C::ScalarField::from_le_bytes_mod_order(b"gamma"); | ||
| transcript.absorb(&gamma_scalar); | ||
| let gamma: C::ScalarField = transcript.get_challenge(); |
There was a problem hiding this comment.
I'm not an expert on transcripts, so not sure about this, but if these strings are not essential maybe we could skip them so in-circuit don't add extra constraints?
The reasoning would be that we are already absorbing the value of the computed challenge, and we're interested into absorbing the values computed through the protocol to prevent the prover from 'choosing' the 'random challenges' obtainted, but some hardcoded strings would not affect that, and furthermore, later when we reproduce the transcript in-circuit, absorbing strings would add extra constraints (although a small amount).
Maybe the 'absorbing the string' tradition comes from Merlin (https://merlin.cool , used by the transcript from the Espresso impl), which allows to use strings as kind of separators when absorbing values into the transcript.
I've skimmed through the Merlin docs but didn't find a reason for absorbing the strings, but again, I'm not sure about this, so posting this as a question for others here. (tagging @CPerezz and @han0110 who might have more insight here)
There was a problem hiding this comment.
I think it's like a domain separator for the underlying hash function, zcash/halo2 does the same thing, so I guess it'd be a good practice.
But I also don't understand the theoretical part, and not sure where convention this comes from.
edited before merging
We want
folding_schemesto have a sum-check implementation generic over hash functions.To achieve this, we re-use
hyperplonk's sumcheck implementation. To be able to make it generic over any hash, we tweak it to be generic over our ownTranscripttrait (not the one found inhyperplonk).This PR:
SumCheck,SumCheckProverandSumCheckVerifiertraits to be generic over aCurveGroup, withproveandverifymethods generic overfolding_schemes::Transcript.IOPProverStateandIOPVerifierstates to be generic over aCurveGroup.Also, we change
hyperplonkcalls to its transcript object when re-implementing theproveandverifymethod cited above:transcript.append_serializable_elementbecomestranscript.absorb(example) ortranscript.absorb_vecwhen needed (example).transcript.get_and_append_challengebecomestranscript.get_challenge(example)The main disadvantage from taking this route is codebase inflation. However, we gain the short-term ability to have a somewhat robust and generic sum-check implementation. Still, although practical, it seems like a temporary implementation which might be re-evaluated in the future (e.g. implementing our own sum-check from scratch,
hyperplonk's implementation might have possible low-hanging optimizations).