Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented add, mult, inv, sub, div, pow and neg for binary fields and wrote tests for each function #864

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
153 changes: 153 additions & 0 deletions ff/src/fields/bf.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
use num_bigint::BigUint;
use num_traits::{Zero, One};

#[derive( Clone, PartialEq, Eq)]
pub struct BinaryFieldElement {
value: BigUint,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Here I'm not sure this way of doing things is the more efficient approach because no matter the size of the field concerned, you store in your value as BigUint a vector of BigDigit (u64 or 32 I think) but for field extensions before 32, we don't need as much space.

So I wonder if it is not more optimized to do something custom made like this:

https://gitlab.com/IrreducibleOSS/binius/-/blob/main/crates/field/src/binary_field.rs?ref_type=heads#L770-L777

so that we can have a very efficient implementation for small extensions.

Copy link
Author

Choose a reason for hiding this comment

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

This makes a lot of sense, but it would mean implementing a separate Binary Field implementation for each field size. Should I submit these changes as part of my next PR, or should I first push the other changes we discussed (operators and moving binmul) and then implement the different field sizes in subsequent PRs?


impl BinaryFieldElement {
// Constructor to create a new BinaryFieldElement
martinsander00 marked this conversation as resolved.
Show resolved Hide resolved
pub fn new(value: BigUint) -> Self {
BinaryFieldElement { value }
martinsander00 marked this conversation as resolved.
Show resolved Hide resolved
}

// Method for addition in GF(2^k)
pub fn add(&self, other: &Self) -> Self {
Copy link
Contributor

Choose a reason for hiding this comment

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

Here why not implementing directly the Add trait like this is done here for example? I think this is also applicable to sub, mul, neg... so that we can use directly +, -, * operations

algebra/ec/src/pairing.rs

Lines 192 to 200 in 9ce33e6

impl<'a, P: Pairing> Add<&'a Self> for PairingOutput<P> {
type Output = Self;
#[inline]
fn add(mut self, other: &'a Self) -> Self {
self += other;
self
}
}

Copy link
Author

Choose a reason for hiding this comment

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

Will do this! Thanks for the suggestion

// Addition in binary fields is done with XOR
BinaryFieldElement::new(&self.value ^ &other.value)
martinsander00 marked this conversation as resolved.
Show resolved Hide resolved
}

pub fn sub(&self, other: &Self) -> Self {
self.add(other)
}

// Method for multiplication in GF(2^k)
pub fn mul(&self, other: &Self) -> Self {
BinaryFieldElement::new(binmul(self.value.clone(), other.value.clone(), None))
martinsander00 marked this conversation as resolved.
Show resolved Hide resolved
}

// Method for division (multiplication by the inverse)
pub fn div(&self, other: &Self) -> Self {
self.mul(&other.inv()) // Division as multiplication by inverse
}

pub fn neg(&self) -> Self {
self.clone()
}

pub fn pow(&self, exponent: u64) -> Self {
if exponent == 0 {
BinaryFieldElement::new(BigUint::one())
} else if exponent == 1 {
self.clone()
} else if exponent % 2 == 0 {
let half_pow = self.pow(exponent / 2);
half_pow.mul(&half_pow)
} else {
self.mul(&self.pow(exponent - 1))
}
}

// In a binary field GF(2^w) the inverse is a^{-1} = a^{2m-2}
pub fn inv(&self) -> Self {
let bit_len = self.value.bits();
let l = 1 << (bit_len - 1).ilog2() + 1;
return self.pow(2_u64.pow(l) - 2);
}
}

// Binary field multiplication with reduction
fn binmul(v1: BigUint, v2: BigUint, length: Option<usize>) -> BigUint {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can integrate this method directly inside the BinaryFieldElement impl no? using:

fn binmul(&self, rhs: Self, length: Option<usize>) -> Self {

Copy link
Author

Choose a reason for hiding this comment

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

SO we could, but why would someone use binmul instead of mul? I think mul is more intuitive right?

if v1.is_zero() || v2.is_zero() {
return BigUint::zero();
}
if v1.is_one() || v2.is_one() {
return v1 * v2;
}

let length = length.unwrap_or_else(|| 1 << (v1.bits().max(v2.bits()) - 1).next_power_of_two());
let halflen = length / 2;
let quarterlen = halflen / 2;
let halfmask = (BigUint::one() << halflen) - BigUint::one();

let l1 = &v1 & &halfmask;
let r1 = &v1 >> halflen;
let l2 = &v2 & &halfmask;
let r2 = &v2 >> halflen;

// Optimized case for (L1, R1) == (0, 1)
if l1.is_zero() && r1 == BigUint::one() {
let out_r = binmul(BigUint::one() << quarterlen, r2.clone(), Some(halflen)) ^ l2.clone();
return r2 ^ (out_r << halflen);
}

// Perform Karatsuba multiplication with three main sub-multiplications
let l1l2 = binmul(l1.clone(), l2.clone(), Some(halflen));
let r1r2 = binmul(r1.clone(), r2.clone(), Some(halflen));
let r1r2_high = binmul(BigUint::one() << quarterlen, r1r2.clone(), Some(halflen));
let z3 = binmul(l1 ^ r1.clone(), l2 ^ r2.clone(), Some(halflen));

l1l2.clone() ^ r1r2.clone() ^ ((z3 ^ l1l2 ^ r1r2 ^ r1r2_high) << halflen)
}



#[cfg(test)]
mod tests {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add some randomized tests here as this is done here for example? https://github.com/arkworks-rs/algebra/blob/master/ff/src/biginteger/tests.rs

Copy link
Author

Choose a reason for hiding this comment

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

done! Thanks for the suggestion

use super::*;
use num_bigint::ToBigUint;

#[test]
fn add_test() {
let a = BinaryFieldElement::new(1u64.to_biguint().unwrap());
let b = BinaryFieldElement::new(0u64.to_biguint().unwrap());
let c = BinaryFieldElement::new(100u64.to_biguint().unwrap());

// Test cases
assert_eq!(a.add(&b), BinaryFieldElement::new(1u64.to_biguint().unwrap()));
assert_eq!(a.add(&a), BinaryFieldElement::new(0u64.to_biguint().unwrap())); // 1 + 1 in GF(2) should be 0
assert_eq!(a.add(&c), BinaryFieldElement::new(101u64.to_biguint().unwrap())); // 1 + 100 = 101 in binary (XOR)
assert_eq!(b.add(&c), BinaryFieldElement::new(100u64.to_biguint().unwrap())); // 0 + 100 = 100
}

#[test]
fn sub_test() {
let a = BinaryFieldElement::new(1u64.to_biguint().unwrap());
let b = BinaryFieldElement::new(100u64.to_biguint().unwrap());

assert_eq!(a.sub(&b), BinaryFieldElement::new(101u64.to_biguint().unwrap())); // 1 - 100 in GF(2) should be 101 (same as add)
}

#[test]
fn neg_test() {
let a = BinaryFieldElement::new(15u64.to_biguint().unwrap());
assert_eq!(a.neg(), a); // Negation in GF(2) has no effect, so a == -a
}

#[test]
fn multiplication_test() {
let a = BinaryFieldElement::new(7u64.to_biguint().unwrap());
let b = BinaryFieldElement::new(13u64.to_biguint().unwrap());
assert_eq!(a.mul(&b), BinaryFieldElement::new(8u64.to_biguint().unwrap())); // 7 * 13 = 8
}

#[test]
fn exponentiation_test() {
let a = BinaryFieldElement::new(7u64.to_biguint().unwrap());
assert_eq!(a.pow(3), BinaryFieldElement::new(4u64.to_biguint().unwrap())); // 7 ** 3 = 4
}

#[test]
fn inverse_test() {
let a = BinaryFieldElement::new(7u64.to_biguint().unwrap());
assert_eq!(a.inv(), BinaryFieldElement::new(15u64.to_biguint().unwrap())); // Inverse of 7 = 15
}

#[test]
fn division_test() {
let a = BinaryFieldElement::new(7u64.to_biguint().unwrap());
let b = BinaryFieldElement::new(13u64.to_biguint().unwrap());
assert_eq!(a.div(&b), BinaryFieldElement::new(10u64.to_biguint().unwrap())); // 7 / 13 = 10
}
}
Loading