From ea7d2bda9f96c56dc753290c8c1fe8c556cbb1b0 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Sun, 28 Jan 2024 21:25:33 +0000 Subject: [PATCH] chore(ffi_interface): Switch from Big Endian to Little Endian (#76) * Change from Big endian to Little Endian Co-authored-by: Dragan Pilipovic * modify interop tests to show how to change the java tests * remove allocation --------- Co-authored-by: Dragan Pilipovic --- ffi_interface/src/interop.rs | 48 +++++++++++++++++++++++++++--------- ffi_interface/src/lib.rs | 36 +++++++++++---------------- 2 files changed, 51 insertions(+), 33 deletions(-) diff --git a/ffi_interface/src/interop.rs b/ffi_interface/src/interop.rs index f250d78..8af0600 100644 --- a/ffi_interface/src/interop.rs +++ b/ffi_interface/src/interop.rs @@ -123,7 +123,7 @@ pub(crate) fn group_to_field(point: &Element) -> Fr { mod test { use super::Java_org_hyperledger_besu_nativelib_ipamultipoint_LibIpaMultipoint_pedersenHash; use crate::{ - commit_to_scalars, deprecated_serialize_commitment, fr_to_be_bytes, get_tree_key_hash, + commit_to_scalars, deprecated_serialize_commitment, fr_to_le_bytes, get_tree_key_hash, hash_commitment, interop::{ Java_org_hyperledger_besu_nativelib_ipamultipoint_LibIpaMultipoint_commit, @@ -152,42 +152,68 @@ mod test { #[test] fn interop_commit() { - let scalars: Vec<_> = (0..256) + let scalars_le: Vec<_> = (0..256) .map(|i| { let val = Fr::from((i + 1) as u128); - fr_to_be_bytes(-val) + fr_to_le_bytes(-val) }) .flatten() .collect(); - let expected_hash = - Java_org_hyperledger_besu_nativelib_ipamultipoint_LibIpaMultipoint_commit(&scalars); + let scalars_be: Vec<_> = (0..256) + .map(|i| { + let val = Fr::from((i + 1) as u128); + let mut arr = fr_to_le_bytes(-val); + arr.reverse(); + arr + }) + .flatten() + .collect(); + + // The previous implementation will return the hash in big endian format + // The new implementation will return the hash in little endian format + // however. + let expected_hash_be = + Java_org_hyperledger_besu_nativelib_ipamultipoint_LibIpaMultipoint_commit(&scalars_be); + let expected_hash_le: Vec<_> = expected_hash_be.iter().rev().cloned().collect(); let crs = CRS::default(); let committer = DefaultCommitter::new(&crs.G); - let got_commitment = commit_to_scalars(&committer, &scalars).unwrap(); + let got_commitment = commit_to_scalars(&committer, &scalars_le).unwrap(); let got_hash = hash_commitment(got_commitment); - assert_eq!(got_hash.to_vec(), expected_hash) + assert_eq!(got_hash.to_vec(), expected_hash_le) } #[test] fn interop_commit_root() { - let scalars: Vec<_> = (0..256) + let scalars_le: Vec<_> = (0..256) + .map(|i| { + let val = Fr::from((i + 1) as u128); + fr_to_le_bytes(-val) + }) + .flatten() + .collect(); + + let scalars_be: Vec<_> = (0..256) .map(|i| { let val = Fr::from((i + 1) as u128); - fr_to_be_bytes(-val) + let mut arr = fr_to_le_bytes(-val); + arr.reverse(); + arr }) .flatten() .collect(); let expected_hash = - Java_org_hyperledger_besu_nativelib_ipamultipoint_LibIpaMultipoint_commitRoot(&scalars); + Java_org_hyperledger_besu_nativelib_ipamultipoint_LibIpaMultipoint_commitRoot( + &scalars_be, + ); let crs = CRS::default(); let committer = DefaultCommitter::new(&crs.G); - let got_commitment = commit_to_scalars(&committer, &scalars).unwrap(); + let got_commitment = commit_to_scalars(&committer, &scalars_le).unwrap(); let got_hash = deprecated_serialize_commitment(got_commitment); assert_eq!(got_hash.to_vec(), expected_hash) } diff --git a/ffi_interface/src/lib.rs b/ffi_interface/src/lib.rs index be0f23c..ac0cc1e 100644 --- a/ffi_interface/src/lib.rs +++ b/ffi_interface/src/lib.rs @@ -95,7 +95,7 @@ fn _commit_to_scalars(committer: &DefaultCommitter, scalars: &[u8]) -> Result Result { let old_commitment = Element::from_bytes_unchecked_uncompressed(old_commitment_bytes); - let old_scalar = fr_from_be_bytes(&old_scalar_bytes)?; - let new_scalar = fr_from_be_bytes(&new_scalar_bytes)?; + let old_scalar = fr_from_le_bytes(&old_scalar_bytes)?; + let new_scalar = fr_from_le_bytes(&new_scalar_bytes)?; // w-v let delta = new_scalar - old_scalar; @@ -151,7 +151,7 @@ pub fn hash_commitment(commitment: CommitmentBytes) -> ScalarBytes { // TODO: We could introduce a method named `hash_commit_to_scalars` // TODO: which would save this serialization roundtrip. We should profile/check that // TODO: this is actually a bottleneck for the average workflow before doing this. - fr_to_be_bytes(Element::from_bytes_unchecked_uncompressed(commitment).map_to_scalar_field()) + fr_to_le_bytes(Element::from_bytes_unchecked_uncompressed(commitment).map_to_scalar_field()) } /// Hashes a vector of commitments. /// @@ -166,7 +166,7 @@ pub fn hash_commitments(commitments: &[CommitmentBytes]) -> Vec { Element::batch_map_to_scalar_field(&elements) .into_iter() - .map(fr_to_be_bytes) + .map(fr_to_le_bytes) .collect() } @@ -176,23 +176,15 @@ pub fn deprecated_serialize_commitment(commitment: CommitmentBytes) -> [u8; 32] Element::from_bytes_unchecked_uncompressed(commitment).to_bytes() } -// TODO: We use big endian bytes here to be interopable with the java implementation -// TODO: we should stick to one endianness everywhere to avoid confusion -fn fr_to_be_bytes(fr: banderwagon::Fr) -> [u8; 32] { +fn fr_to_le_bytes(fr: banderwagon::Fr) -> [u8; 32] { let mut bytes = [0u8; 32]; fr.serialize_compressed(&mut bytes[..]) .expect("Failed to serialize scalar to bytes"); - // serialized compressed outputs bytes in LE order, so we reverse to get BE order - bytes.reverse(); bytes } -fn fr_from_be_bytes(bytes: &[u8]) -> Result { - let mut bytes = bytes.to_vec(); - bytes.reverse(); // deserialize expects the bytes to be in little endian order - banderwagon::Fr::deserialize_uncompressed(&bytes[..]).map_err(|_| { - Error::FailedToDeserializeScalar { - bytes: bytes.to_vec(), - } +fn fr_from_le_bytes(bytes: &[u8]) -> Result { + banderwagon::Fr::deserialize_uncompressed(bytes).map_err(|_| Error::FailedToDeserializeScalar { + bytes: bytes.to_vec(), }) } @@ -267,7 +259,7 @@ mod tests { crs::CRS, }; - use crate::{fr_from_be_bytes, fr_to_be_bytes}; + use crate::{fr_from_le_bytes, fr_to_le_bytes}; #[test] fn commitment_update() { let crs = CRS::default(); @@ -295,8 +287,8 @@ mod tests { &committer, commitment.to_bytes_uncompressed(), 0, - fr_to_be_bytes(a_0), - fr_to_be_bytes(a_2), + fr_to_le_bytes(a_0), + fr_to_le_bytes(a_2), ) .unwrap(); @@ -306,8 +298,8 @@ mod tests { #[test] fn from_be_to_be_bytes() { let value = banderwagon::Fr::from(123456u128); - let bytes = fr_to_be_bytes(value); - let got_value = fr_from_be_bytes(&bytes).unwrap(); + let bytes = fr_to_le_bytes(value); + let got_value = fr_from_le_bytes(&bytes).unwrap(); assert_eq!(got_value, value) } }