Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

fix attempt for issue #339 #397

Closed
wants to merge 0 commits into from
Closed

fix attempt for issue #339 #397

wants to merge 0 commits into from

Conversation

iccicci
Copy link

@iccicci iccicci commented Mar 25, 2019

Hi all,

probably there are thousands of better ways to fix this issue, but at least this will allow to use web3.js version 1.0.0-beta.47 or higher.

Hope this helps,
iCC

@iccicci
Copy link
Author

iccicci commented Mar 29, 2019

Hi @davidmurdoch ,

can you please give me some feedback about this task?
Is there something else I should do? IS there a documentation I have to read in order to do a good PR?

Thank you,
iCC

@davidmurdoch davidmurdoch self-requested a review April 3, 2019 15:44
Copy link
Member

@davidmurdoch davidmurdoch left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this PR, @iccicci! Looks like a great start!

I think we'll need to expose a new ganache option, chain_id. We'll need to investigate what the default value should be. We'll also need to investigate if setting the chain ID should change the way transactions are signed, per https://github.com/ethereum/EIPs/blob/master/EIPS/eip-155.md.

If you don't have the bandwidth to do this investigation let me know and we'll have somewhere here at Truffle do the leg work here.

@iccicci
Copy link
Author

iccicci commented Apr 3, 2019

Thank you for appreciating my job @davidmurdoch ;

unfortunately the problem is not my bandwidth, it is my knowledge; it seems I have to study a lot in order to be able to answer to all your question/investigations. :(
I'll try to learn something, but feel free to go on with this topic on some different way.

By the way, if I try to run all tests on my local machine (I use WSL) many of them fails; I can see there is a docker file, can you please explain me how to run tests locally?

Thank you,
iCC

@davidmurdoch
Copy link
Member

@iccicci, All you need to do is run npm test (after npm install) and you all tests should pass. I think I recall seeing timeouts when running the tests under wsl due to the tests timing out, as wsl can be a little slow. You can develop on ganache-core in windows, including running the tests. Give it a shot and let me know if you have any problems running it.

@iccicci
Copy link
Author

iccicci commented Apr 4, 2019

@davidmurdoch ,

I paid a bit more attention to error messages; upgraded node.js from v9 to v10; increased timeout in Gas tests; now all tests pass.

Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants