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

Add Tiger hash #50

Closed
wants to merge 2 commits into from
Closed

Add Tiger hash #50

wants to merge 2 commits into from

Conversation

jmcomets
Copy link

@jmcomets jmcomets commented Feb 15, 2018

Note that this depends on the addition of a len_padding_with method to
BlockBuffer* structs (RustCrypto/utils#7).

@newpavlov
Copy link
Member

Thank you for you contribution and welcome!

I will try to look over your code in the following week.

@newpavlov newpavlov mentioned this pull request Feb 17, 2018
18 tasks
@jmcomets
Copy link
Author

jmcomets commented Apr 5, 2018

Any updates on this?

@rawler
Copy link

rawler commented Jul 15, 2018

Also wondering about updates on this?

@newpavlov
Copy link
Member

Sorry for the long waiting time! I will need to introduce some minor changes before merging this PR.

Note that this depends on the addition of a `len_padding_with` method to
BlockBuffer* structs.
@jmcomets
Copy link
Author

jmcomets commented Aug 21, 2019

Hi there! I've rebased & updated the implementation to use the new API for padding.

I noticed that while the prefix was previously an argument to len_padding_with, this prefix argument disappeared in this commit, after the method was renamed to len64_padding_be.

Instead, the prefix was hardcoded to 0x80, which works for version 2 of the Tiger hash...but version 1 pads with a prefix of 0x01. I adapted the implementation to only provide version 2, but I don't understand why this prefix argument was dropped. 🤔

Finally, since the rebase my tests no longer build. I had a look & it seems that there's an ambiguity when calling .input(some_bytes). If someone could help me fix it, I'd appreciate it. 😅

@tarcieri
Copy link
Member

tarcieri commented Jun 10, 2020

@jmcomets sorry this PR has been so long in the tooth!

If you can please rebase it again and add a .github/workflows/tiger.yml (you can copy and paste it from another digest) I'll see to it we get this merged.

Note that we've released new versions of several of the dependencies, namely digest v0.9. See also #164 for a (presently WIP) block-buffer upgrade.

@tarcieri
Copy link
Member

tarcieri commented Feb 4, 2021

Closing in favor of #229

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.

4 participants