Benchmark Ripemd160 and optimize it#20
Conversation
About 50% reduction in instructions.
| // final block. Calling sum() a second time without an intervening | ||
| // reset() will hash additional padding bytes on top of the already- | ||
| // padded state and produce a different (incorrect) result. Call | ||
| // reset() before reusing the Digest for another message. |
There was a problem hiding this comment.
There is a high chance that developers miss this comment and (unknowingly) produce incorrect hashes.
How about:
- A: adding a defensive guard: trap on misuse so callers fail loudly instead of silently producing a wrong hash
- B: make sum() fully non-consuming (snapshot/restore): save the bits that padding mutates, the restore at the end.
Option A is clearly the easiest change, and it seems sufficient here to remove the footgun. If this is implemented, it would be good to mention this in the changelog.
There was a problem hiding this comment.
I did something similar in the new sha2 version ( https://github.com/research-ag/sha2/tree/static, not yet published).
Ok, I did option A here now for Ripemd160 and for Hmac where the same problem existed. After sum(), a subsequent write... or second sum() call will trap.
There was a problem hiding this comment.
Thanks for addressing the comment. The doc-comment still says that calling sum() a second time will produce an incorrect result.
There was a problem hiding this comment.
Fixed it. And fixed a couple of other very small points brought up by CodeRabbit. Ready for final review now.
github-actions
Bot
dismissed
fspreiss’s stale review
Review dismissed by automation script.
2 participants
Reduces instructions by 50%, GC pressure by 75%.