TransparentUpgradeableProxy could be optimized by making admin immutable

[wighawag]

🧐 Motivation
Currently the Transparent proxy read the admin slot for every call to it.
Since TransparentUpgradeableProxy are most likely to be managed through a admin proxy contract that itself handle admin changes (Ownable),
it would be great to have a variant of the Transparent proxy where the admin cannot change.
With that you can store the admin both on storage (for compatibility with EIP-1967) and in an immutable variable (so you can read it cheaply).

The Proxy Admin contract will then be assigned indefinitely to that proxy and the admin changes will be handled on the ProxyAdmin only.

📝 Details
See an example implementation here : https://github.com/wighawag/hardhat-deploy/blob/4d5b527e07a4742cf97f9648a6e1d2795edbfd22/solc_0.7/proxy/OptimizedTransparentUpgradeableProxy.sol

[Amxx]

Hello @wighawag, Good to see you here!

I'll definitely think about adding such a proxy, even though this breaks some of the workflows that we think are part of security.

In the meantime, maybe you want to have a look at the "new" UUPS proxies, which still use the 1967 slot for implementation, are upgradeable, but let you defined you own upgradability restriction.

This is already available in 4.1-rc.0, or on the master branch.

• the proxy: proxy/ERC1967/ERC1967Proxy.sol
• the upgrade code that the underlying implementation should inherit from: proxy/utils/UUPSUpgradeable.sol

We have a guide to that coming soon, but I think you'll be able to figure things out by yourself :)
What you want to focus on is adding the proper "require" statement in
function _authorizeUpgrade(address newImplementation) internal virtual;
That would verify the sender matches the immutable address of the owner, which can be an EOA or a proxy admin

[wighawag]

Hi @Amxx thanks for the quick reply.

Regarding the use of UUPSUpgradeable this will not solve my issue as I still want to use Transparent Proxies.

[frangio]

@wighawag Would you mind sharing why you want to use Transparent Proxies as opposed to UUPS?

[wighawag]

My deployment tool (https://github.com/wighawag/hardhat-deploy) supports them and I want to continue offering that choice for my users.

They also offer the benefit that no collision are possible between the implementation abi and the proxy abi. My tool prevent deployment/upgrade of proxy where such collision is present but this can be an annoyance for the user as it require them to change the name of functions in the implementation.

[frangio]

This is a breaking change because it removes the feature of changing the admin of a proxy.

They also offer the benefit that no collision are possible between the implementation abi and the proxy abi.

Is this a statement about transparent proxies? Because if I'm reading it correctly it is not true about transparent proxies, but it is true about UUPS proxies, and is in fact one of the reasons why we are tending to prefer UUPS proxies.

Our proxy deployment tool (OpenZeppelin Upgrades Plugins) also supports transparent proxies and will continue to support them.

[wighawag]

Is this a statement about transparent proxies? Because if I'm reading it correctly it is not true about transparent proxies, but it is true about UUPS proxies, and is in fact one of the reasons why we are tending to prefer UUPS proxies.

What I meant is that with Transparent Proxies, contract developer can add any function they want to the implementation contract for purpose unrelated to proxy management and they ll be sure that users (non-admin) have no chance to hit a administration function. They can use upgradeTo for a function unrelated to proxy for example.

With UUPS proxies, developer are constrained to only use upgradeToAndCall and upgradeTo for the purpose of proxy administration. They cannot use it for other things while still being "proxy adiminstrable".

They are still great, but they have this drawback. Transparent Proxies have other drawbacks :)

[wighawag]

This is a breaking change because it removes the feature of changing the admin of a proxy.

This is only true if you modify the existing contract, you can make a different version, so user can choose if they want the non-negligible efficiency gain

[frangio]

We're trying to avoid adding multiple similar variants of a contract because it can become confusing for users, and difficult for us to maintain. But I'm happy to leave this issue open and hear from other people who are interested in this. If there is enough support for the idea we'll consider adding the variant. Our recommendation for more gas-efficient proxies will for now be focused on UUPS proxies.

[scott-silver]

@frangio @Amxx Have your thoughts around immutable admins in TransparentUpgradeableProxy contracts changed at all since April?

Here's my understanding:

Transparent Proxies are great, but checking whether a caller is the admin adds an extra SLOAD to every call; with the gas price of SLOADs increasing over time, this has become an expensive solution.

UUPS proxies are able to get the same functionality with one fewer SLOAD; so same functionality, but less cost.

But the rollback mechanism in UUPS proxies is somewhat frightening, and has already been involved in one serious vulnerability caught in the wild.

Basically, what @wighawag was asking for is a TransparentUpgradeableProxy with an immutable admin. This would maintain the functionality of a Transparent Proxy but without the added cost of an SLOAD per call. You lose the ability to change the admin, but that seems like a compromise that many projects could accept.

Have you reconsidered this approach at all?

[frangio]

@scott-silver First of all, we're reconsidering the UUPS rollback test mechanism for next release (either removing it or vastly simplifying it).

For Transparent proxies, I think we agree that an immutable admin variable is almost always superior. However, we're constrained by backwards compatibility. Since there is a changeAdmin function, we can't use an immutable variable. It would be possible for us to add a new variant of transparent proxy that uses an immutable variable, but we've avoided this so far because we're reluctant to offer too many variants of a contract as this can be confusing for users, and the options then need to propagate out to other affected libraries (e.g. upgrades plugins) and services. These are not great reasons to decide not to do this, but they make progress on this aspect slow.

Our preferred solution right now for the increased cost of SLOAD per call is UUPS proxies, and we're committed to address the concerns about the rollback test in the next release.