Merge pull request #33 from dadadave80/dev

refactor!: make Diamond an abstract fallback-only base

Author: dadadave80

DEVELOPER_GUIDE.md

@@ -237,12 +237,11 @@ bytes32 constant STORAGE_LOCATION_2 = keccak256(abi.encode("1"));
 
 ```solidity
 contract DiamondTest is Test {
-    Diamond diamond;
+    MyDiamond diamond;
     
     function setUp() public {
-        // Deploy and initialize
-        diamond = new Diamond();
-        // ... initialization code ...
+        // Constructor applies the cuts and runs the init contract
+        diamond = new MyDiamond(cuts, address(diamondInit), initData);
     }
     
     function test_DiamondInitialized() public {

GLOSSARY.md

@@ -144,8 +144,8 @@ Process of setting up the Diamond when it's first deployed.
 - Must be atomic (all succeed or all fail)
 
 **Process**:
-1. Deploy Diamond (empty, no functions yet)
-2. Call `initialize(cuts, initContract, initData)`
+1. Deploy facets and the initializer contract
+2. Deploy your concrete diamond — its constructor calls `diamondCut(cuts, initContract, initData)`
 3. Facets are added via diamond cuts
 4. Initialization contract runs (sets owner, etc.)
 5. Diamond is ready
@@ -165,27 +165,6 @@ function init() external {
 
 **Execution**: Runs via delegatecall in Diamond's context
 
-### Reinitialization
-Updating initialization state without resetting everything.
-
-**Use case**: Upgrade with new state variables
-
-**Protection**: Version tracking prevents accidental reinitialization
-- v1 → v2 (success)
-- v2 → v2 (failure: already at this version)
-- v2 → v1 (failure: cannot go backward)
-
-### Initialization Guard
-Mechanism preventing reentrancy during initialization.
-
-**Implementation**:
-```solidity
-// Flag: 0 = not init, 1 = currently initializing, 2 = done
-if (isInitializing) revert InvalidInitialization();
-```
-
-**Prevents**: Accidentally calling initialization again during initialization
-
 ---
 
 ## Access Control
@@ -422,13 +401,6 @@ Function selector doesn't route to any facet.
 
 **Fix**: Use loupe to verify `facetAddress(selector)` returns non-zero
 
-### "Invalid initialization" Error
-Attempt to reinitialize to same or lower version.
-
-**Cause**: Already initialized and version unchanged
-
-**Fix**: Increment version number for reinitializations
-
 ### "Cannot add function to diamond that already exists" Error
 Selector already has a mapping.
 

README.md

@@ -60,7 +60,6 @@ src/
 ├── libraries/
 │   ├── DiamondLib.sol         # Core Diamond logic
 │   ├── OwnableLib.sol         # Ownership primitives
-│   ├── InitializableLib.sol   # Initialization guards
 │   └── ERC165Lib.sol          # Interface registration
 └── script/
     └── DeployDiamond.s.sol    # Foundry deployment script

SPECIFICATION.md

@@ -39,7 +39,6 @@ graph TB
     
     DiamondLib["DiamondLib<br/>Storage & cuts"]
     OwnableLib["OwnableLib<br/>Owner state"]
-    InitLib["InitializableLib<br/>Init guards"]
     ERC165Lib["ERC165Lib<br/>Interface registry"]
     
     SharedStorage["Shared Storage<br/>Diamond + Libraries"]
@@ -57,11 +56,9 @@ graph TB
     Own --> OwnableLib
     Custom --> DiamondLib
     Custom --> OwnableLib
-    Custom --> InitLib
     
     DiamondLib --> SharedStorage
     OwnableLib --> SharedStorage
-    InitLib --> SharedStorage
     ERC165Lib --> SharedStorage
 ```
 
@@ -182,15 +179,15 @@ Initialization differs from constructors because Diamond is a proxy:
 **Solution: Delegatecall-based initialization:**
 
 ```solidity
-// Initialize during Diamond deployment
-Diamond.initialize(
-    [facetCuts...],
-    address(diamondInit),
-    abi.encodeCall(DiamondInit.init, ())
-);
+// Concrete diamond: cut facets and run the init contract atomically at deployment
+contract MyDiamond is Diamond {
+    constructor(FacetCut[] memory _facetCuts, address _init, bytes memory _calldata) {
+        DiamondLib.diamondCut(_facetCuts, _init, _calldata);
+    }
+}
 ```
 
-Functions run in Diamond's delegatecall context, so initialization happens in the right storage.
+The init contract runs in the Diamond's delegatecall context, so initialization happens in the right storage. For factory/CREATE2/clone deployments where a constructor cannot run per-instance, expose an initializer-guarded `initialize` function in your concrete diamond instead (see `test/mocks/MockDiamond.sol` for a reference).
 
 ---
 
@@ -256,27 +253,6 @@ Single 32-byte slot:
 - Atomic state transitions (single SSTORE)
 - Storage efficiency
 
-### Initialization Storage Location
-
-**Namespace**: Initialization state  
-**Location**: `0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffbf601132`
-
-```
-Bit-packed single slot:
-  Bit 0:        initializing flag (1 = currently initializing)
-  Bits 1-64:    initialized version number
-```
-
-**Example values:**
-- `0x0` = never initialized
-- `0x3` = initializing version 1 (bits: 1 | (1<<1))
-- `0x2` = initialized v1, not initializing
-
-**Why bit packing:**
-- Two values in one storage slot
-- Atomic updates (single SSTORE)
-- Fast assembly checks
-
 ### ERC165 Interface Support
 
 **Namespace**: `"diamond.lib.storage.ERC165"`  
@@ -302,46 +278,22 @@ struct ERC165Storage {
 
 ```mermaid
 sequenceDiagram
-    participant User
+    participant Deployer
     participant Diamond
-    participant InitLib
     participant DiamondLib
     participant InitContract
     
-    User->>Diamond: initialize(cuts, initAddr, initData)
-    Diamond->>InitLib: preInitializer()
-    InitLib->>InitLib: set initializing=true, version=1
-    InitLib->>Diamond: continue
-    
+    Deployer->>Diamond: constructor(cuts, initAddr, initData)
     Diamond->>DiamondLib: diamondCut(cuts)
     DiamondLib->>DiamondLib: validate & apply all cuts
     DiamondLib->>Diamond: emit DiamondCut event
     
     Diamond->>InitContract: delegatecall(initData)
     InitContract->>Diamond: write state (owner, etc)
     InitContract-->>Diamond: return
-    
-    Diamond->>InitLib: postInitializer()
-    InitLib->>InitLib: set initializing=false, version++
-    InitLib->>Diamond: emit Initialized event
-```
-
-### Reinitialization Prevention
-
-**Problem**: Without guards, anyone could reset state by calling init again.
-
-**Solution: Version tracking**
-
-```
-First init:    version 0 → 1 (success)
-Retry same:    version 1 → 1 (fails: InvalidInitialization)
-Upgrade init:  version 1 → 2 (success: reinitializer)
 ```
 
-**Guarantees:**
-- Cannot reinitialize to same or lower version
-- Can upgrade to higher version (supports versioned upgrades)
-- Initialization flag prevents reentrancy during init
+Running the cut in the constructor is atomic: the diamond is never observable in a facetless, ownerless state, and there is no separate `initialize` transaction to front-run.
 
 ### MultiInit for Complex Setup
 

script/DeployDiamond.s.sol

@@ -12,7 +12,6 @@ import {ERC165Init} from "@diamond/initializers/ERC165Init.sol";
 import {MultiInit} from "@diamond/initializers/MultiInit.sol";
 import {OwnableInit} from "@diamond/initializers/OwnableInit.sol";
 import {IFacet} from "@diamond/interfaces/IFacet.sol";
-import {ContextLib} from "@diamond/libraries/ContextLib.sol";
 import {FacetCut, FacetCutAction} from "@diamond/libraries/DiamondLib.sol";
 import {Script} from "forge-std/Script.sol";
 
@@ -70,7 +69,7 @@ contract DeployDiamond is Script {
 
         // Deploy the Diamond contract with the facets and initialization args
         MockDiamond diamond = new MockDiamond();
-        diamond.initialize(cut, diamondInit, abi.encodeWithSignature("init(address)", ContextLib.msgSender()));
+        diamond.initialize(cut, diamondInit, abi.encodeWithSignature("init(address)", msg.sender));
         diamond_ = address(diamond);
         vm.stopBroadcast();
     }

src/Diamond.sol

@@ -1,8 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.20;
 
-import {DiamondLib, FacetCut} from "@diamond/libraries/DiamondLib.sol";
-import {InitializableLib} from "@diamond/libraries/InitializableLib.sol";
+import {DiamondLib} from "@diamond/libraries/DiamondLib.sol";
 
 /*
 ⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⢀⣀⣀⣀⣀⣀⣀⣀⣀⣀⣀
@@ -35,23 +34,7 @@ import {InitializableLib} from "@diamond/libraries/InitializableLib.sol";
 /// @notice Implements ERC-2535 Diamond proxy pattern, allowing dynamic addition, replacement, and removal of facets
 /// @author David Dada <daveproxy80@gmail.com> (https://github.com/dadadave80)
 /// @author Modified from Nick Mudge (https://github.com/mudgen/diamond-3-hardhat/blob/main/contracts/Diamond.sol)
-contract Diamond {
-    /// @notice Initializes the Diamond proxy with the provided facets and initialization parameters
-    /// @param _init Address of the initialization contract
-    /// @param _calldata Calldata to be passed to the initialization contract
-    function initialize(FacetCut[] calldata _facetCuts, address _init, bytes calldata _calldata)
-        public
-        payable
-        virtual
-    {
-        bytes32 s = InitializableLib.initializableSlot();
-        InitializableLib.preInitializer(s);
-
-        DiamondLib.diamondCut(_facetCuts, _init, _calldata);
-
-        InitializableLib.postInitializer(s);
-    }
-
+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 {
@@ -76,6 +59,4 @@ contract Diamond {
             }
         }
     }
-
-    receive() external payable virtual {}
 }

src/libraries/ContextLib.sol

@@ -3,6 +3,8 @@ pragma solidity ^0.8.20;
 
 import {Selectors} from "@diamond-test/helpers/Selectors.sol";
 import {ReinitializableDiamond} from "@diamond-test/mocks/ReinitializableDiamond.sol";
+import {Initializable} from "@diamond-test/utils/Initializable.sol";
+import {Initialized, InvalidInitialization} from "@diamond-test/utils/InitializableLib.sol";
 import {DiamondCutFacet} from "@diamond/facets/DiamondCutFacet.sol";
 import {DiamondLoupeFacet} from "@diamond/facets/DiamondLoupeFacet.sol";
 import {ERC165Facet} from "@diamond/facets/ERC165Facet.sol";
@@ -12,10 +14,24 @@ import {ERC165Init} from "@diamond/initializers/ERC165Init.sol";
 import {MultiInit} from "@diamond/initializers/MultiInit.sol";
 import {OwnableInit} from "@diamond/initializers/OwnableInit.sol";
 import {IFacet} from "@diamond/interfaces/IFacet.sol";
-import {ContextLib} from "@diamond/libraries/ContextLib.sol";
 import {FacetCut, FacetCutAction} from "@diamond/libraries/DiamondLib.sol";
-import {Initialized, InvalidInitialization} from "@diamond/libraries/InitializableLib.sol";
-import {Test} from "forge-std/Test.sol";
+import {Test, Vm} from "forge-std/Test.sol";
+
+/// @dev Exercises nested `initializer` calls within a constructor.
+contract NestedInitMock is Initializable {
+    bool public wasInitializingAfterNested;
+
+    constructor() {
+        outer();
+    }
+
+    function outer() public initializer {
+        inner();
+        wasInitializingAfterNested = _isInitializing();
+    }
+
+    function inner() public initializer {}
+}
 
 /// @title InitializableTester
 /// @notice Tests for the initializable Diamond pattern
@@ -106,6 +122,22 @@ contract InitializableTester is Test {
         assertFalse(diamond.isInitializing());
     }
 
+    /// @notice A nested initializer inside a constructor finalizes exactly once:
+    /// the inner call must not clear the initializing flag or emit its own event
+    function testNestedConstructorInitializerFinalizesOnce() public {
+        vm.recordLogs();
+        NestedInitMock mock = new NestedInitMock();
+
+        uint256 count;
+        Vm.Log[] memory logs = vm.getRecordedLogs();
+        for (uint256 i; i < logs.length; ++i) {
+            if (logs[i].topics[0] == keccak256("Initialized(uint64)")) ++count;
+        }
+
+        assertEq(count, 1);
+        assertTrue(mock.wasInitializingAfterNested());
+    }
+
     /// @notice Second call to initialize reverts with InvalidInitialization
     function testCannotInitializeTwice() public {
         (FacetCut[] memory facetCuts, address init, bytes memory initCalldata) = _buildInitArgs(address(this));

test/InitializableTester.t.sol

@@ -1,6 +1,25 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.20;
 
-import {Diamond, FacetCut} from "@diamond/Diamond.sol";
+import {Initializable} from "@diamond-test/utils/Initializable.sol";
+import {Diamond} from "@diamond/Diamond.sol";
+import {DiamondLib, FacetCut} from "@diamond/libraries/DiamondLib.sol";
 
-contract MockDiamond is Diamond {}
+/// @notice Reference initializable diamond: the abstract `Diamond` base composed
+/// with `Initializable` for factory/CREATE2 deployments where a constructor cut
+/// cannot run per-instance.
+contract MockDiamond is Diamond, Initializable {
+    /// @notice Adds the provided facets and runs the initialization contract, at most once
+    /// @param _init Address of the initialization contract
+    /// @param _calldata Calldata to be passed to the initialization contract
+    function initialize(FacetCut[] calldata _facetCuts, address _init, bytes calldata _calldata)
+        public
+        payable
+        virtual
+        initializer
+    {
+        DiamondLib.diamondCut(_facetCuts, _init, _calldata);
+    }
+
+    receive() external payable virtual {}
+}