About proxyAdmin for TransparentUpgradeableProxy.sol

[idyllsss]

🧐 Motivation
My concern is whether it is reasonable to forcibly generate a new ProxyAdmin in the constructor of TransparentUpgradeableProxy.sol:

📝 Details
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/proxy/transparent/TransparentUpgradeableProxy.sol

constructor(address _logic, address initialOwner, bytes memory _data) payable ERC1967Proxy(_logic, _data) {
    _admin = address(new ProxyAdmin(initialOwner));
    // Set the storage value and emit an event for ERC-1967 compatibility
    ERC1967Utils.changeAdmin(_proxyAdmin());
}

If I want to use a TimelockController as the ProxyAdmin, it becomes very difficult. The contract's extensibility seems to be reduced.

[ernestognw]

Hi @idyllsss,

We had a long discussion around this and concluded that both contracts are tightly coupled since the ProxyAdmin can't do other than upgrade its corresponding proxy and the proxy doesn't allow the admin to interact in any other way.

The case where a TimelockController is the upgrader, then the ProxyAdmin can be owned by a TimelockController instead. Making the TimelockController the ProxyAdmin itself will prevent it from performing any other action (e.g. if your proxy is a token and the TimelockController has balance, it won't be able to transfer if it's the admin).

Let me know if that works!

[sakulstra]

@ernestognw Hey, also just stumbled over this.

For us the usecase is slightly different.
Currently, on aave there are multiple "Executors(I guess similar to timelockControllers)", from which each owns a ProxyAdmin and these proxy admins control the TransparentUpgradeableProxy instances.
So there is 1 ProxyAdmin per executor and when the executor ever migrates, the ProxyAdmin owner will be replaced.

With the new method of having a forced ProxyAdmin per TransparentUpgradeableProxy this pattern is no longer possible.
Therefore it would be great if we could pass our existing admin instead of creating a new one per contract.

[ernestognw]

Got it.

So the _admin was customizable in our 4.x version but one of the turning points to keep it immutable is that It brings big gas savings for token contracts that make intensive use of functions like transfer or transferFrom. Having the _admin immutable reduced SLOADs significantly.

Although this is the default behavior, I'd recommend overriding the _proxyAdmin internal function to bring your own admin address. You can store such address in ERC-1967 slots:

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.22;

import {TransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
import {ERC1967Utils} from "../ERC1967/ERC1967Utils.sol";

contract TransparentUpgradeableProxyCustomAdmin is TransparentUpgradeableProxy {
    function _proxyAdmin() internal view override returns (address) {
        return ERC1967Utils.getAdmin();
    }
}

This should be enough customization to enable changing the admin. You may need another mechanism to change the admin (e.g. I suggest using ERC1967Utils.changeAdmin), just consider that exposing another function in the proxy may compromise its transparency by introducing a small chance of collision.

Of course you can use an alternative method to store the admin.