refactor(world): add default-reverting methods to Module (#3581)

Author: frolic

.changeset/clean-apricots-hide.md

@@ -0,0 +1,8 @@
+---
+"@latticexyz/world-module-callwithsignature": patch
+"@latticexyz/world-module-erc20": patch
+"@latticexyz/world-module-metadata": patch
+"@latticexyz/world-modules": patch
+---
+
+Removed unsupported install methods as these now automatically revert in the base `Module` contract.

.changeset/gorgeous-falcons-roll.md

@@ -0,0 +1,17 @@
+---
+"@latticexyz/world": patch
+---
+
+The base `Module` contract now includes default implementations of `install` and `installRoot` that immediately revert, avoiding the need to implement these manually in each module.
+
+If you've written a module, you may need to update your install methods with `override` when using this new base contract.
+
+```diff
+-function install(bytes memory) public {
++function install(bytes memory) public override {
+```
+
+```diff
+-function installRoot(bytes memory) public {
++function installRoot(bytes memory) public override {
+```

docs/pages/guides/modules.mdx

@@ -132,11 +132,7 @@ contract TimerModule is Module {
   ResourceId private immutable namespaceResource = WorldResourceIdLib.encodeNamespace(bytes14("timer"));
   ResourceId private immutable timerSystemResource = WorldResourceIdLib.encode(RESOURCE_SYSTEM, "timer", "TimerSystem");
 
-  function installRoot(bytes memory) public pure override {
-    revert Module_RootInstallNotSupported();
-  }
-
-  function install(bytes memory) public {
+  function install(bytes memory) public override {
     IWorld world = IWorld(_world());
 
     // Register the namespace.
@@ -210,15 +206,7 @@ We need these identifiers to register the namespace and `System`s.
 There is no point wasting gas on recalculating these values either, so it's best to do it once.
 
 ```solidity
-function installRoot(bytes memory) public pure override {
-  revert Module_RootInstallNotSupported();
-}
-```
-
-We do _not_ need root privileges, so it's best if we [don't have them](https://en.wikipedia.org/wiki/Principle_of_least_privilege).
-
-```solidity
-  function install(bytes memory) public {
+  function install(bytes memory) public override {
     IWorld world = IWorld(_world());
 
     // Register the namespace.

docs/pages/world/reference/internal/init-module.mdx

@@ -60,16 +60,6 @@ _Registers core tables, systems, and function selectors in the World._
 function installRoot(bytes memory) public override;
 ```
 
-#### install
-
-Non-root installation of the module.
-
-_Installation is only supported at root level, so this function will always revert._
-
-```solidity
-function install(bytes memory) public pure;
-```
-
 #### \_registerTables
 
 Register World's tables.

docs/pages/world/reference/module.mdx

@@ -78,3 +78,38 @@ function requireNotInstalled(address moduleAddress, bytes memory encodedArgs) in
 | --------------- | --------- | ------------------------------------------------------ |
 | `moduleAddress` | `address` | The address of the module.                             |
 | `encodedArgs`   | `bytes`   | The ABI encoded arguments for the module installation. |
+
+#### installRoot
+
+Installs the module as a root module.
+
+_This function is invoked by the World contract during `installRootModule` process.
+The module expects to be called via the World contract and thus installs itself on the `msg.sender`._
+
+```solidity
+function installRoot(bytes memory encodedArgs) public virtual;
+```
+
+**Parameters**
+
+| Name          | Type    | Description                                                                   |
+| ------------- | ------- | ----------------------------------------------------------------------------- |
+| `encodedArgs` | `bytes` | The ABI encoded arguments that may be needed during the installation process. |
+
+#### install
+
+Installs the module.
+
+_This function is invoked by the World contract during `installModule` process.
+The module expects to be called via the World contract and thus installs itself on the `msg.sender`.
+Logic might differ from `installRoot`, for example, this might accept namespace parameters._
+
+```solidity
+function install(bytes memory encodedArgs) public virtual;
+```
+
+**Parameters**
+
+| Name          | Type    | Description                                                                   |
+| ------------- | ------- | ----------------------------------------------------------------------------- |
+| `encodedArgs` | `bytes` | The ABI encoded arguments that may be needed during the installation process. |

packages/world-module-callwithsignature/src/CallWithSignatureModule.sol

@@ -14,7 +14,7 @@ import { DELEGATION_SYSTEM_ID } from "./constants.sol";
 contract CallWithSignatureModule is Module {
   CallWithSignatureSystem private immutable callWithSignatureSystem = new CallWithSignatureSystem();
 
-  function installRoot(bytes memory encodedArgs) public {
+  function installRoot(bytes memory encodedArgs) public override {
     requireNotInstalled(__self, encodedArgs);
 
     IBaseWorld world = IBaseWorld(_world());
@@ -41,8 +41,4 @@ contract CallWithSignatureModule is Module {
     );
     if (!success) revertWithBytes(data);
   }
-
-  function install(bytes memory) public pure {
-    revert Module_NonRootInstallNotSupported();
-  }
 }

packages/world-module-erc20/src/experimental/ERC20Module.sol

@@ -18,7 +18,7 @@ contract ERC20Module is Module {
   error ERC20Module_InvalidNamespace(bytes14 namespace);
   error ERC20Module_NamespaceAlreadyExists(bytes14 namespace);
 
-  function install(bytes memory encodedArgs) public {
+  function install(bytes memory encodedArgs) public override {
     // TODO: we should probably check just for namespace, not for all args
     requireNotInstalled(__self, encodedArgs);
 
@@ -52,10 +52,6 @@ contract ERC20Module is Module {
 
     ERC20RegistryLib.register(world, namespaceId, address(token));
   }
-
-  function installRoot(bytes memory) public pure {
-    revert Module_RootInstallNotSupported();
-  }
 }
 
 library ERC20RegistryLib {

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

@@ -22,11 +22,7 @@ contract MetadataModule is Module {
 
   MetadataSystem private immutable metadataSystem = new MetadataSystem();
 
-  function installRoot(bytes memory) public pure {
-    revert Module_RootInstallNotSupported();
-  }
-
-  function install(bytes memory args) public {
+  function install(bytes memory args) public override {
     // naive check to ensure this is only installed once
     // TODO: update this + deployer to be idempotent
     requireNotInstalled(__self, args);

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

@@ -24,7 +24,7 @@ import { ERC20Metadata, ERC20MetadataData } from "./tables/ERC20Metadata.sol";
 contract ERC20Module is Module {
   error ERC20Module_InvalidNamespace(bytes14 namespace);
 
-  function install(bytes memory encodedArgs) public {
+  function install(bytes memory encodedArgs) public override {
     // Require the module to not be installed with these args yet
     requireNotInstalled(__self, encodedArgs);
 
@@ -58,10 +58,6 @@ contract ERC20Module is Module {
     }
     ERC20Registry.set(ERC20_REGISTRY_TABLE_ID, namespaceId, puppet);
   }
-
-  function installRoot(bytes memory) public pure {
-    revert Module_RootInstallNotSupported();
-  }
 }
 
 library ERC20ModuleRegistrationLib {

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

@@ -27,7 +27,7 @@ import { ERC721Metadata, ERC721MetadataData } from "./tables/ERC721Metadata.sol"
 contract ERC721Module is Module {
   error ERC721Module_InvalidNamespace(bytes14 namespace);
 
-  function install(bytes memory encodedArgs) public {
+  function install(bytes memory encodedArgs) public override {
     // Require the module to not be installed with these args yet
     requireNotInstalled(__self, encodedArgs);
 
@@ -61,10 +61,6 @@ contract ERC721Module is Module {
     }
     ERC721Registry.set(ERC721_REGISTRY_TABLE_ID, namespaceId, puppet);
   }
-
-  function installRoot(bytes memory) public pure {
-    revert Module_RootInstallNotSupported();
-  }
 }
 
 library ERC721ModuleRegistrationLib {