refactor(world): make AccessControl lib usable outside of world package

.changeset/quick-fans-jog.md

@@ -0,0 +1,7 @@
+---
+"@latticexyz/world-module-metadata": patch
+"@latticexyz/world-modules": patch
+"@latticexyz/world": patch
+---
+
+Refactored `AccessControl` library exported from `@latticexyz/world` to be usable outside of the world package and updated module packages to use it.

packages/world-module-metadata/src/MetadataModule.sol

@@ -3,7 +3,7 @@ pragma solidity >=0.8.24;
 
 import { IBaseWorld } from "@latticexyz/world/src/codegen/interfaces/IBaseWorld.sol";
 import { Module } from "@latticexyz/world/src/Module.sol";
-import { requireOwner } from "./common.sol";
+import { AccessControl } from "@latticexyz/world/src/AccessControl.sol";
 import { ResourceId, WorldResourceIdLib, WorldResourceIdInstance } from "@latticexyz/world/src/WorldResourceId.sol";
 import { ResourceIds } from "@latticexyz/store/src/codegen/tables/ResourceIds.sol";
 import { RESOURCE_SYSTEM } from "@latticexyz/world/src/worldResourceTypes.sol";
@@ -33,7 +33,7 @@ contract MetadataModule is Module {
     if (!ResourceIds.getExists(namespace)) {
       world.registerNamespace(namespace);
     }
-    requireOwner(namespace, address(this));
+    AccessControl.requireOwner(namespace, address(this));
 
     if (!ResourceIds.getExists(ResourceTag._tableId)) {
       ResourceTag.register();

packages/world-module-metadata/src/MetadataSystem.sol

@@ -3,7 +3,8 @@ pragma solidity >=0.8.24;
 
 import { ResourceId } from "@latticexyz/store/src/ResourceId.sol";
 import { System } from "@latticexyz/world/src/System.sol";
-import { requireExistence, requireOwner } from "./common.sol";
+import { AccessControl } from "@latticexyz/world/src/AccessControl.sol";
+
 import { ResourceTag } from "./codegen/tables/ResourceTag.sol";
 
 contract MetadataSystem is System {
@@ -12,14 +13,14 @@ contract MetadataSystem is System {
   }
 
   function setResourceTag(ResourceId resource, bytes32 tag, bytes memory value) public {
-    requireExistence(resource);
-    requireOwner(resource, _msgSender());
+    AccessControl.requireExistence(resource);
+    AccessControl.requireOwner(resource, _msgSender());
     ResourceTag.set(resource, tag, value);
   }
 
   function deleteResourceTag(ResourceId resource, bytes32 tag) public {
-    requireExistence(resource);
-    requireOwner(resource, _msgSender());
+    AccessControl.requireExistence(resource);
+    AccessControl.requireOwner(resource, _msgSender());
     ResourceTag.deleteRecord(resource, tag);
   }
 }

packages/world-modules/src/modules/erc20-puppet/ERC20System.sol

@@ -6,8 +6,8 @@ import { System } from "@latticexyz/world/src/System.sol";
 import { WorldResourceIdInstance } from "@latticexyz/world/src/WorldResourceId.sol";
 import { NamespaceOwner } from "@latticexyz/world/src/codegen/tables/NamespaceOwner.sol";
 import { SystemRegistry } from "@latticexyz/world/src/codegen/tables/SystemRegistry.sol";
+import { AccessControl } from "@latticexyz/world/src/AccessControl.sol";
 
-import { AccessControlLib } from "../../utils/AccessControlLib.sol";
 import { PuppetMaster } from "../puppet/PuppetMaster.sol";
 import { toTopic } from "../puppet/utils.sol";
 import { Balances } from "../tokens/tables/Balances.sol";
@@ -281,6 +281,6 @@ contract ERC20System is System, IERC20Mintable, PuppetMaster {
   }
 
   function _requireOwner() internal view {
-    AccessControlLib.requireOwner(SystemRegistry.get(address(this)), _msgSender());
+    AccessControl.requireOwner(SystemRegistry.get(address(this)), _msgSender());
   }
 }

packages/world-modules/src/modules/erc721-puppet/ERC721System.sol

@@ -5,8 +5,8 @@ import { ResourceId } from "@latticexyz/store/src/ResourceId.sol";
 import { System } from "@latticexyz/world/src/System.sol";
 import { WorldResourceIdInstance } from "@latticexyz/world/src/WorldResourceId.sol";
 import { SystemRegistry } from "@latticexyz/world/src/codegen/tables/SystemRegistry.sol";
+import { AccessControl } from "@latticexyz/world/src/AccessControl.sol";
 
-import { AccessControlLib } from "../../utils/AccessControlLib.sol";
 import { PuppetMaster } from "../puppet/PuppetMaster.sol";
 import { toTopic } from "../puppet/utils.sol";
 import { Balances } from "../tokens/tables/Balances.sol";
@@ -526,6 +526,6 @@ contract ERC721System is IERC721Mintable, System, PuppetMaster {
   }
 
   function _requireOwner() internal view {
-    AccessControlLib.requireOwner(SystemRegistry.get(address(this)), _msgSender());
+    AccessControl.requireOwner(SystemRegistry.get(address(this)), _msgSender());
   }
 }

packages/world-modules/src/modules/puppet/PuppetFactorySystem.sol

@@ -4,8 +4,7 @@ pragma solidity >=0.8.24;
 import { ResourceId } from "@latticexyz/store/src/ResourceId.sol";
 import { System } from "@latticexyz/world/src/System.sol";
 import { IBaseWorld } from "@latticexyz/world/src/codegen/interfaces/IBaseWorld.sol";
-
-import { AccessControlLib } from "../../utils/AccessControlLib.sol";
+import { AccessControl } from "@latticexyz/world/src/AccessControl.sol";
 
 import { PuppetRegistry } from "./tables/PuppetRegistry.sol";
 import { Puppet } from "./Puppet.sol";
@@ -14,7 +13,7 @@ import { PUPPET_TABLE_ID } from "./constants.sol";
 contract PuppetFactorySystem is System {
   function createPuppet(ResourceId systemId) public returns (address puppet) {
     // Only the owner of a system can create a puppet for it
-    AccessControlLib.requireOwner(systemId, _msgSender());
+    AccessControl.requireOwner(systemId, _msgSender());
 
     // Deploy a new puppet contract
     puppet = address(new Puppet(IBaseWorld(_world()), systemId));

packages/world/gas-report.json

@@ -3,43 +3,43 @@
     "file": "test/AccessControl.t.sol",
     "test": "testAccessControl",
     "name": "AccessControl: hasAccess (cold)",
-    "gasUsed": 6521
+    "gasUsed": 9111
   },
   {
     "file": "test/AccessControl.t.sol",
     "test": "testAccessControl",
     "name": "AccessControl: hasAccess (warm, namespace only)",
-    "gasUsed": 1300
+    "gasUsed": 1595
   },
   {
     "file": "test/AccessControl.t.sol",
     "test": "testAccessControl",
     "name": "AccessControl: hasAccess (warm)",
-    "gasUsed": 2527
+    "gasUsed": 3117
   },
   {
     "file": "test/AccessControl.t.sol",
     "test": "testRequireAccess",
     "name": "AccessControl: requireAccess (cold)",
-    "gasUsed": 6564
+    "gasUsed": 9154
   },
   {
     "file": "test/AccessControl.t.sol",
     "test": "testRequireAccess",
     "name": "AccessControl: requireAccess (warm)",
-    "gasUsed": 2566
+    "gasUsed": 3156
   },
   {
     "file": "test/AccessControl.t.sol",
     "test": "testRequireOwner",
     "name": "AccessControl: requireOwner (cold)",
-    "gasUsed": 3106
+    "gasUsed": 5401
   },
   {
     "file": "test/AccessControl.t.sol",
     "test": "testRequireOwner",
     "name": "AccessControl: requireOwner (warm)",
-    "gasUsed": 1107
+    "gasUsed": 1402
   },
   {
     "file": "test/BatchCall.t.sol",

packages/world/src/AccessControl.sol

@@ -24,16 +24,29 @@ library AccessControl {
    * @return true if the caller has access, false otherwise.
    */
   function hasAccess(ResourceId resourceId, address caller) internal view returns (bool) {
+    return
+      // First check access based on the namespace. If caller has no namespace access, check access on the resource.
+      ResourceAccess.get(resourceId.getNamespaceId(), caller) || ResourceAccess.get(resourceId, caller);
+  }
+
+  /**
+   * @notice Checks if the caller has access to the given resource ID or its namespace.
+   * @dev This bypasses StoreSwitch and assumes its being called within the Store, saving gas for known call contexts.
+   * @param resourceId The resource ID to check access for.
+   * @param caller The address of the caller.
+   * @return true if the caller has access, false otherwise.
+   */
+  function _hasAccess(ResourceId resourceId, address caller) internal view returns (bool) {
     return
       // First check access based on the namespace. If caller has no namespace access, check access on the resource.
       ResourceAccess._get(resourceId.getNamespaceId(), caller) || ResourceAccess._get(resourceId, caller);
   }
 
   /**
    * @notice Check for access at the given namespace or resource.
+   * @dev Reverts with IWorldErrors.World_AccessDenied if access is denied.
    * @param resourceId The resource ID to check access for.
    * @param caller The address of the caller.
-   * @dev Reverts with IWorldErrors.World_AccessDenied if access is denied.
    */
   function requireAccess(ResourceId resourceId, address caller) internal view {
     // Check if the given caller has access to the given namespace or name
@@ -42,13 +55,40 @@ library AccessControl {
     }
   }
 
+  /**
+   * @notice Check for access at the given namespace or resource.
+   * @dev Reverts with IWorldErrors.World_AccessDenied if access is denied.
+   * @dev This bypasses StoreSwitch and assumes its being called within the Store, saving gas for known call contexts.
+   * @param resourceId The resource ID to check access for.
+   * @param caller The address of the caller.
+   */
+  function _requireAccess(ResourceId resourceId, address caller) internal view {
+    // Check if the given caller has access to the given namespace or name
+    if (!_hasAccess(resourceId, caller)) {
+      revert IWorldErrors.World_AccessDenied(resourceId.toString(), caller);
+    }
+  }
+
   /**
    * @notice Check for ownership of the namespace of the given resource ID.
    * @dev Reverts with IWorldErrors.World_AccessDenied if caller is not owner of the namespace of the resource.
    * @param resourceId The resource ID to check ownership for.
    * @param caller The address of the caller.
    */
   function requireOwner(ResourceId resourceId, address caller) internal view {
+    if (NamespaceOwner.get(resourceId.getNamespaceId()) != caller) {
+      revert IWorldErrors.World_AccessDenied(resourceId.toString(), caller);
+    }
+  }
+
+  /**
+   * @notice Check for ownership of the namespace of the given resource ID.
+   * @dev Reverts with IWorldErrors.World_AccessDenied if caller is not owner of the namespace of the resource.
+   * @dev This bypasses StoreSwitch and assumes its being called within the Store, saving gas for known call contexts.
+   * @param resourceId The resource ID to check ownership for.
+   * @param caller The address of the caller.
+   */
+  function _requireOwner(ResourceId resourceId, address caller) internal view {
     if (NamespaceOwner._get(resourceId.getNamespaceId()) != caller) {
       revert IWorldErrors.World_AccessDenied(resourceId.toString(), caller);
     }
@@ -60,6 +100,18 @@ library AccessControl {
    * @param resourceId The resource ID to check existence for.
    */
   function requireExistence(ResourceId resourceId) internal view {
+    if (!ResourceIds.getExists(resourceId)) {
+      revert IWorldErrors.World_ResourceNotFound(resourceId, resourceId.toString());
+    }
+  }
+
+  /**
+   * @notice Check for existence of the given resource ID.
+   * @dev Reverts with IWorldErrors.World_ResourceNotFound if the resource does not exist.
+   * @dev This bypasses StoreSwitch and assumes its being called within the Store, saving gas for known call contexts.
+   * @param resourceId The resource ID to check existence for.
+   */
+  function _requireExistence(ResourceId resourceId) internal view {
     if (!ResourceIds._getExists(resourceId)) {
       revert IWorldErrors.World_ResourceNotFound(resourceId, resourceId.toString());
     }

packages/world/src/SystemCall.sol

@@ -49,7 +49,7 @@ library SystemCall {
     if (systemAddress == address(0)) revert IWorldErrors.World_ResourceNotFound(systemId, systemId.toString());
 
     // Allow access if the system is public or the caller has access to the namespace or name
-    if (!publicAccess) AccessControl.requireAccess(systemId, caller);
+    if (!publicAccess) AccessControl._requireAccess(systemId, caller);
 
     // If the msg.value is non-zero, update the namespace's balance
     if (value > 0) {