Skip to content
Open
Show file tree
Hide file tree
Changes from 9 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
84 changes: 84 additions & 0 deletions crates/math/src/field/fields/u64_goldilocks_field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@
use core::fmt::{self, Display};

use crate::errors::CreationError;
use crate::field::traits::HasDefaultTranscript;
use crate::field::traits::{IsFFTField, IsField, IsPrimeField, IsSubFieldOf};
use crate::field::{element::FieldElement, errors::FieldError};
#[cfg(feature = "alloc")]
use crate::traits::AsBytes;
use crate::traits::ByteConversion;

// =====================================================
Expand Down Expand Up @@ -299,6 +302,13 @@ impl ByteConversion for FieldElement<Goldilocks64Field> {
}
}

#[cfg(feature = "alloc")]
impl AsBytes for FieldElement<Goldilocks64Field> {
fn as_bytes(&self) -> alloc::vec::Vec<u8> {
self.to_bytes_be()
}
}

// =====================================================
// x86-64 ASSEMBLY OPTIMIZATIONS
// =====================================================
Expand Down Expand Up @@ -836,6 +846,55 @@ impl Fp2E {
}
}

impl ByteConversion for FieldElement<Degree2GoldilocksExtensionField> {
#[cfg(feature = "alloc")]
fn to_bytes_be(&self) -> alloc::vec::Vec<u8> {
let mut bytes = alloc::vec::Vec::with_capacity(16);
bytes.extend_from_slice(&self.value()[0].to_bytes_be());
bytes.extend_from_slice(&self.value()[1].to_bytes_be());
bytes
}

#[cfg(feature = "alloc")]
fn to_bytes_le(&self) -> alloc::vec::Vec<u8> {
let mut bytes = alloc::vec::Vec::with_capacity(16);
bytes.extend_from_slice(&self.value()[0].to_bytes_le());
bytes.extend_from_slice(&self.value()[1].to_bytes_le());
bytes
}

fn from_bytes_be(bytes: &[u8]) -> Result<Self, crate::errors::ByteConversionError>
where
Self: Sized,
{
if bytes.len() < 16 {
return Err(crate::errors::ByteConversionError::FromBEBytesError);
}
let x0 = FpE::from_bytes_be(&bytes[..8])?;
let x1 = FpE::from_bytes_be(&bytes[8..16])?;
Ok(Self::new([x0, x1]))
}

fn from_bytes_le(bytes: &[u8]) -> Result<Self, crate::errors::ByteConversionError>
where
Self: Sized,
{
if bytes.len() < 16 {
return Err(crate::errors::ByteConversionError::FromLEBytesError);
}
let x0 = FpE::from_bytes_le(&bytes[..8])?;
let x1 = FpE::from_bytes_le(&bytes[8..16])?;
Ok(Self::new([x0, x1]))
}
}

#[cfg(feature = "alloc")]
impl AsBytes for FieldElement<Degree2GoldilocksExtensionField> {
fn as_bytes(&self) -> alloc::vec::Vec<u8> {
self.to_bytes_be()
}
}

// =====================================================
// CUBIC EXTENSION (Fp3)
// =====================================================
Expand Down Expand Up @@ -1000,6 +1059,31 @@ impl IsSubFieldOf<Degree3GoldilocksExtensionField> for Goldilocks64Field {
/// Field element type for the cubic extension
pub type Fp3E = FieldElement<Degree3GoldilocksExtensionField>;

// =====================================================
// DEFAULT TRANSCRIPT SUPPORT
// =====================================================

impl HasDefaultTranscript for Goldilocks64Field {
fn get_random_field_element_from_rng(rng: &mut impl rand::Rng) -> FieldElement<Self> {
let mut sample = [0u8; 8];
loop {
rng.fill(&mut sample);
let int_sample = u64::from_be_bytes(sample);
if int_sample < GOLDILOCKS_PRIME {
return FieldElement::from(int_sample);
}
}
}
}

impl HasDefaultTranscript for Degree2GoldilocksExtensionField {
fn get_random_field_element_from_rng(rng: &mut impl rand::Rng) -> FieldElement<Self> {
let c0 = Goldilocks64Field::get_random_field_element_from_rng(rng);
let c1 = Goldilocks64Field::get_random_field_element_from_rng(rng);
FieldElement::<Self>::new([c0, c1])
}
}

// =====================================================
// TESTS
// =====================================================

Choose a reason for hiding this comment

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

Correctness

  • Check that the changes involving ByteConversion for FieldElement<Degree2GoldilocksExtensionField> handle edge cases correctly. Particularly, ensure the new implementation correctly handles the case where the input byte slice is exactly 16 bytes long.

Security

  • In the get_random_field_element_from_rng function, ensure that the sample byte array is securely zeroized after use to avoid any leakage of sensitive data.
  • Consider the possibility of timing side-channels in the if int_sample < GOLDILOCKS_PRIME check due to branching based on secret data (the prime check).

Performance

  • Ensure that the use of alloc::vec::Vec::<u8>::with_capacity(16) is efficient and necessary. Consider alternatives if allocations can be reduced.

Bugs & Errors

  • Use of unchecked unwrap or panic could cause potential runtime issues; verify that this is expected behavior and double-check that all error cases are properly handled (e.g., unexpected byte lengths).

Code Simplicity

  • The implementation of from_bytes_be and from_bytes_le is quite similar; consider refactoring to avoid code duplication and improve maintainability.

Overall, this pull request addresses the implementation of byte conversion traits and transcript support for cryptographic field elements. Correct attention is needed for edge case handling, potential security vulnerabilities, such as timing attacks and proper zeroization of sensitive data, and ensuring efficient memory usage. Address these concerns before merging.

Expand Down
3 changes: 2 additions & 1 deletion crates/provers/stark/src/examples/fibonacci_rap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ where
&self,
trace: &mut TraceTable<Self::Field, Self::FieldExtension>,
challenges: &[FieldElement<F>],
) {
) -> Result<(), crate::prover::ProvingError> {
let main_segment_cols = trace.columns_main();
let not_perm = &main_segment_cols[0];
let perm = &main_segment_cols[1];
Expand All @@ -233,6 +233,7 @@ where
for (i, aux_elem) in aux_col.iter().enumerate().take(trace.num_rows()) {
trace.set_aux(i, 0, aux_elem.clone())
}
Ok(())
}

fn build_rap_challenges(
Expand Down
3 changes: 2 additions & 1 deletion crates/provers/stark/src/examples/read_only_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ where
&self,
trace: &mut TraceTable<Self::Field, Self::FieldExtension>,
challenges: &[FieldElement<F>],
) {
) -> Result<(), crate::prover::ProvingError> {
let main_segment_cols = trace.columns_main();
let a = &main_segment_cols[0];
let v = &main_segment_cols[1];
Expand All @@ -311,6 +311,7 @@ where
for (i, aux_elem) in aux_col.iter().enumerate().take(trace.num_rows()) {
trace.set_aux(i, 0, aux_elem.clone())
}
Ok(())
}

fn build_rap_challenges(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ where
&self,
trace: &mut TraceTable<Self::Field, Self::FieldExtension>,
challenges: &[FieldElement<E>],
) {
) -> Result<(), crate::prover::ProvingError> {
// Main table
let main_segment_cols = trace.columns_main();
let a = &main_segment_cols[0];
Expand Down Expand Up @@ -449,6 +449,7 @@ where
for (i, aux_elem) in aux_col.iter().enumerate().take(trace.num_rows()) {
trace.set_aux(i, 0, aux_elem.clone())
}
Ok(())
}

fn build_rap_challenges(
Expand Down
1 change: 1 addition & 0 deletions crates/provers/stark/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub mod examples;
pub mod frame;
pub mod fri;
pub mod grinding;
pub mod lookup;
pub mod multi_table_prover;
pub mod multi_table_verifier;
pub mod proof;

Choose a reason for hiding this comment

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

The code introduces a new module lookup, but no specific code was provided for review in the context given. Please ensure to review the following aspects of the new module:

  • Correctness: Ensure that operations such as lookup tables (if applicable) correctly handle all edge cases, including out-of-bound indices and zero elements.
  • Security: Verify that any use of secret data is constant-time and does not introduce side-channel vulnerabilities. Additionally, confirm that any randomness required is cryptographically secure and properly zeroizes sensitive data.
  • Performance: Check for unnecessary allocations and ensure efficient lookups, possibly amortizing costs of repeated operations.
  • Bugs & Errors: Make sure there are no panics with index operations and no potential for buffer overflows or memory safety issues.

Without the actual implementation code for the lookup module, it's challenging to assess these critical aspects fully. Please provide the code changes to conduct a thorough review.

Expand Down
Loading
Loading