Draft: Implement streaming Dory

[jprider63]

This draft PR implements the streaming version of Dory commitments. The commitments and proof hints match the non-streaming versions.

@zutshi and I still need to:

• Add parallelization
• Benchmark and optimize
• Cleanup code

One thing to note is that we were trying to avoid padding the trace length to a power of two when computing the commitments. It turns out the padding is necessary since the one hot polynomials treat the padded values differently. Some of the one hots were padding with Some(0) while others were padding with None.

[graphite-app]

The transpose function is implemented with todo!() but is called in production code at line 476 in jolt_dag.rs. This will cause a runtime panic when executed. A proper implementation is needed to transpose the matrix of row commitments from [row_chunks][polys] to [polys][row_chunks] format. Consider implementing this function before merging to prevent runtime failures.

Suggested change

	pub fn transpose<T>(v: Vec<Vec<T>>) -> Vec<Vec<T>> {
	todo!()
	}
	pub fn transpose<T: Clone>(v: Vec<Vec<T>>) -> Vec<Vec<T>> {
	if v.is_empty() {
	return vec![];
	}
	let len = v[0].len();
	let mut result = vec![Vec::with_capacity(v.len()); len];
	for row in v {
	for (i, val) in row.into_iter().enumerate() {
	result[i].push(val);
	}
	}
	result
	}

Spotted by Diamond



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

The process_chunk_t implementation for StreamingOneHotWitness returns todo!() which will cause a runtime panic when processing OneHot polynomials. This method needs to be completed to properly handle row commitments for OneHot polynomials, similar to how it's implemented for the other witness types.

Consider returning the computed row_commitments vector that's already being populated in the method, rather than the todo!() placeholder.

Suggested change

	// row_commitments // TODO
	todo!()
	}
	// row_commitments
	row_commitments
	}

Spotted by Diamond



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

There's a potential out-of-bounds access in the loop. When i equals row_cycles.len() - 1, the code attempts to access row_cycles[i+1] which would be row_cycles[row_cycles.len()], causing a panic.

Consider either:

1. Limiting the loop to 0..row_cycles.len()-1
2. Adding boundary checking for the last element
3. Using a safe method like row_cycles.get(i+1).unwrap_or(&default_cycle)

This issue appears in the streaming witness generation code which is critical for the Dory commitment scheme.

Suggested change

	let cycle = row_cycles[i];
	let next_cycle = row_cycles[i+1];
	witness_type.generate_streaming_witness(preprocessing, &cycle, &next_cycle, ram_d)
	let cycle = row_cycles[i];
	let next_cycle = row_cycles.get(i+1).unwrap_or(&cycle);
	witness_type.generate_streaming_witness(preprocessing, &cycle, next_cycle, ram_d)

Spotted by Diamond



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

The ChunkWithPeekIterator trait and ChunksWithPeek iterator implementation contain todo!() placeholders but are being used in production code at line 466 in jolt_dag.rs with .chunks_with_peek(row_len). This will cause runtime panics when executed.

These methods need proper implementations to chunk the lazy trace iterator while providing peek functionality before this code can be merged. Consider implementing the peek functionality similar to how Peekable works in the standard library, but for chunks of items rather than individual items.

Suggested change

	fn chunks_with_peek(self, size: usize) -> ChunksWithPeek<Self> {
	todo!()
	}
	}
	impl<I: Iterator + Sized> ChunkWithPeekIterator for I { }
	impl<I:Iterator> Iterator for ChunksWithPeek<I> {
	type Item = Vec<I::Item>;
	fn next(&mut self) -> Option<Self::Item> {
	todo!()
	}
	}
	fn chunks_with_peek(self, size: usize) -> ChunksWithPeek<Self> {
	ChunksWithPeek {
	iter: self,
	size,
	peeked: None,
	}
	}
	}
	impl<I: Iterator + Sized> ChunkWithPeekIterator for I { }
	impl<I: Iterator> Iterator for ChunksWithPeek<I> {
	type Item = Vec<I::Item>;
	fn next(&mut self) -> Option<Self::Item> {
	if let Some(peeked) = self.peeked.take() {
	return Some(peeked);
	}
	let mut chunk = Vec::with_capacity(self.size);
	for _ in 0..self.size {
	match self.iter.next() {
	Some(item) => chunk.push(item),
	None => break,
	}
	}
	if chunk.is_empty() {
	None
	} else {
	Some(chunk)
	}
	}
	}
	impl<I: Iterator> ChunksWithPeek<I> {
	pub fn peek(&mut self) -> Option<&Vec<I::Item>> {
	if self.peeked.is_none() {
	self.peeked = self.next();
	}
	self.peeked.as_ref()
	}
	}

Spotted by Diamond



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

The StreamingProcessChunk implementations for HyperKZG all use todo!("Not supported"), which will cause runtime panics if called. Consider either:

1. Implementing these methods properly, or
2. Adding a compile-time check to prevent HyperKZG from being used with streaming operations, or
3. Returning a more graceful error that can be handled by the caller

Since this is a draft PR, documenting this as a known limitation to be addressed before finalization would be appropriate.

Spotted by Diamond



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

Consider handling the lazy_trace unwrap more gracefully. This could panic if lazy_trace is None, potentially causing runtime failures in production. A more robust approach would be to use pattern matching or if let Some(...) to handle the case where lazy_trace is not initialized, returning an appropriate error to the caller.

Suggested change

	.unwrap() //AZ: todo: maybe not unwrap and handle it better
	.ok_or_else(|| Error::InvalidState("lazy_trace not initialized".to_string()))?

Spotted by Diamond



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

Potential Panic in OneHot Processing

The code currently panics if K is None when processing OneHot polynomials, but there's no validation during initialization to ensure K is properly set. If a polynomial is initialized with a type other than Multilinear::OneHot{K} but later processed as an OneHot polynomial, this will cause a runtime panic.

Consider adding validation during initialization or when setting the polynomial type, or alternatively, return a proper error instead of panicking. This would make the code more robust against type mismatches and provide better diagnostics when errors occur.

// Example safer approach
fn process_chunk_t<'a>(s: &Self::State<'a>, chunk: &[StreamingOneHotWitness<Fr>]) -> Self::ChunkState {
    match s.K {
        Some(K) => {
            // Process with K
            // ...
        },
        None => {
            // Return an error or default value
            log::error!("K must be provided for OneHot polynomials");
            vec![JoltG1Wrapper::identity()]
        }
    }
}

Suggested change

	let Some(K) = s.K else {
	panic!("K must be provided for OneHot polynomials.");
	};
	let K = match s.K {
	Some(k) => k,
	None => {
	log::error!("K must be provided for OneHot polynomials");
	return vec![JoltG1Wrapper::identity()]; // Return default/empty state
	}
	};

Spotted by Diamond



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

The debug_assert_eq!(chunk.len(), DoryGlobals::get_num_columns()) check will be disabled in release builds. Consider replacing this with a regular assert! or adding proper runtime validation to ensure chunk lengths always match the expected number of columns. This would prevent potential buffer overflows or incorrect computations when running in release mode.

Spotted by Diamond



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

The early return at chunk.len() == 1 creates a potential issue where an item is consumed from self.peek but never returned to the caller. When the iterator reaches its last element, this condition will trigger, causing that final element to be discarded rather than returned as a single-element chunk. Consider returning Some(chunk) instead of None to ensure the last item is properly yielded, maintaining the iterator contract that all items should be processed.

Suggested change

	if chunk.len() == 1
	{
	return None;
	}
	if chunk.len() == 1
	{
	return Some(chunk);
	}

Spotted by Diamond



Is this helpful? React 👍 or 👎 to let us know.