feat(world-modules): only install modules once (#1756)

latticexyz · Oct 13, 2023 · 6ca1874 · 6ca1874
1 parent 1cc2c50
commit 6ca1874
Show file tree

Hide file tree

Showing 11 changed files with 83 additions and 16 deletions.
diff --git a/.changeset/cyan-baboons-breathe.md b/.changeset/cyan-baboons-breathe.md
@@ -0,0 +1,5 @@
+---
+"@latticexyz/world": minor
+---
+
+Added a `Module_AlreadyInstalled` error to `IModule`.
diff --git a/.changeset/rude-cycles-travel.md b/.changeset/rude-cycles-travel.md
@@ -0,0 +1,7 @@
+---
+"@latticexyz/world-modules": major
+---
+
+Modules now revert with `Module_AlreadyInstalled` if attempting to install more than once with the same calldata.
+
+This is a temporary workaround for our deploy pipeline. We'll make these install steps more idempotent in the future.
diff --git a/package.json b/package.json
@@ -13,7 +13,7 @@
     "dev": "turbo run dev --concurrency 100",
     "dist-tag-rm": "pnpm recursive exec -- sh -c 'npm dist-tag rm $(cat package.json | jq -r \".name\") $TAG || true'",
     "foundryup": "curl -L https://foundry.paradigm.xyz | bash && bash ~/.foundry/bin/foundryup",
-    "gas-report": "pnpm recursive run gas-report",
+    "gas-report": "pnpm run --recursive --parallel gas-report",
     "lint": "pnpm prettier:check && eslint . --ext .ts --ext .tsx",
     "prepare": "husky install && (forge --version || pnpm foundryup)",
     "prettier": "prettier --write '**/*.{ts,tsx,css,md,mdx,sol}'",

diff --git a/packages/world-modules/gas-report.json b/packages/world-modules/gas-report.json
@@ -3,13 +3,13 @@
     "file": "test/KeysInTableModule.t.sol",
     "test": "testInstallComposite",
     "name": "install keys in table module",
-    "gasUsed": 1407698
+    "gasUsed": 1409435
   },
   {
     "file": "test/KeysInTableModule.t.sol",
     "test": "testInstallGas",
     "name": "install keys in table module",
-    "gasUsed": 1407698
+    "gasUsed": 1409435
   },
   {
     "file": "test/KeysInTableModule.t.sol",
@@ -21,13 +21,13 @@
     "file": "test/KeysInTableModule.t.sol",
     "test": "testInstallSingleton",
     "name": "install keys in table module",
-    "gasUsed": 1407698
+    "gasUsed": 1409435
   },
   {
     "file": "test/KeysInTableModule.t.sol",
     "test": "testSetAndDeleteRecordHookCompositeGas",
     "name": "install keys in table module",
-    "gasUsed": 1407698
+    "gasUsed": 1409435
   },
   {
     "file": "test/KeysInTableModule.t.sol",
@@ -45,7 +45,7 @@
     "file": "test/KeysInTableModule.t.sol",
     "test": "testSetAndDeleteRecordHookGas",
     "name": "install keys in table module",
-    "gasUsed": 1407698
+    "gasUsed": 1409435
   },
   {
     "file": "test/KeysInTableModule.t.sol",
@@ -63,7 +63,7 @@
     "file": "test/KeysWithValueModule.t.sol",
     "test": "testGetKeysWithValueGas",
     "name": "install keys with value module",
-    "gasUsed": 648994
+    "gasUsed": 650708
   },
   {
     "file": "test/KeysWithValueModule.t.sol",
@@ -81,7 +81,7 @@
     "file": "test/KeysWithValueModule.t.sol",
     "test": "testInstall",
     "name": "install keys with value module",
-    "gasUsed": 648994
+    "gasUsed": 650708
   },
   {
     "file": "test/KeysWithValueModule.t.sol",
@@ -93,7 +93,7 @@
     "file": "test/KeysWithValueModule.t.sol",
     "test": "testSetAndDeleteRecordHook",
     "name": "install keys with value module",
-    "gasUsed": 648994
+    "gasUsed": 650708
   },
   {
     "file": "test/KeysWithValueModule.t.sol",
@@ -111,7 +111,7 @@
     "file": "test/KeysWithValueModule.t.sol",
     "test": "testSetField",
     "name": "install keys with value module",
-    "gasUsed": 648994
+    "gasUsed": 650708
   },
   {
     "file": "test/KeysWithValueModule.t.sol",
@@ -219,7 +219,7 @@
     "file": "test/UniqueEntityModule.t.sol",
     "test": "testInstall",
     "name": "install unique entity module",
-    "gasUsed": 675275
+    "gasUsed": 678996
   },
   {
     "file": "test/UniqueEntityModule.t.sol",
@@ -231,7 +231,7 @@
     "file": "test/UniqueEntityModule.t.sol",
     "test": "testInstallRoot",
     "name": "installRoot unique entity module",
-    "gasUsed": 642601
+    "gasUsed": 644433
   },
   {
     "file": "test/UniqueEntityModule.t.sol",

diff --git a/packages/world-modules/src/modules/keysintable/KeysInTableModule.sol b/packages/world-modules/src/modules/keysintable/KeysInTableModule.sol
@@ -7,6 +7,7 @@ import { ResourceIds } from "@latticexyz/store/src/codegen/tables/ResourceIds.so
 import { Module } from "@latticexyz/world/src/Module.sol";
 
 import { IBaseWorld } from "@latticexyz/world/src/codegen/interfaces/IBaseWorld.sol";
+import { InstalledModules } from "@latticexyz/world/src/codegen/index.sol";
 
 import { ResourceId, WorldResourceIdInstance } from "@latticexyz/world/src/WorldResourceId.sol";
 import { revertWithBytes } from "@latticexyz/world/src/revertWithBytes.sol";
@@ -37,6 +38,12 @@ contract KeysInTableModule is Module {
   }
 
   function installRoot(bytes memory args) public override {
+    // Naive check to ensure this is only installed once
+    // TODO: only revert if there's nothing to do
+    if (InstalledModules.getModuleAddress(getName(), keccak256(args)) != address(0)) {
+      revert Module_AlreadyInstalled();
+    }
+
     // Extract source table id from args
     ResourceId sourceTableId = ResourceId.wrap(abi.decode(args, (bytes32)));
 

diff --git a/packages/world-modules/src/modules/keyswithvalue/KeysWithValueModule.sol b/packages/world-modules/src/modules/keyswithvalue/KeysWithValueModule.sol
@@ -5,6 +5,7 @@ import { BEFORE_SET_RECORD, BEFORE_SPLICE_STATIC_DATA, AFTER_SPLICE_STATIC_DATA,
 import { Module } from "@latticexyz/world/src/Module.sol";
 
 import { IBaseWorld } from "@latticexyz/world/src/codegen/interfaces/IBaseWorld.sol";
+import { InstalledModules } from "@latticexyz/world/src/codegen/index.sol";
 
 import { WorldContextConsumer } from "@latticexyz/world/src/WorldContext.sol";
 import { ResourceId, WorldResourceIdInstance } from "@latticexyz/world/src/WorldResourceId.sol";
@@ -38,6 +39,12 @@ contract KeysWithValueModule is Module {
   }
 
   function installRoot(bytes memory args) public {
+    // Naive check to ensure this is only installed once
+    // TODO: only revert if there's nothing to do
+    if (InstalledModules.getModuleAddress(getName(), keccak256(args)) != address(0)) {
+      revert Module_AlreadyInstalled();
+    }
+
     // Extract source table id from args
     ResourceId sourceTableId = ResourceId.wrap(abi.decode(args, (bytes32)));
     ResourceId targetTableSelector = getTargetTableId(MODULE_NAMESPACE, sourceTableId);

diff --git a/packages/world-modules/src/modules/uniqueentity/UniqueEntityModule.sol b/packages/world-modules/src/modules/uniqueentity/UniqueEntityModule.sol
@@ -2,6 +2,7 @@
 pragma solidity >=0.8.21;
 
 import { IBaseWorld } from "@latticexyz/world/src/codegen/interfaces/IBaseWorld.sol";
+import { InstalledModules } from "@latticexyz/world/src/codegen/index.sol";
 
 import { Module } from "@latticexyz/world/src/Module.sol";
 import { WorldContextConsumer } from "@latticexyz/world/src/WorldContext.sol";
@@ -25,7 +26,13 @@ contract UniqueEntityModule is Module {
     return MODULE_NAME;
   }
 
-  function installRoot(bytes memory) public {
+  function installRoot(bytes memory args) public {
+    // Naive check to ensure this is only installed once
+    // TODO: only revert if there's nothing to do
+    if (InstalledModules.getModuleAddress(getName(), keccak256(args)) != address(0)) {
+      revert Module_AlreadyInstalled();
+    }
+
     IBaseWorld world = IBaseWorld(_world());
 
     // Register table
@@ -44,11 +51,17 @@ contract UniqueEntityModule is Module {
     if (!success) revertWithBytes(data);
   }
 
-  function install(bytes memory) public {
+  function install(bytes memory args) public {
+    // Naive check to ensure this is only installed once
+    // TODO: only revert if there's nothing to do
+    if (InstalledModules.getModuleAddress(getName(), keccak256(args)) != address(0)) {
+      revert Module_AlreadyInstalled();
+    }
+
     IBaseWorld world = IBaseWorld(_world());
 
     // Register table
-    UniqueEntity.register(world, TABLE_ID);
+    UniqueEntity.register(TABLE_ID);
 
     // Register system
     world.registerSystem(SYSTEM_ID, uniqueEntitySystem, true);

diff --git a/packages/world-modules/test/KeysInTableModule.t.sol b/packages/world-modules/test/KeysInTableModule.t.sol
@@ -12,6 +12,7 @@ import { SchemaEncodeHelper } from "@latticexyz/store/test/SchemaEncodeHelper.so
 import { SchemaType } from "@latticexyz/schema-type/src/solidity/SchemaType.sol";
 
 import { World } from "@latticexyz/world/src/World.sol";
+import { IModule } from "@latticexyz/world/src/IModule.sol";
 import { IBaseWorld } from "@latticexyz/world/src/codegen/interfaces/IBaseWorld.sol";
 import { ResourceId, WorldResourceIdLib, WorldResourceIdInstance } from "@latticexyz/world/src/WorldResourceId.sol";
 import { ROOT_NAMESPACE } from "@latticexyz/world/src/constants.sol";
@@ -161,7 +162,13 @@ contract KeysInTableModuleTest is Test, GasReporter {
     assertEq(keysInTable[0][0], key1);
   }
 
-  function testInstallTwice(bytes32 keyA, bytes32 keyB, uint256 value1, uint256 value2) public {
+  function testInstallTwice() public {
+    world.installRootModule(keysInTableModule, abi.encode(tableId));
+    vm.expectRevert(IModule.Module_AlreadyInstalled.selector);
+    world.installRootModule(keysInTableModule, abi.encode(tableId));
+  }
+
+  function testInstallMultiple(bytes32 keyA, bytes32 keyB, uint256 value1, uint256 value2) public {
     _installKeysInTableModule();
 
     bytes32[] memory keyTuple = new bytes32[](1);

diff --git a/packages/world-modules/test/KeysWithValueModule.t.sol b/packages/world-modules/test/KeysWithValueModule.t.sol
@@ -15,6 +15,7 @@ import { FieldLayout } from "@latticexyz/store/src/FieldLayout.sol";
 import { FieldLayoutEncodeHelper } from "@latticexyz/store/test/FieldLayoutEncodeHelper.sol";
 
 import { World } from "@latticexyz/world/src/World.sol";
+import { IModule } from "@latticexyz/world/src/IModule.sol";
 import { IBaseWorld } from "@latticexyz/world/src/codegen/interfaces/IBaseWorld.sol";
 import { WorldResourceIdLib, WorldResourceIdInstance, NAME_BITS, TYPE_BITS } from "@latticexyz/world/src/WorldResourceId.sol";
 import { ROOT_NAMESPACE } from "@latticexyz/world/src/constants.sol";
@@ -104,6 +105,12 @@ contract KeysWithValueModuleTest is Test, GasReporter {
     assertEq(keysWithValue[0], key1);
   }
 
+  function testInstallTwice() public {
+    world.installRootModule(keysWithValueModule, abi.encode(sourceTableId));
+    vm.expectRevert(IModule.Module_AlreadyInstalled.selector);
+    world.installRootModule(keysWithValueModule, abi.encode(sourceTableId));
+  }
+
   function testSetAndDeleteRecordHook() public {
     _installKeysWithValueModule();
 

diff --git a/packages/world-modules/test/UniqueEntityModule.t.sol b/packages/world-modules/test/UniqueEntityModule.t.sol
@@ -6,6 +6,7 @@ import { GasReporter } from "@latticexyz/gas-report/src/GasReporter.sol";
 
 import { StoreSwitch } from "@latticexyz/store/src/StoreSwitch.sol";
 import { World } from "@latticexyz/world/src/World.sol";
+import { IModule } from "@latticexyz/world/src/IModule.sol";
 import { IBaseWorld } from "@latticexyz/world/src/codegen/interfaces/IBaseWorld.sol";
 import { IWorldErrors } from "@latticexyz/world/src/IWorldErrors.sol";
 import { System } from "@latticexyz/world/src/System.sol";
@@ -58,6 +59,12 @@ contract UniqueEntityModuleTest is Test, GasReporter {
     assertEq(UniqueEntity.get(tableId), uniqueEntity + 1);
   }
 
+  function testInstallTwice() public {
+    world.installModule(uniqueEntityModule, new bytes(0));
+    vm.expectRevert(IModule.Module_AlreadyInstalled.selector);
+    world.installModule(uniqueEntityModule, new bytes(0));
+  }
+
   function testInstallRoot() public {
     ResourceId tableId = _tableId;
 
@@ -76,6 +83,12 @@ contract UniqueEntityModuleTest is Test, GasReporter {
     assertEq(UniqueEntity.get(tableId), uniqueEntity + 1);
   }
 
+  function testInstallRootTwice() public {
+    world.installRootModule(uniqueEntityModule, new bytes(0));
+    vm.expectRevert(IModule.Module_AlreadyInstalled.selector);
+    world.installRootModule(uniqueEntityModule, new bytes(0));
+  }
+
   function testPublicAccess() public {
     ResourceId tableId = _tableId;
 

diff --git a/packages/world/src/IModule.sol b/packages/world/src/IModule.sol
@@ -23,6 +23,7 @@ interface IModule is IERC165 {
   /// @dev Errors to represent non-support of specific installation types.
   error Module_RootInstallNotSupported();
   error Module_NonRootInstallNotSupported();
+  error Module_AlreadyInstalled();
 
   /**
    * @notice Return the name of the module.