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

Clarify sha3 #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Clarify sha3 #12

wants to merge 1 commit into from

Conversation

subtly
Copy link

@subtly subtly commented Jun 15, 2015

No description provided.

@jbenet
Copy link
Member

jbenet commented Jun 15, 2015

Thanks great point. I had missed the internal state discussion and just labeled sha3. I've been using sha3-512: https://github.com/jbenet/go-multihash/blob/master/sum.go#L67-L71 and not sure if people have been using it. so i dont want to change that code to mean sha3-256 now.

Also, I think we should have all functions:

SHA3-224
SHA3-256
SHA3-384
SHA3-512
SHAKE128
SHAKE256

perhaps:

0x14, sha3-512
0x17, sha3-224
0x16, sha3-256
0x15, sha3-384
0x18, shake-128
0x19, shake-256

?

@ivilata
Copy link
Contributor

ivilata commented Jan 19, 2016

@jbenet I think that your last proposal makes total sense and bears no conflict (besides maybe adopting sha3 as a deprecated name for sha3-512), also nothing in the spec claims that functions with a higher code should be better, newer, more secure or have bigger hash sizes than functions with lower codes, so they may appear in any order.

I'll be implementing it like this in pymultihash.

@jbenet
Copy link
Member

jbenet commented Jan 19, 2016

@ivilata great sounds good. could you file a PR against this repo with code table updates you see fit?

@ivilata
Copy link
Contributor

ivilata commented Jan 19, 2016

@jbenet Done, see #20.

ivilata added a commit to ivilata/pymultihash that referenced this pull request Jan 19, 2016
@jbenet
Copy link
Member

jbenet commented Jul 30, 2016

@ivilata sorry for the months delay on review.

(besides maybe adopting sha3 as a deprecated name for sha3-512

i think we SHOULD keep sha3-512, as thats' what's been established so far by implementations. i wouldn't want a bad surprise for someone.

@@ -2,6 +2,6 @@ code, name
0x11, sha1
0x12, sha2-256
0x13, sha2-512
0x14, sha3
0x14, sha3-256; KECCAK[512]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this should be aliased to sha3-512, see #22

@kumavis kumavis mentioned this pull request Oct 18, 2016
@celeduc
Copy link
Contributor

celeduc commented Sep 4, 2017

This PR is out of date and should be closed.

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