Setup SuperNova public parameter infrastructure #51

winston-h-zhang · 2023-09-19T18:36:32Z

We split out the PublicParams within each RunningClaim into a new supernova::PublicParams. This split prepares for downstream changes in lurk-rs (argumentcomputer/lurk-beta#648) to working with the public parameter infra.

We aggregate the common primary commitment key and any other shared information amongst all the params into the main PublicParams structure and create a new CircuitShape structure to manage per-circuit parameters.
We mirror the Nova API and modify the supernova prove/verify functions to take a public params, as well as computing digests.
We remove RunningClaims, which ends being a glorified usize.

Closes #29

huitseeker

I think your intuition that the old RunningClaim is a shallow, and mostly unnecessary shell around a Vec of PublicParams is mostly correct, but I left comments inline that make me think the current struct re-organization isn’t quite where we want to be. I’ve left comments inline, but overall:

the new RunningClaim is even more shallow (it’s a glorified usize)
it’s become hard to set the commitment keys, because they are copied in every PublicParam.

I think some principles that point the direction of a better place are:

the CK is common to a whole proof,
in the PublicParams, everything but the circuit shapes is mutualized,
the RunningClaims only exist in their plural form, we have usize-indexed APIs that can get the required info out of them,
in order to not have to revamp all the APIs, we pass structs (parametrized by a ’ a lifetime) that embark references to shared elements rather than copies.

src/lib.rs

huitseeker · 2023-09-22T19:37:37Z

src/supernova/mod.rs

+  /// realign them if necessary. If a digest is given, we assume that it
+  /// will correctly match the public params after realigning them, and
+  /// we do not check for its validity.
+  pub fn from_pp_vec(pp_vec: Vec<PublicParams<G1, G2>>, digest: OnceCell<G1::Scalar>) -> Self {


Have you tried implementing FromIterator<PublicParams<G1, G2>> for RunningClaimParams<G1, G2> instead? you’d save a lot of allocations!

I'm consuming the vector so it has to be allocated

This is obsolete given the new from_parts, but the idea is to avoid the allocation of the vector that precedes this call.

src/supernova/mod.rs

src/lib.rs

src/supernova/mod.rs

src/lib.rs

winston-h-zhang · 2023-09-25T05:53:42Z

@huitseeker The biggest changes from your suggestions was getting rid of RunningClaims and just using a usize, renaming RunningClaimParams to PublicParams and PublicParams to CircuitParams.

huitseeker

Would it be possible to adapt benches/recursive-snark-supernova.rs?

src/supernova/mod.rs

src/lib.rs

src/supernova/mod.rs

src/lib.rs

huitseeker

Just one remark left, otherwise, this LGTM! You may want to mark that this closes #29

src/supernova/mod.rs

huitseeker

This LGTM, but the current version of Lurk would break (gist) if we were to merge this, so I'll wait until argumentcomputer/lurk-beta#648 is ready to stamp here.

src/supernova/test.rs

src/supernova/mod.rs

huitseeker

Thanks a bunch!

arthurpaulino · 2023-10-02T13:53:15Z

Is this a breaking change?

huitseeker · 2023-10-02T14:35:45Z

Is this a breaking change?

Yes.

This reverts commit c657b04. See upstream issue: argumentcomputer/lurk-beta#720

This backports the following Arecibo PRs: - #2 - #3 - #10 - #16 - #23 - #30 - #28 - #41 - #45 - #50 - #56 - #51 - #72 - #92 - #95 - #97 - #101 - #110 - #106 - #112 - #114 - #119 - #120 - #127 - #123 - #131 - #174 - #175 - #182 Co-authored-by: WYATT <[email protected]> Co-authored-by: Hanting Zhang <[email protected]> Co-authored-by: Ming <[email protected]> Co-authored-by: porcuquine <[email protected]> Co-authored-by: Samuel Burnham <[email protected]> Co-authored-by: Matej Penciak <[email protected]> Co-authored-by: Adrian Hamelink <[email protected]>

winston-h-zhang requested review from huitseeker and porcuquine and removed request for huitseeker September 19, 2023 18:38

winston-h-zhang force-pushed the supernova-digest branch 6 times, most recently from dec2f5a to a9d6c90 Compare September 21, 2023 00:00

winston-h-zhang marked this pull request as ready for review September 21, 2023 00:06

winston-h-zhang force-pushed the supernova-digest branch 2 times, most recently from 859586a to 7009e58 Compare September 21, 2023 00:22

winston-h-zhang mentioned this pull request Sep 21, 2023

Intergrate SuperNova's PublicParams, benchmark NIVC, and Stablize Param API argumentcomputer/lurk-beta#648

Merged

huitseeker reviewed Sep 22, 2023

View reviewed changes

winston-h-zhang force-pushed the supernova-digest branch 2 times, most recently from 9144e96 to 4fda86b Compare September 25, 2023 04:37

winston-h-zhang mentioned this pull request Sep 25, 2023

Manage CommitmentKeyHints properly in supernova #53

Closed

winston-h-zhang commented Sep 25, 2023

View reviewed changes

src/lib.rs Outdated Show resolved Hide resolved

winston-h-zhang force-pushed the supernova-digest branch from 4fda86b to 874c06c Compare September 25, 2023 05:52

huitseeker reviewed Sep 25, 2023

View reviewed changes

winston-h-zhang force-pushed the supernova-digest branch from 874c06c to 31a4e88 Compare September 26, 2023 01:13

winston-h-zhang requested a review from huitseeker September 26, 2023 01:13

huitseeker reviewed Sep 26, 2023

View reviewed changes

src/supernova/mod.rs Show resolved Hide resolved

winston-h-zhang force-pushed the supernova-digest branch from 31a4e88 to 85049c5 Compare September 26, 2023 18:26

winston-h-zhang requested a review from huitseeker September 26, 2023 18:26

huitseeker reviewed Sep 26, 2023

View reviewed changes

src/supernova/test.rs Outdated Show resolved Hide resolved

src/supernova/mod.rs Outdated Show resolved Hide resolved

winston-h-zhang force-pushed the supernova-digest branch from 85049c5 to f486c10 Compare September 26, 2023 20:30

winston-h-zhang requested a review from huitseeker September 26, 2023 20:51

winston-h-zhang force-pushed the supernova-digest branch from 7f07a46 to 5eb2983 Compare September 29, 2023 21:26

CircuitParams -> CircuitShape

5eb3117

winston-h-zhang force-pushed the supernova-digest branch from 5eb2983 to 5eb3117 Compare September 29, 2023 21:30

add and fix pp tests

d63af52

winston-h-zhang changed the title ~~Split out RunningClaimParams from RunningClaims~~ Setup SuperNova public parameter infrastructure Sep 30, 2023

huitseeker approved these changes Oct 2, 2023

View reviewed changes

winston-h-zhang added this pull request to the merge queue Oct 2, 2023

Merged via the queue into dev with commit c657b04 Oct 2, 2023
2 checks passed

winston-h-zhang deleted the supernova-digest branch October 2, 2023 14:30

arthurpaulino mentioned this pull request Oct 2, 2023

minor public-params issues argumentcomputer/lurk-beta#719

Open

huitseeker added a commit that referenced this pull request Oct 2, 2023

Revert "Setup SuperNova public parameter infrastructure (#51)"

11be0ff

This reverts commit c657b04. See upstream issue: argumentcomputer/lurk-beta#720

huitseeker added the supernova label Dec 17, 2023

huitseeker mentioned this pull request Dec 18, 2023

feat: implement SuperNova microsoft/Nova#283

Open

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Setup SuperNova public parameter infrastructure #51

Setup SuperNova public parameter infrastructure #51

winston-h-zhang commented Sep 19, 2023 •

edited

Loading

huitseeker left a comment

huitseeker Sep 22, 2023

winston-h-zhang Sep 25, 2023

huitseeker Sep 25, 2023

winston-h-zhang commented Sep 25, 2023

huitseeker left a comment

huitseeker left a comment •

edited

Loading

huitseeker left a comment

huitseeker left a comment

arthurpaulino commented Oct 2, 2023

huitseeker commented Oct 2, 2023

Setup SuperNova public parameter infrastructure #51

Setup SuperNova public parameter infrastructure #51

Conversation

winston-h-zhang commented Sep 19, 2023 • edited Loading

huitseeker left a comment

Choose a reason for hiding this comment

huitseeker Sep 22, 2023

Choose a reason for hiding this comment

winston-h-zhang Sep 25, 2023

Choose a reason for hiding this comment

huitseeker Sep 25, 2023

Choose a reason for hiding this comment

winston-h-zhang commented Sep 25, 2023

huitseeker left a comment

Choose a reason for hiding this comment

huitseeker left a comment • edited Loading

Choose a reason for hiding this comment

huitseeker left a comment

Choose a reason for hiding this comment

huitseeker left a comment

Choose a reason for hiding this comment

arthurpaulino commented Oct 2, 2023

huitseeker commented Oct 2, 2023

winston-h-zhang commented Sep 19, 2023 •

edited

Loading

huitseeker left a comment •

edited

Loading