openzeppelin-contracts icon indicating copy to clipboard operation
openzeppelin-contracts copied to clipboard

Confusion around what's maintained, what's not, and compatibility of packages.

Open hickscorp opened this issue 3 years ago • 13 comments

Good morning,

We've been using OpenZeppelin for a while now and have deployed contracts relying on the upgrade pattern a few years ago, and have upgraded them several times ever since.

Last year or so, we started using packages that require Solc to be >= 8.0.0, so we started patching OpenZeppelin headers in our node_modules folder using a yarn build script - so instead of having pragma solidity >=0.4.24 <0.7.0; in initializable.sol, we would have a requirement matching our 8.0.x compiler.

I have a question - and possibly an issue based on the answer:

  • Is there anything preventing initializable.sol to relax its requirements to pragma solidity >=0.4.24 <0.9.0;? The only problem that I could see would be around the assembly { cs := extcodesize(self) } call - with 0.8.0, if I understood correctly, there's a problem with the reported size? But the problem disappears in 0.8.1 - is this statement correct?
  • If this last statement is correct, would it be possible to have this compiler version requirement bumped as suggested here - but instead with a warning in the documentation?

hickscorp avatar Jan 04 '22 11:01 hickscorp

Hello @hickscorp

Solidity versions are breaking change. You cannot just simply take any 0.6.x or 0.7.x contract and mark it as 0.8.x. Stuff will break.

Openzeppelin provides different versions of its contract depending on which solidity version you use

  • @openzeppelin/contracts 3.x uses solidity 0.6.x (old, but still receiving security updates)
  • @openzeppelin/contracts 3.x-solc-0.7 uses solidity 0.7.x (old, but still receiving security updates)
  • @openzeppelin/contracts 4.x uses solidity 0.8.x (latest version, actively concerned)

You should choose the version of openzeppelin contract that matches your solidity requirements, and not try to modify the contract pragma ... as this can lead to unexpected behavior.

Amxx avatar Jan 05 '22 09:01 Amxx

Good morning @Amxx and thanks a lot for explaining.

In this case - is there a way to re-deploy the contracts compiled using a newer version to a new address and keeping the data segment of the proxy?

To make things a bit clearer - we started using features that at that time were experimental (for example having data structure and not just scalars) as return values. So we will thought that it would be fairly important to migrate to the compiler that accepted these features as stable when possible.

Also - imagine a contract deployed a while ago using solidity 0.7. If we want to upgrade to solc 0.8 - can we simply have the OpenZeppelin version bumped accordingly, or would this break too, for the existing / deployed contracts? What we're not 100% clear about is upgrading - given a contract already deployed - can we upgrade that contract using a newer version of Solc and OpenZeppelin?

hickscorp avatar Jan 05 '22 11:01 hickscorp

A quick bump on this - we are not finding documentation around this topic - Solc version upgrade. We're in a state where we already upgraded our compiler and upgraded smart contracts live without problem - but now we've put things in stand-by as it feels like we need to understand more.

  • If we upgrade Solc and OpenZeppelin on an existing project, would it work? Or are we "stuck" forever with the versions used for the very first deploy?
  • Also - if we actually made that mistake - would you say that we should rollback to previous versions?

Just so our situation is clearer - when we updated to Solc 0.8.x, we also upgraded OZ dependencies, such as:

    "@openzeppelin/cli": "^2.5.2",
    "@openzeppelin/contracts": "^4.4.0",
    "@openzeppelin/contracts-ethereum-package": "^3.0.0",
    "@openzeppelin/upgrades": "^2.5.2",

But again - we already had deployed this on-chain - and the contract upgradability is provided by @openzeppelin/upgrades as it was the only way by then. Is it an issue?

Maybe this is where we should have started the discussion :) We actually are using @openzeppelin/contracts at 4.4.0. But we're getting the pragma warning, as @openzeppelin/upgrades/contracts/Initializable.sol has a requirement on Solc being <7.0.0. So which @openzeppelin/upgrades version should we use - and most importantly - can we update it as we now use Solc 0.8.x? We understand that @openzeppelin/upgrades is not developed anymore so shall we switch to an OpenZeppelin plugin as suggested here?

hickscorp avatar Jan 07 '22 11:01 hickscorp

There are a lot of things here:

  • You cannot change the code of a "normal" contract that is already deployed onchain. If your contract is upgradeable, you can have the proxy point to a new instance (which is doing an upgrade) ... and we made sure that we have backward compatibility between OpenZeppelin 3.x and 4.x, so you could technically do such an upgrade if you need to add new features in your contracts.

  • Contract compiled with solidity 0.8.x, 0.7.x, 0.6.x (and even earlier version) CAN interact with one another. If you have a contract created with solidity 0.6.x, you can create another contract with soldity 0.8.x and have it interact with it (call functions, ...). You'll been the interface available, but that shouldn't be a major issue.

  • The current packages are:

    • @openzeppelin/contracts → "vanilla" contracts (4.4.1 is for solidity 0.8.x)
    • @openzeppelin/contracts-upgradeable → the upgradeable version of the contracts (4.4.1 is for solidity 0.8.x)
    • @openzeppelin/hardhat-upgrades → hardhat plugin for doing upgrades
    • @openzeppelin/truffle-upgrades → truffle plugin for doing upgrades

The ***-plugins are really just tooling to create deployment/upgrade scripts with all the necessary checks. The contracts and contracts-upgradeable are what you use to write your smart-contract in the first place.

Amxx avatar Jan 07 '22 13:01 Amxx

Hi @Amxx Hadrien, and thanks a lot again for taking the time.

We are really familiar with the underlying concepts (Eg the fact that you can't upgrade code on chain, what you do is upgrade the implementation, and the proxy delegates to it and runs it "atop" the proxy data segment). This is very clear but thanks for a recap ;)

Ideally, we don't want to move to any OpenZeppelin plugin - we're happy with what we have and how things work. We actually do not use @openzeppelin/contracts at all except for a few ERC20 interfaces - today we removed the dependency and simply declared these interfaces as part of our own project - all good.

What we use however is @openzeppelin/upgrades. We cannot update to plugins because during the upgrade process it finds what it calls legacy libraries in our manifest. We won't go that route.

The problem that I think we have is the fact that we upgraded Solc several times manually (From 0.5.x all the way to 0.8.x) - to a point where Initializable.sol won't compile anymore because of the Solc version requirement. But when looking at initializable.sol code, we realised that the only potential breaking point is assembly { cs := extcodesize(self) }, which however would never be called unless deploying a new contract (it's in a guard for the initializer). This is why we thought that patching the pragma was OK.

The other problem we spotted was that in @openzeppelin/upgrades, all files are marked pragma solidity ^0.5.0; (Eg AdminUpgradeabilityProxy.sol etc) - but we don't know if there's no upper limit because it would simply work, or if it is because @openzeppelin/upgrades wasn't updated for a very long time.

So in a nutshell - patching Initializable.sol still seems fine - doesn't it?

hickscorp avatar Jan 07 '22 15:01 hickscorp

@openzeppelin/upgrades is not supported anymore (it was deprecated before I even joined OZ 😄). It was replaced by the upgrades plugin, which contains the tooling for managing upgrades but not the contracts. This is why I was mentioning the plugins.

Initializable.sol is now provided as part of our contract libraries. Note that this files received a recent changes. The relevant version that you may be interested in are:

  • https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.4.1/contracts/proxy/utils/Initializable.sol (0.8.x latest)
  • https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.4.0/contracts/proxy/utils/Initializable.sol (0.8.x historical)
  • https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v3.4.2-solc-0.7/contracts/proxy/Initializable.sol (0.7.x)
  • https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v3.4.2/contracts/proxy/Initializable.sol (0.6.x)

Considering this contract has no constructor, you can indeed create a version that has a looser pragma that will support a wide range of compilers from 0.5.x to 0.8.x and package it with your code.

AdminUpgradeabilityProxy was renamed/replaced with TransparentUpgradeableProxy that you can also find in 0.6.x, 0.7.x and 0.8.x variations directly in the @openzeppelin/contracts package.

Amxx avatar Jan 07 '22 15:01 Amxx

This is very helpful, and I'm sure will also be of use for future users. Thank you very much for your candor @Amxx Hadrian!

Our final choice is to stick with @openzeppelin/upgrades for now - as we are not using anything else from the OpenZeppelin suite except for the cli - and we're happy with it.

At some point, we will see if we can migrate to @openzeppelin/contracts and use that other Initializable.sol contract.

hickscorp avatar Jan 07 '22 17:01 hickscorp

@Amxx I'm going to revive this thread, as we are starting a new project and we're again a bit confused. I've just opened an issue (more of a "what do you guys think") here: https://github.com/OpenZeppelin/openzeppelin-contracts/issues/3391

Related to it, there are a few other questions - but I think these other questions have their place here instead.

[...]

It could be that we need to use is ProxyFactory, as explained on various (outdated?) sources... But it seems that ProxyFactory is deprecated (as it lives in the @openzeppelin/upgrades package, and I remember @Amxx saying that it )?

What should be the dependency we use for our brand new project then - and if the only thing we're interested in is upgradeability features - probably @openzeppelin/contracts?

It's still very confusing - in the sense that when we install @openzeppelin/cli, the deprecated @openzeppelin/upgrades packages gets also added - but apparently it's too old? Is @openzeppelin/cli even still a thing, or should we ideally scratch using it, and do everything using @openzeppelin/truffle-upgrades programmatically?

On the OpenZeppelin SDK main repo, it says that the OpenZeppelin SDK isn't actively deployed anymore - and to use Upgrade Plugins. But if we decide to go for the @openzeppelin/truffle-upgrades, then the Initializable.sol file we would need to use is not usable, as we use solc 0.8.x... Also, @openzeppelin/truffle-upgrades relies on @openzeppelin/upgrades-core, itself with the same issue for us - its Initializable.sol declares pragma solidity >=0.4.24 <0.7.0;.

All in all, I'm super confused around all this. We use Truffle mainly for the test framework - we don't use their migrations - and we were relying on the oz CLI to deploy and upgrade stuff. For that new project, we don't have to use Truffle - but it's still not clear what the requirements are to be able to use a maintained version of OpenZeppelin Upgradeability features... And if we use Truffle, then we can also forget about linking against libraries, unless using /// @custom:oz-upgrades-unsafe-allow external-library-linking... So any guidance would really be appreciated here :)

hickscorp avatar May 04 '22 08:05 hickscorp

  • ... and we made sure that we have backward compatibility between OpenZeppelin 3.x and 4.x, so you could technically do such an upgrade if you need to add new features in your contracts.

@Amxx can you expand on that? it doesn't seem to hold true for all contracts. i had a big issue upgrading ERC721/ERC721Enumerable from v3 to v4 because the storage layout changed. fortunately, i discovered it while still on testnet, but it could be disastrous.

cruzdanilo avatar May 04 '22 21:05 cruzdanilo

@hickscorp

Both @openzeppelin/upgrades and @openzeppelin/cli are deprecated. Right now OpenZeppeling doesn't provide a self-standing SDK for deploying contracts.

Our recommendation is to use truffle or hardhat (I'd personally go for hardhat), with our contract and plugins. For reference, this is my goto package for new hacks/projects. I then usually build custom deployment hardhat scripts using ethers and @openzeppelin/hardhat-upgrades.

Now about Initializable.sol in @openzeppelin/upgrades-core, its here for backwards compatibility, because some people might still rely on it being there (IMO they shouldn't).

IMO:

  • all you contracts dependencies should be on @openzeppelin/contracts and @openzeppelin/contracts-upgradeable.
  • you should not try to get OZ contracts from anywhere else, unless its technological debt that you are carrying forward.
  • the upgrades plugin will deploy proxies compiled with 0.8.2 (if I remember correctly) that are pre-packages, you don't need to worry about them
  • the upgrades plugin might internally use some older contracts for testing, you should just ignore them if you see them in your dependency tree.

For the actual dependency hell:

  • If you depend on our contracts, they don't come with any dependency, so your are good here.
  • If you use our plugins,
    • the hardhat version gets "@openzeppelin/upgrades-core", "chalk" and "proper-lockfile"
    • the truffle version gets "@openzeppelin/upgrades-core", "@truffle/contract", "chalk", "solidity-ast"
      • that will carry a lot, but I would say its part of working with truffle
    • the "@openzeppelin/upgrades-core" gets 8 packages, which IMO are not that bad.

I would say that if you are using truffle/web3.js, and feel like you are in dependency hell, its probably not really because of us.

Amxx avatar May 04 '22 21:05 Amxx

@cruzdanilo

i had a big issue upgrading ERC721/ERC721Enumerable from v3 to v4 because the storage layout changed. fortunately

Could you please open an issue about that, explaining what your upgrade was, and what issue you ran into? I think this deserves its very own topic.

Amxx avatar May 04 '22 21:05 Amxx

To clarify on Initializable.sol: this contract is now included in the OpenZeppelin Contracts packages: @openzeppelin/contracts and @openzeppelin/contracts-upgradeable.

"Dependency hell" is not a good name for this discussion. Please try to make it actionable.

frangio avatar May 04 '22 22:05 frangio

@amxx Thanks a lot for taking the time, it helps a lot. I'm sure it will also clarify the situation for others. @frangio you are absolutely right - this was a mis-naming on my end when I created the issue a long time ago - my bad. I will edit it to suit better the problem I was trying to clarify.

hickscorp avatar May 08 '22 17:05 hickscorp

This seems resolved

JulissaDantes avatar Dec 09 '22 14:12 JulissaDantes