Skip to content

Commit

Permalink
fix: fix curve parameters for bigints (#4900)
Browse files Browse the repository at this point in the history 
# Description

## Problem\*

Resolves #4882 

## Summary\*
There were typos in bigint templates and also in curve modulus
definitions


## Additional Context
I don't understand how there could be so many typos, I thought I was
being cautious... I triple checked all the parameters this time!


## Documentation\*

Check one:
- [X] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [X] I have tested the changes locally.
- [X] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
@guipublic
guipublic authored Apr 24, 2024
1 parent 3ca6d21 commit 5985e42
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 46 deletions.
86 changes: 42 additions & 44 deletions noir_stdlib/src/bigint.nr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,14 @@ use crate::cmp::Eq;

global bn254_fq = &[0x47, 0xFD, 0x7C, 0xD8, 0x16, 0x8C, 0x20, 0x3C, 0x8d, 0xca, 0x71, 0x68, 0x91, 0x6a, 0x81, 0x97,
0x5d, 0x58, 0x81, 0x81, 0xb6, 0x45, 0x50, 0xb8, 0x29, 0xa0, 0x31, 0xe1, 0x72, 0x4e, 0x64, 0x30];
global bn254_fr = &[0x01, 0x00, 0x00, 0x00, 0x3F, 0x59, 0x1F, 0x43, 0x09, 0x97, 0xB9, 0x79, 0x48, 0xE8, 0x33, 0x28,
0x5D, 0x58, 0x81, 0x81, 0xB6, 0x45, 0x50, 0xB8, 0x29, 0xA0, 0x31, 0xE1, 0x72, 0x4E, 0x64, 0x30];
global bn254_fr = &[1, 0, 0, 240, 147, 245, 225, 67, 145, 112, 185, 121, 72, 232, 51, 40, 93, 88, 129, 129, 182, 69, 80, 184, 41, 160, 49, 225, 114, 78, 100, 48];
global secpk1_fr = &[0x41, 0x41, 0x36, 0xD0, 0x8C, 0x5E, 0xD2, 0xBF, 0x3B, 0xA0, 0x48, 0xAF, 0xE6, 0xDC, 0xAE, 0xBA,
0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF];
global secpk1_fq = &[0x2F, 0xFC, 0xFF, 0xFF, 0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF];
global secpr1_fq = &[0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0xFF, 0xFF];
global secpr1_fr = &[0x51, 0x25, 0x63, 0xFC, 0xC2, 0xCA, 0xB9, 0xF3, 0x84, 0x9E, 0x17, 0xA7, 0xAD, 0xFA, 0xE6, 0xBC,
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00,0xFF, 0xFF, 0xFF, 0xFF];
global secpr1_fr = &[81, 37, 99, 252, 194, 202, 185, 243, 132, 158, 23, 167, 173, 250, 230, 188, 255, 255, 255, 255, 255, 255, 255, 255, 0, 0, 0, 0, 255, 255, 255, 255];
// docs:start:big_int_definition
struct BigInt {
pointer: u32,
Expand Down Expand Up @@ -149,35 +147,35 @@ impl BigField for Secpk1Fr {

impl Add for Secpk1Fr {
fn add(self: Self, other: Secpk1Fr) -> Secpk1Fr {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fr);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fr);
Secpk1Fr {
array: a.bigint_add(b).to_le_bytes()
}
}
}
impl Sub for Secpk1Fr {
fn sub(self: Self, other: Secpk1Fr) -> Secpk1Fr {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fr);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fr);
Secpk1Fr {
array: a.bigint_sub(b).to_le_bytes()
}
}
}
impl Mul for Secpk1Fr {
fn mul(self: Self, other: Secpk1Fr) -> Secpk1Fr {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fr);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fr);
Secpk1Fr {
array: a.bigint_mul(b).to_le_bytes()
}
}
}
impl Div for Secpk1Fr {
fn div(self: Self, other: Secpk1Fr) -> Secpk1Fr {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fr);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fr);
Secpk1Fr {
array: a.bigint_div(b).to_le_bytes()
}
Expand Down Expand Up @@ -218,35 +216,35 @@ impl BigField for Bn254Fr {

impl Add for Bn254Fr {
fn add(self: Self, other: Bn254Fr) -> Bn254Fr {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), bn254_fr);
let b = BigInt::from_le_bytes(other.array.as_slice(), bn254_fr);
Bn254Fr {
array: a.bigint_add(b).to_le_bytes()
}
}
}
impl Sub for Bn254Fr {
fn sub(self: Self, other: Bn254Fr) -> Bn254Fr {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), bn254_fr);
let b = BigInt::from_le_bytes(other.array.as_slice(), bn254_fr);
Bn254Fr {
array: a.bigint_sub(b).to_le_bytes()
}
}
}
impl Mul for Bn254Fr {
fn mul(self: Self, other: Bn254Fr) -> Bn254Fr {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), bn254_fr);
let b = BigInt::from_le_bytes(other.array.as_slice(), bn254_fr);
Bn254Fr {
array: a.bigint_mul(b).to_le_bytes()
}
}
}
impl Div for Bn254Fr {
fn div(self: Self, other: Bn254Fr) -> Bn254Fr {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), bn254_fr);
let b = BigInt::from_le_bytes(other.array.as_slice(), bn254_fr);
Bn254Fr {
array: a.bigint_div(b).to_le_bytes()
}
Expand Down Expand Up @@ -287,35 +285,35 @@ impl BigField for Bn254Fq {

impl Add for Bn254Fq {
fn add(self: Self, other: Bn254Fq) -> Bn254Fq {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), bn254_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), bn254_fq);
Bn254Fq {
array: a.bigint_add(b).to_le_bytes()
}
}
}
impl Sub for Bn254Fq {
fn sub(self: Self, other: Bn254Fq) -> Bn254Fq {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), bn254_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), bn254_fq);
Bn254Fq {
array: a.bigint_sub(b).to_le_bytes()
}
}
}
impl Mul for Bn254Fq {
fn mul(self: Self, other: Bn254Fq) -> Bn254Fq {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), bn254_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), bn254_fq);
Bn254Fq {
array: a.bigint_mul(b).to_le_bytes()
}
}
}
impl Div for Bn254Fq {
fn div(self: Self, other: Bn254Fq) -> Bn254Fq {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), bn254_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), bn254_fq);
Bn254Fq {
array: a.bigint_div(b).to_le_bytes()
}
Expand Down Expand Up @@ -356,35 +354,35 @@ impl BigField for Secpr1Fq {

impl Add for Secpr1Fq {
fn add(self: Self, other: Secpr1Fq) -> Secpr1Fq {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), secpr1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpr1_fq);
Secpr1Fq {
array: a.bigint_add(b).to_le_bytes()
}
}
}
impl Sub for Secpr1Fq {
fn sub(self: Self, other: Secpr1Fq) -> Secpr1Fq {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), secpr1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpr1_fq);
Secpr1Fq {
array: a.bigint_sub(b).to_le_bytes()
}
}
}
impl Mul for Secpr1Fq {
fn mul(self: Self, other: Secpr1Fq) -> Secpr1Fq {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), secpr1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpr1_fq);
Secpr1Fq {
array: a.bigint_mul(b).to_le_bytes()
}
}
}
impl Div for Secpr1Fq {
fn div(self: Self, other: Secpr1Fq) -> Secpr1Fq {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), secpr1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpr1_fq);
Secpr1Fq {
array: a.bigint_div(b).to_le_bytes()
}
Expand Down Expand Up @@ -425,35 +423,35 @@ impl BigField for Secpr1Fr {

impl Add for Secpr1Fr {
fn add(self: Self, other: Secpr1Fr) -> Secpr1Fr {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), secpr1_fr);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpr1_fr);
Secpr1Fr {
array: a.bigint_add(b).to_le_bytes()
}
}
}
impl Sub for Secpr1Fr {
fn sub(self: Self, other: Secpr1Fr) -> Secpr1Fr {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), secpr1_fr);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpr1_fr);
Secpr1Fr {
array: a.bigint_sub(b).to_le_bytes()
}
}
}
impl Mul for Secpr1Fr {
fn mul(self: Self, other: Secpr1Fr) -> Secpr1Fr {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), secpr1_fr);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpr1_fr);
Secpr1Fr {
array: a.bigint_mul(b).to_le_bytes()
}
}
}
impl Div for Secpr1Fr {
fn div(self: Self, other: Secpr1Fr) -> Secpr1Fr {
let a = BigInt::from_le_bytes(self.array.as_slice(), secpk1_fq);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpk1_fq);
let a = BigInt::from_le_bytes(self.array.as_slice(), secpr1_fr);
let b = BigInt::from_le_bytes(other.array.as_slice(), secpr1_fr);
Secpr1Fr {
array: a.bigint_div(b).to_le_bytes()
}
Expand Down
20 changes: 18 additions & 2 deletions test_programs/execution_success/bigint/src/main.nr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ fn main(mut x: [u8; 5], y: [u8; 5]) {
let a_field = dep::std::field::bytes32_to_field(a_be_bytes);
let b_field = dep::std::field::bytes32_to_field(b_be_bytes);

// Regression for #4682
// Regression for issue #4682
let c = if x[0] != 0 {
test_unconstrained1(a, b)
} else {
Expand All @@ -27,7 +27,7 @@ fn main(mut x: [u8; 5], y: [u8; 5]) {
assert(a_bytes[i] == x[i]);
assert(b_bytes[i] == y[i]);
}
//Regression for issue #4578
// Regression for issue #4578
let d = a * b;
assert(d / b == a);

Expand All @@ -40,6 +40,22 @@ fn main(mut x: [u8; 5], y: [u8; 5]) {
let d1 = bigint::Secpk1Fq::from_le_bytes_32(result);
assert(d1 == d);
big_int_example(x[0], x[1]);

// Regression for issue #4882
let num_b:[u8;32] = [
0, 0, 0, 240, 147, 245, 225, 67, 145, 112, 185, 121, 72, 232, 51, 40, 93, 88, 129, 129, 182, 69, 80, 184, 41, 160, 49, 225, 114, 78, 100, 48
];
let num2_b:[u8;7] = [126, 193, 45, 39, 188, 84, 11];
let num = bigint::Bn254Fr::from_le_bytes(num_b.as_slice());
let num2 = bigint::Bn254Fr::from_le_bytes(num2_b.as_slice());

let ret_b:[u8;32] = [
131, 62, 210, 200, 215, 160, 214, 67, 145, 112, 185, 121, 72, 232, 51, 40, 93, 88, 129, 129, 182, 69, 80, 184, 41, 160, 49, 225, 114, 78, 100, 48
];
let ret = bigint::Bn254Fr::from_le_bytes(ret_b.as_slice());
assert(ret == num.mul(num2));
let div = num.div(num2);
assert(div.mul(num2) == num);
}

fn test_unconstrained1(a: Secpk1Fq, b: Secpk1Fq) -> Secpk1Fq {
Expand Down

1 comment on commit 5985e42

@skaunov
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you pls take a look to the error I get with this. Following is the copy of it description from https://discord.com/channels/1113924620781883405/1113924622031798313/1234884890210734150.

My guess it doesn't do modulo anymore. That would explain why mul yields 64 bit size instead 32.

...

[plume] Testing test_smoke... FAIL
Failed assertion: 'Bit size for lhs 64 does not match op bit size 32'

error: Assertion failed: 'Bit size for lhs 64 does not match op bit size 32'
    ┌─ std/bigint.nr:173:19
    │
173 │            array: a.bigint_mul(b).to_le_bytes()
    │                   ---------------
    │

I located a related test, but the numbers aren't that big there. 😦 Is there some more tests of the part in question? It mostly related to the recent commit e4d33c1.
...

P.s. I get the error while just multiplying the BigInt with itself.
I'm pretty sure the number I use is irrelevant, but it must be from the following array. [ 0xBD, 0x7F, 0xD2, 0x0B, 0x6B, 0xBF, 0x28, 0xC1, 0x51, 0x8F, 0xCF, 0x53, 0xB7, 0x09, 0x00, 0x5D, 0xF9, 0x12, 0xAD, 0x01, 0x88, 0x8E, 0xA4, 0xB6, 0x97, 0x98, 0x40, 0xA0, 0xFB, 0x71, 0xF4, 0xEF ].

Please sign in to comment.