Small Gadgets refactor #353
Conversation
huitseeker
force-pushed
the
gadgets_refactor
branch
from
301f83d to
8ef2a85
Compare
huitseeker
force-pushed
the
gadgets_refactor
branch
from
a06ba42 to
75a5949
Compare
| use crate::gadgets::utils::alloc_zero; | ||
| use crate::gadgets::alloc_zero; | ||
| use crate::provider::poseidon::PoseidonConstantsCircuit; | ||
| use crate::provider::Bn256EngineIPA; | ||
| use crate::provider::PallasEngine; | ||
| use crate::provider::Secp256k1Engine; | ||
| use crate::provider::VestaEngine; | ||
| use crate::supernova::circuit::{StepCircuit, TrivialSecondaryCircuit, TrivialTestCircuit}; | ||
| use crate::traits::snark::default_ck_hint; | ||
| use crate::{bellpepper::test_shape_cs::TestShapeCS, gadgets::utils::alloc_one}; |
There was a problem hiding this comment.
Do we want to clean up these imports with more {..}s?
| @@ -711,7 +705,7 @@ mod tests { | |||
| test_shape_cs::TestShapeCS, | |||
| }, | |||
| constants::{BN_LIMB_WIDTH, BN_N_LIMBS}, | |||
| gadgets::utils::scalar_as_base, | |||
There was a problem hiding this comment.
More import cleanup possible here too
| @@ -370,7 +367,7 @@ mod tests { | |||
| test_shape_cs::TestShapeCS, | |||
| }, | |||
| constants::{BN_LIMB_WIDTH, BN_N_LIMBS}, | |||
| gadgets::utils::scalar_as_base, | |||
| // the write_be function above assumes Field::to_repr() outputs a representation that's an instance | ||
| // of `AsRef<[u8]>` in lower endian. We test that here, as this is not what the I2OSP standard recommends | ||
| // and may change in some implementations. |
There was a problem hiding this comment.
Good check! This is also implicitly assumed in my current CycleFold implementation as well
| let input_bits = input.unwrap_or(E::Scalar::ZERO).clone().to_le_bits(); | ||
| let mut mult = E::Base::ONE; | ||
| let mut val = E::Base::ZERO; | ||
| for bit in input_bits { | ||
| if bit { | ||
| val += mult; | ||
| } | ||
| mult = mult + mult; | ||
| } |
There was a problem hiding this comment.
Could this be a one-liner?
AllocatedNum::alloc(cs.namespace(|| "allocate scalar as base"), || Ok(scalar_as_base::<E>(input.unwrap_or(E::Scalar::ZERO))))
There was a problem hiding this comment.
It is morally a one-liner, you're commenting on the removed code. Unless you're talking specifically about inlining the val variable, which I'll take as a nit?
|
Thanks @mpenciak, I'll refactor the imports wholesale, through |
This does two things:
Field::to_repr()returns something that translates to lower endian,This helps the move of gadgets away from arecibo, and will be followed by other PRs to crate::gadgets:
areciboutilities inbellpepper-gadgetslurk-lab/bellpepper-gadgets#43bellpepper-gadgets#324