fix: close rache 0.2 release blockers - #10
Merged
Conversation
An AsRef implementation may expose slices of different lengths across calls, so constructor-only validation left unchecked stripe loads relying on stale metadata. Retain the validated length, borrow the secret once per operation, and reject length changes before any unsafe access.
The inherent write aliases on the 128-bit states won method resolution over std::io::Write::write, so callers could not use the advertised trait method with normal syntax. Keep update as the raw streaming API and document the 0.1-to-0.2 migration.
The release guide still prescribed manual publish and tag commands, bypassing the signed-tag automation added to the workspace. Make the dry-run boundary and irreversible execute step explicit, and leave GitHub Release creation as a separate post-publish action.
A release-profile Miri run can catch unsafe precondition failures that normal tests and debug assertions miss. Limit it to the dependency-free library tests without default features so pull requests gain scalar and SSE2 coverage without interpreting the much larger reference-comparison suites.
tisonkun
marked this pull request as ready for review
September 2, 2026 07:31
Mentioning the Write implementation without its data flow left users to infer whether hashes consumed bytes or emitted digests. Describe update as the direct API, explain the standard-I/O adapter's behavior and boundary, and demonstrate how to finish a streamed hash.
The io::copy example is only valid when the std feature is enabled, but doctests are also compiled by the no-default-features workflow. Hide a cfg guard in the rendered example so std builds still exercise it without breaking no_std validation.
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.
Summary
writealiases that shadowstd::io::Write::writeon the two 128-bit streaming states, and document both directupdateinput and thestd::io::Writeadapter from the caller's perspective.Design Notes
XXH3 custom-secret safety
Safe Rust does not require an
AsRef<[u8]>implementation to return the same slice length on every call. The previous streaming implementation validated one borrow at construction, derived block metadata from it, and could later use a shorter borrow in unchecked 64-byte stripe loads.The fix makes the following scoped changes:
secret_lenin the accumulator metadata slot that previously held the derivedstripes_per_block; this does not growXxh3orXxh3_128states.stripes_per_blockfrom the retained length inside the accumulator. The resulting schedule is unchanged, while the original safety premise remains available for later checks and reset.Xxh3SecretBuilder, validate it beforeBuildHasher::build_hasher, and avoid calling potentially statefulAsReffromDebug.AsRefbacked byCellthat changes from a valid 136-byte secret to an empty slice. Update, digest, and builder construction must now panic before unsafe access.Short seeded-with-secret inputs still use the default secret and therefore do not touch custom storage unnecessarily. Ordinary one-shot slice APIs do not pay the new dynamic check.
Streaming documentation
The documentation now leads with the user action: use
updatefor byte slices, or use a streaming state as the destination ofstd::io::copyand otherstd::io::Writeproducers when the defaultstdfeature is enabled. It explicitly states that written bytes become hash input, the entire buffer is accepted,flushis a no-op, and the caller obtains the digest separately after the producer finishes. Rustdoc uses intra-doc links tostd::io,std::io::copy, andstd::io::Write.Miri scope
The Miri job deliberately excludes the reference-heavy integration suites. On Linux x86_64, the selected command covers the scalar implementation and available SSE2 kernel in about 35 seconds of interpretation locally. Release profile is intentional because debug assertions can stop execution before Miri observes an unsafe precondition violation.
Performance
I compared
origin/mainand this branch in a detached worktree on the same machine, using optimized Divan builds and both execution orders.origin/mainmedianThe deltas reverse or disappear across repeated pairs and are within normal run-to-run noise; there is no repeatable throughput regression. The custom-storage check occurs once per streaming
updateor finalization rather than once per stripe.Validation
cargo x lintcargo x checkcargo x testRUSTUP_TOOLCHAIN=1.85.0 cargo x testcargo x bench --no-runMIRIFLAGS='-Zmiri-strict-provenance' cargo +nightly miri test --package rache --lib --no-default-features --target x86_64-unknown-linux-gnu --releasecargo +stable package -p rache --locked --listcargo release 0.2.0 --package rache