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

Transcript usage inclusion into the codebase #43

Merged
merged 42 commits into from
Aug 2, 2023
Merged

Conversation

CPerezz
Copy link
Member

@CPerezz CPerezz commented Jul 25, 2023

This should not be merged unti #41 gets into main.

The PR introduces usage from the associated trait/type of Group called TranscriptEngineTrait to perform the Fiat-Shamir inside the multifolding for the Sigmas and Thetas computations as well as gammas.

This should be rebased on the top of main once #41 is merged and then reviewed. As there could be places where FS should be applied but it isn't.

This PR moves a tep forward to the NIMFS-centric API.
First, removes all the `_Witness` & `_Instance` structs for CCS & CCCS
as they added a bunch of overhead.

Sedonc, it also re-assembles internal fm APIs so that they ask for
slices instead of refs to `Vec`.

Finally, reassembles all the tests to indeed modify CCS & CCCS
interaction as described avobe.
Removes CCS & CCS_MLEs from the struct as well as renaming some
variables.
Aside from that, the functions are being modified so that they always
get the removed fields as refs.
This gets rid of conversions that made no sense from a theoretical
standpoint and were overtaken by a better API design.
Since we need to generate LCCCS instances from no-where to actually be
able to test things or startup the NIMFS object.
LCCCS already contains `z` inside of it. So it's non-sensical to require
twice the same thing that was already served to the struct in the pass
as a creation attribute.
This allows to either generate a `Multifolding` instance from all of
it's params or, instead, from the first `z` and confectioning the LCCCS.
This introduces `Transcript` into NIMFS to have a correct way to fetch
randomness to create Sigmans, Thetas and any other randomness required
by the protocol in the Sum Check for example.

- The commit modifies the `init` to now receive a seed label for the
  Transcript. It also adds the first witness & CCS M MLE into the
  transcript.
- The commit also adds an associated fn for NIMFS `gen_r_x` which allows
  the NIMFS to query the challenge enough times to actually generate the
  `r_x` vector needed to compute sigmas and thetas.
Now, instead of getting passed-by randomness, the NIMFS generates
`r_x_prime` and `rho` by querying the inner transcript instance it
holds.
@CPerezz CPerezz marked this pull request as ready for review July 31, 2023 16:48
@CPerezz
Copy link
Member Author

CPerezz commented Jul 31, 2023

Note that since I'm stupid and decided to squash #41 accidentally, I could not rebase here..

Copy link
Collaborator

@oskarth oskarth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, some comments/questions

src/ccs/multifolding.rs Outdated Show resolved Hide resolved
src/ccs/multifolding.rs Outdated Show resolved Hide resolved
src/ccs/multifolding.rs Outdated Show resolved Hide resolved
src/ccs/multifolding.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@asn-d6 asn-d6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All in all, looks reasonable. Added a few nits. Most of them can be handled post-merge IMO.

src/ccs/lcccs.rs Show resolved Hide resolved
let w_comm = <G as Group>::CE::commit(&ck, &w);

let r_x: Vec<G::Scalar> = vec![G::Scalar::random(&mut rng); ccs.s];
let v = ccs.compute_v_j(&z, &r_x, &ccs_mle);
// Query challenge to get initial `r_x`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yikes, 4 lines to squeeze a challenge? Can we add helper functions for this? Maybe just make gen_r_x() take as input the size?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only happens here because the NIMFS struct hasn't been created yet. From here on, there's a fn that does exactly what you need called exactly as you said.

Is that ok? Or you'd still prefer a helper fn not associated to any struct to do this?

src/ccs/multifolding.rs Outdated Show resolved Hide resolved
@CPerezz CPerezz merged commit 0c8915b into main Aug 2, 2023
1 check passed
@CPerezz CPerezz deleted the transcript_inclusion branch August 2, 2023 10:14
@CPerezz CPerezz restored the transcript_inclusion branch August 2, 2023 10:14
@CPerezz CPerezz deleted the transcript_inclusion branch August 2, 2023 10:14
@CPerezz CPerezz restored the transcript_inclusion branch August 2, 2023 10:14
hero78119 pushed a commit to hero78119/SuperNova that referenced this pull request Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants