fix(world): check table exists for register store and system hook [L-…

…09] (#2195)
latticexyz · Jan 29, 2024 · 745485c · 745485c
1 parent e2d089c
commit 745485c
Show file tree

Hide file tree

Showing 9 changed files with 83 additions and 11 deletions.
diff --git a/.changeset/chilly-fishes-speak.md b/.changeset/chilly-fishes-speak.md
@@ -0,0 +1,5 @@
+---
+"@latticexyz/world": patch
+---
+
+Updated `WorldRegistrationSystem` to check that systems exist before registering system hooks.
diff --git a/.changeset/warm-colts-sleep.md b/.changeset/warm-colts-sleep.md
@@ -0,0 +1,5 @@
+---
+"@latticexyz/store": patch
+---
+
+Updated `StoreCore` to check that tables exist before registering store hooks.
diff --git a/packages/store/gas-report.json b/packages/store/gas-report.json
@@ -705,7 +705,7 @@
     "file": "test/StoreCoreGas.t.sol",
     "test": "testHooks",
     "name": "register subscriber",
-    "gasUsed": 57945
+    "gasUsed": 59184
   },
   {
     "file": "test/StoreCoreGas.t.sol",
@@ -729,7 +729,7 @@
     "file": "test/StoreCoreGas.t.sol",
     "test": "testHooksDynamicData",
     "name": "register subscriber",
-    "gasUsed": 57945
+    "gasUsed": 59184
   },
   {
     "file": "test/StoreCoreGas.t.sol",

diff --git a/packages/store/src/StoreCore.sol b/packages/store/src/StoreCore.sol
@@ -259,6 +259,11 @@ library StoreCore {
       revert IStoreErrors.Store_InvalidResourceType(RESOURCE_TABLE, tableId, string(abi.encodePacked(tableId)));
     }
 
+    // Require the table to exist
+    if (!ResourceIds._getExists(tableId)) {
+      revert IStoreErrors.Store_TableNotFound(tableId, string(abi.encodePacked(tableId)));
+    }
+
     StoreHooks.push(tableId, Hook.unwrap(HookLib.encode(address(hookAddress), enabledHooksBitmap)));
   }
 

diff --git a/packages/store/test/StoreCore.t.sol b/packages/store/test/StoreCore.t.sol
@@ -1131,6 +1131,32 @@ contract StoreCoreTest is Test, StoreMock {
     assertEq(keccak256(indexedData), keccak256(abi.encodePacked(bytes16(0))));
   }
 
+  function testRegisterHookBeforeTable() public {
+    ResourceId tableId = _tableId;
+
+    bytes32[] memory keyTuple = new bytes32[](1);
+    keyTuple[0] = "some key";
+
+    // Register table
+    FieldLayout fieldLayout = FieldLayoutEncodeHelper.encode(16, 0);
+    Schema valueSchema = SchemaEncodeHelper.encode(SchemaType.UINT128);
+
+    // Create subscriber
+    MirrorSubscriber subscriber = new MirrorSubscriber(
+      tableId,
+      fieldLayout,
+      defaultKeySchema,
+      valueSchema,
+      new string[](1),
+      new string[](1)
+    );
+
+    vm.expectRevert(
+      abi.encodeWithSelector(IStoreErrors.Store_TableNotFound.selector, tableId, string(abi.encodePacked(tableId)))
+    );
+    IStore(this).registerStoreHook(tableId, subscriber, BEFORE_ALL);
+  }
+
   function testUnregisterHook() public {
     ResourceId tableId = _tableId;
 

diff --git a/packages/world-modules/gas-report.json b/packages/world-modules/gas-report.json
@@ -75,13 +75,13 @@
     "file": "test/KeysInTableModule.t.sol",
     "test": "testInstallComposite",
     "name": "install keys in table module",
-    "gasUsed": 1438512
+    "gasUsed": 1439780
   },
   {
     "file": "test/KeysInTableModule.t.sol",
     "test": "testInstallGas",
     "name": "install keys in table module",
-    "gasUsed": 1438512
+    "gasUsed": 1439780
   },
   {
     "file": "test/KeysInTableModule.t.sol",
@@ -93,13 +93,13 @@
     "file": "test/KeysInTableModule.t.sol",
     "test": "testInstallSingleton",
     "name": "install keys in table module",
-    "gasUsed": 1438512
+    "gasUsed": 1439780
   },
   {
     "file": "test/KeysInTableModule.t.sol",
     "test": "testSetAndDeleteRecordHookCompositeGas",
     "name": "install keys in table module",
-    "gasUsed": 1438512
+    "gasUsed": 1439780
   },
   {
     "file": "test/KeysInTableModule.t.sol",
@@ -117,7 +117,7 @@
     "file": "test/KeysInTableModule.t.sol",
     "test": "testSetAndDeleteRecordHookGas",
     "name": "install keys in table module",
-    "gasUsed": 1438512
+    "gasUsed": 1439780
   },
   {
     "file": "test/KeysInTableModule.t.sol",
@@ -135,7 +135,7 @@
     "file": "test/KeysWithValueModule.t.sol",
     "test": "testGetKeysWithValueGas",
     "name": "install keys with value module",
-    "gasUsed": 687430
+    "gasUsed": 688698
   },
   {
     "file": "test/KeysWithValueModule.t.sol",
@@ -153,7 +153,7 @@
     "file": "test/KeysWithValueModule.t.sol",
     "test": "testInstall",
     "name": "install keys with value module",
-    "gasUsed": 687430
+    "gasUsed": 688698
   },
   {
     "file": "test/KeysWithValueModule.t.sol",
@@ -165,7 +165,7 @@
     "file": "test/KeysWithValueModule.t.sol",
     "test": "testSetAndDeleteRecordHook",
     "name": "install keys with value module",
-    "gasUsed": 687430
+    "gasUsed": 688698
   },
   {
     "file": "test/KeysWithValueModule.t.sol",
@@ -183,7 +183,7 @@
     "file": "test/KeysWithValueModule.t.sol",
     "test": "testSetField",
     "name": "install keys with value module",
-    "gasUsed": 687430
+    "gasUsed": 688698
   },
   {
     "file": "test/KeysWithValueModule.t.sol",

diff --git a/packages/world-modules/test/KeysWithValueModule.t.sol b/packages/world-modules/test/KeysWithValueModule.t.sol
@@ -105,6 +105,16 @@ contract KeysWithValueModuleTest is Test, GasReporter {
   }
 
   function testInstallTwice() public {
+    // Register source table
+    world.registerTable(
+      sourceTableId,
+      sourceTableFieldLayout,
+      sourceTableKeySchema,
+      sourceTableSchema,
+      new string[](1),
+      new string[](1)
+    );
+
     world.installRootModule(keysWithValueModule, abi.encode(sourceTableId));
     vm.expectRevert(IModule.Module_AlreadyInstalled.selector);
     world.installRootModule(keysWithValueModule, abi.encode(sourceTableId));

diff --git a/packages/world/src/modules/core/implementations/WorldRegistrationSystem.sol b/packages/world/src/modules/core/implementations/WorldRegistrationSystem.sol
@@ -82,6 +82,9 @@ contract WorldRegistrationSystem is System, IWorldErrors, LimitedCallContext {
     // Require the provided address to implement the ISystemHook interface
     requireInterface(address(hookAddress), type(ISystemHook).interfaceId);
 
+    // Require the system to exist
+    AccessControl.requireExistence(systemId);
+
     // Require the system's namespace to exist
     AccessControl.requireExistence(systemId.getNamespaceId());
 

diff --git a/packages/world/test/World.t.sol b/packages/world/test/World.t.sol
@@ -1449,6 +1449,24 @@ contract WorldTest is Test, GasReporter {
     world.call(systemId, callData);
   }
 
+  function testRegisterSystemHookBeforeSystem() public {
+    ResourceId systemId = WorldResourceIdLib.encode({
+      typeId: RESOURCE_SYSTEM,
+      namespace: "namespace",
+      name: "testSystem"
+    });
+
+    // Register a new system
+    world.registerNamespace(systemId.getNamespaceId());
+
+    // Register a new hook
+    ISystemHook systemHook = new EchoSystemHook();
+    vm.expectRevert(
+      abi.encodeWithSelector(IWorldErrors.World_ResourceNotFound.selector, systemId, systemId.toString())
+    );
+    world.registerSystemHook(systemId, systemHook, BEFORE_CALL_SYSTEM | AFTER_CALL_SYSTEM);
+  }
+
   function testUnregisterSystemHook() public {
     ResourceId systemId = WorldResourceIdLib.encode({
       typeId: RESOURCE_SYSTEM,