Skip to content

Commit

Permalink
feat(acir_field): Add little-endian byte serialization for FieldEleme…
Browse files Browse the repository at this point in the history
…nt (#7258)

Co-authored-by: Akosh Farkash <aakoshh@gmail.com>
  • Loading branch information
VolodymyrBg and aakoshh authored Feb 18, 2025
1 parent 40d2763 commit f37eedc
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 4 deletions.
61 changes: 57 additions & 4 deletions acvm-repo/acir_field/src/field_element.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,12 +274,15 @@ impl<F: PrimeField> AcirField for FieldElement<F> {
}

fn to_be_bytes(self) -> Vec<u8> {
// to_be_bytes! uses little endian which is why we reverse the output
// TODO: Add a little endian equivalent, so the caller can use whichever one
// TODO they desire
let mut bytes = self.to_le_bytes();
bytes.reverse();
bytes
}

/// Converts the field element to a vector of bytes in little-endian order
fn to_le_bytes(self) -> Vec<u8> {
let mut bytes = Vec::new();
self.0.serialize_uncompressed(&mut bytes).unwrap();
bytes.reverse();
bytes
}

Expand All @@ -289,6 +292,12 @@ impl<F: PrimeField> AcirField for FieldElement<F> {
FieldElement(F::from_be_bytes_mod_order(bytes))
}

/// Converts bytes in little-endian order into a FieldElement and applies a
/// reduction if needed.
fn from_le_bytes_reduce(bytes: &[u8]) -> FieldElement<F> {
FieldElement(F::from_le_bytes_mod_order(bytes))
}

/// Returns the closest number of bytes to the bits specified
/// This method truncates
fn fetch_nearest_bytes(&self, num_bits: usize) -> Vec<u8> {
Expand Down Expand Up @@ -405,6 +414,50 @@ mod tests {
assert_eq!(max_num_bits_bn254, 254);
}

proptest! {
#[test]
fn test_endianness_prop(value in any::<u64>()) {
let field = FieldElement::<ark_bn254::Fr>::from(value);
// Test serialization consistency
let le_bytes = field.to_le_bytes();
let be_bytes = field.to_be_bytes();

let mut reversed_le = le_bytes.clone();
reversed_le.reverse();
prop_assert_eq!(&be_bytes, &reversed_le, "BE bytes should be reverse of LE bytes");

// Test deserialization consistency
let from_le = FieldElement::from_le_bytes_reduce(&le_bytes);
let from_be = FieldElement::from_be_bytes_reduce(&be_bytes);
prop_assert_eq!(from_le, from_be, "Deserialization should be consistent between LE and BE");
prop_assert_eq!(from_le, field, "Deserialized value should match original");
}
}

#[test]
fn test_endianness() {
let field = FieldElement::<ark_bn254::Fr>::from(0x1234_5678_u32);
let le_bytes = field.to_le_bytes();
let be_bytes = field.to_be_bytes();

// Check that the bytes are reversed between BE and LE
let mut reversed_le = le_bytes.clone();
reversed_le.reverse();
assert_eq!(&be_bytes, &reversed_le);

// Verify we can reconstruct the same field element from either byte order
let from_le = FieldElement::from_le_bytes_reduce(&le_bytes);
let from_be = FieldElement::from_be_bytes_reduce(&be_bytes);
assert_eq!(from_le, from_be);
assert_eq!(from_le, field);

// Additional test with a larger number to ensure proper byte handling
let large_field = FieldElement::<ark_bn254::Fr>::from(0x0123_4567_89AB_CDEF_u64);
let large_le = large_field.to_le_bytes();
let reconstructed = FieldElement::from_le_bytes_reduce(&large_le);
assert_eq!(reconstructed, large_field);
}

proptest! {
// This currently panics due to the fact that we allow inputs which are greater than the field modulus,
// automatically reducing them to fit within the canonical range.
Expand Down
6 changes: 6 additions & 0 deletions acvm-repo/acir_field/src/generic_ark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ pub trait AcirField:
/// Converts bytes into a FieldElement and applies a reduction if needed.
fn from_be_bytes_reduce(bytes: &[u8]) -> Self;

/// Converts bytes in little-endian order into a FieldElement and applies a reduction if needed.
fn from_le_bytes_reduce(bytes: &[u8]) -> Self;

/// Converts the field element to a vector of bytes in little-endian order
fn to_le_bytes(self) -> Vec<u8>;

/// Returns the closest number of bytes to the bits specified
/// This method truncates
fn fetch_nearest_bytes(&self, num_bits: usize) -> Vec<u8>;
Expand Down

1 comment on commit f37eedc

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Compilation Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: f37eedc Previous: 40d2763 Ratio
private-kernel-tail 1.154 s 0.944 s 1.22

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

Please sign in to comment.