From 045ed4f5fb6ab7625a900ce90a9fb81acb6d0b7f Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Thu, 15 Jun 2023 12:53:18 -0600
Subject: [PATCH 01/15] Make TransparentUpgradeable proxy admin immutable

---
 contracts/mocks/DummyImplementation.sol       | 11 +++
 .../mocks/proxy/ClashingImplementation.sol    |  2 +-
 contracts/proxy/ERC1967/ERC1967Upgrade.sol    |  2 +-
 contracts/proxy/transparent/ProxyAdmin.sol    | 11 ---
 .../TransparentUpgradeableProxy.sol           | 27 +++---
 test/proxy/transparent/ProxyAdmin.test.js     | 21 +----
 .../TransparentUpgradeableProxy.behaviour.js  | 92 ++++++++-----------
 7 files changed, 68 insertions(+), 98 deletions(-)

diff --git a/contracts/mocks/DummyImplementation.sol b/contracts/mocks/DummyImplementation.sol
index 85503c36e74..56bada28726 100644
--- a/contracts/mocks/DummyImplementation.sol
+++ b/contracts/mocks/DummyImplementation.sol
@@ -2,6 +2,9 @@
 
 pragma solidity ^0.8.19;
 
+import "../interfaces/IERC1967.sol";
+import "../utils/StorageSlot.sol";
+
 abstract contract Impl {
     function version() public pure virtual returns (string memory);
 }
@@ -11,6 +14,9 @@ contract DummyImplementation {
     string public text;
     uint256[] public values;
 
+    // bytes32(uint256(keccak256('eip1967.proxy.admin')) - 1)
+    bytes32 internal constant _ADMIN_SLOT = 0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103;
+
     function initializeNonPayable() public {
         value = 10;
     }
@@ -44,6 +50,11 @@ contract DummyImplementation {
     function reverts() public pure {
         require(false, "DummyImplementation reverted");
     }
+
+    // Use for forcing an unsafe TransparentUpgradeableProxy admin override
+    function _unsafeOverrideAdmin(address newAdmin) public {
+        StorageSlot.getAddressSlot(_ADMIN_SLOT).value = newAdmin;
+    }
 }
 
 contract DummyImplementationV2 is DummyImplementation {
diff --git a/contracts/mocks/proxy/ClashingImplementation.sol b/contracts/mocks/proxy/ClashingImplementation.sol
index 957bc34be69..89904b91f16 100644
--- a/contracts/mocks/proxy/ClashingImplementation.sol
+++ b/contracts/mocks/proxy/ClashingImplementation.sol
@@ -9,7 +9,7 @@ pragma solidity ^0.8.19;
 contract ClashingImplementation {
     event ClashingImplementationCall();
 
-    function changeAdmin(address) external payable {
+    function upgradeTo(address) external payable {
         emit ClashingImplementationCall();
     }
 
diff --git a/contracts/proxy/ERC1967/ERC1967Upgrade.sol b/contracts/proxy/ERC1967/ERC1967Upgrade.sol
index ca6b92580f0..f08f7d796eb 100644
--- a/contracts/proxy/ERC1967/ERC1967Upgrade.sol
+++ b/contracts/proxy/ERC1967/ERC1967Upgrade.sol
@@ -123,7 +123,7 @@ abstract contract ERC1967Upgrade is IERC1967 {
      * https://eth.wiki/json-rpc/API#eth_getstorageat[`eth_getStorageAt`] RPC call.
      * `0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103`
      */
-    function _getAdmin() internal view returns (address) {
+    function _getAdmin() internal view virtual returns (address) {
         return StorageSlot.getAddressSlot(_ADMIN_SLOT).value;
     }
 
diff --git a/contracts/proxy/transparent/ProxyAdmin.sol b/contracts/proxy/transparent/ProxyAdmin.sol
index 490f552f852..e8578a58561 100644
--- a/contracts/proxy/transparent/ProxyAdmin.sol
+++ b/contracts/proxy/transparent/ProxyAdmin.sol
@@ -16,17 +16,6 @@ contract ProxyAdmin is Ownable {
      */
     constructor(address initialOwner) Ownable(initialOwner) {}
 
-    /**
-     * @dev Changes the admin of `proxy` to `newAdmin`.
-     *
-     * Requirements:
-     *
-     * - This contract must be the current admin of `proxy`.
-     */
-    function changeProxyAdmin(ITransparentUpgradeableProxy proxy, address newAdmin) public virtual onlyOwner {
-        proxy.changeAdmin(newAdmin);
-    }
-
     /**
      * @dev Upgrades `proxy` to `implementation`. See {TransparentUpgradeableProxy-upgradeTo}.
      *
diff --git a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
index 4536e28c881..3ba82dbd9ce 100644
--- a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
+++ b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
@@ -12,8 +12,6 @@ import "../ERC1967/ERC1967Proxy.sol";
  * include them in the ABI so this interface must be used to interact with it.
  */
 interface ITransparentUpgradeableProxy is IERC1967 {
-    function changeAdmin(address) external;
-
     function upgradeTo(address) external;
 
     function upgradeToAndCall(address, bytes memory) external payable;
@@ -46,12 +44,21 @@ interface ITransparentUpgradeableProxy is IERC1967 {
  * fully implement transparency without decoding reverts caused by selector clashes between the proxy and the
  * implementation.
  *
+ * IMPORTANT: This contract avoids unnecessary storage reads by setting the admin only during construction as an immutable variable,
+ * preventing any changes thereafter. However, the admin slot defined in ERC-1967 can still be overwritten by the implementation
+ * logic pointed to by this proxy. In such cases, the contract may end up in an undesirable state where the admin slot is different
+ * from the actual admin.
+ *
  * WARNING: It is not recommended to extend this contract to add additional external functions. If you do so, the compiler
  * will not check that there are no selector conflicts, due to the note above. A selector clash between any new function
  * and the functions declared in {ITransparentUpgradeableProxy} will be resolved in favor of the new one. This could
  * render the admin operations inaccessible, which could prevent upgradeability. Transparency may also be compromised.
  */
 contract TransparentUpgradeableProxy is ERC1967Proxy {
+    // An immutable address for the admin avoid unnecessary SLOADs before each call
+    // at the expense of removing the ability to change the admin once it's set.
+    address immutable _admin;
+
     /**
      * @dev The proxy caller is the current admin, and can't fallback to the proxy target.
      */
@@ -67,6 +74,7 @@ contract TransparentUpgradeableProxy is ERC1967Proxy {
      * optionally initialized with `_data` as explained in {ERC1967Proxy-constructor}.
      */
     constructor(address _logic, address admin_, bytes memory _data) payable ERC1967Proxy(_logic, _data) {
+        _admin = admin_;
         _changeAdmin(admin_);
     }
 
@@ -81,8 +89,6 @@ contract TransparentUpgradeableProxy is ERC1967Proxy {
                 ret = _dispatchUpgradeTo();
             } else if (selector == ITransparentUpgradeableProxy.upgradeToAndCall.selector) {
                 ret = _dispatchUpgradeToAndCall();
-            } else if (selector == ITransparentUpgradeableProxy.changeAdmin.selector) {
-                ret = _dispatchChangeAdmin();
             } else {
                 revert ProxyDeniedAdminAccess();
             }
@@ -95,17 +101,12 @@ contract TransparentUpgradeableProxy is ERC1967Proxy {
     }
 
     /**
-     * @dev Changes the admin of the proxy.
+     * @dev Returns the current immutable admin.
      *
-     * Emits an {AdminChanged} event.
+     * Overrides ERC1967's admin in favor of an immutable value to avoid unnecesary SLOADs on each proxy call.
      */
-    function _dispatchChangeAdmin() private returns (bytes memory) {
-        _requireZeroValue();
-
-        address newAdmin = abi.decode(msg.data[4:], (address));
-        _changeAdmin(newAdmin);
-
-        return "";
+    function _getAdmin() internal view virtual override returns (address) {
+        return _admin;
     }
 
     /**
diff --git a/test/proxy/transparent/ProxyAdmin.test.js b/test/proxy/transparent/ProxyAdmin.test.js
index d660ffc56b4..23f7ce9b286 100644
--- a/test/proxy/transparent/ProxyAdmin.test.js
+++ b/test/proxy/transparent/ProxyAdmin.test.js
@@ -6,11 +6,11 @@ const ProxyAdmin = artifacts.require('ProxyAdmin');
 const TransparentUpgradeableProxy = artifacts.require('TransparentUpgradeableProxy');
 const ITransparentUpgradeableProxy = artifacts.require('ITransparentUpgradeableProxy');
 
-const { getAddressInSlot, ImplementationSlot, AdminSlot } = require('../../helpers/erc1967');
+const { getAddressInSlot, ImplementationSlot } = require('../../helpers/erc1967');
 const { expectRevertCustomError } = require('../../helpers/customError');
 
 contract('ProxyAdmin', function (accounts) {
-  const [proxyAdminOwner, newAdmin, anotherAccount] = accounts;
+  const [proxyAdminOwner, anotherAccount] = accounts;
 
   before('set implementations', async function () {
     this.implementationV1 = await ImplV1.new();
@@ -32,23 +32,6 @@ contract('ProxyAdmin', function (accounts) {
     expect(await this.proxyAdmin.owner()).to.equal(proxyAdminOwner);
   });
 
-  describe('#changeProxyAdmin', function () {
-    it('fails to change proxy admin if its not the proxy owner', async function () {
-      await expectRevertCustomError(
-        this.proxyAdmin.changeProxyAdmin(this.proxy.address, newAdmin, { from: anotherAccount }),
-        'OwnableUnauthorizedAccount',
-        [anotherAccount],
-      );
-    });
-
-    it('changes proxy admin', async function () {
-      await this.proxyAdmin.changeProxyAdmin(this.proxy.address, newAdmin, { from: proxyAdminOwner });
-
-      const newProxyAdmin = await getAddressInSlot(this.proxy, AdminSlot);
-      expect(newProxyAdmin).to.be.equal(newAdmin);
-    });
-  });
-
   describe('#upgrade', function () {
     context('with unauthorized account', function () {
       it('fails to upgrade', async function () {
diff --git a/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js b/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js
index c6e94915626..1ba43eb97c6 100644
--- a/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js
+++ b/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js
@@ -47,6 +47,25 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
     });
   });
 
+  describe('proxyAdmin', function () {
+    it('sets the admin in the storage ', async function () {
+      expect(await getAddressInSlot(this.proxy, AdminSlot)).to.be.equal(proxyAdminAddress);
+    });
+
+    it('can overwrite the admin by the implementation', async function () {
+      const dummy = new DummyImplementation(this.proxyAddress);
+      await dummy._unsafeOverrideAdmin(anotherAccount);
+      const ERC1967AdminSlotValue = await getAddressInSlot(this.proxy, AdminSlot);
+      expect(ERC1967AdminSlotValue).to.be.equal(anotherAccount);
+
+      // Still allows previous admin to execute admin operations
+      expect(ERC1967AdminSlotValue).to.not.equal(proxyAdminAddress);
+      expectEvent(await this.proxy.upgradeTo(this.implementationV1, { from: proxyAdminAddress }), 'Upgraded', {
+        implementation: this.implementationV1,
+      });
+    });
+  });
+
   describe('upgradeTo', function () {
     describe('when the sender is the admin', function () {
       const from = proxyAdminAddress;
@@ -258,51 +277,14 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
     });
   });
 
-  describe('changeAdmin', function () {
-    describe('when the new proposed admin is not the zero address', function () {
-      const newAdmin = anotherAccount;
-
-      describe('when the sender is the admin', function () {
-        beforeEach('transferring', async function () {
-          this.receipt = await this.proxy.changeAdmin(newAdmin, { from: proxyAdminAddress });
-        });
-
-        it('assigns new proxy admin', async function () {
-          const newProxyAdmin = await getAddressInSlot(this.proxy, AdminSlot);
-          expect(newProxyAdmin).to.be.equal(anotherAccount);
-        });
-
-        it('emits an event', function () {
-          expectEvent(this.receipt, 'AdminChanged', {
-            previousAdmin: proxyAdminAddress,
-            newAdmin: newAdmin,
-          });
-        });
-      });
-
-      describe('when the sender is not the admin', function () {
-        it('reverts', async function () {
-          await expectRevert.unspecified(this.proxy.changeAdmin(newAdmin, { from: anotherAccount }));
-        });
-      });
-    });
-
-    describe('when the new proposed admin is the zero address', function () {
-      it('reverts', async function () {
-        await expectRevertCustomError(
-          this.proxy.changeAdmin(ZERO_ADDRESS, { from: proxyAdminAddress }),
-          'ERC1967InvalidAdmin',
-          [ZERO_ADDRESS],
-        );
-      });
-    });
-  });
-
   describe('transparent proxy', function () {
     beforeEach('creating proxy', async function () {
       const initializeData = Buffer.from('');
-      this.impl = await ClashingImplementation.new();
-      this.proxy = await createProxy(this.impl.address, proxyAdminAddress, initializeData, { from: proxyAdminOwner });
+      this.clashingImplV0 = (await ClashingImplementation.new()).address;
+      this.clashingImplV1 = (await ClashingImplementation.new()).address;
+      this.proxy = await createProxy(this.clashingImplV0, proxyAdminAddress, initializeData, {
+        from: proxyAdminOwner,
+      });
       this.clashing = new ClashingImplementation(this.proxy.address);
     });
 
@@ -315,24 +297,28 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
     });
 
     describe('when function names clash', function () {
-      it('when sender is proxy admin should run the proxy function', async function () {
-        const receipt = await this.proxy.changeAdmin(anotherAccount, { from: proxyAdminAddress, value: 0 });
-        expectEvent(receipt, 'AdminChanged');
+      it('executes the proxy function if the sender is the admin', async function () {
+        const receipt = await this.proxy.upgradeTo(this.clashingImplV1, { from: proxyAdminAddress, value: 0 });
+        expectEvent(receipt, 'Upgraded', { implementation: this.clashingImplV1 });
       });
 
-      it('when sender is other should delegate to implementation', async function () {
-        const receipt = await this.proxy.changeAdmin(anotherAccount, { from: anotherAccount, value: 0 });
-        expectEvent.notEmitted(receipt, 'AdminChanged');
+      it('delegates the call to implementation when sender is not the admin', async function () {
+        const receipt = await this.proxy.upgradeTo(this.clashingImplV1, { from: anotherAccount, value: 0 });
+        expectEvent.notEmitted(receipt, 'Upgraded');
         expectEvent.inTransaction(receipt.tx, this.clashing, 'ClashingImplementationCall');
       });
 
-      it('when sender is proxy admin value should not be accepted', async function () {
-        await expectRevert.unspecified(this.proxy.changeAdmin(anotherAccount, { from: proxyAdminAddress, value: 1 }));
+      it('requires 0 value calling upgradeTo by proxy admin', async function () {
+        await expectRevertCustomError(
+          this.proxy.upgradeTo(this.clashingImplV1, { from: proxyAdminAddress, value: 1 }),
+          'ProxyNonPayableFunction',
+          [],
+        );
       });
 
-      it('when sender is other value should be accepted', async function () {
-        const receipt = await this.proxy.changeAdmin(anotherAccount, { from: anotherAccount, value: 1 });
-        expectEvent.notEmitted(receipt, 'AdminChanged');
+      it('allows calling with value if sender is not the admin', async function () {
+        const receipt = await this.proxy.upgradeTo(this.clashingImplV1, { from: anotherAccount, value: 1 });
+        expectEvent.notEmitted(receipt, 'Upgraded');
         expectEvent.inTransaction(receipt.tx, this.clashing, 'ClashingImplementationCall');
       });
     });

From 223c8a79862598a9d59bbd8a76de6191f5e42ae0 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Thu, 15 Jun 2023 12:56:34 -0600
Subject: [PATCH 02/15] Add changeset

---
 .changeset/happy-falcons-walk.md | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 .changeset/happy-falcons-walk.md

diff --git a/.changeset/happy-falcons-walk.md b/.changeset/happy-falcons-walk.md
new file mode 100644
index 00000000000..b339928df78
--- /dev/null
+++ b/.changeset/happy-falcons-walk.md
@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`TransparentUpgradeableProxy`: Added an immutable admin set during construction to avoid unnecessary storage reads on every proxy call, and removed the ability to change the admin from both proxy and `ProxyAdmin`

From e102ddb81472ef06188cb5d094cca64ed5b0f82d Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Thu, 15 Jun 2023 12:59:46 -0600
Subject: [PATCH 03/15] Add solhint note

---
 contracts/mocks/DummyImplementation.sol | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contracts/mocks/DummyImplementation.sol b/contracts/mocks/DummyImplementation.sol
index 56bada28726..6466b73e6d6 100644
--- a/contracts/mocks/DummyImplementation.sol
+++ b/contracts/mocks/DummyImplementation.sol
@@ -52,6 +52,7 @@ contract DummyImplementation {
     }
 
     // Use for forcing an unsafe TransparentUpgradeableProxy admin override
+    // solhint-disable-next-line private-vars-leading-underscore
     function _unsafeOverrideAdmin(address newAdmin) public {
         StorageSlot.getAddressSlot(_ADMIN_SLOT).value = newAdmin;
     }

From 456d6eeeaab3dfc703efbc88fc0360fa12ab27e2 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Thu, 15 Jun 2023 13:00:13 -0600
Subject: [PATCH 04/15] Codespell

---
 contracts/proxy/transparent/TransparentUpgradeableProxy.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
index 3ba82dbd9ce..afe0b033dc0 100644
--- a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
+++ b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
@@ -103,7 +103,7 @@ contract TransparentUpgradeableProxy is ERC1967Proxy {
     /**
      * @dev Returns the current immutable admin.
      *
-     * Overrides ERC1967's admin in favor of an immutable value to avoid unnecesary SLOADs on each proxy call.
+     * Overrides ERC1967's admin in favor of an immutable value to avoid unnecessary SLOADs on each proxy call.
      */
     function _getAdmin() internal view virtual override returns (address) {
         return _admin;

From 1fda92ac76d56dc461d615fe426be0bc1d2f6585 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= <ernestognw@gmail.com>
Date: Thu, 15 Jun 2023 13:15:38 -0600
Subject: [PATCH 05/15] Update
 contracts/proxy/transparent/TransparentUpgradeableProxy.sol

Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
---
 contracts/proxy/transparent/TransparentUpgradeableProxy.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
index afe0b033dc0..82e5517b887 100644
--- a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
+++ b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
@@ -57,7 +57,7 @@ interface ITransparentUpgradeableProxy is IERC1967 {
 contract TransparentUpgradeableProxy is ERC1967Proxy {
     // An immutable address for the admin avoid unnecessary SLOADs before each call
     // at the expense of removing the ability to change the admin once it's set.
-    address immutable _admin;
+    address private immutable _admin;
 
     /**
      * @dev The proxy caller is the current admin, and can't fallback to the proxy target.

From 4af8379d6937ab6929b554908b5e547281ed8542 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Thu, 15 Jun 2023 16:30:28 -0600
Subject: [PATCH 06/15] Apply suggestions

Co-authored-by: Francisco <fg@frang.io>
---
 .changeset/happy-falcons-walk.md                   |  2 +-
 .../transparent/TransparentUpgradeableProxy.sol    |  7 ++++---
 .../TransparentUpgradeableProxy.behaviour.js       | 14 ++++++++++++--
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/.changeset/happy-falcons-walk.md b/.changeset/happy-falcons-walk.md
index b339928df78..0ffa5ea50c1 100644
--- a/.changeset/happy-falcons-walk.md
+++ b/.changeset/happy-falcons-walk.md
@@ -2,4 +2,4 @@
 'openzeppelin-solidity': major
 ---
 
-`TransparentUpgradeableProxy`: Added an immutable admin set during construction to avoid unnecessary storage reads on every proxy call, and removed the ability to change the admin from both proxy and `ProxyAdmin`
+`TransparentUpgradeableProxy`: Added an immutable admin set during construction to avoid unnecessary storage reads on every proxy call, and removed the ability to change the admin from both proxy and `ProxyAdmin`.
diff --git a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
index b1e1631ff55..2d121d8999b 100644
--- a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
+++ b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
@@ -31,9 +31,9 @@ interface ITransparentUpgradeableProxy is IERC1967 {
  * implementation. If the admin tries to call a function on the implementation it will fail with an error that says
  * "admin cannot fallback to proxy target".
  *
- * These properties mean that the admin account can only be used for admin actions like upgrading the proxy or changing
- * the admin, so it's best if it's a dedicated account that is not used for anything else. This will avoid headaches due
- * to sudden errors when trying to call a function from the proxy implementation.
+ * These properties mean that the admin account can only be used for upgrading the proxy, so it's best if it's a dedicated
+ * account that is not used for anything else. This will avoid headaches due to sudden errors when trying to call a function
+ * from the proxy implementation.
  *
  * Our recommendation is for the dedicated account to be an instance of the {ProxyAdmin} contract. If set up this way,
  * you should think of the `ProxyAdmin` instance as the real administrative interface of your proxy.
@@ -75,6 +75,7 @@ contract TransparentUpgradeableProxy is ERC1967Proxy {
      */
     constructor(address _logic, address admin_, bytes memory _data) payable ERC1967Proxy(_logic, _data) {
         _admin = admin_;
+        // Set the storage value and emit an event for ERC-1967 compatibility
         ERC1967Utils.changeAdmin(admin_);
     }
 
diff --git a/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js b/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js
index 1ba43eb97c6..fce47d26eb3 100644
--- a/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js
+++ b/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js
@@ -47,8 +47,18 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
     });
   });
 
-  describe('proxyAdmin', function () {
-    it('sets the admin in the storage ', async function () {
+  describe('proxy admin', function () {
+    it('emits AdminChanged event during construction', async function () {
+      const proxy = await createProxy(this.implementationV0, proxyAdminAddress, Buffer.from(''), {
+        from: proxyAdminOwner,
+      });
+      expectEvent.inConstruction(proxy, 'AdminChanged', {
+        previousAdmin: ZERO_ADDRESS,
+        newAdmin: proxyAdminAddress,
+      });
+    });
+
+    it('sets the admin in the storage and emits AdminChanged', async function () {
       expect(await getAddressInSlot(this.proxy, AdminSlot)).to.be.equal(proxyAdminAddress);
     });
 

From f8d926f1ac6a8ce3cea66b12c51ff9ff17076dee Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Thu, 15 Jun 2023 16:38:00 -0600
Subject: [PATCH 07/15] Doc changes

---
 contracts/proxy/README.adoc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/proxy/README.adoc b/contracts/proxy/README.adoc
index 43b7f65b134..3c4a78d1905 100644
--- a/contracts/proxy/README.adoc
+++ b/contracts/proxy/README.adoc
@@ -16,7 +16,7 @@ In order to avoid clashes with the storage variables of the implementation contr
 
 There are two alternative ways to add upgradeability to an ERC1967 proxy. Their differences are explained below in <<transparent-vs-uups>>.
 
-- {TransparentUpgradeableProxy}: A proxy with a built in admin and upgrade interface.
+- {TransparentUpgradeableProxy}: A proxy with a built-in immutable admin and upgrade interface.
 - {UUPSUpgradeable}: An upgradeability mechanism to be included in the implementation contract.
 
 CAUTION: Using upgradeable proxies correctly and securely is a difficult task that requires deep knowledge of the proxy pattern, Solidity, and the EVM. Unless you want a lot of low level control, we recommend using the xref:upgrades-plugins::index.adoc[OpenZeppelin Upgrades Plugins] for Truffle and Hardhat.

From b8248292ffc894ad6bfae84698de34d1714239ce Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Thu, 15 Jun 2023 16:43:21 -0600
Subject: [PATCH 08/15] Improvements to the docs

---
 .../proxy/transparent/TransparentUpgradeableProxy.sol    | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
index 2d121d8999b..27d64fbf99f 100644
--- a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
+++ b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
@@ -18,7 +18,7 @@ interface ITransparentUpgradeableProxy is IERC1967 {
 }
 
 /**
- * @dev This contract implements a proxy that is upgradeable by an admin.
+ * @dev This contract implements a proxy that is upgradeable by an immutable admin.
  *
  * To avoid https://medium.com/nomic-labs-blog/malicious-backdoors-in-ethereum-proxies-62629adf3357[proxy selector
  * clashing], which can potentially be used in an attack, this contract uses the
@@ -28,15 +28,16 @@ interface ITransparentUpgradeableProxy is IERC1967 {
  * 1. If any account other than the admin calls the proxy, the call will be forwarded to the implementation, even if
  * that call matches one of the admin functions exposed by the proxy itself.
  * 2. If the admin calls the proxy, it can access the admin functions, but its calls will never be forwarded to the
- * implementation. If the admin tries to call a function on the implementation it will fail with an error that says
- * "admin cannot fallback to proxy target".
+ * implementation. If the admin tries to call a function on the implementation it will fail with an error indicating the
+ * proxy admin cannot fallback to the target implementation.
  *
  * These properties mean that the admin account can only be used for upgrading the proxy, so it's best if it's a dedicated
  * account that is not used for anything else. This will avoid headaches due to sudden errors when trying to call a function
  * from the proxy implementation.
  *
  * Our recommendation is for the dedicated account to be an instance of the {ProxyAdmin} contract. If set up this way,
- * you should think of the `ProxyAdmin` instance as the real administrative interface of your proxy.
+ * you should think of the `ProxyAdmin` instance as the real administrative interface of your proxy, which includes the
+ * {Ownable} functions to allow for changing the admin of the proxy.
  *
  * NOTE: The real interface of this proxy is that defined in `ITransparentUpgradeableProxy`. This contract does not
  * inherit from that interface, and instead the admin functions are implicitly implemented using a custom dispatch

From b04e3430044616d99a19a5070a01cb6dcd36682d Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Thu, 15 Jun 2023 16:48:58 -0600
Subject: [PATCH 09/15] Simplified test

---
 .../transparent/TransparentUpgradeableProxy.behaviour.js     | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js b/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js
index fce47d26eb3..a6143c370c0 100644
--- a/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js
+++ b/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js
@@ -49,10 +49,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
 
   describe('proxy admin', function () {
     it('emits AdminChanged event during construction', async function () {
-      const proxy = await createProxy(this.implementationV0, proxyAdminAddress, Buffer.from(''), {
-        from: proxyAdminOwner,
-      });
-      expectEvent.inConstruction(proxy, 'AdminChanged', {
+      expectEvent.inConstruction(this.proxy, 'AdminChanged', {
         previousAdmin: ZERO_ADDRESS,
         newAdmin: proxyAdminAddress,
       });

From 04a087dd86d129c8617f463b6f711b653ec4f10d Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Thu, 15 Jun 2023 16:50:45 -0600
Subject: [PATCH 10/15] Reword

---
 contracts/proxy/transparent/TransparentUpgradeableProxy.sol | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
index 27d64fbf99f..51a1b975efb 100644
--- a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
+++ b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
@@ -36,8 +36,8 @@ interface ITransparentUpgradeableProxy is IERC1967 {
  * from the proxy implementation.
  *
  * Our recommendation is for the dedicated account to be an instance of the {ProxyAdmin} contract. If set up this way,
- * you should think of the `ProxyAdmin` instance as the real administrative interface of your proxy, which includes the
- * {Ownable} functions to allow for changing the admin of the proxy.
+ * you should think of the `ProxyAdmin` instance as the real administrative interface of your proxy, which extends from the
+ * {Ownable} contract to allow for changing the proxy's admin owner.
  *
  * NOTE: The real interface of this proxy is that defined in `ITransparentUpgradeableProxy`. This contract does not
  * inherit from that interface, and instead the admin functions are implicitly implemented using a custom dispatch

From 479fd78eebe549b546b4002dc9a6ad792dd7a340 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= <ernestognw@gmail.com>
Date: Fri, 16 Jun 2023 10:30:15 -0600
Subject: [PATCH 11/15] Update .changeset/happy-falcons-walk.md

Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
---
 .changeset/happy-falcons-walk.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.changeset/happy-falcons-walk.md b/.changeset/happy-falcons-walk.md
index 0ffa5ea50c1..d926d38e56e 100644
--- a/.changeset/happy-falcons-walk.md
+++ b/.changeset/happy-falcons-walk.md
@@ -2,4 +2,4 @@
 'openzeppelin-solidity': major
 ---
 
-`TransparentUpgradeableProxy`: Added an immutable admin set during construction to avoid unnecessary storage reads on every proxy call, and removed the ability to change the admin from both proxy and `ProxyAdmin`.
+`TransparentUpgradeableProxy`: Admin is now stored in an immutable variable (set during construction) to avoid unnecessary storage reads on every proxy call. This removed the ability to ever change the admin. Transfer of the upgrade capability is exclusively handled though the ownership of the `ProxyAdmin`.

From 6df9005fd32b1cbc18ccb4662ef5fcd264e319e9 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Fri, 16 Jun 2023 10:31:37 -0600
Subject: [PATCH 12/15] Rename unsafeOverrideAdmin

---
 contracts/mocks/DummyImplementation.sol                        | 3 +--
 .../proxy/transparent/TransparentUpgradeableProxy.behaviour.js | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/contracts/mocks/DummyImplementation.sol b/contracts/mocks/DummyImplementation.sol
index 3c12c6d8ccd..71761a75561 100644
--- a/contracts/mocks/DummyImplementation.sol
+++ b/contracts/mocks/DummyImplementation.sol
@@ -48,8 +48,7 @@ contract DummyImplementation {
     }
 
     // Use for forcing an unsafe TransparentUpgradeableProxy admin override
-    // solhint-disable-next-line private-vars-leading-underscore
-    function _unsafeOverrideAdmin(address newAdmin) public {
+    function unsafeOverrideAdmin(address newAdmin) public {
         StorageSlot.getAddressSlot(ERC1967Utils.ADMIN_SLOT).value = newAdmin;
     }
 }
diff --git a/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js b/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js
index a6143c370c0..d7449595cef 100644
--- a/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js
+++ b/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js
@@ -61,7 +61,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
 
     it('can overwrite the admin by the implementation', async function () {
       const dummy = new DummyImplementation(this.proxyAddress);
-      await dummy._unsafeOverrideAdmin(anotherAccount);
+      await dummy.unsafeOverrideAdmin(anotherAccount);
       const ERC1967AdminSlotValue = await getAddressInSlot(this.proxy, AdminSlot);
       expect(ERC1967AdminSlotValue).to.be.equal(anotherAccount);
 

From 8926ea74d8ed38441aad481e5844828ffff02eee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= <ernestognw@gmail.com>
Date: Fri, 16 Jun 2023 10:33:20 -0600
Subject: [PATCH 13/15] Update .changeset/happy-falcons-walk.md

---
 .changeset/happy-falcons-walk.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.changeset/happy-falcons-walk.md b/.changeset/happy-falcons-walk.md
index d926d38e56e..bba9642aabf 100644
--- a/.changeset/happy-falcons-walk.md
+++ b/.changeset/happy-falcons-walk.md
@@ -2,4 +2,4 @@
 'openzeppelin-solidity': major
 ---
 
-`TransparentUpgradeableProxy`: Admin is now stored in an immutable variable (set during construction) to avoid unnecessary storage reads on every proxy call. This removed the ability to ever change the admin. Transfer of the upgrade capability is exclusively handled though the ownership of the `ProxyAdmin`.
+`TransparentUpgradeableProxy`: Admin is now stored in an immutable variable (set during construction) to avoid unnecessary storage reads on every proxy call. This removed the ability to ever change the admin. Transfer of the upgrade capability is exclusively handled through the ownership of the `ProxyAdmin`.

From 9ce6f4bdb3b975cf22160b3d86dea2c44a5935b9 Mon Sep 17 00:00:00 2001
From: Francisco <fg@frang.io>
Date: Mon, 19 Jun 2023 23:18:49 -0300
Subject: [PATCH 14/15] fix test description

---
 test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js b/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js
index d7449595cef..1a03b84db5c 100644
--- a/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js
+++ b/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js
@@ -55,7 +55,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
       });
     });
 
-    it('sets the admin in the storage and emits AdminChanged', async function () {
+    it('sets the admin in the storage', async function () {
       expect(await getAddressInSlot(this.proxy, AdminSlot)).to.be.equal(proxyAdminAddress);
     });
 

From 57e68696974e331fefad5130c74f4dbd091fe3a3 Mon Sep 17 00:00:00 2001
From: Francisco <fg@frang.io>
Date: Mon, 19 Jun 2023 23:21:55 -0300
Subject: [PATCH 15/15] extend comment with further explanation

---
 contracts/proxy/transparent/TransparentUpgradeableProxy.sol | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
index 693d78a5a28..71ce665d0c1 100644
--- a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
+++ b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
@@ -59,6 +59,8 @@ interface ITransparentUpgradeableProxy is IERC1967 {
 contract TransparentUpgradeableProxy is ERC1967Proxy {
     // An immutable address for the admin avoid unnecessary SLOADs before each call
     // at the expense of removing the ability to change the admin once it's set.
+    // This is acceptable if the admin is always a ProxyAdmin instance or similar contract
+    // with its own ability to transfer the permissions to another account.
     address private immutable _admin;
 
     /**