Repository navigation
Add permute_hash_values utility - #608
Conversation
PR SummaryMedium Risk Overview The implementation tags each digest word in the free Tests cover valid reordering, duplicate hashes, panic when outputs are not a true hash-level permutation (including per-column word mixing), and equality of built Reviewed by Cursor Bugbot for commit fec28a2. Bugbot is set up for automated code reviews on this repo. Configure here. |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
ba22d17 to
36fd3cf
Compare
leo-starkware
left a comment
There was a problem hiding this comment.
@leo-starkware made 4 comments.
Reviewable status: 0 of 6 files reviewed, 4 unresolved discussions (waiting on ilyalesokhin-starkware).
crates/unpacker/src/lib.rs line 1 at r2 (raw file):
pub mod perm;
why not "permutation"?
Code quote:
permcrates/unpacker/src/perm.rs line 48 at r2 (raw file):
) -> Vec<HashValue<Var>> { let inputs: Vec<HashValue<Var>> = inputs.collect(); let outputs: Vec<HashValue<Var>> = outputs.collect();
why not accept slices?
Code quote:
inputs: impl ExactSizeIterator<Item = HashValue<Var>>,
outputs: impl ExactSizeIterator<Item = HashValue<Var>>,
) -> Vec<HashValue<Var>> {
let inputs: Vec<HashValue<Var>> = inputs.collect();
let outputs: Vec<HashValue<Var>> = outputs.collect();crates/unpacker/src/perm.rs line 69 at r2 (raw file):
let key = std::array::from_fn(|c| ctx.get(*hash[c].get()).unpack_u32()); indices_by_value.entry(key).or_default().push(j); }
Suggestion:
// Tag each input word with its hash index `j`: `(low, high, 0, 0) + (0, 0, j, 0)`.
let mut tagged_in: Vec<[Var; BLAKE2S_DIGEST_N_WORDS]> = Vec::with_capacity(n);
// Map each distinct input-hash value (its eight u32 words) to the input indices that hold it.
let mut indices_by_value: HashMap<[u32; BLAKE2S_DIGEST_N_WORDS], Vec<usize>> = HashMap::new();
for (j, hash) in inputs.iter().enumerate() {
let tag = ctx.constant(qm31_from_u32s(0, 0, j as u32, 0));
let tagged_input = std::array::from_fn(|c| eval!(ctx, (*hash[c].get()) + (tag)));
tagged_in.push(tagged_input);
let key = std::array::from_fn(|c| ctx.get(*hash[c].get()).unpack_u32());
indices_by_value.entry(key).or_default().push(j);
}crates/unpacker/src/perm.rs line 84 at r2 (raw file):
// consumed once). Falls back to 0 if `outputs` is not actually a permutation of `inputs`, // which leaves the per-column multiset checks unsatisfiable. let src = indices_by_value.get_mut(&key).and_then(|idxs| idxs.pop()).unwrap_or(0);
Suggestion:
// Witness: an as-yet-unused input index whose hash equals this output (each input index is
// consumed once).
let src = indices_by_value.get_mut(&key).and_then(|idxs| idxs.pop()).unwrap();
leo-starkware
left a comment
There was a problem hiding this comment.
@leo-starkware made 1 comment.
Reviewable status: 0 of 6 files reviewed, 5 unresolved discussions (waiting on ilyalesokhin-starkware).
crates/unpacker/src/perm.rs line 35 at r2 (raw file):
/// variables, and each such variable is constrained equal to the corresponding tagged output. The /// per-column multiset checks plus the shared-per-hash tag together guarantee each output hash /// equals exactly one input hash, i.e. `outputs` is a genuine permutation of `inputs`.
Suggestion:
/// Every word is *tagged* with the index of its source hash in the `u`
/// coordinate — words are packed as `(low_u16, high_u16, 0, 0)`, so the `u` coordinate is free and
/// a tagged word is `(low_u16, high_u16, index, 0)`:
///
/// * each input hash `j` is tagged with the constant `j`;
/// * each output hash is tagged with a single base-field-guessed source index `s`, reused for all
/// [`BLAKE2S_DIGEST_N_WORDS`] of its words.
///
/// A per-column [`Permutation`] gate then ties the tagged inputs to fresh permutation-output
/// variables, and each such variable is constrained equal to the corresponding tagged output. The
/// per-column multiset checks plus the shared-per-hash tag together guarantee each output hash
/// equals exactly one input hash, i.e. `outputs` is a genuine permutation of `inputs`.4669e18 to
38ec0a8
Compare
ilyalesokhin-starkware
left a comment
There was a problem hiding this comment.
@ilyalesokhin-starkware reviewed 1 file and made 4 comments.
Reviewable status: 0 of 7 files reviewed, 5 unresolved discussions (waiting on leo-starkware).
crates/unpacker/src/lib.rs line 1 at r2 (raw file):
Previously, leo-starkware wrote…
why not "permutation"?
Done.
crates/unpacker/src/perm.rs line 48 at r2 (raw file):
Previously, leo-starkware wrote…
why not accept slices?
Done.
crates/unpacker/src/perm.rs line 35 at r2 (raw file):
/// variables, and each such variable is constrained equal to the corresponding tagged output. The /// per-column multiset checks plus the shared-per-hash tag together guarantee each output hash /// equals exactly one input hash, i.e. `outputs` is a genuine permutation of `inputs`.
Done.
crates/unpacker/src/perm.rs line 69 at r2 (raw file):
let key = std::array::from_fn(|c| ctx.get(*hash[c].get()).unpack_u32()); indices_by_value.entry(key).or_default().push(j); }
Done.
ilyalesokhin-starkware
left a comment
There was a problem hiding this comment.
@ilyalesokhin-starkware partially reviewed 6 files and all commit messages, and made 1 comment.
Reviewable status: 6 of 7 files reviewed, 5 unresolved discussions (waiting on leo-starkware).
crates/unpacker/src/perm.rs line 84 at r2 (raw file):
// consumed once). Falls back to 0 if `outputs` is not actually a permutation of `inputs`, // which leaves the per-column multiset checks unsatisfiable. let src = indices_by_value.get_mut(&key).and_then(|idxs| idxs.pop()).unwrap_or(0);
Done.
leo-starkware
left a comment
There was a problem hiding this comment.
@leo-starkware made 3 comments and resolved 4 discussions.
Reviewable status: 6 of 7 files reviewed, 4 unresolved discussions (waiting on ilyalesokhin-starkware).
crates/unpacker/src/permutation.rs line 55 at r3 (raw file):
// distinct input-hash value (its eight u32 words) to the input indices that hold it, so each // output's source index can be looked up in O(1) instead of by a linear scan. Duplicate input // hashes map to several indices and are consumed one per matching output.
Suggestion:
// Tag each input word with its hash index `j`: `(low, high, 0, 0) + (0, 0, j, 0)`, and map each
// distinct input-hash value (its eight u32 words) to the input indices that hold it. Duplicate input
// hashes map to several indices and are consumed one per matching output.crates/unpacker/src/permutation.rs line 58 at r3 (raw file):
let mut tagged_in: Vec<Vec<Var>> = (0..BLAKE2S_DIGEST_N_WORDS).map(|_| Vec::with_capacity(n)).collect(); let mut indices_by_value: HashMap<[u32; BLAKE2S_DIGEST_N_WORDS], Vec<usize>> = HashMap::new();
Suggestion:
HashMap::with_capacity(n);crates/unpacker/src/permutation.rs line 79 at r3 (raw file):
// constrained to the base field and lifted into the `u` coordinate. One guess per output // hash, shared across all of its words. let src = indices_by_value
or something like that? src is a bit unclear
Suggestion:
let index_in_inputs = indices_by_value
leo-starkware
left a comment
There was a problem hiding this comment.
@leo-starkware partially reviewed 7 files and all commit messages, and resolved 1 discussion.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on ilyalesokhin-starkware).
38ec0a8 to
f587251
Compare
ilyalesokhin-starkware
left a comment
There was a problem hiding this comment.
@ilyalesokhin-starkware reviewed 1 file and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on leo-starkware).
crates/unpacker/src/permutation.rs line 79 at r3 (raw file):
Previously, leo-starkware wrote…
or something like that? src is a bit unclear
Done.
crates/unpacker/src/permutation.rs line 55 at r3 (raw file):
// distinct input-hash value (its eight u32 words) to the input indices that hold it, so each // output's source index can be looked up in O(1) instead of by a linear scan. Duplicate input // hashes map to several indices and are consumed one per matching output.
Done.
crates/unpacker/src/permutation.rs line 58 at r3 (raw file):
let mut tagged_in: Vec<Vec<Var>> = (0..BLAKE2S_DIGEST_N_WORDS).map(|_| Vec::with_capacity(n)).collect(); let mut indices_by_value: HashMap<[u32; BLAKE2S_DIGEST_N_WORDS], Vec<usize>> = HashMap::new();
Done.
Constrains a list of output HashValues to be a multiset permutation of a list of input HashValues, treating each hash as an atomic unit. Each word is tagged with its source-hash index in the u coordinate; inputs are tagged by position and each output hash by a single base-field-guessed source index reused across all of its words, so per-column permutation gates cannot mix words from different input hashes. Both lists are taken as slices. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
f587251 to
fec28a2
Compare
leo-starkware
left a comment
There was a problem hiding this comment.
@leo-starkware reviewed 1 file and all commit messages, made 1 comment, and resolved 3 discussions.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on ilyalesokhin-starkware).

Constrains a list of output HashValues to be a multiset permutation of a
list of input HashValues, treating each hash as an atomic unit. Each word
is tagged with its source-hash index in the u coordinate; inputs are
tagged by position and each output hash by a single base-field-guessed
source index reused across all of its words, so per-column permutation
gates cannot mix words from different input hashes. Both lists are taken
as ExactSizeIterators and the outputs are returned.
Source indices are recovered for witness generation via a HashMap from
each input hash's u32 words to its indices (duplicates consumed one at a
time); this never affects circuit topology.
Lives in utils.rs (a general circuit utility) rather than blake.rs, since
it permutes existing HashValues rather than computing Blake2s hashes.
Tests: a positive reordering; duplicate-hash handling; two soundness
tests rejecting a non-permutation and a column-mixed (per-column-valid)
output; and a witness-independence test asserting the QM31-built and
NoValue-built circuit structures are identical.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com