-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Change Hashing Function from Blake2b to Sha3 #1045
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1045 +/- ##
=======================================
Coverage 72.08% 72.08%
=======================================
Files 75 75
Lines 4579 4579
=======================================
Hits 3301 3301
Misses 961 961
Partials 317 317 |
@@ -27,7 +27,7 @@ func TestRunShuffleTest(t *testing.T) { | |||
} | |||
testCase := &ShuffleTestCase{ | |||
Input: []uint32{1, 2, 3, 4, 5}, | |||
Output: []uint32{2, 4, 1, 3, 5}, | |||
Output: []uint32{2, 4, 5, 1, 3}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why was this changed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shuffle test uses the hashing function. Diff hashing algorithm produces different shuffled output...
// https://github.com/ethereum/eth2.0-specs/blob/master/specs/core/0_beacon-chain.md#appendix | ||
func Hash(data []byte) [32]byte { | ||
var hash [32]byte | ||
h := blake2b.Sum512(data) | ||
|
||
h := sha3.Sum256(data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which sha3 is this ? the keccak-256 used by ethereum is different then what is finalised as sha3 as a standard
For reference:
https://ethereum.stackexchange.com/questions/550/which-cryptographic-hash-function-does-ethereum-use
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this is different @terenc3t - I think we should just use go-ethereum/common with its BytesToHash methods everywhere instead and deprecate our hashutils package
* switched from blake2b to sha3 * fixed test * use sha3 from eth1.0
Switched from blake2b to sha3/keccak-256 for our
Hash
functionSee rationale: ethereum/consensus-specs#227