fix(world): prevent misconfigured delegations, allow unregistering [L…

…-04] (#2096)
latticexyz · Jan 17, 2024 · e4a6189 · e4a6189
1 parent 199adc1
commit e4a6189
Show file tree

Hide file tree

Showing 7 changed files with 76 additions and 14 deletions.
diff --git a/.changeset/smart-games-taste.md b/.changeset/smart-games-taste.md
@@ -0,0 +1,5 @@
+---
+"@latticexyz/world": patch
+---
+
+Prevented invalid delegations by performing full validation regardless of whether `initCallData` is empty. Added an `unregisterDelegation` function which allows explicit unregistration, as opposed of passing in zero bytes into `registerDelegation`.
diff --git a/packages/world-modules/gas-report.json b/packages/world-modules/gas-report.json
@@ -135,7 +135,7 @@
     "file": "test/KeysWithValueModule.t.sol",
     "test": "testGetKeysWithValueGas",
     "name": "install keys with value module",
-    "gasUsed": 674298
+    "gasUsed": 674321
   },
   {
     "file": "test/KeysWithValueModule.t.sol",
@@ -153,7 +153,7 @@
     "file": "test/KeysWithValueModule.t.sol",
     "test": "testInstall",
     "name": "install keys with value module",
-    "gasUsed": 674298
+    "gasUsed": 674321
   },
   {
     "file": "test/KeysWithValueModule.t.sol",
@@ -165,7 +165,7 @@
     "file": "test/KeysWithValueModule.t.sol",
     "test": "testSetAndDeleteRecordHook",
     "name": "install keys with value module",
-    "gasUsed": 674298
+    "gasUsed": 674321
   },
   {
     "file": "test/KeysWithValueModule.t.sol",
@@ -183,7 +183,7 @@
     "file": "test/KeysWithValueModule.t.sol",
     "test": "testSetField",
     "name": "install keys with value module",
-    "gasUsed": 674298
+    "gasUsed": 674321
   },
   {
     "file": "test/KeysWithValueModule.t.sol",
@@ -267,7 +267,7 @@
     "file": "test/StandardDelegationsModule.t.sol",
     "test": "testCallFromCallboundDelegation",
     "name": "register a callbound delegation",
-    "gasUsed": 118172
+    "gasUsed": 118138
   },
   {
     "file": "test/StandardDelegationsModule.t.sol",
@@ -279,7 +279,7 @@
     "file": "test/StandardDelegationsModule.t.sol",
     "test": "testCallFromSystemDelegation",
     "name": "register a systembound delegation",
-    "gasUsed": 115725
+    "gasUsed": 115691
   },
   {
     "file": "test/StandardDelegationsModule.t.sol",
@@ -291,7 +291,7 @@
     "file": "test/StandardDelegationsModule.t.sol",
     "test": "testCallFromTimeboundDelegation",
     "name": "register a timebound delegation",
-    "gasUsed": 112648
+    "gasUsed": 112614
   },
   {
     "file": "test/StandardDelegationsModule.t.sol",
@@ -303,7 +303,7 @@
     "file": "test/UniqueEntityModule.t.sol",
     "test": "testInstall",
     "name": "install unique entity module",
-    "gasUsed": 694710
+    "gasUsed": 694733
   },
   {
     "file": "test/UniqueEntityModule.t.sol",
@@ -315,7 +315,7 @@
     "file": "test/UniqueEntityModule.t.sol",
     "test": "testInstallRoot",
     "name": "installRoot unique entity module",
-    "gasUsed": 663729
+    "gasUsed": 663752
   },
   {
     "file": "test/UniqueEntityModule.t.sol",

diff --git a/packages/world/gas-report.json b/packages/world/gas-report.json
@@ -63,7 +63,7 @@
     "file": "test/Factories.t.sol",
     "test": "testWorldFactory",
     "name": "deploy world via WorldFactory",
-    "gasUsed": 12321908
+    "gasUsed": 12386832
   },
   {
     "file": "test/World.t.sol",
@@ -81,7 +81,7 @@
     "file": "test/World.t.sol",
     "test": "testCallFromUnlimitedDelegation",
     "name": "register an unlimited delegation",
-    "gasUsed": 47552
+    "gasUsed": 47532
   },
   {
     "file": "test/World.t.sol",
@@ -111,7 +111,7 @@
     "file": "test/World.t.sol",
     "test": "testRegisterNamespace",
     "name": "Register a new namespace",
-    "gasUsed": 120895
+    "gasUsed": 120918
   },
   {
     "file": "test/World.t.sol",
@@ -143,6 +143,12 @@
     "name": "Write data to the table",
     "gasUsed": 39056
   },
+  {
+    "file": "test/World.t.sol",
+    "test": "testUnregisterUnlimitedDelegation",
+    "name": "unregister an unlimited delegation",
+    "gasUsed": 26191
+  },
   {
     "file": "test/WorldDynamicUpdate.t.sol",
     "test": "testPopFromDynamicField",

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/core/CoreModule.sol b/packages/world/src/modules/core/CoreModule.sol
@@ -168,7 +168,7 @@ contract CoreModule is Module {
       _registerRootFunctionSelector(BATCH_CALL_SYSTEM_ID, functionSignaturesBatchCall[i]);
     }
 
-    string[12] memory functionSignaturesCoreRegistration = [
+    string[13] memory functionSignaturesCoreRegistration = [
       // --- ModuleInstallationSystem ---
       "installModule(address,bytes)",
       // --- StoreRegistrationSystem ---
@@ -183,6 +183,7 @@ contract CoreModule is Module {
       "registerFunctionSelector(bytes32,string)",
       "registerRootFunctionSelector(bytes32,string,bytes4)",
       "registerDelegation(address,bytes32,bytes)",
+      "unregisterDelegation(address)",
       "registerNamespaceDelegation(bytes32,bytes32,bytes)"
     ];
     for (uint256 i = 0; i < functionSignaturesCoreRegistration.length; i++) {

diff --git a/packages/world/src/modules/core/implementations/WorldRegistrationSystem.sol b/packages/world/src/modules/core/implementations/WorldRegistrationSystem.sol
@@ -245,7 +245,7 @@ contract WorldRegistrationSystem is System, IWorldErrors {
     });
 
     // If the delegation is limited...
-    if (Delegation.isLimited(delegationControlId) && initCallData.length > 0) {
+    if (Delegation.isLimited(delegationControlId)) {
       // Require the delegationControl contract to implement the IDelegationControl interface
       address delegationControl = Systems._getSystem(delegationControlId);
       requireInterface(delegationControl, DELEGATION_CONTROL_INTERFACE_ID);
@@ -260,6 +260,11 @@ contract WorldRegistrationSystem is System, IWorldErrors {
     }
   }
 
+  function unregisterDelegation(address delegatee) public {
+    // Delete the delegation control contract address
+    UserDelegationControl.deleteRecord({ delegator: _msgSender(), delegatee: delegatee });
+  }
+
   /**
    * @notice Registers a delegation for a namespace
    * @dev Sets up a new delegation control for a specific namespace

diff --git a/packages/world/test/World.t.sol b/packages/world/test/World.t.sol
@@ -1112,6 +1112,49 @@ contract WorldTest is Test, GasReporter {
     );
   }
 
+  function testUnregisterUnlimitedDelegation() public {
+    // Register a new system
+    WorldTestSystem system = new WorldTestSystem();
+    ResourceId systemId = WorldResourceIdLib.encode({
+      typeId: RESOURCE_SYSTEM,
+      namespace: "namespace",
+      name: "testSystem"
+    });
+    world.registerNamespace(systemId.getNamespaceId());
+    world.registerSystem(systemId, system, true);
+
+    // Register an unlimited delegation
+    address delegator = address(1);
+    address delegatee = address(2);
+    vm.prank(delegator);
+    world.registerDelegation(delegatee, UNLIMITED_DELEGATION, new bytes(0));
+
+    // Call a system from the delegatee on behalf of the delegator
+    vm.prank(delegatee);
+    bytes memory returnData = world.callFrom(delegator, systemId, abi.encodeCall(WorldTestSystem.msgSender, ()));
+    address returnedAddress = abi.decode(returnData, (address));
+
+    // Expect the system to have received the delegator's address
+    assertEq(returnedAddress, delegator);
+
+    // Unregister the delegation
+    vm.prank(delegator);
+    startGasReport("unregister an unlimited delegation");
+    world.unregisterDelegation(delegatee);
+    endGasReport();
+
+    // Expect a revert when attempting to perform a call on behalf of an address that doesn't have a delegation
+    vm.expectRevert(
+      abi.encodeWithSelector(
+        IWorldErrors.World_DelegationNotFound.selector,
+        delegator,
+        delegatee // Invalid delegatee
+      )
+    );
+    vm.prank(delegatee);
+    world.callFrom(delegator, systemId, abi.encodeCall(WorldTestSystem.msgSender, ()));
+  }
+
   function testRegisterStoreHook() public {
     FieldLayout fieldLayout = Bool.getFieldLayout();
     Schema valueSchema = Bool.getValueSchema();