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

style+perf: clean-up and optimize remove_empty_byte_from_padded_bytes_unchecked fn #41

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/blob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ impl Blob {

/// Returns the length of the blob data.
///
/// This length reflects the size of the data, including any padding if applied.
/// This length reflects the size of the data, including any padding if
/// applied.
///
/// # Returns
///
Expand Down
16 changes: 10 additions & 6 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ pub enum PolynomialError {
#[error("commitment error: {0}")]
CommitError(String),

/// Error related to Fast Fourier Transform (FFT) operations with a descriptive message.
/// Error related to Fast Fourier Transform (FFT) operations with a
/// descriptive message.
#[error("FFT error: {0}")]
FFTError(String),

Expand All @@ -23,8 +24,8 @@ pub enum PolynomialError {
/// Errors related to KZG operations.
///
/// The `KzgError` enum encapsulates all possible errors that can occur during
/// KZG-related operations, including those from `PolynomialError` and `BlobError`.
/// It also includes additional errors specific to KZG operations.
/// KZG-related operations, including those from `PolynomialError` and
/// `BlobError`. It also includes additional errors specific to KZG operations.
#[derive(Clone, Debug, PartialEq, Error)]
pub enum KzgError {
/// Wraps errors originating from Polynomial operations.
Expand All @@ -46,19 +47,22 @@ pub enum KzgError {
#[error("commit error: {0}")]
CommitError(String),

/// Error related to Fast Fourier Transform (FFT) operations with a descriptive message.
/// Error related to Fast Fourier Transform (FFT) operations with a
/// descriptive message.
#[error("FFT error: {0}")]
FFTError(String),

/// A generic error with a descriptive message.
#[error("generic error: {0}")]
GenericError(String),

/// Error indicating an invalid denominator scenario, typically in mathematical operations.
/// Error indicating an invalid denominator scenario, typically in
/// mathematical operations.
#[error("invalid denominator")]
InvalidDenominator,

/// Error indicating an invalid input length scenario, typically in data processing.
/// Error indicating an invalid input length scenario, typically in data
/// processing.
#[error("invalid input length")]
InvalidInputLength,
}
92 changes: 67 additions & 25 deletions src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub fn set_bytes_canonical_manual(data: &[u8]) -> Fr {

// Functions being used

/// Copied the referenced bytes array argument into a Vec, inserting an empty
/// Copies the referenced bytes array argument into a Vec, inserting an empty
/// byte at the front of every 31 bytes. The empty byte is padded at the low
/// address, because we use big endian to interpret a field element.
/// This ensures every 32 bytes is within the valid range of a field element for
Expand Down Expand Up @@ -67,31 +67,72 @@ pub fn convert_by_padding_empty_byte(data: &[u8]) -> Vec<u8> {
valid_data
}

/// Removes the first byte from each 32-byte chunk in a byte slice (including
/// the last potentially incomplete one).
///
/// This function is the reverse of `convert_by_padding_empty_byte`. It takes a
/// byte slice that it assumed contains field elements, where each complete
/// field element is 32 bytes and begins with an empty padding byte that needs
/// to be removed. The final element may be smaller than 32 bytes, but should
/// also be 0-byte prefixed.
///
/// # Arguments
/// * `data` - 0-byte prefixed big-endian encoded 32-byte chunks representing
/// bn254 field elements. The final element may be shorter.
///
/// # Returns
/// A new `Vec<u8>` with the first byte of each field element removed. For
/// complete elements, this removes one byte per 32 bytes. For the final partial
/// element (if any), it still removes the first byte.
///
/// # Safety
/// This function is marked "unchecked" because it assumes without verification
/// that:
/// * The input contains bn254-encoded field elements are exactly 32 bytes
/// * The first byte of each field element is safe to remove
///
/// # Example
/// ```text
/// [0, 1, 2, 3, ..., 31, 0, 1, 2, 3] -> [1, 2, 3, ..., 31, 1, 2, 3]
/// ```
///
/// ```
/// # use rust_kzg_bn254::helpers::remove_empty_byte_from_padded_bytes_unchecked;
/// let mut input = vec![1u8; 70]; // Two complete 32-byte element plus 6 bytes
/// input[0] = 0; input[32] = 0;
///
/// let output = remove_empty_byte_from_padded_bytes_unchecked(&input);
///
/// assert_eq!(output, vec![1u8; 67]); // Two complete 31-byte element plus 5 bytes
/// ```
///
/// # Implementation Detail: this function is equivalent to this simple iterator chain:
/// ```ignore
/// data.chunks(BYTES_PER_FIELD_ELEMENT).flat_map(|chunk| &chunk[1..]).copied().collect()
/// ```
/// However, it is ~30x faster than the above because of the pre-allocation +
/// SIMD instructions optimization.
pub fn remove_empty_byte_from_padded_bytes_unchecked(data: &[u8]) -> Vec<u8> {
let data_size = data.len();
let parse_size = BYTES_PER_FIELD_ELEMENT;
let data_len = data_size.div_ceil(parse_size);

let put_size = BYTES_PER_FIELD_ELEMENT - 1;
let mut valid_data = vec![0u8; data_len * put_size];
let mut valid_len = valid_data.len();

for i in 0..data_len {
let start = i * parse_size + 1; // Skip the first byte which is the empty byte
let mut end = (i + 1) * parse_size;

if end > data_size {
end = data_size;
valid_len = i * put_size + end - start;
}

// Calculate the end of the slice in the output vector
let output_end = i * put_size + end - start;
valid_data[i * put_size..output_end].copy_from_slice(&data[start..end]);
// We pre-allocate the exact size of the output vector by calculating the number
// of zero bytes that will be removed from the input.
let empty_bytes_to_remove = data.len().div_ceil(BYTES_PER_FIELD_ELEMENT);
let mut output = Vec::with_capacity(data.len() - empty_bytes_to_remove);

// We first process all the complete 32-byte chunks (representing bn254 encoded
// field elements). We remove the first byte of each chunk, assuming (but
// unchecked) that it is a zero byte. Note: we could use a single iterator
// loop, but separating like this allows the compiler to generate simd
// instructions for this main loop, which is much faster (see https://en.wikipedia.org/wiki/Automatic_vectorization).
for chunk in data.chunks_exact(BYTES_PER_FIELD_ELEMENT) {
output.extend_from_slice(&chunk[1..]);
}

valid_data.truncate(valid_len);
valid_data
// We handle the last chunk separately, still assuming (but unchecked) that
// it represents a zero prefixed partial field element.
let remainder = data.chunks_exact(BYTES_PER_FIELD_ELEMENT).remainder();
if !remainder.is_empty() {
output.extend_from_slice(&remainder[1..]);
}
output
}

pub fn set_bytes_canonical(data: &[u8]) -> Fr {
Expand Down Expand Up @@ -127,7 +168,8 @@ pub fn to_fr_array(data: &[u8]) -> Vec<Fr> {
/// * `max_data_size` - Maximum allowed size in bytes for the output buffer
///
/// # Returns
/// * `Vec<u8>` - Byte array containing the encoded field elements, truncated if needed
/// * `Vec<u8>` - Byte array containing the encoded field elements, truncated if
/// needed
///
/// # Details
/// - Each field element is converted to BYTES_PER_FIELD_ELEMENT bytes
Expand Down
58 changes: 34 additions & 24 deletions src/kzg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ use crate::{
traits::ReadPointFromBytes,
};

use crate::consts::{
Endianness, FIAT_SHAMIR_PROTOCOL_DOMAIN, KZG_ENDIANNESS, RANDOM_CHALLENGE_KZG_BATCH_DOMAIN,
use crate::{
consts::{
Endianness, FIAT_SHAMIR_PROTOCOL_DOMAIN, KZG_ENDIANNESS, RANDOM_CHALLENGE_KZG_BATCH_DOMAIN,
},
helpers::is_on_curve_g1,
};
use crate::helpers::is_on_curve_g1;
use ark_bn254::{Bn254, Fr, G1Affine, G1Projective, G2Affine, G2Projective};
use ark_ec::{pairing::Pairing, AffineRepr, CurveGroup, VariableBaseMSM};
use ark_ff::{BigInteger, Field, PrimeField};
Expand Down Expand Up @@ -127,11 +129,12 @@ impl KZG {
}

/// Calculates the roots of unities but doesn't assign it to the struct
/// Used in batch verification process as the roots need to be calculated for each blob
/// because of different length.
/// Used in batch verification process as the roots need to be calculated
/// for each blob because of different length.
///
/// # Arguments
/// * `length_of_data_after_padding` - Length of the blob data after padding in bytes.
/// * `length_of_data_after_padding` - Length of the blob data after padding
/// in bytes.
///
/// # Returns
/// * `Result<(Params, Vec<Fr>), KzgError>` - Tuple containing:
Expand Down Expand Up @@ -181,7 +184,8 @@ impl KZG {
// Set the maximum FFT width
params.max_fft_width = 1_u64 << log2_of_evals;

// Check if the length of data after padding is valid with respect to the SRS order
// Check if the length of data after padding is valid with respect to the SRS
// order
if length_of_data_after_padding
.div_ceil(BYTES_PER_FIELD_ELEMENT as u64)
.next_power_of_two()
Expand Down Expand Up @@ -333,8 +337,8 @@ impl KZG {
roots
}

/// Precompute the primitive roots of unity for binary powers that divide r - 1
/// TODO(anupsv): Move this to the constants file. Ref: https://github.com/Layr-Labs/rust-kzg-bn254/issues/31
/// Precompute the primitive roots of unity for binary powers that divide r
/// - 1 TODO(anupsv): Move this to the constants file. Ref: https://github.com/Layr-Labs/rust-kzg-bn254/issues/31
Copy link
Collaborator

Choose a reason for hiding this comment

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

why break a line at -1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

argh that's what our linter when ran with nightly version does.... think I should just revert that commit?
@anupsv we'll need to look at that linter config at some point. It seems not that great.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted the cargo +nightly fmt commit and formatted with stable rust instead. PTAL

fn get_primitive_roots_of_unity() -> Result<Vec<Fr>, KzgError> {
let data: [&str; 29] = [
"1",
Expand Down Expand Up @@ -896,13 +900,14 @@ impl KZG {
let evaluation_challenge = Self::compute_challenge(blob, commitment)?;

// Compute the actual KZG proof using the polynomial and evaluation point
// This creates a proof that the polynomial evaluates to a specific value at the challenge point
// The proof is a single G1 point that can be used to verify the evaluation
// This creates a proof that the polynomial evaluates to a specific value at the
// challenge point The proof is a single G1 point that can be used to
// verify the evaluation
self.compute_proof_impl(&blob_poly, &evaluation_challenge)
}

/// Maps a byte slice to a field element (`Fr`) using SHA-256 from SHA3 family as the
/// hash function.
/// Maps a byte slice to a field element (`Fr`) using SHA-256 from SHA3
/// family as the hash function.
///
/// # Arguments
///
Expand Down Expand Up @@ -1084,7 +1089,8 @@ impl KZG {

// Step 2: Generate Fiat-Shamir challenge
// This creates a "random" evaluation point based on the blob and commitment
// The challenge is deterministic but unpredictable, making the proof non-interactive
// The challenge is deterministic but unpredictable, making the proof
// non-interactive
let evaluation_challenge = Self::compute_challenge(&blobs[i], &commitments[i])?;

// Step 3: Evaluate the polynomial at the challenge point
Expand Down Expand Up @@ -1155,8 +1161,9 @@ impl KZG {
let (evaluation_challenges, ys) =
Self::compute_challenges_and_evaluate_polynomial(blobs, commitments, self.srs_order)?;

// Convert each blob to its polynomial evaluation form and get the length of number of field elements
// This length value is needed for computing the challenge
// Convert each blob to its polynomial evaluation form and get the length of
// number of field elements This length value is needed for computing
// the challenge
let blobs_as_field_elements_length: Vec<u64> = blobs
.iter()
.map(|blob| blob.to_polynomial_eval_form().evaluations().len() as u64)
Expand All @@ -1178,10 +1185,11 @@ impl KZG {
}

/// Ref: https://github.com/ethereum/consensus-specs/blob/master/specs/deneb/polynomial-commitments.md#verify_kzg_proof_batch
/// A helper function to the `helpers::compute_powers` function. This does the below reference code from the 4844 spec.
/// Ref: `# Append all inputs to the transcript before we hash
/// for commitment, z, y, proof in zip(commitments, zs, ys, proofs):
/// data += commitment + bls_field_to_bytes(z) + bls_field_to_bytes(y) + proof``
/// A helper function to the `helpers::compute_powers` function. This does
/// the below reference code from the 4844 spec. Ref: `# Append all
/// inputs to the transcript before we hash for commitment, z, y,
/// proof in zip(commitments, zs, ys, proofs): data +=
/// commitment + bls_field_to_bytes(z) + bls_field_to_bytes(y) + proof``
fn compute_r_powers(
&self,
commitments: &[G1Affine],
Expand All @@ -1201,7 +1209,8 @@ impl KZG {

// Calculate total input size:
// - initial_data_length (40 bytes)
// - For the number of commitments/zs/ys/proofs/blobs_as_field_elements_length (which are all the same length):
// - For the number of commitments/zs/ys/proofs/blobs_as_field_elements_length
// (which are all the same length):
// * BYTES_PER_FIELD_ELEMENT for commitment
// * 2 * BYTES_PER_FIELD_ELEMENT for z and y values
// * BYTES_PER_FIELD_ELEMENT for proof
Expand Down Expand Up @@ -1301,7 +1310,6 @@ impl KZG {
/// * `Ok(true)` if all proofs are valid.
/// * `Ok(false)` if any proof is invalid.
/// * `Err(KzgError)` if an error occurs during verification.
///
fn verify_kzg_proof_batch(
&self,
commitments: &[G1Affine],
Expand Down Expand Up @@ -1346,7 +1354,8 @@ impl KZG {

// Initialize vectors to store:
// c_minus_y: [C_i - [y_i]] (commitment minus the evaluation point encrypted)
// r_times_z: [r^i * z_i] (powers of random challenge times evaluation points)
// r_times_z: [r^i * z_i] (powers of random challenge times evaluation
// points)
let mut c_minus_y: Vec<G1Affine> = Vec::with_capacity(n);
let mut r_times_z: Vec<Fr> = Vec::with_capacity(n);

Expand Down Expand Up @@ -1379,7 +1388,8 @@ impl KZG {
let rhs_g1 = c_minus_y_lincomb + proof_z_lincomb;

// Verify the pairing equation:
// e(Σ(r^i * proof_i), [τ]) = e(Σ(r^i * (C_i - [y_i])) + Σ(r^i * z_i * proof_i), [1])
// e(Σ(r^i * proof_i), [τ]) = e(Σ(r^i * (C_i - [y_i])) + Σ(r^i * z_i * proof_i),
// [1])
let result = Self::pairings_verify(
proof_lincomb,
*self.get_g2_tau()?,
Expand Down
1 change: 0 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@
//! ```rust
//! // TODO:
//! ```
//!

mod arith;
pub mod blob;
Expand Down
9 changes: 6 additions & 3 deletions src/polynomial.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ impl PolynomialEvalForm {
///
/// # Returns
///
/// An `Option` containing a reference to the `Fr` element if the index is within bounds, or `None` otherwise.
/// An `Option` containing a reference to the `Fr` element if the index is
/// within bounds, or `None` otherwise.
pub fn get_evalualtion(&self, i: usize) -> Option<&Fr> {
self.evaluations.get(i)
}
Expand All @@ -87,11 +88,13 @@ impl PolynomialEvalForm {
self.evaluations.is_empty()
}

/// Converts all `Fr` elements in the polynomial to a single big-endian byte vector.
/// Converts all `Fr` elements in the polynomial to a single big-endian byte
/// vector.
///
/// # Returns
///
/// A `Vec<u8>` containing the big-endian byte representation of the polynomial elements.
/// A `Vec<u8>` containing the big-endian byte representation of the
/// polynomial elements.
pub fn to_bytes_be(&self) -> Vec<u8> {
helpers::to_byte_array(&self.evaluations, self.len_underlying_blob_bytes)
}
Expand Down
3 changes: 2 additions & 1 deletion tests/kzg_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,8 @@ mod tests {
let blobs = vec![input1, input2];
let commitments = vec![commitment1, commitment2];
let proofs = vec![proof_1, proof_2];
// let res = kzg.verify_blob_kzg_proof(&input1, &commitment1, &auto_proof).unwrap();
// let res = kzg.verify_blob_kzg_proof(&input1, &commitment1,
// &auto_proof).unwrap();

let pairing_result = kzg
.verify_blob_kzg_proof_batch(&blobs, &commitments, &proofs)
Expand Down
Loading