refactor(world-consumer): remove namespace (#3597)

Author: vdrg

.changeset/rich-islands-stare.md

@@ -0,0 +1,8 @@
+---
+"@latticexyz/world-consumer": patch
+"@latticexyz/world-module-erc20": patch
+---
+
+`WorldConsumer` now doesn't store a single namespace. Instead, child contracts can keep track of namespaces and use the `onlyNamespace(namespace)` and `onlyNamespaceOwner(namespace)` modifiers for access control.
+
+ERC20 module was adapted to use this new version of `WorldConsumer`.

packages/world-consumer/src/examples/SimpleVault.sol

@@ -17,15 +17,18 @@ interface IERC20 {
 /**
  * @title SimpleVault (NOT AUDITED)
  * @dev Simple example of a Vault that allows accounts with namespace access to transfer its tokens out
- * IMPORTANT: this contract expects an existing namespace
  */
 contract SimpleVault is WorldConsumer {
   error SimpleVault_TransferFailed();
 
-  constructor(IBaseWorld world, bytes14 namespace) WorldConsumer(world, namespace, false) {}
+  bytes14 immutable namespace;
+
+  constructor(IBaseWorld world, bytes14 _namespace) WorldConsumer(world) {
+    namespace = _namespace;
+  }
 
   // Only accounts with namespace access (e.g. namespace systems) can transfer the ERC20 tokens held by this contract
-  function transferTo(IERC20 token, address to, uint256 amount) external onlyWorld {
+  function transferTo(IERC20 token, address to, uint256 amount) external onlyNamespace(namespace) {
     require(token.transfer(to, amount), "Transfer failed");
   }
 

packages/world-consumer/src/experimental/WorldConsumer.sol

@@ -6,65 +6,54 @@ import { StoreSwitch } from "@latticexyz/store/src/StoreSwitch.sol";
 import { ResourceIds } from "@latticexyz/store/src/codegen/tables/ResourceIds.sol";
 import { ResourceId } from "@latticexyz/store/src/ResourceId.sol";
 import { ResourceAccess } from "@latticexyz/world/src/codegen/tables/ResourceAccess.sol";
+import { NamespaceOwner } from "@latticexyz/world/src/codegen/tables/NamespaceOwner.sol";
 import { IBaseWorld } from "@latticexyz/world/src/codegen/interfaces/IBaseWorld.sol";
 import { WorldResourceIdLib } from "@latticexyz/world/src/WorldResourceId.sol";
 import { WorldContextConsumer } from "@latticexyz/world/src/WorldContext.sol";
 import { System } from "@latticexyz/world/src/System.sol";
 
 abstract contract WorldConsumer is System {
-  bytes14 public immutable namespace;
-  ResourceId public immutable namespaceId;
-
   error WorldConsumer_RootNamespaceNotAllowed(address worldAddress);
-  error WorldConsumer_NamespaceAlreadyExists(address worldAddress, bytes14 namespace);
-  error WorldConsumer_NamespaceDoesNotExists(address worldAddress, bytes14 namespace);
   error WorldConsumer_CallerHasNoNamespaceAccess(address worldAddress, bytes14 namespace, address caller);
+  error WorldConsumer_CallerIsNotNamespaceOwner(address worldAddress, bytes14 namespace, address caller);
   error WorldConsumer_CallerIsNotWorld(address worldAddress, address caller);
   error WorldConsumer_ValueNotAllowed(address worldAddress);
 
   modifier onlyWorld() {
     address world = _world();
-    if (world != msg.sender) {
-      revert WorldConsumer_CallerIsNotWorld(world, msg.sender);
-    }
+    checkWorldIsCaller(world);
     _;
   }
 
-  modifier onlyNamespace() {
+  modifier onlyNamespace(bytes14 namespace) {
     address world = _world();
-    if (world != msg.sender) {
-      revert WorldConsumer_CallerIsNotWorld(world, msg.sender);
-    }
+    checkWorldIsCaller(world);
 
     // We use WorldContextConsumer directly as we already know the world is the caller
     address sender = WorldContextConsumer._msgSender();
-    if (!ResourceAccess.get(namespaceId, sender)) {
+    if (!ResourceAccess.get(WorldResourceIdLib.encodeNamespace(namespace), sender)) {
       revert WorldConsumer_CallerHasNoNamespaceAccess(world, namespace, sender);
     }
 
     _;
   }
 
-  constructor(IBaseWorld _world, bytes14 _namespace, bool registerNamespace) {
-    address worldAddress = address(_world);
-    StoreSwitch.setStoreAddress(worldAddress);
+  modifier onlyNamespaceOwner(bytes14 namespace) {
+    address world = _world();
+    checkWorldIsCaller(world);
 
-    if (_namespace == bytes14(0)) {
-      revert WorldConsumer_RootNamespaceNotAllowed(worldAddress);
+    // We use WorldContextConsumer directly as we already know the world is the caller
+    address sender = WorldContextConsumer._msgSender();
+    if (NamespaceOwner.get(WorldResourceIdLib.encodeNamespace(namespace)) != sender) {
+      revert WorldConsumer_CallerIsNotNamespaceOwner(world, namespace, sender);
     }
 
-    namespace = _namespace;
-    namespaceId = WorldResourceIdLib.encodeNamespace(_namespace);
-    bool namespaceExists = ResourceIds.getExists(namespaceId);
+    _;
+  }
 
-    if (registerNamespace) {
-      if (namespaceExists) {
-        revert WorldConsumer_NamespaceAlreadyExists(worldAddress, _namespace);
-      }
-      _world.registerNamespace(namespaceId);
-    } else if (!namespaceExists) {
-      revert WorldConsumer_NamespaceDoesNotExists(worldAddress, _namespace);
-    }
+  constructor(IBaseWorld _world) {
+    address worldAddress = address(_world);
+    StoreSwitch.setStoreAddress(worldAddress);
   }
 
   function _msgSender() public view virtual override returns (address sender) {
@@ -79,4 +68,10 @@ abstract contract WorldConsumer is System {
 
     return WorldContextConsumer._msgValue();
   }
+
+  function checkWorldIsCaller(address world) internal view {
+    if (world != msg.sender) {
+      revert WorldConsumer_CallerIsNotWorld(world, msg.sender);
+    }
+  }
 }

packages/world-consumer/test/WorldConsumer.t.sol

@@ -21,29 +21,22 @@ import { StoreSwitch } from "@latticexyz/store/src/StoreSwitch.sol";
 import { WorldConsumer } from "../src/experimental/WorldConsumer.sol";
 
 contract MockWorldConsumer is WorldConsumer {
-  constructor(
-    IBaseWorld world,
-    bytes14 namespace,
-    bool registerNamespace
-  ) WorldConsumer(world, namespace, registerNamespace) {}
+  bytes14 immutable namespace;
+  constructor(IBaseWorld world, bytes14 _namespace) WorldConsumer(world) {
+    namespace = _namespace;
+  }
 
   function getStoreAddress() public view virtual returns (address) {
     return StoreSwitch.getStoreAddress();
   }
 
-  function grantNamespaceAccess(address to) external {
-    IBaseWorld(_world()).grantAccess(namespaceId, to);
-  }
-
-  function transferNamespaceOwnership(address to) external {
-    IBaseWorld(_world()).transferOwnership(namespaceId, to);
-  }
-
   function callableByAnyone() external view {}
 
   function onlyCallableByWorld() external view onlyWorld {}
 
-  function onlyCallableByNamespace() external view onlyNamespace {}
+  function onlyCallableByNamespace() external view onlyNamespace(namespace) {}
+
+  function onlyCallableByNamespaceOwner() external view onlyNamespaceOwner(namespace) {}
 
   function payableFn() external payable returns (uint256 value) {
     return _msgValue();
@@ -53,33 +46,33 @@ contract MockWorldConsumer is WorldConsumer {
 contract WorldConsumerTest is Test, GasReporter {
   using WorldResourceIdInstance for ResourceId;
 
-  function testWorldConsumer() public {
-    IBaseWorld world = createWorld();
-    bytes14 namespace = "myNamespace";
-    ResourceId namespaceId = WorldResourceIdLib.encodeNamespace(namespace);
+  bytes14 constant namespace = "myNamespace";
+  bytes16 constant systemName = "mySystem";
 
-    MockWorldConsumer mock = new MockWorldConsumer(world, namespace, true);
-    assertEq(mock.getStoreAddress(), address(world));
-    assertEq(mock.namespace(), namespace);
-    assertEq(mock.namespaceId().unwrap(), namespaceId.unwrap());
+  ResourceId systemId;
+  ResourceId namespaceId;
+
+  IBaseWorld world;
+  MockWorldConsumer mock;
 
+  function setUp() public {
+    world = createWorld();
     StoreSwitch.setStoreAddress(address(world));
 
-    assertTrue(ResourceIds.getExists(namespaceId), "Namespace not registered");
+    namespaceId = WorldResourceIdLib.encodeNamespace(namespace);
+    world.registerNamespace(namespaceId);
+
+    systemId = WorldResourceIdLib.encode(RESOURCE_SYSTEM, namespace, systemName);
+
+    mock = new MockWorldConsumer(world, namespace);
+  }
+
+  function testWorldConsumer() public view {
+    assertEq(mock.getStoreAddress(), address(world));
   }
 
   // Test internal MUD access control
   function testAccessControl() public {
-    IBaseWorld world = createWorld();
-    StoreSwitch.setStoreAddress(address(world));
-
-    bytes16 systemName = "mySystem";
-    bytes14 namespace = "myNamespace";
-    ResourceId namespaceId = WorldResourceIdLib.encodeNamespace(namespace);
-    ResourceId systemId = WorldResourceIdLib.encode(RESOURCE_SYSTEM, namespace, systemName);
-    MockWorldConsumer mock = new MockWorldConsumer(world, namespace, true);
-    mock.transferNamespaceOwnership(address(this));
-
     // Register the mock as a system with PRIVATE access
     world.registerSystem(systemId, mock, false);
 
@@ -96,15 +89,6 @@ contract WorldConsumerTest is Test, GasReporter {
   }
 
   function testOnlyWorld() public {
-    IBaseWorld world = createWorld();
-    StoreSwitch.setStoreAddress(address(world));
-
-    bytes16 systemName = "mySystem";
-    bytes14 namespace = "myNamespace";
-    ResourceId systemId = WorldResourceIdLib.encode(RESOURCE_SYSTEM, namespace, systemName);
-    MockWorldConsumer mock = new MockWorldConsumer(world, namespace, true);
-    mock.transferNamespaceOwnership(address(this));
-
     // Register the mock as a system with PUBLIC access
     world.registerSystem(systemId, mock, true);
 
@@ -119,16 +103,6 @@ contract WorldConsumerTest is Test, GasReporter {
   }
 
   function testOnlyNamespace() public {
-    IBaseWorld world = createWorld();
-    StoreSwitch.setStoreAddress(address(world));
-
-    bytes16 systemName = "mySystem";
-    bytes14 namespace = "myNamespace";
-    ResourceId namespaceId = WorldResourceIdLib.encodeNamespace(namespace);
-    ResourceId systemId = WorldResourceIdLib.encode(RESOURCE_SYSTEM, namespace, systemName);
-    MockWorldConsumer mock = new MockWorldConsumer(world, namespace, true);
-    mock.transferNamespaceOwnership(address(this));
-
     // Register the mock as a system with PUBLIC access
     world.registerSystem(systemId, mock, true);
 
@@ -150,16 +124,37 @@ contract WorldConsumerTest is Test, GasReporter {
     world.call(systemId, abi.encodeCall(mock.onlyCallableByNamespace, ()));
   }
 
-  function testMsgValue() public {
-    IBaseWorld world = createWorld();
-    StoreSwitch.setStoreAddress(address(world));
+  function testOnlyNamespaceOwner() public {
+    // Register the mock as a system with PUBLIC access
+    world.registerSystem(systemId, mock, true);
 
-    bytes16 systemName = "mySystem";
-    bytes14 namespace = "myNamespace";
-    ResourceId systemId = WorldResourceIdLib.encode(RESOURCE_SYSTEM, namespace, systemName);
-    MockWorldConsumer mock = new MockWorldConsumer(world, namespace, true);
-    mock.transferNamespaceOwnership(address(this));
+    address alice = address(0x1234);
 
+    vm.prank(alice);
+    vm.expectRevert(abi.encodeWithSelector(WorldConsumer.WorldConsumer_CallerIsNotWorld.selector, world, alice));
+    mock.onlyCallableByNamespaceOwner();
+
+    vm.prank(alice);
+    vm.expectRevert(
+      abi.encodeWithSelector(WorldConsumer.WorldConsumer_CallerIsNotNamespaceOwner.selector, world, namespace, alice)
+    );
+    world.call(systemId, abi.encodeCall(mock.onlyCallableByNamespaceOwner, ()));
+
+    // After granting access to namespace, it should not work
+    world.grantAccess(namespaceId, alice);
+    vm.prank(alice);
+    vm.expectRevert(
+      abi.encodeWithSelector(WorldConsumer.WorldConsumer_CallerIsNotNamespaceOwner.selector, world, namespace, alice)
+    );
+    world.call(systemId, abi.encodeCall(mock.onlyCallableByNamespaceOwner, ()));
+
+    // After transfering namespace ownership, it should work
+    world.transferOwnership(namespaceId, alice);
+    vm.prank(alice);
+    world.call(systemId, abi.encodeCall(mock.onlyCallableByNamespaceOwner, ()));
+  }
+
+  function testMsgValue() public {
     // Register the mock as a system with PUBLIC access
     world.registerSystem(systemId, mock, true);