Use diamond storage instead of gaps in upgradeable contracts

[a-d-j-i]

🧐 Motivation
Gaps are problematic and this: https://eips.ethereum.org/EIPS/eip-2535#facets-state-variables-and-diamond-storage seems to be a great idea.

📝 Details
The proposed changes look like: OpenZeppelin/openzeppelin-contracts-upgradeable#134
I think this will improve the usability of upgradeable contracts and avoid a lot of common mistakes that are hard to fix.

[frangio]

Thank you for the proposal @a-d-j-i. I think this is actually a very good idea for the upgradeable contracts, but we will need to carefully consider it for the next major release because it is a breaking change.

Even if we agree that the breaking change is warranted, we are still somewhat concerned about people unknowingly upgrading their existing contracts to an incompatible version, but I wonder if we can have the contract self-check if it comes from a previous incompatible version, by introducing a notion of layout versioning.

Can you please expand on these points:

Gaps are problematic
I think this will improve the usability of upgradeable contracts and avoid a lot of common mistakes that are hard to fix.

[a-d-j-i]

The problem are not the gaps themselves but the “technique”:

• You still depend on the inheritance order.
• It requires a lot of attention from the user, forgetting a gap or adding variables without adjusting it can result in a disaster.
• The gap itself is a line of code that a lot of people don't understand, and don’t know why it is there.

What is the status of this issue?

[frangio]

@byterose As I mentioned above:

we will need to carefully consider it for the next major release because it is a breaking change.

There is currently no major release planned for the near future.

[SuperGNUS]

This is being worked on externally from the OpenZeppelin community right now. I'll ask OZ for a repo to move it to their organization once finished. https://github.com/GeniusVentures/openzeppelin-contracts-diamond

The transpiler update is here: https://github.com/GeniusVentures/openzeppelin-transpiler

Need to generate public getters, fix one bug and do write some testing for it. If anyone wants to help contribute, feel free to jump in.

[frangio]

We don't really want to maintain a third variant of the repo, maintaining two is already significant workload, so if we go this route we would make it the default Upgradeable variant.

It's great that you're working on this though, and you're welcome to maintain your fork.

Are you planning to use this for your project?

[SuperGNUS]

We don't really want to maintain a third variant of the rpeo, maintaining two is already significant workload, so if we go this route we would make it the default Upgradeable variant.

It's great that you're working on this though, and you're welcome to maintain your fork.

Are you planning to use this for your project?

Yes, planning on using this for the project. It now fully transpiles/compiles OpenZeppelin v4.4.1 and I'm about to npm publish it very shortly.

[SuperGNUS]

OpenZeppelin Contracts Upgradeable Diamond

Master Branch is compiling and I published it. I really wish it could be under @OpenZeppelin organization because the documentation needs to change otherwise.

Transpiled Upgradeable OpenZeppelin Contracts using Diamond Pattern is available for OZ 4.4.1

npm published package

<- or ->

npm install @gnus.ai/contracts-upgradeable-diamond

Forked github Repo

[mudgen]

@frangio When or if OpenZeppelin changes its implementations to use diamond storage (or whatever you want to call it) I am very supportive of OpenZeppelin having its own implementation of EIP2535 Diamonds. And of course OpenZeppelin may use as desired any of the code I have written for it. There are more than 40 projects using diamonds now and more coming. A list is here: https://eip2535diamonds.substack.com/p/list-of-projects-using-eip-2535-diamonds?s=w

[a-d-j-i]

@GVTopCoder There is a way to configure the transpiler so we can make it backward compatible (like taking the storage info as input) ?

[frangio]

@a-d-j-i What kinds of backwards compatibility are you looking for?