From f61ca11c67e44e11d3620898f4c0da401892b916 Mon Sep 17 00:00:00 2001 From: vdrg Date: Thu, 30 Jan 2025 15:27:18 -0300 Subject: [PATCH 01/10] Adapt store consumer WithWorld to be a system --- .../src/examples/SimpleVault.sol | 2 +- .../src/experimental/Context.sol | 2 +- .../src/experimental/WithWorld.sol | 22 ++++++++++++++----- .../store-consumer/test/StoreConsumer.t.sol | 15 ++++++++----- .../src/examples/ERC20WithWorld.sol | 11 +++++++--- .../test/ERC20BaseTest.t.sol | 5 +++++ .../test/ERC20Burnable.t.sol | 8 ++++++- .../test/ERC20Pausable.t.sol | 6 +++++ packages/world/src/WorldContext.sol | 2 +- 9 files changed, 55 insertions(+), 18 deletions(-) diff --git a/packages/store-consumer/src/examples/SimpleVault.sol b/packages/store-consumer/src/examples/SimpleVault.sol index edf16e6dd6..7f1eb472de 100644 --- a/packages/store-consumer/src/examples/SimpleVault.sol +++ b/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"); } diff --git a/packages/store-consumer/src/experimental/Context.sol b/packages/store-consumer/src/experimental/Context.sol index a2a9b74081..2fb759e440 100644 --- a/packages/store-consumer/src/experimental/Context.sol +++ b/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; } } diff --git a/packages/store-consumer/src/experimental/WithWorld.sol b/packages/store-consumer/src/experimental/WithWorld.sol index 6202e273c7..bd7e25fd9e 100644 --- a/packages/store-consumer/src/experimental/WithWorld.sol +++ b/packages/store-consumer/src/experimental/WithWorld.sol @@ -6,21 +6,23 @@ 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_CallerIsNotWorld(); - modifier onlyNamespace() { - address sender = _msgSender(); - if (!ResourceAccess.get(getNamespaceId(), sender)) { - revert WithWorld_CallerHasNoNamespaceAccess(); + modifier onlyWorld() { + if (!_callerIsWorld()) { + revert WithWorld_CallerIsNotWorld(); } _; } @@ -55,6 +57,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); } diff --git a/packages/store-consumer/test/StoreConsumer.t.sol b/packages/store-consumer/test/StoreConsumer.t.sol index 65926f8849..201b2bae47 100644 --- a/packages/store-consumer/test/StoreConsumer.t.sol +++ b/packages/store-consumer/test/StoreConsumer.t.sol @@ -9,6 +9,7 @@ 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 { RESOURCE_SYSTEM } from "@latticexyz/world/src/worldResourceTypes.sol"; import { createWorld } from "@latticexyz/world/test/createWorld.sol"; import { ResourceId, ResourceIdLib } from "@latticexyz/store/src/ResourceId.sol"; @@ -51,7 +52,7 @@ contract MockWithWorld is WithWorld, MockStoreConsumer { getWorld().transferOwnership(getNamespaceId(), to); } - function onlyCallableByNamespace() external view onlyNamespace {} + function onlyCallableByWorld() external view onlyWorld {} } contract StoreConsumerTest is Test, GasReporter { @@ -77,23 +78,27 @@ contract StoreConsumerTest is Test, GasReporter { assertTrue(ResourceIds.getExists(namespaceId), "Namespace not registered"); } - function testOnlyNamespace() public { + function testOnlyWorld() 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 + world.registerSystem(systemId, mock, false); + address alice = address(0x1234); vm.prank(alice); vm.expectRevert(); - mock.onlyCallableByNamespace(); + mock.onlyCallableByWorld(); world.grantAccess(namespaceId, alice); - vm.prank(alice); - mock.onlyCallableByNamespace(); + world.call(systemId, abi.encodeCall(mock.onlyCallableByWorld, ())); } } diff --git a/packages/world-module-erc20/src/examples/ERC20WithWorld.sol b/packages/world-module-erc20/src/examples/ERC20WithWorld.sol index 7023600048..74fdc5dc6e 100644 --- a/packages/world-module-erc20/src/examples/ERC20WithWorld.sol +++ b/packages/world-module-erc20/src/examples/ERC20WithWorld.sol @@ -6,6 +6,7 @@ pragma solidity >=0.8.24; import { ResourceId } from "@latticexyz/store/src/ResourceId.sol"; import { IBaseWorld } from "@latticexyz/world/src/codegen/interfaces/IBaseWorld.sol"; import { WithWorld } from "@latticexyz/store-consumer/src/experimental/WithWorld.sol"; +import { Context } from "@latticexyz/store-consumer/src/experimental/Context.sol"; import { ERC20Pausable } from "../experimental/ERC20Pausable.sol"; import { ERC20Burnable } from "../experimental/ERC20Burnable.sol"; @@ -22,20 +23,24 @@ contract ERC20WithWorld is WithWorld, MUDERC20, ERC20Pausable, ERC20Burnable { world.transferOwnership(getNamespaceId(), _msgSender()); } - function mint(address to, uint256 value) public onlyNamespace { + function mint(address to, uint256 value) public onlyWorld { _mint(to, value); } - function pause() public onlyNamespace { + function pause() public onlyWorld { _pause(); } - function unpause() public onlyNamespace { + function unpause() public onlyWorld { _unpause(); } // The following functions are overrides required by Solidity. + function _msgSender() public view override(Context, WithWorld) returns (address sender) { + return super._msgSender(); + } + function _update(address from, address to, uint256 value) internal override(MUDERC20, ERC20Pausable) { super._update(from, to, value); } diff --git a/packages/world-module-erc20/test/ERC20BaseTest.t.sol b/packages/world-module-erc20/test/ERC20BaseTest.t.sol index 17246b4c9e..d483b48ada 100644 --- a/packages/world-module-erc20/test/ERC20BaseTest.t.sol +++ b/packages/world-module-erc20/test/ERC20BaseTest.t.sol @@ -14,6 +14,7 @@ import { StoreSwitch } from "@latticexyz/store/src/StoreSwitch.sol"; import { WithStore } from "@latticexyz/store-consumer/src/experimental/WithStore.sol"; import { WithWorld } from "@latticexyz/store-consumer/src/experimental/WithWorld.sol"; +import { Context } from "@latticexyz/store-consumer/src/experimental/Context.sol"; import { ERC20MetadataData } from "../src/codegen/tables/ERC20Metadata.sol"; import { IERC20 } from "../src/interfaces/IERC20.sol"; @@ -43,6 +44,10 @@ contract MockERC20WithInternalStore is WithStore(address(this)), MockERC20Base { contract MockERC20WithWorld is WithWorld, MockERC20Base { constructor() WithWorld(createWorld(), TestConstants.ERC20_NAMESPACE, true) {} + + function _msgSender() public view virtual override(Context, WithWorld) returns (address sender) { + return super._msgSender(); + } } abstract contract ERC20BehaviorTest is Test, GasReporter, IERC20Events, IERC20Errors { diff --git a/packages/world-module-erc20/test/ERC20Burnable.t.sol b/packages/world-module-erc20/test/ERC20Burnable.t.sol index cdb8342e89..837f9a3757 100644 --- a/packages/world-module-erc20/test/ERC20Burnable.t.sol +++ b/packages/world-module-erc20/test/ERC20Burnable.t.sol @@ -10,6 +10,8 @@ import { GasReporter } from "@latticexyz/gas-report/src/GasReporter.sol"; import { IBaseWorld } from "@latticexyz/world/src/codegen/interfaces/IBaseWorld.sol"; import { createWorld } from "@latticexyz/world/test/createWorld.sol"; +import { Context } from "@latticexyz/store-consumer/src/experimental/Context.sol"; + import { ERC20MetadataData } from "../src/codegen/tables/ERC20Metadata.sol"; import { IERC20Errors } from "../src/interfaces/IERC20Errors.sol"; import { IERC20Events } from "../src/interfaces/IERC20Events.sol"; @@ -19,7 +21,11 @@ import { MockERC20Base, MockERC20WithInternalStore, MockERC20WithWorld, ERC20Beh contract MockERC20WithInternalStoreBurnable is MockERC20WithInternalStore, ERC20Burnable {} -contract MockERC20WithWorldBurnable is MockERC20WithWorld, ERC20Burnable {} +contract MockERC20WithWorldBurnable is MockERC20WithWorld, ERC20Burnable { + function _msgSender() public view override(Context, MockERC20WithWorld) returns (address sender) { + return super._msgSender(); + } +} abstract contract ERC20BurnableTest is ERC20BehaviorTest { function testBurnByAccount() public { diff --git a/packages/world-module-erc20/test/ERC20Pausable.t.sol b/packages/world-module-erc20/test/ERC20Pausable.t.sol index 402de51ee0..dd9daae472 100644 --- a/packages/world-module-erc20/test/ERC20Pausable.t.sol +++ b/packages/world-module-erc20/test/ERC20Pausable.t.sol @@ -7,6 +7,8 @@ import { console } from "forge-std/console.sol"; import { GasReporter } from "@latticexyz/gas-report/src/GasReporter.sol"; import { IBaseWorld } from "@latticexyz/world/src/codegen/interfaces/IBaseWorld.sol"; +import { Context } from "@latticexyz/store-consumer/src/experimental/Context.sol"; + import { ERC20MetadataData } from "../src/codegen/tables/ERC20Metadata.sol"; import { IERC20Errors } from "../src/interfaces/IERC20Errors.sol"; import { IERC20Events } from "../src/interfaces/IERC20Events.sol"; @@ -39,6 +41,10 @@ contract MockERC20WithWorldPausable is MockERC20WithWorld, MockERC20Pausable { function _update(address from, address to, uint256 value) internal override(MUDERC20, MockERC20Pausable) { super._update(from, to, value); } + + function _msgSender() public view override(Context, MockERC20WithWorld) returns (address sender) { + return super._msgSender(); + } } abstract contract ERC20PausableBehaviorTest is ERC20BehaviorTest { diff --git a/packages/world/src/WorldContext.sol b/packages/world/src/WorldContext.sol index 92637732c1..29347a1634 100644 --- a/packages/world/src/WorldContext.sol +++ b/packages/world/src/WorldContext.sol @@ -23,7 +23,7 @@ abstract contract WorldContextConsumer is IWorldContextConsumer { * @return sender The `msg.sender` in the call to the World contract before the World routed the * call to the WorldContextConsumer contract. */ - function _msgSender() public view returns (address sender) { + function _msgSender() public view virtual returns (address sender) { return WorldContextConsumerLib._msgSender(); } From e02e71843cce902b60d8297be2edc7e23842b180 Mon Sep 17 00:00:00 2001 From: vdrg Date: Thu, 30 Jan 2025 15:52:30 -0300 Subject: [PATCH 02/10] Adapt module --- .../src/experimental/ERC20Module.sol | 10 +++++++++- .../world-module-erc20/test/ERC20BaseTest.t.sol | 2 +- .../world-module-erc20/test/ERC20Module.t.sol | 15 +++++++++++++-- .../world-module-erc20/test/ERC20Pausable.t.sol | 2 +- .../world-module-erc20/ts/defineERC20Module.ts | 4 ++-- 5 files changed, 26 insertions(+), 7 deletions(-) diff --git a/packages/world-module-erc20/src/experimental/ERC20Module.sol b/packages/world-module-erc20/src/experimental/ERC20Module.sol index 7c2a371c92..bf45a67c3b 100644 --- a/packages/world-module-erc20/src/experimental/ERC20Module.sol +++ b/packages/world-module-erc20/src/experimental/ERC20Module.sol @@ -5,6 +5,7 @@ import { ResourceIds } from "@latticexyz/store/src/codegen/tables/ResourceIds.so import { ResourceId } from "@latticexyz/store/src/ResourceId.sol"; import { Module } from "@latticexyz/world/src/Module.sol"; +import { RESOURCE_SYSTEM } from "@latticexyz/world/src/worldResourceTypes.sol"; import { WorldResourceIdLib } from "@latticexyz/world/src/WorldResourceId.sol"; import { IBaseWorld } from "@latticexyz/world/src/codegen/interfaces/IBaseWorld.sol"; import { revertWithBytes } from "@latticexyz/world/src/revertWithBytes.sol"; @@ -21,9 +22,13 @@ contract ERC20Module is Module { // TODO: we should probably check just for namespace, not for all args requireNotInstalled(__self, encodedArgs); - (bytes14 namespace, string memory name, string memory symbol) = abi.decode(encodedArgs, (bytes14, string, string)); + (bytes14 namespace, bytes16 systemName, string memory name, string memory symbol) = abi.decode( + encodedArgs, + (bytes14, bytes16, string, string) + ); ResourceId namespaceId = WorldResourceIdLib.encodeNamespace(namespace); + ResourceId systemId = WorldResourceIdLib.encode(RESOURCE_SYSTEM, namespace, systemName); // Require the namespace to not be the module's namespace if (namespace == ModuleConstants.NAMESPACE) { @@ -39,6 +44,9 @@ contract ERC20Module is Module { // Grant access to the token so it can write to tables after transferring ownership world.grantAccess(namespaceId, address(token)); + // Register token as a system so `onlyWorld` functions can be called through the world + world.registerSystem(systemId, token, false); + // The token should have transferred the namespace ownership to this module in its constructor world.transferOwnership(namespaceId, _msgSender()); diff --git a/packages/world-module-erc20/test/ERC20BaseTest.t.sol b/packages/world-module-erc20/test/ERC20BaseTest.t.sol index d483b48ada..16bf2bf3cb 100644 --- a/packages/world-module-erc20/test/ERC20BaseTest.t.sol +++ b/packages/world-module-erc20/test/ERC20BaseTest.t.sol @@ -46,7 +46,7 @@ contract MockERC20WithWorld is WithWorld, MockERC20Base { constructor() WithWorld(createWorld(), TestConstants.ERC20_NAMESPACE, true) {} function _msgSender() public view virtual override(Context, WithWorld) returns (address sender) { - return super._msgSender(); + return WithWorld._msgSender(); } } diff --git a/packages/world-module-erc20/test/ERC20Module.t.sol b/packages/world-module-erc20/test/ERC20Module.t.sol index a3e567d3e1..e8db57edc6 100644 --- a/packages/world-module-erc20/test/ERC20Module.t.sol +++ b/packages/world-module-erc20/test/ERC20Module.t.sol @@ -22,6 +22,7 @@ import { ERC20Module } from "../src/experimental/ERC20Module.sol"; import { ERC20Registry } from "../src/codegen/tables/ERC20Registry.sol"; library TestConstants { + bytes16 constant ERC20_SYSTEM_NAME = "erc20system"; bytes14 constant ERC20_NAMESPACE = "erc20namespace"; } @@ -37,7 +38,12 @@ contract ERC20ModuleTest is Test, GasReporter { } function testInstall() public { - bytes memory args = abi.encode(TestConstants.ERC20_NAMESPACE, "myERC20Token", "MTK"); + bytes memory args = abi.encode( + TestConstants.ERC20_NAMESPACE, + TestConstants.ERC20_SYSTEM_NAME, + "myERC20Token", + "MTK" + ); startGasReport("install erc20 module"); world.installModule(erc20Module, args); endGasReport(); @@ -62,7 +68,12 @@ contract ERC20ModuleTest is Test, GasReporter { ResourceId moduleNamespaceId = ModuleConstants.namespaceId(); world.registerNamespace(moduleNamespaceId); - bytes memory args = abi.encode(TestConstants.ERC20_NAMESPACE, "myERC20Token", "MTK"); + bytes memory args = abi.encode( + TestConstants.ERC20_NAMESPACE, + TestConstants.ERC20_SYSTEM_NAME, + "myERC20Token", + "MTK" + ); // Installing will revert because module namespace already exists vm.expectRevert( diff --git a/packages/world-module-erc20/test/ERC20Pausable.t.sol b/packages/world-module-erc20/test/ERC20Pausable.t.sol index dd9daae472..99bf72047e 100644 --- a/packages/world-module-erc20/test/ERC20Pausable.t.sol +++ b/packages/world-module-erc20/test/ERC20Pausable.t.sol @@ -43,7 +43,7 @@ contract MockERC20WithWorldPausable is MockERC20WithWorld, MockERC20Pausable { } function _msgSender() public view override(Context, MockERC20WithWorld) returns (address sender) { - return super._msgSender(); + return MockERC20WithWorld._msgSender(); } } diff --git a/packages/world-module-erc20/ts/defineERC20Module.ts b/packages/world-module-erc20/ts/defineERC20Module.ts index 5c9533a06c..b632355672 100644 --- a/packages/world-module-erc20/ts/defineERC20Module.ts +++ b/packages/world-module-erc20/ts/defineERC20Module.ts @@ -9,8 +9,8 @@ export type DefineERC20ModuleInput = { export function defineERC20Module({ namespace, name, symbol }: DefineERC20ModuleInput): ModuleInput { const erc20ModuleArgs = encodeAbiParameters( - [{ type: "bytes14" }, { type: "string" }, { type: "string" }], - [stringToHex(namespace, { size: 14 }), name, symbol], + [{ type: "bytes14" }, { type: "bytes16" }, { type: "string" }, { type: "string" }], + [stringToHex(namespace, { size: 14 }), stringToHex(name, { size: 16 }), name, symbol], ); return { From 77e7c774d8050cb8fface7d6945b888e7566817a Mon Sep 17 00:00:00 2001 From: vdrg Date: Thu, 30 Jan 2025 19:04:14 -0300 Subject: [PATCH 03/10] Add onlyNamespace modifier and improve tests --- .../src/experimental/WithWorld.sol | 9 +++ .../store-consumer/test/StoreConsumer.t.sol | 66 ++++++++++++++++++- .../src/examples/ERC20WithWorld.sol | 8 +-- .../src/experimental/ERC20Module.sol | 4 +- .../test/ERC20BaseTest.t.sol | 27 ++++---- 5 files changed, 94 insertions(+), 20 deletions(-) diff --git a/packages/store-consumer/src/experimental/WithWorld.sol b/packages/store-consumer/src/experimental/WithWorld.sol index bd7e25fd9e..2875604899 100644 --- a/packages/store-consumer/src/experimental/WithWorld.sol +++ b/packages/store-consumer/src/experimental/WithWorld.sol @@ -18,6 +18,7 @@ abstract contract WithWorld is WithStore, System { error WithWorld_RootNamespaceNotAllowed(); error WithWorld_NamespaceAlreadyExists(); error WithWorld_NamespaceDoesNotExists(); + error WithWorld_CallerHasNoNamespaceAccess(); error WithWorld_CallerIsNotWorld(); modifier onlyWorld() { @@ -27,6 +28,14 @@ abstract contract WithWorld is WithStore, System { _; } + modifier onlyNamespace() { + // We use WorldContextConsumer directly as we already know the world is the caller + if (!_callerIsWorld() || !ResourceAccess.get(getNamespaceId(), WorldContextConsumer._msgSender())) { + revert WithWorld_CallerHasNoNamespaceAccess(); + } + _; + } + constructor(IBaseWorld _world, bytes14 _namespace, bool registerNamespace) WithStore(address(_world)) { if (_namespace == bytes14(0)) { revert WithWorld_RootNamespaceNotAllowed(); diff --git a/packages/store-consumer/test/StoreConsumer.t.sol b/packages/store-consumer/test/StoreConsumer.t.sol index 201b2bae47..1070654a5a 100644 --- a/packages/store-consumer/test/StoreConsumer.t.sol +++ b/packages/store-consumer/test/StoreConsumer.t.sol @@ -52,7 +52,11 @@ 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 { @@ -78,7 +82,8 @@ contract StoreConsumerTest is Test, GasReporter { assertTrue(ResourceIds.getExists(namespaceId), "Namespace not registered"); } - function testOnlyWorld() public { + // Test internal MUD access control + function testAccessControl() public { IBaseWorld world = createWorld(); StoreSwitch.setStoreAddress(address(world)); @@ -89,16 +94,71 @@ contract StoreConsumerTest is Test, GasReporter { MockWithWorld mock = new MockWithWorld(world, namespace, true); mock.transferNamespaceOwnership(address(this)); - // Register the mock as a system + // Register the mock as a system with PRIVATE access world.registerSystem(systemId, mock, false); address alice = address(0x1234); vm.prank(alice); vm.expectRevert(); - mock.onlyCallableByWorld(); + 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(); + 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(); + mock.onlyCallableByNamespace(); + + vm.prank(alice); + vm.expectRevert(); + world.call(systemId, abi.encodeCall(mock.onlyCallableByNamespace, ())); + + // After granting access to namespace, it should work + world.grantAccess(namespaceId, alice); + vm.prank(alice); + world.call(systemId, abi.encodeCall(mock.onlyCallableByNamespace, ())); + } } diff --git a/packages/world-module-erc20/src/examples/ERC20WithWorld.sol b/packages/world-module-erc20/src/examples/ERC20WithWorld.sol index 74fdc5dc6e..eae98b639e 100644 --- a/packages/world-module-erc20/src/examples/ERC20WithWorld.sol +++ b/packages/world-module-erc20/src/examples/ERC20WithWorld.sol @@ -23,22 +23,22 @@ contract ERC20WithWorld is WithWorld, MUDERC20, ERC20Pausable, ERC20Burnable { world.transferOwnership(getNamespaceId(), _msgSender()); } - function mint(address to, uint256 value) public onlyWorld { + function mint(address to, uint256 value) public onlyNamespace { _mint(to, value); } - function pause() public onlyWorld { + function pause() public onlyNamespace { _pause(); } - function unpause() public onlyWorld { + function unpause() public onlyNamespace { _unpause(); } // The following functions are overrides required by Solidity. function _msgSender() public view override(Context, WithWorld) returns (address sender) { - return super._msgSender(); + return WithWorld._msgSender(); } function _update(address from, address to, uint256 value) internal override(MUDERC20, ERC20Pausable) { diff --git a/packages/world-module-erc20/src/experimental/ERC20Module.sol b/packages/world-module-erc20/src/experimental/ERC20Module.sol index bf45a67c3b..3f35711277 100644 --- a/packages/world-module-erc20/src/experimental/ERC20Module.sol +++ b/packages/world-module-erc20/src/experimental/ERC20Module.sol @@ -44,8 +44,8 @@ contract ERC20Module is Module { // Grant access to the token so it can write to tables after transferring ownership world.grantAccess(namespaceId, address(token)); - // Register token as a system so `onlyWorld` functions can be called through the world - world.registerSystem(systemId, token, false); + // Register token as a system so its functions can be called through the world + world.registerSystem(systemId, token, true); // The token should have transferred the namespace ownership to this module in its constructor world.transferOwnership(namespaceId, _msgSender()); diff --git a/packages/world-module-erc20/test/ERC20BaseTest.t.sol b/packages/world-module-erc20/test/ERC20BaseTest.t.sol index 16bf2bf3cb..dddf7d3d96 100644 --- a/packages/world-module-erc20/test/ERC20BaseTest.t.sol +++ b/packages/world-module-erc20/test/ERC20BaseTest.t.sol @@ -55,6 +55,11 @@ abstract contract ERC20BehaviorTest is Test, GasReporter, IERC20Events, IERC20Er function createToken() internal virtual returns (MockERC20Base); + // Used for validating the addresses in fuzz tests + function validAddress(address addr) internal view returns (bool) { + return addr != address(0) && addr != StoreSwitch.getStoreAddress(); + } + // TODO: startGasReport should be marked virtual so we can override function startGasReportWithPrefix(string memory name) internal { startGasReport(reportNameWithPrefix(name)); @@ -213,7 +218,7 @@ abstract contract ERC20BehaviorTest is Test, GasReporter, IERC20Events, IERC20Er } function testMint(address to, uint256 amount) public { - vm.assume(to != address(0)); + vm.assume(validAddress(to)); vm.expectEmit(true, true, true, true); emit Transfer(address(0), to, amount); @@ -224,7 +229,7 @@ abstract contract ERC20BehaviorTest is Test, GasReporter, IERC20Events, IERC20Er } function testBurn(address from, uint256 mintAmount, uint256 burnAmount) public { - vm.assume(from != address(0)); + vm.assume(validAddress(from)); vm.assume(burnAmount <= mintAmount); token.__mint(from, mintAmount); @@ -237,7 +242,7 @@ abstract contract ERC20BehaviorTest is Test, GasReporter, IERC20Events, IERC20Er } function testApprove(address to, uint256 amount) public { - vm.assume(to != address(0)); + vm.assume(validAddress(to)); assertTrue(token.approve(to, amount)); @@ -245,7 +250,7 @@ abstract contract ERC20BehaviorTest is Test, GasReporter, IERC20Events, IERC20Er } function testTransfer(address to, uint256 amount) public { - vm.assume(to != address(0)); + vm.assume(validAddress(to)); token.__mint(address(this), amount); vm.expectEmit(true, true, true, true); @@ -262,9 +267,9 @@ abstract contract ERC20BehaviorTest is Test, GasReporter, IERC20Events, IERC20Er } function testTransferFrom(address spender, address from, address to, uint256 approval, uint256 amount) public { - vm.assume(from != address(0)); - vm.assume(to != address(0)); - vm.assume(spender != address(0)); + vm.assume(validAddress(from)); + vm.assume(validAddress(to)); + vm.assume(validAddress(spender)); vm.assume(amount <= approval); token.__mint(from, amount); @@ -294,7 +299,7 @@ abstract contract ERC20BehaviorTest is Test, GasReporter, IERC20Events, IERC20Er } function testBurnInsufficientBalanceReverts(address to, uint256 mintAmount, uint256 burnAmount) public { - vm.assume(to != address(0)); + vm.assume(validAddress(to)); vm.assume(mintAmount < type(uint256).max); vm.assume(burnAmount > mintAmount); @@ -304,7 +309,7 @@ abstract contract ERC20BehaviorTest is Test, GasReporter, IERC20Events, IERC20Er } function testTransferInsufficientBalanceReverts(address to, uint256 mintAmount, uint256 sendAmount) public { - vm.assume(to != address(0)); + vm.assume(validAddress(to)); vm.assume(mintAmount < type(uint256).max); vm.assume(sendAmount > mintAmount); @@ -314,7 +319,7 @@ abstract contract ERC20BehaviorTest is Test, GasReporter, IERC20Events, IERC20Er } function testTransferFromInsufficientAllowanceReverts(address to, uint256 approval, uint256 amount) public { - vm.assume(to != address(0)); + vm.assume(validAddress(to)); vm.assume(approval < type(uint256).max); vm.assume(amount > approval); @@ -330,7 +335,7 @@ abstract contract ERC20BehaviorTest is Test, GasReporter, IERC20Events, IERC20Er } function testTransferFromInsufficientBalanceReverts(address to, uint256 mintAmount, uint256 sendAmount) public { - vm.assume(to != address(0)); + vm.assume(validAddress(to)); vm.assume(mintAmount < type(uint256).max); vm.assume(sendAmount > mintAmount); From 4fcb9fded6a16a9dc7ec37616d205e2b4059ca12 Mon Sep 17 00:00:00 2001 From: vdrg Date: Mon, 3 Feb 2025 10:04:24 -0300 Subject: [PATCH 04/10] Update docs and store test snapshot --- docs/pages/world/modules/erc20.mdx | 6 +++++- docs/pages/world/reference/world-context.mdx | 2 +- packages/store/ts/flattenStoreLogs.test.ts | 4 ++-- packages/store/ts/getStoreLogs.test.ts | 4 ++-- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/docs/pages/world/modules/erc20.mdx b/docs/pages/world/modules/erc20.mdx index 78c6f7569e..1da9f6beab 100644 --- a/docs/pages/world/modules/erc20.mdx +++ b/docs/pages/world/modules/erc20.mdx @@ -64,7 +64,9 @@ 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 +98,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 +110,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 diff --git a/docs/pages/world/reference/world-context.mdx b/docs/pages/world/reference/world-context.mdx index aca58289d0..a5416edf2d 100644 --- a/docs/pages/world/reference/world-context.mdx +++ b/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** diff --git a/packages/store/ts/flattenStoreLogs.test.ts b/packages/store/ts/flattenStoreLogs.test.ts index 777c7cb7ca..d2fe1669b9 100644 --- a/packages/store/ts/flattenStoreLogs.test.ts +++ b/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)", diff --git a/packages/store/ts/getStoreLogs.test.ts b/packages/store/ts/getStoreLogs.test.ts index 94962e9d14..2035b67471 100644 --- a/packages/store/ts/getStoreLogs.test.ts +++ b/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)", From 8709d4098cf5c5d893adc0c4bd924977f6b7f52d Mon Sep 17 00:00:00 2001 From: vdrg Date: Mon, 3 Feb 2025 10:26:48 -0300 Subject: [PATCH 05/10] Update gas report --- packages/world-module-erc20/gas-report.json | 44 ++++++++++----------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/packages/world-module-erc20/gas-report.json b/packages/world-module-erc20/gas-report.json index d7486f9da6..64f623122a 100644 --- a/packages/world-module-erc20/gas-report.json +++ b/packages/world-module-erc20/gas-report.json @@ -9,37 +9,37 @@ "file": "test/ERC20BaseTest.t.sol:ERC20WithInternalStoreTest", "test": "testBurn", "name": "internal_burn", - "gasUsed": 56310 + "gasUsed": 56332 }, { "file": "test/ERC20BaseTest.t.sol:ERC20WithInternalStoreTest", "test": "testMint", "name": "internal_mint", - "gasUsed": 90597 + "gasUsed": 90553 }, { "file": "test/ERC20BaseTest.t.sol:ERC20WithInternalStoreTest", "test": "testTransfer", "name": "internal_transfer", - "gasUsed": 67634 + "gasUsed": 67589 }, { "file": "test/ERC20BaseTest.t.sol:ERC20WithInternalStoreTest", "test": "testTransferFrom", "name": "internal_transferFrom", - "gasUsed": 79402 + "gasUsed": 79424 }, { "file": "test/ERC20BaseTest.t.sol:ERC20WithWorldTest", "test": "testApprove", "name": "world_approve", - "gasUsed": 66268 + "gasUsed": 66679 }, { "file": "test/ERC20BaseTest.t.sol:ERC20WithWorldTest", "test": "testBurn", "name": "world_burn", - "gasUsed": 70688 + "gasUsed": 70711 }, { "file": "test/ERC20BaseTest.t.sol:ERC20WithWorldTest", @@ -51,13 +51,13 @@ "file": "test/ERC20BaseTest.t.sol:ERC20WithWorldTest", "test": "testTransfer", "name": "world_transfer", - "gasUsed": 82206 + "gasUsed": 82595 }, { "file": "test/ERC20BaseTest.t.sol:ERC20WithWorldTest", "test": "testTransferFrom", "name": "world_transferFrom", - "gasUsed": 99142 + "gasUsed": 99580 }, { "file": "test/ERC20Burnable.t.sol:ERC20BurnableWithInternalStoreTest", @@ -75,13 +75,13 @@ "file": "test/ERC20Burnable.t.sol:ERC20BurnableWithInternalStoreTest", "test": "testBurnByAccount", "name": "internal_burn", - "gasUsed": 56454 + "gasUsed": 56388 }, { "file": "test/ERC20Burnable.t.sol:ERC20BurnableWithInternalStoreTest", "test": "testMint", "name": "internal_mint", - "gasUsed": 90597 + "gasUsed": 90619 }, { "file": "test/ERC20Burnable.t.sol:ERC20BurnableWithInternalStoreTest", @@ -93,13 +93,13 @@ "file": "test/ERC20Burnable.t.sol:ERC20BurnableWithInternalStoreTest", "test": "testTransferFrom", "name": "internal_transferFrom", - "gasUsed": 79402 + "gasUsed": 79424 }, { "file": "test/ERC20Burnable.t.sol:ERC20BurnableWithWorldTest", "test": "testApprove", "name": "world_approve", - "gasUsed": 66268 + "gasUsed": 66691 }, { "file": "test/ERC20Burnable.t.sol:ERC20BurnableWithWorldTest", @@ -111,7 +111,7 @@ "file": "test/ERC20Burnable.t.sol:ERC20BurnableWithWorldTest", "test": "testBurnByAccount", "name": "world_burn", - "gasUsed": 70810 + "gasUsed": 71240 }, { "file": "test/ERC20Burnable.t.sol:ERC20BurnableWithWorldTest", @@ -123,19 +123,19 @@ "file": "test/ERC20Burnable.t.sol:ERC20BurnableWithWorldTest", "test": "testTransfer", "name": "world_transfer", - "gasUsed": 82162 + "gasUsed": 82585 }, { "file": "test/ERC20Burnable.t.sol:ERC20BurnableWithWorldTest", "test": "testTransferFrom", "name": "world_transferFrom", - "gasUsed": 99209 + "gasUsed": 99592 }, { "file": "test/ERC20Module.t.sol:ERC20ModuleTest", "test": "testInstall", "name": "install erc20 module", - "gasUsed": 4499347 + "gasUsed": 4729741 }, { "file": "test/ERC20Pausable.t.sol:ERC20PausableWithInternalStoreTest", @@ -183,7 +183,7 @@ "file": "test/ERC20Pausable.t.sol:ERC20PausableWithWorldTest", "test": "testApprove", "name": "world_approve", - "gasUsed": 66268 + "gasUsed": 66691 }, { "file": "test/ERC20Pausable.t.sol:ERC20PausableWithWorldTest", @@ -195,30 +195,30 @@ "file": "test/ERC20Pausable.t.sol:ERC20PausableWithWorldTest", "test": "testMint", "name": "world_mint", - "gasUsed": 109697 + "gasUsed": 109719 }, { "file": "test/ERC20Pausable.t.sol:ERC20PausableWithWorldTest", "test": "testPause", "name": "world_pause", - "gasUsed": 66012 + "gasUsed": 66430 }, { "file": "test/ERC20Pausable.t.sol:ERC20PausableWithWorldTest", "test": "testPause", "name": "world_unpause", - "gasUsed": 44097 + "gasUsed": 44526 }, { "file": "test/ERC20Pausable.t.sol:ERC20PausableWithWorldTest", "test": "testTransfer", "name": "world_transfer", - "gasUsed": 86883 + "gasUsed": 87306 }, { "file": "test/ERC20Pausable.t.sol:ERC20PausableWithWorldTest", "test": "testTransferFrom", "name": "world_transferFrom", - "gasUsed": 103931 + "gasUsed": 104314 } ] From 5baa242017192896c2f73904dfdbdc41b3d46ece Mon Sep 17 00:00:00 2001 From: vdrg Date: Mon, 3 Feb 2025 10:48:16 -0300 Subject: [PATCH 06/10] Update erc20 docs --- docs/pages/world/modules/erc20.mdx | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/docs/pages/world/modules/erc20.mdx b/docs/pages/world/modules/erc20.mdx index 1da9f6beab..6907306d24 100644 --- a/docs/pages/world/modules/erc20.mdx +++ b/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,7 +60,7 @@ For example, run this script. -```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"; @@ -142,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 @@ -157,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"); From f7d8eb031c5b4e22376462d9a720e570d83bda24 Mon Sep 17 00:00:00 2001 From: vdrg Date: Tue, 4 Feb 2025 11:47:03 -0300 Subject: [PATCH 07/10] Add error data to expectRevert --- packages/store-consumer/test/StoreConsumer.t.sol | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/store-consumer/test/StoreConsumer.t.sol b/packages/store-consumer/test/StoreConsumer.t.sol index 1070654a5a..0e891e517f 100644 --- a/packages/store-consumer/test/StoreConsumer.t.sol +++ b/packages/store-consumer/test/StoreConsumer.t.sol @@ -8,8 +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"; @@ -60,6 +61,8 @@ contract MockWithWorld is WithWorld, MockStoreConsumer { } contract StoreConsumerTest is Test, GasReporter { + using WorldResourceIdInstance for ResourceId; + function testWithStore() public { MockWithStore mock = new MockWithStore(address(0xBEEF)); assertEq(mock.getStoreAddress(), address(0xBEEF)); @@ -100,7 +103,7 @@ contract StoreConsumerTest is Test, GasReporter { address alice = address(0x1234); vm.prank(alice); - vm.expectRevert(); + 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 @@ -125,7 +128,7 @@ contract StoreConsumerTest is Test, GasReporter { address alice = address(0x1234); vm.prank(alice); - vm.expectRevert(); + vm.expectRevert(WithWorld.WithWorld_CallerIsNotWorld.selector); mock.onlyCallableByWorld(); vm.prank(alice); @@ -149,11 +152,11 @@ contract StoreConsumerTest is Test, GasReporter { address alice = address(0x1234); vm.prank(alice); - vm.expectRevert(); + vm.expectRevert(WithWorld.WithWorld_CallerHasNoNamespaceAccess.selector); mock.onlyCallableByNamespace(); vm.prank(alice); - vm.expectRevert(); + vm.expectRevert(WithWorld.WithWorld_CallerHasNoNamespaceAccess.selector); world.call(systemId, abi.encodeCall(mock.onlyCallableByNamespace, ())); // After granting access to namespace, it should work From c862636a2a6791d81689624e0284dc3924df9d59 Mon Sep 17 00:00:00 2001 From: V Date: Tue, 4 Feb 2025 11:59:28 -0300 Subject: [PATCH 08/10] Create loud-moons-tell.md --- .changeset/loud-moons-tell.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .changeset/loud-moons-tell.md diff --git a/.changeset/loud-moons-tell.md b/.changeset/loud-moons-tell.md new file mode 100644 index 0000000000..7deb58ba94 --- /dev/null +++ b/.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. From 6403c30c45d4449bf26c2405a8b86e31235a4345 Mon Sep 17 00:00:00 2001 From: vdrg Date: Tue, 4 Feb 2025 20:08:34 -0300 Subject: [PATCH 09/10] Add arguments to WithWorld errors --- .../src/experimental/WithWorld.sol | 16 ++++++++-------- packages/store-consumer/test/StoreConsumer.t.sol | 6 +++--- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/store-consumer/src/experimental/WithWorld.sol b/packages/store-consumer/src/experimental/WithWorld.sol index 2875604899..51760b6eff 100644 --- a/packages/store-consumer/src/experimental/WithWorld.sol +++ b/packages/store-consumer/src/experimental/WithWorld.sol @@ -16,14 +16,14 @@ abstract contract WithWorld is WithStore, System { bytes14 public immutable namespace; error WithWorld_RootNamespaceNotAllowed(); - error WithWorld_NamespaceAlreadyExists(); - error WithWorld_NamespaceDoesNotExists(); - error WithWorld_CallerHasNoNamespaceAccess(); - error WithWorld_CallerIsNotWorld(); + 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(); + revert WithWorld_CallerIsNotWorld(msg.sender); } _; } @@ -31,7 +31,7 @@ abstract contract WithWorld is WithStore, System { modifier onlyNamespace() { // We use WorldContextConsumer directly as we already know the world is the caller if (!_callerIsWorld() || !ResourceAccess.get(getNamespaceId(), WorldContextConsumer._msgSender())) { - revert WithWorld_CallerHasNoNamespaceAccess(); + revert WithWorld_CallerHasNoNamespaceAccess(namespace, _msgSender()); } _; } @@ -49,12 +49,12 @@ abstract contract WithWorld is WithStore, System { if (registerNamespace) { if (namespaceExists) { - revert WithWorld_NamespaceAlreadyExists(); + revert WithWorld_NamespaceAlreadyExists(_namespace); } _world.registerNamespace(namespaceId); } else if (!namespaceExists) { - revert WithWorld_NamespaceDoesNotExists(); + revert WithWorld_NamespaceDoesNotExists(_namespace); } } diff --git a/packages/store-consumer/test/StoreConsumer.t.sol b/packages/store-consumer/test/StoreConsumer.t.sol index 0e891e517f..7be5d35c34 100644 --- a/packages/store-consumer/test/StoreConsumer.t.sol +++ b/packages/store-consumer/test/StoreConsumer.t.sol @@ -128,7 +128,7 @@ contract StoreConsumerTest is Test, GasReporter { address alice = address(0x1234); vm.prank(alice); - vm.expectRevert(WithWorld.WithWorld_CallerIsNotWorld.selector); + vm.expectRevert(abi.encodeWithSelector(WithWorld.WithWorld_CallerIsNotWorld.selector, (alice))); mock.onlyCallableByWorld(); vm.prank(alice); @@ -152,11 +152,11 @@ contract StoreConsumerTest is Test, GasReporter { address alice = address(0x1234); vm.prank(alice); - vm.expectRevert(WithWorld.WithWorld_CallerHasNoNamespaceAccess.selector); + vm.expectRevert(abi.encodeWithSelector(WithWorld.WithWorld_CallerHasNoNamespaceAccess.selector, namespace, alice)); mock.onlyCallableByNamespace(); vm.prank(alice); - vm.expectRevert(WithWorld.WithWorld_CallerHasNoNamespaceAccess.selector); + 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 From 46645e30149652bb92bbd339de71a87733f8a56b Mon Sep 17 00:00:00 2001 From: vdrg Date: Wed, 5 Feb 2025 17:04:11 -0300 Subject: [PATCH 10/10] Update gas report --- packages/world-module-erc20/gas-report.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/world-module-erc20/gas-report.json b/packages/world-module-erc20/gas-report.json index 64f623122a..22c0c3bc75 100644 --- a/packages/world-module-erc20/gas-report.json +++ b/packages/world-module-erc20/gas-report.json @@ -135,7 +135,7 @@ "file": "test/ERC20Module.t.sol:ERC20ModuleTest", "test": "testInstall", "name": "install erc20 module", - "gasUsed": 4729741 + "gasUsed": 4744637 }, { "file": "test/ERC20Pausable.t.sol:ERC20PausableWithInternalStoreTest",