ethereum / eth-hash

The Ethereum hashing function, keccak256, sometimes (erroneously) called sha256 or sha3
MIT License
104 stars 64 forks source link

Bottleneck in keccak256 calculation #35

Closed Uxio0 closed 2 years ago

Uxio0 commented 2 years ago

What is wrong?

When optimizing code, we detected there's a long time spent on Web3.keccak method, so we tried to find some information about what's happening ` We runned a benchmark and were shocked about the results (results are in seconds for 500000 keccak):

eth_hash 10.747742728999583
web3 14.371859122999012
cryptodome 7.886241019998124
pysha3 0.6233324849999917

When setting ETH_HASH_BACKEND="pysha3":

eth_hash 6.742922992998501
web3 10.079077139998844

Code that produced the error

import os

from Crypto.Hash import keccak as crypto_keccak
from eth_hash.auto import keccak as eth_hash_keccak
import sha3
from web3 import Web3

def eth_hash_benchmark():
    return eth_hash_keccak(os.urandom(32)).hex()

def web3_benchmark():
    return Web3.keccak(os.urandom(32)).hex()

def cryptodome_benchmark():
    k = crypto_keccak.new(digest_bits=256)
    k.update(os.urandom(32))
    return k.hexdigest()

def pysha3_benchmark():
    return sha3.keccak_256(os.urandom(32)).hexdigest()

if __name__ == '__main__':
    import timeit
    print('eth_hash', timeit.timeit("eth_hash_benchmark()", setup="from __main__ import eth_hash_benchmark", number=500000))
    print('web3', timeit.timeit("web3_benchmark()", setup="from __main__ import web3_benchmark", number=500000))
    print('cryptodome', timeit.timeit("cryptodome_benchmark()", setup="from __main__ import cryptodome_benchmark", number=500000))
    print('pysha3', timeit.timeit("pysha3_benchmark()", setup="from __main__ import pysha3_benchmark", number=500000))

How can it be fixed?

There should be something wrong for this big difference between the native libraries and eth-hash

Uxio0 commented 2 years ago

Hi @carver just quoting for making sure this is the right place for the issue, as this repo was not updated for some time

carver commented 2 years ago

Hi @carver just quoting for making sure this is the right place for the issue, as this repo was not updated for some time

Yeah, this is the right place for it. Thanks for the clear writeup!

I would review a PR that tries to address the issue. I don't have the opportunity to look into it myself right now, unfortunately.

Uxio0 commented 2 years ago

I would review a PR that tries to address the issue. I don't have the opportunity to look into it myself right now, unfortunately.

Awesome. Could you point me to that PR? I cannot find it on this repo

carver commented 2 years ago

To be more clear: if someone were to create a PR, I would review it 😄

Uxio0 commented 2 years ago

To be more clear: if someone were to create a PR, I would review it 😄

Nice, then https://github.com/ethereum/eth-hash/pull/36

fselmo commented 2 years ago

closed by #36... thanks @Uxio0 👌