Added pure-python fallback implementation for ripemd160 #33
Conversation
There was a problem hiding this comment.
Thanks you for reporting and fixing this issue.
I agree it's fine to use a non-constant time implementation, since it's only used for the fingerprint. Being constant time is not a goal of this library at the moment. I wouldn't be surprised if there were non-constant timeness on actual secret data.
nit: Could you add the tests from the original implementation (https://github.com/bitcoin/bitcoin/blob/124e75a41ea0f3f0e90b63b0c41813184ddce2ab/test/functional/test_framework/ripemd160.py#L112-L130) as a sanity check of the custom implementation?
|
Also, linter is failing. We use Black. A small |
Should I use the linter on I'll lint the other file. |
|
Rebased. I added |
| rip = hashlib.new("ripemd160") | ||
| rip.update(data) | ||
| return rip.digest() | ||
| except BaseException: |
There was a problem hiding this comment.
Is it to catch exception even on non-CPython? Because it looks like CPython would always raise a ValueError if it couldn't find the requested hash function.
(Note: i'm not asking to change it, was just curious about the usage of BaseException)
There was a problem hiding this comment.
I just saw it elsewhere (I think electrum), and I didn't bother making sure that it's always a ValueError (anyway, if any error happened, it doesn't feel wrong to use the custom implementation).
|
Oops, I did forget the test. |
|
ACK 7654daf |
Closes: #32