From ae9824d4f24878c89a2a5e28035d8f602e48713f Mon Sep 17 00:00:00 2001 From: Paul Schoenfelder Date: Fri, 6 Sep 2024 17:43:13 -0400 Subject: [PATCH 1/2] fix: incorrect read_slice impl for ReadAdapter This commit fixes a bug that is present in the implementation of ByteReader for ReadAdapter. Specifically, it does not consume any input, so calling `read_slice` and then attempting to read the next value in the input, will read the same bytes again. The documented behavior of this function is that any of the `read_*` methods consume the corresponding amount of input. To catch this, and to prevent future regressions, a new test was added that serializes some data to a file using one approach, and deserializes it using the ReadAdapter. This ensures that we don't accidentally make choices in the writer that aren't matched by the reader, and vice versa. Closes #308 --- utils/core/src/serde/byte_reader.rs | 36 ++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/utils/core/src/serde/byte_reader.rs b/utils/core/src/serde/byte_reader.rs index 966edcfc4..19f6b70b2 100644 --- a/utils/core/src/serde/byte_reader.rs +++ b/utils/core/src/serde/byte_reader.rs @@ -504,7 +504,9 @@ impl<'a> ByteReader for ReadAdapter<'a> { // this will return an error if we hit EOF first self.buffer_at_least(len)?; - Ok(&self.buffer()[0..len]) + let slice = &self.buf[self.pos..(self.pos + len)]; + self.pos += len; + Ok(slice) } #[inline] @@ -740,4 +742,36 @@ mod tests { assert_eq!(adapter.read_usize(), Ok(VALUE)); } + + #[test] + fn read_adapter_for_file() { + use std::fs::File; + + use crate::ByteWriter; + + let path = std::env::temp_dir().join("read_adapter_for_file.bin"); + + // Encode some data to a buffer, then write that buffer to a file + { + let mut buf = Vec::::with_capacity(256); + buf.write_bytes(b"MAGIC\0"); + buf.write_bool(true); + buf.write_u32(0xbeef); + buf.write_usize(0xfeed); + buf.write_u16(0x5); + + std::fs::write(&path, &buf).unwrap(); + } + + // Open the file, and try to decode the encoded items + let mut file = File::open(&path).unwrap(); + let mut reader = ReadAdapter::new(&mut file); + assert_eq!(reader.peek_u8().unwrap(), b'M'); + assert_eq!(reader.read_slice(6).unwrap(), b"MAGIC\0"); + assert!(reader.read_bool().unwrap()); + assert_eq!(reader.read_u32().unwrap(), 0xbeef); + assert_eq!(reader.read_usize().unwrap(), 0xfeed); + assert_eq!(reader.read_u16().unwrap(), 0x5); + assert!(!reader.has_more_bytes(), "expected there to be no more data in the input"); + } } From ec52b2412bc7d163442fdc18dae757897ad80121 Mon Sep 17 00:00:00 2001 From: Paul Schoenfelder Date: Fri, 6 Sep 2024 17:54:27 -0400 Subject: [PATCH 2/2] chore: fix clippy warning about long paragraphs --- air/src/proof/ood_frame.rs | 8 +++++--- crypto/src/merkle/concurrent.rs | 8 +++++--- examples/src/utils/rescue.rs | 9 +++++---- math/src/field/f64/mod.rs | 2 ++ 4 files changed, 17 insertions(+), 10 deletions(-) diff --git a/air/src/proof/ood_frame.rs b/air/src/proof/ood_frame.rs index d4b3f14ec..d71019c88 100644 --- a/air/src/proof/ood_frame.rs +++ b/air/src/proof/ood_frame.rs @@ -230,9 +230,11 @@ impl Deserializable for OodFrame { // ================================================================================================ /// Stores the trace evaluations at `z` and `gz`, where `z` is a random Field element in -/// `current_row` and `next_row`, respectively. If the Air contains a Lagrange kernel auxiliary -/// column, then that column interpolated polynomial will be evaluated at `z`, `gz`, `g^2 z`, ... -/// `g^(2^(v-1)) z`, where `v == log(trace_len)`, and stored in `lagrange_kernel_frame`. +/// `current_row` and `next_row`, respectively. +/// +/// If the Air contains a Lagrange kernel auxiliary column, then that column interpolated polynomial +/// will be evaluated at `z`, `gz`, `g^2 z`, ... `g^(2^(v-1)) z`, where `v == log(trace_len)`, and +/// stored in `lagrange_kernel_frame`. pub struct TraceOodFrame { current_row: Vec, next_row: Vec, diff --git a/crypto/src/merkle/concurrent.rs b/crypto/src/merkle/concurrent.rs index 637bd51b5..cdf1b068f 100644 --- a/crypto/src/merkle/concurrent.rs +++ b/crypto/src/merkle/concurrent.rs @@ -18,9 +18,11 @@ pub const MIN_CONCURRENT_LEAVES: usize = 1024; // PUBLIC FUNCTIONS // ================================================================================================ -/// Builds all internal nodes of the Merkle using all available threads and stores the -/// results in a single vector such that root of the tree is at position 1, nodes immediately -/// under the root is at positions 2 and 3 etc. +/// Builds all internal nodes of the Merkle. +/// +/// All available threads are used, and the results are stored in a single vector, such that +/// the root of the tree is at position 1, and nodes immediately under the root are at positions +/// 2 and 3, and so on. pub fn build_merkle_nodes(leaves: &[H::Digest]) -> Vec { let n = leaves.len() / 2; diff --git a/examples/src/utils/rescue.rs b/examples/src/utils/rescue.rs index 323d44bf0..59b8cb02d 100644 --- a/examples/src/utils/rescue.rs +++ b/examples/src/utils/rescue.rs @@ -21,10 +21,11 @@ pub const RATE_WIDTH: usize = 4; /// Two elements (32-bytes) are returned as digest. const DIGEST_SIZE: usize = 2; -/// The number of rounds is set to 7 to provide 128-bit security level with 40% security margin; -/// computed using algorithm 7 from -/// security margin here differs from Rescue Prime specification which suggests 50% security -/// margin (and would require 8 rounds) primarily to make AIR a bit simpler. +/// The number of rounds is set to 7 to provide 128-bit security level with 40% security margin. +/// +/// Computed using algorithm 7 from , the security margin +/// here differs from Rescue Prime specification which suggests 50% security margin (and would +/// require 8 rounds) primarily to make AIR a bit simpler. pub const NUM_ROUNDS: usize = 7; /// Minimum cycle length required to describe Rescue permutation. diff --git a/math/src/field/f64/mod.rs b/math/src/field/f64/mod.rs index 119676076..783e59425 100644 --- a/math/src/field/f64/mod.rs +++ b/math/src/field/f64/mod.rs @@ -5,10 +5,12 @@ //! An implementation of a 64-bit STARK-friendly prime field with modulus $2^{64} - 2^{32} + 1$ //! using Montgomery representation. +//! //! Our implementation follows and is constant-time. //! //! This field supports very fast modular arithmetic and has a number of other attractive //! properties, including: +//! //! * Multiplication of two 32-bit values does not overflow field modulus. //! * Field arithmetic in this field can be implemented using a few 32-bit addition, subtractions, //! and shifts.