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

x/crypto/{md4,ripemd160,sha3}: implement encoding.BinaryMarshaler and encoding.BinaryUnmarshaler #24617

ValarDragon opened this issue Mar 31, 2018 · 5 comments


Copy link

@ValarDragon ValarDragon commented Mar 31, 2018

This is the same idea as #24548, make these hash functions in crypto implement encoding.BinaryMarshaler and BinaryUnmarshaler to allow for state reuse. The password hashes aren't listed here, since they should be using a unique salt per hash, and therefore they wouldn't benefit from state reuse. (Also it seems that the password hashes don't implement hash.Hash)

Copy link

@FiloSottile FiloSottile commented Mar 31, 2018

Wow, we seriously have md4 in there. Don't bother adding support to that.

Copy link

@sabin-rapan sabin-rapan commented Apr 3, 2018

@ValarDragon are you implementing those interfaces for all the mentioned packages?
I'd like to join 😁.

Copy link

@ValarDragon ValarDragon commented Apr 3, 2018

I have it all implemented (even md4 since I had that done before I saw FiloSottile's comment), I'll push a change for code-review later today, I just need to make the test error messages more verbose. However I think there are some optimizations that can be done for the way I'm marshaling sha3. I think the only use case for marshaling it is when its in the absorbing state, not when its squeezing. (Even the tests for cloning don't cover the case where the state is squeezing) Currently I'm storing a byte for this case, but I'm not sure its needed. (Also, in the case that its in the squeezing state, I actually have to store an additional byte. Right now I am only storing that byte when its needed)

Copy link

@gopherbot gopherbot commented Apr 3, 2018

Change mentions this issue: sha3,md4,ripemd160: implement BinaryMarshaler, BinaryUnmarshaler

Copy link

@ValarDragon ValarDragon commented Apr 19, 2018

Is there anything else that needs to be done for this proposal on gerrit? (Asking because there haven't been any comments in gerrit, and its been awhile)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.