-
Notifications
You must be signed in to change notification settings - Fork 172
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
feat(icicle): Add icicle msm support #322
base: main
Are you sure you want to change the base?
Conversation
d55c28f
to
02582c0
Compare
Currently some of the e2e tests are failing due to failed Hyrax Proof verification. Initially investigating this it seems the msm results of icicle and jolt_core::msm::VariableBase::msm differ. After comparing the results of the msm variants with icicle_msm, it appears the results of icicle_msm and msm_bigint match but differ from the results of msm_u64 and msm_binary. On Sam Rags advice I compared the results icicle_msm() and msm() for random distribtutions of different scalars as following:
The results of these tests match my earlier attempts at debugging (the result of icicle matches msm_bigint and is different for the other msm_vairants). However across 10 runs the results of icicle_msm() and msm_bigint() were different 2 times. My suspicion is that is why the e2e tests are inconsistently failing. Pinging the icicle team for more help: @jeremyfelder @LeonHibnik @DmytroTym for visibility |
It's quite possible this is a bug in the custom Jolt MSMs. We use the same function in prove and verify so I don't think it would cause a verification error on the opening proofs. |
I've isolated a failing test case (below) where msm_bigint() is different from icicle_msm(). This is for msm_length = 3 on bn254 G1. I compared the output of msm_bigint, icicle_msm to arkworks_msm as a baseline. Icicle_msm differs from both msm_bigint and arkworks_msm which match. scalars: bases: Icicle_msm: msm_bigint/arkworks_msm: |
For reference we had some conversion logic between |
654a9ac
to
91bc1a1
Compare
f7da5ae
to
28fb544
Compare
28fb544
to
908bf07
Compare
908bf07
to
1705f51
Compare
1705f51
to
a9d4ba0
Compare
a9d4ba0
to
f23c20a
Compare
Current perf on an H100 accelerates sha2-chain 40% E2E for {Zeromorph, HyperKZG}. Currently slows down hyrax due to the tiny MSMs and call overhead. Remaining waste:
|
hey, I tried this PR and built successfully but I don't see it use any of my GPU |
Note: Arkworks and icicle will generate different projective points, but when compared in affine, arkworks and icicle will be the same. Also the icicle result should still contribute to a correct proof. Also, when converting, do not use bigint conversion for scalars. transmute the scalars into the icicle type instead, it works better and is faster. Edit: the checks fail because the check environment does not have cuda, this should be alleviated or worked around by using mock structs |
Can we do something about the hyrax slowdown? Perhaps batch the msms to reduce call overhead? What is the difference between the raw calculation of the msm for hyrax on the GPU vs CPU (excl data transmission)? |
This PR adds support for Ingonyama's icicle library.
To integrate Icicle with minimal changes the following was done:
Icicle
that maps the corresponding icicle type to its corresponding arkworks CurveGroup was, this lead to an additional trait bound propagated throughout the codebase.2-5 of the e2e tests which fail intermittently. I thought it was weird that the test commit_prove_verify for hyrax passed but the e2e tests failed. I investigated on on my own by comparing the results of the different msm variations done to speed up small msm operations and saw they produce different results. As it stands the results of icicle msm match the results of msm_bigint which is probably why some (hyrax prove, verify) but not all of the tests are passing.
I assume they different msm results between variations is expected behavior. Let me know if you have suggestions on how to address the failing tests.