diff --git a/.changeset/few-jars-turn.md b/.changeset/few-jars-turn.md new file mode 100644 index 0000000000..93c3488d5f --- /dev/null +++ b/.changeset/few-jars-turn.md @@ -0,0 +1,42 @@ +--- +"@latticexyz/store": major +"@latticexyz/world": major +--- + +Moved the registration of store hooks and systems hooks to bitmaps with bitwise operator instead of a struct. + +```diff +- import { StoreHookLib } from "@latticexyz/src/StoreHook.sol"; ++ import { ++ BEFORE_SET_RECORD, ++ BEFORE_SET_FIELD, ++ BEFORE_DELETE_RECORD ++ } from "@latticexyz/store/storeHookTypes.sol"; + + StoreCore.registerStoreHook( + tableId, + subscriber, +- StoreHookLib.encodeBitmap({ +- onBeforeSetRecord: true, +- onAfterSetRecord: false, +- onBeforeSetField: true, +- onAfterSetField: false, +- onBeforeDeleteRecord: true, +- onAfterDeleteRecord: false +- }) ++ BEFORE_SET_RECORD | BEFORE_SET_FIELD | BEFORE_DELETE_RECORD + ); +``` + +```diff +- import { SystemHookLib } from "../src/SystemHook.sol"; ++ import { BEFORE_CALL_SYSTEM, AFTER_CALL_SYSTEM } from "../src/systemHookTypes.sol"; + + world.registerSystemHook( + systemId, + subscriber, +- SystemHookLib.encodeBitmap({ onBeforeCallSystem: true, onAfterCallSystem: true }) ++ BEFORE_CALL_SYSTEM | AFTER_CALL_SYSTEM + ); + +``` diff --git a/packages/store/gas-report.json b/packages/store/gas-report.json index 3a701079a4..839855935a 100644 --- a/packages/store/gas-report.json +++ b/packages/store/gas-report.json @@ -345,7 +345,7 @@ "file": "test/Hook.t.sol", "test": "testIsEnabled", "name": "check if hook is enabled", - "gasUsed": 114 + "gasUsed": 108 }, { "file": "test/KeyEncoding.t.sol", @@ -681,49 +681,49 @@ "file": "test/StoreCoreGas.t.sol", "test": "testHooks", "name": "register subscriber", - "gasUsed": 60413 + "gasUsed": 60150 }, { "file": "test/StoreCoreGas.t.sol", "test": "testHooks", "name": "set record on table with subscriber", - "gasUsed": 71044 + "gasUsed": 71004 }, { "file": "test/StoreCoreGas.t.sol", "test": "testHooks", "name": "set static field on table with subscriber", - "gasUsed": 20820 + "gasUsed": 20780 }, { "file": "test/StoreCoreGas.t.sol", "test": "testHooks", "name": "delete record on table with subscriber", - "gasUsed": 16438 + "gasUsed": 16398 }, { "file": "test/StoreCoreGas.t.sol", "test": "testHooksDynamicData", "name": "register subscriber", - "gasUsed": 60413 + "gasUsed": 60150 }, { "file": "test/StoreCoreGas.t.sol", "test": "testHooksDynamicData", "name": "set (dynamic) record on table with subscriber", - "gasUsed": 164166 + "gasUsed": 164126 }, { "file": "test/StoreCoreGas.t.sol", "test": "testHooksDynamicData", "name": "set (dynamic) field on table with subscriber", - "gasUsed": 24020 + "gasUsed": 23980 }, { "file": "test/StoreCoreGas.t.sol", "test": "testHooksDynamicData", "name": "delete (dynamic) record on table with subscriber", - "gasUsed": 17423 + "gasUsed": 17383 }, { "file": "test/StoreCoreGas.t.sol", @@ -891,13 +891,13 @@ "file": "test/StoreHook.t.sol", "test": "testCallHook", "name": "call an enabled hook", - "gasUsed": 14598 + "gasUsed": 14588 }, { "file": "test/StoreHook.t.sol", "test": "testCallHook", "name": "call a disabled hook", - "gasUsed": 133 + "gasUsed": 123 }, { "file": "test/StoreHook.t.sol", @@ -905,17 +905,11 @@ "name": "get store hook address", "gasUsed": 1 }, - { - "file": "test/StoreHook.t.sol", - "test": "testGetBitmap", - "name": "get store hook bitmap", - "gasUsed": 1 - }, { "file": "test/StoreHook.t.sol", "test": "testIsEnabled", "name": "check if store hook is enabled", - "gasUsed": 129 + "gasUsed": 108 }, { "file": "test/StoreSwitch.t.sol", diff --git a/packages/store/src/Hook.sol b/packages/store/src/Hook.sol index 2d7da97a6f..d7511d4dc2 100644 --- a/packages/store/src/Hook.sol +++ b/packages/store/src/Hook.sol @@ -52,9 +52,8 @@ library HookInstance { /** * Check if the given hook type is enabled in the hook */ - function isEnabled(Hook self, uint8 hookType) internal pure returns (bool) { - // Pick the bitmap encoded in the rightmost byte from the hook and check if the bit at the given hook type is set - return (getBitmap(self) & (1 << uint8(hookType))) != 0; + function isEnabled(Hook self, uint8 hookTypes) internal pure returns (bool) { + return (getBitmap(self) & hookTypes) == hookTypes; } /** diff --git a/packages/store/src/StoreCore.sol b/packages/store/src/StoreCore.sol index 5c2b974148..0d7e48548a 100644 --- a/packages/store/src/StoreCore.sol +++ b/packages/store/src/StoreCore.sol @@ -14,7 +14,7 @@ import { IStoreErrors } from "./IStoreErrors.sol"; import { IStoreHook } from "./IStoreHook.sol"; import { StoreSwitch } from "./StoreSwitch.sol"; import { Hook, HookLib } from "./Hook.sol"; -import { StoreHookLib, StoreHookType } from "./StoreHook.sol"; +import { BEFORE_SET_RECORD, AFTER_SET_RECORD, BEFORE_SET_FIELD, AFTER_SET_FIELD, BEFORE_DELETE_RECORD, AFTER_DELETE_RECORD } from "./storeHookTypes.sol"; library StoreCore { event HelloStore(bytes32 indexed version); @@ -176,7 +176,7 @@ library StoreCore { * Register hooks to be called when a record or field is set or deleted */ function registerStoreHook(bytes32 tableId, IStoreHook hookAddress, uint8 enabledHooksBitmap) internal { - StoreHooks.push(tableId, Hook.unwrap(StoreHookLib.encode(hookAddress, enabledHooksBitmap))); + StoreHooks.push(tableId, Hook.unwrap(HookLib.encode(address(hookAddress), enabledHooksBitmap))); } /** @@ -216,7 +216,7 @@ library StoreCore { bytes21[] memory hooks = StoreHooks._get(tableId); for (uint256 i; i < hooks.length; i++) { Hook hook = Hook.wrap(hooks[i]); - if (hook.isEnabled(uint8(StoreHookType.BEFORE_SET_RECORD))) { + if (hook.isEnabled(BEFORE_SET_RECORD)) { IStoreHook(hook.getAddress()).onBeforeSetRecord( tableId, keyTuple, @@ -269,7 +269,7 @@ library StoreCore { // Call onAfterSetRecord hooks (after modifying the state) for (uint256 i; i < hooks.length; i++) { Hook hook = Hook.wrap(hooks[i]); - if (hook.isEnabled(uint8(StoreHookType.AFTER_SET_RECORD))) { + if (hook.isEnabled(AFTER_SET_RECORD)) { IStoreHook(hook.getAddress()).onAfterSetRecord( tableId, keyTuple, @@ -296,7 +296,7 @@ library StoreCore { bytes21[] memory hooks = StoreHooks._get(tableId); for (uint256 i; i < hooks.length; i++) { Hook hook = Hook.wrap(hooks[i]); - if (hook.isEnabled(uint8(StoreHookType.BEFORE_SET_FIELD))) { + if (hook.isEnabled(BEFORE_SET_FIELD)) { IStoreHook(hook.getAddress()).onBeforeSetField(tableId, keyTuple, fieldIndex, data, fieldLayout); } } @@ -310,7 +310,7 @@ library StoreCore { // Call onAfterSetField hooks (after modifying the state) for (uint256 i; i < hooks.length; i++) { Hook hook = Hook.wrap(hooks[i]); - if (hook.isEnabled(uint8(StoreHookType.AFTER_SET_FIELD))) { + if (hook.isEnabled(AFTER_SET_FIELD)) { IStoreHook(hook.getAddress()).onAfterSetField(tableId, keyTuple, fieldIndex, data, fieldLayout); } } @@ -327,7 +327,7 @@ library StoreCore { bytes21[] memory hooks = StoreHooks._get(tableId); for (uint256 i; i < hooks.length; i++) { Hook hook = Hook.wrap(hooks[i]); - if (hook.isEnabled(uint8(StoreHookType.BEFORE_DELETE_RECORD))) { + if (hook.isEnabled(BEFORE_DELETE_RECORD)) { IStoreHook(hook.getAddress()).onBeforeDeleteRecord(tableId, keyTuple, fieldLayout); } } @@ -345,7 +345,7 @@ library StoreCore { // Call onAfterDeleteRecord hooks for (uint256 i; i < hooks.length; i++) { Hook hook = Hook.wrap(hooks[i]); - if (hook.isEnabled(uint8(StoreHookType.AFTER_DELETE_RECORD))) { + if (hook.isEnabled(AFTER_DELETE_RECORD)) { IStoreHook(hook.getAddress()).onAfterDeleteRecord(tableId, keyTuple, fieldLayout); } } @@ -375,7 +375,7 @@ library StoreCore { bytes21[] memory hooks = StoreHooks._get(tableId); for (uint256 i; i < hooks.length; i++) { Hook hook = Hook.wrap(hooks[i]); - if (hook.isEnabled(uint8(StoreHookType.BEFORE_SET_FIELD))) { + if (hook.isEnabled(BEFORE_SET_FIELD)) { IStoreHook(hook.getAddress()).onBeforeSetField(tableId, keyTuple, fieldIndex, fullData, fieldLayout); } } @@ -385,7 +385,7 @@ library StoreCore { // Call onAfterSetField hooks (after modifying the state) for (uint256 i; i < hooks.length; i++) { Hook hook = Hook.wrap(hooks[i]); - if (hook.isEnabled(uint8(StoreHookType.AFTER_SET_FIELD))) { + if (hook.isEnabled(AFTER_SET_FIELD)) { IStoreHook(hook.getAddress()).onAfterSetField(tableId, keyTuple, fieldIndex, fullData, fieldLayout); } } @@ -416,7 +416,7 @@ library StoreCore { bytes21[] memory hooks = StoreHooks._get(tableId); for (uint256 i; i < hooks.length; i++) { Hook hook = Hook.wrap(hooks[i]); - if (hook.isEnabled(uint8(StoreHookType.BEFORE_SET_FIELD))) { + if (hook.isEnabled(BEFORE_SET_FIELD)) { IStoreHook(hook.getAddress()).onBeforeSetField(tableId, keyTuple, fieldIndex, fullData, fieldLayout); } } @@ -426,7 +426,7 @@ library StoreCore { // Call onAfterSetField hooks (after modifying the state) for (uint256 i; i < hooks.length; i++) { Hook hook = Hook.wrap(hooks[i]); - if (hook.isEnabled(uint8(StoreHookType.AFTER_SET_FIELD))) { + if (hook.isEnabled(AFTER_SET_FIELD)) { IStoreHook(hook.getAddress()).onAfterSetField(tableId, keyTuple, fieldIndex, fullData, fieldLayout); } } @@ -468,7 +468,7 @@ library StoreCore { bytes21[] memory hooks = StoreHooks._get(tableId); for (uint256 i; i < hooks.length; i++) { Hook hook = Hook.wrap(hooks[i]); - if (hook.isEnabled(uint8(StoreHookType.BEFORE_SET_FIELD))) { + if (hook.isEnabled(BEFORE_SET_FIELD)) { IStoreHook(hook.getAddress()).onBeforeSetField(tableId, keyTuple, fieldIndex, fullData, fieldLayout); } } @@ -478,7 +478,7 @@ library StoreCore { // Call onAfterSetField hooks (after modifying the state) for (uint256 i; i < hooks.length; i++) { Hook hook = Hook.wrap(hooks[i]); - if (hook.isEnabled(uint8(StoreHookType.AFTER_SET_FIELD))) { + if (hook.isEnabled(AFTER_SET_FIELD)) { IStoreHook(hook.getAddress()).onAfterSetField(tableId, keyTuple, fieldIndex, fullData, fieldLayout); } } diff --git a/packages/store/src/StoreHook.sol b/packages/store/src/StoreHook.sol index 0d7de0f899..0fff2ce06e 100644 --- a/packages/store/src/StoreHook.sol +++ b/packages/store/src/StoreHook.sol @@ -1,49 +1,9 @@ // SPDX-License-Identifier: MIT pragma solidity >=0.8.0; -import { Hook, HookLib } from "./Hook.sol"; import { IStoreHook, STORE_HOOK_INTERFACE_ID } from "./IStoreHook.sol"; import { ERC165_INTERFACE_ID } from "./IERC165.sol"; -enum StoreHookType { - BEFORE_SET_RECORD, - AFTER_SET_RECORD, - BEFORE_SET_FIELD, - AFTER_SET_FIELD, - BEFORE_DELETE_RECORD, - AFTER_DELETE_RECORD -} - -library StoreHookLib { - /** - * Encode the bitmap into a single byte - */ - function encodeBitmap( - bool onBeforeSetRecord, - bool onAfterSetRecord, - bool onBeforeSetField, - bool onAfterSetField, - bool onBeforeDeleteRecord, - bool onAfterDeleteRecord - ) internal pure returns (uint8) { - uint256 bitmap = 0; - if (onBeforeSetRecord) bitmap |= 1 << uint8(StoreHookType.BEFORE_SET_RECORD); - if (onAfterSetRecord) bitmap |= 1 << uint8(StoreHookType.AFTER_SET_RECORD); - if (onBeforeSetField) bitmap |= 1 << uint8(StoreHookType.BEFORE_SET_FIELD); - if (onAfterSetField) bitmap |= 1 << uint8(StoreHookType.AFTER_SET_FIELD); - if (onBeforeDeleteRecord) bitmap |= 1 << uint8(StoreHookType.BEFORE_DELETE_RECORD); - if (onAfterDeleteRecord) bitmap |= 1 << uint8(StoreHookType.AFTER_DELETE_RECORD); - return uint8(bitmap); - } - - /** - * Encode enabled hooks into a bitmap with 1 bit per hook, and pack the bitmap with the store hook address into a bytes21 value - */ - function encode(IStoreHook storeHook, uint8 enabledHooksBitmap) internal pure returns (Hook) { - return HookLib.encode(address(storeHook), enabledHooksBitmap); - } -} - abstract contract StoreHook is IStoreHook { // ERC-165 supportsInterface (see https://eips.ethereum.org/EIPS/eip-165) function supportsInterface(bytes4 interfaceId) public pure virtual returns (bool) { diff --git a/packages/store/src/storeHookTypes.sol b/packages/store/src/storeHookTypes.sol new file mode 100644 index 0000000000..e6a9f00718 --- /dev/null +++ b/packages/store/src/storeHookTypes.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: MIT +pragma solidity >=0.8.0; + +uint8 constant BEFORE_SET_RECORD = 1 << 0; +uint8 constant AFTER_SET_RECORD = 1 << 1; +// TODO: do we need to differentiate between static and dynamic set field? +uint8 constant BEFORE_SET_FIELD = 1 << 2; +uint8 constant AFTER_SET_FIELD = 1 << 3; +uint8 constant BEFORE_DELETE_RECORD = 1 << 4; +uint8 constant AFTER_DELETE_RECORD = 1 << 5; diff --git a/packages/store/test/Hook.t.sol b/packages/store/test/Hook.t.sol index c328d701d4..d3c87e4060 100644 --- a/packages/store/test/Hook.t.sol +++ b/packages/store/test/Hook.t.sol @@ -6,15 +6,15 @@ import { GasReporter } from "@latticexyz/gas-report/src/GasReporter.sol"; import { Hook, HookLib } from "../src/Hook.sol"; contract HookTest is Test, GasReporter { - uint8 public NONE = 0x00; // 0b00000000 - uint8 public FIRST = 0x01; // 0b00000001 - uint8 public SECOND = 0x02; // 0b00000010 - uint8 public THIRD = 0x04; // 0b00000100 - uint8 public FOURTH = 0x08; // 0b00001000 - uint8 public FIFTH = 0x10; // 0b00010000 - uint8 public SIXTH = 0x20; // 0b00100000 - uint8 public SEVENTH = 0x40; // 0b01000000 - uint8 public EIGHTH = 0x80; // 0b10000000 + uint8 public constant NONE = 0; // 0b00000000 + uint8 public constant FIRST = 1 << 0; // 0b00000001 + uint8 public constant SECOND = 1 << 1; // 0b00000010 + uint8 public constant THIRD = 1 << 2; // 0b00000100 + uint8 public constant FOURTH = 1 << 3; // 0b00001000 + uint8 public constant FIFTH = 1 << 4; // 0b00010000 + uint8 public constant SIXTH = 1 << 5; // 0b00100000 + uint8 public constant SEVENTH = 1 << 6; // 0b01000000 + uint8 public constant EIGHTH = 1 << 7; // 0b10000000 function testFuzzEncode(address hookAddress, uint8 encodedHooks) public { assertEq( @@ -24,20 +24,20 @@ contract HookTest is Test, GasReporter { } function testIsEnabled() public { - Hook hook = HookLib.encode(address(0), THIRD); + Hook hook = HookLib.encode(address(0), THIRD | FIFTH); startGasReport("check if hook is enabled"); - hook.isEnabled(0); + hook.isEnabled(EIGHTH); endGasReport(); - assertFalse(hook.isEnabled(0)); - assertFalse(hook.isEnabled(1)); - assertTrue(hook.isEnabled(2)); - assertFalse(hook.isEnabled(3)); - assertFalse(hook.isEnabled(4)); - assertFalse(hook.isEnabled(5)); - assertFalse(hook.isEnabled(6)); - assertFalse(hook.isEnabled(7)); + assertFalse(hook.isEnabled(FIRST), "FIRST"); + assertFalse(hook.isEnabled(SECOND), "SECOND"); + assertTrue(hook.isEnabled(THIRD), "THIRD"); + assertFalse(hook.isEnabled(FOURTH), "FOURTH"); + assertTrue(hook.isEnabled(FIFTH), "FIFTH"); + assertFalse(hook.isEnabled(SIXTH), "SIXTH"); + assertFalse(hook.isEnabled(SEVENTH), "SEVENTH"); + assertFalse(hook.isEnabled(EIGHTH), "EIGHTH"); } function testFuzzIsEnabled( @@ -51,7 +51,7 @@ contract HookTest is Test, GasReporter { bool seventh, bool eighth ) public { - uint8 encodedHooks = 0x00; + uint8 encodedHooks = 0; if (first) encodedHooks |= FIRST; if (second) encodedHooks |= SECOND; if (third) encodedHooks |= THIRD; @@ -63,14 +63,14 @@ contract HookTest is Test, GasReporter { Hook hook = HookLib.encode(hookAddress, encodedHooks); - assertEq(hook.isEnabled(0), first); - assertEq(hook.isEnabled(1), second); - assertEq(hook.isEnabled(2), third); - assertEq(hook.isEnabled(3), fourth); - assertEq(hook.isEnabled(4), fifth); - assertEq(hook.isEnabled(5), sixth); - assertEq(hook.isEnabled(6), seventh); - assertEq(hook.isEnabled(7), eighth); + assertEq(hook.isEnabled(FIRST), first, "FIRST"); + assertEq(hook.isEnabled(SECOND), second, "SECOND"); + assertEq(hook.isEnabled(THIRD), third, "THIRD"); + assertEq(hook.isEnabled(FOURTH), fourth, "FOURTH"); + assertEq(hook.isEnabled(FIFTH), fifth, "FIFTH"); + assertEq(hook.isEnabled(SIXTH), sixth, "SIXTH"); + assertEq(hook.isEnabled(SEVENTH), seventh, "SEVENTH"); + assertEq(hook.isEnabled(EIGHTH), eighth, "EIGHTH"); } function testFuzzGetAddressAndBitmap(address hookAddress, uint8 encodedHooks) public { diff --git a/packages/store/test/StoreCore.t.sol b/packages/store/test/StoreCore.t.sol index fc1f25134e..5228db737d 100644 --- a/packages/store/test/StoreCore.t.sol +++ b/packages/store/test/StoreCore.t.sol @@ -16,7 +16,7 @@ import { IStore } from "../src/IStore.sol"; import { StoreSwitch } from "../src/StoreSwitch.sol"; import { Tables, TablesTableId } from "../src/codegen/Tables.sol"; import { FieldLayoutEncodeHelper } from "./FieldLayoutEncodeHelper.sol"; -import { StoreHookLib } from "../src/StoreHook.sol"; +import { BEFORE_SET_RECORD, AFTER_SET_RECORD, BEFORE_SET_FIELD, AFTER_SET_FIELD, BEFORE_DELETE_RECORD, AFTER_DELETE_RECORD } from "../src/storeHookTypes.sol"; import { SchemaEncodeHelper } from "./SchemaEncodeHelper.sol"; import { StoreMock } from "./StoreMock.sol"; import { MirrorSubscriber, indexerTableId } from "./MirrorSubscriber.sol"; @@ -977,18 +977,7 @@ contract StoreCoreTest is Test, StoreMock { new string[](1) ); - IStore(this).registerStoreHook( - tableId, - subscriber, - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: true, - onAfterSetRecord: false, - onBeforeSetField: true, - onAfterSetField: false, - onBeforeDeleteRecord: true, - onAfterDeleteRecord: false - }) - ); + IStore(this).registerStoreHook(tableId, subscriber, BEFORE_SET_RECORD | BEFORE_SET_FIELD | BEFORE_DELETE_RECORD); bytes memory staticData = abi.encodePacked(bytes16(0x0102030405060708090a0b0c0d0e0f10)); @@ -1031,27 +1020,23 @@ contract StoreCoreTest is Test, StoreMock { IStore(this).registerStoreHook( tableId, revertSubscriber, - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: true, - onAfterSetRecord: true, - onBeforeSetField: true, - onAfterSetField: true, - onBeforeDeleteRecord: true, - onAfterDeleteRecord: true - }) + BEFORE_SET_RECORD | + AFTER_SET_RECORD | + BEFORE_SET_FIELD | + AFTER_SET_FIELD | + BEFORE_DELETE_RECORD | + AFTER_DELETE_RECORD ); // Register both subscribers IStore(this).registerStoreHook( tableId, echoSubscriber, - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: true, - onAfterSetRecord: true, - onBeforeSetField: true, - onAfterSetField: true, - onBeforeDeleteRecord: true, - onAfterDeleteRecord: true - }) + BEFORE_SET_RECORD | + AFTER_SET_RECORD | + BEFORE_SET_FIELD | + AFTER_SET_FIELD | + BEFORE_DELETE_RECORD | + AFTER_DELETE_RECORD ); bytes memory data = abi.encodePacked(bytes16(0x0102030405060708090a0b0c0d0e0f10)); @@ -1122,18 +1107,7 @@ contract StoreCoreTest is Test, StoreMock { new string[](2) ); - IStore(this).registerStoreHook( - tableId, - subscriber, - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: true, - onAfterSetRecord: false, - onBeforeSetField: true, - onAfterSetField: false, - onBeforeDeleteRecord: true, - onAfterDeleteRecord: false - }) - ); + IStore(this).registerStoreHook(tableId, subscriber, BEFORE_SET_RECORD | BEFORE_SET_FIELD | BEFORE_DELETE_RECORD); uint32[] memory arrayData = new uint32[](1); arrayData[0] = 0x01020304; diff --git a/packages/store/test/StoreCoreGas.t.sol b/packages/store/test/StoreCoreGas.t.sol index 5c30514ccd..cd8227872d 100644 --- a/packages/store/test/StoreCoreGas.t.sol +++ b/packages/store/test/StoreCoreGas.t.sol @@ -15,10 +15,10 @@ import { StoreMock } from "../test/StoreMock.sol"; import { IStoreErrors } from "../src/IStoreErrors.sol"; import { IStore } from "../src/IStore.sol"; import { FieldLayoutEncodeHelper } from "./FieldLayoutEncodeHelper.sol"; -import { StoreHookLib } from "../src/StoreHook.sol"; import { SchemaEncodeHelper } from "./SchemaEncodeHelper.sol"; import { StoreMock } from "./StoreMock.sol"; import { MirrorSubscriber } from "./MirrorSubscriber.sol"; +import { BEFORE_SET_RECORD, AFTER_SET_RECORD, BEFORE_SET_FIELD, AFTER_SET_FIELD, BEFORE_DELETE_RECORD, AFTER_DELETE_RECORD } from "../src/storeHookTypes.sol"; struct TestStruct { uint128 firstData; @@ -612,18 +612,7 @@ contract StoreCoreGasTest is Test, GasReporter, StoreMock { ); startGasReport("register subscriber"); - StoreCore.registerStoreHook( - tableId, - subscriber, - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: true, - onAfterSetRecord: false, - onBeforeSetField: true, - onAfterSetField: false, - onBeforeDeleteRecord: true, - onAfterDeleteRecord: false - }) - ); + StoreCore.registerStoreHook(tableId, subscriber, BEFORE_SET_RECORD | BEFORE_SET_FIELD | BEFORE_DELETE_RECORD); endGasReport(); bytes memory staticData = abi.encodePacked(bytes16(0x0102030405060708090a0b0c0d0e0f10)); @@ -665,18 +654,7 @@ contract StoreCoreGasTest is Test, GasReporter, StoreMock { ); startGasReport("register subscriber"); - StoreCore.registerStoreHook( - tableId, - subscriber, - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: true, - onAfterSetRecord: false, - onBeforeSetField: true, - onAfterSetField: false, - onBeforeDeleteRecord: true, - onAfterDeleteRecord: false - }) - ); + StoreCore.registerStoreHook(tableId, subscriber, BEFORE_SET_RECORD | BEFORE_SET_FIELD | BEFORE_DELETE_RECORD); endGasReport(); uint32[] memory arrayData = new uint32[](1); diff --git a/packages/store/test/StoreHook.t.sol b/packages/store/test/StoreHook.t.sol index 311dcbc90a..e8264e5aef 100644 --- a/packages/store/test/StoreHook.t.sol +++ b/packages/store/test/StoreHook.t.sol @@ -7,12 +7,11 @@ import { GasReporter } from "@latticexyz/gas-report/src/GasReporter.sol"; import { EchoSubscriber } from "./EchoSubscriber.sol"; import { RevertSubscriber } from "./RevertSubscriber.sol"; -import { Hook } from "../src/Hook.sol"; -import { StoreHookType } from "../src/StoreHook.sol"; -import { StoreHookLib } from "../src/StoreHook.sol"; +import { Hook, HookLib } from "../src/Hook.sol"; import { IStoreHook } from "../src/IStore.sol"; import { PackedCounter } from "../src/PackedCounter.sol"; import { FieldLayout } from "../src/FieldLayout.sol"; +import { BEFORE_SET_RECORD, AFTER_SET_RECORD, BEFORE_SET_FIELD, AFTER_SET_FIELD, BEFORE_DELETE_RECORD, AFTER_DELETE_RECORD } from "../src/storeHookTypes.sol"; contract StoreHookTest is Test, GasReporter { event HookCalled(bytes); @@ -29,106 +28,20 @@ contract StoreHookTest is Test, GasReporter { FieldLayout private fieldLayout = FieldLayout.wrap(0); function testEncodeBitmap() public { - assertEq( - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: false, - onAfterSetRecord: false, - onBeforeSetField: false, - onAfterSetField: false, - onBeforeDeleteRecord: false, - onAfterDeleteRecord: false - }), - uint8(0x00), - "0b00000000" - ); - - assertEq( - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: true, - onAfterSetRecord: false, - onBeforeSetField: false, - onAfterSetField: false, - onBeforeDeleteRecord: false, - onAfterDeleteRecord: false - }), - uint8(0x01), - "0b00000001" - ); - - assertEq( - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: false, - onAfterSetRecord: true, - onBeforeSetField: false, - onAfterSetField: false, - onBeforeDeleteRecord: false, - onAfterDeleteRecord: false - }), - uint8(0x02), - "0b00000010" - ); - - assertEq( - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: false, - onAfterSetRecord: false, - onBeforeSetField: true, - onAfterSetField: false, - onBeforeDeleteRecord: false, - onAfterDeleteRecord: false - }), - uint8(0x04), - "0b00000100" - ); - - assertEq( - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: false, - onAfterSetRecord: false, - onBeforeSetField: false, - onAfterSetField: true, - onBeforeDeleteRecord: false, - onAfterDeleteRecord: false - }), - uint8(0x08), - "0b00001000" - ); - - assertEq( - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: false, - onAfterSetRecord: false, - onBeforeSetField: false, - onAfterSetField: false, - onBeforeDeleteRecord: true, - onAfterDeleteRecord: false - }), - uint8(0x10), - "0b00010000" - ); + assertEq(BEFORE_SET_RECORD, uint8(0x01), "0b00000001"); + assertEq(AFTER_SET_RECORD, uint8(0x02), "0b00000010"); + assertEq(BEFORE_SET_FIELD, uint8(0x04), "0b00000100"); + assertEq(AFTER_SET_FIELD, uint8(0x08), "0b00001000"); + assertEq(BEFORE_DELETE_RECORD, uint8(0x10), "0b00010000"); + assertEq(AFTER_DELETE_RECORD, uint8(0x20), "0b00100000"); assertEq( - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: false, - onAfterSetRecord: false, - onBeforeSetField: false, - onAfterSetField: false, - onBeforeDeleteRecord: false, - onAfterDeleteRecord: true - }), - uint8(0x20), - "0b00100000" - ); - - assertEq( - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: true, - onAfterSetRecord: true, - onBeforeSetField: true, - onAfterSetField: true, - onBeforeDeleteRecord: true, - onAfterDeleteRecord: true - }), + BEFORE_SET_RECORD | + AFTER_SET_RECORD | + BEFORE_SET_FIELD | + AFTER_SET_FIELD | + BEFORE_DELETE_RECORD | + AFTER_DELETE_RECORD, uint8(0x3f), "0b00111111" ); @@ -137,16 +50,14 @@ contract StoreHookTest is Test, GasReporter { function testEncode() public { assertEq( Hook.unwrap( - StoreHookLib.encode( - echoSubscriber, - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: true, - onAfterSetRecord: true, - onBeforeSetField: true, - onAfterSetField: true, - onBeforeDeleteRecord: true, - onAfterDeleteRecord: true - }) + HookLib.encode( + address(echoSubscriber), + BEFORE_SET_RECORD | + AFTER_SET_RECORD | + BEFORE_SET_FIELD | + AFTER_SET_FIELD | + BEFORE_DELETE_RECORD | + AFTER_DELETE_RECORD ) ), bytes21(abi.encodePacked(echoSubscriber, uint8(0x3f))) @@ -162,43 +73,33 @@ contract StoreHookTest is Test, GasReporter { bool enableBeforeDeleteRecord, bool enableAfterDeleteRecord ) public { - uint8 encodedBitmap = StoreHookLib.encodeBitmap({ - onBeforeSetRecord: enableBeforeSetRecord, - onAfterSetRecord: enableAfterSetRecord, - onBeforeSetField: enableBeforeSetField, - onAfterSetField: enableAfterSetField, - onBeforeDeleteRecord: enableBeforeDeleteRecord, - onAfterDeleteRecord: enableAfterDeleteRecord - }); + uint8 enabledHooks = 0; + if (enableBeforeSetRecord) enabledHooks |= BEFORE_SET_RECORD; + if (enableAfterSetRecord) enabledHooks |= AFTER_SET_RECORD; + if (enableBeforeSetField) enabledHooks |= BEFORE_SET_FIELD; + if (enableAfterSetField) enabledHooks |= AFTER_SET_FIELD; + if (enableBeforeDeleteRecord) enabledHooks |= BEFORE_DELETE_RECORD; + if (enableAfterDeleteRecord) enabledHooks |= AFTER_DELETE_RECORD; + assertEq( - Hook.unwrap(StoreHookLib.encode(IStoreHook(hookAddress), encodedBitmap)), - bytes21(abi.encodePacked(hookAddress, encodedBitmap)) + Hook.unwrap(HookLib.encode(hookAddress, enabledHooks)), + bytes21(abi.encodePacked(hookAddress, enabledHooks)) ); } function testIsEnabled() public { - Hook storeHook = StoreHookLib.encode( - echoSubscriber, - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: false, - onAfterSetRecord: false, - onBeforeSetField: true, - onAfterSetField: false, - onBeforeDeleteRecord: false, - onAfterDeleteRecord: false - }) - ); + Hook storeHook = HookLib.encode(address(echoSubscriber), BEFORE_SET_FIELD | AFTER_DELETE_RECORD); startGasReport("check if store hook is enabled"); - storeHook.isEnabled(uint8(StoreHookType.BEFORE_SET_RECORD)); + storeHook.isEnabled(BEFORE_SET_RECORD); endGasReport(); - assertEq(storeHook.isEnabled(uint8(StoreHookType.BEFORE_SET_RECORD)), false); - assertEq(storeHook.isEnabled(uint8(StoreHookType.AFTER_SET_RECORD)), false); - assertEq(storeHook.isEnabled(uint8(StoreHookType.BEFORE_SET_FIELD)), true); - assertEq(storeHook.isEnabled(uint8(StoreHookType.AFTER_SET_FIELD)), false); - assertEq(storeHook.isEnabled(uint8(StoreHookType.BEFORE_DELETE_RECORD)), false); - assertEq(storeHook.isEnabled(uint8(StoreHookType.AFTER_DELETE_RECORD)), false); + assertFalse(storeHook.isEnabled(BEFORE_SET_RECORD), "BEFORE_SET_RECORD"); + assertFalse(storeHook.isEnabled(AFTER_SET_RECORD), "AFTER_SET_RECORD"); + assertTrue(storeHook.isEnabled(BEFORE_SET_FIELD), "BEFORE_SET_FIELD"); + assertFalse(storeHook.isEnabled(AFTER_SET_FIELD), "AFTER_SET_FIELD"); + assertFalse(storeHook.isEnabled(BEFORE_DELETE_RECORD), "BEFORE_DELETE_RECORD"); + assertTrue(storeHook.isEnabled(AFTER_DELETE_RECORD), "AFTER_DELETE_RECORD"); } function testFuzzIsEnabled( @@ -210,38 +111,26 @@ contract StoreHookTest is Test, GasReporter { bool enableBeforeDeleteRecord, bool enableAfterDeleteRecord ) public { - Hook storeHook = StoreHookLib.encode( - IStoreHook(hookAddress), - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: enableBeforeSetRecord, - onAfterSetRecord: enableAfterSetRecord, - onBeforeSetField: enableBeforeSetField, - onAfterSetField: enableAfterSetField, - onBeforeDeleteRecord: enableBeforeDeleteRecord, - onAfterDeleteRecord: enableAfterDeleteRecord - }) - ); - - assertEq(storeHook.isEnabled(uint8(StoreHookType.BEFORE_SET_RECORD)), enableBeforeSetRecord); - assertEq(storeHook.isEnabled(uint8(StoreHookType.AFTER_SET_RECORD)), enableAfterSetRecord); - assertEq(storeHook.isEnabled(uint8(StoreHookType.BEFORE_SET_FIELD)), enableBeforeSetField); - assertEq(storeHook.isEnabled(uint8(StoreHookType.AFTER_SET_FIELD)), enableAfterSetField); - assertEq(storeHook.isEnabled(uint8(StoreHookType.BEFORE_DELETE_RECORD)), enableBeforeDeleteRecord); - assertEq(storeHook.isEnabled(uint8(StoreHookType.AFTER_DELETE_RECORD)), enableAfterDeleteRecord); + uint8 enabledHooks = 0; + if (enableBeforeSetRecord) enabledHooks |= BEFORE_SET_RECORD; + if (enableAfterSetRecord) enabledHooks |= AFTER_SET_RECORD; + if (enableBeforeSetField) enabledHooks |= BEFORE_SET_FIELD; + if (enableAfterSetField) enabledHooks |= AFTER_SET_FIELD; + if (enableBeforeDeleteRecord) enabledHooks |= BEFORE_DELETE_RECORD; + if (enableAfterDeleteRecord) enabledHooks |= AFTER_DELETE_RECORD; + + Hook storeHook = HookLib.encode(hookAddress, enabledHooks); + + assertEq(storeHook.isEnabled(BEFORE_SET_RECORD), enableBeforeSetRecord); + assertEq(storeHook.isEnabled(AFTER_SET_RECORD), enableAfterSetRecord); + assertEq(storeHook.isEnabled(BEFORE_SET_FIELD), enableBeforeSetField); + assertEq(storeHook.isEnabled(AFTER_SET_FIELD), enableAfterSetField); + assertEq(storeHook.isEnabled(BEFORE_DELETE_RECORD), enableBeforeDeleteRecord); + assertEq(storeHook.isEnabled(AFTER_DELETE_RECORD), enableAfterDeleteRecord); } function testGetAddress() public { - Hook storeHook = StoreHookLib.encode( - echoSubscriber, - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: false, - onAfterSetRecord: false, - onBeforeSetField: true, - onAfterSetField: false, - onBeforeDeleteRecord: false, - onAfterDeleteRecord: false - }) - ); + Hook storeHook = HookLib.encode(address(echoSubscriber), BEFORE_SET_FIELD); startGasReport("get store hook address"); storeHook.getAddress(); @@ -250,37 +139,8 @@ contract StoreHookTest is Test, GasReporter { assertEq(storeHook.getAddress(), address(echoSubscriber)); } - function testGetBitmap() public { - uint8 encodedBitmap = StoreHookLib.encodeBitmap({ - onBeforeSetRecord: false, - onAfterSetRecord: false, - onBeforeSetField: true, - onAfterSetField: false, - onBeforeDeleteRecord: false, - onAfterDeleteRecord: false - }); - - Hook storeHook = StoreHookLib.encode(echoSubscriber, encodedBitmap); - - startGasReport("get store hook bitmap"); - storeHook.getBitmap(); - endGasReport(); - - assertEq(storeHook.getBitmap(), encodedBitmap); - } - function testCallHook() public { - Hook storeHook = StoreHookLib.encode( - echoSubscriber, - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: true, - onAfterSetRecord: false, - onBeforeSetField: false, - onAfterSetField: false, - onBeforeDeleteRecord: false, - onAfterDeleteRecord: false - }) - ); + Hook storeHook = HookLib.encode(address(echoSubscriber), BEFORE_SET_RECORD); // TODO temporary variable until https://github.com/foundry-rs/foundry/issues/5811 is fixed bytes memory emptyDynamicData = new bytes(0); @@ -288,7 +148,7 @@ contract StoreHookTest is Test, GasReporter { vm.expectEmit(true, true, true, true); emit HookCalled(abi.encode(tableId, key, staticData, encodedLengths, emptyDynamicData, fieldLayout)); startGasReport("call an enabled hook"); - if (storeHook.isEnabled(uint8(StoreHookType.BEFORE_SET_RECORD))) { + if (storeHook.isEnabled(BEFORE_SET_RECORD)) { IStoreHook(storeHook.getAddress()).onBeforeSetRecord( tableId, key, @@ -300,21 +160,11 @@ contract StoreHookTest is Test, GasReporter { } endGasReport(); - Hook revertHook = StoreHookLib.encode( - revertSubscriber, - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: false, - onAfterSetRecord: false, - onBeforeSetField: false, - onAfterSetField: false, - onBeforeDeleteRecord: false, - onAfterDeleteRecord: false - }) - ); + Hook revertHook = HookLib.encode(address(revertSubscriber), 0); // Expect the to not be called - otherwise the test will fail with a revert startGasReport("call a disabled hook"); - if (revertHook.isEnabled(uint8(StoreHookType.BEFORE_SET_RECORD))) { + if (revertHook.isEnabled(BEFORE_SET_RECORD)) { IStoreHook(revertHook.getAddress()).onBeforeSetRecord( tableId, key, diff --git a/packages/world/gas-report.json b/packages/world/gas-report.json index cb6c335b40..24a569c163 100644 --- a/packages/world/gas-report.json +++ b/packages/world/gas-report.json @@ -39,67 +39,67 @@ "file": "test/KeysInTableModule.t.sol", "test": "testInstallComposite", "name": "install keys in table module", - "gasUsed": 1415308 + "gasUsed": 1415019 }, { "file": "test/KeysInTableModule.t.sol", "test": "testInstallGas", "name": "install keys in table module", - "gasUsed": 1415308 + "gasUsed": 1415019 }, { "file": "test/KeysInTableModule.t.sol", "test": "testInstallGas", "name": "set a record on a table with keysInTableModule installed", - "gasUsed": 158915 + "gasUsed": 158881 }, { "file": "test/KeysInTableModule.t.sol", "test": "testInstallSingleton", "name": "install keys in table module", - "gasUsed": 1415308 + "gasUsed": 1415019 }, { "file": "test/KeysInTableModule.t.sol", "test": "testSetAndDeleteRecordHookCompositeGas", "name": "install keys in table module", - "gasUsed": 1415308 + "gasUsed": 1415019 }, { "file": "test/KeysInTableModule.t.sol", "test": "testSetAndDeleteRecordHookCompositeGas", "name": "change a composite record on a table with keysInTableModule installed", - "gasUsed": 21994 + "gasUsed": 21960 }, { "file": "test/KeysInTableModule.t.sol", "test": "testSetAndDeleteRecordHookCompositeGas", "name": "delete a composite record on a table with keysInTableModule installed", - "gasUsed": 171498 + "gasUsed": 171464 }, { "file": "test/KeysInTableModule.t.sol", "test": "testSetAndDeleteRecordHookGas", "name": "install keys in table module", - "gasUsed": 1415308 + "gasUsed": 1415019 }, { "file": "test/KeysInTableModule.t.sol", "test": "testSetAndDeleteRecordHookGas", "name": "change a record on a table with keysInTableModule installed", - "gasUsed": 20716 + "gasUsed": 20682 }, { "file": "test/KeysInTableModule.t.sol", "test": "testSetAndDeleteRecordHookGas", "name": "delete a record on a table with keysInTableModule installed", - "gasUsed": 88149 + "gasUsed": 88115 }, { "file": "test/KeysWithValueModule.t.sol", "test": "testGetKeysWithValueGas", "name": "install keys with value module", - "gasUsed": 654598 + "gasUsed": 654303 }, { "file": "test/KeysWithValueModule.t.sol", @@ -117,49 +117,49 @@ "file": "test/KeysWithValueModule.t.sol", "test": "testInstall", "name": "install keys with value module", - "gasUsed": 654598 + "gasUsed": 654303 }, { "file": "test/KeysWithValueModule.t.sol", "test": "testInstall", "name": "set a record on a table with KeysWithValueModule installed", - "gasUsed": 134451 + "gasUsed": 134417 }, { "file": "test/KeysWithValueModule.t.sol", "test": "testSetAndDeleteRecordHook", "name": "install keys with value module", - "gasUsed": 654598 + "gasUsed": 654303 }, { "file": "test/KeysWithValueModule.t.sol", "test": "testSetAndDeleteRecordHook", "name": "change a record on a table with KeysWithValueModule installed", - "gasUsed": 104026 + "gasUsed": 103992 }, { "file": "test/KeysWithValueModule.t.sol", "test": "testSetAndDeleteRecordHook", "name": "delete a record on a table with KeysWithValueModule installed", - "gasUsed": 32905 + "gasUsed": 32871 }, { "file": "test/KeysWithValueModule.t.sol", "test": "testSetField", "name": "install keys with value module", - "gasUsed": 654598 + "gasUsed": 654303 }, { "file": "test/KeysWithValueModule.t.sol", "test": "testSetField", "name": "set a field on a table with KeysWithValueModule installed", - "gasUsed": 139065 + "gasUsed": 139031 }, { "file": "test/KeysWithValueModule.t.sol", "test": "testSetField", "name": "change a field on a table with KeysWithValueModule installed", - "gasUsed": 103824 + "gasUsed": 103790 }, { "file": "test/query.t.sol", diff --git a/packages/world/src/SystemCall.sol b/packages/world/src/SystemCall.sol index f2863b00c4..e6ec08e28f 100644 --- a/packages/world/src/SystemCall.sol +++ b/packages/world/src/SystemCall.sol @@ -10,7 +10,7 @@ import { ResourceSelector } from "./ResourceSelector.sol"; import { ROOT_NAMESPACE } from "./constants.sol"; import { WorldContextProvider } from "./WorldContext.sol"; import { revertWithBytes } from "./revertWithBytes.sol"; -import { SystemHookType } from "./SystemHook.sol"; +import { BEFORE_CALL_SYSTEM, AFTER_CALL_SYSTEM } from "./systemHookTypes.sol"; import { IWorldErrors } from "./interfaces/IWorldErrors.sol"; import { ISystemHook } from "./interfaces/ISystemHook.sol"; @@ -81,7 +81,7 @@ library SystemCall { // Call onBeforeCallSystem hooks (before calling the system) for (uint256 i; i < hooks.length; i++) { Hook hook = Hook.wrap(hooks[i]); - if (hook.isEnabled(uint8(SystemHookType.BEFORE_CALL_SYSTEM))) { + if (hook.isEnabled(BEFORE_CALL_SYSTEM)) { ISystemHook(hook.getAddress()).onBeforeCallSystem(caller, resourceSelector, funcSelectorAndArgs); } } @@ -97,7 +97,7 @@ library SystemCall { // Call onAfterCallSystem hooks (after calling the system) for (uint256 i; i < hooks.length; i++) { Hook hook = Hook.wrap(hooks[i]); - if (hook.isEnabled(uint8(SystemHookType.AFTER_CALL_SYSTEM))) { + if (hook.isEnabled(AFTER_CALL_SYSTEM)) { ISystemHook(hook.getAddress()).onAfterCallSystem(caller, resourceSelector, funcSelectorAndArgs); } } diff --git a/packages/world/src/SystemHook.sol b/packages/world/src/SystemHook.sol index f663b9b8f7..ef8af9587c 100644 --- a/packages/world/src/SystemHook.sol +++ b/packages/world/src/SystemHook.sol @@ -1,34 +1,9 @@ // SPDX-License-Identifier: MIT pragma solidity >=0.8.0; -import { Hook, HookLib } from "@latticexyz/store/src/Hook.sol"; import { ISystemHook, SYSTEM_HOOK_INTERFACE_ID } from "./interfaces/ISystemHook.sol"; import { ERC165_INTERFACE_ID } from "./interfaces/IERC165.sol"; -enum SystemHookType { - BEFORE_CALL_SYSTEM, - AFTER_CALL_SYSTEM -} - -library SystemHookLib { - /** - * Encode the bitmap into a single byte - */ - function encodeBitmap(bool onBeforeCallSystem, bool onAfterCallSystem) internal pure returns (uint8) { - uint256 bitmap = 0; - if (onBeforeCallSystem) bitmap |= 1 << uint8(SystemHookType.BEFORE_CALL_SYSTEM); - if (onAfterCallSystem) bitmap |= 1 << uint8(SystemHookType.AFTER_CALL_SYSTEM); - return uint8(bitmap); - } - - /** - * Encode enabled hooks into a bitmap with 1 bit per hook, and pack the bitmap with the system hook address into a bytes21 value - */ - function encode(ISystemHook systemHook, uint8 enabledHooksBitmap) internal pure returns (Hook) { - return HookLib.encode(address(systemHook), enabledHooksBitmap); - } -} - abstract contract SystemHook is ISystemHook { // ERC-165 supportsInterface (see https://eips.ethereum.org/EIPS/eip-165) function supportsInterface(bytes4 interfaceId) public view virtual returns (bool) { diff --git a/packages/world/src/modules/core/implementations/WorldRegistrationSystem.sol b/packages/world/src/modules/core/implementations/WorldRegistrationSystem.sol index 3502e5bd6c..03cd41a7ec 100644 --- a/packages/world/src/modules/core/implementations/WorldRegistrationSystem.sol +++ b/packages/world/src/modules/core/implementations/WorldRegistrationSystem.sol @@ -8,7 +8,6 @@ import { WorldContextConsumer, WORLD_CONTEXT_CONSUMER_INTERFACE_ID } from "../.. import { ResourceSelector } from "../../../ResourceSelector.sol"; import { Resource } from "../../../Types.sol"; import { SystemCall } from "../../../SystemCall.sol"; -import { SystemHookLib } from "../../../SystemHook.sol"; import { ROOT_NAMESPACE, ROOT_NAME, UNLIMITED_DELEGATION } from "../../../constants.sol"; import { AccessControl } from "../../../AccessControl.sol"; import { requireInterface } from "../../../requireInterface.sol"; @@ -66,7 +65,7 @@ contract WorldRegistrationSystem is System, IWorldErrors { AccessControl.requireOwner(resourceSelector, _msgSender()); // Register the hook - SystemHooks.push(resourceSelector, Hook.unwrap(SystemHookLib.encode(hookAddress, enabledHooksBitmap))); + SystemHooks.push(resourceSelector, Hook.unwrap(HookLib.encode(address(hookAddress), enabledHooksBitmap))); } /** diff --git a/packages/world/src/modules/keysintable/KeysInTableModule.sol b/packages/world/src/modules/keysintable/KeysInTableModule.sol index f04834d110..b5c64cb8af 100644 --- a/packages/world/src/modules/keysintable/KeysInTableModule.sol +++ b/packages/world/src/modules/keysintable/KeysInTableModule.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity >=0.8.0; -import { StoreHookLib } from "@latticexyz/store/src/StoreHook.sol"; +import { BEFORE_SET_RECORD, AFTER_SET_FIELD, BEFORE_DELETE_RECORD } from "@latticexyz/store/src/storeHookTypes.sol"; import { ResourceType } from "../core/tables/ResourceType.sol"; import { Resource } from "../../Types.sol"; @@ -95,18 +95,7 @@ contract KeysInTableModule is Module { (success, returnData) = address(world).delegatecall( abi.encodeCall( world.registerStoreHook, - ( - sourceTableId, - hook, - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: true, - onAfterSetRecord: false, - onBeforeSetField: false, - onAfterSetField: true, - onBeforeDeleteRecord: true, - onAfterDeleteRecord: false - }) - ) + (sourceTableId, hook, BEFORE_SET_RECORD | AFTER_SET_FIELD | BEFORE_DELETE_RECORD) ) ); } diff --git a/packages/world/src/modules/keyswithvalue/KeysWithValueModule.sol b/packages/world/src/modules/keyswithvalue/KeysWithValueModule.sol index a2b3c6a388..453450cfaf 100644 --- a/packages/world/src/modules/keyswithvalue/KeysWithValueModule.sol +++ b/packages/world/src/modules/keyswithvalue/KeysWithValueModule.sol @@ -2,7 +2,7 @@ pragma solidity >=0.8.0; import { StoreSwitch } from "@latticexyz/store/src/StoreSwitch.sol"; -import { StoreHookLib } from "@latticexyz/store/src/StoreHook.sol"; +import { BEFORE_SET_RECORD, BEFORE_SET_FIELD, AFTER_SET_FIELD, BEFORE_DELETE_RECORD } from "@latticexyz/store/src/storeHookTypes.sol"; import { Module } from "../../Module.sol"; import { IBaseWorld } from "../../interfaces/IBaseWorld.sol"; @@ -55,18 +55,7 @@ contract KeysWithValueModule is Module { (bool success, bytes memory returnData) = address(world).delegatecall( abi.encodeCall( world.registerStoreHook, - ( - sourceTableId, - hook, - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: true, - onAfterSetRecord: false, - onBeforeSetField: true, - onAfterSetField: true, - onBeforeDeleteRecord: true, - onAfterDeleteRecord: false - }) - ) + (sourceTableId, hook, BEFORE_SET_RECORD | BEFORE_SET_FIELD | AFTER_SET_FIELD | BEFORE_DELETE_RECORD) ) ); if (!success) revertWithBytes(returnData); diff --git a/packages/world/src/systemHookTypes.sol b/packages/world/src/systemHookTypes.sol new file mode 100644 index 0000000000..0f578dfed5 --- /dev/null +++ b/packages/world/src/systemHookTypes.sol @@ -0,0 +1,5 @@ +// SPDX-License-Identifier: MIT +pragma solidity >=0.8.0; + +uint8 constant BEFORE_CALL_SYSTEM = 1 << 0; +uint8 constant AFTER_CALL_SYSTEM = 1 << 1; diff --git a/packages/world/test/SystemHook.t.sol b/packages/world/test/SystemHook.t.sol index b1d423be0f..bcc9f66784 100644 --- a/packages/world/test/SystemHook.t.sol +++ b/packages/world/test/SystemHook.t.sol @@ -4,60 +4,31 @@ pragma solidity >=0.8.0; import "forge-std/Test.sol"; import { GasReporter } from "@latticexyz/gas-report/src/GasReporter.sol"; -import { Hook } from "@latticexyz/store/src/Hook.sol"; -import { SystemHookType } from "../src/SystemHook.sol"; -import { SystemHookLib } from "../src/SystemHook.sol"; +import { Hook, HookLib } from "@latticexyz/store/src/Hook.sol"; +import { BEFORE_CALL_SYSTEM, AFTER_CALL_SYSTEM } from "../src/systemHookTypes.sol"; import { ISystemHook } from "../src/interfaces/ISystemHook.sol"; contract SystemHookTest is Test, GasReporter { - function testEncodeBitmap() public { - assertEq( - SystemHookLib.encodeBitmap({ onBeforeCallSystem: false, onAfterCallSystem: false }), - uint8(0x00), - "0b00000000" - ); - - assertEq( - SystemHookLib.encodeBitmap({ onBeforeCallSystem: true, onAfterCallSystem: false }), - uint8(0x01), - "0b00000001" - ); - - assertEq( - SystemHookLib.encodeBitmap({ onBeforeCallSystem: false, onAfterCallSystem: true }), - uint8(0x02), - "0b00000010" - ); - - assertEq( - SystemHookLib.encodeBitmap({ onBeforeCallSystem: true, onAfterCallSystem: true }), - uint8(0x03), - "0b00000011" - ); - } - function testFuzzEncode(address hookAddress, bool enableBeforeCallSystem, bool enableAfterCallSystem) public { - uint8 enabledHooksBitmap = SystemHookLib.encodeBitmap({ - onBeforeCallSystem: enableBeforeCallSystem, - onAfterCallSystem: enableAfterCallSystem - }); + uint8 enabledHooksBitmap = 0; + if (enableBeforeCallSystem) enabledHooksBitmap |= BEFORE_CALL_SYSTEM; + if (enableAfterCallSystem) enabledHooksBitmap |= AFTER_CALL_SYSTEM; assertEq( - Hook.unwrap(SystemHookLib.encode(ISystemHook(hookAddress), enabledHooksBitmap)), + Hook.unwrap(HookLib.encode(hookAddress, enabledHooksBitmap)), bytes21(abi.encodePacked(hookAddress, enabledHooksBitmap)) ); } function testFuzzIsEnabled(address hookAddress, bool enableBeforeCallSystem, bool enableAfterCallSystem) public { - uint8 enabledHooksBitmap = SystemHookLib.encodeBitmap({ - onBeforeCallSystem: enableBeforeCallSystem, - onAfterCallSystem: enableAfterCallSystem - }); + uint8 enabledHooksBitmap = 0; + if (enableBeforeCallSystem) enabledHooksBitmap |= BEFORE_CALL_SYSTEM; + if (enableAfterCallSystem) enabledHooksBitmap |= AFTER_CALL_SYSTEM; - Hook systemHook = SystemHookLib.encode(ISystemHook(hookAddress), enabledHooksBitmap); + Hook systemHook = HookLib.encode(hookAddress, enabledHooksBitmap); - assertEq(systemHook.isEnabled(uint8(SystemHookType.BEFORE_CALL_SYSTEM)), enableBeforeCallSystem); - assertEq(systemHook.isEnabled(uint8(SystemHookType.AFTER_CALL_SYSTEM)), enableAfterCallSystem); + assertEq(systemHook.isEnabled(BEFORE_CALL_SYSTEM), enableBeforeCallSystem); + assertEq(systemHook.isEnabled(AFTER_CALL_SYSTEM), enableAfterCallSystem); } function testFuzzGetAddressAndBitmap( @@ -65,12 +36,11 @@ contract SystemHookTest is Test, GasReporter { bool enableBeforeCallSystem, bool enableAfterCallSystem ) public { - uint8 enabledHooksBitmap = SystemHookLib.encodeBitmap({ - onBeforeCallSystem: enableBeforeCallSystem, - onAfterCallSystem: enableAfterCallSystem - }); + uint8 enabledHooksBitmap = 0; + if (enableBeforeCallSystem) enabledHooksBitmap |= BEFORE_CALL_SYSTEM; + if (enableAfterCallSystem) enabledHooksBitmap |= AFTER_CALL_SYSTEM; - Hook systemHook = SystemHookLib.encode(ISystemHook(hookAddress), enabledHooksBitmap); + Hook systemHook = HookLib.encode(hookAddress, enabledHooksBitmap); assertEq(systemHook.getAddress(), hookAddress); assertEq(systemHook.getBitmap(), enabledHooksBitmap); diff --git a/packages/world/test/World.t.sol b/packages/world/test/World.t.sol index 6a3406fe63..e1129733e0 100644 --- a/packages/world/test/World.t.sol +++ b/packages/world/test/World.t.sol @@ -16,7 +16,7 @@ import { PackedCounter } from "@latticexyz/store/src/PackedCounter.sol"; import { SchemaEncodeHelper } from "@latticexyz/store/test/SchemaEncodeHelper.sol"; import { Tables, TablesTableId } from "@latticexyz/store/src/codegen/Tables.sol"; import { EncodeArray } from "@latticexyz/store/src/tightcoder/EncodeArray.sol"; -import { StoreHookLib } from "@latticexyz/store/src/StoreHook.sol"; +import { BEFORE_SET_RECORD, AFTER_SET_RECORD, BEFORE_SET_FIELD, AFTER_SET_FIELD, BEFORE_DELETE_RECORD, AFTER_DELETE_RECORD } from "@latticexyz/store/src/storeHookTypes.sol"; import { RevertSubscriber } from "@latticexyz/store/test/RevertSubscriber.sol"; import { EchoSubscriber } from "@latticexyz/store/test/EchoSubscriber.sol"; @@ -27,7 +27,8 @@ import { ResourceSelector } from "../src/ResourceSelector.sol"; import { ROOT_NAMESPACE, ROOT_NAME, UNLIMITED_DELEGATION } from "../src/constants.sol"; import { Resource } from "../src/Types.sol"; import { WorldContextProvider, WORLD_CONTEXT_CONSUMER_INTERFACE_ID } from "../src/WorldContext.sol"; -import { SystemHookLib, SystemHook } from "../src/SystemHook.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"; import { NamespaceOwner, NamespaceOwnerTableId } from "../src/tables/NamespaceOwner.sol"; @@ -842,14 +843,12 @@ contract WorldTest is Test, GasReporter { world.registerStoreHook( tableId, tableHook, - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: true, - onAfterSetRecord: true, - onBeforeSetField: true, - onAfterSetField: true, - onBeforeDeleteRecord: true, - onAfterDeleteRecord: true - }) + BEFORE_SET_RECORD | + AFTER_SET_RECORD | + BEFORE_SET_FIELD | + AFTER_SET_FIELD | + BEFORE_DELETE_RECORD | + AFTER_DELETE_RECORD ); // Prepare data to write to the table @@ -893,14 +892,12 @@ contract WorldTest is Test, GasReporter { world.registerStoreHook( tableId, IStoreHook(address(world)), // the World contract does not implement the store hook interface - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: true, - onAfterSetRecord: true, - onBeforeSetField: true, - onAfterSetField: true, - onBeforeDeleteRecord: true, - onAfterDeleteRecord: true - }) + BEFORE_SET_RECORD | + AFTER_SET_RECORD | + BEFORE_SET_FIELD | + AFTER_SET_FIELD | + BEFORE_DELETE_RECORD | + AFTER_DELETE_RECORD ); } @@ -917,28 +914,24 @@ contract WorldTest is Test, GasReporter { world.registerStoreHook( tableId, revertSubscriber, - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: true, - onAfterSetRecord: true, - onBeforeSetField: true, - onAfterSetField: true, - onBeforeDeleteRecord: true, - onAfterDeleteRecord: true - }) + BEFORE_SET_RECORD | + AFTER_SET_RECORD | + BEFORE_SET_FIELD | + AFTER_SET_FIELD | + BEFORE_DELETE_RECORD | + AFTER_DELETE_RECORD ); // Register a new EchoSubscriber IStoreHook echoSubscriber = new EchoSubscriber(); world.registerStoreHook( tableId, echoSubscriber, - StoreHookLib.encodeBitmap({ - onBeforeSetRecord: true, - onAfterSetRecord: true, - onBeforeSetField: true, - onAfterSetField: true, - onBeforeDeleteRecord: true, - onAfterDeleteRecord: true - }) + BEFORE_SET_RECORD | + AFTER_SET_RECORD | + BEFORE_SET_FIELD | + AFTER_SET_FIELD | + BEFORE_DELETE_RECORD | + AFTER_DELETE_RECORD ); // Prepare data to write to the table @@ -1005,16 +998,12 @@ contract WorldTest is Test, GasReporter { world.registerSystemHook( systemId, ISystemHook(address(world)), // the World contract does not implement the system hook interface - SystemHookLib.encodeBitmap({ onBeforeCallSystem: true, onAfterCallSystem: true }) + BEFORE_CALL_SYSTEM | AFTER_CALL_SYSTEM ); // Register a new hook ISystemHook systemHook = new EchoSystemHook(); - world.registerSystemHook( - systemId, - systemHook, - SystemHookLib.encodeBitmap({ onBeforeCallSystem: true, onAfterCallSystem: true }) - ); + world.registerSystemHook(systemId, systemHook, BEFORE_CALL_SYSTEM | AFTER_CALL_SYSTEM); bytes memory funcSelectorAndArgs = abi.encodeWithSelector(bytes4(keccak256("fallbackselector"))); @@ -1041,19 +1030,11 @@ contract WorldTest is Test, GasReporter { // Register a new RevertSystemHook ISystemHook revertSystemHook = new RevertSystemHook(); - world.registerSystemHook( - systemId, - revertSystemHook, - SystemHookLib.encodeBitmap({ onBeforeCallSystem: true, onAfterCallSystem: true }) - ); + world.registerSystemHook(systemId, revertSystemHook, BEFORE_CALL_SYSTEM | AFTER_CALL_SYSTEM); // Register a new EchoSystemHook ISystemHook echoSystemHook = new EchoSystemHook(); - world.registerSystemHook( - systemId, - echoSystemHook, - SystemHookLib.encodeBitmap({ onBeforeCallSystem: true, onAfterCallSystem: true }) - ); + world.registerSystemHook(systemId, echoSystemHook, BEFORE_CALL_SYSTEM | AFTER_CALL_SYSTEM); bytes memory funcSelectorAndArgs = abi.encodeWithSelector(bytes4(keccak256("fallbackselector")));