Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why use siphasher? #3

Closed
FilipAndersson245 opened this issue Jul 10, 2023 · 5 comments
Closed

Why use siphasher? #3

FilipAndersson245 opened this issue Jul 10, 2023 · 5 comments

Comments

@FilipAndersson245
Copy link

Hello,
I was just wondering why specificaly this library use siphash as the default hashing algorithm?
As we hash u128 I think ahash would be a good alternative as it seem to be upwards of 20x faster for those sizes

image

@laurmaedje
Copy link
Member

We need collision resistance because we assume that hash collisions never happen. I don't know about aHash, but FnvHash and FxHash are not designed for such use cases. The decision to use SipHash follows rustc's incremental compiler: rust-lang/rust#107925

@FilipAndersson245
Copy link
Author

FilipAndersson245 commented Jul 11, 2023

It seem as ahash would not be suitable then, they use a u64 hash, but their hash quality seem to be better then FnvHash

@briansmith
Copy link

We need collision resistance because we assume that hash collisions never happen. I don't know about aHash, but FnvHash and FxHash are not designed for such use cases.

SipHash128 is not a collision-resistant hash function. If you read the original SipHash whitepaper they even say that it is "obvious" that it is not.

The decision to use SipHash follows rustc's incremental compiler: rust-lang/rust#107925

See rust-lang/rust#10389 for extensive discussion.

From your source code:

comemo/src/prehashed.rs

Lines 22 to 27 in 1275982

/// Because comemo uses high-quality 128 bit hashes in all places, the risk of a
/// hash collision is reduced to an absolute minimum. Therefore, this type
/// additionally provides `PartialEq` and `Eq` implementations that compare by
/// hash instead of by value. For this to be correct, your hash implementation
/// **must feed all information relevant to the `PartialEq` impl to the
/// hasher.**

Even when used with a CSPRNG-provided random key, the original use case for SipHash was hash functions, which do a full equality check done after the hash equality check, to verify there are no collisions.

@laurmaedje
Copy link
Member

The idea here isn't that an attacker can't find a collision, but rather than we don't run into a collision accidentally. Reading this comment rust-lang/rust#10389 (comment), it appears that 128-bit SipHash gives us that.

@laurmaedje
Copy link
Member

I admit that the docs don't make that fully clear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants