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

Use namespaced storage for upgradeable contracts #4534

Merged
merged 13 commits into from
Sep 11, 2023

Conversation

frangio
Copy link
Contributor

@frangio frangio commented Aug 19, 2023

This PR configures the transpiler to use namespaces instead of gaps in the upgradeable code.

There are two changes to the code that I found necessary. In one case, the transpiled code resulted in a stack too deep compilation error, I assume because the $ storage pointer takes up a place in the stack. In another case, I had to change a public immutable variable into a private variable, because immutables without an override get converted to storage variables and the namespace transformation enforces storage variables to be private. This implies that we can't use public immutables in the codebase in general.

The transpiler code can be found in OpenZeppelin/openzeppelin-transpiler#127.

Fixes #2964

@changeset-bot
Copy link

changeset-bot bot commented Aug 19, 2023

🦋 Changeset detected

Latest commit: 8972612

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
openzeppelin-solidity Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@socket-security
Copy link

Updated dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
solidity-ast 0.4.49...0.4.50 None +1/-0 264 kB frangio

@frangio
Copy link
Contributor Author

frangio commented Aug 23, 2023

I uploaded the result of this transpilation to a branch in openzeppelin-contracts-upgradeable so we can review the resulting changes: OpenZeppelin/openzeppelin-contracts-upgradeable@d16ec78

@frangio frangio marked this pull request as ready for review September 11, 2023 19:24
@frangio
Copy link
Contributor Author

frangio commented Sep 11, 2023

The latest results are in OpenZeppelin/openzeppelin-contracts-upgradeable@3b8d9b1. What changed was the location of initializer functions, and the latest updates to zero out the least significant byte in the location.

@frangio frangio merged commit b6111fa into OpenZeppelin:master Sep 11, 2023
@frangio frangio deleted the transpiler-namespaces branch September 11, 2023 19:32
@mudgen
Copy link

mudgen commented Sep 15, 2023

With this change OpenZeppelin upgradeable contracts should now be compatible with EIP-2535 Diamonds. Would be great to see an implementation of it in OpenZeppelin.

This was referenced Sep 10, 2024
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.

Use diamond storage instead of gaps in upgradeable contracts
3 participants