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

feat: add groestl hash functions with unit tests #80

Closed
wants to merge 1 commit into from

Conversation

HashEngineering
Copy link

The Groestl hash function is used by some crypto-curriencies as part of their proof of work algorithm. Examples include Groestlcoin. Others use Groestl along with about 10 or more others that are chained together.

Groestl was one of the SHA-3 candidates. While the Groestl hash function can support many output widths, this PR proposed the addition of two variants of the Groestl that returns 512 bits.

  • groestl512 - returns the full 512 bit (64 byte) hash value generated by Groestl512 hash function.
  • groestl256 - returns the first 256 bits (32 bytes) of the Groestl512 hash function

This PR was inspired primarily by https://github.com/Groestlcoin/groestl-hash-js, the code of which was heavily refactored to follow the scheme of noble-hashes.

@paulmillr
Copy link
Owner

Thanks. Why don’t you publish this as a separate package depending on noble-hashes?

@HashEngineering
Copy link
Author

We didn't consider the option of a separate package. Our thinking was centered around these ideas:

  • More cryptocurrency projects can switch to using noble-hash that use groestl (like dash, dgb etc)
  • We are beginning to use noble-hash for its current list algorithms and it would be easier if all algorithms were under a single repo.
  • We are planning to move more projects in using noble-hash if groestl gets added.
  • It would be easier for us to contribute (as in opening issues and pr’s), since it would be all in one place.

Some of these ideas would have been satisfied by a separate package, except for having a single package that contains everything.

You have done great work on this project and we appreciate your time in considering this addition.

@paulmillr
Copy link
Owner

  1. Unfortunately we can’t add all existing hashes, there must be some criterias filtering out remaining ones. Even if it will be included, it would need to get audited, to be considered secure. Groestl is cool, however, not very popular.
  2. If you publish the groestl package, and if it would depend on noble-hashes for some primitives, which means, whoever uses groestl AND noble, will get deduplicated code for free.
  3. Everyone who is building on top of noble is welcome and I will specifically mention your published package on noble website / docs.

@HashEngineering
Copy link
Author

Thank you for your time and we understand your point of view. We will consider the path of a separate package that depends on noble-hashes or forking noble-hashes depending on our requirements.

@paulmillr
Copy link
Owner

If there are any vars or classes that are not exported externally, which prevents you from using noble as dependency, let me know. It’s better for discoverability and community building to have dependents rather than 10 forks.

@gruve-p gruve-p deleted the add-groestl-hash branch January 25, 2024 08:30
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.

2 participants