Skip to content

Commit bd408a2

Browse files
committed
refactor(syscalls): keccak review fixes — LE guard, shared squeeze, StdRng tests
- update(): gate the whole-lane fast path on cfg!(target_endian = "little"). The raw *const u64 lane read equals the required little-endian value only on LE targets; the cfg! folds to a compile-time constant (codegen unchanged on every real target) and the byte fallback is endian-correct everywhere. - Deduplicate the squeeze: extract squeeze32_into(state, out), shared by Keccak256::finalize and keccak256_pair. Write-into (rather than returning [u8; 32]) so finalize fills its output reference in place — verified guest-codegen and cycle-identical to the pre-dedup loops. - Tests: replace the hand-rolled xorshift RNG with rand's StdRng + SeedableRng (matching src/random.rs), keeping fixed seeds for reproducibility. - Document at the byte-wise fallback that a from_le_bytes lane-assembly middle path was measured at +4.7% cycles (dropped commit f6d575ed) so it is not re-proposed on this 1-cycle-per-instruction VM. Digests remain byte-identical to sha3 Keccak-256 (host differential tests); guest cycles unchanged (recursion-min 41,747,766 / recursion-blowup8 260,967,578, keccak call counts identical).
1 parent fb9d062 commit bd408a2

1 file changed

Lines changed: 38 additions & 30 deletions

File tree

‎syscalls/src/keccak.rs‎

Lines changed: 38 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,17 @@ impl Keccak256 {
9494
while !input.is_empty() {
9595
// Whole-lane fast path: sponge offset on a lane boundary AND input
9696
// pointer 8-aligned (the VM traps on unaligned doubleword loads).
97-
// Keccak state lanes are little-endian, so an in-memory u64 load
98-
// equals the from_le_bytes lane value.
99-
if self.offset % 8 == 0 && (input.as_ptr() as usize) % 8 == 0 && input.len() >= 8 {
97+
// LE-only by construction: the raw `*const u64` read below equals the
98+
// required little-endian lane value only on a little-endian target, so
99+
// gate on it. `cfg!(target_endian = ...)` is a compile-time constant
100+
// that folds away on every real target (riscv64im-lambda-vm and the
101+
// host CI targets are all little-endian), leaving codegen unchanged;
102+
// the byte-wise fallback is endian-correct everywhere.
103+
if cfg!(target_endian = "little")
104+
&& self.offset % 8 == 0
105+
&& (input.as_ptr() as usize) % 8 == 0
106+
&& input.len() >= 8
107+
{
100108
let lanes_left = (RATE_BYTES - self.offset) / 8;
101109
let take = lanes_left.min(input.len() / 8);
102110
let base = self.offset / 8;
@@ -114,6 +122,11 @@ impl Keccak256 {
114122
self.offset = 0;
115123
}
116124
} else {
125+
// Byte-wise fallback for misaligned input. A middle path that
126+
// assembled each lane with `from_le_bytes` (three formulations,
127+
// dropped commit f6d575ed) measured +4.7% cycles: on this
128+
// 1-cycle-per-instruction VM the extra shifts/ORs cost more than
129+
// the byte loop they replace, so don't re-propose it.
117130
self.absorb_byte(input[0]);
118131
input = &input[1..];
119132
}
@@ -129,10 +142,20 @@ impl Keccak256 {
129142
self.state[RATE_LANES - 1] ^= FINAL_PAD_LANE_BIT;
130143
keccak_permute(&mut self.state);
131144

132-
// Squeeze the first 32 bytes (4 u64 lanes).
133-
for (i, chunk) in output.chunks_exact_mut(8).enumerate() {
134-
chunk.copy_from_slice(&self.state[i].to_le_bytes());
135-
}
145+
squeeze32_into(&self.state, output);
146+
}
147+
}
148+
149+
/// Squeeze the 32-byte Keccak-256 digest out of a permuted state into `out`:
150+
/// rate lanes 0..4, little-endian. Shared by the streaming
151+
/// [`Keccak256::finalize`] and the fixed-shape [`keccak256_pair`] so the squeeze
152+
/// loop lives in exactly one place. Writes into a caller-owned buffer (rather
153+
/// than returning `[u8; 32]`) so `finalize` fills its `output` reference in
154+
/// place, keeping the guest codegen identical to the pre-dedup loop.
155+
#[inline(always)]
156+
fn squeeze32_into(state: &[u64; 25], out: &mut [u8; 32]) {
157+
for (i, chunk) in out.chunks_exact_mut(8).enumerate() {
158+
chunk.copy_from_slice(&state[i].to_le_bytes());
136159
}
137160
}
138161

@@ -172,9 +195,7 @@ pub fn keccak256_pair(left: &[u8; 32], right: &[u8; 32]) -> [u8; 32] {
172195
keccak_permute(&mut state);
173196

174197
let mut out = [0u8; 32];
175-
for (i, chunk) in out.chunks_exact_mut(8).enumerate() {
176-
chunk.copy_from_slice(&state[i].to_le_bytes());
177-
}
198+
squeeze32_into(&state, &mut out);
178199
out
179200
}
180201

@@ -188,26 +209,13 @@ pub fn keccak256_pair(left: &[u8; 32], right: &[u8; 32]) -> [u8; 32] {
188209
#[cfg(all(test, not(target_arch = "riscv64")))]
189210
mod tests {
190211
use super::*;
212+
use rand::{Rng, SeedableRng, rngs::StdRng};
191213
use sha3::{Digest, Keccak256 as RefKeccak256};
192214

193215
fn reference(input: &[u8]) -> [u8; 32] {
194216
RefKeccak256::digest(input).into()
195217
}
196218

197-
/// Deterministic xorshift64* so the "fuzz" cases are reproducible.
198-
struct Rng(u64);
199-
impl Rng {
200-
fn next(&mut self) -> u64 {
201-
self.0 ^= self.0 << 13;
202-
self.0 ^= self.0 >> 7;
203-
self.0 ^= self.0 << 17;
204-
self.0
205-
}
206-
fn below(&mut self, n: usize) -> usize {
207-
(self.next() % n.max(1) as u64) as usize
208-
}
209-
}
210-
211219
/// Every length from empty through three full rate blocks (+2), so every
212220
/// padding boundary (135/136/137, 271/272/273, …) is hit.
213221
#[test]
@@ -230,16 +238,16 @@ mod tests {
230238
#[test]
231239
fn chunked_misaligned_updates_match_reference() {
232240
let data: Vec<u8> = (0..1500).map(|i| (i * 131 + 17) as u8).collect();
233-
let mut rng = Rng(0x9E37_79B9_7F4A_7C15);
241+
let mut rng = StdRng::seed_from_u64(0x9E37_79B9_7F4A_7C15);
234242
for case in 0..300 {
235-
let len = rng.below(data.len());
236-
let start = rng.below(data.len() - len + 1);
243+
let len = rng.random_range(0..data.len());
244+
let start = rng.random_range(0..data.len() - len + 1);
237245
let slice = &data[start..start + len];
238246

239247
let mut hasher = Keccak256::new();
240248
let mut fed = 0;
241249
while fed < slice.len() {
242-
let n = 1 + rng.below((slice.len() - fed).min(200));
250+
let n = 1 + rng.random_range(0..(slice.len() - fed).min(200));
243251
hasher.update(&slice[fed..fed + n]);
244252
fed += n;
245253
}
@@ -256,12 +264,12 @@ mod tests {
256264
/// The fixed-shape parent path must equal hashing the 64-byte concatenation.
257265
#[test]
258266
fn pair_matches_reference() {
259-
let mut rng = Rng(0xD1B5_4A32_D192_ED03);
267+
let mut rng = StdRng::seed_from_u64(0xD1B5_4A32_D192_ED03);
260268
for case in 0..64 {
261269
let mut left = [0u8; 32];
262270
let mut right = [0u8; 32];
263271
for b in left.iter_mut().chain(right.iter_mut()) {
264-
*b = rng.next() as u8;
272+
*b = rng.random();
265273
}
266274
let mut concat = [0u8; 64];
267275
concat[..32].copy_from_slice(&left);

0 commit comments

Comments
 (0)