refactor(store-consumer): adapt WithWorld to be a System (#3546)

Author: vdrg

.changeset/loud-moons-tell.md

@@ -0,0 +1,8 @@
+---
+"@latticexyz/store-consumer": patch
+"@latticexyz/store": patch
+"@latticexyz/world-module-erc20": patch
+"@latticexyz/world": patch
+---
+
+Updates `WithWorld` to be a `System`, so that functions in child contracts that use the `onlyWorld` or `onlyNamespace` modifiers must be called through the world in order to safely support calls from systems.

docs/pages/world/modules/erc20.mdx

@@ -12,11 +12,13 @@ This module is unaudited and may change in the future.
 The [`erc20` module](https://github.com/latticexyz/mud/tree/main/packages/world-module-erc20/) lets you create [ERC-20](https://ethereum.org/en/developers/docs/standards/tokens/erc-20/) tokens as part of a MUD `World`.
 The advantage of doing this, rather than creating a separate [ERC-20 contract](https://github.com/OpenZeppelin/openzeppelin-contracts/tree/master/contracts/token/ERC20) and merely controlling it from MUD, is that all the information is in MUD tables and is immediately available in the client.
 
+The token contract can be seen as a hybrid system contract which contains functions directly callable from outside the world (ERC20 functions like `transfer`, `balanceOf`, etc), and restricted functions that must be called as a system function through the World (`mint`, `pause`, etc).
+
 The ERC20Module receives the namespace, name and symbol of the token as parameters, and deploys the new token. Currently it installs a [default ERC20](https://github.com/latticexyz/mud/tree/main/packages/world-module-erc20/src/examples/ERC20WithWorld.sol) with the following features:
 
 - ERC20Burnable: Allows users to burn their tokens (or the ones approved to them) using the `burn` and `burnFrom` function.
-- ERC20Pausable: Supports pausing and unpausing token operations. This is combined with the `pause` and `unpause` public functions that can be called by addresses with access to the token's namespace.
-- Minting: Addresses with namespace access can call the `mint` function to mint tokens to any address.
+- ERC20Pausable: Supports pausing and unpausing token operations. This is combined with the `pause` and `unpause` public functions that can be called by addresses and systems with access to the token's namespace. Must be done through a World call.
+- Minting: Addresses and Systems with namespace access can call the `mint` function to mint tokens to any address. This must be done through a World call.
 
 ## Installation
 
@@ -58,13 +60,15 @@ For example, run this script.
 
 <CollapseCode>
 
-```solidity filename="ManageERC20.s.sol" copy showLineNumbers {16,34-38,43,48-56}
+```solidity filename="ManageERC20.s.sol" copy showLineNumbers {18,36-41,46,51-60}
 import { Script } from "forge-std/Script.sol";
 import { console } from "forge-std/console.sol";
 import { StoreSwitch } from "@latticexyz/store/src/StoreSwitch.sol";
 import { ResourceId } from "@latticexyz/store/src/ResourceId.sol";
 import { RESOURCE_TABLE } from "@latticexyz/store/src/storeResourceTypes.sol";
+import { IWorldCall } from "@latticexyz/world/src/IWorldKernel.sol";
 import { WorldResourceIdLib } from "@latticexyz/world/src/WorldResourceId.sol";
+import { SystemRegistry } from "@latticexyz/world/src/codegen/tables/SystemRegistry.sol";
 import { ERC20Registry } from "@latticexyz/world-module-erc20/src/codegen/index.sol";
 import { ERC20WithWorld as ERC20 } from "@latticexyz/world-module-erc20/src/examples/ERC20WithWorld.sol";
 
@@ -96,6 +100,7 @@ contract ManageERC20 is Script {
     ResourceId namespaceResource = WorldResourceIdLib.encodeNamespace(bytes14("erc20Namespace"));
     ResourceId erc20RegistryResource = WorldResourceIdLib.encode(RESOURCE_TABLE, "erc20-module", "ERC20_REGISTRY");
     address tokenAddress = ERC20Registry.getTokenAddress(erc20RegistryResource, namespaceResource);
+    ResourceId tokenSystem = SystemRegistry.get(tokenAddress);
     console.log("Token address", tokenAddress);
 
     address alice = address(0x600D);
@@ -107,8 +112,9 @@ contract ManageERC20 is Script {
     reportBalances(erc20, myAddress);
 
     // Mint some tokens
+    // We must call the token system (instead of calling mint directly)
     console.log("Minting for myself");
-    erc20.mint(myAddress, 1000);
+    IWorldCall(worldAddress).call(tokenSystem, abi.encodeCall(ERC20.mint, (myAddress, 1000)));
     reportBalances(erc20, myAddress);
 
     // Transfer tokens
@@ -138,12 +144,13 @@ contract ManageERC20 is Script {
     ResourceId namespaceResource = WorldResourceIdLib.encodeNamespace(bytes14("erc20Namespace"));
     ResourceId erc20RegistryResource = WorldResourceIdLib.encode(RESOURCE_TABLE, "erc20-module", "ERC20_REGISTRY");
     address tokenAddress = ERC20Registry.getTokenAddress(erc20RegistryResource, namespaceResource);
+    ResourceId tokenSystem = SystemRegistry.get(tokenAddress);
     console.log("Token address", tokenAddress);
 ```
 
-This is the process to get the address of our token contract.
+This is the process to get the address of our token contract and the system id of the token.
 First, we get the [`resourceId` values](/world/resource-ids) for the `erc20-module__ERC20Registry` table and the namespace we are interested in (each namespace can only have one ERC-20 token).
-Then we use that table to get the token address.
+Then we use that table to get the token address. Finally, we obtain the token system id from the `SystemRegistry` table.
 
 ```solidity
     // Use the token
@@ -153,13 +160,15 @@ Then we use that table to get the token address.
 Cast the token address to an `ERC20` contract so we can call its methods.
 
 ```solidity
+    // Mint some tokens
+    // We must call the token system (instead of calling mint directly)
     console.log("Minting for myself");
-    erc20.mint(myAddress, 1000);
+    IWorldCall(worldAddress).call(tokenSystem, abi.encodeCall(ERC20.mint, (myAddress, 1000)));
     reportBalances(erc20, myAddress);
 ```
 
 Mint tokens for your address.
-Note that only the owner of the name space is authorized to mint tokens.
+Note that the mint function must be called through the world as a system function, as it is restricted to entities with access to the token's namespace.
 
 ```solidity
     console.log("Transfering to Alice");

docs/pages/world/reference/world-context.mdx

@@ -20,7 +20,7 @@ _This contract should only be used for contracts without their own storage, like
 Extract the `msg.sender` from the context appended to the calldata.
 
 ```solidity
-function _msgSender() public view returns (address sender);
+function _msgSender() public view virtual returns (address sender);
 ```
 
 **Returns**

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

@@ -25,7 +25,7 @@ contract SimpleVault is WithWorld {
   constructor(IBaseWorld world, bytes14 namespace) WithWorld(world, namespace, false) {}
 
   // 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 onlyNamespace {
+  function transferTo(IERC20 token, address to, uint256 amount) external onlyWorld {
     require(token.transfer(to, amount), "Transfer failed");
   }
 

packages/store-consumer/src/experimental/Context.sol

@@ -4,7 +4,7 @@ pragma solidity >=0.8.24;
 // @dev Provides information about the current execution context
 // We only use it in these contracts in case we want to extend it in the future
 abstract contract Context {
-  function _msgSender() internal view virtual returns (address) {
+  function _msgSender() public view virtual returns (address) {
     return msg.sender;
   }
 }

packages/store-consumer/src/experimental/WithWorld.sol

@@ -6,21 +6,32 @@ import { ResourceId } from "@latticexyz/store/src/ResourceId.sol";
 import { ResourceAccess } from "@latticexyz/world/src/codegen/tables/ResourceAccess.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";
 
+import { Context } from "./Context.sol";
 import { WithStore } from "./WithStore.sol";
 
-abstract contract WithWorld is WithStore {
+abstract contract WithWorld is WithStore, System {
   bytes14 public immutable namespace;
 
   error WithWorld_RootNamespaceNotAllowed();
-  error WithWorld_NamespaceAlreadyExists();
-  error WithWorld_NamespaceDoesNotExists();
-  error WithWorld_CallerHasNoNamespaceAccess();
+  error WithWorld_NamespaceAlreadyExists(bytes14 namespace);
+  error WithWorld_NamespaceDoesNotExists(bytes14 namespace);
+  error WithWorld_CallerHasNoNamespaceAccess(bytes14 namespace, address caller);
+  error WithWorld_CallerIsNotWorld(address caller);
+
+  modifier onlyWorld() {
+    if (!_callerIsWorld()) {
+      revert WithWorld_CallerIsNotWorld(msg.sender);
+    }
+    _;
+  }
 
   modifier onlyNamespace() {
-    address sender = _msgSender();
-    if (!ResourceAccess.get(getNamespaceId(), sender)) {
-      revert WithWorld_CallerHasNoNamespaceAccess();
+    // We use WorldContextConsumer directly as we already know the world is the caller
+    if (!_callerIsWorld() || !ResourceAccess.get(getNamespaceId(), WorldContextConsumer._msgSender())) {
+      revert WithWorld_CallerHasNoNamespaceAccess(namespace, _msgSender());
     }
     _;
   }
@@ -38,12 +49,12 @@ abstract contract WithWorld is WithStore {
 
     if (registerNamespace) {
       if (namespaceExists) {
-        revert WithWorld_NamespaceAlreadyExists();
+        revert WithWorld_NamespaceAlreadyExists(_namespace);
       }
 
       _world.registerNamespace(namespaceId);
     } else if (!namespaceExists) {
-      revert WithWorld_NamespaceDoesNotExists();
+      revert WithWorld_NamespaceDoesNotExists(_namespace);
     }
   }
 
@@ -55,6 +66,14 @@ abstract contract WithWorld is WithStore {
     return IBaseWorld(getStore());
   }
 
+  function _msgSender() public view virtual override(Context, WorldContextConsumer) returns (address sender) {
+    return _callerIsWorld() ? WorldContextConsumer._msgSender() : Context._msgSender();
+  }
+
+  function _callerIsWorld() internal view returns (bool) {
+    return msg.sender == address(getWorld());
+  }
+
   function _encodeResourceId(bytes2 typeId, bytes16 name) internal view virtual override returns (ResourceId) {
     return WorldResourceIdLib.encode(typeId, namespace, name);
   }

packages/store-consumer/test/StoreConsumer.t.sol

@@ -8,7 +8,9 @@ import { GasReporter } from "@latticexyz/gas-report/src/GasReporter.sol";
 
 import { IBaseWorld } from "@latticexyz/world/src/codegen/interfaces/IBaseWorld.sol";
 import { ResourceAccess } from "@latticexyz/world/src/codegen/tables/ResourceAccess.sol";
-import { WorldResourceIdLib } from "@latticexyz/world/src/WorldResourceId.sol";
+import { WorldResourceIdLib, WorldResourceIdInstance } from "@latticexyz/world/src/WorldResourceId.sol";
+import { RESOURCE_SYSTEM } from "@latticexyz/world/src/worldResourceTypes.sol";
+import { IWorldErrors } from "@latticexyz/world/src/IWorldErrors.sol";
 import { createWorld } from "@latticexyz/world/test/createWorld.sol";
 
 import { ResourceId, ResourceIdLib } from "@latticexyz/store/src/ResourceId.sol";
@@ -51,10 +53,16 @@ contract MockWithWorld is WithWorld, MockStoreConsumer {
     getWorld().transferOwnership(getNamespaceId(), to);
   }
 
+  function callableByAnyone() external view {}
+
+  function onlyCallableByWorld() external view onlyWorld {}
+
   function onlyCallableByNamespace() external view onlyNamespace {}
 }
 
 contract StoreConsumerTest is Test, GasReporter {
+  using WorldResourceIdInstance for ResourceId;
+
   function testWithStore() public {
     MockWithStore mock = new MockWithStore(address(0xBEEF));
     assertEq(mock.getStoreAddress(), address(0xBEEF));
@@ -77,23 +85,83 @@ contract StoreConsumerTest is Test, GasReporter {
     assertTrue(ResourceIds.getExists(namespaceId), "Namespace not registered");
   }
 
+  // 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);
+    MockWithWorld mock = new MockWithWorld(world, namespace, true);
+    mock.transferNamespaceOwnership(address(this));
+
+    // Register the mock as a system with PRIVATE access
+    world.registerSystem(systemId, mock, false);
+
+    address alice = address(0x1234);
+
+    vm.prank(alice);
+    vm.expectRevert(abi.encodeWithSelector(IWorldErrors.World_AccessDenied.selector, systemId.toString(), alice));
+    world.call(systemId, abi.encodeCall(mock.callableByAnyone, ()));
+
+    // After granting access to namespace, it should work
+    world.grantAccess(namespaceId, alice);
+    vm.prank(alice);
+    world.call(systemId, abi.encodeCall(mock.callableByAnyone, ()));
+  }
+
+  function testOnlyWorld() public {
+    IBaseWorld world = createWorld();
+    StoreSwitch.setStoreAddress(address(world));
+
+    bytes16 systemName = "mySystem";
+    bytes14 namespace = "myNamespace";
+    ResourceId systemId = WorldResourceIdLib.encode(RESOURCE_SYSTEM, namespace, systemName);
+    MockWithWorld mock = new MockWithWorld(world, namespace, true);
+    mock.transferNamespaceOwnership(address(this));
+
+    // Register the mock as a system with PUBLIC access
+    world.registerSystem(systemId, mock, true);
+
+    address alice = address(0x1234);
+
+    vm.prank(alice);
+    vm.expectRevert(abi.encodeWithSelector(WithWorld.WithWorld_CallerIsNotWorld.selector, (alice)));
+    mock.onlyCallableByWorld();
+
+    vm.prank(alice);
+    world.call(systemId, abi.encodeCall(mock.onlyCallableByWorld, ()));
+  }
+
   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);
     MockWithWorld mock = new MockWithWorld(world, namespace, true);
     mock.transferNamespaceOwnership(address(this));
 
+    // Register the mock as a system with PUBLIC access
+    world.registerSystem(systemId, mock, true);
+
     address alice = address(0x1234);
 
     vm.prank(alice);
-    vm.expectRevert();
+    vm.expectRevert(abi.encodeWithSelector(WithWorld.WithWorld_CallerHasNoNamespaceAccess.selector, namespace, alice));
     mock.onlyCallableByNamespace();
 
+    vm.prank(alice);
+    vm.expectRevert(abi.encodeWithSelector(WithWorld.WithWorld_CallerHasNoNamespaceAccess.selector, namespace, alice));
+    world.call(systemId, abi.encodeCall(mock.onlyCallableByNamespace, ()));
+
+    // After granting access to namespace, it should work
     world.grantAccess(namespaceId, alice);
     vm.prank(alice);
-    mock.onlyCallableByNamespace();
+    world.call(systemId, abi.encodeCall(mock.onlyCallableByNamespace, ()));
   }
 }

packages/store/ts/flattenStoreLogs.test.ts

@@ -134,8 +134,8 @@ describe("flattenStoreLogs", async () => {
         "Store_SetRecord  store__ResourceIds  (0x746200000000000000000000000000005465727261696e000000000000000000)",
         "Store_SetRecord  store__ResourceIds  (0x737900000000000000000000000000004d6f766553797374656d000000000000)",
         "Store_SetRecord  world__Systems  (0x737900000000000000000000000000004d6f766553797374656d000000000000)",
-        "Store_SetRecord  world__SystemRegistry  (0x00000000000000000000000008f2b45d8787be8a81869d9968f25323861352b0)",
-        "Store_SetRecord  world__ResourceAccess  (0x6e73000000000000000000000000000000000000000000000000000000000000,0x00000000000000000000000008f2b45d8787be8a81869d9968f25323861352b0)",
+        "Store_SetRecord  world__SystemRegistry  (0x0000000000000000000000006eb355773196079fe643ed78724140adb1f86c11)",
+        "Store_SetRecord  world__ResourceAccess  (0x6e73000000000000000000000000000000000000000000000000000000000000,0x0000000000000000000000006eb355773196079fe643ed78724140adb1f86c11)",
         "Store_SetRecord  world__FunctionSelector  (0xb591186e00000000000000000000000000000000000000000000000000000000)",
         "Store_SetRecord  world__FunctionSignatur  (0xb591186e00000000000000000000000000000000000000000000000000000000)",
         "Store_SetRecord  store__Tables  (0x7462000000000000000000000000000043616c6c576974685369676e61747572)",

packages/store/ts/getStoreLogs.test.ts

@@ -157,8 +157,8 @@ describe("getStoreLogs", async () => {
         "Store_SpliceStaticData  store__ResourceIds  (0x746200000000000000000000000000005465727261696e000000000000000000)",
         "Store_SpliceStaticData  store__ResourceIds  (0x737900000000000000000000000000004d6f766553797374656d000000000000)",
         "Store_SetRecord  world__Systems  (0x737900000000000000000000000000004d6f766553797374656d000000000000)",
-        "Store_SpliceStaticData  world__SystemRegistry  (0x00000000000000000000000008f2b45d8787be8a81869d9968f25323861352b0)",
-        "Store_SpliceStaticData  world__ResourceAccess  (0x6e73000000000000000000000000000000000000000000000000000000000000,0x00000000000000000000000008f2b45d8787be8a81869d9968f25323861352b0)",
+        "Store_SpliceStaticData  world__SystemRegistry  (0x0000000000000000000000006eb355773196079fe643ed78724140adb1f86c11)",
+        "Store_SpliceStaticData  world__ResourceAccess  (0x6e73000000000000000000000000000000000000000000000000000000000000,0x0000000000000000000000006eb355773196079fe643ed78724140adb1f86c11)",
         "Store_SetRecord  world__FunctionSelector  (0xb591186e00000000000000000000000000000000000000000000000000000000)",
         "Store_SetRecord  world__FunctionSignatur  (0xb591186e00000000000000000000000000000000000000000000000000000000)",
         "Store_SetRecord  world__FunctionSignatur  (0xb591186e00000000000000000000000000000000000000000000000000000000)",