Planning the demise of OpenZeppelin Contracts' evil twin

TL;DR: How can we merge contracts and contracts-ethereum-package into a single offering, making it transparent to the user whether they are using upgradeable contracts or not?

If you are here in the OpenZeppelin forum, dear reader, you are probably familiar with our @openzeppelin/contracts library, which sports popular contracts like SafeMath, Ownable, or the most widely used ERC20 implementation.

However, it’s possible that you have missed our @openzeppelin/contracts-ethereum-package. This is a fork of the vanilla contracts package, where all contracts have been adapted to be upgradeable. This means, for instance, that they use initializer functions instead of constructors.

Note that the ethereum-package variant not only includes upgradeable versions, but also provides the addresses of reusable pre-deployed logic contracts, to save gas on deployments in public networks. But we will not focus on this here.

This duplicity has caused much confusion among our community, so we want to explore alternatives for removing it, and unifying everything under a single package. But let’s first recap how we got here in the first place.

The SDK and upgradeable contracts

The OpenZeppelin SDK (formerly known as ZeppelinOS) shipped since its version with upgradeable contracts as its main selling point. However, for a contract to be upgradeable, and thus used within the SDK, it needs to conform to a set of rules required by the proxy pattern we use:

  • It cannot have a constructor, nor can any of the contracts it extends from
  • It cannot assign initial values in variable declarations
  • It cannot remove or change the type of any existing storage variable when upgrading
  • Any contracts created from Solidity using MyContract.new() cannot be upgradeable

Since contracts did not conform to this pattern, and we did not want to remove constructors (they are useful after all!), we built the contracts-ethereum-package version.

To illustrate this, here is the constructor of the Ownable contract in the contracts package:

constructor () internal {
  _owner = msg.sender;
  emit OwnershipTransferred(address(0), _owner);
}

And the corresponding initializer of the Ownable version in the contracts-ethereum-package:

function initialize(address sender) public initializer {
  _owner = sender;
   emit OwnershipTransferred(address(0), _owner);
}

This duplicity in the contracts packages meant that users needed to use a different one depending whether they were building with our SDK or in a different platform. As explained in the SDK documentation:

Make sure you install @openzeppelin/contracts-ethereum-package and not the vanilla @openzeppelin/contracts . The latter is set up for general usage, while @openzeppelin/contracts-ethereum-package is tailored for being used with the OpenZeppelin SDK. This means that its contracts are already set up to be upgradeable.

This has lead to much confusion among the community, not to mention the efforts to port every change in the vanilla contracts repository to its upgradeable counterpart. Thus, we have had in the back of our minds different options to solve this problem.

Towards frictionless upgradeability

Over a year ago, @frangio wrote a fantastic blogpost explaining different approaches to automating the process of making a regular contract upgradeable. Go read it if you want to learn more about this!

The main takeaway of the post is that there are two main approaches to accomplish this feat:

  1. Manipulate the Solidity source code to change all constructors to initializers. This can be done through the AST that the compiler exposes, taking care to preserve the constructor semantics involved with multiple inheritance.
  2. Extract the constructor bytecode from the compilation result. This can be either deployed as an initializer contract, or passed as the proxy’s constructor. This is more direct, but requires some transformations directly on the assembly.

The first approach is more sensitive to changes in the Solidity source code, while the second to changes in the compiler codegen. Either way, both seemed promising at the time, and still do.

Our requirements

At this point, it’s worthwhile pausing and understand what are our requirements for what we are trying to accomplish.

  1. We want users of the SDK to just install @openzeppelin/contracts and use them.
  2. We want the SDK to support both upgradeable and vanilla contracts.
  3. We want the deployed proxy and logic contracts to be verifiable on Etherscan.

The last item means that we cannot freely manipulate the bytecode, but we need to be able to produce a Solidity contract that compiles to it. The second one implies that we need to watch out for name clashing between upgradeable and vanilla contracts on the same project.

And the first one leads us to two paths: transforming the contracts at the package and shipping duplicated versions, or adding these automated transformations directly on the SDK. Let’s explore both approaches.

Plan A: Ship upgradeable versions within @oz/contracts

The seemingly easiest option is to package all the upgradeable versions in a separate folder in the contracts package. Thus, we could have both ownership/Ownable.sol and upgradeable/ownership/Ownable.sol. The upgradeable versions could be generated manually, as we do today, or via a script that modifies the source code automatically, and then validated by us.

However, this has the problem that the user needs to knowingly import from a different path - not much of an improvement over the current situation. We could try to automatically manipulate the import paths if the user is writing an upgradeable contract, but since Solidity imports bring into the current namespace pretty much anything they find, this could easily get out of hand.

Furthermore, this is bound to end up in name clashings. If we have two contracts with the same name in the same project, and they accidentally get both imported into the same file, there is no way to refer to one or the other, and the compilation fails.

This means that we would need to ship all contracts with a prefix, ending up with Ownable and UpgradeableOwnable in the contracts package. Users would be able to install a single contracts package, but they would need to be explicit on which contract they use when writing their own, depending on whether they are writing vanilla or upgradeable contracts.

It’s unclear how much of an improvement we get here. At least, we can identify when the user is writing an upgradeable contract, we can walk through the base contracts, and warn them if they are using a non-upgradeable version.

It’s worth discussing as well how to detect when the user is writing an upgradeable contract. We have a few options here: 1) adding a magic comment to signal it, 2) extending a marker interface, or 3) deciding based on the create command issued by the user. While (3) is less cumbersome for the user, it has the downside of not having information on how the contract will be used until deploy-time.

Plan B: Automate initializers in the SDK at compile-time

The alternative is to build the process of automating initializers into the SDK itself. When we detect that a contract is marked as upgradeable (note that this needs to be only on the most-derived contracts, not the base ones) we can reprocess it to remove the constructors from it and all its ancestors, modifying either the source code or the bytecode. The resulting artifact can be compiled into a separate build/upgradeable folder to avoid name clashes, and then pulled from either that folder or the vanilla build/contracts one depending on which version the user wants to use.

The main advantage of this approach is that any contract that the user pulls in to their repo becomes automatically upgradeable, not just those from @openzeppelin/contracts. As a bonus, we could also inject storage gaps in intermediate contracts to make upgrades easier during these transformations.

On the other hand, this requires more effort than Plan A, since it requires (just to begin) a production-grade automatic script for transforming contracts on-the-fly.

There is also the risk of turning the SDK into too much of a black box. Would you as a developer feel comfortable having a tool that deploys a contract different to the one you’ve written? If the transformation is done at the Solidity level, we could store all the modified source codes in a separate folder (such as build/upgradeable-sources) so users can review them.

Also, there is the matter of tooling. How would a debugger behave in this case, given that sources are in a non-standard path? We would need to check that tools properly honour the sourcePath property in the build artifacts.

There’s also the matter of creating upgradeable contracts from Solidity itself. How should we manage a MyContract.new(params) in Solidity code, when making these transformations? An option is replacing it for a sequence of instance = MyContract.new(); instance.initialize(params);, but transformations start becoming more complex - more than we would like.

A solution to this would be to autogenerate the upgradeable versions in Solidity within the contracts source folder in the project, in a separate /upgradeable subfolder. This way users could explicitly refer to these versions. Note that this would also imply renaming each contract to avoid clashes.


As you can see, there are many approaches to this problem. We could start with Plan A, and as the tool for autogenerating initializable contracts matures, then shift into B. However, Plan A introduces some inconveniences for the user that we would need to patch in the SDK, that would go away if we go straight with B. Alternatively, we could stay with separate packages for a while, with the difference that we autogenerate contracts-ethereum-package using this tool.

We also need to settle on whether to make the transformations at the bytecode or the source code level. While bytecode level sounds more straightforward, since the constructor code is already packaged into a single chunk, making changes at the source level make it more auditable and easier to interact with.

What are your thoughts? And what have been your experiences around the duplicity of the contracts packages?

6 Likes

I like plan A. I don’t mind the naming convention of UpgradeableOwnable; in fact I think it’s very intention-revealing in that it’s showing there is additional functionality on top of the Ownable.

Personally, I’ve had some issues with having the IERC20 interfaces duplicated across the repos. I needed the ERC777, which wasn’t available yet in the @openzeppelin/contracts-ethereum-package, but which pointed to different interfaces. Having a unified codebase would allow all of that to be shared.

Additionally, there may be opportunities for abstract classes. In the example above, I’m sure the Ownable and UpgradeableOwnable have a lot in common and only their construction is different. It would be sensible to have a base AbstractOwnable and then an UpgradeableOwnable and StaticOwnable, perhaps. Can you tell that I used to code Java? :wink:

4 Likes

Woot, what an awesome writeup! You’ve laid out the rationale behind each decision and the pro’s and con’s masterfully. Almost as if you were a professional writer :stuck_out_tongue:

I say, go team plan B! It might be the hardest one, but it is the only actual solution to the problem, as opposed to a patch (of dubious usefulness in some cases, I’m not sure duplicating all sources under a different name will be a vast improvement).

I’ve never quite liked storage gaps: in my mind, a better solution would be to create a base Storage contract that had all state variables as private, with internal getters and setters.

The process will be auditable both by the opensource-ness of the code, and the fact that the end result is human-readable Solidity code. I’d argue that the output of such a tool is more trustworthy that what we’re currently asking users to do: manually modify their code, making sure all cases are covered, or be exposed to disaster. This is something not even I feel entirely comfortable doing, and I’m extremely familiar with the Contracts codebase.

3 Likes

Great post! I also think we should go with plan B. Because it is the more general solution and will also result in automating upgradeability for the users.

I think I would feel comfortable. The JavaScript ecosystem heavily relies on preprocessing: Babel, TypeScript, Rollup, etc. Perhaps for a new tool like this one I would want to manually verify the source code initially.

I think a big part of this comes down to how we communicate it. I like the framing of it as a “build step”, and the idea of placing the processed files in build/upgradeable-sources reinforces that very well. The CLI would automatically run this build step, but it would also be possible to run it on its own.

Doing this would imply also modifying every variable access into a function call. That increases the number of necessary changes per contract quite a lot.

3 Likes

Thank you @spalladino for this write up. :pray:

I see problems caused by the evil twin all the time for the community.

Often it is the community not being aware that we need to use contracts-ethereum-package in the first place with upgradeable contracts.

The non-automatic nature of creating the package means that contracts-ethereum-package can be behind OpenZeppelin Contracts versions. Currently there is an issue with contracts-ethereum-package 2.3 thanks to a previous Truffle artifacts issue, meaning that the community needs to use version 2.2.3, missing out on the goodness that is in OpenZeppelin Contracts 2.3 and 2.4.

I am less keen on a black box conversion and would prefer the upgradeable versions available in source code e.g. PlanB auto-generated source.
Though whichever option was used, would ideally want as much of the existing tooling to support this.

Coming from a C# background, I am ok with explicit named versions of contracts UpgradeableAAA.

I have seen storage gaps but haven’t asked how they are used (and I can’t find this in the documentation, but haven’t dug into the repository).

As an aside, for standalone deployed contracts, I assume that these could be separated out to make them easy to use.

Finally I would want to understand how we could upgrade a contract depending on OpenZeppelin Contracts when improvements are added to OpenZeppelin Contracts e.g. what upgrade initialization is required.

3 Likes

Wow what a write up!

I am also personally in favor of plan B, as @nventuro And @frangio mention it’s the only real solution and preprocessing is the norm in the JavaScript ecosystem.

If the bytecode was to be altered, I would want the option to just output just the transpiled solidity code rather than go straight to bytecode.

Another idea would be if the process could happen in linting so that the transpiled code would be immediately viewable and editable directly in the code editor but that might be a one-way operation for users.

2 Likes

As @abcoathup said, I was one of them who had the problem with contracts packages when installed first. Of course, it is because my little knowledge of Openzeppelin and didn’t read the docs carefully, but as a beginner, I think both packages should be more distinguishable(or should be one) and I wish developers are able to choose between “upgradable” and “non-upgradable”.

Both plans have pros and cons but if I have to choose one, B is the solution. Frankly speaking, “evil twin” is not bad(I know what it means “Contracts” and “SDK” now :grin:)

2 Likes

Tagging some of the community to get their input into the discussion (though appreciate input from everyone :pray:): @itinance, @paulinablaszk, @PaulRBerg, @pkr, @gitpusha, @costech, @CIX

1 Like

What is awesome writeup! @spalladino your style looks like a professor at university!
I’ve been pushing for automating upgradable contracts for the entire time so my voice is already cast.
Speaking of which way to generate upgradable contracts, I’d say Solidity source code generation because developers want to audit and verify their source code.

@frangio

The JavaScript ecosystem heavily relies on preprocessing: Babel, TypeScript, Rollup, etc. Perhaps for a new tool like this one I would want to manually verify the source code initially.

I would argue that this logic doesn’t work for Solidity because in the case of JavaScript preprocessing millions of dollars are not at stake of one tiny bug.

2 Likes