-
Notifications
You must be signed in to change notification settings - Fork 654
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
fix: make blake2b optional #2012
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks good but CI errors need to be addressed. Also just pinging @carver since that might affect Py-EVM and he's closer to that project than I am.
As far as I can tell, none of the linting errors have to do with files I edited.
Yes, this definitely might affect downstream users of |
Sorry for the late response.
Yes, but they still need to be addressed since we can not merge a PR that didn't pass all CI checks. I'm not working on this project anymore (and I just actually noticed this is a Py-EVM PR not a blake2b PR 🙈 ) but I know that @carver is currently working on EIP-1559 support for Py-EVM so if you don't feel like investing time into fixing these linting issues you may just wait for them to be fixed in master and then rebase at a later point :) |
Yes, this seemed like the best solution to this issue as adding Windows support to the Rust-based blake2-py would be much more difficult, and the blake2 precompile is not used nearly enough in production to justify that work.
I would prefer this sooner than later, I will take a look at the linting issues. This is a blocker for me. |
19c1b97
to
1d8d79e
Compare
Thanks @fubuloubu |
Great! Thanks! Any idea, when this gets released (uploaded to PyPI)? |
The next release will probably happen after London tests pass. You can watch #2021 |
Note: this will not fix #1941 because the issue there is the |
What was wrong?
fixes: #1959
may fix: #1941
How was it fixed?
Used the native implementation of blake2b compression function if
blake2b-py
is not installed.Made
blake2b-py
an optional dependency viaeth-extra
.To-Do
Cute Animal Picture