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

wyhash fv4.1 #277

Closed
wants to merge 1 commit into from
Closed

wyhash fv4.1 #277

wants to merge 1 commit into from

Conversation

tansy
Copy link
Contributor

@tansy tansy commented Oct 3, 2023

V4.1 to make it more stream friendly

Change is cosmetic from a tester point of view. Just so you know.

@rurban
Copy link
Owner

rurban commented Oct 4, 2023

But that's still the bad wyhash version, which is slower and as has more bad seeds.

@wangyi-fudan
Copy link
Contributor

4.1 has bug due to make_secret function which came from someone's PR.
4.2 should work

@tansy
Copy link
Contributor Author

tansy commented Oct 4, 2023

4.2 should work

Didn't know it's already changed.

But that's still the bad wyhash version, which is slower and as has more bad seeds.

After taking a look I conclude the difference is in secret only. Is it that much of a difference?

@wangyi-fudan
Copy link
Contributor

yes. V4.1 seeds are generated by a "simplified" make_secret function. This function is from someone's early PR, and I didn't notice till now that he removed an essential code line (to make sure secrets are prime numbers). Thus your PR awake me to fixed this bug :-)

@rurban
Copy link
Owner

rurban commented Oct 5, 2023

I"m working on the 4.2 update instead

@rurban rurban closed this Oct 5, 2023
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

Successfully merging this pull request may close these issues.

3 participants