feat: use Halo2 compatible dependencies #205

DrPeterVanNostrand · 2021-07-30T19:14:49Z

No description provided.

src/bls.rs

src/gpu/fft.rs

DrPeterVanNostrand · 2021-08-11T15:54:58Z

Note that this PR removes the ci jobs test_pairing_gpu_x86_64-unknown-linux-gnu and test_pairing_x86_64-unknown-linux-gnu and those github checks will never pass. I'm not sure how to remove those github checks and/or I may not have permission in CircleCI.

DrPeterVanNostrand

Cargo.toml's should be updated before merging.

DrPeterVanNostrand · 2021-08-13T14:06:50Z

Cargo.toml

-
-# pairing feature
-paired = { version = "0.22.0", optional = true }
+blstrs = { git = "https://github.com/filecoin-project/blstrs", branch = "halo" }


This should be changed before merging.

DrPeterVanNostrand · 2021-08-13T14:07:00Z

Cargo.toml


 # gpu feature 
 rust-gpu-tools = { version = "0.4.0", optional = true }
-ff-cl-gen = { version = "0.3.0", optional = true }
+ff-cl-gen = { git = "https://github.com/filecoin-project/ff-cl-gen", branch = "halo", optional = true }


This should be changed before merging.

vmx

Lots of mul_assign() where replaced by +, but not all of them (e.g. in src/groth16/aggregate/poly.rs). Not sure if that's a problem or not.

Some calculations changed a bit, but I'd expect that the test capture it if they were wrong now.

Lot's of comments, but all the them are rather style wise and nothing really important. So feel free to ignore those.

Cargo.toml

examples/verifier_bench/Cargo.toml

examples/verifier_bench/src/bin/main.rs

src/gadgets/test/mod.rs

src/groth16/aggregate/msm/fixed_base.rs

vmx · 2021-09-06T15:00:28Z

src/groth16/tests/dummy_engine.rs

+    type Output = Fr;
+
+    fn sub(self, rhs: &Fr) -> Fr {
+        self + -rhs


+ - might be needed here, but it reads funny.

yeah I think it is needed, definitely looks funny

vmx · 2021-09-06T15:04:35Z

src/multiexp.rs

@@ -194,54 +197,87 @@ impl DensityTracker {
    }
 }

+// Right shift the repr of a field element by `n` bits.


I haven't reviewed this part in-depth, if needed, let me know.

src/multiexp.rs

tests/groth16_aggregation.rs

tests/mimc.rs

This ensure it is actually checked on ci and testing and makes it easier to run

cryptonemo

Looks good. Exciting 👍

cryptonemo · 2021-09-17T17:09:25Z

Cargo.toml


 # gpu feature 
 rust-gpu-tools = { version = "0.4.0", optional = true }
-ff-cl-gen = { version = "0.3.0", optional = true }
+ec-gpu = { git = "https://github.com/filecoin-project/ec-gpu", branch = "master", optional = true }
+ec-gpu-gen = { git = "https://github.com/filecoin-project/ec-gpu", branch = "master", optional = true }


Just to clarify, is ff-cl-gen going to be released before this is merged, or are we sitting at master while things are in flux?

The plan is to sit on master until at least the cuda things are also merged

DrPeterVanNostrand force-pushed the halo branch 2 times, most recently from f3f3faa to 7f8c71f Compare August 4, 2021 18:27

dignifiedquire reviewed Aug 6, 2021

View reviewed changes

src/bls.rs Outdated Show resolved Hide resolved

dignifiedquire reviewed Aug 6, 2021

View reviewed changes

src/gpu/fft.rs Outdated Show resolved Hide resolved

DrPeterVanNostrand force-pushed the halo branch 3 times, most recently from 5ae9e10 to 4584efc Compare August 9, 2021 15:11

DrPeterVanNostrand marked this pull request as ready for review August 9, 2021 15:33

DrPeterVanNostrand force-pushed the halo branch 4 times, most recently from ccf34e3 to bab07b0 Compare August 10, 2021 20:13

DrPeterVanNostrand commented Aug 13, 2021

View reviewed changes

dignifiedquire force-pushed the halo branch from 4dc24fe to b8e0686 Compare August 24, 2021 13:02

dignifiedquire force-pushed the halo branch from 9346c3d to 8e7a2d7 Compare September 6, 2021 11:08

vmx reviewed Sep 6, 2021

View reviewed changes

dignifiedquire mentioned this pull request Sep 7, 2021

Migrate to upstream ff, group and pairing filecoin-project/rust-fil-proofs#1429

Closed

6 tasks

dignifiedquire force-pushed the halo branch 2 times, most recently from 7a147fb to dc763c8 Compare September 7, 2021 13:43

dignifiedquire mentioned this pull request Sep 10, 2021

refactor: remove Group abstraction in domain #224

Merged

DrPeterVanNostrand and others added 7 commits September 17, 2021 15:10

feat: use Halo2 compatible dependencies

410994a

refactor: move verifier-bench out of examples

ab3e359

This ensure it is actually checked on ci and testing and makes it easier to run

CR cleanup

ccd404c

improve multiscalar code

a621846

refactor: remove unused trait bounds

905ee9a

fixup: add back send for cs

00c9f7a

update deps

c833c2d

dignifiedquire force-pushed the halo branch from a83ecaa to 85a6905 Compare September 17, 2021 13:24

fixup

c7009b6

dignifiedquire force-pushed the halo branch from 85a6905 to c833c2d Compare September 17, 2021 13:29

style: fix clippy

e69818d

cryptonemo approved these changes Sep 17, 2021

View reviewed changes

dignifiedquire merged commit d1b9939 into master Sep 18, 2021

dignifiedquire deleted the halo branch September 18, 2021 10:34

cryptonemo mentioned this pull request Sep 21, 2021

Next Proofs Release (v10.0.0) filecoin-project/rust-fil-proofs#1494

Closed

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use Halo2 compatible dependencies #205

feat: use Halo2 compatible dependencies #205

DrPeterVanNostrand commented Jul 30, 2021

DrPeterVanNostrand commented Aug 11, 2021

DrPeterVanNostrand left a comment

DrPeterVanNostrand Aug 13, 2021

DrPeterVanNostrand Aug 13, 2021

vmx left a comment

vmx Sep 6, 2021

dignifiedquire Sep 7, 2021

vmx Sep 6, 2021

cryptonemo left a comment

cryptonemo Sep 17, 2021

dignifiedquire Sep 18, 2021

feat: use Halo2 compatible dependencies #205

feat: use Halo2 compatible dependencies #205

Conversation

DrPeterVanNostrand commented Jul 30, 2021

DrPeterVanNostrand commented Aug 11, 2021

DrPeterVanNostrand left a comment

Choose a reason for hiding this comment

DrPeterVanNostrand Aug 13, 2021

Choose a reason for hiding this comment

DrPeterVanNostrand Aug 13, 2021

Choose a reason for hiding this comment

vmx left a comment

Choose a reason for hiding this comment

vmx Sep 6, 2021

Choose a reason for hiding this comment

dignifiedquire Sep 7, 2021

Choose a reason for hiding this comment

vmx Sep 6, 2021

Choose a reason for hiding this comment

cryptonemo left a comment

Choose a reason for hiding this comment

cryptonemo Sep 17, 2021

Choose a reason for hiding this comment

dignifiedquire Sep 18, 2021

Choose a reason for hiding this comment