-
Notifications
You must be signed in to change notification settings - Fork 5.1k
[WIP] Remove eth-lib package and replace with ethereumjs-utils functions
#4101
Conversation
|
Hey @akshatamohanty thanks for the contirbution, just a heads up you should be targeting |
|
@GregTheGreek using this from the |
|
Sorry you're correct, looks like they haven't cut a new release yet |
|
So my pr #4083 actually could benefit from this PR. The final thing is tackling the |
| return (_.isUndefined(value) || _.isNull(value)); | ||
| }; | ||
|
|
||
| var flattenBytes = function(a) { |
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.
@GregTheGreek Yes, I'm getting stuck with the Account package. There don't seem to be direct mappings in ethereumjs-utils for most of it. I've added the functions from eth-lib inline, within the file as helpers (has missing references right now) and planning to cleanup each function to use internal utils instead. If they don't exist already, will add them to web3-utils (eg, Bytes.flatten, Bytes.slice etc). Does that sound like the right approach?
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.
|
This pull request introduces 1 alert when merging 701fc12 into 1ed3c68 - view on LGTM.com new alerts:
|
|
We covered this in the berlin hardfork |
Description
Fixes #4095
Type of change
Checklist:
npm run dtslintwith success and extended the tests and types if necessary.npm run test:unitwith success.npm run test:covand my test cases cover all the lines and branches of the added code.npm run buildand testeddist/web3.min.jsin a browser.CHANGELOG.mdfile in the root folder.