Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(store,world): move hooks to bit flags #1527

Merged
merged 3 commits into from
Sep 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions .changeset/few-jars-turn.md
Original file line number Diff line number Diff line change
@@ -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
);

```
30 changes: 12 additions & 18 deletions packages/store/gas-report.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -891,31 +891,25 @@
"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",
"test": "testGetAddress",
"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",
Expand Down
5 changes: 2 additions & 3 deletions packages/store/src/Hook.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Comment on lines -55 to +56
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✨✨✨ so much nicer

}

/**
Expand Down
28 changes: 14 additions & 14 deletions packages/store/src/StoreCore.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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)));
}

/**
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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);
}
}
Expand All @@ -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);
}
}
Expand All @@ -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);
}
}
Expand All @@ -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);
}
}
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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);
}
}
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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);
}
}
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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);
}
}
Expand Down
40 changes: 0 additions & 40 deletions packages/store/src/StoreHook.sol
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down
10 changes: 10 additions & 0 deletions packages/store/src/storeHookTypes.sol
Original file line number Diff line number Diff line change
@@ -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?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cross posting here from discord for context: the "set field" hook is soon replaced with a "splice" hook (see #1510)

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;
Loading