chore: add ScalarField WNAF validation for N >= 64 #80
Conversation
jialinli98
changed the title
…o jl/unconstrained_nibbles
src/scalar_field.nr
Outdated
| if N >= 64 { | ||
| unsafe { compare_scalar_field_to_bignum(result) }; | ||
| } |
There was a problem hiding this comment.
This is unconstrained so it's not enforcing anything.
src/scalar_field.nr
Outdated
| fn from(input: Field) -> Self { | ||
| let result = unsafe { get_wnaf_slices(input) }; | ||
|
|
||
| if std::runtime::is_unconstrained() { |
| let scalar_field2 = scalar_field.into(); | ||
| assert(val as Field == scalar_field2); | ||
| } | ||
| #[test] |
There was a problem hiding this comment.
Let's add some tests for compare_scalar_field_to_bignum
Co-authored-by: Tom French <[email protected]>
…ng/noir_bigcurve into jl/unconstrained_nibbles
jialinli98
force-pushed
the
jl/unconstrained_nibbles
branch
from
b3cf8dc to
8fcec0c
Compare
src/scalar_field.nr
Outdated
| { | ||
| x.validate_in_field(); | ||
| let mut (slices, skew): ([u8; N], bool) = unsafe { get_wnaf_slices2(x) }; | ||
| //let mut (slices, skew): ([u8; N], bool) = unsafe { get_wnaf_slices2(x) }; |
There was a problem hiding this comment.
please don't leave commented code.
src/scalar_field.nr
Outdated
| } | ||
| #[test] | ||
| fn test_get_modulus_slices() { | ||
| let modulus_slices: [u8; 64] = unsafe { get_modulus_slices::<64>() }; |
There was a problem hiding this comment.
These lines are throwing "Unnecessary unsafe block" warnings. Ideally this library should not be throwing warnings but we should avoid adding more.
Does your editor show these with yellow squiggly lines?
src/scalar_field.nr
Outdated
| should_continue = false; | ||
| } else if result.base4_slices[i] > expected_slices[i] { | ||
| // Found a strictly larger number, this is invalid | ||
| assert(false); |
There was a problem hiding this comment.
This can be panic with a useful error message stating that the scalar field is larger than the field modulus, etc.
src/scalar_field.nr
Outdated
| ], | ||
| ); | ||
| } | ||
| #[test(should_fail)] |
There was a problem hiding this comment.
We can then make these tests more effective by ensuring that we're failing the expected error message.
src/scalar_field.nr
Outdated
| 15, 10, 12, 9, 15, 8, 0, 0, 0, 0, 0, 0, | ||
| ]; | ||
| let mut result: ScalarField<65> = ScalarField { base4_slices: [0; 65], skew: true }; | ||
| for i in 0..64 { |
src/scalar_field.nr
Outdated
| let modulus_slices: [u8; 64] = [ | ||
| 9, 8, 3, 2, 2, 7, 3, 9, 7, 0, 9, 8, 14, 0, 1, 4, 13, 12, 2, 8, 2, 2, 13, 11, 4, 0, 12, | ||
| 0, 10, 12, 2, 14, 9, 4, 1, 9, 15, 4, 2, 4, 3, 12, 13, 12, 11, 8, 4, 8, 10, 1, 15, 0, 15, | ||
| 10, 12, 9, 15, 8, 0, 0, 0, 0, 0, 0, | ||
| ]; | ||
| let mut result: ScalarField<64> = ScalarField { base4_slices: [0; 64], skew: true }; | ||
| for i in 0..64 { | ||
| result.base4_slices[i] = modulus_slices[i]; | ||
| } |
There was a problem hiding this comment.
Why not write ScalarField {base4_slices: modulus_slices, skew: true }?
…o jl/unconstrained_nibbles
…ng/noir_bigcurve into jl/unconstrained_nibbles
Description
Add WNAF validation for ScalarField when N ≥ 64 to ensure the reconstructed value fits within a Field element.
Problem*
Resolves #76
Summary*
Additional Context
PR Checklist*
cargo fmton default settings.BEGIN_COMMIT_OVERRIDE
fix: add
ScalarFieldWNAF validation forN >= 64(#80)END_COMMIT_OVERRIDE