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

Update to Solidity v0.5 and Truffle v5.0 - 'v1' branch #170

Merged
merged 2 commits into from
Jul 22, 2019

Conversation

Muhammad-Altabba
Copy link
Contributor

  • Update to Truffle v5.0.9
  • Update .sol files for the breaking changes of Solidity v0.5
  • Explicitly configure Truffle to use solc v0.5.6
    (because some updated dependencies require solc version above the current default v0.5.0 version that came with current Truffle version)
  • Add solc v0.5.6 to package.json
  • Add compiler configuration to truffle.js
  • Update js test files to work with Web3 v1.0.
    Updates are mainly:
    - Use BigNumber.js and .toString() instead of .valueOf() because of issues with BN.js: Concern over bn.js dependency in 1.0 web3/web3.js#2171
    - Use async / await with Web3 because all Web3 v1.0 calls are asynchronous
  • Update Open Zeppelin to v2.2.0 and Canonical WEth to v1.4
  • Configure Travis to use Node 10 and 9 (Truffle v5 does not work with Node 7)

 - Update to Truffle v5.0.9
 - Update .sol files for the breaking changes of Solidity v0.5
 - Explicitly configure Truffle to use solc v0.5.6
(because some updated dependencies require solc version above the current default v0.5.0 version that came with current Truffle version)
 - Add solc v0.5.6 to package.json
 - Add compiler configuration to truffle.js
 - Update js test files to work with Web3 v1.0.
Updates are mainly:
        - Use BigNumber.js and `.toString()` instead of `.valueOf()` because of issues with BN.js: web3/web3.js#2171
        - Use async / await with Web3 because all Web3 v1.0 calls are asynchronous
 - Update Open Zeppelin to v2.2.0 and Canonical WEth to v1.4
 - Configure Travis to use Node 10 and 9 (Truffle v5 does not work with Node 7)
@Muhammad-Altabba
Copy link
Contributor Author

I found planning to do the transition to Solidity 0.5. And I hope to participate in this.

This pull request should also resolve the error raised when executing npm run migrate (#166). This issue is caused by versions incompatibility. And it is resolved when referencing util-contracts v3; This needed the current repository to be upgraded to solc v0.5.

Copy link
Contributor

@cag cag left a comment

Choose a reason for hiding this comment

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

@InfiniteStyles Hey, check this PR out.

@Muhammad-Altabba
Copy link
Contributor Author

Hello @InfiniteStyles,
Did you have some time to check?
Thanks,

@Muhammad-Altabba
Copy link
Contributor Author

Hi @InfiniteStyles

@cag
Copy link
Contributor

cag commented Jul 19, 2019

Hey, I'll take a closer look at this myself at some point. @InfiniteStyles isn't maintaining this repo anymore.

Copy link
Contributor

@cag cag left a comment

Choose a reason for hiding this comment

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

Hey, this is quite a huge changeset, but it all looks good to me. I do appreciate the drudge work done here. Internally, our focus has shifted elsewhere, so thanks for this maintenance work. I have one nitpick, if you don't mind, and I'll probably have to release this as a major version update, since even though nothing on the contract interface level has changed, I would guess that most of the dependencies might be using legacy toolchains which would break with this update.

assert(
isClose(actual, expected),
isClose(actual, expected, relTol=1e-3, absTol=1e-9),
Copy link
Contributor

@cag cag Jul 19, 2019

Choose a reason for hiding this comment

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

I think this will actually create relTol and absTol as global variables and set them to those values. I believe what you actually want to do is just:

isClose(actual, expected, 1e-3, 1e-9)

I do like that feature from Python though ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your appreciation.
I updated the code accordingly.
Regarding the version, do you like me to update the version at package.json to be 1.3.0? Anything else do I have to change regarding?
Many thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I can take care of the versioning later. Thanks!

@cag cag mentioned this pull request Jul 19, 2019
@cag cag merged commit bd5a66a into gnosis:v1 Jul 22, 2019
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.

2 participants