fix: update the return type to 'Hash32'

ethereum / eth-utils

Utility functions for working with ethereum related codebases.

https://eth-utils.readthedocs.io/en/latest/

MIT License

320 stars 152 forks source link

fix: update the return type to 'Hash32' #230

Closed e3243eric closed 2 weeks ago

e3243eric commented 2 years ago

What was wrong?

Issue #192

How was it fixed?

Update the return type from bytes to Hash32 for all methods that apply keccak. I explicitly cast the return type of eth_hash.auto.keccak to Hash32 in crypto.py, since its return type is bytes, maybe this should be fixed in eth_hash too.

To-Do

[x] Clean up commit history
[x] Add entry to the release notes

Cute Animal Picture

put a cute animal picture link inside the parentheses

e3243eric commented 2 years ago

Hi @carver, does this fix the issue? I need someone to do a code review. Thanks!

carver commented 2 years ago

I'm no longer an active maintainer on this project, paging @kclowes

carver commented 2 years ago

Note that I did not do a thorough search for all methods that should be updated, so part of the task is to identify all functions that should be updated.

carver commented 2 years ago

Also note that because some projects (especially some of ours) use type checking as part of CI, we typically treat type signature changes as breaking changes.

e3243eric commented 2 years ago

It seems like the newsfragment doesn't have a breaking type. We should update the validate_files.py.

carver commented 2 years ago

Ah yeah, that implies that eth-utils is due for a merge from the latest template

pacrob commented 2 weeks ago

Looking further into this, I think it's best to leave the return type as-is and allow downstream users to be more specific about types if they want. This would be breaking for at least 7 of our libs for not much gain. Open to discuss and reopen if someone has strong feels otherwise.