Skip to content

Commit ea79d1f

Browse files
committed
fix(common): reject FIELD_IDs whose .np magic aliases another format
`np_format` derives the `.np` magic by offsetting the final byte with `FIELD_ID`, which reserves all 256 values of that byte for `.np`. Nothing recorded or enforced that reservation. Assert it at compile time against the other three format constants, so a field whose magic would collide is a build error rather than a file one reader silently accepts as another. Offsetting one byte is a bijection on `u8`, so distinct ids always give distinct magics and no field can collide with another. What `np_format` cannot see, working one field at a time, is two backends declaring the same `FIELD_ID`. `field_tags.rs` is the only place all three fields are in scope together, so it asserts the ids and magics are pairwise unique. Reading `FORMAT` there also const-evaluates the new assert for the non-bn254 fields, which nothing else in the workspace does.
1 parent 85798ea commit ea79d1f

2 files changed

Lines changed: 86 additions & 5 deletions

File tree

provekit/common/src/whir_r1cs.rs

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -259,13 +259,45 @@ pub struct ProvekitProof<P: ProofField> {
259259
pub whir_r1cs_proof: WhirR1CSProof,
260260
}
261261

262+
/// Formats whose magic `.np` must never alias.
263+
#[cfg(not(target_arch = "wasm32"))]
264+
const NON_NP_FORMATS: [[u8; 8]; 3] = [
265+
binary_format::PROVER_FORMAT,
266+
binary_format::VERIFIER_FORMAT,
267+
binary_format::NOIR_PROOF_SCHEME_FORMAT,
268+
];
269+
270+
#[cfg(not(target_arch = "wasm32"))]
271+
const fn formats_eq(a: [u8; 8], b: [u8; 8]) -> bool {
272+
let mut i = 0;
273+
while i < 8 {
274+
if a[i] != b[i] {
275+
return false;
276+
}
277+
i += 1;
278+
}
279+
true
280+
}
281+
262282
/// Derive the `.np` format magic from [`ProofField::FIELD_ID`] by offsetting
263283
/// the final magic byte: bn254 (id 0) keeps the historical magic, other fields
264284
/// a distinct one.
285+
///
286+
/// The offset is a bijection on `u8`, so distinct ids always yield distinct
287+
/// magics — at the cost of reserving all 256 values of the final byte.
265288
#[cfg(not(target_arch = "wasm32"))]
266289
const fn np_format(field_id: u8) -> [u8; 8] {
267290
let mut f = binary_format::NOIR_PROOF_FORMAT;
268291
f[7] = f[7].wrapping_add(field_id);
292+
293+
let mut i = 0;
294+
while i < NON_NP_FORMATS.len() {
295+
assert!(
296+
!formats_eq(f, NON_NP_FORMATS[i]),
297+
"FIELD_ID makes the `.np` magic collide with another ProveKit file format"
298+
);
299+
i += 1;
300+
}
269301
f
270302
}
271303

@@ -286,13 +318,26 @@ impl<P: ProofField> MaybeHashAware for ProvekitProof<P> {
286318

287319
#[cfg(all(test, not(target_arch = "wasm32")))]
288320
mod tests {
289-
use super::{binary_format, np_format};
321+
use {
322+
super::{binary_format, np_format, NON_NP_FORMATS},
323+
std::collections::HashSet,
324+
};
290325

291326
#[test]
292-
fn np_format_preserves_bn254_and_distinguishes_fields() {
327+
fn np_format_preserves_bn254() {
293328
assert_eq!(np_format(0), binary_format::NOIR_PROOF_FORMAT);
294-
assert_ne!(np_format(1), np_format(0));
295-
assert_ne!(np_format(2), np_format(0));
296-
assert_ne!(np_format(2), np_format(1));
329+
}
330+
331+
#[test]
332+
fn np_format_is_collision_free_over_every_field_id() {
333+
let magics: HashSet<[u8; 8]> = (0..=u8::MAX).map(np_format).collect();
334+
assert_eq!(magics.len(), 256, "FIELD_ID -> magic must be injective");
335+
for other in NON_NP_FORMATS {
336+
assert!(
337+
!magics.contains(&other),
338+
"`.np` magic space collides with {:?}",
339+
std::str::from_utf8(&other)
340+
);
341+
}
297342
}
298343
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
//! Every proof field must carry a distinct `FIELD_ID` and `.np` magic, so one
2+
//! field's proof is rejected by another field's verifier.
3+
//!
4+
//! `np_format` sees one field at a time, so only a place with every backend in
5+
//! scope can catch two of them declaring the same `FIELD_ID`. Reading `FORMAT`
6+
//! here is also what const-evaluates `np_format`'s collision assert for the
7+
//! non-bn254 fields — nothing else in the workspace does.
8+
9+
use {
10+
provekit_backend_bn254::Bn254Field,
11+
provekit_backend_goldilocks::{GoldilocksEfField, GoldilocksField},
12+
provekit_common::{file::FileFormat, ProofField, ProvekitProof},
13+
std::collections::HashSet,
14+
};
15+
16+
#[test]
17+
fn field_tags_are_unique() {
18+
fn tag<P: ProofField>(name: &'static str) -> (&'static str, u8, [u8; 8]) {
19+
(name, P::FIELD_ID, <ProvekitProof<P> as FileFormat>::FORMAT)
20+
}
21+
let tags = [
22+
tag::<Bn254Field>("Bn254Field"),
23+
tag::<GoldilocksField>("GoldilocksField"),
24+
tag::<GoldilocksEfField>("GoldilocksEfField"),
25+
];
26+
27+
let ids: HashSet<u8> = tags.iter().map(|&(_, id, _)| id).collect();
28+
let magics: HashSet<[u8; 8]> = tags.iter().map(|&(_, _, magic)| magic).collect();
29+
let named: Vec<_> = tags
30+
.iter()
31+
.map(|&(name, id, magic)| (name, id, String::from_utf8_lossy(&magic).into_owned()))
32+
.collect();
33+
34+
assert_eq!(ids.len(), tags.len(), "FIELD_ID collision: {named:?}");
35+
assert_eq!(magics.len(), tags.len(), "`.np` magic collision: {named:?}");
36+
}

0 commit comments

Comments
 (0)