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

Remove dependence on ethash (pyethash) #2121

Merged
merged 6 commits into from
Sep 6, 2023

Conversation

fselmo
Copy link
Collaborator

@fselmo fselmo commented Sep 5, 2023

What was wrong?

The move toward prioritization of the execution layer & EVM logic.

  • The goal for py-evm was once to provide the Trinity client with an EVM accessible via Python. This is not the case anymore as Trinity was sunset. With different consensus configurations, py-evm still serves well today for EVM logic testing. With the move to PoS (Proof of Stake) after the merge, there is a drive to remove the dependency on the ethash library. There are a few implementations of the Ethash algorithm in python and the goal of internalizing this logic into python-only code if desired is accessible without too much dev time being taken up. Using ethereum.org and the execution-specs as references, implement an in-house ethash algorithm that works well enough, even if it is significantly slower.

  • Added motivation: Python 3.10+ support in ethash leads to unknown segfaults. Maintaining the C bindings file in ethash for little benefit becomes an added chore to debug, find someone to review + approve, and update. That repository does not seem to be very actively maintained anymore and larger projects like go-ethereum have already dropped mining and ethash dependence altogether. Internalizing the ethash algorithm gives ease of control for supporting new python versions.

How was it fixed?

  • ethash implemented within py-evm as python code and POW is un-prioritized as a consensus choice within the library.

Todo:

  • Clean up commit history
  • Add or update documentation related to these changes
  • Add entry to the release notes

Cute Animal Picture

1000018598

- Remove 'c' file
- Revamp mkcache - working in ~3 mins
- Use pyethash master
@fselmo fselmo marked this pull request as ready for review September 5, 2023 22:07
Copy link
Member

@wolovim wolovim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not at all convinced i know the full implications, but can definitely appreciate the dep removal and better modern python support 👏 for our purposes, i don't see the algo performance being a blocker.

tests/core/consensus/test_pow_mining.py Show resolved Hide resolved
@fselmo fselmo merged commit 5782ad0 into ethereum:master Sep 6, 2023
32 checks passed
@fselmo fselmo deleted the keri-ethash-impl branch September 6, 2023 19:48
This was referenced Sep 6, 2023
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 this pull request may close these issues.

4 participants