diff --git a/.changeset/smart-games-taste.md b/.changeset/smart-games-taste.md new file mode 100644 index 0000000000..7de3a638ed --- /dev/null +++ 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 index 24157d1541..e3e5dc4b90 100644 --- 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 index 93aa67a001..b8db9e3bb7 100644 --- 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 index bfe03b5fd9..2c889779b7 100644 --- a/packages/world/src/codegen/interfaces/IWorldRegistrationSystem.sol +++ b/packages/world/src/codegen/interfaces/IWorldRegistrationSystem.sol @@ -33,6 +33,8 @@ interface IWorldRegistrationSystem { function registerDelegation(address delegatee, ResourceId delegationControlId, bytes memory initCallData) external; + function unregisterDelegation(address delegatee) external; + function registerNamespaceDelegation( ResourceId namespaceId, ResourceId delegationControlId, diff --git a/packages/world/src/modules/core/CoreModule.sol b/packages/world/src/modules/core/CoreModule.sol index 632a87a1b7..9a2d1979a5 100644 --- 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 index 11cd866970..a4fe1f8ad1 100644 --- 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 index 3bfca2ff7c..3e9f6db4d3 100644 --- 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();