Make TransparentUpgradeableProxy deploy its ProxyAdmin and optimize proxy interfaces (#4382)

.changeset/empty-taxis-kiss.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`UUPSUpgradeable`, `ProxyAdmin`: Removed `upgradeTo` and `upgrade` functions, and made `upgradeToAndCall` and `upgradeAndCall` ignore the data argument if it is empty. It is no longer possible to invoke the receive function along with an upgrade.

.changeset/tender-shirts-turn.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`BeaconProxy`: Reject value in initialization unless a payable function is explicitly invoked.

contracts/mocks/proxy/ClashingImplementation.sol

@@ -9,7 +9,7 @@ pragma solidity ^0.8.19;
 contract ClashingImplementation {
     event ClashingImplementationCall();
 
-    function upgradeTo(address) external payable {
+    function upgradeToAndCall(address, bytes calldata) external payable {
         emit ClashingImplementationCall();
     }
 

contracts/mocks/proxy/UUPSUpgradeableMock.sol

@@ -23,12 +23,8 @@ contract UUPSUpgradeableMock is NonUpgradeableMock, UUPSUpgradeable {
 }
 
 contract UUPSUpgradeableUnsafeMock is UUPSUpgradeableMock {
-    function upgradeTo(address newImplementation) public override {
-        ERC1967Utils.upgradeToAndCall(newImplementation, bytes(""), false);
-    }
-
     function upgradeToAndCall(address newImplementation, bytes memory data) public payable override {
-        ERC1967Utils.upgradeToAndCall(newImplementation, data, false);
+        ERC1967Utils.upgradeToAndCall(newImplementation, data);
     }
 }
 

contracts/proxy/ERC1967/ERC1967Proxy.sol

@@ -20,7 +20,7 @@ contract ERC1967Proxy is Proxy {
      * function call, and allows initializing the storage of the proxy like a Solidity constructor.
      */
     constructor(address _logic, bytes memory _data) payable {
-        ERC1967Utils.upgradeToAndCall(_logic, _data, false);
+        ERC1967Utils.upgradeToAndCall(_logic, _data);
     }
 
     /**

contracts/proxy/ERC1967/ERC1967Utils.sol

@@ -52,6 +52,11 @@ library ERC1967Utils {
      */
     error ERC1967InvalidBeacon(address beacon);
 
+    /**
+     * @dev An upgrade function sees `msg.value > 0` that may be lost.
+     */
+    error ERC1967NonPayable();
+
     /**
      * @dev Returns the current implementation address.
      */
@@ -69,25 +74,19 @@ library ERC1967Utils {
         StorageSlot.getAddressSlot(IMPLEMENTATION_SLOT).value = newImplementation;
     }
 
-    /**
-     * @dev Perform implementation upgrade
-     *
-     * Emits an {IERC1967-Upgraded} event.
-     */
-    function upgradeTo(address newImplementation) internal {
-        _setImplementation(newImplementation);
-        emit Upgraded(newImplementation);
-    }
-
     /**
      * @dev Perform implementation upgrade with additional setup call.
      *
      * Emits an {IERC1967-Upgraded} event.
      */
-    function upgradeToAndCall(address newImplementation, bytes memory data, bool forceCall) internal {
-        upgradeTo(newImplementation);
-        if (data.length > 0 || forceCall) {
+    function upgradeToAndCall(address newImplementation, bytes memory data) internal {
+        _setImplementation(newImplementation);
+        emit Upgraded(newImplementation);
+
+        if (data.length > 0) {
             Address.functionDelegateCall(newImplementation, data);
+        } else {
+            _checkNonPayable();
         }
     }
 
@@ -161,7 +160,7 @@ library ERC1967Utils {
     }
 
     /**
-     * @dev Change the beacon and trigger a setup call.
+     * @dev Change the beacon and trigger a setup call if data is nonempty.
      *
      * Emits an {IERC1967-BeaconUpgraded} event.
      *
@@ -169,11 +168,20 @@ library ERC1967Utils {
      * it uses an immutable beacon without looking at the value of the ERC-1967 beacon slot for
      * efficiency.
      */
-    function upgradeBeaconToAndCall(address newBeacon, bytes memory data, bool forceCall) internal {
+    function upgradeBeaconToAndCall(address newBeacon, bytes memory data) internal {
         _setBeacon(newBeacon);
         emit BeaconUpgraded(newBeacon);
-        if (data.length > 0 || forceCall) {
+
+        if (data.length > 0) {
             Address.functionDelegateCall(IBeacon(newBeacon).implementation(), data);
+        } else {
+            _checkNonPayable();
+        }
+    }
+
+    function _checkNonPayable() private {
+        if (msg.value > 0) {
+            revert ERC1967NonPayable();
         }
     }
 }

contracts/proxy/beacon/BeaconProxy.sol

@@ -36,7 +36,7 @@ contract BeaconProxy is Proxy {
      * - `beacon` must be a contract with the interface {IBeacon}.
      */
     constructor(address beacon, bytes memory data) payable {
-        ERC1967Utils.upgradeBeaconToAndCall(beacon, data, false);
+        ERC1967Utils.upgradeBeaconToAndCall(beacon, data);
         _beacon = beacon;
     }
 

contracts/proxy/transparent/ProxyAdmin.sol

@@ -3,7 +3,7 @@
 
 pragma solidity ^0.8.19;
 
-import {ITransparentUpgradeableProxy, TransparentUpgradeableProxy} from "./TransparentUpgradeableProxy.sol";
+import {ITransparentUpgradeableProxy} from "./TransparentUpgradeableProxy.sol";
 import {Ownable} from "../../access/Ownable.sol";
 
 /**
@@ -11,25 +11,25 @@ import {Ownable} from "../../access/Ownable.sol";
  * explanation of why you would want to use this see the documentation for {TransparentUpgradeableProxy}.
  */
 contract ProxyAdmin is Ownable {
+    /**
+     * @dev The version of the upgrade interface of the contract. If this getter is missing, both `upgrade(address)`
+     * and `upgradeAndCall(address,bytes)` are present, and `upgradeTo` must be used if no function should be called,
+     * while `upgradeAndCall` will invoke the `receive` function if the second argument is the empty byte string.
+     * If the getter returns `"5.0.0"`, only `upgradeAndCall(address,bytes)` is present, and the second argument must
+     * be the empty byte string if no function should be called, being impossible to invoke the `receive` function
+     * during an upgrade.
+     */
+    // solhint-disable-next-line private-vars-leading-underscore
+    string internal constant UPGRADE_INTERFACE_VERSION = "5.0.0";
+
     /**
      * @dev Sets the initial owner who can perform upgrades.
      */
     constructor(address initialOwner) Ownable(initialOwner) {}
 
     /**
-     * @dev Upgrades `proxy` to `implementation`. See {TransparentUpgradeableProxy-upgradeTo}.
-     *
-     * Requirements:
-     *
-     * - This contract must be the admin of `proxy`.
-     */
-    function upgrade(ITransparentUpgradeableProxy proxy, address implementation) public virtual onlyOwner {
-        proxy.upgradeTo(implementation);
-    }
-
-    /**
-     * @dev Upgrades `proxy` to `implementation` and calls a function on the new implementation. See
-     * {TransparentUpgradeableProxy-upgradeToAndCall}.
+     * @dev Upgrades `proxy` to `implementation` and calls a function on the new implementation.
+     * See {TransparentUpgradeableProxy-_dispatchUpgradeToAndCall}.
      *
      * Requirements:
      *

contracts/proxy/transparent/TransparentUpgradeableProxy.sol

@@ -6,21 +6,20 @@ pragma solidity ^0.8.19;
 import {ERC1967Utils} from "../ERC1967/ERC1967Utils.sol";
 import {ERC1967Proxy} from "../ERC1967/ERC1967Proxy.sol";
 import {IERC1967} from "../../interfaces/IERC1967.sol";
+import {ProxyAdmin} from "./ProxyAdmin.sol";
 
 /**
  * @dev Interface for {TransparentUpgradeableProxy}. In order to implement transparency, {TransparentUpgradeableProxy}
- * does not implement this interface directly, and some of its functions are implemented by an internal dispatch
+ * does not implement this interface directly, and its upgradeability mechanism is implemented by an internal dispatch
  * mechanism. The compiler is unaware that these functions are implemented by {TransparentUpgradeableProxy} and will not
  * include them in the ABI so this interface must be used to interact with it.
  */
 interface ITransparentUpgradeableProxy is IERC1967 {
-    function upgradeTo(address) external;
-
-    function upgradeToAndCall(address, bytes memory) external payable;
+    function upgradeToAndCall(address, bytes calldata) external payable;
 }
 
 /**
- * @dev This contract implements a proxy that is upgradeable by an immutable admin.
+ * @dev This contract implements a proxy that is upgradeable through an associated {ProxyAdmin} instance.
  *
  * 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,23 +27,22 @@ interface ITransparentUpgradeableProxy is IERC1967 {
  * things that go hand in hand:
  *
  * 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
+ * that call matches the {ITransparentUpgradeableProxy-upgradeToAndCall} function exposed by the proxy itself.
+ * 2. If the admin calls the proxy, it can call the `upgradeToAndCall` function but any other call won't be forwarded to the
  * 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, which extends from the
- * {Ownable} contract to allow for changing the proxy's admin owner.
+ * from the proxy implementation. For this reason, the proxy deploys an instance of {ProxyAdmin} and allows upgrades
+ * only if they come through it.
+ * You should think of the `ProxyAdmin` instance as the administrative interface of the proxy, including the ability to
+ * change who can trigger upgrades by transferring ownership.
  *
  * 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
- * mechanism in `_fallback`. Consequently, the compiler will not produce an ABI for this contract. This is necessary to
- * fully implement transparency without decoding reverts caused by selector clashes between the proxy and the
+ * inherit from that interface, and instead `upgradeToAndCall` is implicitly implemented using a custom dispatch mechanism
+ * in `_fallback`. Consequently, the compiler will not produce an ABI for this contract. This is necessary to 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,
@@ -55,10 +53,10 @@ interface ITransparentUpgradeableProxy is IERC1967 {
  * 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.
+ * render the `upgradeToAndCall` function inaccessible, preventing upgradeability and compromising transparency.
  */
 contract TransparentUpgradeableProxy is ERC1967Proxy {
-    // An immutable address for the admin avoid unnecessary SLOADs before each call
+    // An immutable address for the admin to 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.
@@ -69,19 +67,14 @@ contract TransparentUpgradeableProxy is ERC1967Proxy {
      */
     error ProxyDeniedAdminAccess();
 
-    /**
-     * @dev msg.value is not 0.
-     */
-    error ProxyNonPayableFunction();
-
     /**
      * @dev Initializes an upgradeable proxy managed by `_admin`, backed by the implementation at `_logic`, and
      * optionally initialized with `_data` as explained in {ERC1967Proxy-constructor}.
      */
-    constructor(address _logic, address admin_, bytes memory _data) payable ERC1967Proxy(_logic, _data) {
-        _admin = admin_;
+    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(admin_);
+        ERC1967Utils.changeAdmin(_admin);
     }
 
     /**
@@ -89,18 +82,11 @@ contract TransparentUpgradeableProxy is ERC1967Proxy {
      */
     function _fallback() internal virtual override {
         if (msg.sender == _admin) {
-            bytes memory ret;
-            bytes4 selector = msg.sig;
-            if (selector == ITransparentUpgradeableProxy.upgradeTo.selector) {
-                ret = _dispatchUpgradeTo();
-            } else if (selector == ITransparentUpgradeableProxy.upgradeToAndCall.selector) {
-                ret = _dispatchUpgradeToAndCall();
+            if (msg.sig == ITransparentUpgradeableProxy.upgradeToAndCall.selector) {
+                _dispatchUpgradeToAndCall();
             } else {
                 revert ProxyDeniedAdminAccess();
             }
-            assembly {
-                return(add(ret, 0x20), mload(ret))
-            }
         } else {
             super._fallback();
         }
@@ -109,34 +95,8 @@ contract TransparentUpgradeableProxy is ERC1967Proxy {
     /**
      * @dev Upgrade the implementation of the proxy.
      */
-    function _dispatchUpgradeTo() private returns (bytes memory) {
-        _requireZeroValue();
-
-        address newImplementation = abi.decode(msg.data[4:], (address));
-        ERC1967Utils.upgradeToAndCall(newImplementation, bytes(""), false);
-
-        return "";
-    }
-
-    /**
-     * @dev Upgrade the implementation of the proxy, and then call a function from the new implementation as specified
-     * by `data`, which should be an encoded function call. This is useful to initialize new storage variables in the
-     * proxied contract.
-     */
-    function _dispatchUpgradeToAndCall() private returns (bytes memory) {
+    function _dispatchUpgradeToAndCall() private {
         (address newImplementation, bytes memory data) = abi.decode(msg.data[4:], (address, bytes));
-        ERC1967Utils.upgradeToAndCall(newImplementation, data, true);
-
-        return "";
-    }
-
-    /**
-     * @dev To keep this contract fully transparent, the fallback is payable. This helper is here to enforce
-     * non-payability of function implemented through dispatchers while still allowing value to pass through.
-     */
-    function _requireZeroValue() private {
-        if (msg.value != 0) {
-            revert ProxyNonPayableFunction();
-        }
+        ERC1967Utils.upgradeToAndCall(newImplementation, data);
     }
 }

contracts/proxy/utils/UUPSUpgradeable.sol

@@ -20,6 +20,17 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable {
     /// @custom:oz-upgrades-unsafe-allow state-variable-immutable state-variable-assignment
     address private immutable __self = address(this);
 
+    /**
+     * @dev The version of the upgrade interface of the contract. If this getter is missing, both `upgradeTo(address)`
+     * and `upgradeToAndCall(address,bytes)` are present, and `upgradeTo` must be used if no function should be called,
+     * while `upgradeToAndCall` will invoke the `receive` function if the second argument is the empty byte string.
+     * If the getter returns `"5.0.0"`, only `upgradeToAndCall(address,bytes)` is present, and the second argument must
+     * be the empty byte string if no function should be called, being impossible to invoke the `receive` function
+     * during an upgrade.
+     */
+    // solhint-disable-next-line private-vars-leading-underscore
+    string internal constant UPGRADE_INTERFACE_VERSION = "5.0.0";
+
     /**
      * @dev The call is from an unauthorized context.
      */
@@ -71,20 +82,6 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable {
         return ERC1967Utils.IMPLEMENTATION_SLOT;
     }
 
-    /**
-     * @dev Upgrade the implementation of the proxy to `newImplementation`.
-     *
-     * Calls {_authorizeUpgrade}.
-     *
-     * Emits an {Upgraded} event.
-     *
-     * @custom:oz-upgrades-unsafe-allow-reachable delegatecall
-     */
-    function upgradeTo(address newImplementation) public virtual onlyProxy {
-        _authorizeUpgrade(newImplementation);
-        _upgradeToAndCallUUPS(newImplementation, new bytes(0), false);
-    }
-
     /**
      * @dev Upgrade the implementation of the proxy to `newImplementation`, and subsequently execute the function call
      * encoded in `data`.
@@ -97,17 +94,17 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable {
      */
     function upgradeToAndCall(address newImplementation, bytes memory data) public payable virtual onlyProxy {
         _authorizeUpgrade(newImplementation);
-        _upgradeToAndCallUUPS(newImplementation, data, true);
+        _upgradeToAndCallUUPS(newImplementation, data);
     }
 
     /**
      * @dev Function that should revert when `msg.sender` is not authorized to upgrade the contract. Called by
-     * {upgradeTo} and {upgradeToAndCall}.
+     * {upgradeToAndCall}.
      *
      * Normally, this function will use an xref:access.adoc[access control] modifier such as {Ownable-onlyOwner}.
      *
      * ```solidity
-     * function _authorizeUpgrade(address) internal  onlyOwner {}
+     * function _authorizeUpgrade(address) internal onlyOwner {}
      * ```
      */
     function _authorizeUpgrade(address newImplementation) internal virtual;
@@ -117,12 +114,12 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable {
      *
      * Emits an {IERC1967-Upgraded} event.
      */
-    function _upgradeToAndCallUUPS(address newImplementation, bytes memory data, bool forceCall) private {
+    function _upgradeToAndCallUUPS(address newImplementation, bytes memory data) private {
         try IERC1822Proxiable(newImplementation).proxiableUUID() returns (bytes32 slot) {
             if (slot != ERC1967Utils.IMPLEMENTATION_SLOT) {
                 revert UUPSUnsupportedProxiableUUID(slot);
             }
-            ERC1967Utils.upgradeToAndCall(newImplementation, data, forceCall);
+            ERC1967Utils.upgradeToAndCall(newImplementation, data);
         } catch {
             // The implementation is not UUPS
             revert ERC1967Utils.ERC1967InvalidImplementation(newImplementation);

test/governance/compatibility/GovernorCompatibilityBravo.test.js

@@ -1,6 +1,6 @@
 const { expectEvent } = require('@openzeppelin/test-helpers');
 const { expect } = require('chai');
-const RLP = require('rlp');
+const { computeCreateAddress } = require('../../helpers/create');
 const Enums = require('../../helpers/enums');
 const { GovernorHelper } = require('../../helpers/governance');
 const { clockFromReceipt } = require('../../helpers/time');
@@ -12,15 +12,6 @@ const CallReceiver = artifacts.require('CallReceiverMock');
 
 const { shouldBehaveLikeEIP6372 } = require('../utils/EIP6372.behavior');
 
-function makeContractAddress(creator, nonce) {
-  return web3.utils.toChecksumAddress(
-    web3.utils
-      .sha3(RLP.encode([creator, nonce]))
-      .slice(12)
-      .substring(14),
-  );
-}
-
 const TOKENS = [
   { Token: artifacts.require('$ERC20Votes'), mode: 'blocknumber' },
   { Token: artifacts.require('$ERC20VotesTimestampMock'), mode: 'timestamp' },
@@ -58,7 +49,7 @@ contract('GovernorCompatibilityBravo', function (accounts) {
 
         // Need to predict governance address to set it as timelock admin with a delayed transfer
         const nonce = await web3.eth.getTransactionCount(deployer);
-        const predictGovernor = makeContractAddress(deployer, nonce + 1);
+        const predictGovernor = computeCreateAddress(deployer, nonce + 1);
 
         this.timelock = await Timelock.new(predictGovernor, 2 * 86400);
         this.mock = await Governor.new(

test/governance/extensions/GovernorTimelockCompound.test.js

@@ -1,10 +1,10 @@
 const { constants, expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
 const { expect } = require('chai');
-const RLP = require('rlp');
 
 const Enums = require('../../helpers/enums');
 const { GovernorHelper, proposalStatesToBitMap } = require('../../helpers/governance');
 const { expectRevertCustomError } = require('../../helpers/customError');
+const { computeCreateAddress } = require('../../helpers/create');
 
 const { shouldSupportInterfaces } = require('../../utils/introspection/SupportsInterface.behavior');
 
@@ -14,15 +14,6 @@ const CallReceiver = artifacts.require('CallReceiverMock');
 const ERC721 = artifacts.require('$ERC721');
 const ERC1155 = artifacts.require('$ERC1155');
 
-function makeContractAddress(creator, nonce) {
-  return web3.utils.toChecksumAddress(
-    web3.utils
-      .sha3(RLP.encode([creator, nonce]))
-      .slice(12)
-      .substring(14),
-  );
-}
-
 const TOKENS = [
   { Token: artifacts.require('$ERC20Votes'), mode: 'blocknumber' },
   { Token: artifacts.require('$ERC20VotesTimestampMock'), mode: 'timestamp' },
@@ -49,7 +40,7 @@ contract('GovernorTimelockCompound', function (accounts) {
 
         // Need to predict governance address to set it as timelock admin with a delayed transfer
         const nonce = await web3.eth.getTransactionCount(deployer);
-        const predictGovernor = makeContractAddress(deployer, nonce + 1);
+        const predictGovernor = computeCreateAddress(deployer, nonce + 1);
 
         this.timelock = await Timelock.new(predictGovernor, 2 * 86400);
         this.mock = await Governor.new(

test/helpers/account.js

@@ -0,0 +1,14 @@
+const { web3 } = require('hardhat');
+const { impersonateAccount, setBalance } = require('@nomicfoundation/hardhat-network-helpers');
+
+// Hardhat default balance
+const DEFAULT_BALANCE = web3.utils.toBN('10000000000000000000000');
+
+async function impersonate(account, balance = DEFAULT_BALANCE) {
+  await impersonateAccount(account);
+  await setBalance(account, balance);
+}
+
+module.exports = {
+  impersonate,
+};

test/helpers/create.js

@@ -0,0 +1,23 @@
+const { rlp } = require('ethereumjs-util');
+
+function computeCreateAddress(deployer, nonce) {
+  return web3.utils.toChecksumAddress(web3.utils.sha3(rlp.encode([deployer.address ?? deployer, nonce])).slice(-40));
+}
+
+function computeCreate2Address(saltHex, bytecode, deployer) {
+  return web3.utils.toChecksumAddress(
+    web3.utils
+      .sha3(
+        '0x' +
+          ['ff', deployer.address ?? deployer, saltHex, web3.utils.soliditySha3(bytecode)]
+            .map(x => x.replace(/0x/, ''))
+            .join(''),
+      )
+      .slice(-40),
+  );
+}
+
+module.exports = {
+  computeCreateAddress,
+  computeCreate2Address,
+};

test/helpers/create2.js

@@ -1,11 +0,0 @@
-function computeCreate2Address(saltHex, bytecode, deployer) {
-  return web3.utils.toChecksumAddress(
-    `0x${web3.utils
-      .sha3(`0x${['ff', deployer, saltHex, web3.utils.soliditySha3(bytecode)].map(x => x.replace(/0x/, '')).join('')}`)
-      .slice(-40)}`,
-  );
-}
-
-module.exports = {
-  computeCreate2Address,
-};

test/proxy/Clones.test.js

@@ -1,5 +1,5 @@
 const { expectEvent } = require('@openzeppelin/test-helpers');
-const { computeCreate2Address } = require('../helpers/create2');
+const { computeCreate2Address } = require('../helpers/create');
 const { expect } = require('chai');
 
 const { expectRevertCustomError } = require('../helpers/customError');

test/proxy/ERC1967/ERC1967Proxy.test.js

@@ -3,11 +3,9 @@ const shouldBehaveLikeProxy = require('../Proxy.behaviour');
 const ERC1967Proxy = artifacts.require('ERC1967Proxy');
 
 contract('ERC1967Proxy', function (accounts) {
-  const [proxyAdminOwner] = accounts;
-
-  const createProxy = async function (implementation, _admin, initData, opts) {
+  const createProxy = async function (implementation, initData, opts) {
     return ERC1967Proxy.new(implementation, initData, opts);
   };
 
-  shouldBehaveLikeProxy(createProxy, undefined, proxyAdminOwner);
+  shouldBehaveLikeProxy(createProxy, accounts);
 });

test/proxy/Proxy.behaviour.js

@@ -2,15 +2,18 @@ const { expectRevert } = require('@openzeppelin/test-helpers');
 const { getSlot, ImplementationSlot } = require('../helpers/erc1967');
 
 const { expect } = require('chai');
+const { expectRevertCustomError } = require('../helpers/customError');
 
 const DummyImplementation = artifacts.require('DummyImplementation');
 
-module.exports = function shouldBehaveLikeProxy(createProxy, proxyAdminAddress, proxyCreator) {
+module.exports = function shouldBehaveLikeProxy(createProxy, accounts) {
+  const [proxyCreator] = accounts;
+
   it('cannot be initialized with a non-contract address', async function () {
     const nonContractAddress = proxyCreator;
     const initializeData = Buffer.from('');
     await expectRevert.unspecified(
-      createProxy(nonContractAddress, proxyAdminAddress, initializeData, {
+      createProxy(nonContractAddress, initializeData, {
         from: proxyCreator,
       }),
     );
@@ -43,7 +46,7 @@ module.exports = function shouldBehaveLikeProxy(createProxy, proxyAdminAddress,
     describe('when not sending balance', function () {
       beforeEach('creating proxy', async function () {
         this.proxy = (
-          await createProxy(this.implementation, proxyAdminAddress, initializeData, {
+          await createProxy(this.implementation, initializeData, {
             from: proxyCreator,
           })
         ).address;
@@ -55,16 +58,16 @@ module.exports = function shouldBehaveLikeProxy(createProxy, proxyAdminAddress,
     describe('when sending some balance', function () {
       const value = 10e5;
 
-      beforeEach('creating proxy', async function () {
-        this.proxy = (
-          await createProxy(this.implementation, proxyAdminAddress, initializeData, {
+      it('reverts', async function () {
+        await expectRevertCustomError(
+          createProxy(this.implementation, initializeData, {
             from: proxyCreator,
             value,
-          })
-        ).address;
+          }),
+          'ERC1967NonPayable',
+          [],
+        );
       });
-
-      assertProxyInitialization({ value: 0, balance: value });
     });
   });
 
@@ -76,7 +79,7 @@ module.exports = function shouldBehaveLikeProxy(createProxy, proxyAdminAddress,
       describe('when not sending balance', function () {
         beforeEach('creating proxy', async function () {
           this.proxy = (
-            await createProxy(this.implementation, proxyAdminAddress, initializeData, {
+            await createProxy(this.implementation, initializeData, {
               from: proxyCreator,
             })
           ).address;
@@ -93,7 +96,7 @@ module.exports = function shouldBehaveLikeProxy(createProxy, proxyAdminAddress,
 
         it('reverts', async function () {
           await expectRevert.unspecified(
-            createProxy(this.implementation, proxyAdminAddress, initializeData, { from: proxyCreator, value }),
+            createProxy(this.implementation, initializeData, { from: proxyCreator, value }),
           );
         });
       });
@@ -106,7 +109,7 @@ module.exports = function shouldBehaveLikeProxy(createProxy, proxyAdminAddress,
       describe('when not sending balance', function () {
         beforeEach('creating proxy', async function () {
           this.proxy = (
-            await createProxy(this.implementation, proxyAdminAddress, initializeData, {
+            await createProxy(this.implementation, initializeData, {
               from: proxyCreator,
             })
           ).address;
@@ -123,7 +126,7 @@ module.exports = function shouldBehaveLikeProxy(createProxy, proxyAdminAddress,
 
         beforeEach('creating proxy', async function () {
           this.proxy = (
-            await createProxy(this.implementation, proxyAdminAddress, initializeData, {
+            await createProxy(this.implementation, initializeData, {
               from: proxyCreator,
               value,
             })
@@ -148,7 +151,7 @@ module.exports = function shouldBehaveLikeProxy(createProxy, proxyAdminAddress,
       describe('when not sending balance', function () {
         beforeEach('creating proxy', async function () {
           this.proxy = (
-            await createProxy(this.implementation, proxyAdminAddress, initializeData, {
+            await createProxy(this.implementation, initializeData, {
               from: proxyCreator,
             })
           ).address;
@@ -165,7 +168,7 @@ module.exports = function shouldBehaveLikeProxy(createProxy, proxyAdminAddress,
 
         it('reverts', async function () {
           await expectRevert.unspecified(
-            createProxy(this.implementation, proxyAdminAddress, initializeData, { from: proxyCreator, value }),
+            createProxy(this.implementation, initializeData, { from: proxyCreator, value }),
           );
         });
       });
@@ -180,7 +183,7 @@ module.exports = function shouldBehaveLikeProxy(createProxy, proxyAdminAddress,
       describe('when not sending balance', function () {
         beforeEach('creating proxy', async function () {
           this.proxy = (
-            await createProxy(this.implementation, proxyAdminAddress, initializeData, {
+            await createProxy(this.implementation, initializeData, {
               from: proxyCreator,
             })
           ).address;
@@ -197,7 +200,7 @@ module.exports = function shouldBehaveLikeProxy(createProxy, proxyAdminAddress,
 
         beforeEach('creating proxy', async function () {
           this.proxy = (
-            await createProxy(this.implementation, proxyAdminAddress, initializeData, {
+            await createProxy(this.implementation, initializeData, {
               from: proxyCreator,
               value,
             })
@@ -216,7 +219,7 @@ module.exports = function shouldBehaveLikeProxy(createProxy, proxyAdminAddress,
 
       it('reverts', async function () {
         await expectRevert(
-          createProxy(this.implementation, proxyAdminAddress, initializeData, { from: proxyCreator }),
+          createProxy(this.implementation, initializeData, { from: proxyCreator }),
           'DummyImplementation reverted',
         );
       });

test/proxy/beacon/BeaconProxy.test.js

@@ -59,9 +59,8 @@ contract('BeaconProxy', function (accounts) {
 
     it('no initialization', async function () {
       const data = Buffer.from('');
-      const balance = '10';
-      this.proxy = await BeaconProxy.new(this.beacon.address, data, { value: balance });
-      await this.assertInitialized({ value: '0', balance });
+      this.proxy = await BeaconProxy.new(this.beacon.address, data);
+      await this.assertInitialized({ value: '0', balance: '0' });
     });
 
     it('non-payable initialization', async function () {
@@ -79,7 +78,16 @@ contract('BeaconProxy', function (accounts) {
       await this.assertInitialized({ value, balance });
     });
 
-    it('reverting initialization', async function () {
+    it('reverting initialization due to value', async function () {
+      const data = Buffer.from('');
+      await expectRevertCustomError(
+        BeaconProxy.new(this.beacon.address, data, { value: '1' }),
+        'ERC1967NonPayable',
+        [],
+      );
+    });
+
+    it('reverting initialization function', async function () {
       const data = this.implementationV0.contract.methods.reverts().encodeABI();
       await expectRevert(BeaconProxy.new(this.beacon.address, data), 'DummyImplementation reverted');
     });

test/proxy/transparent/ProxyAdmin.test.js

@@ -8,6 +8,7 @@ const ITransparentUpgradeableProxy = artifacts.require('ITransparentUpgradeableP
 
 const { getAddressInSlot, ImplementationSlot } = require('../../helpers/erc1967');
 const { expectRevertCustomError } = require('../../helpers/customError');
+const { computeCreateAddress } = require('../../helpers/create');
 
 contract('ProxyAdmin', function (accounts) {
   const [proxyAdminOwner, anotherAccount] = accounts;
@@ -19,12 +20,12 @@ contract('ProxyAdmin', function (accounts) {
 
   beforeEach(async function () {
     const initializeData = Buffer.from('');
-    this.proxyAdmin = await ProxyAdmin.new(proxyAdminOwner);
-    const proxy = await TransparentUpgradeableProxy.new(
-      this.implementationV1.address,
-      this.proxyAdmin.address,
-      initializeData,
-    );
+    const proxy = await TransparentUpgradeableProxy.new(this.implementationV1.address, proxyAdminOwner, initializeData);
+
+    const proxyNonce = await web3.eth.getTransactionCount(proxy.address);
+    const proxyAdminAddress = computeCreateAddress(proxy.address, proxyNonce - 1); // Nonce already used
+    this.proxyAdmin = await ProxyAdmin.at(proxyAdminAddress);
+
     this.proxy = await ITransparentUpgradeableProxy.at(proxy.address);
   });
 
@@ -32,11 +33,13 @@ contract('ProxyAdmin', function (accounts) {
     expect(await this.proxyAdmin.owner()).to.equal(proxyAdminOwner);
   });
 
-  describe('#upgrade', function () {
+  describe('without data', function () {
     context('with unauthorized account', function () {
       it('fails to upgrade', async function () {
         await expectRevertCustomError(
-          this.proxyAdmin.upgrade(this.proxy.address, this.implementationV2.address, { from: anotherAccount }),
+          this.proxyAdmin.upgradeAndCall(this.proxy.address, this.implementationV2.address, '0x', {
+            from: anotherAccount,
+          }),
           'OwnableUnauthorizedAccount',
           [anotherAccount],
         );
@@ -45,7 +48,9 @@ contract('ProxyAdmin', function (accounts) {
 
     context('with authorized account', function () {
       it('upgrades implementation', async function () {
-        await this.proxyAdmin.upgrade(this.proxy.address, this.implementationV2.address, { from: proxyAdminOwner });
+        await this.proxyAdmin.upgradeAndCall(this.proxy.address, this.implementationV2.address, '0x', {
+          from: proxyAdminOwner,
+        });
 
         const implementationAddress = await getAddressInSlot(this.proxy, ImplementationSlot);
         expect(implementationAddress).to.be.equal(this.implementationV2.address);
@@ -53,7 +58,7 @@ contract('ProxyAdmin', function (accounts) {
     });
   });
 
-  describe('#upgradeAndCall', function () {
+  describe('with data', function () {
     context('with unauthorized account', function () {
       it('fails to upgrade', async function () {
         const callData = new ImplV1('').contract.methods.initializeNonPayableWithValue(1337).encodeABI();

test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js

@@ -5,6 +5,8 @@ const { expectRevertCustomError } = require('../../helpers/customError');
 
 const { expect } = require('chai');
 const { web3 } = require('hardhat');
+const { computeCreateAddress } = require('../../helpers/create');
+const { impersonate } = require('../../helpers/account');
 
 const Implementation1 = artifacts.require('Implementation1');
 const Implementation2 = artifacts.require('Implementation2');
@@ -17,8 +19,21 @@ const InitializableMock = artifacts.require('InitializableMock');
 const DummyImplementation = artifacts.require('DummyImplementation');
 const ClashingImplementation = artifacts.require('ClashingImplementation');
 
-module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProxy, accounts) {
-  const [proxyAdminAddress, proxyAdminOwner, anotherAccount] = accounts;
+module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProxy, initialOwner, accounts) {
+  const [proxyCreator, anotherAccount] = accounts;
+
+  async function createProxyWithImpersonatedProxyAdmin(logic, initData, opts) {
+    const proxy = await createProxy(logic, initData, { from: proxyCreator, ...opts });
+
+    // Expect proxy admin to be the first and only contract created by the proxy
+    const proxyAdminAddress = computeCreateAddress(proxy.address, 1);
+    await impersonate(proxyAdminAddress);
+
+    return {
+      proxy,
+      proxyAdminAddress,
+    };
+  }
 
   before(async function () {
     this.implementationV0 = (await DummyImplementation.new()).address;
@@ -27,10 +42,12 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
 
   beforeEach(async function () {
     const initializeData = Buffer.from('');
-    this.proxy = await createProxy(this.implementationV0, proxyAdminAddress, initializeData, {
-      from: proxyAdminOwner,
-    });
-    this.proxyAddress = this.proxy.address;
+    const { proxy, proxyAdminAddress } = await createProxyWithImpersonatedProxyAdmin(
+      this.implementationV0,
+      initializeData,
+    );
+    this.proxy = proxy;
+    this.proxyAdminAddress = proxyAdminAddress;
   });
 
   describe('implementation', function () {
@@ -40,7 +57,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
     });
 
     it('delegates to the implementation', async function () {
-      const dummy = new DummyImplementation(this.proxyAddress);
+      const dummy = new DummyImplementation(this.proxy.address);
       const value = await dummy.get();
 
       expect(value).to.equal(true);
@@ -49,64 +66,31 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
 
   describe('proxy admin', function () {
     it('emits AdminChanged event during construction', async function () {
-      expectEvent.inConstruction(this.proxy, 'AdminChanged', {
+      await expectEvent.inConstruction(this.proxy, 'AdminChanged', {
         previousAdmin: ZERO_ADDRESS,
-        newAdmin: proxyAdminAddress,
+        newAdmin: this.proxyAdminAddress,
       });
     });
 
-    it('sets the admin in the storage', async function () {
-      expect(await getAddressInSlot(this.proxy, AdminSlot)).to.be.equal(proxyAdminAddress);
+    it('sets the proxy admin in storage', async function () {
+      expect(await getAddressInSlot(this.proxy, AdminSlot)).to.be.equal(this.proxyAdminAddress);
     });
 
     it('can overwrite the admin by the implementation', async function () {
-      const dummy = new DummyImplementation(this.proxyAddress);
+      const dummy = new DummyImplementation(this.proxy.address);
       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;
-
-      describe('when the given implementation is different from the current one', function () {
-        it('upgrades to the requested implementation', async function () {
-          await this.proxy.upgradeTo(this.implementationV1, { from });
-
-          const implementationAddress = await getAddressInSlot(this.proxy, ImplementationSlot);
-          expect(implementationAddress).to.be.equal(this.implementationV1);
-        });
-
-        it('emits an event', async function () {
-          expectEvent(await this.proxy.upgradeTo(this.implementationV1, { from }), 'Upgraded', {
-            implementation: this.implementationV1,
-          });
-        });
-      });
-
-      describe('when the given implementation is the zero address', function () {
-        it('reverts', async function () {
-          await expectRevertCustomError(this.proxy.upgradeTo(ZERO_ADDRESS, { from }), 'ERC1967InvalidImplementation', [
-            ZERO_ADDRESS,
-          ]);
-        });
-      });
-    });
-
-    describe('when the sender is not the admin', function () {
-      const from = anotherAccount;
-
-      it('reverts', async function () {
-        await expectRevert.unspecified(this.proxy.upgradeTo(this.implementationV1, { from }));
-      });
+      expect(ERC1967AdminSlotValue).to.not.equal(this.proxyAdminAddress);
+      expectEvent(
+        await this.proxy.upgradeToAndCall(this.implementationV1, '0x', { from: this.proxyAdminAddress }),
+        'Upgraded',
+        {
+          implementation: this.implementationV1,
+        },
+      );
     });
   });
 
@@ -120,11 +104,13 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
         const initializeData = new InitializableMock('').contract.methods['initializeWithX(uint256)'](42).encodeABI();
 
         describe('when the sender is the admin', function () {
-          const from = proxyAdminAddress;
           const value = 1e5;
 
           beforeEach(async function () {
-            this.receipt = await this.proxy.upgradeToAndCall(this.behavior.address, initializeData, { from, value });
+            this.receipt = await this.proxy.upgradeToAndCall(this.behavior.address, initializeData, {
+              from: this.proxyAdminAddress,
+              value,
+            });
           });
 
           it('upgrades to the requested implementation', async function () {
@@ -137,13 +123,13 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
           });
 
           it('calls the initializer function', async function () {
-            const migratable = new InitializableMock(this.proxyAddress);
+            const migratable = new InitializableMock(this.proxy.address);
             const x = await migratable.x();
             expect(x).to.be.bignumber.equal('42');
           });
 
           it('sends given value to the proxy', async function () {
-            const balance = await web3.eth.getBalance(this.proxyAddress);
+            const balance = await web3.eth.getBalance(this.proxy.address);
             expect(balance.toString()).to.be.bignumber.equal(value.toString());
           });
 
@@ -151,7 +137,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
             // storage layout should look as follows:
             //  - 0: Initializable storage ++ initializerRan ++ onlyInitializingRan
             //  - 1: x
-            const storedValue = await web3.eth.getStorageAt(this.proxyAddress, 1);
+            const storedValue = await web3.eth.getStorageAt(this.proxy.address, 1);
             expect(parseInt(storedValue)).to.eq(42);
           });
         });
@@ -170,7 +156,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
 
         it('reverts', async function () {
           await expectRevert.unspecified(
-            this.proxy.upgradeToAndCall(this.behavior.address, initializeData, { from: proxyAdminAddress }),
+            this.proxy.upgradeToAndCall(this.behavior.address, initializeData, { from: this.proxyAdminAddress }),
           );
         });
       });
@@ -178,7 +164,6 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
 
     describe('with migrations', function () {
       describe('when the sender is the admin', function () {
-        const from = proxyAdminAddress;
         const value = 1e5;
 
         describe('when upgrading to V1', function () {
@@ -186,8 +171,11 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
 
           beforeEach(async function () {
             this.behaviorV1 = await MigratableMockV1.new();
-            this.balancePreviousV1 = new BN(await web3.eth.getBalance(this.proxyAddress));
-            this.receipt = await this.proxy.upgradeToAndCall(this.behaviorV1.address, v1MigrationData, { from, value });
+            this.balancePreviousV1 = new BN(await web3.eth.getBalance(this.proxy.address));
+            this.receipt = await this.proxy.upgradeToAndCall(this.behaviorV1.address, v1MigrationData, {
+              from: this.proxyAdminAddress,
+              value,
+            });
           });
 
           it('upgrades to the requested version and emits an event', async function () {
@@ -197,12 +185,12 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
           });
 
           it("calls the 'initialize' function and sends given value to the proxy", async function () {
-            const migratable = new MigratableMockV1(this.proxyAddress);
+            const migratable = new MigratableMockV1(this.proxy.address);
 
             const x = await migratable.x();
             expect(x).to.be.bignumber.equal('42');
 
-            const balance = await web3.eth.getBalance(this.proxyAddress);
+            const balance = await web3.eth.getBalance(this.proxy.address);
             expect(new BN(balance)).to.be.bignumber.equal(this.balancePreviousV1.addn(value));
           });
 
@@ -211,9 +199,9 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
 
             beforeEach(async function () {
               this.behaviorV2 = await MigratableMockV2.new();
-              this.balancePreviousV2 = new BN(await web3.eth.getBalance(this.proxyAddress));
+              this.balancePreviousV2 = new BN(await web3.eth.getBalance(this.proxy.address));
               this.receipt = await this.proxy.upgradeToAndCall(this.behaviorV2.address, v2MigrationData, {
-                from,
+                from: this.proxyAdminAddress,
                 value,
               });
             });
@@ -225,7 +213,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
             });
 
             it("calls the 'migrate' function and sends given value to the proxy", async function () {
-              const migratable = new MigratableMockV2(this.proxyAddress);
+              const migratable = new MigratableMockV2(this.proxy.address);
 
               const x = await migratable.x();
               expect(x).to.be.bignumber.equal('10');
@@ -233,7 +221,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
               const y = await migratable.y();
               expect(y).to.be.bignumber.equal('42');
 
-              const balance = new BN(await web3.eth.getBalance(this.proxyAddress));
+              const balance = new BN(await web3.eth.getBalance(this.proxy.address));
               expect(balance).to.be.bignumber.equal(this.balancePreviousV2.addn(value));
             });
 
@@ -242,9 +230,9 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
 
               beforeEach(async function () {
                 this.behaviorV3 = await MigratableMockV3.new();
-                this.balancePreviousV3 = new BN(await web3.eth.getBalance(this.proxyAddress));
+                this.balancePreviousV3 = new BN(await web3.eth.getBalance(this.proxy.address));
                 this.receipt = await this.proxy.upgradeToAndCall(this.behaviorV3.address, v3MigrationData, {
-                  from,
+                  from: this.proxyAdminAddress,
                   value,
                 });
               });
@@ -256,7 +244,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
               });
 
               it("calls the 'migrate' function and sends given value to the proxy", async function () {
-                const migratable = new MigratableMockV3(this.proxyAddress);
+                const migratable = new MigratableMockV3(this.proxy.address);
 
                 const x = await migratable.x();
                 expect(x).to.be.bignumber.equal('42');
@@ -264,7 +252,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
                 const y = await migratable.y();
                 expect(y).to.be.bignumber.equal('10');
 
-                const balance = new BN(await web3.eth.getBalance(this.proxyAddress));
+                const balance = new BN(await web3.eth.getBalance(this.proxy.address));
                 expect(balance).to.be.bignumber.equal(this.balancePreviousV3.addn(value));
               });
             });
@@ -289,15 +277,18 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
       const initializeData = Buffer.from('');
       this.clashingImplV0 = (await ClashingImplementation.new()).address;
       this.clashingImplV1 = (await ClashingImplementation.new()).address;
-      this.proxy = await createProxy(this.clashingImplV0, proxyAdminAddress, initializeData, {
-        from: proxyAdminOwner,
-      });
+      const { proxy, proxyAdminAddress } = await createProxyWithImpersonatedProxyAdmin(
+        this.clashingImplV0,
+        initializeData,
+      );
+      this.proxy = proxy;
+      this.proxyAdminAddress = proxyAdminAddress;
       this.clashing = new ClashingImplementation(this.proxy.address);
     });
 
     it('proxy admin cannot call delegated functions', async function () {
       await expectRevertCustomError(
-        this.clashing.delegatedFunction({ from: proxyAdminAddress }),
+        this.clashing.delegatedFunction({ from: this.proxyAdminAddress }),
         'ProxyDeniedAdminAccess',
         [],
       );
@@ -305,26 +296,16 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
 
     describe('when function names clash', function () {
       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 });
+        const receipt = await this.proxy.upgradeToAndCall(this.clashingImplV1, '0x', {
+          from: this.proxyAdminAddress,
+        });
         expectEvent(receipt, 'Upgraded', { implementation: this.clashingImplV1 });
       });
 
       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('requires 0 value calling upgradeTo by proxy admin', async function () {
-        await expectRevertCustomError(
-          this.proxy.upgradeTo(this.clashingImplV1, { from: proxyAdminAddress, value: 1 }),
-          'ProxyNonPayableFunction',
-          [],
-        );
-      });
-
-      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 });
+        const receipt = await this.proxy.upgradeToAndCall(this.clashingImplV1, '0x', {
+          from: anotherAccount,
+        });
         expectEvent.notEmitted(receipt, 'Upgraded');
         expectEvent.inTransaction(receipt.tx, this.clashing, 'ClashingImplementationCall');
       });
@@ -336,13 +317,16 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
 
     it('should add new function', async () => {
       const instance1 = await Implementation1.new();
-      const proxy = await createProxy(instance1.address, proxyAdminAddress, initializeData, { from: proxyAdminOwner });
+      const { proxy, proxyAdminAddress } = await createProxyWithImpersonatedProxyAdmin(
+        instance1.address,
+        initializeData,
+      );
 
       const proxyInstance1 = new Implementation1(proxy.address);
       await proxyInstance1.setValue(42);
 
       const instance2 = await Implementation2.new();
-      await proxy.upgradeTo(instance2.address, { from: proxyAdminAddress });
+      await proxy.upgradeToAndCall(instance2.address, '0x', { from: proxyAdminAddress });
 
       const proxyInstance2 = new Implementation2(proxy.address);
       const res = await proxyInstance2.getValue();
@@ -351,7 +335,10 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
 
     it('should remove function', async () => {
       const instance2 = await Implementation2.new();
-      const proxy = await createProxy(instance2.address, proxyAdminAddress, initializeData, { from: proxyAdminOwner });
+      const { proxy, proxyAdminAddress } = await createProxyWithImpersonatedProxyAdmin(
+        instance2.address,
+        initializeData,
+      );
 
       const proxyInstance2 = new Implementation2(proxy.address);
       await proxyInstance2.setValue(42);
@@ -359,7 +346,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
       expect(res.toString()).to.eq('42');
 
       const instance1 = await Implementation1.new();
-      await proxy.upgradeTo(instance1.address, { from: proxyAdminAddress });
+      await proxy.upgradeToAndCall(instance1.address, '0x', { from: proxyAdminAddress });
 
       const proxyInstance1 = new Implementation2(proxy.address);
       await expectRevert.unspecified(proxyInstance1.getValue());
@@ -367,13 +354,16 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
 
     it('should change function signature', async () => {
       const instance1 = await Implementation1.new();
-      const proxy = await createProxy(instance1.address, proxyAdminAddress, initializeData, { from: proxyAdminOwner });
+      const { proxy, proxyAdminAddress } = await createProxyWithImpersonatedProxyAdmin(
+        instance1.address,
+        initializeData,
+      );
 
       const proxyInstance1 = new Implementation1(proxy.address);
       await proxyInstance1.setValue(42);
 
       const instance3 = await Implementation3.new();
-      await proxy.upgradeTo(instance3.address, { from: proxyAdminAddress });
+      await proxy.upgradeToAndCall(instance3.address, '0x', { from: proxyAdminAddress });
       const proxyInstance3 = new Implementation3(proxy.address);
 
       const res = await proxyInstance3.getValue(8);
@@ -383,10 +373,13 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
     it('should add fallback function', async () => {
       const initializeData = Buffer.from('');
       const instance1 = await Implementation1.new();
-      const proxy = await createProxy(instance1.address, proxyAdminAddress, initializeData, { from: proxyAdminOwner });
+      const { proxy, proxyAdminAddress } = await createProxyWithImpersonatedProxyAdmin(
+        instance1.address,
+        initializeData,
+      );
 
       const instance4 = await Implementation4.new();
-      await proxy.upgradeTo(instance4.address, { from: proxyAdminAddress });
+      await proxy.upgradeToAndCall(instance4.address, '0x', { from: proxyAdminAddress });
       const proxyInstance4 = new Implementation4(proxy.address);
 
       const data = '0x';
@@ -398,10 +391,13 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
 
     it('should remove fallback function', async () => {
       const instance4 = await Implementation4.new();
-      const proxy = await createProxy(instance4.address, proxyAdminAddress, initializeData, { from: proxyAdminOwner });
+      const { proxy, proxyAdminAddress } = await createProxyWithImpersonatedProxyAdmin(
+        instance4.address,
+        initializeData,
+      );
 
       const instance2 = await Implementation2.new();
-      await proxy.upgradeTo(instance2.address, { from: proxyAdminAddress });
+      await proxy.upgradeToAndCall(instance2.address, '0x', { from: proxyAdminAddress });
 
       const data = '0x';
       await expectRevert.unspecified(web3.eth.sendTransaction({ to: proxy.address, from: anotherAccount, data }));

test/proxy/transparent/TransparentUpgradeableProxy.test.js

@@ -5,13 +5,14 @@ const TransparentUpgradeableProxy = artifacts.require('TransparentUpgradeablePro
 const ITransparentUpgradeableProxy = artifacts.require('ITransparentUpgradeableProxy');
 
 contract('TransparentUpgradeableProxy', function (accounts) {
-  const [proxyAdminAddress, proxyAdminOwner] = accounts;
+  const [owner, ...otherAccounts] = accounts;
 
-  const createProxy = async function (logic, admin, initData, opts) {
-    const { address } = await TransparentUpgradeableProxy.new(logic, admin, initData, opts);
-    return ITransparentUpgradeableProxy.at(address);
+  const createProxy = async function (logic, initData, opts = {}) {
+    const { address, transactionHash } = await TransparentUpgradeableProxy.new(logic, owner, initData, opts);
+    const instance = await ITransparentUpgradeableProxy.at(address);
+    return { ...instance, transactionHash };
   };
 
-  shouldBehaveLikeProxy(createProxy, proxyAdminAddress, proxyAdminOwner);
-  shouldBehaveLikeTransparentUpgradeableProxy(createProxy, accounts);
+  shouldBehaveLikeProxy(createProxy, otherAccounts);
+  shouldBehaveLikeTransparentUpgradeableProxy(createProxy, owner, otherAccounts);
 });

test/proxy/utils/UUPSUpgradeable.test.js

@@ -26,7 +26,7 @@ contract('UUPSUpgradeable', function () {
   });
 
   it('upgrade to upgradeable implementation', async function () {
-    const { receipt } = await this.instance.upgradeTo(this.implUpgradeOk.address);
+    const { receipt } = await this.instance.upgradeToAndCall(this.implUpgradeOk.address, '0x');
     expect(receipt.logs.filter(({ event }) => event === 'Upgraded').length).to.be.equal(1);
     expectEvent(receipt, 'Upgraded', { implementation: this.implUpgradeOk.address });
     expect(await getAddressInSlot(this.instance, ImplementationSlot)).to.be.equal(this.implUpgradeOk.address);
@@ -48,7 +48,7 @@ contract('UUPSUpgradeable', function () {
 
   it('calling upgradeTo on the implementation reverts', async function () {
     await expectRevertCustomError(
-      this.implInitial.upgradeTo(this.implUpgradeOk.address),
+      this.implInitial.upgradeToAndCall(this.implUpgradeOk.address, '0x'),
       'UUPSUnauthorizedCallContext',
       [],
     );
@@ -72,7 +72,7 @@ contract('UUPSUpgradeable', function () {
     );
 
     await expectRevertCustomError(
-      instance.upgradeTo(this.implUpgradeUnsafe.address),
+      instance.upgradeToAndCall(this.implUpgradeUnsafe.address, '0x'),
       'UUPSUnauthorizedCallContext',
       [],
     );
@@ -93,14 +93,14 @@ contract('UUPSUpgradeable', function () {
 
   it('rejects upgrading to an unsupported UUID', async function () {
     await expectRevertCustomError(
-      this.instance.upgradeTo(this.implUnsupportedUUID.address),
+      this.instance.upgradeToAndCall(this.implUnsupportedUUID.address, '0x'),
       'UUPSUnsupportedProxiableUUID',
       [web3.utils.keccak256('invalid UUID')],
     );
   });
 
   it('upgrade to and unsafe upgradeable implementation', async function () {
-    const { receipt } = await this.instance.upgradeTo(this.implUpgradeUnsafe.address);
+    const { receipt } = await this.instance.upgradeToAndCall(this.implUpgradeUnsafe.address, '0x');
     expectEvent(receipt, 'Upgraded', { implementation: this.implUpgradeUnsafe.address });
     expect(await getAddressInSlot(this.instance, ImplementationSlot)).to.be.equal(this.implUpgradeUnsafe.address);
   });
@@ -108,7 +108,7 @@ contract('UUPSUpgradeable', function () {
   // delegate to a non existing upgradeTo function causes a low level revert
   it('reject upgrade to non uups implementation', async function () {
     await expectRevertCustomError(
-      this.instance.upgradeTo(this.implUpgradeNonUUPS.address),
+      this.instance.upgradeToAndCall(this.implUpgradeNonUUPS.address, '0x'),
       'ERC1967InvalidImplementation',
       [this.implUpgradeNonUUPS.address],
     );
@@ -118,8 +118,10 @@ contract('UUPSUpgradeable', function () {
     const { address } = await ERC1967Proxy.new(this.implInitial.address, '0x');
     const otherInstance = await UUPSUpgradeableMock.at(address);
 
-    await expectRevertCustomError(this.instance.upgradeTo(otherInstance.address), 'ERC1967InvalidImplementation', [
-      otherInstance.address,
-    ]);
+    await expectRevertCustomError(
+      this.instance.upgradeToAndCall(otherInstance.address, '0x'),
+      'ERC1967InvalidImplementation',
+      [otherInstance.address],
+    );
   });
 });

test/utils/Create2.test.js

@@ -1,5 +1,5 @@
 const { balance, ether, expectEvent, expectRevert, send } = require('@openzeppelin/test-helpers');
-const { computeCreate2Address } = require('../helpers/create2');
+const { computeCreate2Address } = require('../helpers/create');
 const { expect } = require('chai');
 const { expectRevertCustomError } = require('../helpers/customError');