chore: Move to the Ingonyama MSM #374
Conversation
winston-h-zhang
force-pushed
the
ingonyama-msm
branch
from
6bf655b
to
086832c
Compare
|
There is a little kink with the Ingonyama licenses -- should we make an upstream PR? |
We should just open an issue to ingonyama, and fix it on our side with a |
winston-h-zhang
force-pushed
the
ingonyama-msm
branch
from
086832c
to
a46622f
Compare
src/provider/bn256_grumpkin.rs
Outdated
| @@ -8,7 +8,7 @@ use digest::{ExtendableOutput, Update}; | |||
| use ff::{FromUniformBytes, PrimeField}; | |||
| use group::{cofactor::CofactorCurveAffine, Curve, Group as AnotherGroup}; | |||
| #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))] | |||
| use grumpkin_msm::{bn256 as bn256_msm, grumpkin as grumpkin_msm}; | |||
| use grumpkin_msm::grumpkin as grumpkin_msm; | |||
There was a problem hiding this comment.
Why is it that the Grumpkin msm should still be on sppark? Doesn't ingonyama support grumpkin as well?
There was a problem hiding this comment.
This is merged and we should use it:
ingonyama-zk/icicle#379
| let stream = ingonyama_grumpkin_msm::Config::new(); | ||
| let cfg = ingonyama_grumpkin_msm::default_config(&stream.stream); |
There was a problem hiding this comment.
Could we make this a static in this configuration ?
| let stream = ingonyama_grumpkin_msm::Config::new(); | ||
| let cfg = ingonyama_grumpkin_msm::default_config(&stream.stream); |
There was a problem hiding this comment.
Could we make this a static to not create it on every call?
There was a problem hiding this comment.
I tried this, but ran into issues like
error[E0277]: `*mut icicle_cuda_runtime::bindings::CUstream_st` cannot be sent between threads safely
--> src/provider/bn256_grumpkin.rs:39:16
|
39 | static STREAM: Lazy<ingonyama_grumpkin_msm::Config> = Lazy::new(|| ingonyama_grumpkin_msm::Config::new());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `*mut icicle_cuda_runtime::bindings::CUstream_st` cannot be sent between threads safely
|
= help: within `ingonyama_grumpkin_msm::Config`, the trait `Send` is not implemented for `*mut icicle_cuda_runtime::bindings::CUstream_st`
Dmytro mentioned that the first stream creation will have some delay, but each time after that, the Icicle/CUDA runtime will be able to reuse them with little overhead.
Yeah, unfortunately the first CUDA call in any application always incurs some latency, it's one-time and fairly constant, 70 ms. in your case. If you try to do the same operations (create stream, alloc memory) again, the time will no longer be noticeable.
Cargo.toml
Outdated
| @@ -51,6 +51,7 @@ rayon-scan = "0.1.0" | |||
| # grumpkin-msm has been patched to support MSMs for the pasta curve cycle | |||
| # see: https://github.com/lurk-lab/grumpkin-msm/pull/3 | |||
| grumpkin-msm = { git = "https://github.com/lurk-lab/grumpkin-msm", branch = "dev" } | |||
| ingonyama-grumpkin-msm = { git = "https://github.com/lurk-lab/ingonyama-grumpkin-msm", branch = "add-grumpkin-curve", optional = true } | |||
There was a problem hiding this comment.
Could we PR this small commit?
winston-h-zhang
force-pushed
the
ingonyama-msm
branch
from
4cb457f
to
da8a13c
Compare
This PR switches the MSM on the bn256 curve to the Ingonyama MSM. We don't implement it directly, but rather use a wrapper repo https://github.com/lurk-lab/ingonyama-grumpkin-msm, which is similar to
grumpkin-msm.