From 1215784cc0bc629d2c92b5ab44f411d469351f45 Mon Sep 17 00:00:00 2001 From: alvrs Date: Wed, 27 Sep 2023 14:34:40 +0100 Subject: [PATCH 1/3] refactor(world): expose library for WorldContextConsumer so _msgSender() can be used in internal libraries --- packages/world-modules/gas-report.json | 30 ++++++++--------- packages/world/gas-report.json | 22 ++++++------- packages/world/src/SystemCall.sol | 8 ++--- packages/world/src/World.sol | 4 +-- packages/world/src/WorldContext.sol | 32 ++++++++++++++----- .../world/src/modules/core/CoreModule.sol | 6 ++-- .../ModuleInstallationSystem.sol | 4 +-- .../StoreRegistrationSystem.sol | 2 +- packages/world/test/World.t.sol | 4 +-- packages/world/test/WorldContext.t.sol | 8 ++--- 10 files changed, 68 insertions(+), 52 deletions(-) diff --git a/packages/world-modules/gas-report.json b/packages/world-modules/gas-report.json index dbe08a14b7..c806f19a87 100644 --- 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": 1407523 + "gasUsed": 1407698 }, { "file": "test/KeysInTableModule.t.sol", "test": "testInstallGas", "name": "install keys in table module", - "gasUsed": 1407523 + "gasUsed": 1407698 }, { "file": "test/KeysInTableModule.t.sol", @@ -21,13 +21,13 @@ "file": "test/KeysInTableModule.t.sol", "test": "testInstallSingleton", "name": "install keys in table module", - "gasUsed": 1407523 + "gasUsed": 1407698 }, { "file": "test/KeysInTableModule.t.sol", "test": "testSetAndDeleteRecordHookCompositeGas", "name": "install keys in table module", - "gasUsed": 1407523 + "gasUsed": 1407698 }, { "file": "test/KeysInTableModule.t.sol", @@ -45,7 +45,7 @@ "file": "test/KeysInTableModule.t.sol", "test": "testSetAndDeleteRecordHookGas", "name": "install keys in table module", - "gasUsed": 1407523 + "gasUsed": 1407698 }, { "file": "test/KeysInTableModule.t.sol", @@ -63,7 +63,7 @@ "file": "test/KeysWithValueModule.t.sol", "test": "testGetKeysWithValueGas", "name": "install keys with value module", - "gasUsed": 648854 + "gasUsed": 648994 }, { "file": "test/KeysWithValueModule.t.sol", @@ -81,7 +81,7 @@ "file": "test/KeysWithValueModule.t.sol", "test": "testInstall", "name": "install keys with value module", - "gasUsed": 648854 + "gasUsed": 648994 }, { "file": "test/KeysWithValueModule.t.sol", @@ -93,7 +93,7 @@ "file": "test/KeysWithValueModule.t.sol", "test": "testSetAndDeleteRecordHook", "name": "install keys with value module", - "gasUsed": 648854 + "gasUsed": 648994 }, { "file": "test/KeysWithValueModule.t.sol", @@ -111,7 +111,7 @@ "file": "test/KeysWithValueModule.t.sol", "test": "testSetField", "name": "install keys with value module", - "gasUsed": 648854 + "gasUsed": 648994 }, { "file": "test/KeysWithValueModule.t.sol", @@ -195,31 +195,31 @@ "file": "test/StandardDelegationsModule.t.sol", "test": "testCallFromCallboundDelegation", "name": "register a callbound delegation", - "gasUsed": 117315 + "gasUsed": 117420 }, { "file": "test/StandardDelegationsModule.t.sol", "test": "testCallFromCallboundDelegation", "name": "call a system via a callbound delegation", - "gasUsed": 36583 + "gasUsed": 36688 }, { "file": "test/StandardDelegationsModule.t.sol", "test": "testCallFromTimeboundDelegation", "name": "register a timebound delegation", - "gasUsed": 111809 + "gasUsed": 111914 }, { "file": "test/StandardDelegationsModule.t.sol", "test": "testCallFromTimeboundDelegation", "name": "call a system via a timebound delegation", - "gasUsed": 26745 + "gasUsed": 26815 }, { "file": "test/UniqueEntityModule.t.sol", "test": "testInstall", "name": "install unique entity module", - "gasUsed": 675100 + "gasUsed": 675275 }, { "file": "test/UniqueEntityModule.t.sol", @@ -231,7 +231,7 @@ "file": "test/UniqueEntityModule.t.sol", "test": "testInstallRoot", "name": "installRoot unique entity module", - "gasUsed": 642496 + "gasUsed": 642601 }, { "file": "test/UniqueEntityModule.t.sol", diff --git a/packages/world/gas-report.json b/packages/world/gas-report.json index 148fd99d39..36848f6dbc 100644 --- a/packages/world/gas-report.json +++ b/packages/world/gas-report.json @@ -45,37 +45,37 @@ "file": "test/BatchCall.t.sol", "test": "testBatchCallFromReturnData", "name": "call systems with batchCallFrom", - "gasUsed": 46377 + "gasUsed": 46444 }, { "file": "test/BatchCall.t.sol", "test": "testBatchCallReturnData", "name": "call systems with batchCall", - "gasUsed": 45171 + "gasUsed": 45206 }, { "file": "test/World.t.sol", "test": "testCall", "name": "call a system via the World", - "gasUsed": 12335 + "gasUsed": 12370 }, { "file": "test/World.t.sol", "test": "testCallFromNamespaceDelegation", "name": "call a system via a namespace fallback delegation", - "gasUsed": 26073 + "gasUsed": 26143 }, { "file": "test/World.t.sol", "test": "testCallFromUnlimitedDelegation", "name": "register an unlimited delegation", - "gasUsed": 47588 + "gasUsed": 47623 }, { "file": "test/World.t.sol", "test": "testCallFromUnlimitedDelegation", "name": "call a system via an unlimited delegation", - "gasUsed": 12771 + "gasUsed": 12806 }, { "file": "test/World.t.sol", @@ -93,31 +93,31 @@ "file": "test/World.t.sol", "test": "testRegisterFunctionSelector", "name": "Register a function selector", - "gasUsed": 83114 + "gasUsed": 83149 }, { "file": "test/World.t.sol", "test": "testRegisterNamespace", "name": "Register a new namespace", - "gasUsed": 120817 + "gasUsed": 120887 }, { "file": "test/World.t.sol", "test": "testRegisterRootFunctionSelector", "name": "Register a root function selector", - "gasUsed": 80398 + "gasUsed": 80433 }, { "file": "test/World.t.sol", "test": "testRegisterSystem", "name": "register a system", - "gasUsed": 162559 + "gasUsed": 162594 }, { "file": "test/World.t.sol", "test": "testRegisterTable", "name": "Register a new table in the namespace", - "gasUsed": 637329 + "gasUsed": 637399 }, { "file": "test/World.t.sol", diff --git a/packages/world/src/SystemCall.sol b/packages/world/src/SystemCall.sol index d5cd06a788..fdc75abda9 100644 --- a/packages/world/src/SystemCall.sol +++ b/packages/world/src/SystemCall.sol @@ -4,10 +4,10 @@ pragma solidity >=0.8.21; import { Hook } from "@latticexyz/store/src/Hook.sol"; import { ResourceId, WorldResourceIdInstance } from "./WorldResourceId.sol"; -import { WorldContextProvider } from "./WorldContext.sol"; +import { WorldContextProviderLib } from "./WorldContext.sol"; import { AccessControl } from "./AccessControl.sol"; import { ROOT_NAMESPACE } from "./constants.sol"; -import { WorldContextProvider } from "./WorldContext.sol"; +import { WorldContextProviderLib } from "./WorldContext.sol"; import { revertWithBytes } from "./revertWithBytes.sol"; import { BEFORE_CALL_SYSTEM, AFTER_CALL_SYSTEM } from "./systemHookTypes.sol"; @@ -50,13 +50,13 @@ library SystemCall { // Call the system and forward any return data (success, data) = systemId.getNamespace() == ROOT_NAMESPACE // Use delegatecall for root systems (= registered in the root namespace) - ? WorldContextProvider.delegatecallWithContext({ + ? WorldContextProviderLib.delegatecallWithContext({ msgSender: caller, msgValue: value, target: systemAddress, callData: callData }) - : WorldContextProvider.callWithContext({ + : WorldContextProviderLib.callWithContext({ msgSender: caller, msgValue: value, target: systemAddress, diff --git a/packages/world/src/World.sol b/packages/world/src/World.sol index ed3917430b..cf0e0c6e77 100644 --- a/packages/world/src/World.sol +++ b/packages/world/src/World.sol @@ -15,7 +15,7 @@ import { ResourceId, WorldResourceIdInstance } from "./WorldResourceId.sol"; import { ROOT_NAMESPACE_ID, ROOT_NAMESPACE, ROOT_NAME } from "./constants.sol"; import { AccessControl } from "./AccessControl.sol"; import { SystemCall } from "./SystemCall.sol"; -import { WorldContextProvider } from "./WorldContext.sol"; +import { WorldContextProviderLib } from "./WorldContext.sol"; import { revertWithBytes } from "./revertWithBytes.sol"; import { Delegation } from "./Delegation.sol"; import { requireInterface } from "./requireInterface.sol"; @@ -91,7 +91,7 @@ contract World is StoreData, IWorldKernel { // Require the provided address to implement the IModule interface requireInterface(address(module), MODULE_INTERFACE_ID); - WorldContextProvider.delegatecallWithContextOrRevert({ + WorldContextProviderLib.delegatecallWithContextOrRevert({ msgSender: msg.sender, msgValue: 0, target: address(module), diff --git a/packages/world/src/WorldContext.sol b/packages/world/src/WorldContext.sol index 525ab48f2c..e5da820165 100644 --- a/packages/world/src/WorldContext.sol +++ b/packages/world/src/WorldContext.sol @@ -14,6 +14,27 @@ uint256 constant CONTEXT_BYTES = 20 + 32; abstract contract WorldContextConsumer is IWorldContextConsumer { // Extract the trusted msg.sender value appended to the calldata function _msgSender() public view returns (address sender) { + return WorldContextConsumerLib._msgSender(); + } + + // Extract the trusted msg.value value appended to the calldata + function _msgValue() public pure returns (uint256 value) { + return WorldContextConsumerLib._msgValue(); + } + + function _world() public view returns (address) { + return StoreSwitch.getStoreAddress(); + } + + // ERC-165 supportsInterface (see https://eips.ethereum.org/EIPS/eip-165) + function supportsInterface(bytes4 interfaceId) public pure virtual returns (bool) { + return interfaceId == WORLD_CONTEXT_CONSUMER_INTERFACE_ID || interfaceId == ERC165_INTERFACE_ID; + } +} + +library WorldContextConsumerLib { + // Extract the trusted msg.sender value appended to the calldata + function _msgSender() internal view returns (address sender) { assembly { // Load 32 bytes from calldata at position calldatasize() - context size, // then shift left 96 bits (to right-align the address) @@ -24,27 +45,22 @@ abstract contract WorldContextConsumer is IWorldContextConsumer { } // Extract the trusted msg.value value appended to the calldata - function _msgValue() public pure returns (uint256 value) { + function _msgValue() internal pure returns (uint256 value) { assembly { // Load 32 bytes from calldata at position calldatasize() - 32 bytes, value := calldataload(sub(calldatasize(), 32)) } } - function _world() public view returns (address) { + function _world() internal view returns (address) { return StoreSwitch.getStoreAddress(); } - - // ERC-165 supportsInterface (see https://eips.ethereum.org/EIPS/eip-165) - function supportsInterface(bytes4 interfaceId) public pure virtual returns (bool) { - return interfaceId == WORLD_CONTEXT_CONSUMER_INTERFACE_ID || interfaceId == ERC165_INTERFACE_ID; - } } /** * Simple utility function to call a contract and append the msg.sender to the calldata (to be consumed by WorldContextConsumer) */ -library WorldContextProvider { +library WorldContextProviderLib { function appendContext( bytes memory callData, address msgSender, diff --git a/packages/world/src/modules/core/CoreModule.sol b/packages/world/src/modules/core/CoreModule.sol index 5137adaa96..c0b1089d84 100644 --- a/packages/world/src/modules/core/CoreModule.sol +++ b/packages/world/src/modules/core/CoreModule.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity >=0.8.21; -import { WorldContextProvider } from "../../WorldContext.sol"; +import { WorldContextProviderLib } from "../../WorldContext.sol"; import { ROOT_NAMESPACE, ROOT_NAMESPACE_ID, WORLD_NAMESPACE_ID } from "../../constants.sol"; import { Module } from "../../Module.sol"; @@ -90,7 +90,7 @@ contract CoreModule is Module { */ function _registerCoreSystem() internal { // Use the CoreSystem's `registerSystem` implementation to register itself on the World. - WorldContextProvider.delegatecallWithContextOrRevert({ + WorldContextProviderLib.delegatecallWithContextOrRevert({ msgSender: _msgSender(), msgValue: 0, target: coreSystem, @@ -133,7 +133,7 @@ contract CoreModule is Module { for (uint256 i = 0; i < functionSignatures.length; i++) { // Use the CoreSystem's `registerRootFunctionSelector` to register the // root function selectors in the World. - WorldContextProvider.delegatecallWithContextOrRevert({ + WorldContextProviderLib.delegatecallWithContextOrRevert({ msgSender: _msgSender(), msgValue: 0, target: coreSystem, diff --git a/packages/world/src/modules/core/implementations/ModuleInstallationSystem.sol b/packages/world/src/modules/core/implementations/ModuleInstallationSystem.sol index 69fa9f3fbb..2d17185dbb 100644 --- a/packages/world/src/modules/core/implementations/ModuleInstallationSystem.sol +++ b/packages/world/src/modules/core/implementations/ModuleInstallationSystem.sol @@ -4,7 +4,7 @@ pragma solidity >=0.8.21; import { IModule, MODULE_INTERFACE_ID } from "../../../IModule.sol"; import { System } from "../../../System.sol"; import { AccessControl } from "../../../AccessControl.sol"; -import { WorldContextProvider } from "../../../WorldContext.sol"; +import { WorldContextProviderLib } from "../../../WorldContext.sol"; import { ResourceAccess } from "../../../codegen/tables/ResourceAccess.sol"; import { InstalledModules } from "../../../codegen/tables/InstalledModules.sol"; import { requireInterface } from "../../../requireInterface.sol"; @@ -20,7 +20,7 @@ contract ModuleInstallationSystem is System { // Require the provided address to implement the IModule interface requireInterface(address(module), MODULE_INTERFACE_ID); - WorldContextProvider.callWithContextOrRevert({ + WorldContextProviderLib.callWithContextOrRevert({ msgSender: _msgSender(), msgValue: 0, target: address(module), diff --git a/packages/world/src/modules/core/implementations/StoreRegistrationSystem.sol b/packages/world/src/modules/core/implementations/StoreRegistrationSystem.sol index 77fcbad182..370064e295 100644 --- a/packages/world/src/modules/core/implementations/StoreRegistrationSystem.sol +++ b/packages/world/src/modules/core/implementations/StoreRegistrationSystem.sol @@ -13,7 +13,7 @@ import { ROOT_NAMESPACE, ROOT_NAME } from "../../../constants.sol"; import { AccessControl } from "../../../AccessControl.sol"; import { requireInterface } from "../../../requireInterface.sol"; import { revertWithBytes } from "../../../revertWithBytes.sol"; -import { WorldContextProvider } from "../../../WorldContext.sol"; +import { WorldContextProviderLib } from "../../../WorldContext.sol"; import { NamespaceOwner } from "../../../codegen/tables/NamespaceOwner.sol"; import { ResourceAccess } from "../../../codegen/tables/ResourceAccess.sol"; diff --git a/packages/world/test/World.t.sol b/packages/world/test/World.t.sol index eb22836447..730e8ab6dd 100644 --- a/packages/world/test/World.t.sol +++ b/packages/world/test/World.t.sol @@ -27,7 +27,7 @@ import { System } from "../src/System.sol"; import { ResourceId, WorldResourceIdLib, WorldResourceIdInstance } from "../src/WorldResourceId.sol"; import { ROOT_NAMESPACE, ROOT_NAME, ROOT_NAMESPACE_ID, UNLIMITED_DELEGATION } from "../src/constants.sol"; import { RESOURCE_TABLE, RESOURCE_SYSTEM, RESOURCE_NAMESPACE } from "../src/worldResourceTypes.sol"; -import { WorldContextProvider, WORLD_CONTEXT_CONSUMER_INTERFACE_ID } from "../src/WorldContext.sol"; +import { WorldContextProviderLib, WORLD_CONTEXT_CONSUMER_INTERFACE_ID } from "../src/WorldContext.sol"; import { SystemHook } from "../src/SystemHook.sol"; import { BEFORE_CALL_SYSTEM, AFTER_CALL_SYSTEM } from "../src/systemHookTypes.sol"; import { Module, MODULE_INTERFACE_ID } from "../src/Module.sol"; @@ -900,7 +900,7 @@ contract WorldTest is Test, GasReporter { WorldTestSystem.delegateCallSubSystem, // Function in system ( address(subSystem), // Address of subsystem - WorldContextProvider.appendContext({ + WorldContextProviderLib.appendContext({ callData: abi.encodeCall(WorldTestSystem.msgSender, ()), msgSender: address(this), msgValue: uint256(0) diff --git a/packages/world/test/WorldContext.t.sol b/packages/world/test/WorldContext.t.sol index 5015e0968f..e0f74baa83 100644 --- a/packages/world/test/WorldContext.t.sol +++ b/packages/world/test/WorldContext.t.sol @@ -3,7 +3,7 @@ pragma solidity >=0.8.21; import "forge-std/Test.sol"; import { GasReporter } from "@latticexyz/gas-report/src/GasReporter.sol"; -import { WorldContextProvider, WorldContextConsumer } from "../src/WorldContext.sol"; +import { WorldContextProviderLib, WorldContextConsumer } from "../src/WorldContext.sol"; contract TestContextConsumer is WorldContextConsumer { event Context(bytes args, address msgSender, uint256 msgValue); @@ -21,7 +21,7 @@ contract WorldContextTest is Test, GasReporter { function testFuzzAppendContext(bytes memory callData, address msgSender, uint256 msgValue) public { assertEq( keccak256(abi.encodePacked(callData, msgSender, msgValue)), - keccak256(WorldContextProvider.appendContext(callData, msgSender, msgValue)) + keccak256(WorldContextProviderLib.appendContext(callData, msgSender, msgValue)) ); } @@ -30,7 +30,7 @@ contract WorldContextTest is Test, GasReporter { vm.expectEmit(true, true, true, true); emit Context(args, msgSender, msgValue); - WorldContextProvider.callWithContextOrRevert({ + WorldContextProviderLib.callWithContextOrRevert({ msgSender: msgSender, msgValue: msgValue, target: address(consumer), @@ -43,7 +43,7 @@ contract WorldContextTest is Test, GasReporter { vm.expectEmit(true, true, true, true); emit Context(args, msgSender, msgValue); - WorldContextProvider.delegatecallWithContextOrRevert({ + WorldContextProviderLib.delegatecallWithContextOrRevert({ msgSender: msgSender, msgValue: msgValue, target: address(consumer), From 5e1215b70a37c240ae32ad5db25c009a6f586353 Mon Sep 17 00:00:00 2001 From: alvarius Date: Wed, 27 Sep 2023 14:40:30 +0100 Subject: [PATCH 2/3] Create olive-foxes-shop.md --- .changeset/olive-foxes-shop.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/olive-foxes-shop.md diff --git a/.changeset/olive-foxes-shop.md b/.changeset/olive-foxes-shop.md new file mode 100644 index 0000000000..8f55c2bf74 --- /dev/null +++ b/.changeset/olive-foxes-shop.md @@ -0,0 +1,7 @@ +--- +"@latticexyz/world-modules": patch +"@latticexyz/world": minor +--- + +We now expose a `WorldContextConsumerLib` library with the same functionality as the `WorldContextConsumer` contract, but the ability to be used inside of internal libraries. +We also renamed the `WorldContextProvider` library to `WorldContextProviderLib` for consistency. From 6be4852b8ced26183b9d021f3ae2095caa6efcb7 Mon Sep 17 00:00:00 2001 From: alvarius Date: Sat, 30 Sep 2023 22:51:26 +0100 Subject: [PATCH 3/3] Update .changeset/olive-foxes-shop.md --- .changeset/olive-foxes-shop.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/olive-foxes-shop.md b/.changeset/olive-foxes-shop.md index 8f55c2bf74..5ad83f87fe 100644 --- a/.changeset/olive-foxes-shop.md +++ b/.changeset/olive-foxes-shop.md @@ -1,6 +1,6 @@ --- "@latticexyz/world-modules": patch -"@latticexyz/world": minor +"@latticexyz/world": major --- We now expose a `WorldContextConsumerLib` library with the same functionality as the `WorldContextConsumer` contract, but the ability to be used inside of internal libraries.