From b8a00831198bf7a0d91dd16921ce2f609a91e162 Mon Sep 17 00:00:00 2001 From: ihoroleksiienko <144142848+ihoroleksiienko@users.noreply.github.com> Date: Mon, 18 Nov 2024 12:43:31 +0200 Subject: [PATCH] feat: introduce `UUPSExtUpgradeable` base contract (#23) * chore: update UUPSupgradable * chore: remove extra errors * chore: update comment * chore: move uups mock to base folder * chore: change error names * fix: remove redundant internal validation functions * style: reorder errors alphabetically * test: remove redundant tests that duplicate the base contract ones * feat: update version --------- Co-authored-by: Evgenii Zaitsev --- contracts/Cashier.sol | 30 +--- contracts/CashierShard.sol | 28 +--- contracts/base/UUPSExtUpgradeable.sol | 48 +++++++ contracts/base/Versionable.sol | 2 +- contracts/interfaces/ICashier.sol | 12 +- contracts/interfaces/ICashierShard.sol | 8 +- .../mocks/base/UUPSExtUpgradableMock.sol | 23 ++++ test/CashierSharded.test.ts | 128 ++---------------- test/base/UUPSExtUbgradable.test.ts | 70 ++++++++++ 9 files changed, 167 insertions(+), 182 deletions(-) create mode 100644 contracts/base/UUPSExtUpgradeable.sol create mode 100644 contracts/mocks/base/UUPSExtUpgradableMock.sol create mode 100644 test/base/UUPSExtUbgradable.test.ts diff --git a/contracts/Cashier.sol b/contracts/Cashier.sol index 60cea2f..be2e774 100644 --- a/contracts/Cashier.sol +++ b/contracts/Cashier.sol @@ -10,6 +10,7 @@ import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils import { AccessControlExtUpgradeable } from "./base/AccessControlExtUpgradeable.sol"; import { PausableExtUpgradeable } from "./base/PausableExtUpgradeable.sol"; import { RescuableUpgradeable } from "./base/RescuableUpgradeable.sol"; +import { UUPSExtUpgradeable } from "./base/UUPSExtUpgradeable.sol"; import { ICashier } from "./interfaces/ICashier.sol"; import { ICashierPrimary } from "./interfaces/ICashier.sol"; @@ -33,7 +34,7 @@ contract Cashier is AccessControlExtUpgradeable, PausableExtUpgradeable, RescuableUpgradeable, - UUPSUpgradeable, + UUPSExtUpgradeable, ICashier, ICashierHookable, Versionable @@ -771,24 +772,6 @@ contract Cashier is } } - /** - * @dev Validates the provided root. - * @param root The cashier root contract address. - */ - function _validateRootContract(address root) internal view { - if (root == address(0)) { - revert Cashier_RootAddressZero(); - } - - if (root.code.length == 0) { - revert Cashier_RootAddressNotContract(); - } - - try ICashier(root).proveCashierRoot() {} catch { - revert Cashier_ContractNotRoot(); - } - } - /** * @dev Checks the error code returned by the shard contract and reverts with the appropriate error message. * @param err The error code returned by the shard contract. @@ -881,12 +864,13 @@ contract Cashier is } /** - * @dev The upgrade authorization function for UUPSProxy. + * @dev The upgrade validation function for the UUPSExtUpgradeable contract. * @param newImplementation The address of the new implementation. */ - function _authorizeUpgrade(address newImplementation) internal view override onlyRole(OWNER_ROLE) { - _validateRootContract(newImplementation); - newImplementation; // Suppresses a compiler warning about the unused variable. + function _validateUpgrade(address newImplementation) internal view override onlyRole(OWNER_ROLE) { + try ICashier(newImplementation).proveCashierRoot() {} catch { + revert Cashier_ImplementationAddressInvalid(); + } } // ------------------ Service functions ----------------------- // diff --git a/contracts/CashierShard.sol b/contracts/CashierShard.sol index 2b02421..b5b4b14 100644 --- a/contracts/CashierShard.sol +++ b/contracts/CashierShard.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.24; import { OwnableUpgradeable } from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; +import { UUPSExtUpgradeable } from "./base/UUPSExtUpgradeable.sol"; import { ICashierShard } from "./interfaces/ICashierShard.sol"; import { ICashierShardPrimary } from "./interfaces/ICashierShard.sol"; @@ -16,7 +17,7 @@ import { Versionable } from "./base/Versionable.sol"; * @author CloudWalk Inc. (See https://www.cloudwalk.io) * @dev The contract responsible for storing sharded cash-in and cash-out operations. */ -contract CashierShard is CashierShardStorage, OwnableUpgradeable, UUPSUpgradeable, ICashierShard, Versionable { +contract CashierShard is CashierShardStorage, OwnableUpgradeable, UUPSExtUpgradeable, ICashierShard, Versionable { // ------------------ Initializers ---------------------------- // /** @@ -326,29 +327,12 @@ contract CashierShard is CashierShardStorage, OwnableUpgradeable, UUPSUpgradeabl } /** - * @dev The upgrade authorization function for UUPSProxy. + * @dev The upgrade validation function for the UUPSExtUpgradeable contract. * @param newImplementation The address of the new implementation. */ - function _authorizeUpgrade(address newImplementation) internal view override onlyOwnerOrAdmin { - _validateShardContract(newImplementation); - newImplementation; // Suppresses a compiler warning about the unused variable. - } - - /** - * @dev Validates the provided shard. - * @param shard The cashier shard contract address. - */ - function _validateShardContract(address shard) internal view { - if (shard == address(0)) { - revert CashierShard_ShardAddressZero(); - } - - if (shard.code.length == 0) { - revert CashierShard_ShardAddressNotContract(); - } - - try ICashierShard(shard).proveCashierShard() {} catch { - revert CashierShard_ContractNotShard(); + function _validateUpgrade(address newImplementation) internal view override onlyOwnerOrAdmin { + try ICashierShard(newImplementation).proveCashierShard() {} catch { + revert CashierShard_ImplementationAddressInvalid(); } } diff --git a/contracts/base/UUPSExtUpgradeable.sol b/contracts/base/UUPSExtUpgradeable.sol new file mode 100644 index 0000000..0d7e7e2 --- /dev/null +++ b/contracts/base/UUPSExtUpgradeable.sol @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; + +/** + * @title UUPSExtUpgradeable base contract + * @author CloudWalk Inc. (See https://www.cloudwalk.io) + * @dev Extends the OpenZeppelin's {UUPSUpgradeable} contract by adding additional checks for + * the new implementation address. + * + * This contract is used through inheritance. It introduces the virtual `_validateUpgrade()` function that must be + * implemented in the parent contract. + */ +abstract contract UUPSExtUpgradeable is UUPSUpgradeable { + // ------------------ Errors ---------------------------------- // + + /// @dev Thrown if the provided new implementation address is not a contract. + error UUPSExtUpgradeable_ImplementationAddressNotContract(); + + /// @dev Thrown if the provided new implementation contract address is zero. + error UUPSExtUpgradeable_ImplementationAddressZero(); + + // ------------------ Internal functions ---------------------- // + + /** + * @dev The upgrade authorization function for UUPSProxy. + * @param newImplementation The address of the new implementation. + */ + function _authorizeUpgrade(address newImplementation) internal override { + if (newImplementation == address(0)) { + revert UUPSExtUpgradeable_ImplementationAddressZero(); + } + + if (newImplementation.code.length == 0) { + revert UUPSExtUpgradeable_ImplementationAddressNotContract(); + } + + _validateUpgrade(newImplementation); + } + + /** + * @dev Executes further validation steps of the upgrade including authorization and implementation address checks. + * @param newImplementation The address of the new implementation. + */ + function _validateUpgrade(address newImplementation) internal virtual; +} diff --git a/contracts/base/Versionable.sol b/contracts/base/Versionable.sol index 5621cdb..a5330ad 100644 --- a/contracts/base/Versionable.sol +++ b/contracts/base/Versionable.sol @@ -16,6 +16,6 @@ abstract contract Versionable is IVersionable { * @inheritdoc IVersionable */ function $__VERSION() external pure returns (Version memory) { - return Version(4, 0, 0); + return Version(4, 1, 0); } } diff --git a/contracts/interfaces/ICashier.sol b/contracts/interfaces/ICashier.sol index 3456b93..bf1e082 100644 --- a/contracts/interfaces/ICashier.sol +++ b/contracts/interfaces/ICashier.sol @@ -31,9 +31,6 @@ interface ICashierErrors { /// @dev Thrown if the cash-out operation with the provided txId has an inappropriate status. error Cashier_CashOutStatusInappropriate(); - /// @dev Thrown if the contract is not a cashier root contract. - error Cashier_ContractNotRoot(); - /// @dev Thrown if the contract is not a cashier shard contract. error Cashier_ContractNotShard(); @@ -49,15 +46,12 @@ interface ICashierErrors { /// @dev The provided bit flags to configure the hook logic are invalid. error Cashier_HookFlagsInvalid(); + /// @dev Thrown if the contract is not a cashier root contract. + error Cashier_ImplementationAddressInvalid(); + /// @dev Thrown if the provided release time for the premint operation is inappropriate. error Cashier_PremintReleaseTimeInappropriate(); - /// @dev Thrown if the provided root address is not a contract. - error Cashier_RootAddressNotContract(); - - /// @dev Thrown if the provided root address is zero. - error Cashier_RootAddressZero(); - /// @dev Thrown if the provided shard address is not a contract. error Cashier_ShardAddressNotContract(); diff --git a/contracts/interfaces/ICashierShard.sol b/contracts/interfaces/ICashierShard.sol index a548a9b..b11e6ed 100644 --- a/contracts/interfaces/ICashierShard.sol +++ b/contracts/interfaces/ICashierShard.sol @@ -11,13 +11,7 @@ import { ICashierTypes } from "./ICashierTypes.sol"; */ interface ICashierShardErrors { /// @dev Thrown if the contract is not a cashier shard contract. - error CashierShard_ContractNotShard(); - - /// @dev Thrown if the provided shard address is not a contract. - error CashierShard_ShardAddressNotContract(); - - /// @dev Thrown if the provided shard address is zero. - error CashierShard_ShardAddressZero(); + error CashierShard_ImplementationAddressInvalid(); /// @dev Thrown if the caller is not an admin. error CashierShard_Unauthorized(); diff --git a/contracts/mocks/base/UUPSExtUpgradableMock.sol b/contracts/mocks/base/UUPSExtUpgradableMock.sol new file mode 100644 index 0000000..0e5dcc6 --- /dev/null +++ b/contracts/mocks/base/UUPSExtUpgradableMock.sol @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import { UUPSExtUpgradeable } from "../../base/UUPSExtUpgradeable.sol"; + +/** + * @title UUPSExtUpgradableMock contract + * @author CloudWalk Inc. (See https://www.cloudwalk.io) + * @dev An implementation of the {UUPSExtUpgradable} contract for test purposes. + */ +contract UUPSExtUpgradeableMock is UUPSExtUpgradeable { + /// @dev Emitted when the internal `_validateUpgrade()` function is called with the parameters of the function. + event MockValidateUpgradeCall(address newImplementation); + + /** + * @dev Executes further validation steps of the upgrade including authorization and implementation address checks. + * @param newImplementation The address of the new implementation. + */ + function _validateUpgrade(address newImplementation) internal virtual override { + emit MockValidateUpgradeCall(newImplementation); + } +} diff --git a/test/CashierSharded.test.ts b/test/CashierSharded.test.ts index b95ae16..859cceb 100644 --- a/test/CashierSharded.test.ts +++ b/test/CashierSharded.test.ts @@ -210,7 +210,7 @@ describe("Contracts 'Cashier' and `CashierShard`", async () => { const EXPECTED_VERSION: Version = { major: 4, - minor: 0, + minor: 1, patch: 0 }; @@ -230,20 +230,16 @@ describe("Contracts 'Cashier' and `CashierShard`", async () => { const REVERT_ERROR_IF_CASH_IN_STATUS_INAPPROPRIATE = "Cashier_CashInStatusInappropriate"; const REVERT_ERROR_IF_CASH_OUT_ACCOUNT_INAPPROPRIATE = "Cashier_CashOutAccountInappropriate"; const REVERT_ERROR_IF_CASH_OUT_STATUS_INAPPROPRIATE = "Cashier_CashOutStatusInappropriate"; - const REVERT_ERROR_IF_CONTRACT_NOT_ROOT = "Cashier_ContractNotRoot"; + const REVERT_ERROR_IF_IMPLEMENTATION_ADDRESS_INVALID_ON_ROOT = "Cashier_ImplementationAddressInvalid"; const REVERT_ERROR_IF_HOOK_CALLABLE_CONTRACT_ADDRESS_ZERO = "Cashier_HookCallableContractAddressZero"; const REVERT_ERROR_IF_HOOK_CALLABLE_CONTRACT_ADDRESS_NON_ZERO = "Cashier_HookCallableContractAddressNonZero"; const REVERT_ERROR_IF_HOOK_FLAGS_ALREADY_REGISTERED = "Cashier_HookFlagsAlreadyRegistered"; const REVERT_ERROR_IF_HOOK_FLAGS_INVALID = "Cashier_HookFlagsInvalid"; const REVERT_ERROR_IF_PREMINT_RELEASE_TIME_INAPPROPRIATE = "Cashier_PremintReleaseTimeInappropriate"; - const REVERT_ERROR_IF_ROOT_ADDRESS_IS_NOT_CONTRACT = "Cashier_RootAddressNotContract"; - const REVERT_ERROR_IF_ROOT_ADDRESS_IS_ZERO = "Cashier_RootAddressZero"; const REVERT_ERROR_IF_SHARD_ADDRESS_IS_NOT_CONTRACT_ON_ROOT = "Cashier_ShardAddressNotContract"; const REVERT_ERROR_IF_SHARD_ADDRESS_IS_ZERO_ON_ROOT = "Cashier_ShardAddressZero"; const REVERT_ERROR_IF_CONTRACT_NOT_SHARD_ON_ROOT = "Cashier_ContractNotShard"; - const REVERT_ERROR_IF_SHARD_ADDRESS_IS_NOT_CONTRACT_ON_SHARD = "CashierShard_ShardAddressNotContract"; - const REVERT_ERROR_IF_SHARD_ADDRESS_IS_ZERO_ON_SHARD = "CashierShard_ShardAddressZero"; - const REVERT_ERROR_IF_CONTRACT_NOT_SHARD_ON_SHARD = "CashierShard_ContractNotShard"; + const REVERT_ERROR_IF_CONTRACT_NOT_SHARD_ON_SHARD = "CashierShard_ImplementationAddressInvalid"; const REVERT_ERROR_IF_SHARD_COUNT_EXCESS = "Cashier_ShardCountExcess"; const REVERT_ERROR_IF_SHARD_REPLACEMENT_COUNT_EXCESS = "Cashier_ShardReplacementCountExcess"; const REVERT_ERROR_IF_TOKEN_ADDRESS_IS_ZERO = "Cashier_TokenAddressZero"; @@ -811,7 +807,7 @@ describe("Contracts 'Cashier' and `CashierShard`", async () => { it("Is reverted if the caller is not the owner for the root contract", async () => { const { cashierRoot } = await setUpFixture(deployContracts); - await expect(connect(cashierRoot, user).upgradeToAndCall(user.address, "0x")) + await expect(connect(cashierRoot, user).upgradeToAndCall(cashierRoot, "0x")) .to.be.revertedWithCustomError(cashierRoot, REVERT_ERROR_IF_UNAUTHORIZED_ACCOUNT) .withArgs(user.address, ownerRole); }); @@ -819,7 +815,7 @@ describe("Contracts 'Cashier' and `CashierShard`", async () => { it("Is reverted if the caller is not the owner or an admin for the shard contract", async () => { const anotherCashierShard: Contract = await upgrades.deployProxy(cashierShardFactory, [deployer.address]); - await expect(connect(anotherCashierShard, user).upgradeToAndCall(user.address, "0x")) + await expect(connect(anotherCashierShard, user).upgradeToAndCall(anotherCashierShard, "0x")) .to.be.revertedWithCustomError(anotherCashierShard, REVERT_ERROR_IF_UNAUTHORIZED); }); @@ -828,7 +824,7 @@ describe("Contracts 'Cashier' and `CashierShard`", async () => { const wrongRootImplementationAddress = await getImplementationAddress(cashierShards[0]); await expect(cashierRoot.upgradeToAndCall(wrongRootImplementationAddress, "0x")) - .to.be.revertedWithCustomError(cashierRoot, REVERT_ERROR_IF_CONTRACT_NOT_ROOT); + .to.be.revertedWithCustomError(cashierRoot, REVERT_ERROR_IF_IMPLEMENTATION_ADDRESS_INVALID_ON_ROOT); }); it("Is reverted if the provided shard implementation is not a shard contract", async () => { @@ -839,30 +835,6 @@ describe("Contracts 'Cashier' and `CashierShard`", async () => { await expect(anotherCashierShard.upgradeToAndCall(wrongShardImplementationAddress, "0x")) .to.be.revertedWithCustomError(anotherCashierShard, REVERT_ERROR_IF_CONTRACT_NOT_SHARD_ON_SHARD); }); - - it("Is reverted if the provided root address is not a contract", async () => { - const { cashierRoot } = await setUpFixture(deployContracts); - await expect(cashierRoot.upgradeToAndCall(user.address, "0x")) - .to.be.revertedWithCustomError(cashierRoot, REVERT_ERROR_IF_ROOT_ADDRESS_IS_NOT_CONTRACT); - }); - - it("Is reverted if the provided root address is zero", async () => { - const { cashierRoot } = await setUpFixture(deployContracts); - await expect(cashierRoot.upgradeToAndCall(ADDRESS_ZERO, "0x")) - .to.be.revertedWithCustomError(cashierRoot, REVERT_ERROR_IF_ROOT_ADDRESS_IS_ZERO); - }); - - it("Is reverted if the provided shard address is not a contract", async () => { - const anotherCashierShard: Contract = await upgrades.deployProxy(cashierShardFactory, [deployer.address]); - await expect(anotherCashierShard.upgradeToAndCall(user.address, "0x")) - .to.be.revertedWithCustomError(anotherCashierShard, REVERT_ERROR_IF_SHARD_ADDRESS_IS_NOT_CONTRACT_ON_SHARD); - }); - - it("Is reverted if the provided shard address is zero", async () => { - const anotherCashierShard: Contract = await upgrades.deployProxy(cashierShardFactory, [deployer.address]); - await expect(anotherCashierShard.upgradeToAndCall(ADDRESS_ZERO, "0x")) - .to.be.revertedWithCustomError(anotherCashierShard, REVERT_ERROR_IF_SHARD_ADDRESS_IS_ZERO_ON_SHARD); - }); }); describe("Function 'upgradeTo()'", async () => { @@ -898,7 +870,7 @@ describe("Contracts 'Cashier' and `CashierShard`", async () => { const wrongRootImplementationAddress = await getImplementationAddress(cashierShards[0]); await expect(cashierRoot.upgradeTo(wrongRootImplementationAddress)) - .to.be.revertedWithCustomError(cashierRoot, REVERT_ERROR_IF_CONTRACT_NOT_ROOT); + .to.be.revertedWithCustomError(cashierRoot, REVERT_ERROR_IF_IMPLEMENTATION_ADDRESS_INVALID_ON_ROOT); }); it("Is reverted if the provided shard implementation is not a shard contract", async () => { @@ -909,30 +881,6 @@ describe("Contracts 'Cashier' and `CashierShard`", async () => { await expect(anotherCashierShard.upgradeTo(wrongShardImplementationAddress)) .to.be.revertedWithCustomError(anotherCashierShard, REVERT_ERROR_IF_CONTRACT_NOT_SHARD_ON_SHARD); }); - - it("Is reverted if the provided root address is not a contract", async () => { - const { cashierRoot } = await setUpFixture(deployContracts); - await expect(cashierRoot.upgradeTo(user.address)) - .to.be.revertedWithCustomError(cashierRoot, REVERT_ERROR_IF_ROOT_ADDRESS_IS_NOT_CONTRACT); - }); - - it("Is reverted if the provided root address is zero", async () => { - const { cashierRoot } = await setUpFixture(deployContracts); - await expect(cashierRoot.upgradeTo(ADDRESS_ZERO)) - .to.be.revertedWithCustomError(cashierRoot, REVERT_ERROR_IF_ROOT_ADDRESS_IS_ZERO); - }); - - it("Is reverted if the provided shard address is not a contract", async () => { - const anotherCashierShard: Contract = await upgrades.deployProxy(cashierShardFactory, [deployer.address]); - await expect(anotherCashierShard.upgradeTo(user.address)) - .to.be.revertedWithCustomError(anotherCashierShard, REVERT_ERROR_IF_SHARD_ADDRESS_IS_NOT_CONTRACT_ON_SHARD); - }); - - it("Is reverted if the provided shard address is zero", async () => { - const anotherCashierShard: Contract = await upgrades.deployProxy(cashierShardFactory, [deployer.address]); - await expect(anotherCashierShard.upgradeTo(ADDRESS_ZERO)) - .to.be.revertedWithCustomError(anotherCashierShard, REVERT_ERROR_IF_SHARD_ADDRESS_IS_ZERO_ON_SHARD); - }); }); describe("Function 'initHookAdminRole()'", async () => { @@ -1152,20 +1100,6 @@ describe("Contracts 'Cashier' and `CashierShard`", async () => { ).withArgs(user.address, ownerRole); }); - it("Is reverted if the shard implementation address is zero", async () => { - const { cashierRoot, cashierShards } = await setUpFixture(deployAndConfigureContracts); - await expect( - cashierRoot.upgradeShardsTo(ADDRESS_ZERO) - ).to.be.revertedWithCustomError(cashierShards[0], REVERT_ERROR_IF_SHARD_ADDRESS_IS_ZERO_ON_SHARD); - }); - - it("Is reverted if the provided shard address is not a contract", async () => { - const { cashierRoot, cashierShards } = await setUpFixture(deployAndConfigureContracts); - await expect( - cashierRoot.upgradeShardsTo(user.address) - ).to.be.revertedWithCustomError(cashierShards[0], REVERT_ERROR_IF_SHARD_ADDRESS_IS_NOT_CONTRACT_ON_SHARD); - }); - it("Is reverted if the shard implementation address is not a shard contract", async () => { const { cashierRoot, cashierShards } = await setUpFixture(deployAndConfigureContracts); const wrongShardImplementationAddress = await getImplementationAddress(cashierRoot); @@ -1232,41 +1166,6 @@ describe("Contracts 'Cashier' and `CashierShard`", async () => { ).withArgs(user.address, ownerRole); }); - it("Is reverted if the root implementation address is zero", async () => { - const { cashierRoot, cashierShards } = await setUpFixture(deployAndConfigureContracts); - const targetShardImplementationAddress = await getImplementationAddress(cashierShards[0]); - - await expect( - cashierRoot.upgradeRootAndShardsTo( - ADDRESS_ZERO, - targetShardImplementationAddress - ) - ).to.be.revertedWithCustomError(cashierRoot, REVERT_ERROR_IF_ROOT_ADDRESS_IS_ZERO); - }); - - it("Is reverted if the shard implementation address is zero", async () => { - const { cashierRoot, cashierShards } = await setUpFixture(deployAndConfigureContracts); - const targetRootImplementationAddress = await getImplementationAddress(cashierRoot); - - await expect( - cashierRoot.upgradeRootAndShardsTo( - targetRootImplementationAddress, - ADDRESS_ZERO - ) - ).to.be.revertedWithCustomError(cashierShards[0], REVERT_ERROR_IF_SHARD_ADDRESS_IS_ZERO_ON_SHARD); - }); - - it("Is reverted if the provided root address is not a contract", async () => { - const { cashierRoot, cashierShards } = await setUpFixture(deployAndConfigureContracts); - const targetShardImplementationAddress = await getImplementationAddress(cashierShards[0]); - await expect( - cashierRoot.upgradeRootAndShardsTo( - user.address, - targetShardImplementationAddress - ) - ).to.be.revertedWithCustomError(cashierRoot, REVERT_ERROR_IF_ROOT_ADDRESS_IS_NOT_CONTRACT); - }); - it("Is reverted if the provided root implementation is not a cashier root contract", async () => { const { cashierRoot, cashierShards } = await setUpFixture(deployAndConfigureContracts); const targetShardImplementationAddress = await getImplementationAddress(cashierShards[0]); @@ -1277,18 +1176,7 @@ describe("Contracts 'Cashier' and `CashierShard`", async () => { wrongRootImplementationAddress, targetShardImplementationAddress ) - ).to.be.revertedWithCustomError(cashierRoot, REVERT_ERROR_IF_CONTRACT_NOT_ROOT); - }); - - it("Is reverted if the provided shard address is not a contract", async () => { - const { cashierRoot, cashierShards } = await setUpFixture(deployAndConfigureContracts); - const targetRootImplementationAddress = await getImplementationAddress(cashierRoot); - await expect( - cashierRoot.upgradeRootAndShardsTo( - targetRootImplementationAddress, - user.address - ) - ).to.be.revertedWithCustomError(cashierShards[0], REVERT_ERROR_IF_SHARD_ADDRESS_IS_NOT_CONTRACT_ON_SHARD); + ).to.be.revertedWithCustomError(cashierRoot, REVERT_ERROR_IF_IMPLEMENTATION_ADDRESS_INVALID_ON_ROOT); }); it("Is reverted if the shard implementation address is not a cashier shard contract", async () => { diff --git a/test/base/UUPSExtUbgradable.test.ts b/test/base/UUPSExtUbgradable.test.ts new file mode 100644 index 0000000..5288026 --- /dev/null +++ b/test/base/UUPSExtUbgradable.test.ts @@ -0,0 +1,70 @@ +import { ethers, network, upgrades } from "hardhat"; +import { expect } from "chai"; +import { Contract, ContractFactory } from "ethers"; +import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers"; +import { loadFixture } from "@nomicfoundation/hardhat-network-helpers"; +import { connect } from "../../test-utils/eth"; + +const ADDRESS_ZERO = ethers.ZeroAddress; + +async function setUpFixture(func: () => Promise): Promise { + if (network.name === "hardhat") { + return loadFixture(func); + } else { + return func(); + } +} + +describe("Contracts 'UUPSExtUpgradeable'", async () => { + // Errors of the lib contracts + const REVERT_ERROR_IMPLEMENTATION_ADDRESS_NOT_CONTRACT = "UUPSExtUpgradeable_ImplementationAddressNotContract"; + const REVERT_ERROR_IMPLEMENTATION_ADDRESS_ZERO = "UUPSExtUpgradeable_ImplementationAddressZero"; + + // Events of the contracts under test + const EVENT_NAME_MOCK_VALIDATE_UPGRADE_CALL = "MockValidateUpgradeCall"; + + let uupsExtensionFactory: ContractFactory; + let deployer: HardhatEthersSigner; + + before(async () => { + [deployer] = await ethers.getSigners(); + + // The contract factory with the explicitly specified deployer account + uupsExtensionFactory = await ethers.getContractFactory("UUPSExtUpgradeableMock"); + uupsExtensionFactory = uupsExtensionFactory.connect(deployer); + }); + + async function deployContract(): Promise<{ uupsExtension: Contract }> { + let uupsExtension: Contract = await upgrades.deployProxy(uupsExtensionFactory, [], { initializer: false }); + await uupsExtension.waitForDeployment(); + uupsExtension = connect(uupsExtension, deployer); // Explicitly specifying the initial account + + return { uupsExtension }; + } + + describe("Function 'upgradeToAndCall()'", async () => { + it("Executes as expected", async () => { + const { uupsExtension } = await setUpFixture(deployContract); + + const newImplementation = await uupsExtensionFactory.deploy(); + await newImplementation.waitForDeployment(); + const newImplementationAddress = await newImplementation.getAddress(); + + await expect(uupsExtension.upgradeToAndCall(newImplementationAddress, "0x")) + .to.emit(uupsExtension, EVENT_NAME_MOCK_VALIDATE_UPGRADE_CALL) + .withArgs(newImplementationAddress); + }); + + it("Is reverted if the new implementation address is zero", async () => { + const { uupsExtension } = await setUpFixture(deployContract); + await expect(uupsExtension.upgradeToAndCall(ADDRESS_ZERO, "0x")) + .to.be.revertedWithCustomError(uupsExtension, REVERT_ERROR_IMPLEMENTATION_ADDRESS_ZERO); + }); + + it("Is reverted if the new implementation address is not a contract", async () => { + const { uupsExtension } = await setUpFixture(deployContract); + await expect(uupsExtension.upgradeToAndCall(deployer.address, "0x")) + .to.be.revertedWithCustomError(uupsExtension, REVERT_ERROR_IMPLEMENTATION_ADDRESS_NOT_CONTRACT); + }); + }); +});