Merge pull request Perfect-Abstractions#272 from maxnorm/fix-storage-positions

maxnorm · web-flow · commit 4342a053d418 · 2026-02-20T21:14:38.000-05:00
review and fix storage position definitions
diff --git a/src/access/AccessControl/AccessControlFacet.sol b/src/access/AccessControl/AccessControlFacet.sol
@@ -56,6 +56,7 @@ contract AccessControlFacet {
 
     /**
      * @notice storage struct for the AccessControl.
+     * @custom:storage-location erc8042:compose.accesscontrol
      */
     struct AccessControlStorage {
         mapping(address account => mapping(bytes32 role => bool hasRole)) hasRole;
diff --git a/src/access/AccessControlPausable/AccessControlPausableMod.sol b/src/access/AccessControlPausable/AccessControlPausableMod.sol
@@ -42,7 +42,7 @@ bytes32 constant ACCESS_CONTROL_STORAGE_POSITION = keccak256("compose.accesscont
  */
 bytes32 constant PAUSABLE_STORAGE_POSITION = keccak256("compose.accesscontrol.pausable");
 
-/*
+/**
  * @notice Storage struct for AccessControl (reused struct definition).
  * @dev Must match the struct definition in AccessControlFacet.
  * @custom:storage-location erc8042:compose.accesscontrol
@@ -52,7 +52,7 @@ struct AccessControlStorage {
     mapping(bytes32 role => bytes32 adminRole) adminRole;
 }
 
-/*
+/**
  * @notice Storage struct for AccessControlPausable.
  * @custom:storage-location erc8042:compose.accesscontrol.pausable
  */
diff --git a/src/access/AccessControlTemporal/AccessControlTemporalMod.sol b/src/access/AccessControlTemporal/AccessControlTemporalMod.sol
@@ -48,7 +48,7 @@ bytes32 constant ACCESS_CONTROL_STORAGE_POSITION = keccak256("compose.accesscont
  */
 bytes32 constant TEMPORAL_STORAGE_POSITION = keccak256("compose.accesscontrol.temporal");
 
-/*
+/**
  * @notice Storage struct for AccessControl (reused struct definition).
  * @dev Must match the struct definition in AccessControlFacet.
  * @custom:storage-location erc8042:compose.accesscontrol
@@ -58,7 +58,7 @@ struct AccessControlStorage {
     mapping(bytes32 role => bytes32 adminRole) adminRole;
 }
 
-/*
+/**
  * @notice Storage struct for AccessControlTemporal.
  * @custom:storage-location erc8042:compose.accesscontrol.temporal
  */
diff --git a/src/access/Owner/OwnerFacet.sol b/src/access/Owner/OwnerFacet.sol
@@ -19,10 +19,10 @@ contract OwnerFacet {
      */
     error OwnerUnauthorizedAccount();
 
-    bytes32 constant STORAGE_POSITION = keccak256("compose.owner");
+    bytes32 constant STORAGE_POSITION = keccak256("erc173.owner");
 
     /**
-     * @custom:storage-location erc8042:compose.owner
+     * @custom:storage-location erc8042:erc173.owner
      */
     struct OwnerStorage {
         address owner;
diff --git a/src/access/Owner/OwnerMod.sol b/src/access/Owner/OwnerMod.sol
@@ -24,10 +24,10 @@ error OwnerUnauthorizedAccount();
  */
 error OwnerAlreadyRenounced();
 
-bytes32 constant STORAGE_POSITION = keccak256("compose.owner");
+bytes32 constant STORAGE_POSITION = keccak256("erc173.owner");
 
 /**
- * @custom:storage-location erc8042:compose.owner
+ * @custom:storage-location erc8042:erc173.owner
  */
 struct OwnerStorage {
     address owner;
diff --git a/src/access/OwnerTwoSteps/OwnerTwoStepsFacet.sol b/src/access/OwnerTwoSteps/OwnerTwoStepsFacet.sol
@@ -24,10 +24,10 @@ contract OwnerTwoStepsFacet {
      */
     error OwnerUnauthorizedAccount();
 
-    bytes32 constant OWNER_STORAGE_POSITION = keccak256("compose.owner");
+    bytes32 constant OWNER_STORAGE_POSITION = keccak256("erc173.owner");
 
     /**
-     * @custom:storage-location erc8042:compose.owner
+     * @custom:storage-location erc8042:erc173.owner
      */
     struct OwnerStorage {
         address owner;
@@ -45,10 +45,10 @@ contract OwnerTwoStepsFacet {
         }
     }
 
-    bytes32 constant PENDING_OWNER_STORAGE_POSITION = keccak256("compose.owner.pending");
+    bytes32 constant PENDING_OWNER_STORAGE_POSITION = keccak256("erc173.owner.pending");
 
     /**
-     * @custom:storage-location erc8042:compose.owner.pending
+     * @custom:storage-location erc8042:erc173.owner.pending
      */
     struct PendingOwnerStorage {
         address pendingOwner;
diff --git a/src/access/OwnerTwoSteps/OwnerTwoStepsMod.sol b/src/access/OwnerTwoSteps/OwnerTwoStepsMod.sol
@@ -28,10 +28,10 @@ error OwnerUnauthorizedAccount();
  */
 error OwnerAlreadyRenounced();
 
-bytes32 constant OWNER_STORAGE_POSITION = keccak256("compose.owner");
+bytes32 constant OWNER_STORAGE_POSITION = keccak256("erc173.owner");
 
 /**
- * @custom:storage-location erc8042:compose.owner
+ * @custom:storage-location erc8042:erc173.owner
  */
 struct OwnerStorage {
     address owner;
@@ -49,10 +49,10 @@ function getOwnerStorage() pure returns (OwnerStorage storage s) {
     }
 }
 
-bytes32 constant PENDING_OWNER_STORAGE_POSITION = keccak256("compose.owner.pending");
+bytes32 constant PENDING_OWNER_STORAGE_POSITION = keccak256("erc173.owner.pending");
 
 /**
- * @custom:storage-location erc8042:compose.owner.pending
+ * @custom:storage-location erc8042:erc173.owner.pending
  */
 struct PendingOwnerStorage {
     address pendingOwner;
diff --git a/src/diamond/DiamondMod.sol b/src/diamond/DiamondMod.sol
@@ -128,7 +128,7 @@ function importSelectors(address _facet) view returns (bytes memory selectors) {
         }
     }
 
-    // Validate ABI offset == 0x20 for a single dynamic return
+    /* Validate ABI offset == 0x20 for a single dynamic return */
     uint256 offset;
     assembly ("memory-safe") {
         offset := mload(add(data, 0x20))
diff --git a/src/diamond/DiamondUpgradeFacet.sol b/src/diamond/DiamondUpgradeFacet.sol
@@ -121,7 +121,7 @@ contract DiamondUpgradeFacet {
     }
 
     /**
-     * @custom:storage-location erc8042:erc8109.diamond
+     * @custom:storage-location erc8042:erc8153.diamond
      */
     struct DiamondStorage {
         mapping(bytes4 functionSelector => FacetNode) facetNodes;
@@ -228,7 +228,7 @@ contract DiamondUpgradeFacet {
             }
         }
 
-        // Validate ABI offset == 0x20 for a single dynamic return
+        /* Validate ABI offset == 0x20 for a single dynamic return */
         uint256 offset;
         assembly ("memory-safe") {
             offset := mload(add(data, 0x20))
diff --git a/src/diamond/DiamondUpgradeMod.sol b/src/diamond/DiamondUpgradeMod.sol
@@ -201,7 +201,7 @@ function importSelectors(address _facet) view returns (bytes memory selectors) {
         }
     }
 
-    // Validate ABI offset == 0x20 for a single dynamic return
+    /* Validate ABI offset == 0x20 for a single dynamic return */
     uint256 offset;
     assembly ("memory-safe") {
         offset := mload(add(data, 0x20))
diff --git a/src/interfaceDetection/ERC165/ERC165Facet.sol b/src/interfaceDetection/ERC165/ERC165Facet.sol
@@ -33,11 +33,11 @@ contract ERC165Facet {
      * @notice Storage slot identifier for ERC-165 interface detection
      * @dev Defined using keccak256 hash following ERC-8042 standard
      */
-    bytes32 constant STORAGE_POSITION = keccak256("compose.erc165");
+    bytes32 constant STORAGE_POSITION = keccak256("erc165");
 
     /**
      * @notice ERC-165 storage layout using the ERC-8042 standard
-     * @custom:storage-location erc8042:compose.erc165
+     * @custom:storage-location erc8042:erc165
      */
     struct ERC165Storage {
         /**
diff --git a/src/interfaceDetection/ERC165/ERC165Mod.sol b/src/interfaceDetection/ERC165/ERC165Mod.sol
@@ -14,11 +14,11 @@ pragma solidity >=0.8.30;
 /*
  * Storage slot identifier, defined using keccak256 hash of the library diamond storage identifier.
  */
-bytes32 constant STORAGE_POSITION = keccak256("compose.erc165");
+bytes32 constant STORAGE_POSITION = keccak256("erc165");
 
 /*
  * @notice ERC-165 storage layout using the ERC-8042 standard.
- * @custom:storage-location erc8042:compose.erc165
+ * @custom:storage-location erc8042:erc165
  */
 struct ERC165Storage {
     /*
diff --git a/src/token/ERC1155/ERC1155DataFacet.sol b/src/token/ERC1155/ERC1155DataFacet.sol
@@ -24,7 +24,7 @@ contract ERC1155DataFacet {
 
     /**
      * @dev ERC-8042 compliant storage struct for ERC-1155 token data.
-     * @custom:storage-location erc8042:compose.erc1155
+     * @custom:storage-location erc8042:erc1155
      */
     struct ERC1155Storage {
         mapping(uint256 id => mapping(address account => uint256 balance)) balanceOf;
diff --git a/src/token/ERC1155/ERC1155Facet.sol b/src/token/ERC1155/ERC1155Facet.sol
@@ -60,7 +60,7 @@ interface IERC1155Receiver {
  *
  *      For developers creating custom facets that need to interact with ERC-1155 storage (e.g., custom minting logic),
  *      use the LibERC1155 library which provides helper functions to access this facet's storage.
- *      This facet does NOT depend on LibERC1155 - both access the same storage at keccak256("compose.erc1155").
+ *      This facet does NOT depend on LibERC1155 - both access the same storage at keccak256("erc1155").
  */
 contract ERC1155Facet {
     /**
@@ -156,7 +156,7 @@ contract ERC1155Facet {
 
     /**
      * @dev ERC-8042 compliant storage struct for ERC-1155 token data.
-     * @custom:storage-location erc8042:compose.erc1155
+     * @custom:storage-location erc8042:erc1155
      */
     struct ERC1155Storage {
         mapping(uint256 id => mapping(address account => uint256 balance)) balanceOf;
diff --git a/src/token/ERC1155/ERC1155Mod.sol b/src/token/ERC1155/ERC1155Mod.sol
@@ -129,11 +129,11 @@ event URI(string _value, uint256 indexed _id);
 /**
  * @dev Storage position determined by the keccak256 hash of the diamond storage identifier.
  */
-bytes32 constant STORAGE_POSITION = keccak256("compose.erc1155");
+bytes32 constant STORAGE_POSITION = keccak256("erc1155");
 
 /**
  * @dev ERC-8042 compliant storage struct for ERC-1155 token data.
- * @custom:storage-location erc8042:compose.erc1155
+ * @custom:storage-location erc8042:erc1155
  */
 struct ERC1155Storage {
     mapping(uint256 id => mapping(address account => uint256 balance)) balanceOf;
diff --git a/src/token/ERC20/Bridgeable/ERC20BridgeableFacet.sol b/src/token/ERC20/Bridgeable/ERC20BridgeableFacet.sol
@@ -116,6 +116,7 @@ contract ERC20BridgeableFacet {
 
     /**
      * @notice storage struct for the AccessControl.
+     * @custom:storage-location erc8042:compose.accesscontrol
      */
     struct AccessControlStorage {
         mapping(address account => mapping(bytes32 role => bool hasRole)) hasRole;
diff --git a/src/token/ERC20/Bridgeable/ERC20BridgeableMod.sol b/src/token/ERC20/Bridgeable/ERC20BridgeableMod.sol
@@ -114,8 +114,9 @@ function getERC20Storage() pure returns (ERC20Storage storage s) {
  */
 bytes32 constant ACCESS_STORAGE_POSITION = keccak256("compose.accesscontrol");
 
-/*
+/**
  * @notice storage struct for the AccessControl.
+ * @custom:storage-location erc8042:compose.accesscontrol
  */
 struct AccessControlStorage {
     mapping(address account => mapping(bytes32 role => bool hasRole)) hasRole;
diff --git a/src/token/ERC20/Burn/ERC20BurnMod.sol b/src/token/ERC20/Burn/ERC20BurnMod.sol
@@ -38,7 +38,7 @@ event Transfer(address indexed _from, address indexed _to, uint256 _value);
  */
 bytes32 constant STORAGE_POSITION = keccak256("erc20");
 
-/*
+/**
  * @notice ERC-20 storage layout using the ERC-8042 standard.
  * @custom:storage-location erc8042:erc20
  */
diff --git a/src/token/ERC20/Data/ERC20DataFacet.sol b/src/token/ERC20/Data/ERC20DataFacet.sol
@@ -13,7 +13,7 @@ contract ERC20DataFacet {
 
     /**
      * @dev ERC-8042 compliant storage struct for ERC20 token data.
-     * @custom:storage-location erc8042:erc20.transfer
+     * @custom:storage-location erc8042:erc20
      */
     struct ERC20Storage {
         mapping(address owner => uint256 balance) balanceOf;
diff --git a/src/token/ERC6909/ERC6909/ERC6909Facet.sol b/src/token/ERC6909/ERC6909/ERC6909Facet.sol
@@ -55,10 +55,10 @@ contract ERC6909Facet {
     /**
      * @dev Storage position determined by the keccak256 hash of the diamond storage identifier.
      */
-    bytes32 constant STORAGE_POSITION = keccak256("compose.erc6909");
+    bytes32 constant STORAGE_POSITION = keccak256("erc6909");
 
     /**
-     * @custom:storage-location erc8042:compose.erc6909
+     * @custom:storage-location erc8042:erc6909
      */
     struct ERC6909Storage {
         mapping(address owner => mapping(uint256 id => uint256 amount)) balanceOf;
diff --git a/src/token/ERC6909/ERC6909/ERC6909Mod.sol b/src/token/ERC6909/ERC6909/ERC6909Mod.sol
@@ -87,10 +87,10 @@ event Approval(address indexed _owner, address indexed _spender, uint256 indexed
 /**
  * @dev Storage position determined by the keccak256 hash of the diamond storage identifier.
  */
-bytes32 constant STORAGE_POSITION = keccak256("compose.erc6909");
+bytes32 constant STORAGE_POSITION = keccak256("erc6909");
 
 /**
- * @custom:storage-location erc8042:compose.erc6909
+ * @custom:storage-location erc8042:erc6909
  */
 struct ERC6909Storage {
     mapping(address owner => mapping(uint256 id => uint256 amount)) balanceOf;
diff --git a/src/token/ERC721/Burn/ERC721BurnFacet.sol b/src/token/ERC721/Burn/ERC721BurnFacet.sol
@@ -29,7 +29,7 @@ contract ERC721BurnFacet {
     bytes32 constant STORAGE_POSITION = keccak256("erc721");
 
     /**
-     * @custom:storage-location erc8042:compose.erc721
+     * @custom:storage-location erc8042:erc721
      */
     struct ERC721Storage {
         mapping(uint256 tokenId => address owner) ownerOf;
diff --git a/src/token/ERC721/Burn/ERC721BurnMod.sol b/src/token/ERC721/Burn/ERC721BurnMod.sol
@@ -24,10 +24,10 @@ event Transfer(address indexed _from, address indexed _to, uint256 indexed _toke
  */
 bytes32 constant STORAGE_POSITION = keccak256("erc721");
 
-/*
- * @custom:storage-location erc8042:erc721
+/**
  * @notice Storage layout for ERC-721 token management.
  * @dev Defines ownership, balances, approvals, and operator mappings per ERC-721 standard.
+ * @custom:storage-location erc8042:erc721
  */
 struct ERC721Storage {
     mapping(uint256 tokenId => address owner) ownerOf;
diff --git a/src/token/ERC721/Metadata/ERC721MetadataFacet.sol b/src/token/ERC721/Metadata/ERC721MetadataFacet.sol
@@ -20,7 +20,7 @@ contract ERC721MetadataFacet {
     bytes32 constant STORAGE_POSITION = keccak256("erc721.metadata");
 
     /**
-     * @custom:storage-location erc8042:erc721
+     * @custom:storage-location erc8042:erc721.metadata
      */
     struct ERC721MetadataStorage {
         string name;
diff --git a/src/token/ERC721/Metadata/ERC721MetadataMod.sol b/src/token/ERC721/Metadata/ERC721MetadataMod.sol
@@ -14,7 +14,7 @@ error ERC721InvalidOwner(address _owner);
 bytes32 constant STORAGE_POSITION = keccak256("erc721.metadata");
 
 /**
- * @custom:storage-location erc8042:erc721
+ * @custom:storage-location erc8042:erc721.metadata
  */
 struct ERC721MetadataStorage {
     string name;
diff --git a/src/token/ERC721/Transfer/ERC721TransferMod.sol b/src/token/ERC721/Transfer/ERC721TransferMod.sol
@@ -42,10 +42,10 @@ event Transfer(address indexed _from, address indexed _to, uint256 indexed _toke
  */
 bytes32 constant STORAGE_POSITION = keccak256("erc721");
 
-/*
- * @custom:storage-location erc8042:erc721
+/**
  * @notice Storage layout for ERC-721 token management.
  * @dev Defines ownership, balances, approvals, and operator mappings per ERC-721 standard.
+ * @custom:storage-location erc8042:erc721
  */
 struct ERC721Storage {
     mapping(uint256 tokenId => address owner) ownerOf;
diff --git a/src/token/Royalty/RoyaltyFacet.sol b/src/token/Royalty/RoyaltyFacet.sol
@@ -16,7 +16,7 @@ contract RoyaltyFacet {
     /**
      * @notice Storage slot identifier for royalty storage.
      */
-    bytes32 constant STORAGE_POSITION = keccak256("compose.erc2981");
+    bytes32 constant STORAGE_POSITION = keccak256("erc2981");
 
     /**
      * @dev The denominator with which to interpret royalty fees as a percentage of sale price.
@@ -36,7 +36,7 @@ contract RoyaltyFacet {
     }
 
     /**
-     * @custom:storage-location erc8042:compose.erc2981
+     * @custom:storage-location erc8042:erc2981
      */
     struct RoyaltyStorage {
         RoyaltyInfo defaultRoyaltyInfo;
diff --git a/src/token/Royalty/RoyaltyMod.sol b/src/token/Royalty/RoyaltyMod.sol
@@ -40,7 +40,7 @@ error ERC2981InvalidTokenRoyalty(uint256 _tokenId, uint256 _numerator, uint256 _
  */
 error ERC2981InvalidTokenRoyaltyReceiver(uint256 _tokenId, address _receiver);
 
-bytes32 constant STORAGE_POSITION = keccak256("compose.erc2981");
+bytes32 constant STORAGE_POSITION = keccak256("erc2981");
 
 /**
  * @dev The denominator with which to interpret royalty fees as a percentage of sale price.
@@ -62,7 +62,7 @@ struct RoyaltyInfo {
 }
 
 /**
- * @custom:storage-location erc8042:compose.erc2981
+ * @custom:storage-location erc8042:erc2981
  */
 struct RoyaltyStorage {
     RoyaltyInfo defaultRoyaltyInfo;
diff --git a/website/docs/contribution/code-style-guide.mdx b/website/docs/contribution/code-style-guide.mdx
@@ -32,6 +32,13 @@ Follow the [Solidity feature ban](/docs/design/banned-solidity-features).
     delete balances[_owner];
     ```
 
+### Storage Position Conventions
+
+Diamond storage slots (and the matching `@custom:storage-location erc8042:...` annotations on structs) follow:
+
+- **ERC standards:** Use the `erc` prefix and the standard number or name (e.g. `erc20`, `erc721`, `erc1155`, `erc165`, `erc2981`, `erc6909`, `erc173.owner`, `erc8153.diamond`). Sub-features use a dot (e.g. `erc20.metadata`, `erc721.enumerable`).
+- **Custom / Compose features:** Use the `compose.` prefix (e.g. `compose.accesscontrol`, `compose.accesscontrol.pausable`, `compose.accesscontrol.temporal`).
+
 ### Avoid Assembly
  - Avoid using assembly if you can. If you can't and you access memory, do it [safely](https://docs.soliditylang.org/en/latest/assembly.html#memory-safety), and use `assembly ("memory-safe")`.
    `"memory safe"` tells the Solidity compiler that memory is being used safely so it should not disable optimizations.

Original file line number	Diff line number	Diff line change
`@@ -128,7 +128,7 @@ function importSelectors(address _facet) view returns (bytes memory selectors) {`
`128`	`128`	`}`
`129`	`129`	`}`
`130`	`130`
`131`		`- // Validate ABI offset == 0x20 for a single dynamic return`
	`131`	`+ /* Validate ABI offset == 0x20 for a single dynamic return */`
`132`	`132`	`uint256 offset;`
`133`	`133`	`assembly ("memory-safe") {`
`134`	`134`	`offset := mload(add(data, 0x20))`
Original file line number	Diff line number	Diff line change
`@@ -121,7 +121,7 @@ contract DiamondUpgradeFacet {`
`121`	`121`	`}`
`122`	`122`
`123`	`123`	`/**`
`124`		`- * @custom:storage-location erc8042:erc8109.diamond`
	`124`	`+ * @custom:storage-location erc8042:erc8153.diamond`
`125`	`125`	`*/`
`126`	`126`	`struct DiamondStorage {`
`127`	`127`	`mapping(bytes4 functionSelector => FacetNode) facetNodes;`
`@@ -228,7 +228,7 @@ contract DiamondUpgradeFacet {`
`228`	`228`	`}`
`229`	`229`	`}`
`230`	`230`
`231`		`- // Validate ABI offset == 0x20 for a single dynamic return`
	`231`	`+ /* Validate ABI offset == 0x20 for a single dynamic return */`
`232`	`232`	`uint256 offset;`
`233`	`233`	`assembly ("memory-safe") {`
`234`	`234`	`offset := mload(add(data, 0x20))`
Original file line number	Diff line number	Diff line change
`@@ -201,7 +201,7 @@ function importSelectors(address _facet) view returns (bytes memory selectors) {`
`201`	`201`	`}`
`202`	`202`	`}`
`203`	`203`
`204`		`- // Validate ABI offset == 0x20 for a single dynamic return`
	`204`	`+ /* Validate ABI offset == 0x20 for a single dynamic return */`
`205`	`205`	`uint256 offset;`
`206`	`206`	`assembly ("memory-safe") {`
`207`	`207`	`offset := mload(add(data, 0x20))`