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 <[email protected]>
cloudwalk · Nov 18, 2024 · b8a0083 · b8a0083
1 parent 76b6adf
commit b8a0083
Show file tree

Hide file tree

Showing 9 changed files with 167 additions and 182 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
+    }
+}