-
Notifications
You must be signed in to change notification settings - Fork 773
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
Util: bytes cleanup + do not import from ethereum-cryptography/utils #2845
Conversation
Hmm great, merge conflicts. Anyways, will stop for today. There are 2 test suits failing: wallet (seems to hang?) and client. To debug keep this in mind: The new bytesToHex is what was (and is re-exported) The new hexToBytes is what was (and is re-exported) |
Codecov Report
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. |
Remove |
b079375
to
bd9131e
Compare
Performance will go in a different PR because my changes in b079375 broke tests |
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.
OMG is this an insane PR.
Have nevertheless forced me to go at least once through the whole thing.
Beautiful.
🤩
LGTM.
Closes #2843
This PR:
prefixedHexStringToBytes
tohexStringToBytes
bytesToPrefixedHexString
tobytesToHex
ethereum-crypto/utils
(re-exported from util'sbytes
)TODOs
ethereum-cryptography/utils
prefixedHexStringToBytes
andbytesToPrefixedHexString
Upgrade performance of hexToBytes (do not slice + pad, this copies the string)- will go in a different PR, this commit b079375 breaks tests. See Update bytes performance #2848