Merge pull request #22 from dadadave80:chore/replace-ownable-roles-with-ownable

Refactor and enhance Diamond library with initializable pattern

Author: dadadave80

script/DeployDiamond.s.sol

@@ -5,7 +5,7 @@ import {GetSelectors} from "@diamond-test/helpers/GetSelectors.sol";
 import {MockDiamond} from "@diamond-test/mocks/MockDiamond.sol";
 import {DiamondCutFacet} from "@diamond/facets/DiamondCutFacet.sol";
 import {DiamondLoupeFacet} from "@diamond/facets/DiamondLoupeFacet.sol";
-import {OwnableRolesFacet} from "@diamond/facets/OwnableRolesFacet.sol";
+import {OwnableFacet} from "@diamond/facets/OwnableFacet.sol";
 import {ERC165Init} from "@diamond/initializers/ERC165Init.sol";
 import {MultiInit} from "@diamond/initializers/MultiInit.sol";
 import {OwnableInit} from "@diamond/initializers/OwnableInit.sol";
@@ -17,7 +17,7 @@ import {Script} from "forge-std/Script.sol";
 /// @notice Deployment script for an EIP-2535 Diamond proxy contract with core facets and ERC165 initialization
 /// @author David Dada
 ///
-/// @dev Uses Foundry's `Script` and a helper contract to deploy and wire up DiamondCutFacet, DiamondLoupeFacet, and OwnableRolesFacet
+/// @dev Uses Foundry's `Script` and a helper contract to deploy and wire up DiamondCutFacet, DiamondLoupeFacet, and OwnableFacet
 contract DeployDiamond is Script, GetSelectors {
     /// @notice Executes the deployment of the Diamond contract with the initial facets and ERC165 interface setup
     /// @dev Broadcasts transactions using Foundry's scripting environment (`vm.startBroadcast()` and `vm.stopBroadcast()`).
@@ -28,7 +28,7 @@ contract DeployDiamond is Script, GetSelectors {
         // Deploy core facet contracts
         DiamondCutFacet diamondCutFacet = new DiamondCutFacet();
         DiamondLoupeFacet diamondLoupeFacet = new DiamondLoupeFacet();
-        OwnableRolesFacet ownableRolesFacet = new OwnableRolesFacet();
+        OwnableFacet ownableFacet = new OwnableFacet();
 
         // Deploy initializer contracts
         address multiInit = address(new MultiInit());
@@ -52,11 +52,11 @@ contract DeployDiamond is Script, GetSelectors {
             functionSelectors: _getSelectors("DiamondLoupeFacet")
         });
 
-        // Add OwnableRolesFacet to the cut list
+        // Add OwnableFacet to the cut list
         cut[2] = FacetCut({
-            facetAddress: address(ownableRolesFacet),
+            facetAddress: address(ownableFacet),
             action: FacetCutAction.Add,
-            functionSelectors: _getSelectors("OwnableRolesFacet")
+            functionSelectors: _getSelectors("OwnableFacet")
         });
 
         // Build MultiInit arrays for granular initialization

src/Diamond.sol

@@ -55,26 +55,13 @@ abstract contract Diamond {
     /// @notice Fallback function that delegates calls to the appropriate facet based on function selector
     /// @dev Reads the facet address from diamond storage and performs a delegatecall; reverts if selector is not found
     fallback() external payable virtual {
-        _delegate(_facet());
-    }
-
-    receive() external payable virtual {}
-
-    /// @notice Retrieves the implementation address for the current function call
-    /// @dev A Facet is one of many implementations in a Diamond Proxy
-    function _facet() internal view virtual returns (address) {
-        return DiamondLib.selectorToFacet(msg.sig);
-    }
-
-    /// @notice Internal function to perform a delegatecall to an implementation
-    /// @param _implementation Address of the implementation to delegate to
-    function _delegate(address _implementation) internal virtual {
+        address implementation = DiamondLib.selectorToFacet(msg.sig);
         assembly {
             // Copy calldata to memory
             calldatacopy(0, 0, calldatasize())
 
             // Delegate call to implementation
-            let result := delegatecall(gas(), _implementation, 0, calldatasize(), 0, 0)
+            let result := delegatecall(gas(), implementation, 0, calldatasize(), 0, 0)
 
             // Copy returned data
             returndatacopy(0, 0, returndatasize())
@@ -89,4 +76,6 @@ abstract contract Diamond {
             }
         }
     }
+
+    receive() external payable virtual {}
 }

src/facets/DiamondCutFacet.sol

@@ -3,7 +3,7 @@ pragma solidity ^0.8.20;
 
 import {IDiamondCut} from "@diamond/interfaces/IDiamondCut.sol";
 import {DiamondLib, FacetCut} from "@diamond/libraries/DiamondLib.sol";
-import {OwnableRolesLib} from "@diamond/libraries/OwnableRolesLib.sol";
+import {OwnableLib} from "@diamond/libraries/OwnableLib.sol";
 
 /// @title DiamondCutFacet
 /// @notice Simple single owner and multiroles authorization mixin.
@@ -22,7 +22,7 @@ contract DiamondCutFacet is IDiamondCut {
     ///                  _calldata is executed with delegatecall on _init
     function diamondCut(FacetCut[] calldata _diamondCut, address _init, bytes calldata _calldata) external payable {
         // Check that the caller is the owner
-        OwnableRolesLib.checkOwner();
+        OwnableLib.checkOwner();
         // Call the diamond cut function from the library
         DiamondLib.diamondCut(_diamondCut, _init, _calldata);
     }

src/facets/OwnableFacet.sol

@@ -0,0 +1,73 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.20;
+
+import {OwnableLib} from "@diamond/libraries/OwnableLib.sol";
+
+/// @title OwnableRolesFacet
+/// @notice Simple single owner and multiroles authorization mixin.
+/// @author David Dada
+/// @author Modified from Solady (https://github.com/vectorized/solady/blob/main/src/auth/OwnableRoles.sol)
+///
+/// @dev Note:
+/// This implementation does NOT auto-initialize the owner to `msg.sender`.
+/// You MUST call the `_initializeOwner` in the constructor / initializer.
+///
+/// While the ownable portion follows
+/// [EIP-173](https://eips.ethereum.org/EIPS/eip-173) for compatibility,
+/// the nomenclature for the 2-step ownership handover may be unique to this codebase.
+contract OwnableFacet {
+    /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
+    /*                  PUBLIC UPDATE FUNCTIONS                   */
+    /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/
+
+    /// @dev Allows the owner to transfer the ownership to `newOwner`.
+    function transferOwnership(address _newOwner) public onlyOwner {
+        OwnableLib.transferOwnership(_newOwner);
+    }
+
+    /// @dev Allows the owner to renounce their ownership.
+    function renounceOwnership() public onlyOwner {
+        OwnableLib.renounceOwnership();
+    }
+
+    /// @dev Request a two-step ownership handover to the caller.
+    /// The request will automatically expire in 48 hours (172800 seconds) by default.
+    function requestOwnershipHandover() public {
+        OwnableLib.requestOwnershipHandover();
+    }
+
+    /// @dev Cancels the two-step ownership handover to the caller, if any.
+    function cancelOwnershipHandover() public {
+        OwnableLib.cancelOwnershipHandover();
+    }
+
+    /// @dev Allows the owner to complete the two-step ownership handover to `pendingOwner`.
+    /// Reverts if there is no existing ownership handover requested by `pendingOwner`.
+    function completeOwnershipHandover(address _pendingOwner) public onlyOwner {
+        OwnableLib.completeOwnershipHandover(_pendingOwner);
+    }
+
+    /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
+    /*                   PUBLIC READ FUNCTIONS                    */
+    /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/
+
+    /// @dev Returns the owner of the contract.
+    function owner() public view returns (address) {
+        return OwnableLib.owner();
+    }
+
+    /// @dev Returns the expiry timestamp for the two-step ownership handover to `pendingOwner`.
+    function ownershipHandoverExpiresAt(address _pendingOwner) public view returns (uint256) {
+        return OwnableLib.ownershipHandoverExpiresAt(_pendingOwner);
+    }
+
+    /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
+    /*                         MODIFIERS                          */
+    /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/
+
+    /// @dev Marks a function as only callable by the owner.
+    modifier onlyOwner() {
+        OwnableLib.checkOwner();
+        _;
+    }
+}

src/facets/OwnableRolesFacet.sol

@@ -7,6 +7,10 @@ error AddressAndCalldataLengthMismatch();
 error NoBytecodeAtAddress(address initAddress);
 error InitializeReverted(address initAddress, bytes initCalldata);
 
+/// @title MultiInit
+/// @author David Dada <daveproxy80@gmail.com> (https://github.com/dadadave80)
+/// @notice Provides a utility function to perform multiple delegatecall initializations in a single transaction,
+///         with robust error handling and validation.
 contract MultiInit {
     function multiInit(address[] calldata _initAddresses, bytes[] calldata _initData) public {
         uint256 initAddressesLength = _initAddresses.length;
@@ -23,8 +27,8 @@ contract MultiInit {
                 if (err.length > 0) {
                     // bubble up error
                     assembly ("memory-safe") {
-                        let returndata_size := mload(err)
-                        revert(add(32, err), returndata_size)
+                        let returndataSize := mload(err)
+                        revert(add(32, err), returndataSize)
                     }
                 } else {
                     revert InitializeReverted(initAddress, _initData[i]);

src/initializers/DiamondInit.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.20;
 
-import {OwnableRolesLib} from "@diamond/libraries/OwnableRolesLib.sol";
+import {OwnableLib} from "@diamond/libraries/OwnableLib.sol";
 
 /// @title OwnableInit
 /// @notice Provides an initializer to set the contract owner
@@ -13,6 +13,6 @@ contract OwnableInit {
     /// @dev This function is called during the diamond cut process to set up the owner.
     /// @param _owner The address to set as the contract owner.
     function initOwner(address _owner) public {
-        OwnableRolesLib.initializeOwner(_owner);
+        OwnableLib.initializeOwner(_owner);
     }
 }