refactor uups: proxy upgradeable is now uupsproxy

contracts/proxy/transparent/TransparentUpgradeableProxy.sol

@@ -2,7 +2,7 @@
 
 pragma solidity ^0.8.0;
 
-import "../UpgradeableProxy.sol";
+import "../uups/UUPSProxy.sol";
 
 /**
  * @dev This contract implements a proxy that is upgradeable by an admin.
@@ -25,12 +25,12 @@ import "../UpgradeableProxy.sol";
  * 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.
  */
-contract TransparentUpgradeableProxy is UpgradeableProxy {
+contract TransparentUpgradeableProxy is UUPSProxy {
     /**
      * @dev Initializes an upgradeable proxy managed by `_admin`, backed by the implementation at `_logic`, and
      * optionally initialized with `_data` as explained in {UpgradeableProxy-constructor}.
      */
-    constructor(address _logic, address admin_, bytes memory _data) payable UpgradeableProxy(_logic, _data) {
+    constructor(address _logic, address admin_, bytes memory _data) payable UUPSProxy(_logic, _data) {
         assert(_ADMIN_SLOT == bytes32(uint256(keccak256("eip1967.proxy.admin")) - 1));
         _setAdmin(admin_);
     }

contracts/proxy/uups/UUPS.sol

@@ -2,22 +2,49 @@
 
 pragma solidity ^0.8.0;
 
-library UUPS {
-    // bytes32 private constant _UUPS_SLOT = keccak256("PROXIABLE");
-    bytes32 private constant _UUPS_SLOT = 0xc5f16f0fcc639fa48a6947836d9850f504798523bf8c9a3a87d5876cf622bcf7;
+import "../../utils/Address.sol";
 
-    struct UUPSStore {
-        address implementation;
+abstract contract UUPS {
+    /**
+     * @dev Storage slot with the address of the current implementation.
+     * This is the keccak-256 hash of "eip1967.proxy.implementation" subtracted by 1, and is
+     * validated in the constructor.
+     */
+    bytes32 private constant _IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;
+
+    /**
+     * @dev Emitted when the implementation is upgraded.
+     */
+    event Upgraded(address indexed implementation);
+
+    /**
+     *
+     */
+    function _uuid() internal view virtual returns (bytes32) {
+        return _IMPLEMENTATION_SLOT;
     }
 
-    function instance() internal pure returns (UUPSStore storage uups) {
-        bytes32 position = _UUPS_SLOT;
-        assembly {
-            uups.slot := position
-        }
+    /**
+     * @dev Upgrades the proxy to a new implementation.
+     *
+     * Emits an {Upgraded} event.
+     */
+    function _upgradeTo(address newImplementation) internal virtual {
+        _setImplementation(newImplementation);
+        emit Upgraded(newImplementation);
     }
 
-    function uuid() internal pure returns (bytes32) {
-        return _UUPS_SLOT;
+    /**
+     * @dev Stores a new address in the EIP1967 implementation slot.
+     */
+    function _setImplementation(address newImplementation) private {
+        require(Address.isContract(newImplementation), "UUPS: new implementation is not a contract");
+
+        bytes32 slot = _IMPLEMENTATION_SLOT;
+
+        // solhint-disable-next-line no-inline-assembly
+        assembly {
+            sstore(slot, newImplementation)
+        }
     }
 }

contracts/proxy/uups/UUPSProxiable.sol

@@ -4,13 +4,13 @@ pragma solidity ^0.8.0;
 
 import "./UUPS.sol";
 
-abstract contract UUPSProxiable {
-    function proxiableUUID() public pure returns (bytes32) {
-        return UUPS.uuid();
+abstract contract UUPSProxiable is UUPS {
+    function proxiableUUID() public view returns (bytes32) {
+        return _uuid();
     }
 
     function _updateCodeAddress(address newAddress) internal {
-        require(UUPS.uuid() == UUPSProxiable(newAddress).proxiableUUID(), "Not compatible");
-        UUPS.instance().implementation = newAddress;
+        require(_uuid() == UUPSProxiable(newAddress).proxiableUUID(), "UUPS: new impelmentation is not compatible");
+        _upgradeTo(newAddress);
     }
 }

contracts/proxy/uups/UUPSProxy.sol

@@ -3,20 +3,40 @@
 pragma solidity ^0.8.0;
 
 import "./UUPS.sol";
-import "./UUPSProxiable.sol";
 import "../Proxy.sol";
-import "../../utils/Address.sol";
 
-contract UUPSProxy is Proxy {
+/**
+ * @dev This contract implements an upgradeable proxy. It is upgradeable because calls are delegated to an
+ * implementation address that can be changed. This address is stored in storage in the location specified by
+ * https://eips.ethereum.org/EIPS/eip-1967[EIP1967], so that it doesn't conflict with the storage layout of the
+ * implementation behind the proxy.
+ *
+ * Upgradeability is only provided internally through {_upgradeTo}. For an externally upgradeable proxy see
+ * {TransparentUpgradeableProxy}.
+ */
+contract UUPSProxy is Proxy, UUPS {
+    /**
+     * @dev Initializes the upgradeable proxy with an initial implementation specified by `_logic`.
+     *
+     * If `_data` is nonempty, it's used as data in a delegate call to `_logic`. This will typically be an encoded
+     * function call, and allows initializating the storage of the proxy like a Solidity constructor.
+     */
     constructor(address _logic, bytes memory _data) payable {
-        require(UUPS.uuid() == UUPSProxiable(_logic).proxiableUUID(), "Not compatible");
-        UUPS.instance().implementation = _logic;
-        if (_data.length > 0) {
+        assert(_uuid() == bytes32(uint256(keccak256("eip1967.proxy.implementation")) - 1));
+        _upgradeTo(_logic);
+        if(_data.length > 0) {
             Address.functionDelegateCall(_logic, _data);
         }
     }
 
-    function _implementation() internal view virtual override returns (address) {
-        return UUPS.instance().implementation;
+    /**
+     * @dev Returns the current implementation address.
+     */
+    function _implementation() internal view virtual override returns (address impl) {
+        bytes32 slot = _uuid();
+        // solhint-disable-next-line no-inline-assembly
+        assembly {
+            impl := sload(slot)
+        }
     }
 }

test/proxy/Proxy.behaviour.js

@@ -11,13 +11,7 @@ function toChecksumAddress (address) {
   return ethereumjsUtil.toChecksumAddress('0x' + address.replace(/^0x/, '').padStart(40, '0'));
 }
 
-module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress, proxyCreator, opts = {}) {
-  const { artefact, slot } = Object.assign({
-    artefact: DummyImplementation,
-    slot: '0x' + new BN(ethereumjsUtil.keccak256(Buffer.from(IMPLEMENTATION_LABEL))).subn(1).toString(16),
-  }, opts);
-
-
+module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress, proxyCreator) {
   it('cannot be initialized with a non-contract address', async function () {
     const nonContractAddress = proxyCreator;
     const initializeData = Buffer.from('');
@@ -29,17 +23,18 @@ module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress,
   });
 
   before('deploy implementation', async function () {
-    this.implementation = web3.utils.toChecksumAddress((await artefact.new()).address);
+    this.implementation = web3.utils.toChecksumAddress((await DummyImplementation.new()).address);
   });
 
   const assertProxyInitialization = function ({ value, balance }) {
     it('sets the implementation address', async function () {
+      const slot = '0x' + new BN(ethereumjsUtil.keccak256(Buffer.from(IMPLEMENTATION_LABEL))).subn(1).toString(16);
       const implementation = toChecksumAddress((await web3.eth.getStorageAt(this.proxy, slot)).substr(-40));
       expect(implementation).to.be.equal(this.implementation);
     });
 
     it('initializes the proxy', async function () {
-      const dummy = new artefact(this.proxy);
+      const dummy = new DummyImplementation(this.proxy);
       expect(await dummy.value()).to.be.bignumber.equal(value.toString());
     });
 
@@ -82,7 +77,7 @@ module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress,
   describe('initialization without parameters', function () {
     describe('non payable', function () {
       const expectedInitializedValue = 10;
-      const initializeData = new artefact('').contract.methods['initializeNonPayable()']().encodeABI();
+      const initializeData = new DummyImplementation('').contract.methods['initializeNonPayable()']().encodeABI();
 
       describe('when not sending balance', function () {
         beforeEach('creating proxy', async function () {
@@ -112,7 +107,7 @@ module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress,
 
     describe('payable', function () {
       const expectedInitializedValue = 100;
-      const initializeData = new artefact('').contract.methods['initializePayable()']().encodeABI();
+      const initializeData = new DummyImplementation('').contract.methods['initializePayable()']().encodeABI();
 
       describe('when not sending balance', function () {
         beforeEach('creating proxy', async function () {
@@ -152,7 +147,7 @@ module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress,
   describe('initialization with parameters', function () {
     describe('non payable', function () {
       const expectedInitializedValue = 10;
-      const initializeData = new artefact('').contract
+      const initializeData = new DummyImplementation('').contract
         .methods.initializeNonPayableWithValue(expectedInitializedValue).encodeABI();
 
       describe('when not sending balance', function () {
@@ -183,7 +178,7 @@ module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress,
 
     describe('payable', function () {
       const expectedInitializedValue = 42;
-      const initializeData = new artefact('').contract
+      const initializeData = new DummyImplementation('').contract
         .methods.initializePayableWithValue(expectedInitializedValue).encodeABI();
 
       describe('when not sending balance', function () {
@@ -221,7 +216,7 @@ module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress,
     });
 
     describe('reverting initialization', function () {
-      const initializeData = new artefact('').contract
+      const initializeData = new DummyImplementation('').contract
         .methods.reverts().encodeABI();
 
       it('reverts', async function () {

test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js

@@ -80,7 +80,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy (createPro
         it('reverts', async function () {
           await expectRevert(
             this.proxy.upgradeTo(ZERO_ADDRESS, { from }),
-            'UpgradeableProxy: new implementation is not a contract',
+            'UUPS: new implementation is not a contract',
           );
         });
       });

test/proxy/uups/UUPSProxiable.test.js

@@ -0,0 +1,53 @@
+const { expectRevert } = require('@openzeppelin/test-helpers');
+const ethereumjsUtil = require('ethereumjs-util');
+
+const DummyImplementation = artifacts.require('DummyImplementation');
+const DummyImplementationProxiable = artifacts.require('DummyImplementationProxiable');
+const DummyImplementationV2Proxiable = artifacts.require('DummyImplementationV2Proxiable');
+const UUPSProxy = artifacts.require('UUPSProxy');
+
+const UUPS_UUID = '0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc';
+
+function toChecksumAddress (address) {
+  return ethereumjsUtil.toChecksumAddress('0x' + address.replace(/^0x/, '').padStart(40, '0'));
+}
+
+contract('UUPSProxiable', function (accounts) {
+  const [ proxyCreator ] = accounts;
+
+  const createProxy = async function (implementation, _admin, initData, opts) {
+    return UUPSProxy.new(implementation, initData, opts);
+  };
+
+  context('UUPS', function () {
+    beforeEach(async function () {
+      this.impl = await DummyImplementation.new();
+      this.implproxiable = await DummyImplementationProxiable.new();
+      this.implproxiablev2 = await DummyImplementationV2Proxiable.new();
+      const { address } = await createProxy(this.implproxiable.address, undefined, '0x', { from: proxyCreator });
+      this.dummy = new DummyImplementationProxiable(address);
+    });
+
+    it('check uuid', async function () {
+      expect(await this.dummy.proxiableUUID()).to.be.equal(UUPS_UUID);
+    });
+
+    it('can update to proxiable', async function () {
+      await this.dummy.updateCodeAddress(this.implproxiablev2.address);
+      const implementation = toChecksumAddress((await web3.eth.getStorageAt(
+        this.dummy.address,
+        UUPS_UUID,
+      )).substr(-40));
+      expect(implementation).to.be.equal(this.implproxiablev2.address);
+    });
+
+    it('does not deploy with non-proxiable', async function () {
+      this.skip(); // NOT SUPPORTED YET
+      await expectRevert.unspecified(createProxy(this.impl.address, undefined, '0x', { from: proxyCreator }));
+    });
+
+    it('does not update to non-proxiable', async function () {
+      await expectRevert.unspecified(this.dummy.updateCodeAddress(this.impl.address));
+    });
+  });
+});