perf: zero-heap-allocation Poseidon hash path - #64
Draft
ananas-block wants to merge 8 commits into
Draft
ananas-block wants to merge 8 commits into
ananas-block wants to merge 8 commits into
Conversation
Captures 144 vectors from the current implementation covering widths 2..=13 across the field API and both byte endiannesses: all-zero, all-one, sequential and p-1 inputs plus eight deterministic pseudo-random cases per width. This crate backs the sol_poseidon syscall, so these outputs are consensus-affecting. The vectors exist to prove the upcoming zero-allocation rewrite changes no hash output. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three allocation sources are removed: - PoseidonParameters held ark as Vec<F> and mds as Vec<Vec<F>>, so every hasher construction allocated width + 2 times. Both are now &'static slices with mds flattened row-major, and the generated constants are static arrays built with the const fn Fr::new, which performs the Montgomery reduction at compile time instead of on every call. - apply_mds rebuilt the state with collect() once per round, allocating 64 times per width-2 hash and 73 times per width-13 hash. The permutation now runs over a fixed-size stack array. - The byte path built a num_bigint::BigUint per input and a Vec<u8> for the output. Values are assembled directly into the field's limb array and serialized into a fixed-size array. Poseidon no longer carries a state field; the permutation state lives on the stack for the duration of a hash. Byte inputs convert straight into that state, so there is no intermediate input buffer that could overflow. Poseidon::new is now fallible and rejects a width outside 2..=MAX_X5_LEN up front, which is what makes the fixed-capacity state provably safe. PoseidonParameters gains new_unchecked, used by the generated parameter sets whose dimensions are fixed at generation time, alongside a validating new for caller-supplied parameters. new_circom, hash_bytes_be and hash_bytes_le keep their signatures, so solana-poseidon and Agave need no coordinated change. Hash outputs are unchanged: all 144 frozen vectors pass across widths 2..=13 for the field API and both endiannesses, and the 30 existing integration tests pass unmodified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds boundary coverage for the paths that a fixed-capacity stack state makes reachable: widths 0, 1, 2, 13, 14 and beyond through both constructors, input counts of 0, width-1, width, width+1, 12, 13, 14, 32 and 200 through the field and both byte APIs, and a valid call following a rejected one to show no state leaks between calls. Also covers parameter dimension validation, including a truncated MDS reaching the permutation through new_unchecked, which returns an error rather than panicking or hashing with missing terms. The allocation test asserts zero heap allocations for construction plus hashing at every supported width, across the field API and both byte endiannesses, using a counting global allocator. The Criterion benchmark previously exercised only PoseidonHasher::hash on field elements, leaving the byte parser and serializer unmeasured. It now also covers hash_bytes_be/le and the shape solana_poseidon actually uses, where a hasher is constructed on every call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The byte path was the only user of num_bigint::BigUint. Values are now assembled directly into the field's limb array, so nothing in the crate references it. Also applies rustfmt and fixes a clippy manual_is_multiple_of lint in the differential test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
solana-poseidon matches PoseidonError exhaustively when converting it to PoseidonSyscallError, so adding variants breaks its compilation even though every function signature is unchanged. Verified by building solana-poseidon 5.0.0 against this crate with a path patch. The two new failure modes, an out-of-range width and a parameter set whose dimensions do not match its declared width, now reuse InvalidWidthCircom instead of introducing InvalidWidth and InvalidParameterDimensions. Its message is generalised to cover both. With this, solana-poseidon compiles unedited, the wrapper agrees with the library for both endiannesses, input counts 1..=12 succeed, 13 is rejected with InvalidWidthCircom, and a value at the modulus is still rejected with InputLargerThanModulus. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI runs stable 1.98, which added chunks_exact_to_as_chunks; the local toolchain here is 1.95 and does not have it, so this was not caught before pushing. Replaces the three constant-size chunk iterations with as_chunks::<8>() and as_chunks_mut::<8>(), which also lets the limb conversions work on [u8; 8] directly instead of going through try_into or copy_from_slice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ananas-block
marked this pull request as draft
September 22, 2026 15:05
The benchmark inputs zeroed byte 0 to keep the value below the modulus. That is the most significant byte only under big-endian interpretation. Read little-endian, byte 31 is most significant and stayed random, and the BN254 modulus starts at 0x30, so roughly 81% of seeds exceeded it. The closure discarded the Result, so hash_bytes_le returned InputLargerThanModulus immediately and criterion reported that as the measurement: 12.6 ns against 90.9 us for big-endian at width 10, flat across every width. The little-endian benchmark had never hashed anything. Inputs now zero both ends, so the value is valid whichever way it is read, and every benchmark unwraps its result so an error path cannot masquerade as a fast hash again. Little-endian now measures 92.9 us against 93.2 us for big-endian at width 10. Also from review: - permute validates ark and mds dimensions once up front, letting the loop body iterate chunks_exact directly instead of doing index arithmetic per row. The check is load-bearing rather than defensive: chunks_exact and zip both stop at the shorter side, so without it a truncated mds would hash successfully with missing terms instead of being rejected. A width of 0 is rejected too, since chunks_exact(0) panics. - bigint_to_hash_bytes_be/le collapse into a macro, matching how the file already handles the other endianness pair. - The differential test uses the hex dev-dependency instead of hand-rolled conversion, and its repeat-hashing case now covers one vector per width rather than twelve vectors of width 2. - The truncated-MDS test asserts the error payload, not just the variant, since the dimension and out-of-range-width failures share InvalidWidthCircom and only the width distinguishes them. - The parameter generator asserts a 32-byte constant rather than silently leaving high limbs zero, which would emit a wrong constant. - Unused PartialEq and Eq derives dropped; Copy is load-bearing. - .gitattributes marks the generated parameter file linguist-generated so it collapses in review. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three findings from review: - as_chunks/as_chunks_mut, adopted to satisfy a newer clippy lint, are stable only since 1.88.0. The crate declared no rust-version, and CI installs stable, so nothing caught it: a downstream on a pinned older toolchain would get "no method named as_chunks_mut" with no hint that it is an MSRV problem. Now declared. - with_domain_tag_circom computed `nr_inputs + 1` unchecked, so new_circom(usize::MAX) overflowed. Under overflow-checks = true, which Agave's release profile sets, that aborts instead of returning an error, contradicting the panic-freedom this crate otherwise provides. The add is now checked and usize::MAX is covered by a test, verified with overflow checks forced on. The overflow predates this branch. - Fixes a broken intra-doc link to PoseidonError::InvalidWidth, a variant that no longer exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removes every heap allocation from the Poseidon hash path and cuts per-call cost. Hash outputs are unchanged.
Why
light-poseidon backs the
sol_poseidonsyscall throughsolana-poseidon, and compiles natively into the validator rather than into BPF programs (its dependency is gated behindcfg(not(any(target_os = "solana", target_arch = "bpf")))).solana_poseidon::hashvconstructs a fresh hasher on every call, so parameter construction sits on the hot path alongside hashing.Three sources of allocation, measured with a counting global allocator:
PoseidonParametersasVec<F>+Vec<Vec<F>>, rebuilt per constructionapply_mdsrebuilding state withcollect()once per roundBigUintper input, aVec<u8>for the outputResults
Interleaved A/B against
origin/main, three runs each, release, native arm64, for the shapehashvuses (construct, thenhash_bytes_be):The gain is largest at small widths, where fixed per-call overhead dominates. At width 13 the O(width^2) MDS multiply dominates and is untouched here; that is what the sparse-MDS follow-up targets.
How
&'staticand flat.arkandmdsare&'static [F], withmdsflattened row-major. The generated constants arestaticarrays built withFr::new, aconst fnthat performs the Montgomery reduction at compile time, whereFrom<BigInt>calledfrom_bigintat run time on every construction.Poseidonno longer holds a state field at all; state lives in a fixed-size stack array for the duration of a hash.Poseidon::newis now fallible and rejects a width outside2..=MAX_X5_LENup front, which is what makes the fixed-capacity state provably safe.PoseidonParameters::new_uncheckedskips dimension validation for the generated sets, whose dimensions are fixed at generation time;PoseidonParameters::newvalidates, for parameters from anywhere else.Compatibility with Agave
solana-poseidon5.0.0 compiles against this branch unedited, verified by building and running it with a path patch, not just by comparing signatures. The wrapper agrees with the library for both endiannesses, input counts 1..=12 succeed, 13 is rejected withInvalidWidthCircom, and a value at the modulus is still rejected withInputLargerThanModulus.Two things this constrained:
Poseidoncould not become const-generic over width, becausenew_circomtakes a runtime length. A const-generic version measured meaningfully faster in a prototype (4.70 us at width 2, about -39%) because the inner loops unroll. Kept out to avoid forcing a dispatch change on callers.No new
PoseidonErrorvariants.solana-poseidon-5.0.0/src/lib.rs:227-249matches the enum exhaustively with no wildcard arm when converting toPoseidonSyscallError, so adding a variant is an E0004 compile error there even with every signature unchanged. (Agave's syscall handler does not match on the variant, but thisFromimpl in the crate does, and it is what breaks.) The out-of-range width and inconsistent-dimensions cases therefore reuseInvalidWidthCircom, whose message is generalised. Caught by the downstream build, not by the test suite.The distinction from the other breaking changes above is deliberate rather than inconsistent:
PoseidonParameters,Poseidon::newandget_poseidon_parametersare not used bysolana-poseidonat all, so breaking them is invisible to Agave. An error variant is not. Tests that would otherwise lose the ability to tell the two failures apart assert the error payload instead of the variant.new_circom,with_domain_tag_circom,hash_bytes_be,hash_bytes_le,validate_bytes_length,bytes_to_prime_field_element_beandbytes_to_prime_field_element_leall keep their signatures. Byte-API error precedence is unchanged: lengths are validated first, then values converted, then the input count checked. That ordering is load-bearing and is covered by the existingpadding_collisiontests, which caught an earlier version of this branch getting it wrong.Proving outputs did not change
This is consensus-relevant code, so that claim needed more than the existing tests.
light-poseidon/tests/differential.rschecks 144 vectors captured frommainbefore any change: widths 2..=13, field API and both byte endiannesses, covering all-zero, all-one, sequential andp-1inputs plus eight deterministic pseudo-random cases per width. If one fails, the fix is the code, never the fixture.The parameter file was regenerated from the existing in-crate constants rather than re-derived from the sage script, so the values are identical by construction. It then survived a full read-write round trip through the new API with the vectors still passing.
Independently confirmed from another direction: the parallel sparse-MDS work derived its constants twice, once from the pre-rewrite parameter file and once from the regenerated one, and got byte-identical output. That derivation multiplies and inverts these constants across 60-odd rounds per width, so a single differing limb anywhere in
arkormdswould have diverged.Tests
51 tests, all passing. The 30 existing integration tests are unmodified.
differential.rs(5): outputs against the frozen oracle.boundaries.rs(10): widths 0, 1, 2, 13, 14 and beyond through both constructors; input counts 0, width-1, width, width+1, 12, 13, 14, 32, 200 through the field and both byte APIs; a valid call after a rejected one; and a truncated MDS reaching the permutation, which errors rather than panicking or hashing with missing terms.allocations.rs(1): asserts zero allocations for construction plus hashing at every width across all three APIs, so the property cannot silently regress.The Criterion benchmark previously measured only
hashon field elements, leaving the byte parser and serializer unmeasured. It now also covershash_bytes_be/leand the construct-per-call syscall shape.cargo clippy --workspace --all-targets -- -D warningsis clean.Review notes
The 50k-line diff is almost entirely the generated
parameters/bn254_x5.rs(43,056 deletions, 7,237 insertions; it shrinks because each constant collapses from six lines to one). The reviewable diff is roughly 630 lines:lib.rs, the xtask generator, and the new tests. The generator is the thing to check, not its output.xtask/src/generate_parameters.rsis updated to emit the new format but could not be executed here: it needs SageMath and a clone of hadeshash, andtarget/paramsis not in the repo. It is untested by construction.num-bigintis dropped as a direct dependency, since the byte path was its only user in this crate. It is still compiled:ark-ff,ark-ec,ark-serializeandark-ff-macrosall depend on it (cargo tree -i num-bigint). This is not a build-time or dependency-tree win, only one fewer crate this code calls into directly.Two behaviour changes worth knowing
Oversized parameter tables are now rejected.
permutevalidatesark.len()andmds.len()against the declared dimensions exactly, once, before the round loop. The old code indexed rather than iterated, so an over-long table was silently tolerated. Anyone sharing one constant table across widths now gets an error instead of a hash. The bundled sets match exactly, so the syscall path is unaffected.That check is load-bearing rather than defensive:
chunks_exactandzipboth stop at the shorter side, so without it a truncatedmdswould hash successfully with missing terms — the same silent-truncation failure mode as PR #60. A width of0is rejected for the same reason, sincechunks_exact(0)panics.MSRV is now 1.88, declared in
Cargo.toml.slice::as_chunks/as_chunks_mutare stable since 1.88.0; they were adopted to satisfy a clippy lint that CI's newer toolchain raised.Out of scope
Sparse MDS for partial rounds, which is where the remaining width-13 cost lives, is a separate change and is being worked on in parallel. PRs #60 and #61 overlap this work and are deliberately left independent.
The version is deliberately not bumped. These changes are breaking (
PoseidonParametersfield types,Poseidon::newandPoseidonParameters::newnow fallible,get_poseidon_parametersspecialised toFr) and will need a bump whenever a release is cut.🤖 Generated with Claude Code