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.

Sign up for GitHub

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

Jump to bottom

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

Closed
ValarDragon opened this issue Mar 31, 2018 · 7 comments
Closed

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

ValarDragon opened this issue Mar 31, 2018 · 7 comments
Labels
FeatureRequest help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@ValarDragon
Copy link

ValarDragon commented Mar 31, 2018 •
edited
Loading

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)
@gopherbot gopherbot added this to the Unreleased milestone Mar 31, 2018
@FiloSottile FiloSottile added help wanted NeedsFix The path to resolution is known, but the work has not been done. FeatureRequest labels Mar 31, 2018
@FiloSottile
Copy link
Contributor

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

@sabin-rapan
Copy link
Contributor

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

@ValarDragon
Copy link
Author

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)

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/104576 mentions this issue: sha3,md4,ripemd160: implement BinaryMarshaler, BinaryUnmarshaler

@ValarDragon
Copy link
Author

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)

@suhjohn
Copy link

suhjohn commented Jul 9, 2022

What's the reason for this not being merged

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/616635 mentions this issue: sha3: add MarshalBinary, AppendBinary, and UnmarshalBinary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
FeatureRequest help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@FiloSottile @ValarDragon @gopherbot @sabin-rapan @suhjohn