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

Review UUPS anti-bricking mechanism #933

Open
theethernaut opened this issue May 11, 2022 · 0 comments
Open

Review UUPS anti-bricking mechanism #933

theethernaut opened this issue May 11, 2022 · 0 comments
Assignees
Labels

Comments

@theethernaut
Copy link
Contributor

Our use of UUPS proxies allows for significant runtime gas cost savings vs transparent proxies. What enables this performance enhancement is the fact that UUPS proxies hold the admin upgradeability code (upgradeTo) in the implementation instead of in the proxy. This allows the proxy to not have a Solidity dispatcher or "function hub" that needs to check if the call is an admin call or something to be forwarded to the implementation.

However, the danger of having upgradeTo in the implementation instead of in the proxy, is that a proxy could be upgraded to a "sterile" implementation that no longer has upgradeTo (or more precisely that cannot write into the address storage slot that the proxy forwards calls to). I.e. the proxy could be "bricked" in an upgrade.

To ensure that this doesn't happen, we've come up with an anti-bricking mechanism that can be found here:
https://github.com/Synthetixio/synthetix-v3/blob/main/packages/core-contracts/contracts/proxy/UUPSImplementation.sol#L31-L33

And is tested here:
https://github.com/Synthetixio/synthetix-v3/blob/main/packages/core-contracts/test/contracts/proxy/UUPSProxy.test.js

This can also be tested manually by deploying an instance of one of our packages (E.g. spartan-council), and then re-deploying without the UpgradeModule.

The gist of the technique is to simulate an upgrade back to the old implementation whenever a new implementation is proposed. The simulation always reverts in a second EVM thread, so there are no side effects. The main EVM execution thread doesn't revert, but overlooks the second thread, using it to determine the "sterility" of the incoming implementation.

When we developed the technique, we looked into OpenZeppelin's anti-bricking implementation, but didn't like it because it had side effects (and was also vulnerable to an attacker being able to destroy the implementation code and hence brick the proxy). Maybe OZ has changed this. We would need to check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants