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

Branch/support for Solidity 0.5.0 #1498

Closed
axic opened this issue Nov 14, 2018 · 24 comments

Comments

@axic
Copy link
Contributor

commented Nov 14, 2018

I am a bit unsure how Zeppelin plans to support both 0.4.x and 0.5.x lines of Solidity, because clearly some form of support for 0.4.x seems to be needed in the short term given the stability of the ecosystem around it.

Here we've received an indication to use the solc-nightly branch, but I think that is just an intermediate solution.

I do not have any proposal at hand, but this may be a good place to discuss this in the public. Please delete if you think otherwise.

@elopio

This comment has been minimized.

Copy link
Member

commented Nov 14, 2018

Here we are close to get all our tests passing with solidity 0.5.0: #1496

That is the first step. Then, I see two options because we want OpenZeppelin 2.0 to have support and a stable API for a while.

If we consider that the addition of payable address is a breaking change of our stable API, we would have to release OpenZeppelin 3.0.

If we don't consider that a breaking change, we do what we usually do with solidity releases: wait a month or so and then release the update as v2.1.0. This is stretching a little the concept of stable API and semver, but maybe we can document it as an exception justified by the fact that we depend so closely on a project that is on the v0.x phase. Then when Solidity releases 1.0 we release OpenZeppelin 3.0 with a stable-for-real-this-time API.

In both cases, I think we should push everybody to use solidity 0.5.0. If they want an old compiler, they can use an old OpenZeppelin version.

@frangio what are your thoughts?

ps: we should announce this discussion on twitter so people affected can leave their proposals.

@maraoz

This comment has been minimized.

Copy link
Contributor

commented Nov 14, 2018

I'm slightly leaning towards maintaining this in a separate branch and then releasing it as 3.0.
A big question is: do we want to support the 0.4.x line in OpenZeppelin, or are we ok with forcing our users to use 0.5.x? Again, here I'm softly leaning towards the latter.
I'll tweet about this discussion to gather more opinions!

@j-h-scheufen

This comment has been minimized.

Copy link

commented Nov 14, 2018

You have to consider that some OZ users have already deployed code and therefore need a way to mix 0.4.x and 0.5.x code, i.e. make future deployments of 0.5.x-compatible code that leverages older 0.4.x contracts. I've not looked through all OZ contracts, but some of it is likely covered by the creation of 0.5 interfaces for 0.4 contracts as described in the breaking changes: https://solidity.readthedocs.io/en/latest/050-breaking-changes.html#interoperability-with-older-contracts
Where applicable, I'd hope that the next OZ release would contain such interfaces.

@maxsam4

This comment has been minimized.

Copy link

commented Nov 14, 2018

I think you should wait till truffle stable supports solidity 0.5 before making the default version of openzeppelin target solidity 0.5.

Meanwhile, maintaining a different branch and beta/next releases of openzeppelin for solidity 0.5 seem like a good way to me.

@nachomazzara

This comment has been minimized.

Copy link

commented Nov 14, 2018

I'm slightly leaning towards maintaining this in a separate branch and then releasing it as 3.0.
A big question is: do we want to support the 0.4.x line in OpenZeppelin, or are we ok with forcing our users to use 0.5.x? Again, here I'm softly leaning towards the latter.
I'll tweet about this discussion to gather more opinions!

Maybe it would be great to give the users some time until deprecate 0.4.x like an adoption lapse.
And release a 4.0 version of OZ with no support for 0.4.x.

@axic

This comment has been minimized.

Copy link
Contributor Author

commented Nov 30, 2018

Some key point from @nventuro in #42:

We're planning up upgrading to 0.5 once truffle releases a stable version supporting it.

@gnidan 👀 😉

@gnidan

This comment has been minimized.

Copy link

commented Nov 30, 2018

Some key point from @nventuro in #42:

We're planning up upgrading to 0.5 once truffle releases a stable version supporting it.

@gnidan 👀 😉

Workin' on it!

@icherko

This comment has been minimized.

Copy link

commented Dec 3, 2018

I forked the project, changed all the contracts to v0.5.0, fixed all the errors and most of the warnings. I created an ERC20 token using the code but don't know how to test all the files. I am still learning how to use github. Shall I do pull requests? Is there a branch I should use?

https://github.com/icherko/openzeppelin-solidity/tree/master/contracts

@JonahGroendal

This comment has been minimized.

Copy link

commented Dec 6, 2018

Any update on Solidity v0.5.x support? Truffle 5.0.0 beta no longer compiles solidity v0.4.x files.

@icherko

This comment has been minimized.

Copy link

commented Dec 6, 2018

Any update on Solidity v0.5.x support? Truffle 5.0.0 beta no longer compiles solidity v0.4.x files.

Not sure how to get Truffle working with v0.5 . I used the Solidity extension in Visual Studio Code.

Downloaded the latest compiler:
https://github.com/ethereum/solc-bin/blob/gh-pages/bin/soljson-v0.5.1%2Bcommit.c8a2cb62.js

Then in the user settings:
"solidity.compileUsingLocalVersion" : "C:\your path\soljson-v0.5.1+commit.c8a2cb62.js"

Open the sol file, F1, choose Solidity:compile, then each time use F5 to compile the opened file. Compile errors showing in the output section.

@gnidan

This comment has been minimized.

Copy link

commented Dec 6, 2018

Any update on Solidity v0.5.x support? Truffle 5.0.0 beta no longer compiles solidity v0.4.x files.

@JonahGroendal Add this to your Truffle config:

module.exports = {
  /* ... */

  compilers: {
    solc: {
      version: "0.4.25"
    }
  }
}
@JonahGroendal

This comment has been minimized.

Copy link

commented Dec 10, 2018

@gnidan thanks that worked, but I'm stuck in a situation where some of my dependencies are v0.5.x and some are v0.4.x. Maybe I'll use @icherko's fork for now until it gets merged or someone else updates the openzeppelin repo to 0.5.x

@nventuro

This comment has been minimized.

Copy link
Member

commented Dec 10, 2018

@JonahGroendal the 2.1 release (for which there will be a release candidate this Thursday, the actual release should be in about one or two weeks) will include 0.5.x support.

@nventuro

This comment has been minimized.

Copy link
Member

commented Dec 19, 2018

In #1568 all contracts have been ported to compile using v0.5.0. We will most likely release a 2.1 version with a tag for 0.5.x support (e.g. 2.1.0-solc-0.5), but expect this to be temporary: we're not yet sure what our long term strategy for handling Solidity breaking releases will be (minor bump? new major? separate distribution channel? thoughts on this are welcome!).

@dmitridb

This comment has been minimized.

Copy link

commented Dec 20, 2018

In the meantime might wanna let people know in the docs either to use old truffle, or to set the truffle-config.js compiler to 0.4.24 cause I'm gonna guess anyone going through them for the first time from now until 2.1 will be confused

@isaacsultan

This comment has been minimized.

Copy link

commented Dec 20, 2018

@dmitridb Good idea, it certainly would have saved me a lot of confusion. However, I believe the issue of web3js switching from BigNumber.js to BN persists with the compiler set to 0.4.24 or 0.4.25. @nventuro It would be great please if some recommendations were added to the docs regarding the truffle transition?

@nventuro

This comment has been minimized.

Copy link
Member

commented Dec 21, 2018

@j-h-scheufen those are definitely something we'll consider soon, we just haven't had the time yet.

@dmitridb thanks for the heads up, we'll be updating our readme and docs accordingly.

@isaacsultan indeed, that's why we haven't made the switch yet - the whole test suite needs to be updated. That said, since we only provide Solidity code, whether we use truffle 4 or 5 (and therefore web3 0.2.x or 1.0) doesn't matter to our users.

@nventuro

This comment has been minimized.

Copy link
Member

commented Dec 21, 2018

@axic @gnidan @JonahGroendal

Regarding supporting Solidity 0.5.x, they key issue is that we've committed ourselves to having a stable API, and requiring a compiler upgrade goes against that (in a nutshell, a stable API means everything will keep working the same accross minor version upgrades).

We've been discussing this (extensively) over the last couple days, since there seem to be no good solutions: all of them involve some sort of compromise, and we weren't sure which were the right ones. Below is a summary of things we considered and different possible approaches, and how we intend to move forward from now on. There is no final decision yet: we'd greatly appreciate everyone's input on this.

Some facts:

  • truffle now compiles using Solidity 0.5.0 by default, so any guides indicating to run npm install truffle openzeppelin-solidity are now broken.
  • Minor releases of Solidity may include features we want to add to OpenZeppelin, like 0.4.x's emit for events, constructor, view and pure, etc.
  • Not including said features by keeping OpenZeppelin in 0.5.0 (with a ^0.5.0 pragma) would mean that any users compiling with the latest version (which can be done on the very day of the release, thanks to truffle 5) will get warnings on OpenZeppelin code due to it not using them (i.e. the same thing that happens on 0.4.24 when using the old constructor syntax).
  • The Solidity team plans to release major versions, containing breaking changes, at a relatively fast pace (less than every one year). While (IMO) a good thing, given the current state of the ecosystem and need to iterate, this means that this issue will arise again in the not too distant future, suggesting some sort of long-lasting policy is required on our side.

Given this, we've considered:

  • Releasing a new major release (i.e. OpenZeppelin 3.0) on new Solidity major versions. This 'preserves' semver, but has some major problems: it doesn't address new desirable features being added in minors, makes our release schedule dependent on Solidity's (e.g. in this case, we'd be releasing 3.0 less than 3 months after 2.0, with no 2.1 in between), and makes OpenZeppelin's versioning scheme not very meaningful (since we'd have majors both for API-breakage and for compatibility, making the whole thing harder to understand). In particular, having to bump majors every couple of months due to reasons external to the planning of the library feels like a bad idea.
  • Having new releases support 0.4.24 by default, and 0.5.x on some side distribution channel. This could be either an npm tag (e.g. 2.1-solc-0.5), a subdirectory inside the package (import 'openzeppelin/contracts/0.5/ERC20.sol'), or a separate npm package. None of these solve desirable features added in minors, and add an extra layer of complexity on both maintaining the project and using it that we're not comfortable with. We don't want to add yet another package, and due to semver quirkiness it is not always obvious that the right package will be installed (npm isn't really designed to support having multiple versions of the same version).
  • Dropping the Solidity version entirely out of our stability guarantees. This means keeping 2.0 as the last 0.4.24 compatible OpenZeppelin release, and have 2.1 require Solidity 0.5.2 to compile. OpenZeppelin's stability will be restricted to its API, made up of contracts, functions and events, regardless of the tools used to achieve said API (the best ones available, which will generally mean the latest compiler version). Users that do not want to upgrade their Solidity version will still be able to use older releases, for which we would still provide support in the form of patch updates to e.g. fix bugs. npm's --save-exact flag can be used to pin an OpenZeppelin version, avoiding any future compatibility issues until users are ready to make the switch.

Our conclusion from the previous discussion is that these issues stem from a general immaturity on the development ecosystem. We'd like to both not be hindered by it, and push forward towards adoption of newer and better tools. Because of that we're heavily leaning towards the third option (having 2.1 require Solidity 0.5.2). All of this will of course be documented on our documentation, to make it as clear as possible.

This is not a decision we're making lightly, and once again, would love to hear your thoughts about it. If you have any suggestions that you think are better than our proposed scheme, please share them!

@maxsam4

This comment has been minimized.

Copy link

commented Dec 21, 2018

I am fine with any of the first or third option. Please avoid the second path, it's gonna add to the mess imo.

The libraries and tools in this ecosystem are updating so rapidly that you are forced to save exact version of the dependencies anyway. very subtly points at web3 :)

@AntonioJuliano

This comment has been minimized.

Copy link

commented Dec 21, 2018

Either the first or the third option sound good. I'd generally be in favor of whatever path leads to supporting new solidity releases the fastest.

@sowmyakannan

This comment has been minimized.

Copy link

commented Dec 27, 2018

Looking forward to update as I am having the same issues after upgrading truffle solc and openzeppelin.

@frangio

This comment has been minimized.

Copy link
Member

commented Dec 27, 2018

Support for 0.5 will be released next week, but in the meantime you can install v2.1.0-rc.2.

npm install openzeppelin-solidity@next
@SirPhemmiey

This comment has been minimized.

Copy link

commented Jan 2, 2019

Please has any update been made?

@nventuro

This comment has been minimized.

Copy link
Member

commented Jan 4, 2019

The 2.1 release can now be used with Solidity 0.5.0!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.