update halo2curves to 0.6.0

[kilic]

Apperently this update won't require zkcrypto changes.

• Bounds between pairing::Engine and {CurveAffine, CurveExt} are satisfied
• It wasn't compiling on my end because of ilog2 so I bumped the toolchain to 1.67.0 from 1.66.0. But then lots of new clippy warnings are appeared. So should I rollback to 1.66 or fix clippy also here in this PR?

Question: should we bump halo2_proofs version to 0.2.1?

It should replace #234

[CPerezz]

LGTM modulo tests passing.

I think we should bump to 0.3.0 as we now have a different version (that breaks) of halo2curves

[kilic]

@CPerezz can you check again if compilation issues fixed correctly? Then I'll fix clippy

[CPerezz]

@CPerezz can you check again if compilation issues fixed correctly? Then I'll fix clippy

Wasm build/tests aren't working for the same errors than previous. I think the issue is that WASM disables the default features (causing multicore to not be loaded as dependency).
I should look at things prior to speaking.. The error is derived from the fact that rayon does not support WASM-targets straight away. And indeed the setup is quite complex.

So looks like it will be easier to simply disable multicore when testing for WASM targets.

I can take a look! And also I think privacy-scaling-explorations/halo2curves#117 can help towards this.

[CPerezz]

@kilic

This commit: ab4e2fe (feel free to copy the Manifest config into this PR)

And the changes I introduce in privacy-scaling-explorations/halo2curves#121 should completely fix this for the future.

EDIT:
Alternatively, you can import the library in the following way as suggested by Francois:

[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
halo2curves = { version = "0.5.0", features = ["derive_serde", "multicore"] }

[target.'cfg(target_arch = "wasm32")'.dependencies]
# bypass the default "multicore" feature
halo2curves = { version = "0.5.0", default-features = false, features = ["derive_serde", "multicore"] }

[han0110]

LGTM!

[CPerezz]

Hey @kilic can you try to see how https://github.com/privacy-scaling-explorations/halo2curves/tree/main now that we merged privacy-scaling-explorations/halo2curves#122??

If works well, I'd rather release a new halo2curves version and use it here.

[kilic]

If works well, I'd rather release a new halo2curves version and use it here.

It worked and is waiting for new halo2curves version @CPerezz

[CPerezz]

@kilic There it is: https://crates.io/crates/halo2curves/0.6.0

[CPerezz]

Why are removing the bits feature? We should bring it back even if it's not as default.

We should import halo2curves with default-features=false and then, add bits feature which triggers halo2curves/bits. And then we can decide whether we keep bits as default or not.

WDYT?

[CPerezz]

I think we could maybe unify now assert_satisfied_par and assert_satisfied since rayon can internally handle multicore by itself?

On this way, we should only have the parallel version which gets renamed to not have par on it.

Should we go for this now also?

[kilic]

I think we could maybe unify now assert_satisfied_par and assert_satisfied since rayon can internally handle multicore by itself?

Also done!

[CPerezz]

I think we can merge now @kilic

[CPerezz]

Sadly, just after seeing the merge, we got a message from @huitseker which says:

have tests hashing public parameters in Supernova on Arecibo, and I noticed the hash of our reference circuits on Bn254/Grumpkin changed when (only) switching from halo2curves 0.5.0 to 0.6.0. This could come from the field ops outputs changing, or the msm output changing, or the serialization format changing. I'll look into this, but: Is that expected? is there a PR I should look at to narrow down what happened?

So I'll not release until we figure out what happens.