feat: add from field method #87
Conversation
| @@ -99,6 +101,15 @@ pub trait BigNumTrait { | |||
| pub fn conditional_select(lhs: Self, rhs: Self, predicate: bool) -> Self; | |||
| } | |||
|
|
|||
| // impl<let N: u32, let MOD_BITS: u32, Params> std::convert::From<Field> for BigNum<N, MOD_BITS, Params> | |||
There was a problem hiding this comment.
ideally I would like to use this method, but there's an issue with a similar impl in U60Repr
src/fns/constrained_ops.nr
Outdated
| ) -> [Field; N] { | ||
| let result = __from_field::<N>(field); | ||
| // validate the limbs are in range and the value in total is less than 2^254 | ||
| validate_in_range::<N, 254>(result); |
There was a problem hiding this comment.
I wonder if there's a bug here. it might be that the result is 254 bits long but larger than the field modulus.
Should I create a field params and do a validate_in_field?
| @@ -57,6 +58,29 @@ impl<let N: u32, let NumSegments: u32> std::convert::From<[Field; N]> for U60Rep | |||
| } | |||
| } | |||
|
|
|||
| // impl<let N: u32, let NumSegments: u32> std::convert::From<Field> for U60Repr<N, NumSegments> { | |||
There was a problem hiding this comment.
this would be my preferred way of doing the casting for U60Repr but the issue is as soon as I have the conversion like this, I can not use U60repr::from() anymore. because it does not know the type generic. @TomAFrench any suggestions on how to fix this?
There was a problem hiding this comment.
@TomAFrench, I'm just pinging you to see if you have an idea if we can fix this.
| let mut result: Self = U60Repr { limbs: [0; N * NumSegments] }; | ||
| let N_u60: u32 = N * NumSegments; | ||
| assert(N_u60 >= 1, "N must be at least 1"); | ||
| if N_u60 == 1 { |
There was a problem hiding this comment.
very ugly, very ugly. but I'm not sure if there's a workaround.
src/fns/constrained_ops.nr
Outdated
| params: P<N, MOD_BITS>, | ||
| field: Field, | ||
| ) -> [Field; N] { | ||
| let result = __from_field::<N>(field); |
There was a problem hiding this comment.
Should this be in an unsafe block with a safety comment describing why it’s safe?
There was a problem hiding this comment.
yep, I'll add comments explaining the safety.
| @@ -790,3 +790,10 @@ fn test_expressions() { | |||
| assert(wx_constrained.limbs == wx.limbs); | |||
| } | |||
|
|
|||
| #[test] | |||
There was a problem hiding this comment.
I guess missing a test case where Field is large enough so it will produce 2 limbs
There was a problem hiding this comment.
added tests for 2 and 3 digits.
| grumpkin_modulus[0] = 0x33e84879b9709143e1f593f0000001; | ||
| grumpkin_modulus[1] = 0x4e72e131a029b85045b68181585d28; | ||
| grumpkin_modulus[2] = 0x3064; | ||
| validate_gt::<N, 254>(grumpkin_modulus, result); |
There was a problem hiding this comment.
we have to check the 254-bit bignum returned by __from_field is actually a grumpkin field element.
to do this I'm using the validate_gt function and comparing the result to the grumpkin_modulus.
At first, I wanted to use the validate_in_field function, but the issue is, it only works when the generics match the generics used to create the field (in this case grumpkin_Fq which will have 3 limbs. So instead I just did the validate_gt check.
Description
added a method to create bignums from field elements
Problem*
Resolves
Summary*
Additional Context
PR Checklist*
cargo fmton default settings.