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

Upgrade web3.js for latest node.js (<11) support #76

Merged
merged 4 commits into from
Aug 27, 2019

Conversation

schmidsi
Copy link
Contributor

The scrypt packages which is a dependency of web3-eth (and others) does not work with node.js >=11.15. This issue is already solved in web3/web3.js#2938, so theoretically it should be sufficient to just update web3 to the latest 1.x version (which happens in this pull-request). Unfortunately, after this update, some of the tests fail :(

I did not have time to find the root cause of this, but just wanted to inform about my failing attempt.

Maybe in the meantime, add something like this to package.json?

    "engines": {
        "node": ">=8.0.0 <=11.15.0"
    },

I can confirm that it works with [email protected] (LTS)

@CLAassistant
Copy link

CLAassistant commented Aug 22, 2019

CLA assistant check
All committers have signed the CLA.

@sohkai
Copy link
Contributor

sohkai commented Aug 22, 2019

Thanks @schmidsi, I'll look into why the tests are failing!

I think with web3.js having a proper v1 we can also remove the pin, since that was a fix for more unpredictable releases of web3.js previously :).

@schmidsi
Copy link
Contributor Author

I think with web3.js having a proper v1 we can also remove the pin, since that was a fix for more unpredictable releases of web3.js previously :).

As far as I know, web3 v1 is pretty stable: https://medium.com/@samuel_91690/1-0-release-web3-js-ddd23d3c8f62

@sohkai
Copy link
Contributor

sohkai commented Aug 22, 2019

Yep, exactly :).

…for each input

Otherwise web3.js thinks the input is of type `tuple`.
@sohkai sohkai marked this pull request as ready for review August 26, 2019 22:09
@coveralls
Copy link

coveralls commented Aug 26, 2019

Coverage Status

Coverage increased (+0.01%) to 90.85% when pulling 7b906f8 on schmidsi:upgrade-web3 into 1dc05b6 on aragon:master.

Copy link
Contributor

@sohkai sohkai left a comment

Choose a reason for hiding this comment

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

@schmidsi I've tracked down the issue to web3's ABIEncoderV2 implementation (circa beta.36 IIRC), where it now assumes all non-named inputs in a function are tuples.

I'm not sure where the rationale came about, and it does seem to be fixed in web3.js@2, but setting a non-empty name value fixes the issue for now.

I've also unpinned the web3.js version, since it has more stable versioning now than before :).

Thanks for trying this out @schmidsi!

@sohkai
Copy link
Contributor

sohkai commented Aug 27, 2019

Will release as a release candidate and test with a few existing examples to make sure nothing else was broken :).

@sohkai sohkai merged commit 6b6b844 into aragon:master Aug 27, 2019
@schmidsi schmidsi deleted the upgrade-web3 branch August 27, 2019 09:43
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