Skip to content
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

Blake2b Compression Function F is too slow #1836

Closed
carver opened this issue Aug 27, 2019 · 5 comments · Fixed by #1854
Closed

Blake2b Compression Function F is too slow #1836

carver opened this issue Aug 27, 2019 · 5 comments · Fixed by #1854

Comments

@carver
Copy link
Contributor

carver commented Aug 27, 2019

What is wrong?

Blake2b compression takes way too long in the all-python implementation. It's now trivial to DoS trinity & py-evm by filling a block with calls to the precompile one call to the precompile, specifying millions of rounds.

See #1818 for rough analysis:

It takes 25 seconds to run 2 million rounds (which is 2M gas). At that rate, it will take:

  • 100 seconds to run a full 8 million gas block (just confirmed, it took 91s)
  • 14 hours to run test vector 8, which has 4 billion rounds

So it's about 6 times too slow, at a minimum, assuming 15s blocks.

How can it be fixed

Ideally, write a python wrapper around the C implementation that is already distributed the stdlib blake2b module.

It should meet the same API as the current implementation:

def blake2b_compress(
num_rounds: int,
h_starting_state: TMessageBlock,
block: bytes,
t_offset_counters: Tuple[int, int],
final_block_flag: bool) -> bytes:

It is not a requirement that everything beneath that ^ API is in C. If you want a python layer that prepares & returns the appropriate types, that's fine/great. The primary requirement is that it runs fast enough.

It should run 8 million rounds in under 8 seconds (of course, faster is better).

A working PR should include a hypothesis test that shows equivalence between the all-python implementation and the new API. It should also include a test showing that it runs fast enough, maybe run 100k rounds to make sure they complete in under 0.1s.

Note that some of the nomenclature on the types and variable names may be a little funky, since the author (@carver) was not familiar with the internals of Blake2. Bonus points for identifying better names, with citations. (Especially for variables and types like message, block, and state)

@carver carver added the Bounty label Aug 27, 2019
@carver
Copy link
Contributor Author

carver commented Aug 27, 2019

I submitted a request to get this bountied.

@davesque
Copy link
Contributor

Have a working Rust implementation: https://github.com/davesque/blake2.git

Still need to write and package Python bindings.

@carver carver removed the Bounty label Sep 18, 2019
@davesque
Copy link
Contributor

@pipermerriam @carver
My solution for this is now available:

pip install 'blake2b-py==0.1.2'

The functions in that package have essentially the same API as the native python implementation. The docstrings are pretty informative (though they're abridged here):

In [1]: import blake2b

In [2]: blake2b.compress?
Signature:
blake2b.compress(
    rounds,
    starting_state,
    block,
    offset_counters,
    final_block_flag,
)
...

In [3]: blake2b.decode_parameters?
Signature: blake2b.decode_parameters(input)
...

In [4]: blake2b.decode_and_compress?
Signature: blake2b.decode_and_compress(input)
...

@davesque
Copy link
Contributor

@carver @pipermerriam By the way, if there's any need that blake2b-py be even faster, I saw a couple other optimized implementations in Rust that use SIMD instructions. We could try to incorporate some of those features into blake2b-py if necessary.

@carver
Copy link
Contributor Author

carver commented Sep 23, 2019

It's not likely to be a problem, at least for a while. I ran it against 4B gas and it took 90s. That is about 400 blocks worth of gas at today's gas limits. Assuming it scales down linearly, you could fill a block and the compression function would run for less than 250ms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants