Skip to content

Commit 3be1eed

Browse files
MauroToscanoOppen
andauthored
test(math): pin stream_bytes byte parity with as_bytes (#832)
* test(math): pin stream_bytes byte parity with as_bytes #828 rewired the Merkle backends and DefaultTranscript::append_field_element from as_bytes()/to_bytes_be() to the new AsBytes::stream_bytes. That swap is only sound because stream_bytes emits byte-identical output, but nothing enforces it: stream_bytes is a defaulted trait method, so an override that disagrees with as_bytes compiles cleanly and then silently changes Merkle roots and Fiat-Shamir challenges rather than failing a test. Adds a parity sweep over both Goldilocks fields, including non-canonical u64s around the modulus where a raw-value override would diverge from as_bytes, and pins the ext3 byte layout that math-cuda's keccak_leaves_ext3 kernel mirrors component-by-component. The GPU parity tests only run on a CUDA host, so this keeps the CPU half of that contract honest on a GPU-less runner. Documents the invariant on the trait method itself, where an implementor would look for it. Also corrects the stream_bytes comment in extensions_goldilocks.rs: it described cutting three sink calls down to one, but as_bytes() already produced a single 24-byte Vec and a single Digest::update. The per-element allocation, not the call count, is what the override removes. * convert to proptest * test(math): merge stream_bytes edge-case tests into one Four near-identical `_edge_cases` wrappers, one per property, added pure boilerplate on top of the shared check_* helpers. Fold them into a single edge_cases test that runs all the checks over EDGE_VALUES. --------- Co-authored-by: Mario Rugiero <mrugiero@gmail.com>
1 parent a864832 commit 3be1eed

3 files changed

Lines changed: 193 additions & 4 deletions

File tree

‎crypto/math/src/field/extensions_goldilocks.rs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -555,10 +555,14 @@ impl AsBytes for FieldElement<Degree3GoldilocksExtensionField> {
555555
self.to_bytes_be()
556556
}
557557

558-
// One sink call over a stack buffer instead of three (one per limb): each
559-
// sink call lands as its own `Digest::update` on the guest, and dyn dispatch
560-
// here is fully devirtualized by the #[inline(always)] chain, so call count
561-
// — not indirection — is the cost being cut.
558+
// Same 24 bytes as `as_bytes`, staged in a stack buffer so the guest skips
559+
// the per-element `Vec`; `#[inline(always)]` is what lets the `dyn` sink
560+
// devirtualize at the call site. Emitting them in one call rather than one
561+
// per limb keeps it to a single `Digest::update`.
562+
//
563+
// The layout is load-bearing beyond this crate: `math-cuda`'s
564+
// `keccak_leaves_ext3` kernel reads components in order 0,1,2 to match
565+
// `write_bytes_be`, and CPU/GPU leaf parity depends on the two agreeing.
562566
#[inline(always)]
563567
fn stream_bytes(&self, sink: &mut dyn FnMut(&[u8])) {
564568
let mut buf = [0u8; 24];

‎crypto/math/src/traits.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,14 @@ pub trait AsBytes {
4242

4343
/// Streams the byte representation to `sink` without heap-allocating a `Vec`.
4444
/// Default falls back to `as_bytes`; override for zero-allocation hashing/transcript hot paths.
45+
///
46+
/// An override must stream exactly the bytes `as_bytes` would return, in
47+
/// order; splitting them across several `sink` calls is fine, but the
48+
/// concatenation must be identical. Merkle leaf hashes and the Fiat-Shamir
49+
/// transcript take their input through here, so an override that disagrees
50+
/// with `as_bytes` silently changes commitments and challenges rather than
51+
/// failing to compile. `math/tests/stream_bytes_parity.rs` pins this for the
52+
/// Goldilocks fields.
4553
fn stream_bytes(&self, sink: &mut dyn FnMut(&[u8])) {
4654
sink(&self.as_bytes());
4755
}
Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
//! `AsBytes::stream_bytes` must emit exactly the bytes `as_bytes` returns.
2+
//!
3+
//! Nothing in the type system enforces it: `stream_bytes` is a defaulted trait
4+
//! method, so an override that disagrees with `as_bytes` compiles cleanly and
5+
//! then silently changes every Merkle leaf hash and Fiat-Shamir challenge that
6+
//! flows through it — the transcript and the Merkle backends stream their input
7+
//! rather than calling `as_bytes`. A divergence would surface as proofs that no
8+
//! longer verify against previously committed roots, not as a test failure, so
9+
//! it is pinned here.
10+
//!
11+
//! `ext3_stream_bytes_matches_gpu_kernel_contract` additionally pins the ext3
12+
//! byte layout that `crypto/math-cuda/src/merkle.rs` mirrors: the GPU
13+
//! `keccak_leaves_ext3` kernel reads three canonical u64s per column in
14+
//! component order 0,1,2 to match `write_bytes_be`. CPU/GPU leaf parity depends
15+
//! on the two staying in agreement, and the GPU parity tests only run on a CUDA
16+
//! host, so this keeps the CPU half honest on a GPU-less runner.
17+
//!
18+
//! Each check is a plain function shared by two tests: a deterministic `#[test]`
19+
//! over hand-picked edge cases (always runs, no reliance on proptest landing on
20+
//! them) and a `proptest!` sweep over arbitrary input for everything else.
21+
22+
use math::field::element::FieldElement;
23+
use math::field::extensions_goldilocks::Degree3GoldilocksExtensionField;
24+
use math::field::goldilocks::GoldilocksField;
25+
use math::traits::{AsBytes, ByteConversion};
26+
use proptest::collection::vec;
27+
use proptest::prelude::*;
28+
29+
type Fp = FieldElement<GoldilocksField>;
30+
type Fp3 = FieldElement<Degree3GoldilocksExtensionField>;
31+
32+
fn streamed<T: AsBytes>(e: &T) -> Vec<u8> {
33+
let mut out = Vec::new();
34+
e.stream_bytes(&mut |b| out.extend_from_slice(b));
35+
out
36+
}
37+
38+
fn fp3(t: [u64; 3]) -> Fp3 {
39+
Fp3::new([Fp::from(t[0]), Fp::from(t[1]), Fp::from(t[2])])
40+
}
41+
42+
const GOLDILOCKS_P: u64 = 0xFFFF_FFFF_0000_0001;
43+
44+
/// Values around the modulus matter: both encodings reduce through
45+
/// `canonical_u64`, so a non-canonical `u64` is where a raw-value override
46+
/// would diverge from `as_bytes`.
47+
const EDGE_VALUES: [u64; 10] = [
48+
0,
49+
1,
50+
2,
51+
u32::MAX as u64,
52+
1u64 << 32,
53+
GOLDILOCKS_P - 1,
54+
GOLDILOCKS_P, // 0 in the field
55+
GOLDILOCKS_P + 1, // 1 in the field
56+
u64::MAX - 1,
57+
u64::MAX,
58+
];
59+
60+
fn check_goldilocks_stream_bytes(v: u64) {
61+
let e = Fp::from(v);
62+
let s = streamed(&e);
63+
assert_eq!(s.len(), 8, "goldilocks stream must be 8 bytes (v={v:#x})");
64+
// The Merkle backends stream instead of calling `as_bytes`.
65+
assert_eq!(s, e.as_bytes(), "stream != as_bytes (v={v:#x})");
66+
// `DefaultTranscript::append_field_element` streams instead of
67+
// appending `to_bytes_be`.
68+
assert_eq!(
69+
s,
70+
ByteConversion::to_bytes_be(&e),
71+
"stream != to_bytes_be (v={v:#x})"
72+
);
73+
}
74+
75+
fn check_ext3_stream_bytes(t: [u64; 3]) {
76+
let e = fp3(t);
77+
let s = streamed(&e);
78+
assert_eq!(s.len(), 24, "ext3 stream must be 24 bytes (t={t:?})");
79+
assert_eq!(s, e.as_bytes(), "stream != as_bytes (t={t:?})");
80+
assert_eq!(
81+
s,
82+
ByteConversion::to_bytes_be(&e),
83+
"stream != to_bytes_be (t={t:?})"
84+
);
85+
}
86+
87+
fn check_ext3_stream_bytes_gpu_kernel_contract(t: [u64; 3]) {
88+
let e = fp3(t);
89+
90+
// What the CUDA kernel builds: canonical u64 per component, big-endian,
91+
// component order 0,1,2.
92+
let mut expected = Vec::new();
93+
for component in e.value() {
94+
expected.extend_from_slice(&component.canonical_u64().to_be_bytes());
95+
}
96+
assert_eq!(
97+
streamed(&e),
98+
expected,
99+
"ext3 stream != canonical-BE 0,1,2 (t={t:?})"
100+
);
101+
102+
let mut buf = [0u8; 24];
103+
ByteConversion::write_bytes_be(&e, &mut buf);
104+
assert_eq!(streamed(&e), buf, "ext3 stream != write_bytes_be (t={t:?})");
105+
}
106+
107+
/// The default `stream_bytes` body forwards to `as_bytes`; a type that does not
108+
/// override it must still round-trip identically.
109+
struct Unoverridden(Vec<u8>);
110+
impl AsBytes for Unoverridden {
111+
fn as_bytes(&self) -> Vec<u8> {
112+
self.0.clone()
113+
}
114+
}
115+
116+
fn check_default_stream_bytes_impl(bytes: Vec<u8>) {
117+
let v = Unoverridden(bytes.clone());
118+
assert_eq!(streamed(&v), bytes);
119+
}
120+
121+
#[test]
122+
fn edge_cases() {
123+
for v in EDGE_VALUES {
124+
check_goldilocks_stream_bytes(v);
125+
check_ext3_stream_bytes([v, v, v]);
126+
check_ext3_stream_bytes([v, 0, 1]);
127+
check_ext3_stream_bytes_gpu_kernel_contract([v, v, v]);
128+
check_ext3_stream_bytes_gpu_kernel_contract([v, 0, 1]);
129+
}
130+
for bytes in [vec![], vec![0u8], vec![1, 2, 3, 4, 5], vec![0xff; 64]] {
131+
check_default_stream_bytes_impl(bytes);
132+
}
133+
}
134+
135+
proptest! {
136+
#![proptest_config(ProptestConfig::with_cases(1024))]
137+
138+
#[test]
139+
fn goldilocks_stream_bytes_matches_as_bytes_and_to_bytes_be(v in any::<u64>()) {
140+
check_goldilocks_stream_bytes(v);
141+
}
142+
143+
#[test]
144+
fn ext3_stream_bytes_matches_as_bytes_and_to_bytes_be(a in any::<u64>(), b in any::<u64>(), c in any::<u64>()) {
145+
check_ext3_stream_bytes([a, b, c]);
146+
}
147+
148+
#[test]
149+
fn ext3_stream_bytes_matches_gpu_kernel_contract(a in any::<u64>(), b in any::<u64>(), c in any::<u64>()) {
150+
check_ext3_stream_bytes_gpu_kernel_contract([a, b, c]);
151+
}
152+
153+
// Keccak absorption means `update(a); update(b)` == `update(a || b)`, so a
154+
// digest can only move if the concatenated stream moves. Pins the multi-element
155+
// hash paths (`hash_data`, `hash_data_from_slices`) against the old
156+
// `as_bytes`-per-element input.
157+
#[test]
158+
fn concatenated_stream_matches_concatenated_as_bytes(
159+
triples in vec((any::<u64>(), any::<u64>(), any::<u64>()), 0..64)
160+
) {
161+
let elements: Vec<Fp3> = triples.into_iter().map(|(a, b, c)| fp3([a, b, c])).collect();
162+
163+
let mut via_as_bytes = Vec::new();
164+
let mut via_stream = Vec::new();
165+
for e in &elements {
166+
via_as_bytes.extend_from_slice(&e.as_bytes());
167+
e.stream_bytes(&mut |b| via_stream.extend_from_slice(b));
168+
}
169+
170+
prop_assert_eq!(via_as_bytes, via_stream, "concatenated hasher input stream changed");
171+
}
172+
173+
#[test]
174+
fn default_stream_bytes_impl_matches_as_bytes(bytes in vec(any::<u8>(), 0..64)) {
175+
check_default_stream_bytes_impl(bytes);
176+
}
177+
}

0 commit comments

Comments
 (0)