From 6353c9a4bff7d0e94ba7c60094c89757844107ca Mon Sep 17 00:00:00 2001 From: Pablo Dallegri Date: Wed, 11 Dec 2024 23:57:37 +0000 Subject: [PATCH 1/5] test: border cases of umod_div --- src/tests/bignum_test.nr | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/tests/bignum_test.nr b/src/tests/bignum_test.nr index ffc8f524..c16c136d 100644 --- a/src/tests/bignum_test.nr +++ b/src/tests/bignum_test.nr @@ -632,6 +632,21 @@ fn test_udiv_mod_U256() { assert(product == a); } +#[test] +fn test_1_udiv_mod_2() { + let _0: U256 = BigNum::new(); + let _1: U256 = BigNum::one(); + assert(_1.udiv_mod(_1 + _1) == (_0, _1)); +} + +#[test] +fn test_20_udiv_mod_11() { + let _1: U256 = BigNum::one(); + let _2_POW_120: U256 = BigNum::from_slice([0, 1, 0]); + let _2_POW_121: U256 = BigNum::from_slice([0, 2, 0]); + assert(_2_POW_121.udiv_mod(_2_POW_120 + _1) == (_1, _2_POW_120 - _1)); +} + // // N.B. witness generation times make these tests take ~15 minutes each! Uncomment at your peril // #[test] // fn test_div_2048() { From ed8bc515318cfab27d056539f36aa2ea25defc22 Mon Sep 17 00:00:00 2001 From: Pablo Dallegri Date: Wed, 11 Dec 2024 23:58:57 +0000 Subject: [PATCH 2/5] fix: underflow when numerator is shorter than divisor --- src/fns/unconstrained_ops.nr | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/fns/unconstrained_ops.nr b/src/fns/unconstrained_ops.nr index 68e12df0..5c9209f3 100644 --- a/src/fns/unconstrained_ops.nr +++ b/src/fns/unconstrained_ops.nr @@ -171,6 +171,9 @@ pub(crate) unconstrained fn __udiv_mod( let mut divisor_u60: U60Repr = U60Repr::from(divisor); let b = divisor_u60; + if !remainder_u60.gte(divisor_u60) { + ([0; N], numerator) + } else { let mut bit_difference = remainder_u60.get_msb() - divisor_u60.get_msb(); let mut accumulator_u60: U60Repr = U60Repr::one(); @@ -199,6 +202,7 @@ pub(crate) unconstrained fn __udiv_mod( } (U60Repr::into(quotient_u60), U60Repr::into(remainder_u60)) + } } pub(crate) unconstrained fn __invmod( From 0d1234373429a2e6df596cc12fa75a15af980e56 Mon Sep 17 00:00:00 2001 From: Pablo Dallegri Date: Thu, 12 Dec 2024 00:00:37 +0000 Subject: [PATCH 3/5] chore: indent --- src/fns/unconstrained_ops.nr | 44 ++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/fns/unconstrained_ops.nr b/src/fns/unconstrained_ops.nr index 5c9209f3..b8683465 100644 --- a/src/fns/unconstrained_ops.nr +++ b/src/fns/unconstrained_ops.nr @@ -174,34 +174,34 @@ pub(crate) unconstrained fn __udiv_mod( if !remainder_u60.gte(divisor_u60) { ([0; N], numerator) } else { - let mut bit_difference = remainder_u60.get_msb() - divisor_u60.get_msb(); + let mut bit_difference = remainder_u60.get_msb() - divisor_u60.get_msb(); - let mut accumulator_u60: U60Repr = U60Repr::one(); - divisor_u60 = divisor_u60.shl(bit_difference); - accumulator_u60 = accumulator_u60.shl(bit_difference); + let mut accumulator_u60: U60Repr = U60Repr::one(); + divisor_u60 = divisor_u60.shl(bit_difference); + accumulator_u60 = accumulator_u60.shl(bit_difference); - if (divisor_u60.gte(remainder_u60 + U60Repr::one())) { - divisor_u60.shr1(); - accumulator_u60.shr1(); - } - for _ in 0..(N * 120) { - if (remainder_u60.gte(b) == false) { - break; + if (divisor_u60.gte(remainder_u60 + U60Repr::one())) { + divisor_u60.shr1(); + accumulator_u60.shr1(); } + for _ in 0..(N * 120) { + if (remainder_u60.gte(b) == false) { + break; + } - // we've shunted 'divisor' up to have the same bit length as our remainder. - // If remainder >= divisor, then a is at least '1 << bit_difference' multiples of b - if (remainder_u60.gte(divisor_u60)) { - remainder_u60 -= divisor_u60; - // we can use OR here instead of +, as - // accumulator is always a nice power of two - quotient_u60 = quotient_u60 + accumulator_u60; + // we've shunted 'divisor' up to have the same bit length as our remainder. + // If remainder >= divisor, then a is at least '1 << bit_difference' multiples of b + if (remainder_u60.gte(divisor_u60)) { + remainder_u60 -= divisor_u60; + // we can use OR here instead of +, as + // accumulator is always a nice power of two + quotient_u60 = quotient_u60 + accumulator_u60; + } + divisor_u60.shr1(); // >>= 1; + accumulator_u60.shr1(); // >>= 1; } - divisor_u60.shr1(); // >>= 1; - accumulator_u60.shr1(); // >>= 1; - } - (U60Repr::into(quotient_u60), U60Repr::into(remainder_u60)) + (U60Repr::into(quotient_u60), U60Repr::into(remainder_u60)) } } From 2fd30f9c842ee5b3c26d2b5da136e7d8fef20166 Mon Sep 17 00:00:00 2001 From: Pablo Dallegri Date: Thu, 12 Dec 2024 00:02:11 +0000 Subject: [PATCH 4/5] fix: borrow flags miscalculation on lower limb limits --- src/fns/unconstrained_helpers.nr | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/fns/unconstrained_helpers.nr b/src/fns/unconstrained_helpers.nr index 0afcfb40..28eee2f3 100644 --- a/src/fns/unconstrained_helpers.nr +++ b/src/fns/unconstrained_helpers.nr @@ -39,12 +39,11 @@ pub(crate) unconstrained fn __validate_gt_remainder( let mut b_u60: U60Repr = U60Repr::from(rhs); let underflow = b_u60.gte(a_u60); - b_u60 += U60Repr::one(); assert(underflow == false, "BigNum::validate_gt check fails"); let mut result_u60: U60Repr = U60Repr { limbs: [0; 2 * N] }; let mut carry_in: u64 = 0; - let mut borrow_in: u64 = 0; + let mut borrow_in: u64 = 1; let mut borrow_flags: [bool; N] = [false; N]; let mut carry_flags: [bool; N] = [false; N]; for i in 0..2 * N { From ecb9dec0a2ca6cfc23fbd3e2001c5590693c8958 Mon Sep 17 00:00:00 2001 From: Pablo Dallegri Date: Tue, 17 Dec 2024 05:25:15 +0000 Subject: [PATCH 5/5] chore: modify if condition to comparing msbs directly --- src/fns/unconstrained_ops.nr | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/fns/unconstrained_ops.nr b/src/fns/unconstrained_ops.nr index b8683465..86711fe7 100644 --- a/src/fns/unconstrained_ops.nr +++ b/src/fns/unconstrained_ops.nr @@ -171,10 +171,12 @@ pub(crate) unconstrained fn __udiv_mod( let mut divisor_u60: U60Repr = U60Repr::from(divisor); let b = divisor_u60; - if !remainder_u60.gte(divisor_u60) { + let numerator_msb = remainder_u60.get_msb(); + let divisor_msb = divisor_u60.get_msb(); + if divisor_msb > numerator_msb { ([0; N], numerator) } else { - let mut bit_difference = remainder_u60.get_msb() - divisor_u60.get_msb(); + let mut bit_difference = numerator_msb - divisor_msb; let mut accumulator_u60: U60Repr = U60Repr::one(); divisor_u60 = divisor_u60.shl(bit_difference);