From 2a0bea55879287b3878f0a591f724c1b6fa91cf9 Mon Sep 17 00:00:00 2001 From: jaybuidl Date: Mon, 16 Oct 2023 23:37:14 +0100 Subject: [PATCH] refactor: proxy mocks simplification --- ...entations.sol => UUPSUpgradeableMocks.sol} | 55 ---------------- .../by-inheritance/UpgradedByInheritance.sol | 62 +++++++++++++++++++ .../UpgradedByRewrite.sol} | 6 +- .../UpgradedByRewriteV2.sol} | 6 +- contracts/test/proxy/index.ts | 45 ++++++-------- 5 files changed, 86 insertions(+), 88 deletions(-) rename contracts/src/proxy/mock/{MockImplementations.sol => UUPSUpgradeableMocks.sol} (55%) create mode 100644 contracts/src/proxy/mock/by-inheritance/UpgradedByInheritance.sol rename contracts/src/proxy/mock/{UUPSUpgradableInitializable.sol => by-rewrite/UpgradedByRewrite.sol} (86%) rename contracts/src/proxy/mock/{UUPSUpgradableInitializableV2.sol => by-rewrite/UpgradedByRewriteV2.sol} (87%) diff --git a/contracts/src/proxy/mock/MockImplementations.sol b/contracts/src/proxy/mock/UUPSUpgradeableMocks.sol similarity index 55% rename from contracts/src/proxy/mock/MockImplementations.sol rename to contracts/src/proxy/mock/UUPSUpgradeableMocks.sol index b24f478e6..ec31d62f6 100644 --- a/contracts/src/proxy/mock/MockImplementations.sol +++ b/contracts/src/proxy/mock/UUPSUpgradeableMocks.sol @@ -62,58 +62,3 @@ contract UUPSUnsupportedProxiableUUID is UUPSUpgradeableMock { return "UUPSUnsupportedProxiableUUID 1.0.0"; } } - -contract UUPSUpgradableInitializableInheritanceV1 is UUPSProxiable, Initializable { - address public governor; - uint256 public counter; - uint256[50] __gap; - - constructor() { - _disableInitializers(); - } - - function initialize(address _governor) external virtual reinitializer(1) { - governor = _governor; - counter = 1; - } - - function _authorizeUpgrade(address) internal view override { - require(governor == msg.sender, "No privilege to upgrade"); - } - - function increment() external { - ++counter; - } - - function version() external pure virtual returns (string memory) { - return "V1"; - } -} - -contract UUPSUpgradableInitializableInheritanceV2 is UUPSUpgradableInitializableInheritanceV1 { - string public newVariable; - uint256[50] __gap2; - - constructor() { - _disableInitializers(); - } - - function initializeV2(string memory _newVariable) external reinitializer(2) { - newVariable = _newVariable; - this.increment(); - } - - function version() external pure virtual override returns (string memory) { - return "V2"; - } -} - -contract UUPSUpgradableInitializableInheritanceV3Bad is UUPSUpgradableInitializableInheritanceV2 { - constructor() { - _disableInitializers(); - } - - function initializeV3() external reinitializer(1) { - // Wrong reinitializer version. - } -} diff --git a/contracts/src/proxy/mock/by-inheritance/UpgradedByInheritance.sol b/contracts/src/proxy/mock/by-inheritance/UpgradedByInheritance.sol new file mode 100644 index 000000000..bb8a8c17f --- /dev/null +++ b/contracts/src/proxy/mock/by-inheritance/UpgradedByInheritance.sol @@ -0,0 +1,62 @@ +//SPDX-License-Identifier: MIT +// Adapted from + +pragma solidity 0.8.18; + +import "../../UUPSProxiable.sol"; +import "../../Initializable.sol"; + +contract UpgradedByInheritanceV1 is UUPSProxiable, Initializable { + address public governor; + uint256 public counter; + uint256[50] __gap; + + constructor() { + _disableInitializers(); + } + + function initialize(address _governor) external virtual reinitializer(1) { + governor = _governor; + counter = 1; + } + + function _authorizeUpgrade(address) internal view override { + require(governor == msg.sender, "No privilege to upgrade"); + } + + function increment() external { + ++counter; + } + + function version() external pure virtual returns (string memory) { + return "V1"; + } +} + +contract UpgradedByInheritanceV2 is UpgradedByInheritanceV1 { + string public newVariable; + uint256[50] __gap2; + + constructor() { + _disableInitializers(); + } + + function initializeV2(string memory _newVariable) external reinitializer(2) { + newVariable = _newVariable; + this.increment(); + } + + function version() external pure virtual override returns (string memory) { + return "V2"; + } +} + +contract UpgradedByInheritanceV3Bad is UpgradedByInheritanceV2 { + constructor() { + _disableInitializers(); + } + + function initializeV3() external reinitializer(1) { + // Wrong reinitializer version. + } +} diff --git a/contracts/src/proxy/mock/UUPSUpgradableInitializable.sol b/contracts/src/proxy/mock/by-rewrite/UpgradedByRewrite.sol similarity index 86% rename from contracts/src/proxy/mock/UUPSUpgradableInitializable.sol rename to contracts/src/proxy/mock/by-rewrite/UpgradedByRewrite.sol index 0f73a7757..aefc7c1c7 100644 --- a/contracts/src/proxy/mock/UUPSUpgradableInitializable.sol +++ b/contracts/src/proxy/mock/by-rewrite/UpgradedByRewrite.sol @@ -3,10 +3,10 @@ pragma solidity 0.8.18; -import "../UUPSProxiable.sol"; -import "../Initializable.sol"; +import "../../UUPSProxiable.sol"; +import "../../Initializable.sol"; -contract UUPSUpgradableInitializable is UUPSProxiable, Initializable { +contract UpgradedByRewrite is UUPSProxiable, Initializable { //------------------------ // V1 State //------------------------ diff --git a/contracts/src/proxy/mock/UUPSUpgradableInitializableV2.sol b/contracts/src/proxy/mock/by-rewrite/UpgradedByRewriteV2.sol similarity index 87% rename from contracts/src/proxy/mock/UUPSUpgradableInitializableV2.sol rename to contracts/src/proxy/mock/by-rewrite/UpgradedByRewriteV2.sol index 9dd25e747..749041679 100644 --- a/contracts/src/proxy/mock/UUPSUpgradableInitializableV2.sol +++ b/contracts/src/proxy/mock/by-rewrite/UpgradedByRewriteV2.sol @@ -3,10 +3,10 @@ pragma solidity 0.8.18; -import "../UUPSProxiable.sol"; -import "../Initializable.sol"; +import "../../UUPSProxiable.sol"; +import "../../Initializable.sol"; -contract UUPSUpgradableInitializable is UUPSProxiable, Initializable { +contract UpgradedByRewrite is UUPSProxiable, Initializable { //------------------------ // V1 State //------------------------ diff --git a/contracts/test/proxy/index.ts b/contracts/test/proxy/index.ts index 6325ea35f..81f5fbfc8 100644 --- a/contracts/test/proxy/index.ts +++ b/contracts/test/proxy/index.ts @@ -3,10 +3,9 @@ import { log } from "console"; import { ethers, deployments } from "hardhat"; import { DeployResult } from "hardhat-deploy/types"; import { deployUpgradable } from "../../deploy/utils/deployUpgradable"; -import { - UUPSUpgradableInitializableInheritanceV1, - UUPSUpgradableInitializableInheritanceV2, -} from "../../typechain-types"; +import { UpgradedByInheritanceV1, UpgradedByInheritanceV2 } from "../../typechain-types"; +import { UpgradedByRewrite as UpgradedByRewriteV1 } from "../../typechain-types/src/proxy/mock/by-rewrite"; +import { UpgradedByRewrite as UpgradedByRewriteV2 } from "../../typechain-types/src/proxy/mock/by-rewrite/UpgradedByRewriteV2.sol"; let deployer; let user1; @@ -119,8 +118,8 @@ describe("Upgradability", async () => { before("Setup Contracts", async () => { [deployer] = await ethers.getSigners(); - proxyDeployment = await deployUpgradable(deployments, "UUPSUpgradableInitializable", { - contract: "src/proxy/mock/UUPSUpgradableInitializable.sol:UUPSUpgradableInitializable", + proxyDeployment = await deployUpgradable(deployments, "UpgradedByRewrite", { + contract: "src/proxy/mock/by-rewrite/UpgradedByRewrite.sol:UpgradedByRewrite", from: deployer.address, args: [deployer.address], log: true, @@ -131,11 +130,9 @@ describe("Upgradability", async () => { }); it("Initializes v1", async () => { - proxy = (await ethers.getContract("UUPSUpgradableInitializable")) as UUPSUpgradableInitializableInheritanceV1; + proxy = (await ethers.getContract("UpgradedByRewrite")) as UpgradedByRewriteV1; - implementation = (await ethers.getContract( - "UUPSUpgradableInitializable_Implementation" - )) as UUPSUpgradableInitializableInheritanceV1; + implementation = (await ethers.getContract("UpgradedByRewrite_Implementation")) as UpgradedByRewriteV1; expect(await proxy.governor()).to.equal(deployer.address); @@ -150,8 +147,8 @@ describe("Upgradability", async () => { }); it("Upgrades to v2 and initializes", async () => { - proxyDeployment = await deployUpgradable(deployments, "UUPSUpgradableInitializable", { - contract: "src/proxy/mock/UUPSUpgradableInitializableV2.sol:UUPSUpgradableInitializable", + proxyDeployment = await deployUpgradable(deployments, "UpgradedByRewrite", { + contract: "src/proxy/mock/by-rewrite/UpgradedByRewriteV2.sol:UpgradedByRewrite", from: deployer.address, args: ["Future of France"], log: true, @@ -159,7 +156,7 @@ describe("Upgradability", async () => { if (!proxyDeployment.implementation) { throw new Error("No implementation address"); } - proxy = (await ethers.getContract("UUPSUpgradableInitializable")) as UUPSUpgradableInitializableInheritanceV2; + proxy = (await ethers.getContract("UpgradedByRewrite")) as UpgradedByRewriteV2; expect(await proxy.governor()).to.equal(deployer.address); expect(await proxy.counter()).to.equal(3); @@ -176,7 +173,7 @@ describe("Upgradability", async () => { before("Setup Contracts", async () => { [deployer] = await ethers.getSigners(); - proxyDeployment = await deployUpgradable(deployments, "UUPSUpgradableInitializableInheritanceV1", { + proxyDeployment = await deployUpgradable(deployments, "UpgradedByInheritanceV1", { from: deployer.address, args: [deployer.address], log: true, @@ -187,13 +184,9 @@ describe("Upgradability", async () => { }); it("Initializes v1", async () => { - proxy = (await ethers.getContract( - "UUPSUpgradableInitializableInheritanceV1" - )) as UUPSUpgradableInitializableInheritanceV1; + proxy = (await ethers.getContract("UpgradedByInheritanceV1")) as UpgradedByInheritanceV1; - implementation = (await ethers.getContract( - "UUPSUpgradableInitializableInheritanceV1_Implementation" - )) as UUPSUpgradableInitializableInheritanceV1; + implementation = (await ethers.getContract("UpgradedByInheritanceV1_Implementation")) as UpgradedByInheritanceV1; expect(await proxy.governor()).to.equal(deployer.address); @@ -208,17 +201,15 @@ describe("Upgradability", async () => { }); it("Upgrades to v2 and initializes", async () => { - proxyDeployment = await deployUpgradable(deployments, "UUPSUpgradableInitializableInheritanceV1", { - newImplementation: "UUPSUpgradableInitializableInheritanceV2", + proxyDeployment = await deployUpgradable(deployments, "UpgradedByInheritanceV1", { + newImplementation: "UpgradedByInheritanceV2", initializer: "initializeV2", from: deployer.address, args: ["Future of France"], log: true, }); - proxy = (await ethers.getContract( - "UUPSUpgradableInitializableInheritanceV1" - )) as UUPSUpgradableInitializableInheritanceV2; + proxy = (await ethers.getContract("UpgradedByInheritanceV1")) as UpgradedByInheritanceV2; expect(await proxy.governor()).to.equal(deployer.address); @@ -233,8 +224,8 @@ describe("Upgradability", async () => { it("Cannot upgrade to v3 which has an invalid initializer", async () => { await expect( - deployUpgradable(deployments, "UUPSUpgradableInitializableInheritanceV1", { - newImplementation: "UUPSUpgradableInitializableInheritanceV3Bad", + deployUpgradable(deployments, "UpgradedByInheritanceV1", { + newImplementation: "UpgradedByInheritanceV3Bad", initializer: "initializeV3", from: deployer.address, args: [],