refactor(world): registerRootFunctionSelector takes system signature (#…

…2395) Co-authored-by: Kevin Ingersoll <kingersoll@gmail.com>
latticexyz · Mar 11, 2024 · 5debcca · 5debcca
1 parent adc6822
commit 5debcca
Show file tree

Hide file tree

Showing 12 changed files with 40 additions and 49 deletions.
diff --git a/.changeset/funny-countries-hang.md b/.changeset/funny-countries-hang.md
@@ -0,0 +1,5 @@
+---
+"@latticexyz/world": minor
+---
+
+`registerRootFunctionSelector` now expects a `systemFunctionSignature` instead of a `systemFunctionSelector`. Internally, we compute the selector from the signature. This allows us to track system function signatures that are registered at the root so we can later generate ABIs for these systems.
diff --git a/docs/pages/world/reference/internal/init-module-implementation.mdx b/docs/pages/world/reference/internal/init-module-implementation.mdx
@@ -435,17 +435,17 @@ _Creates a mapping for a root World function without namespace or name prefix_
 function registerRootFunctionSelector(
   ResourceId systemId,
   string memory worldFunctionSignature,
-  bytes4 systemFunctionSelector
+  string memory systemFunctionSignature
 ) public onlyDelegatecall returns (bytes4 worldFunctionSelector);
 ```
 
 **Parameters**
 
-| Name                     | Type         | Description                         |
-| ------------------------ | ------------ | ----------------------------------- |
-| `systemId`               | `ResourceId` | The system ID                       |
-| `worldFunctionSignature` | `string`     | The signature of the World function |
-| `systemFunctionSelector` | `bytes4`     | The selector of the system function |
+| Name                      | Type         | Description                          |
+| ------------------------- | ------------ | ------------------------------------ |
+| `systemId`                | `ResourceId` | The system ID                        |
+| `worldFunctionSignature`  | `string`     | The signature of the World function  |
+| `systemFunctionSignature` | `string`     | The signature of the system function |
 
 **Returns**
 

diff --git a/docs/pages/world/reference/world-external.mdx b/docs/pages/world/reference/world-external.mdx
@@ -357,17 +357,17 @@ _Creates a mapping for a root World function without namespace or name prefix_
 function registerRootFunctionSelector(
   ResourceId systemId,
   string memory worldFunctionSignature,
-  bytes4 systemFunctionSelector
+  string memory systemFunctionSignature
 ) public onlyDelegatecall returns (bytes4 worldFunctionSelector);
 ```
 
 **Parameters**
 
-| Name                     | Type         | Description                         |
-| ------------------------ | ------------ | ----------------------------------- |
-| `systemId`               | `ResourceId` | The system ID                       |
-| `worldFunctionSignature` | `string`     | The signature of the World function |
-| `systemFunctionSelector` | `bytes4`     | The selector of the system function |
+| Name                      | Type         | Description                          |
+| ------------------------- | ------------ | ------------------------------------ |
+| `systemId`                | `ResourceId` | The system ID                        |
+| `worldFunctionSignature`  | `string`     | The signature of the World function  |
+| `systemFunctionSignature` | `string`     | The signature of the system function |
 
 **Returns**
 

diff --git a/packages/cli/src/deploy/ensureFunctions.ts b/packages/cli/src/deploy/ensureFunctions.ts
@@ -50,7 +50,7 @@ export async function ensureFunctions({
               abi: worldAbi,
               // TODO: replace with batchCall (https://github.com/latticexyz/mud/issues/1645)
               functionName: "registerRootFunctionSelector",
-              args: [func.systemId, func.systemFunctionSignature, func.systemFunctionSelector],
+              args: [func.systemId, func.systemFunctionSignature, func.systemFunctionSignature],
             }),
           {
             retries: 3,

diff --git a/packages/world-modules/test/SystemSwitch.t.sol b/packages/world-modules/test/SystemSwitch.t.sol
@@ -125,11 +125,7 @@ contract SystemSwitchTest is Test, GasReporter {
   }
 
   function testCallRootFromRootWorldSelector() public {
-    bytes4 worldFunctionSelector = world.registerRootFunctionSelector(
-      rootSystemBId,
-      "echo(string)",
-      EchoSystem.echo.selector
-    );
+    bytes4 worldFunctionSelector = world.registerRootFunctionSelector(rootSystemBId, "echo(string)", "echo(string)");
     bytes memory callData = abi.encodeWithSelector(worldFunctionSelector, "hello");
 
     vm.prank(caller);
@@ -164,11 +160,7 @@ contract SystemSwitchTest is Test, GasReporter {
   }
 
   function testCallRootFromNonRootWorldSelector() public {
-    bytes4 worldFunctionSelector = world.registerRootFunctionSelector(
-      rootSystemBId,
-      "echo(string)",
-      EchoSystem.echo.selector
-    );
+    bytes4 worldFunctionSelector = world.registerRootFunctionSelector(rootSystemBId, "echo(string)", "echo(string)");
     bytes memory callData = abi.encodeWithSelector(worldFunctionSelector, "hello");
 
     vm.prank(caller);
@@ -203,11 +195,7 @@ contract SystemSwitchTest is Test, GasReporter {
   }
 
   function testCallNonRootFromRootWorldSelector() public {
-    bytes4 worldFunctionSelector = world.registerRootFunctionSelector(
-      systemBId,
-      "echo(string)",
-      EchoSystem.echo.selector
-    );
+    bytes4 worldFunctionSelector = world.registerRootFunctionSelector(systemBId, "echo(string)", "echo(string)");
     bytes memory callData = abi.encodeWithSelector(worldFunctionSelector, "hello");
 
     vm.prank(caller);
@@ -250,11 +238,7 @@ contract SystemSwitchTest is Test, GasReporter {
   }
 
   function testCallNonRootFromNonRootWorldSelector() public {
-    bytes4 worldFunctionSelector = world.registerRootFunctionSelector(
-      systemBId,
-      "echo(string)",
-      EchoSystem.echo.selector
-    );
+    bytes4 worldFunctionSelector = world.registerRootFunctionSelector(systemBId, "echo(string)", "echo(string)");
     bytes memory callData = abi.encodeWithSelector(worldFunctionSelector, "hello");
 
     vm.prank(caller);

diff --git a/packages/world/gas-report.json b/packages/world/gas-report.json
@@ -63,7 +63,7 @@
     "file": "test/Factories.t.sol",
     "test": "testWorldFactoryGas",
     "name": "deploy world via WorldFactory",
-    "gasUsed": 12657699
+    "gasUsed": 12796228
   },
   {
     "file": "test/World.t.sol",
@@ -117,7 +117,7 @@
     "file": "test/World.t.sol",
     "test": "testRegisterRootFunctionSelector",
     "name": "Register a root function selector",
-    "gasUsed": 107427
+    "gasUsed": 113693
   },
   {
     "file": "test/World.t.sol",

diff --git a/packages/world/src/codegen/interfaces/IWorldRegistrationSystem.sol b/packages/world/src/codegen/interfaces/IWorldRegistrationSystem.sol
diff --git a/packages/world/src/modules/init/InitModule.sol b/packages/world/src/modules/init/InitModule.sol
@@ -168,7 +168,7 @@ contract InitModule is Module {
       target: registrationSystem,
       callData: abi.encodeCall(
         WorldRegistrationSystem.registerRootFunctionSelector,
-        (systemId, functionSignature, bytes4(keccak256(bytes(functionSignature))))
+        (systemId, functionSignature, functionSignature)
       )
     });
   }

diff --git a/packages/world/src/modules/init/functionSignatures.sol b/packages/world/src/modules/init/functionSignatures.sol
@@ -53,7 +53,7 @@ function getFunctionSignaturesRegistration() pure returns (string[14] memory) {
     "unregisterSystemHook(bytes32,address)",
     "registerSystem(bytes32,address,bool)",
     "registerFunctionSelector(bytes32,string)",
-    "registerRootFunctionSelector(bytes32,string,bytes4)",
+    "registerRootFunctionSelector(bytes32,string,string)",
     "registerDelegation(address,bytes32,bytes)",
     "unregisterDelegation(address)",
     "registerNamespaceDelegation(bytes32,bytes32,bytes)",

diff --git a/packages/world/src/modules/init/implementations/WorldRegistrationSystem.sol b/packages/world/src/modules/init/implementations/WorldRegistrationSystem.sol
@@ -224,19 +224,20 @@ contract WorldRegistrationSystem is System, IWorldErrors, LimitedCallContext {
    * @dev Creates a mapping for a root World function without namespace or name prefix
    * @param systemId The system ID
    * @param worldFunctionSignature The signature of the World function
-   * @param systemFunctionSelector The selector of the system function
+   * @param systemFunctionSignature The signature of the system function
    * @return worldFunctionSelector The selector of the World function
    */
   function registerRootFunctionSelector(
     ResourceId systemId,
     string memory worldFunctionSignature,
-    bytes4 systemFunctionSelector
+    string memory systemFunctionSignature
   ) public onlyDelegatecall returns (bytes4 worldFunctionSelector) {
     // Require the caller to own the root namespace
     AccessControl.requireOwner(ROOT_NAMESPACE_ID, _msgSender());
 
     // Compute the function selector from the provided signature
     worldFunctionSelector = bytes4(keccak256(bytes(worldFunctionSignature)));
+    bytes4 systemFunctionSelector = bytes4(keccak256(bytes(systemFunctionSignature)));
 
     // Require the function selector to be globally unique
     ResourceId existingSystemId = FunctionSelectors._getSystemId(worldFunctionSelector);
@@ -246,7 +247,8 @@ contract WorldRegistrationSystem is System, IWorldErrors, LimitedCallContext {
     // Register the function selector
     FunctionSelectors._set(worldFunctionSelector, systemId, systemFunctionSelector);
 
-    // Register the function signature for offchain use
+    // Register the function signatures for offchain use
+    FunctionSignatures._set(systemFunctionSelector, systemFunctionSignature);
     FunctionSignatures._set(worldFunctionSelector, worldFunctionSignature);
   }
 

diff --git a/packages/world/test/World.t.sol b/packages/world/test/World.t.sol
@@ -1646,7 +1646,7 @@ contract WorldTest is Test, GasReporter {
     world.registerSystem(systemId, system, true);
 
     string memory worldFunc = "testSelector()";
-    bytes4 sysFunc = WorldTestSystem.msgSender.selector;
+    string memory sysFunc = "msgSender()";
 
     // Expect an error when trying to register a root function selector from an account without access
     _expectAccessDenied(address(0x01), "", "", RESOURCE_NAMESPACE);
@@ -1675,7 +1675,7 @@ contract WorldTest is Test, GasReporter {
     assertEq(abi.decode(data, (address)), address(this), "wrong address returned");
 
     // Register a function selector to the error function
-    functionSelector = world.registerRootFunctionSelector(systemId, "err(string)", WorldTestSystem.err.selector);
+    functionSelector = world.registerRootFunctionSelector(systemId, "err(string)", "err(string)");
 
     // Expect errors to be passed through
     vm.expectRevert(abi.encodeWithSelector(WorldTestSystem.WorldTestSystemError.selector, "test error"));
@@ -1712,7 +1712,7 @@ contract WorldTest is Test, GasReporter {
 
     world.registerNamespace(systemId.getNamespaceId());
     world.registerSystem(systemId, system, true);
-    world.registerRootFunctionSelector(systemId, "receiveEther()", WorldTestSystem.receiveEther.selector);
+    world.registerRootFunctionSelector(systemId, "receiveEther()", "receiveEther()");
 
     // create new funded address and impersonate
     address alice = makeAddr("alice");
@@ -1739,7 +1739,7 @@ contract WorldTest is Test, GasReporter {
     ResourceId systemId = WorldResourceIdLib.encode({ typeId: RESOURCE_SYSTEM, namespace: namespace, name: name });
     world.registerNamespace(systemId.getNamespaceId());
     world.registerSystem(systemId, system, true);
-    world.registerRootFunctionSelector(systemId, "msgSender()", WorldTestSystem.msgSender.selector);
+    world.registerRootFunctionSelector(systemId, "msgSender()", "msgSender()");
 
     // create new funded address and impersonate
     address alice = makeAddr("alice");
@@ -1766,7 +1766,7 @@ contract WorldTest is Test, GasReporter {
     bytes16 name = "testSystem";
     ResourceId systemId = WorldResourceIdLib.encode({ typeId: RESOURCE_SYSTEM, namespace: namespace, name: name });
     world.registerSystem(systemId, system, true);
-    world.registerRootFunctionSelector(systemId, "systemFallback()", bytes4(""));
+    world.registerRootFunctionSelector(systemId, "systemFallback()", "");
 
     // create new funded address and impersonate
     address alice = makeAddr("alice");
@@ -1792,7 +1792,7 @@ contract WorldTest is Test, GasReporter {
     bytes16 name = "testSystem";
     ResourceId systemId = WorldResourceIdLib.encode({ typeId: RESOURCE_SYSTEM, namespace: namespace, name: name });
     world.registerSystem(systemId, system, true);
-    world.registerRootFunctionSelector(systemId, "systemFallback()", bytes4(""));
+    world.registerRootFunctionSelector(systemId, "systemFallback()", "");
 
     // create new funded address and impersonate
     address alice = makeAddr("alice");
@@ -1818,7 +1818,7 @@ contract WorldTest is Test, GasReporter {
     bytes16 name = "testSystem";
     ResourceId systemId = WorldResourceIdLib.encode({ typeId: RESOURCE_SYSTEM, namespace: namespace, name: name });
     world.registerSystem(systemId, system, true);
-    world.registerRootFunctionSelector(systemId, "receiveEther()", WorldTestSystem.receiveEther.selector);
+    world.registerRootFunctionSelector(systemId, "receiveEther()", "receiveEther()");
 
     // create new funded address and impersonate
     address alice = makeAddr("alice");

diff --git a/packages/world/test/WorldBalance.t.sol b/packages/world/test/WorldBalance.t.sol
@@ -43,7 +43,7 @@ contract WorldBalanceTest is Test, GasReporter {
     world.registerSystem(rootSystemId, rootSystem, true);
     world.registerSystem(nonRootSystemId, nonRootSystem, true);
 
-    world.registerRootFunctionSelector(rootSystemId, "echoValue()", rootSystem.echoValue.selector);
+    world.registerRootFunctionSelector(rootSystemId, "echoValue()", "echoValue()");
     world.registerFunctionSelector(nonRootSystemId, "echoValue()");
   }