Replace Ahash with Fnv for Hashbrown

[shamatar]

Alternative to #824 , will fix #812 too for systems without atomics. Fnv provides no key randomization, but it shouldn't be required for commitment scheme.

• Targeted PR against correct branch (master)
• Linked to GitHub issue with discussion and accepted design OR have an explanation in the PR that describes this work.
• Wrote unit tests
• Updated relevant documentation in the code
• Added a relevant changelog entry to the Pending section in CHANGELOG.md
• Re-reviewed Files changed in the GitHub PR explorer

[Pratyush]

How about we use SipHash for non-atomic platforms, and ahash elsewhere?

[shamatar]

To be honest I'm not sure from top of my head if I can make cargo dependency optional based on whether a platform has atomics. Will try

[shamatar]

Another question is whether or not try to include aHash with or without randomization if it's done as external dependency instead of default hash in hashbrown. Hashbrown itself doesn't have and option to continue aHash's features, but maybe it can be imported independently and without randomization is may have no atomics by itself

[Pratyush]

To be honest I'm not sure from top of my head if I can make cargo dependency optional based on whether a platform has atomics. Will try

I think it's possible like so: https://github.com/PyO3/pyo3/blob/fe79f548174eb8108813f202bb0df9428ddfd806/Cargo.toml#L48

[shamatar]

restored features (inlines) on hashbrown + ahash is used on platforms with atomics

[autquis]

Hi! Can anyone give me some pointers on why the default-hasher of hashbrown is not suitable for us? They have changed it rust-lang/hashbrown#563 to FoldHash orlp/foldhash.

[shamatar]

Mainly because it was using atomics and would not compile on some platforms

[autquis]

Thanks for the prompt answer!
I didn't know FoldHash also uses atomics.
Another question, so it is safe to replace aHash with FoldHash in the changes of this PR, right?

[shamatar]

I didn’t check FoldHash. The main problem was that aHash was seeded, and seeing mechanism itself used atomics. So it may apply to FoldHash too as seeding is standard mechanism to avoid timing attacks on hash maps if queries could be externally provided/uncontrolled. For arkworks it’s not needed, but default configuration was to use such mechanism

[autquis]

Thanks a lot for the pointers! I just checked the seeding mechanism in FoldHash here and it seems they provide a fixed global state for the platforms that do not support atomics.
I would appreciate it very much if you could have a look and confirm if I understood correctly.

In that case, I can go ahead and enable default-hasher from hashbrown and that would simplify things.

[autquis]

Hi again! I would be very grateful if you could look into this.
I believe FoldHash is similar to FNV in the sense of seeding on non-atomic systems. A thumbs up from you would suffice :)

[shamatar]

What’s the interest for the default hasher? It’s just a feature flag and only provides few extra constructor methods. Maybe Default::default() starts to work, but not sure. At the moment I do not have capacity to check building on atomic-free platforms. Seeding code will most likely work, but it also depends if hashbrown doesn’t do some manual seeding itself

[autquis]

Thanks for the answer.
Regarding the interest in the default hasher: I think it simplifies the code. Currently, we have this conditional dependency on aHash and FNV, while having this feature enabled (assuming that FoldHash works for both atomic/non-atomic) we can get rid of the conditional dependency + we will have more concise code for instantiating a hashmap.

I will need this in some other repos of arkworks as well.