From da35ef7c03e9f6baf9c17c08f664829e56f4dffa Mon Sep 17 00:00:00 2001 From: Alexander Wu Date: Sun, 30 Jan 2022 22:52:02 -0800 Subject: [PATCH 1/6] Rename BigInteger add_nocarry to add_with_carry and sub_noborrow to sub_with_borrow --- ff/src/biginteger/mod.rs | 28 +++++++++++---------- ff/src/biginteger/tests.rs | 10 ++++---- ff/src/fields/models/quadratic_extension.rs | 2 +- 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/ff/src/biginteger/mod.rs b/ff/src/biginteger/mod.rs index 369e8aa15..b4fc02041 100644 --- a/ff/src/biginteger/mod.rs +++ b/ff/src/biginteger/mod.rs @@ -235,7 +235,7 @@ impl BigInteger for BigInt { #[inline] #[ark_ff_asm::unroll_for_loops] - fn add_nocarry(&mut self, other: &Self) -> bool { + fn add_with_carry(&mut self, other: &Self) -> bool { let mut carry = 0; for i in 0..N { @@ -257,7 +257,7 @@ impl BigInteger for BigInt { #[inline] #[ark_ff_asm::unroll_for_loops] - fn sub_noborrow(&mut self, other: &Self) -> bool { + fn sub_with_borrow(&mut self, other: &Self) -> bool { let mut borrow = 0; for i in 0..N { @@ -701,7 +701,7 @@ pub trait BigInteger: /// Number of 64-bit limbs representing `Self`. const NUM_LIMBS: usize; - /// Add another representation to this one, returning the carry bit. + /// Mutably add another bigint representation to this one, returning the carry bit. /// # Example /// /// ``` @@ -709,18 +709,19 @@ pub trait BigInteger: /// /// // Basic /// let (mut one, mut two_add) = (B::from(1u64), B::from(2u64)); - /// two_add.add_nocarry(&one); + /// two_add.add_with_carry(&one); /// assert_eq!(two_add, B::from(3u64)); /// /// // Edge-Case /// let mut max_one = B::from(u64::MAX); - /// let carry = max_one.add_nocarry(&one); + /// let carry = max_one.add_with_carry(&one); /// assert_eq!(max_one, B::from(0u64)); /// assert_eq!(carry, true) /// ``` - fn add_nocarry(&mut self, other: &Self) -> bool; + fn add_with_carry(&mut self, other: &Self) -> bool; - /// Subtract another representation from this one, returning the borrow bit. + /// Mutably subtract another bigint representation from this one, returning + /// the borrow bit. /// # Example /// /// ``` @@ -728,15 +729,15 @@ pub trait BigInteger: /// /// // Basic /// let (mut one, mut two, mut three_sub) = (B::from(1u64), B::from(2u64), B::from(3u64)); - /// three_sub.sub_noborrow(&two); + /// three_sub.sub_with_borrow(&two); /// assert_eq!(three_sub, one); /// /// // Edge-Case - /// let borrow = one.sub_noborrow(&two); + /// let borrow = one.sub_with_borrow(&two); /// assert_eq!(borrow, true); /// assert_eq!(one, B::from(u64::MAX)); /// ``` - fn sub_noborrow(&mut self, other: &Self) -> bool; + fn sub_with_borrow(&mut self, other: &Self) -> bool; /// Performs a leftwise bitshift of this number, effectively multiplying /// it by 2. Overflow is ignored. @@ -763,7 +764,8 @@ pub trait BigInteger: /// ``` fn mul2(&mut self); - /// Performs a leftwise bitshift of this number by some amount. + /// Performs a leftwise bitshift of this number by n bits, effectively multiplying + /// it by 2^n. Overflow is ignored. /// # Example /// /// ``` @@ -996,9 +998,9 @@ pub trait BigInteger: if e.is_odd() { z = signed_mod_reduction(e.as_ref()[0], 1 << w); if z >= 0 { - e.sub_noborrow(&Self::from(z as u64)); + e.sub_with_borrow(&Self::from(z as u64)); } else { - e.add_nocarry(&Self::from((-z) as u64)); + e.add_with_carry(&Self::from((-z) as u64)); } } else { z = 0; diff --git a/ff/src/biginteger/tests.rs b/ff/src/biginteger/tests.rs index 55bed2ec8..0d0cdfe2a 100644 --- a/ff/src/biginteger/tests.rs +++ b/ff/src/biginteger/tests.rs @@ -14,24 +14,24 @@ fn biginteger_arithmetic_test(a: B, b: B, zero: B) { // a + 0 = a let mut a0_add = a.clone(); - a0_add.add_nocarry(&zero); + a0_add.add_with_carry(&zero); assert_eq!(a0_add, a); // a - 0 = a let mut a0_sub = a.clone(); - a0_sub.sub_noborrow(&zero); + a0_sub.sub_with_borrow(&zero); assert_eq!(a0_sub, a); // a - a = 0 let mut aa_sub = a.clone(); - aa_sub.sub_noborrow(&a); + aa_sub.sub_with_borrow(&a); assert_eq!(aa_sub, zero); // a + b = b + a let mut ab_add = a.clone(); - ab_add.add_nocarry(&b); + ab_add.add_with_carry(&b); let mut ba_add = b.clone(); - ba_add.add_nocarry(&a); + ba_add.add_with_carry(&a); assert_eq!(ab_add, ba_add); } diff --git a/ff/src/fields/models/quadratic_extension.rs b/ff/src/fields/models/quadratic_extension.rs index fbcb7f447..75436f264 100644 --- a/ff/src/fields/models/quadratic_extension.rs +++ b/ff/src/fields/models/quadratic_extension.rs @@ -432,7 +432,7 @@ where // This is cheaper than `P::BaseField::one().double().inverse()` let mut two_inv = P::BasePrimeField::MODULUS; - two_inv.add_nocarry(&1u64.into()); + two_inv.add_with_carry(&1u64.into()); two_inv.div2(); let two_inv = P::BasePrimeField::from(two_inv); From 145b37e099596432126e0c68b17bd301b20b0640 Mon Sep 17 00:00:00 2001 From: Alexander Wu Date: Sun, 30 Jan 2022 22:56:27 -0800 Subject: [PATCH 2/6] Add more assertions regarding the carry/borrow bit in bigint tests --- ff/src/biginteger/mod.rs | 24 +++++++++++++----------- ff/src/biginteger/tests.rs | 14 +++++++++----- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/ff/src/biginteger/mod.rs b/ff/src/biginteger/mod.rs index b4fc02041..66102e37f 100644 --- a/ff/src/biginteger/mod.rs +++ b/ff/src/biginteger/mod.rs @@ -708,14 +708,15 @@ pub trait BigInteger: /// use ark_ff::{biginteger::BigInteger64 as B, BigInteger as _}; /// /// // Basic - /// let (mut one, mut two_add) = (B::from(1u64), B::from(2u64)); - /// two_add.add_with_carry(&one); - /// assert_eq!(two_add, B::from(3u64)); + /// let (mut one, mut x) = (B::from(1u64), B::from(2u64)); + /// let carry = x.add_with_carry(&one); + /// assert_eq!(x, B::from(3u64)); + /// assert_eq!(carry, false); /// /// // Edge-Case - /// let mut max_one = B::from(u64::MAX); - /// let carry = max_one.add_with_carry(&one); - /// assert_eq!(max_one, B::from(0u64)); + /// let mut x = B::from(u64::MAX); + /// let carry = x.add_with_carry(&one); + /// assert_eq!(x, B::from(0u64)); /// assert_eq!(carry, true) /// ``` fn add_with_carry(&mut self, other: &Self) -> bool; @@ -728,14 +729,15 @@ pub trait BigInteger: /// use ark_ff::{biginteger::BigInteger64 as B, BigInteger as _}; /// /// // Basic - /// let (mut one, mut two, mut three_sub) = (B::from(1u64), B::from(2u64), B::from(3u64)); - /// three_sub.sub_with_borrow(&two); - /// assert_eq!(three_sub, one); + /// let (mut one_sub, two, mut three_sub) = (B::from(1u64), B::from(2u64), B::from(3u64)); + /// let borrow = three_sub.sub_with_borrow(&two); + /// assert_eq!(three_sub, one_sub); + /// assert_eq!(borrow, false); /// /// // Edge-Case - /// let borrow = one.sub_with_borrow(&two); + /// let borrow = one_sub.sub_with_borrow(&two); + /// assert_eq!(one_sub, B::from(u64::MAX)); /// assert_eq!(borrow, true); - /// assert_eq!(one, B::from(u64::MAX)); /// ``` fn sub_with_borrow(&mut self, other: &Self) -> bool; diff --git a/ff/src/biginteger/tests.rs b/ff/src/biginteger/tests.rs index 0d0cdfe2a..b3f51eca3 100644 --- a/ff/src/biginteger/tests.rs +++ b/ff/src/biginteger/tests.rs @@ -14,25 +14,29 @@ fn biginteger_arithmetic_test(a: B, b: B, zero: B) { // a + 0 = a let mut a0_add = a.clone(); - a0_add.add_with_carry(&zero); + let carry = a0_add.add_with_carry(&zero); assert_eq!(a0_add, a); + assert_eq!(carry, false); // a - 0 = a let mut a0_sub = a.clone(); - a0_sub.sub_with_borrow(&zero); + let borrow = a0_sub.sub_with_borrow(&zero); assert_eq!(a0_sub, a); + assert_eq!(borrow, false); // a - a = 0 let mut aa_sub = a.clone(); - aa_sub.sub_with_borrow(&a); + let borrow = aa_sub.sub_with_borrow(&a); assert_eq!(aa_sub, zero); + assert_eq!(borrow, false); // a + b = b + a let mut ab_add = a.clone(); - ab_add.add_with_carry(&b); + let ab_carry = ab_add.add_with_carry(&b); let mut ba_add = b.clone(); - ba_add.add_with_carry(&a); + let ba_carry = ba_add.add_with_carry(&a); assert_eq!(ab_add, ba_add); + assert_eq!(ab_carry, ba_carry); } // Test correctness of BigInteger's bit values From 1d8a7a321d911565c71b03eb8036326a6b89d129 Mon Sep 17 00:00:00 2001 From: Alexander Wu Date: Sun, 30 Jan 2022 22:58:04 -0800 Subject: [PATCH 3/6] Add tests for big integer multiplication --- ff/src/biginteger/tests.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/ff/src/biginteger/tests.rs b/ff/src/biginteger/tests.rs index b3f51eca3..1ea9f63ac 100644 --- a/ff/src/biginteger/tests.rs +++ b/ff/src/biginteger/tests.rs @@ -37,6 +37,18 @@ fn biginteger_arithmetic_test(a: B, b: B, zero: B) { let ba_carry = ba_add.add_with_carry(&a); assert_eq!(ab_add, ba_add); assert_eq!(ab_carry, ba_carry); + + // a * 1 = a + let mut a_mul1 = a.clone(); + a_mul1.muln(0); + assert_eq!(a_mul1, a); + + // a * 2 = a + a + let mut a_mul2 = a.clone(); + a_mul2.mul2(); + let mut a_plus_a = a.clone(); + a_plus_a.add_with_carry(&a); // Won't assert anything about carry bit. + assert_eq!(a_mul2, a_plus_a); } // Test correctness of BigInteger's bit values From 0058e27731037bf31353daa2d74dbdbb19480f55 Mon Sep 17 00:00:00 2001 From: Alexander Wu Date: Sun, 30 Jan 2022 23:08:05 -0800 Subject: [PATCH 4/6] Update changelog breaking change --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86d2ac9fe..f62b5dcc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,8 @@ - Rename `Fp*Parameters` to `Fp*Config`. - Add `From`, `From`, and `From` `impl`s for `BigInt`. - Remove `FftConfig`; move its contents to `FftField`. +- [\#383](https://github.com/arkworks-rs/algebra/pull/383) (arc-ff) Rename `BigInteger::add_nocarry` to `add_with_carry` and `sub_noborrow` to `sub_with_borrow`. + ### Features From 568f483e58a3ece480085364b398e140c72c773a Mon Sep 17 00:00:00 2001 From: Alexander Wu Date: Mon, 31 Jan 2022 21:01:00 -0800 Subject: [PATCH 5/6] Reconcile with new changes on master --- ff/src/biginteger/mod.rs | 4 ++-- ff/src/fields/models/fp/mod.rs | 4 ++-- ff/src/fields/models/fp/montgomery_backend.rs | 22 +++++++++---------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/ff/src/biginteger/mod.rs b/ff/src/biginteger/mod.rs index 66102e37f..2ae690c2d 100644 --- a/ff/src/biginteger/mod.rs +++ b/ff/src/biginteger/mod.rs @@ -85,7 +85,7 @@ macro_rules! const_modulo { remainder = remainder.const_mul2(); remainder.0[0] |= $a.get_bit(i as usize) as u64; if remainder.const_geq($divisor) { - let (r, borrow) = remainder.const_sub_noborrow($divisor); + let (r, borrow) = remainder.const_sub_with_borrow($divisor); remainder = r; assert!(!borrow); } @@ -183,7 +183,7 @@ impl BigInt { #[inline] #[ark_ff_asm::unroll_for_loops] - pub(crate) const fn const_sub_noborrow(mut self, other: &Self) -> (Self, bool) { + pub(crate) const fn const_sub_with_borrow(mut self, other: &Self) -> (Self, bool) { let mut borrow = 0; const_for!((i in 0..N) { diff --git a/ff/src/fields/models/fp/mod.rs b/ff/src/fields/models/fp/mod.rs index 6d080fae3..297220fb3 100644 --- a/ff/src/fields/models/fp/mod.rs +++ b/ff/src/fields/models/fp/mod.rs @@ -198,7 +198,7 @@ impl, const N: usize> Fp { #[inline] fn subtract_modulus(&mut self) { if !self.is_less_than_modulus() { - self.0.sub_noborrow(&Self::MODULUS); + self.0.sub_with_borrow(&Self::MODULUS); } } @@ -733,7 +733,7 @@ impl, const N: usize> Neg for Fp { fn neg(self) -> Self { if !self.is_zero() { let mut tmp = P::MODULUS; - tmp.sub_noborrow(&self.0); + tmp.sub_with_borrow(&self.0); Fp::new(tmp) } else { self diff --git a/ff/src/fields/models/fp/montgomery_backend.rs b/ff/src/fields/models/fp/montgomery_backend.rs index 0795b877e..ed7a932e6 100644 --- a/ff/src/fields/models/fp/montgomery_backend.rs +++ b/ff/src/fields/models/fp/montgomery_backend.rs @@ -52,7 +52,7 @@ pub trait MontConfig: 'static + Sync + Send + Sized { /// Set a += b; fn add_assign(a: &mut Fp, N>, b: &Fp, N>) { // This cannot exceed the backing capacity. - a.0.add_nocarry(&b.0); + a.0.add_with_carry(&b.0); // However, it may need to be reduced a.subtract_modulus(); } @@ -60,9 +60,9 @@ pub trait MontConfig: 'static + Sync + Send + Sized { fn sub_assign(a: &mut Fp, N>, b: &Fp, N>) { // If `other` is larger than `self`, add the modulus to self first. if b.0 > a.0 { - a.0.add_nocarry(&Self::MODULUS); + a.0.add_with_carry(&Self::MODULUS); } - a.0.sub_noborrow(&b.0); + a.0.sub_with_borrow(&b.0); } fn double_in_place(a: &mut Fp, N>) { @@ -220,7 +220,7 @@ pub trait MontConfig: 'static + Sync + Send + Sized { if b.0.is_even() { b.0.div2(); } else { - b.0.add_nocarry(&Self::MODULUS); + b.0.add_with_carry(&Self::MODULUS); b.0.div2(); } } @@ -231,16 +231,16 @@ pub trait MontConfig: 'static + Sync + Send + Sized { if c.0.is_even() { c.0.div2(); } else { - c.0.add_nocarry(&Self::MODULUS); + c.0.add_with_carry(&Self::MODULUS); c.0.div2(); } } if v < u { - u.sub_noborrow(&v); + u.sub_with_borrow(&v); b -= &c; } else { - v.sub_noborrow(&u); + v.sub_with_borrow(&u); c -= &b; } } @@ -428,7 +428,7 @@ impl Fp, N> { const fn const_neg(self, modulus: BigInt) -> Self { if !self.const_is_zero() { - Self::new(Self::sub_noborrow(&modulus, &self.0)) + Self::new(Self::sub_with_borrow(&modulus, &self.0)) } else { self } @@ -539,13 +539,13 @@ impl Fp, N> { #[inline] const fn const_reduce(mut self, modulus: BigInt) -> Self { if !self.const_is_valid(modulus) { - self.0 = Self::sub_noborrow(&self.0, &modulus); + self.0 = Self::sub_with_borrow(&self.0, &modulus); } self } - const fn sub_noborrow(a: &BigInt, b: &BigInt) -> BigInt { - a.const_sub_noborrow(b).0 + const fn sub_with_borrow(a: &BigInt, b: &BigInt) -> BigInt { + a.const_sub_with_borrow(b).0 } } From 85a610d8f4fff36758fdde24904f2abaa12c2f81 Mon Sep 17 00:00:00 2001 From: Pratyush Mishra Date: Tue, 1 Feb 2022 01:22:35 -0800 Subject: [PATCH 6/6] Tweak Documentation --- ff/src/biginteger/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ff/src/biginteger/mod.rs b/ff/src/biginteger/mod.rs index 2ae690c2d..27c91256e 100644 --- a/ff/src/biginteger/mod.rs +++ b/ff/src/biginteger/mod.rs @@ -701,7 +701,8 @@ pub trait BigInteger: /// Number of 64-bit limbs representing `Self`. const NUM_LIMBS: usize; - /// Mutably add another bigint representation to this one, returning the carry bit. + /// Add another [`BigInteger`] to `self`. This method stores the result in `self`, + /// and returns a carry bit. /// # Example /// /// ``` @@ -721,7 +722,8 @@ pub trait BigInteger: /// ``` fn add_with_carry(&mut self, other: &Self) -> bool; - /// Mutably subtract another bigint representation from this one, returning + /// Subtract another [`BigInteger`] from this one. This method stores the result in + /// `self`, and returns a borrow. /// the borrow bit. /// # Example ///