feat(world): change registerFunctionSelector signature to accept syst…

…em signature as a single string (#1574)

.changeset/real-ducks-hope.md

@@ -0,0 +1,29 @@
+---
+"@latticexyz/cli": major
+"@latticexyz/world": major
+---
+
+The `registerFunctionSelector` function now accepts a single `functionSignature` string paramemer instead of separating function name and function arguments into separate parameters.
+
+```diff
+IBaseWorld {
+  function registerFunctionSelector(
+    ResourceId systemId,
+-   string memory systemFunctionName,
+-   string memory systemFunctionArguments
++   string memory systemFunctionSignature
+  ) external returns (bytes4 worldFunctionSelector);
+}
+```
+
+This is a breaking change if you were manually registering function selectors, e.g. in a `PostDeploy.s.sol` script or a module.
+To upgrade, simply replace the separate `systemFunctionName` and `systemFunctionArguments` parameters with a single `systemFunctionSignature` parameter.
+
+```diff
+  world.registerFunctionSelector(
+    systemId,
+-   systemFunctionName,
+-   systemFunctionArguments,
++   string(abi.encodePacked(systemFunctionName, systemFunctionArguments))
+  );
+```

e2e/packages/contracts/worlds.json

@@ -1,5 +1,5 @@
 {
   "31337": {
-    "address": "0x3Aa5ebB10DC797CAC828524e59A333d0A371443c"
+    "address": "0x4C4a2f8c81640e47606d3fd77B353E87Ba015584"
   }
 }

examples/minimal/packages/contracts/script/PostDeploy.s.sol

@@ -26,7 +26,7 @@ contract PostDeploy is Script {
       name: "ChatNamespaced"
     });
     IWorld(worldAddress).registerSystem(systemId, chatNamespacedSystem, true);
-    IWorld(worldAddress).registerFunctionSelector(systemId, "sendMessage", "(string)");
+    IWorld(worldAddress).registerFunctionSelector(systemId, "sendMessage(string)");
     // Grant this system access to MessageTable
     IWorld(worldAddress).grantAccess(MessageTableTableId, address(chatNamespacedSystem));
 

packages/cli/src/utils/deploy.ts

@@ -171,18 +171,18 @@ export async function deploy(
 
   console.log(chalk.blue("Registering Systems and Functions"));
   const systemCalls = await Promise.all(
-    Object.entries(resolvedConfig.systems).map(([systemName, system]) =>
+    Object.entries(resolvedConfig.systems).map(([systemKey, system]) =>
       getRegisterSystemCallData({
         systemContracts: deployedContracts,
-        systemName,
+        systemKey,
         system,
         namespace: mudConfig.namespace,
       })
     )
   );
-  const functionCalls = Object.entries(resolvedConfig.systems).flatMap(([systemName, system]) =>
+  const functionCalls = Object.entries(resolvedConfig.systems).flatMap(([systemKey, system]) =>
     getRegisterFunctionSelectorsCallData({
-      systemName,
+      systemContractName: systemKey,
       system,
       namespace: mudConfig.namespace,
       forgeOutDirectory,

packages/cli/src/utils/systems/getRegisterFunctionSelectorsCallData.ts

@@ -4,29 +4,28 @@ import { loadFunctionSignatures, toFunctionSelector } from "./utils";
 import { CallData } from "../utils/types";
 
 export function getRegisterFunctionSelectorsCallData(input: {
-  systemName: string;
+  systemContractName: string;
   system: System;
   namespace: string;
   forgeOutDirectory: string;
 }): CallData[] {
   // Register system at route
   const callData: CallData[] = [];
-  const { systemName, namespace, forgeOutDirectory, system } = input;
+  const { systemContractName, namespace, forgeOutDirectory, system } = input;
 
   if (system.registerFunctionSelectors) {
-    const baseSystemFunctionNames = loadFunctionSignatures("System", forgeOutDirectory).map((sig) => sig.functionName);
-    const functionSignatures = loadFunctionSignatures(systemName, forgeOutDirectory).filter(
-      (sig) => systemName === "System" || !baseSystemFunctionNames.includes(sig.functionName)
+    const baseSystemFunctionSignatures = loadFunctionSignatures("System", forgeOutDirectory);
+    const systemFunctionSignatures = loadFunctionSignatures(systemContractName, forgeOutDirectory).filter(
+      (functionSignature) =>
+        systemContractName === "System" || !baseSystemFunctionSignatures.includes(functionSignature)
     );
     const isRoot = namespace === "";
-    for (const { functionName, functionArgs } of functionSignatures) {
+    for (const systemFunctionSignature of systemFunctionSignatures) {
       callData.push(
         getRegisterFunctionSelectorCallData({
           namespace,
           name: system.name,
-          systemName,
-          functionName,
-          functionArgs,
+          systemFunctionSignature,
           isRoot,
         })
       );
@@ -38,31 +37,21 @@ export function getRegisterFunctionSelectorsCallData(input: {
 function getRegisterFunctionSelectorCallData(input: {
   namespace: string;
   name: string;
-  systemName: string;
-  functionName: string;
-  functionArgs: string;
+  systemFunctionSignature: string;
   isRoot: boolean;
 }): CallData {
-  const { namespace, name, systemName, functionName, functionArgs, isRoot } = input;
-  const functionSignature = isRoot
-    ? functionName + functionArgs
-    : `${namespace}_${name}_${functionName}${functionArgs}`;
+  const { namespace, name, systemFunctionSignature, isRoot } = input;
 
   if (isRoot) {
-    const worldFunctionSelector = toFunctionSelector(
-      functionSignature === ""
-        ? { functionName: systemName, functionArgs } // Register the system's fallback function as `<systemName>(<args>)`
-        : { functionName, functionArgs }
-    );
-    const systemFunctionSelector = toFunctionSelector({ functionName, functionArgs });
+    const functionSelector = toFunctionSelector(systemFunctionSignature);
     return {
       func: "registerRootFunctionSelector",
-      args: [resourceIdToHex({ type: "system", namespace, name }), worldFunctionSelector, systemFunctionSelector],
+      args: [resourceIdToHex({ type: "system", namespace, name }), functionSelector, functionSelector],
     };
   } else {
     return {
-      func: "registerRootFunctionSelector",
-      args: [resourceIdToHex({ type: "system", namespace, name }), functionName, functionArgs],
+      func: "registerFunctionSelector",
+      args: [resourceIdToHex({ type: "system", namespace, name }), systemFunctionSignature],
     };
   }
 }

packages/cli/src/utils/systems/getRegisterSystemCallData.ts

@@ -4,12 +4,12 @@ import { CallData } from "../utils/types";
 
 export async function getRegisterSystemCallData(input: {
   systemContracts: Record<string, Promise<string>>;
-  systemName: string;
+  systemKey: string;
   system: System;
   namespace: string;
 }): Promise<CallData> {
-  const { namespace, systemName, systemContracts, system } = input;
-  const systemAddress = await systemContracts[systemName];
+  const { namespace, systemContracts, systemKey, system } = input;
+  const systemAddress = await systemContracts[systemKey];
   return {
     func: "registerSystem",
     args: [resourceIdToHex({ type: "system", namespace, name: system.name }), systemAddress, system.openAccess],

packages/cli/src/utils/systems/types.ts

@@ -7,8 +7,3 @@ export type System = {
 };
 
 export type SystemsConfig = Record<string, System>;
-
-export interface FunctionSignature {
-  functionName: string;
-  functionArgs: string;
-}

packages/cli/src/utils/systems/utils.ts

@@ -1,28 +1,20 @@
 import { ethers } from "ethers";
 import { ParamType } from "ethers/lib/utils.js";
-import { FunctionSignature } from "./types";
 import { getContractData } from "../utils/getContractData";
 
-export function loadFunctionSignatures(contractName: string, forgeOutDirectory: string): FunctionSignature[] {
+export function loadFunctionSignatures(contractName: string, forgeOutDirectory: string): string[] {
   const { abi } = getContractData(contractName, forgeOutDirectory);
 
   return abi
     .filter((item) => ["fallback", "function"].includes(item.type))
     .map((item) => {
-      if (item.type === "fallback") return { functionName: "", functionArgs: "" };
-
-      return {
-        functionName: item.name,
-        functionArgs: parseComponents(item.inputs),
-      };
+      return `${item.name}${parseComponents(item.inputs)}`;
     });
 }
 
 // TODO: move this to utils as soon as utils are usable inside cli
 // (see https://github.com/latticexyz/mud/issues/499)
-export function toFunctionSelector({ functionName, functionArgs }: FunctionSignature): string {
-  const functionSignature = functionName + functionArgs;
-  if (functionSignature === "") return "0x";
+export function toFunctionSelector(functionSignature: string): string {
   return sigHash(functionSignature);
 }
 

packages/world/gas-report.json

@@ -255,7 +255,7 @@
     "file": "test/UniqueEntityModule.t.sol",
     "test": "testInstall",
     "name": "install unique entity module",
-    "gasUsed": 678565
+    "gasUsed": 676985
   },
   {
     "file": "test/UniqueEntityModule.t.sol",
@@ -267,7 +267,7 @@
     "file": "test/UniqueEntityModule.t.sol",
     "test": "testInstallRoot",
     "name": "installRoot unique entity module",
-    "gasUsed": 646138
+    "gasUsed": 644325
   },
   {
     "file": "test/UniqueEntityModule.t.sol",
@@ -305,23 +305,11 @@
     "name": "Push data to the table",
     "gasUsed": 86698
   },
-  {
-    "file": "test/World.t.sol",
-    "test": "testRegisterFallbackSystem",
-    "name": "Register a fallback system",
-    "gasUsed": 58902
-  },
-  {
-    "file": "test/World.t.sol",
-    "test": "testRegisterFallbackSystem",
-    "name": "Register a root fallback system",
-    "gasUsed": 52738
-  },
   {
     "file": "test/World.t.sol",
     "test": "testRegisterFunctionSelector",
     "name": "Register a function selector",
-    "gasUsed": 79468
+    "gasUsed": 77897
   },
   {
     "file": "test/World.t.sol",
@@ -339,7 +327,7 @@
     "file": "test/World.t.sol",
     "test": "testRegisterSystem",
     "name": "register a system",
-    "gasUsed": 165292
+    "gasUsed": 165280
   },
   {
     "file": "test/World.t.sol",

packages/world/src/interfaces/IWorldRegistrationSystem.sol

@@ -18,8 +18,7 @@ interface IWorldRegistrationSystem {
 
   function registerFunctionSelector(
     ResourceId systemId,
-    string memory systemFunctionName,
-    string memory systemFunctionArguments
+    string memory systemFunctionSignature
   ) external returns (bytes4 worldFunctionSelector);
 
   function registerRootFunctionSelector(

packages/world/src/modules/core/implementations/WorldRegistrationSystem.sol

@@ -148,15 +148,10 @@ contract WorldRegistrationSystem is System, IWorldErrors {
 
   /**
    * Register a World function selector for the given namespace, name and system function.
-   * TODO: instead of mapping to a resource, the function selector could map direcly to a system function,
-   * which would save one sload per call, but add some complexity to upgrading systems. TBD.
-   * (see https://github.com/latticexyz/mud/issues/444)
-   * TODO: replace separate systemFunctionName and systemFunctionArguments with a signature argument
    */
   function registerFunctionSelector(
     ResourceId systemId,
-    string memory systemFunctionName,
-    string memory systemFunctionArguments
+    string memory systemFunctionSignature
   ) public returns (bytes4 worldFunctionSelector) {
     // Require the caller to own the namespace
     AccessControl.requireOwner(systemId, _msgSender());
@@ -165,7 +160,7 @@ contract WorldRegistrationSystem is System, IWorldErrors {
     string memory namespaceString = WorldResourceIdLib.toTrimmedString(systemId.getNamespace());
     string memory nameString = WorldResourceIdLib.toTrimmedString(systemId.getName());
     worldFunctionSelector = bytes4(
-      keccak256(abi.encodePacked(namespaceString, "_", nameString, "_", systemFunctionName, systemFunctionArguments))
+      keccak256(abi.encodePacked(namespaceString, "_", nameString, "_", systemFunctionSignature))
     );
 
     // Require the function selector to be globally unique
@@ -174,19 +169,13 @@ contract WorldRegistrationSystem is System, IWorldErrors {
     if (existingSystemId != 0) revert World_FunctionSelectorAlreadyExists(worldFunctionSelector);
 
     // Register the function selector
-    bytes memory systemFunctionSignature = abi.encodePacked(systemFunctionName, systemFunctionArguments);
-    bytes4 systemFunctionSelector = systemFunctionSignature.length == 0
-      ? bytes4(0) // Save gas by storing 0x0 for empty function signatures (= fallback function)
-      : bytes4(keccak256(systemFunctionSignature));
+    bytes4 systemFunctionSelector = bytes4(keccak256(bytes(systemFunctionSignature)));
     FunctionSelectors._set(worldFunctionSelector, ResourceId.unwrap(systemId), systemFunctionSelector);
   }
 
   /**
    * Register a root World function selector (without namespace / name prefix).
    * Requires the caller to own the root namespace.
-   * TODO: instead of mapping to a resource, the function selector could map direcly to a system function,
-   * which would save one sload per call, but add some complexity to upgrading systems. TBD.
-   * (see https://github.com/latticexyz/mud/issues/444)
    */
   function registerRootFunctionSelector(
     ResourceId systemId,

packages/world/src/modules/uniqueentity/UniqueEntityModule.sol

@@ -39,7 +39,7 @@ contract UniqueEntityModule is Module {
 
     // Register system's functions
     (success, data) = address(world).delegatecall(
-      abi.encodeCall(world.registerFunctionSelector, (SYSTEM_ID, "getUniqueEntity", "()"))
+      abi.encodeCall(world.registerFunctionSelector, (SYSTEM_ID, "getUniqueEntity()"))
     );
     if (!success) revertWithBytes(data);
   }
@@ -54,6 +54,6 @@ contract UniqueEntityModule is Module {
     world.registerSystem(SYSTEM_ID, uniqueEntitySystem, true);
 
     // Register system's functions
-    world.registerFunctionSelector(SYSTEM_ID, "getUniqueEntity", "()");
+    world.registerFunctionSelector(SYSTEM_ID, "getUniqueEntity()");
   }
 }

packages/world/test/World.t.sol

@@ -1362,7 +1362,7 @@ contract WorldTest is Test, GasReporter {
     world.registerSystem(systemId, system, true);
 
     startGasReport("Register a function selector");
-    bytes4 functionSelector = world.registerFunctionSelector(systemId, "msgSender", "()");
+    bytes4 functionSelector = world.registerFunctionSelector(systemId, "msgSender()");
     endGasReport();
 
     string memory expectedWorldFunctionSignature = "testNamespace_testSystem_msgSender()";
@@ -1376,7 +1376,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.registerFunctionSelector(systemId, "err", "(string)");
+    functionSelector = world.registerFunctionSelector(systemId, "err(string)");
 
     // Expect errors to be passed through
     vm.expectRevert(abi.encodeWithSelector(WorldTestSystem.WorldTestSystemError.selector, "test error"));
@@ -1433,40 +1433,6 @@ contract WorldTest is Test, GasReporter {
     WorldTestSystem(address(world)).err("test error");
   }
 
-  function testRegisterFallbackSystem() public {
-    bytes14 namespace = "testNamespace";
-    bytes16 name = "testSystem";
-    ResourceId systemId = WorldResourceIdLib.encode({ typeId: RESOURCE_SYSTEM, namespace: namespace, name: name });
-
-    // Register a new system
-    WorldTestSystem system = new WorldTestSystem();
-    world.registerSystem(systemId, system, true);
-
-    startGasReport("Register a fallback system");
-    bytes4 funcSelector1 = world.registerFunctionSelector(systemId, "", "");
-    endGasReport();
-
-    // Call the system's fallback function
-    vm.expectEmit(true, true, true, true);
-    emit WorldTestSystemLog("fallback");
-    (bool success, bytes memory data) = address(world).call(abi.encodeWithSelector(funcSelector1));
-    assertTrue(success, "call failed");
-
-    bytes4 worldFunc = bytes4(abi.encodeWithSignature("testSelector()"));
-
-    startGasReport("Register a root fallback system");
-    bytes4 funcSelector2 = world.registerRootFunctionSelector(systemId, worldFunc, 0);
-    endGasReport();
-
-    assertEq(funcSelector2, worldFunc, "wrong function selector returned");
-
-    // Call the system's fallback function
-    vm.expectEmit(true, true, true, true);
-    emit WorldTestSystemLog("fallback");
-    (success, data) = address(world).call(abi.encodeWithSelector(worldFunc));
-    assertTrue(success, "call failed");
-  }
-
   function testPayable() public {
     address alice = makeAddr("alice");
     startHoax(alice, 1 ether);

packages/world/test/WorldBalance.t.sol

@@ -40,7 +40,7 @@ contract WorldBalanceTest is Test, GasReporter {
     world.registerSystem(nonRootSystemId, nonRootSystem, true);
 
     world.registerRootFunctionSelector(rootSystemId, rootSystem.echoValue.selector, rootSystem.echoValue.selector);
-    world.registerFunctionSelector(nonRootSystemId, "echoValue", "()");
+    world.registerFunctionSelector(nonRootSystemId, "echoValue()");
   }
 
   function testCallWithValue() public {