private-attribution / ipa

A raw implementation of Interoperable Private Attribution
MIT License
40 stars 23 forks source link

Add constants for proof parameter sets #1105

Closed danielmasny closed 3 months ago

danielmasny commented 3 months ago

add constants & add small parameter set

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 91.08%. Comparing base (6657e45) to head (678de63). Report is 5 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1105 +/- ## ========================================== + Coverage 90.98% 91.08% +0.09% ========================================== Files 185 186 +1 Lines 26213 26260 +47 ========================================== + Hits 23850 23918 +68 + Misses 2363 2342 -21 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

andyleiserson commented 3 months ago

To reduce the code duplication, how about this: https://github.com/danielmasny/ipa/compare/aliases-for-constants...andyleiserson:ipa:prover-configs?diff=split&w=

There are two commits on that branch with two different versions. One (the first commit) puts the F: PrimeField on the struct, and the other (the second commit / head of the branch) puts it on the functions. The benefit of putting it on the functions is it avoids PhantomData, but requires more type annotations where things are invoked. Putting it on the struct (and thus in the type alias) eliminates the most type annotations, but requires PhantomData.

Putting it on the struct also might enable caching the Lagrange table within the prover, but doing that is more involved due to the limitations on generic statics.

Overall I prefer the first version.

danielmasny commented 3 months ago

To reduce the code duplication, how about this: https://github.com/danielmasny/ipa/compare/aliases-for-constants...andyleiserson:ipa:prover-configs?diff=split&w=

There are two commits on that branch with two different versions. One (the first commit) puts the F: PrimeField on the struct, and the other (the second commit / head of the branch) puts it on the functions. The benefit of putting it on the functions is it avoids PhantomData, but requires more type annotations where things are invoked. Putting it on the struct (and thus in the type alias) eliminates the most type annotations, but requires PhantomData.

Putting it on the struct also might enable caching the Lagrange table within the prover, but doing that is more involved due to the limitations on generic statics.

Overall I prefer the first version.

To reduce the code duplication, how about this: https://github.com/danielmasny/ipa/compare/aliases-for-constants...andyleiserson:ipa:prover-configs?diff=split&w=

There are two commits on that branch with two different versions. One (the first commit) puts the F: PrimeField on the struct, and the other (the second commit / head of the branch) puts it on the functions. The benefit of putting it on the functions is it avoids PhantomData, but requires more type annotations where things are invoked. Putting it on the struct (and thus in the type alias) eliminates the most type annotations, but requires PhantomData.

Putting it on the struct also might enable caching the Lagrange table within the prover, but doing that is more involved due to the limitations on generic statics.

Overall I prefer the first version.

I also like the first version @benjaminsavage what do you think?

benjaminsavage commented 3 months ago

I like @andyleiserson's first version best as well.