-
Notifications
You must be signed in to change notification settings - Fork 12
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
add support for Google HighwayHash #5
Conversation
Highway Hash 128 and 256 were finalized a few days ago. |
I think they haven't finalized the changes in the C implementation then. I just skimmed over the commit. Should I poke 'em, y'think? |
I believe the changes were running the final mix 6 times for 128 and 10 times for 256. You can also check the Go repo for minio's implementation since they also have updated their version to the latest. |
Hmm. Going based on what's in highwayhash... Look in c_bindings.h. Doesn't look like they export the 128 and 256-bit versions. I see them plainly in the source sitting right there. Strange. |
Yeah I did the same last night. But im sure yours must be less hacky than
mine.
Interestingly HighwayHash256 fails avalanche tests. They have many bits
that are too high in terms of percentage.
I have been working on another hash test framework that has some more
advanced tests, I will see how it works out.
Anyway, thanks Ryan, Damian. Hope you are both well!
Yves
…On 12 January 2018 at 03:59, Ryan Bastic ***@***.***> wrote:
@rbastic <https://github.com/rbastic> requested your review on:
demerphq/smhasher#5 <#5> add
support for Google HighwayHash.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#5 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAGex_fVKF0Zu_RyMAiYLaXytnRdheK2ks5tJsqBgaJpZM4RbvMY>
.
--
perl -Mre=debug -e "/just|another|perl|hacker/"
|
Id like to reconcile your changes with mine: https://github.com/demerphq/smhasher/tree/yves/highwayhash And then merge them. I used the c++ part, but for the life of me I had to do dodgy shite to make the linking work. Eventually i symlinked highwayhash similarly to you, but I also had to symlink libhighwayhash.so and libhighwayhash.so.0 as well. One thing that is weird is the validation hash for HighwayHash64 is different. I wonder why. Another interesting thing is that HighwayHash256 fails avalanche tests. For the number of tests we do it has too many bit which are too far from 50%. This does not seem to affect 128 or 64. BTW, the timing data from my patch is garbage. I am sure I am not handling the seed properly. Damian if you feel like poking that it would be cool. Yves |
Seems the reason that HighwayHash256 fails avalanche tests is #7. |
I got distracted with other things, I will revisit this when I get a chance. Yves |
Hi,
Here's a pull request to implement what Damian mentioned re: Highway hash ( #4 )
It mentions in Google HighwayHash's tests that they haven't finalized 128- and 256-bit yet in C++, so I've included what I think that code might be... but it's left commented out until that time comes in the future. (edit: this is in a specific comment in the code still but evidently is wrong)
Only the 64-bit is enabled currently and it's implemented in C form.
Some other things in the code: You can see that I wrapped things in HAVE_HIGHWAY, but I also wanted to make special mention that I intentionally left it so that CMakeLists.txt needs touch-ups.. It doesn't conditionally include the library.
( Not a cmake expert and just did this quickly with the hope that you'd clean it up ;) )
Cheers!