fix(hasher): make the deterministic hasher endian-independent#142
Merged
Conversation
The vendored AHash fallback reinterpreted byte slices in native endianness (zerocopy::transmute), so the consensus-critical bucket_id hash would assign the same key to different buckets on big-endian targets. Read u16/u32/u64/u128 with explicit from_le_bytes instead: bit-identical on every current (little-endian) target - the pinned hash-value tests prove the network format is unchanged - and deterministic everywhere else. Beyond these four slice reads, the Convert trait and its ~40 transmute impls were dead vendored code, and zerocopy was salt's only use of the crate, so both are removed. Closes #127. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Performance Benchmark ComparisonCompared Detailed Comparison
|
This was referenced Jul 8, 2026
Mutation testing - PASSMutation score: 100.0% (36/36 viable mutants killed)
No unsuppressed survivors or timeouts remain. |
Troublor
approved these changes
Jul 9, 2026
flyq
approved these changes
Jul 10, 2026
flyq
left a comment
Member
There was a problem hiding this comment.
Stale doc line. salt/src/state/ahash/mod.rs:11 still reads "Convert utilities extracted from AHash for efficient byte operations" — the Convert trait is gone. Suggest something like "Little-endian byte-slice readers for the deterministic hasher." (Keeping the convert.rs filename itself is fine — it matches the upstream AHash file and aids diffing against it.)
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
zerocopy::transmute!, i.e. in native endianness, sobucket_id— a consensus-critical hash — would map the same key to different buckets on big-endian targets.ReadFromSlicenow uses explicitu{16,32,64,128}::from_le_bytes. Little-endian is the canonical interpretation: every current deployment target (x86_64 sequencer, riscv64 fault-proof/zkVM clients, wasm32) is little-endian, so this is bit-identical on all of them.fallback.rsis pure integer arithmetic,hash_with_noncealready usesto_le_bytesfor the nonce, andwrite_usizewidens by value (also safe on 32-bit wasm).Converttrait with its ~40transmute!impls was dead vendored code beyond these four slice reads, and this was salt's only use ofzerocopy, so both are removed (Cargo.lock updated accordingly).Why this is consensus-safe to merge
The mutation-testing PR (#140) pinned the exact outputs of
hash(),hash_with_nonce(),bucket_id(), andread_small()as unit tests. Those tests passing in CI proves the network format is byte-for-byte unchanged on little-endian; big-endian targets now compute the same canonical values instead of diverging.🤖 Generated with Claude Code