Refactor/merkle hash agnostic#506
Conversation
…ytes Add hash_bytes() inherent method to FieldElementVectorBackend that surfaces the generic digest parameter D. The prover now calls BatchedMerkleTreeBackend::<E>::hash_bytes() instead of hardcoding sha3::Keccak256, so changing the config.rs type alias automatically propagates the hash function to the commit hot paths. Also adds missing debug_assert for power-of-two in commit_columns_bit_reversed for consistency with commit_composition_polynomial.
|
/bench |
Codex Code ReviewNo actionable issues found in this PR diff. I checked for security regressions (unsafe/panics/memory/crypto consistency), logic bugs, and significant performance/simplicity concerns in the changed code paths. The Residual risk/testing gap: I could not run Rust tests in this environment because |
Benchmark — fib_iterative_8M (median of 10)Table parallelism: 32 (auto = cores / 3)
Commit: ec2ddf7 · Baseline: built from main · Runner: self-hosted bench |
PR #506 Review: merkle-hash-agnostic refactorSummary: Introduces a Medium
LowDuplicate The original code had two identical Missing test: non-canonical component values for the degree-3 extension. The Goldilocks tests cover a non-canonical base field element ( Overall the refactor is clean and correct for the current configuration. The main concern is the |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
/bench 10 |
a41bdf8
into
perf/merkle-zero-alloc-hashing
…#503) * perf: eliminate per-element Vec<u8> allocation in Merkle tree hashing Add WriteBytes trait for zero-allocation byte serialization and use it in the prover's Merkle commitment hot paths. Problem: as_bytes() returns Vec<u8> (heap allocation) for every field element. For CPU table commitment (2^20 rows × 74 cols), that's ~77M allocations of 8-byte Vecs. Benchmarks show this as a 1.3-1.7x slowdown. Solution: - WriteBytes trait: write_bytes_be(&self, buf: &mut [u8]) writes to caller-provided buffer, zero heap allocations per element. - Implemented for Goldilocks (8 bytes) and cubic extension (24 bytes). - commit_columns_bit_reversed: single buffer per row, write all columns directly, hash once. Eliminates both the row Vec and per-element Vec. - commit_composition_polynomial: same approach for composition pairs. The AsBytes trait and FieldElementVectorBackend are unchanged — only the prover hot paths are optimized. Verifier and generic code still use as_bytes() for backward compatibility. Benchmarked in lambdaworks (~/dev/lambdaworks bench/merkle-comparison): - 16 cols: 1.3-1.5x speedup, beats Plonky3 by 1.3-1.4x - 64 cols: 1.6-1.8x speedup, beats Plonky3 by 1.5-1.6x * add debug_asserts, fix comments, remove dead code, add tests * fix lint * refactor: make prover leaf-hashing hash-agnostic via backend's hash_bytes (#506) Add hash_bytes() inherent method to FieldElementVectorBackend that surfaces the generic digest parameter D. The prover now calls BatchedMerkleTreeBackend::<E>::hash_bytes() instead of hardcoding sha3::Keccak256, so changing the config.rs type alias automatically propagates the hash function to the commit hot paths. Also adds missing debug_assert for power-of-two in commit_columns_bit_reversed for consistency with commit_composition_polynomial. * refactor: fold WriteBytes into ByteConversion trait Consolidate serialization traits by adding BYTE_LEN and write_bytes_be to ByteConversion instead of having a separate WriteBytes trait. Goldilocks and cubic extension override write_bytes_be with zero-alloc implementations; other types use the default which delegates to to_bytes_be. --------- Co-authored-by: Nicole <nicole.graus@lambdaclass.com> Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com> Co-authored-by: MauroFab <maurotoscano2@gmail.com>
1 participant
No description provided.