Add Ligero PCS #125

autquis · 2023-09-29T10:56:46Z

So, this is Ligero PCS. We had to copy-paste some code from jellyfish since jellyfish depends on the release 0.4.0 of crypto-primitive, which does not have CanonicalSerialize. Once jellyfish updates its dependency, we can use their crate.

Also, in Cargo.toml, we use a specific revision of crypto-primitive since later commits break KZG and other PCSs in the repo. I'll address this issue in another PR.

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

Targeted PR against correct branch (master)
Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
Wrote unit tests
Updated relevant documentation in the code
Added a relevant changelog entry to the Pending section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer

…unctions

PCS Ligero constructor

… compiling

…d work on commit function

Commit

Commitment with transcript

* Add Breakdown and Ligero to `README.md` * Fix newline * Apply suggestions from code review remove brakedown reference for now --------- Co-authored-by: mmagician <marcin.gorny.94@protonmail.com>

* make test util functions cfg-gated * Add constant_poly_test * Add quadratic tests * Add linear poly tests * Add all other tests from test suite * fix check_combinations fix combinations remove prints remove old comments * remove deprecated deny * change the signature of rand_point to allow other fields * remove tests superseded by automated test suite * Create a `new` method for `LigeroPCUniversalParams` * document `LigeroPCUniversalParams`

…clippy lints (#31) * replace direct construction by `new` call to LigeroPCUniversalParams * some clippy fixes * replace `todo()`s by `default()` or `()` * move implementation of `Ligero` to data_structures * Add more info to `Ligero` struct incl. link and no-hiding note

* Copy from `jellyfish` and patch dependency * Use `borrow` from `ark-std` * Remove default from `PhantomData`

mmagician · 2023-09-29T11:50:48Z

We would like to thank Modulus Labs @Modulus-Labs for the support, motivation and excellent discussions around the implementation and security of Ligero; and Aleo @AleoHQ for the hackathon sponsorship where some of the development and progress were discussed.

* use Vec/ToString from ark_std * rayon should only be enabled under the parallel feature * for no-std targets, enable Float arithmetic from num_traits

Pratyush · 2023-09-29T17:19:15Z

src/ligero/utils.rs

+pub(crate) struct IOPTranscript<F: PrimeField> {
+    transcript: Transcript,
+    is_empty: bool,
+    #[doc(hidden)]
+    phantom: PhantomData<F>,
+}
+
+// TODO: merge this with jf_plonk::transcript
+impl<F: PrimeField> IOPTranscript<F> {


This seems to me be yet another implementation of a sponge. We should try to consolidate all the usages so that we don't have duplication. cc @mmaker

It is indeed a YASI (yet-another-sponge-implentation).
Let me give some background. The idea was to use concrete instantiation directly, instead of adding more generics for the user to choose from. We already have some hashing-related generics on Ligero due to this PCS inherently relying on hashing in multiple places. Since we also rely on interaction, we additionally need to use a Fiat-Shamir transcript as part of the construction. In principle, one could opt in for a different choice for the hash for transcript than what is used for hashing the columns.

There is actually one further spot where potentially a different hash function can (and maybe later, should) be used: hashing the leaves of the Merkle Tree. This distinction becomes evident especially when we consider sth like Poseidon, where the rate of 2 makes much more sense for MT, whereas a higher rate can speed up hashing large columns. So that brings the total choice of hashing parameters (in the non-interactive scheme) to 3. The current design attempts to simplify that interface by forcing a transcript sponge on the user, and also forcing the same hash parameters for column and MT.

I think we could swap out IOPTranscript (which uses merlin under the hood) used here for something from crypto-primitives - the problem is that we don't have any good defaults provided there for the sponge construction AFAIK, and it wasn't clear to me how to safely choose the parameters for non-testing purposes.

src/ligero/data_structures.rs

Pratyush · 2023-09-29T17:22:37Z

src/ligero/data_structures.rs

+    <<C as Config>::TwoToOneHash as TwoToOneCRHScheme>::Parameters: Debug,
+    <<C as Config>::LeafHash as CRHScheme>::Parameters: Debug,


Perhaps we should just make Parameters: Debug in crypto-primitives directly? E.g. a companion to this PR: arkworks-rs/crypto-primitives#114

That should be resolved in 61f3c64

Pratyush · 2023-09-29T17:23:27Z

src/ligero/data_structures.rs

+        if F::TWO_ADICITY < self.rho_inv as u32 {
+            0


We should probably just return an error if this happens

We actually do: https://github.com/HungryCatsStudio/poly-commit/blob/46a6a859c3fb15427a1659124d119c550b415701/src/ligero/mod.rs#L80.
Without changing the interface we just propagate the zero to the caller and check there.

Pratyush · 2023-09-29T17:24:13Z

src/ligero/data_structures.rs

+    <<C as Config>::TwoToOneHash as TwoToOneCRHScheme>::Parameters: Debug,
+    <<C as Config>::LeafHash as CRHScheme>::Parameters: Debug,


If you're ignoring the parameters for debugging, then I guess you don't need this bound, no?

Great point, makes the trait bounds much cleaner!

Pratyush · 2023-09-29T17:25:13Z

src/ligero/data_structures.rs

+    }
+}
+
+pub(crate) type LigeroPCPreparedCommitment = ();


Perhaps this should just be the standard commitment, as opposed to ()?

Yes, but maybe instead: #127

Pratyush · 2023-09-29T17:25:38Z

src/ligero/data_structures.rs

+impl PCRandomness for LigeroPCRandomness {
+    fn empty() -> Self {}
+
+    fn rand<R: RngCore>(
+        _num_queries: usize,
+        _has_degree_bound: bool,
+        _num_vars: Option<usize>,
+        _rng: &mut R,
+    ) -> Self {
+    }
+}


Maybe these methods should return unimplemented!() for now?

Pratyush · 2023-09-29T17:27:27Z

src/ligero/mod.rs

+    ) -> Result<(Self::CommitterKey, Self::VerifierKey), Self::Error> {
+        if pp.max_degree() == 0 {
+            return Err(Error::InvalidParameters(
+                "This field is not suitable for the proposed parameters".to_string(),


Might make sense to make this a const and reuse this.

Done: 780f525

Pratyush · 2023-09-29T17:29:04Z

src/ligero/mod.rs

+        Self::Randomness: 'a,
+        Self::Commitment: 'a,


Should we add 'static bounds to Self::Randomness and Self::Commitment in the trait definition directly?

Do you mean like:

... Self::Randomness: 'static, Self::Commitment: 'static, { let mut proof_array = LPCPArray::default(); let labeled_commitments: Vec<&LabeledCommitment<Self::Commitment>> = commitments.into_iter().collect();

?

Pratyush · 2023-09-29T17:32:23Z

btw it would be useful to compare notes with https://github.com/arkworks-rs/ldt and https://github.com/arkworks-rs/bcs and see if any infrastructure can be combined/reused

mmagician · 2023-09-29T17:43:48Z

Definitely! I was mostly oblivious to those repos.

mmagician · 2023-11-03T19:58:04Z

Closing in favour of #132

mmagician and others added 30 commits June 27, 2023 12:07

Stub out the PolynomialCommitment trait for Ligero

a4c6e2b

implementing matrix functionality, not tested

8f6becd

further matrix functionality, commit function started

aa94494

testing commit signing

d0f00b1

first part of commit function compiling

848933b

requested PR changes implemented

543c8e1

added to_field function one-liner and tests for all existing matrix f…

a39289d

…unctions

changed matrix-related functions from F:PrimeField to F:Field

d82c4e3

Add params and generics to the Ligero constructor

c20d28d

Create a custom and default constructors for Ligero PCS struct

008151b

Merge pull request #2 from mmagician/pcs-constructor

ffea25d

PCS Ligero constructor

in the middle of commit function, not compiling

91184aa

merged with ligero (constructor added)

2941c8a

added reed-solomon encoding and a test for the fft interface

4ef9adb

continued commit method, left placeholders for Merkle-tree parts; NOT…

b008983

… compiling

changed reed-solomon code to receive fft_domain from caller; continue…

c13cf28

…d work on commit function

make a Commitment struct with MT root & transcipt

dd7e28f

stub out the Commitment verify method (well-formedness check)

bb3b6bb

Ligero with a sponge for Fiat-Shamir Commitment well-formed check

1d6c30b

use jf_primitives for IOPTranscript for FS transform

129cbfb

deleted settings.json file

7183c1e

Merge pull request #1 from Antonio95/commit

903eaaf

Commit

Merge branch 'ligero' into commitment-with-transcript

5d349b6

Merge pull request #4 from mmagician/commitment-with-transcript

edae282

Commitment with transcript

remove merge artifacts

fe63bc2

Fix getting the indices from the transcript

d58c366

path verification with the random index retrieved from the transcript

aa913ef

verify the random linear combinations in the F-S commitment check

243be3d

hashing columns - WIP

4c7631d

well_formedness_check completed, not tested

348c88e

autquis and others added 5 commits September 28, 2023 10:07

Add Breakdown and Ligero to README.md (#27)

d450683

* Add Breakdown and Ligero to `README.md` * Fix newline * Apply suggestions from code review remove brakedown reference for now --------- Co-authored-by: mmagician <marcin.gorny.94@protonmail.com>

code and comments cleanup (#30)

2b72639

Copy from jellyfish and patch dependency (#32)

9e40efb

* Copy from `jellyfish` and patch dependency * Use `borrow` from `ark-std` * Remove default from `PhantomData`

autquis requested a review from a team as a code owner September 29, 2023 10:56

autquis requested review from z-tech, mmagician and weikengchen and removed request for a team September 29, 2023 10:56

Merge branch 'master' into ligero

6ce243b

no-std compat (#33)

840ee52

* use Vec/ToString from ark_std * rayon should only be enabled under the parallel feature * for no-std targets, enable Float arithmetic from num_traits

Pratyush reviewed Sep 29, 2023

View reviewed changes

src/ligero/data_structures.rs Outdated Show resolved Hide resolved

mmagician added 3 commits September 29, 2023 11:21

Merge branch 'master' into ligero

6e2c946

post merge fmt

46a6a85

place all PhantomData in one struct

a46ea25

Pratyush reviewed Sep 29, 2023

View reviewed changes

mmagician added 6 commits October 4, 2023 21:03

remove Debug bounds

61f3c64

PreparedCommitment = Commitment

44d2004

define an error string once, reuse in setup and trim

780f525

Merge branch 'master' into ligero

46a28c8

LigeroPCRandomness left temporarily unimplemented

8d15d4e

empty is unimplemented, so let's explicitly insert ()

6f3c663

mmagician mentioned this pull request Oct 25, 2023

Add Ligero univariate and multilinear PCS #132

Merged

6 tasks

mmagician closed this Nov 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Ligero PCS #125

Add Ligero PCS #125

autquis commented Sep 29, 2023

mmagician commented Sep 29, 2023

Pratyush Sep 29, 2023

mmagician Oct 4, 2023

Pratyush Sep 29, 2023

mmagician Oct 4, 2023

Pratyush Sep 29, 2023

mmagician Oct 4, 2023

Pratyush Sep 29, 2023

mmagician Oct 4, 2023

mmagician Oct 4, 2023

Pratyush Sep 29, 2023

mmagician Oct 4, 2023

Pratyush Sep 29, 2023

mmagician Oct 4, 2023

Pratyush Sep 29, 2023

mmagician Oct 4, 2023

Pratyush Sep 29, 2023

mmagician Oct 4, 2023

Pratyush commented Sep 29, 2023

mmagician commented Sep 29, 2023

mmagician commented Nov 3, 2023

		<<C as Config>::TwoToOneHash as TwoToOneCRHScheme>::Parameters: Debug,
		<<C as Config>::LeafHash as CRHScheme>::Parameters: Debug,

Add Ligero PCS #125

Add Ligero PCS #125

Conversation

autquis commented Sep 29, 2023

mmagician commented Sep 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pratyush commented Sep 29, 2023

mmagician commented Sep 29, 2023

mmagician commented Nov 3, 2023