Skip to content

Commit 68a120a

Browse files
authored
perf(syscalls,crypto): in-place keccak sponge + direct finalize + fixed-shape parents (−40% recursion-verifier cycles at real query counts) (#847)
* perf(syscalls): absorb keccak sponge input in place The state is the only buffer: input bytes XOR directly into the rate lanes at a running offset (tiny-keccak's design), padding lands in place, and the digest is read straight out of the state. This removes the staging buffer and its zeroing, the full-block copy, the 17-lane byte re-extraction, and the separate pad block — the largest slice of the ~1,322 cycles of software plumbing measured per 1-cycle keccak_permute ecall (measured for this commit alone: −1.545B cycles on the 219-query ethrex-block recursion benchmark; the residual is addressed by the follow-up commits). The whole-lane fast path gates on the input pointer's runtime alignment (the VM traps on unaligned doubleword loads); misaligned input falls back to byte-wise absorption, so correctness never depends on alignment. Digests are byte-identical to the previous implementation. * perf(crypto): finalize keccak Merkle hashes straight into the node array The field-element Merkle backends finalized every leaf and parent hash through the `Digest::finalize` blanket: it allocates a zeroed `GenericArray`, the riscv64 keccak adapter fills a local `[u8; 32]` and copies it into that `Output`, then the caller copies the `Output` once more into its own node array. That is two 32-byte memcpys plus a 32-byte memset of pure plumbing around the single permutation each hash actually costs. Route the leaf (`hash_data`, `hash_data_from_slices`, `hash_bytes`) and parent (`hash_new_parent`) hashes of `FieldElementPairBackend` and `FieldElementVectorBackend` through a shared `hash_streamed` helper. On the riscv64 guest, when `D` is the platform keccak digest and nodes are 32 bytes, it drives the syscall sponge directly and squeezes straight into the result array — no GenericArray, no intermediate buffer, no double copy. Every other digest / node size and the entire host build keep the generic `Digest` path, so output stays byte-identical (blobs still verify). Recursion verifier guest cycles: min 42,185,756 -> 41,991,672 (-0.46%), blowup8 291,147,000 -> 273,789,625 (-5.96%); keccak permutation counts unchanged (3,025 / 134,173). * perf(crypto,syscalls): fixed-shape keccak256_pair for Merkle parents Every Merkle parent hash is exactly 64 bytes (two concatenated 32-byte nodes), which fits the keccak rate in a single block. Add `keccak256_pair` to the syscall sponge: it loads the eight data lanes straight from the two nodes, XORs the pad10*1 bits in place, runs one permutation, and squeezes four lanes — skipping the incremental sponge's per-byte absorb, running-offset bookkeeping, and separate padding pass. Route `hash_new_parent` of both keccak field-element backends to it on the riscv64 guest via the same TypeId + node-size dispatch used for the direct finalize; every other digest / node size and the host build keep the generic streaming path, so output stays byte-identical (blobs verify, keccak counts unchanged at 3,025 / 134,173). Cumulative recursion verifier guest cycles after this commit: min 41,973,461, blowup8 271,979,593. C's isolated effect vs the prior commit: min -266,369, blowup8 -14,646,310. (The prior commit is a measured regression DROP-candidate; A and C together, with B dropped, are the recommended stack.) * test(syscalls): differential-test the keccak sponge against sha3 on host The sponge's absorption chunking, padding, and squeezing had no unit coverage — its only oracle was end-to-end proof-blob acceptance. Host tests now inject a software Keccak-f[1600] in place of the ecall and check digest byte-identity against sha3::Keccak256: every input length through three rate blocks (all padding boundaries), 300 randomized misaligned-chunking cases, and the fixed-shape pair path against both the reference and the streaming sponge. The guest global allocator registration is now gated to riscv64 so a host `cargo test` of this crate uses the system allocator instead of aborting on the never-initialized guest heap. Guest builds unchanged (cycle counts bit-identical). * test(syscalls): wire keccak differential tests into make; drop unused critical-section dev-dep The syscalls crate is excluded from the workspace (riscv-only bare-metal allocator/entrypoints that assemble only for the guest target), so the root `cargo test` never reached its host keccak-vs-sha3 differential tests and CI couldn't run them. Add a `test-syscalls` target that runs `cargo test` in the crate dir and make `test` depend on it. Drop the `critical-section` dev-dependency: the embedded_alloc global allocator is gated to `target_arch = "riscv64"` (src/allocator.rs), so on host test builds it is never the active allocator and its critical-section path is never linked. A clean `cargo test` links and passes without it. critical-section stays in the lock transitively (via riscv/embedded-alloc) but needs no host impl. Commit syscalls/Cargo.lock (now that CI builds this crate standalone) to match the repo convention for excluded crates and pin dev-deps for reproducible tests. * 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). * docs(crypto,syscalls): record measured dead-end optimizations at their code sites Two refactors that reviewers (and optimization-hunting agents) will keep re-proposing were implemented and measured slower on the guest; pin the numbers where the edit would happen so the effort isn't repeated: - replacing the TypeId dispatch in hash_streamed with a generic Digest::finalize_into route: +0.5% guest cycles at blowup8 across every formulation (by-value sponge through the trait layer, not elidable cross-crate without LTO) - return-value squeeze32: +81k cycles from the extra stack temporary The misaligned lane-assembly absorb path (+4.7%) is already documented at the byte fallback in update(). * fix(ci,test,docs): close the review gaps on the keccak sponge PR - CI never invokes 'make test', so the syscalls differential tests still did not run in CI despite the Makefile wiring; run 'make test-syscalls' as a dedicated step in the cli-test job. - Swap the test RNG from StdRng (algorithm unstable across rand releases) to ChaCha8Rng so the fixed seeds pin the exact fuzz case streams across versions and checkouts. - Correct the finalize_into dead-end comment to give both preset percentages (+0.14% min / +0.48% blowup8) instead of one figure. - Cite PR #847 instead of a commit hash unreachable from any ref for the dropped lane-assembly measurement. * fix(syscalls): gate the guest entrypoint module to riscv64 The _start entry symbol (and its imported main) only exist for the guest; on a Linux host they collide with the C runtime / test harness entry and 'cargo test' fails with "entry symbol `main` declared multiple times" (macOS tolerates the duplicate, which is why local host tests passed). Same treatment as the global-allocator gating: guest builds are unchanged. * test(syscalls),docs(crypto): make absorb-path coverage structural; pin the adapter passthrough invariant Review feedback (two reviewers independently): the whole-lane fast path was only exercised because Vec<u8> bases happen to be 8-aligned on current platforms. A repr(align(8)) buffer now guarantees the aligned path and a +1-offset view guarantees the byte fallback, across all padding boundaries. Also documents two load-bearing implicit facts: the TypeId specializations depend on PlatformKeccak256 remaining a pure passthrough of the syscall sponge (INVARIANT note in the adapter), and the host tests' coverage boundary (the ecall and riscv64 branches are validated only by the proof-blob oracle).
1 parent 3be1eed commit 68a120a

9 files changed

Lines changed: 814 additions & 82 deletions

File tree

.github/workflows/pr_main.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,9 @@ jobs:
131131
- name: Run CLI tests
132132
run: cargo test -p cli
133133

134+
- name: Run syscalls host tests (keccak differential vs sha3)
135+
run: make test-syscalls
136+
134137
# "Test" is a required check — keep this name to avoid branch protection changes.
135138
# This gate job passes only when CLI, executor, disk-spill, and prover tests succeed.
136139
test:

Makefile

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
.PHONY: deps deps-linux deps-macos compile-programs-asm compile-programs-rust compile-bench \
22
compile-programs compile-recursion-elfs clean-asm clean-rust clean-bench clean-shared \
33
clean-recursion-elfs clean test test-asm \
4-
test-rust test-ethrex test-executor test-flamegraph flamegraph-prover test-profile-recursion test-profile-recursion-single test-profile-recursion-multi \
4+
test-rust test-ethrex test-executor test-syscalls test-flamegraph flamegraph-prover test-profile-recursion test-profile-recursion-single test-profile-recursion-multi \
55
test-fast test-prover test-prover-all test-prover-debug test-disk-spill test-math-cuda test-cuda-integration test-cuda-fallback \
66
test-prover-cuda test-prover-comprehensive-cuda \
77
bench-math-cuda bench-prover bench-prover-cuda build check clippy fmt lint regen-ethrex-fixtures \
@@ -293,7 +293,16 @@ update-ethrex-fixture-checksums:
293293
check-ethrex-fixture-checksums:
294294
python3 tooling/ethrex-fixtures/update_readme_checksums.py --check
295295

296-
test: compile-programs
296+
# The syscalls crate is excluded from the workspace (riscv-only bare-metal
297+
# entrypoints/allocator that assemble only for the guest target — see the root
298+
# Cargo.toml exclude), so the root `cargo test` never reaches its host
299+
# differential tests (the keccak sponge vs sha3 reference). Run them explicitly
300+
# in the crate dir; wired into `test` below and run as a dedicated step
301+
# in CI's cli-test job (pr_main.yaml).
302+
test-syscalls:
303+
cd syscalls && cargo test
304+
305+
test: compile-programs test-syscalls
297306
cargo test
298307

299308
# === Quick test shortcuts ===

crypto/crypto/src/hash/platform_keccak.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,15 @@ mod imp {
1111
};
1212
use lambda_vm_syscalls::keccak::Keccak256 as SyscallKeccak256;
1313

14+
// INVARIANT (load-bearing): this adapter must remain a PURE PASSTHROUGH of
15+
// `SyscallKeccak256`. The TypeId specializations in
16+
// crypto/crypto/src/merkle_tree/backends/field_element_vector.rs bypass it
17+
// and drive the syscall sponge directly, on the assumption that both paths
18+
// hash identically. Adding ANY behavior here (a domain prefix, extra
19+
// absorption, a different reset policy) silently desyncs the specialized
20+
// branches from the generic path — and the failure surfaces as in-guest
21+
// proof rejection, not as a host test failure.
22+
1423
#[derive(Clone, Default)]
1524
pub struct PlatformKeccak256(SyscallKeccak256);
1625

crypto/crypto/src/merkle_tree/backends/field_element_vector.rs

Lines changed: 98 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,88 @@ use math::{
99
traits::AsBytes,
1010
};
1111

12+
#[cfg(target_arch = "riscv64")]
13+
use crate::hash::platform_keccak::PlatformKeccak256;
14+
#[cfg(target_arch = "riscv64")]
15+
use core::any::TypeId;
16+
#[cfg(target_arch = "riscv64")]
17+
use lambda_vm_syscalls::keccak::Keccak256 as SyscallKeccak256;
18+
19+
/// Absorb `feed`'s byte stream into a fresh `D` and return the digest as a
20+
/// fixed `[u8; NUM_BYTES]`.
21+
///
22+
/// On the riscv64 guest, when `D` is the platform keccak digest and the node
23+
/// is 32 bytes, this drives the syscall sponge directly and squeezes straight
24+
/// into the result array. That skips the `Digest::finalize` blanket, which
25+
/// allocates a zeroed `GenericArray`, has the adapter fill a local `[u8; 32]`
26+
/// and copy it into that `Output`, then leaves the caller to copy the `Output`
27+
/// once more into its own array — two 32-byte memcpys plus a 32-byte memset of
28+
/// pure plumbing around the one permutation. Byte-identical to the generic
29+
/// path; every other digest / node size (and the entire host build) takes the
30+
/// generic path unchanged.
31+
///
32+
/// DO NOT replace this `TypeId` dispatch with a generic `Digest::finalize_into`
33+
/// fix "at the adapter altitude" — that exact refactor was implemented and
34+
/// MEASURED SLOWER on the guest across every formulation tried (best:
35+
/// +60k min = +0.14%, +1.25M blowup8 = +0.48%), including `#[inline]`
36+
/// adapters and a check-free `AsMut` output conversion. The residual is
37+
/// intrinsic: `FixedOutput::finalize_into` moves the 208-byte sponge by value
38+
/// through the newtype + trait layer into a non-inlined cross-crate call, and
39+
/// without LTO the placement isn't elided; the direct branch below builds the
40+
/// sponge in place at the call's ABI slot. Deleting the dispatch also cannot
41+
/// remove the `'static` bounds — `hash_new_parent_bytes` needs them regardless.
42+
#[inline]
43+
fn hash_streamed<D: Digest + 'static, const NUM_BYTES: usize>(
44+
feed: impl Fn(&mut dyn FnMut(&[u8])),
45+
) -> [u8; NUM_BYTES] {
46+
#[cfg(target_arch = "riscv64")]
47+
if NUM_BYTES == 32 && TypeId::of::<D>() == TypeId::of::<PlatformKeccak256>() {
48+
let mut hasher = SyscallKeccak256::new();
49+
feed(&mut |bytes| hasher.update(bytes));
50+
let mut result = [0u8; NUM_BYTES];
51+
// NUM_BYTES == 32 in this branch, so the slice is exactly a [u8; 32].
52+
let out: &mut [u8; 32] = (&mut result[..]).try_into().unwrap();
53+
hasher.finalize(out);
54+
return result;
55+
}
56+
57+
let mut hasher = D::new();
58+
feed(&mut |bytes| hasher.update(bytes));
59+
let mut result_hash = [0_u8; NUM_BYTES];
60+
result_hash.copy_from_slice(&hasher.finalize());
61+
result_hash
62+
}
63+
64+
/// Hash a Merkle parent — always exactly two concatenated 32-byte nodes.
65+
///
66+
/// On the riscv64 guest, when `D` is the platform keccak digest and nodes are
67+
/// 32 bytes, this is one fixed-shape 64-byte compression ([`keccak256_pair`]):
68+
/// a single permutation with the input lanes and padding written straight into
69+
/// the state, skipping the incremental sponge's per-byte absorb, running
70+
/// offset, and separate padding pass. Byte-identical to streaming both nodes
71+
/// through the digest; every other digest / node size (and the host build)
72+
/// takes the generic streaming-and-finalize path unchanged.
73+
#[inline]
74+
fn hash_new_parent_bytes<D: Digest + 'static, const NUM_BYTES: usize>(
75+
left: &[u8; NUM_BYTES],
76+
right: &[u8; NUM_BYTES],
77+
) -> [u8; NUM_BYTES] {
78+
#[cfg(target_arch = "riscv64")]
79+
if NUM_BYTES == 32 && TypeId::of::<D>() == TypeId::of::<PlatformKeccak256>() {
80+
let l: &[u8; 32] = left[..].try_into().unwrap();
81+
let r: &[u8; 32] = right[..].try_into().unwrap();
82+
let hash = lambda_vm_syscalls::keccak::keccak256_pair(l, r);
83+
let mut result = [0u8; NUM_BYTES];
84+
result.copy_from_slice(&hash);
85+
return result;
86+
}
87+
88+
hash_streamed::<D, NUM_BYTES>(|sink| {
89+
sink(left);
90+
sink(right);
91+
})
92+
}
93+
1294
/// A backend for Merkle trees that uses fixed-size pairs of field elements.
1395
/// This is more efficient than `FieldElementVectorBackend` when the batch size is always 2,
1496
/// as it avoids Vec allocation overhead.
@@ -27,7 +109,7 @@ impl<F, D: Digest, const NUM_BYTES: usize> Default for FieldElementPairBackend<F
27109
}
28110
}
29111

30-
impl<F, D: Digest, const NUM_BYTES: usize> IsMerkleTreeBackend
112+
impl<F, D: Digest + 'static, const NUM_BYTES: usize> IsMerkleTreeBackend
31113
for FieldElementPairBackend<F, D, NUM_BYTES>
32114
where
33115
F: IsField,
@@ -38,21 +120,14 @@ where
38120
type Data = [FieldElement<F>; 2];
39121

40122
fn hash_data(input: &[FieldElement<F>; 2]) -> [u8; NUM_BYTES] {
41-
let mut hasher = D::new();
42-
input[0].stream_bytes(&mut |b| hasher.update(b));
43-
input[1].stream_bytes(&mut |b| hasher.update(b));
44-
let mut result_hash = [0_u8; NUM_BYTES];
45-
result_hash.copy_from_slice(&hasher.finalize());
46-
result_hash
123+
hash_streamed::<D, NUM_BYTES>(|sink| {
124+
input[0].stream_bytes(sink);
125+
input[1].stream_bytes(sink);
126+
})
47127
}
48128

49129
fn hash_new_parent(left: &[u8; NUM_BYTES], right: &[u8; NUM_BYTES]) -> [u8; NUM_BYTES] {
50-
let mut hasher = D::new();
51-
hasher.update(left);
52-
hasher.update(right);
53-
let mut result_hash = [0_u8; NUM_BYTES];
54-
result_hash.copy_from_slice(&hasher.finalize());
55-
result_hash
130+
hash_new_parent_bytes::<D, NUM_BYTES>(left, right)
56131
}
57132
}
58133

@@ -71,7 +146,7 @@ impl<F, D: Digest, const NUM_BYTES: usize> Default for FieldElementVectorBackend
71146
}
72147
}
73148

74-
impl<F, D: Digest, const NUM_BYTES: usize> FieldElementVectorBackend<F, D, NUM_BYTES>
149+
impl<F, D: Digest + 'static, const NUM_BYTES: usize> FieldElementVectorBackend<F, D, NUM_BYTES>
75150
where
76151
[u8; NUM_BYTES]: From<Output<D>>,
77152
{
@@ -80,15 +155,11 @@ where
80155
/// once, avoiding per-element allocations while staying consistent with the
81156
/// backend's hash function.
82157
pub fn hash_bytes(data: &[u8]) -> [u8; NUM_BYTES] {
83-
let mut hasher = D::new();
84-
hasher.update(data);
85-
let mut result = [0u8; NUM_BYTES];
86-
result.copy_from_slice(&hasher.finalize());
87-
result
158+
hash_streamed::<D, NUM_BYTES>(|sink| sink(data))
88159
}
89160
}
90161

91-
impl<F, D: Digest, const NUM_BYTES: usize> FieldElementVectorBackend<F, D, NUM_BYTES>
162+
impl<F, D: Digest + 'static, const NUM_BYTES: usize> FieldElementVectorBackend<F, D, NUM_BYTES>
92163
where
93164
F: IsField,
94165
FieldElement<F>: AsBytes,
@@ -100,17 +171,15 @@ where
100171
/// `hash_data(&[a, b].concat())`: the sponge absorbs the same element bytes
101172
/// in the same order, just without the intermediate `Vec`.
102173
pub fn hash_data_from_slices(a: &[FieldElement<F>], b: &[FieldElement<F>]) -> [u8; NUM_BYTES] {
103-
let mut hasher = D::new();
104-
for element in a.iter().chain(b.iter()) {
105-
element.stream_bytes(&mut |bytes| hasher.update(bytes));
106-
}
107-
let mut result_hash = [0_u8; NUM_BYTES];
108-
result_hash.copy_from_slice(&hasher.finalize());
109-
result_hash
174+
hash_streamed::<D, NUM_BYTES>(|sink| {
175+
for element in a.iter().chain(b.iter()) {
176+
element.stream_bytes(sink);
177+
}
178+
})
110179
}
111180
}
112181

113-
impl<F, D: Digest, const NUM_BYTES: usize> IsMerkleTreeBackend
182+
impl<F, D: Digest + 'static, const NUM_BYTES: usize> IsMerkleTreeBackend
114183
for FieldElementVectorBackend<F, D, NUM_BYTES>
115184
where
116185
F: IsField,
@@ -129,12 +198,7 @@ where
129198
}
130199

131200
fn hash_new_parent(left: &[u8; NUM_BYTES], right: &[u8; NUM_BYTES]) -> [u8; NUM_BYTES] {
132-
let mut hasher = D::new();
133-
hasher.update(left);
134-
hasher.update(right);
135-
let mut result_hash = [0_u8; NUM_BYTES];
136-
result_hash.copy_from_slice(&hasher.finalize());
137-
result_hash
201+
hash_new_parent_bytes::<D, NUM_BYTES>(left, right)
138202
}
139203
}
140204

0 commit comments

Comments
 (0)