Add RIPEMD160 hash function and EVM precompile #505
Conversation
|
The base branch of this PR is |
| " " & "result: " & r.toHex() & "\n" & | ||
| " " & "expected: " & expectedbytes.toHex() & '\n' | ||
|
|
||
| stdout.write "Success\n" |
There was a problem hiding this comment.
That's the only test vector available?
I think we need to add fuzzing vs OpenSSL on Linux as well, similar to SHA256.
There was a problem hiding this comment.
There are some extra in last page of https://www.esat.kuleuven.be/cosic/publications/article-317.pdf
Hash of "" =
0x9c1185a5c5e9fc54612808977ee8f548b2258d31
Hash of "a" =
0x0bdc9d2d256b3ee9daae347be6f4dc835a467ffe
Hash of "abc" =
0x8eb208f7e05d987a9b044a8e98c6b087f15a0bfc
Hash of "message digest" =
0x5d0689ef49d2fae572b881b123a85ffa21595f36
Hash of "abcdefghijklmnopqrstuvwxyz" =
0xf71c27109c692c1b56bbdceb5b9d2865b3708dbc
Hash of "abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq" =
0x12a053384a9c0c88e405a06c27dcf49ada62eb2b
Hash of "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789" =
0xb0e20b6e3116640286ed3a87a5713079b21f5189
Hash of 8 times "1234567890" =
0x9b752e45573d4b39f4dbd3323cab82bf63326bfb
Hash of 1 million times "a" =
0x52783243c1697bdbe16d37f97f68f08325dc1528
There was a problem hiding this comment.
Yes, those test cases are where the ones in the file under isMainModule are from (and what the TODO in the PR message refers to).
| R51(c1, d1, e1, a1, b1, w15, 5) | ||
| R52(c2, d2, e2, a2, b2, w9 , 11) | ||
| R51(b1, c1, d1, e1, a1, w13, 6) | ||
| R52(b2, c2, d2, e2, a2, w11, 11) |
There was a problem hiding this comment.
That's super annoying to audit and RipeMD-160 is actually unused (https://ethereum-magicians.org/t/discussion-removal-of-ripemd-160-and-blake2f-precompiles/14857)
ideally we rewrite with this simpler pseudocode:
for(i := 0 to blocks - 1) {
aLeft := h0
bLeft := h1
cLeft := h2
dLeft := h3
eLeft := h4
aRight := h0
bRight := h1
cRight := h2
dRight := h3
eRight := h4
for(int j := 0 to 79) {
t := rotleft(s[j]) (aLeft + f(bLeft, cLeft, dLeft) + X[r[i]]) + eLeft
aLeft := eLeft;
eLeft := dLeft
dLeft := rotleft(10) (c)
cLeft := bLeft
bLeft := t
Do same for right
}
t := h1 + cLeft + dRight
h1 := h2 + dLeft + eRight
h2 := h3 + eLeft + aRight
h3 := h4 + aLeft + bRight
h4 := h0 + bLeft + cRight
h0 := t
}
- https://en.bitcoin.it/wiki/RIPEMD-160
- https://www.esat.kuleuven.be/cosic/publications/article-317.pdf (page 10)
- https://homes.esat.kuleuven.be/~bosselae/ripemd160/pdf/AB-9601/AB-9601.pdf (page 11)
but at the same time, probably not worth the effort and Bitcoin is the biggest user of RIPEMD-160 (and with most stakes). It can be made a low-priority issue for first-time contributors.
There was a problem hiding this comment.
I went with the current implementation (aside from time constraints), because it is essentially identical to the Bitcoin implementation (and I assume that one has seen plenty of eyes).
A first step towards a more custom solution could be to simply generate all those RXY calls from a macro & a constant array that contains the integers for the w_i to use.
But as you say, probably not that high on the priority list.
|
This needs a rebase so that it uses |
Given that the docstring explicitly promises to also reinitialize, this is the safer bet.
In this case the context is already defined in the generic implementation, so we just export it again.
|
Rebased, force pushed and added the test vectors from https://homes.esat.kuleuven.be/~bosselae/ripemd160.html as well as fuzzing against OpenSSL following the SHA256 test case. |
The hash function implementation right now is a direct port of the bitcoin-core implementation here:
https://github.com/bitcoin-core/gui/blob/228aba2c4d9ac0b2ca3edd3c2cdf0a92e55f669b/src/crypto/ripemd160.cpp
ripemd160_generic.nimtest file under awhen isMainModulesection, which need to move to a proper test case. Probably these static ones and then similar to the sha256 / keccak256 cases compared to OpenSSL for random data.