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): use user-types for ResourceId, FieldLayout and Schema in table libraries #1586

Merged
merged 15 commits into from
Sep 24, 2023
Merged
7 changes: 7 additions & 0 deletions .changeset/seven-mangos-roll.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@latticexyz/store-sync": patch
"@latticexyz/store": patch
"@latticexyz/world": patch
---

All `Store` and `World` tables now use the appropriate user-types for `ResourceId`, `FieldLayout` and `Schema` to avoid manual `wrap`/`unwrap`.
7 changes: 7 additions & 0 deletions .changeset/thin-chairs-compare.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@latticexyz/cli": patch
"@latticexyz/common": patch
"@latticexyz/store": major
---

Changed the `userTypes` property to accept `{ filePath: string, internalType: SchemaAbiType }` to enable strong inference of types from the config.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ contract ChatNamespacedTest is MudTest {
MessageTableTableId,
keyTuple,
new bytes(0),
MessageTable.encodeLengths(value).unwrap(),
MessageTable.encodeLengths(value),
MessageTable.encodeDynamic(value)
);
IChatNamespacedSystem(worldAddress).namespace_ChatNamespaced_sendMessage(value);
Expand Down
10 changes: 5 additions & 5 deletions packages/cli/scripts/generate-test-tables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,11 @@ try {
},

userTypes: {
TestTypeAddress: "./contracts/src/types.sol",
TestTypeInt64: "./contracts/src/types.sol",
TestTypeBool: "./contracts/src/types.sol",
TestTypeUint128: "./contracts/src/types.sol",
ResourceId: "@latticexyz/store/src/ResourceId.sol",
TestTypeAddress: { filePath: "./contracts/src/types.sol", internalType: "address" },
TestTypeInt64: { filePath: "./contracts/src/types.sol", internalType: "int64" },
TestTypeBool: { filePath: "./contracts/src/types.sol", internalType: "bool" },
TestTypeUint128: { filePath: "./contracts/src/types.sol", internalType: "uint128" },
ResourceId: { filePath: "@latticexyz/store/src/ResourceId.sol", internalType: "bytes32" },
},
});
} catch (error: unknown) {
Expand Down
23 changes: 20 additions & 3 deletions packages/common/src/codegen/utils/loadUserTypesFile.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
import { readFileSync } from "fs";
import path from "path";
import { SolidityUserDefinedType, extractUserTypes } from "./extractUserTypes";
import { MUDError } from "../../errors";

export type UserType = {
filePath: string;
internalType: string;
};

export function loadAndExtractUserTypes(
userTypes: Record<string, string>,
userTypes: Record<string, UserType>,
outputBaseDirectory: string,
remappings: [string, string][]
): Record<string, SolidityUserDefinedType> {
const userTypesPerFile: Record<string, string[]> = {};
for (const [userTypeName, unresolvedFilePath] of Object.entries(userTypes)) {
for (const [userTypeName, { filePath: unresolvedFilePath }] of Object.entries(userTypes)) {
if (!(unresolvedFilePath in userTypesPerFile)) {
userTypesPerFile[unresolvedFilePath] = [];
}
Expand All @@ -17,7 +23,18 @@ export function loadAndExtractUserTypes(
let extractedUserTypes: Record<string, SolidityUserDefinedType> = {};
for (const [unresolvedFilePath, userTypeNames] of Object.entries(userTypesPerFile)) {
const { filePath, data } = loadUserTypesFile(outputBaseDirectory, unresolvedFilePath, remappings);
extractedUserTypes = Object.assign(userTypes, extractUserTypes(data, userTypeNames, filePath));
const userTypesInFile = extractUserTypes(data, userTypeNames, filePath);

// Verify the actual user type matches the internalType specified in the config
for (const [userTypeName, userType] of Object.entries(userTypesInFile)) {
if (userType.internalTypeId !== userTypes[userTypeName].internalType) {
throw new MUDError(
`User type "${userTypeName}" has internal type "${userType.internalTypeId}" but config specifies "${userTypes[userTypeName].internalType}"`
);
}
}

extractedUserTypes = Object.assign(extractedUserTypes, userTypesInFile);
}
return extractedUserTypes;
}
Expand Down
9 changes: 7 additions & 2 deletions packages/store-sync/src/common.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Address, Block, Hex, Log, PublicClient } from "viem";
import { StoreConfig, StoreEventsAbiItem, StoreEventsAbi } from "@latticexyz/store";
import { StoreConfig, StoreEventsAbiItem, StoreEventsAbi, storeEvents, resolveUserTypes } from "@latticexyz/store";
import storeConfig from "@latticexyz/store/mud.config";
import { Observable } from "rxjs";
import { resourceIdToHex } from "@latticexyz/common";
Expand Down Expand Up @@ -76,7 +76,12 @@ export type StorageAdapterBlock = { blockNumber: BlockLogs["blockNumber"]; logs:
export type StorageAdapter = (block: StorageAdapterBlock) => Promise<void>;

// TODO: adjust when we get namespace support (https://github.com/latticexyz/mud/issues/994) and when table has namespace key (https://github.com/latticexyz/mud/issues/1201)
export const schemasTable = storeConfig.tables.Tables;
// TODO: adjust when schemas are automatically resolved
export const schemasTable = {
...storeConfig.tables.Tables,
valueSchema: resolveUserTypes(storeConfig.tables.Tables.valueSchema, storeConfig.userTypes),
};

Comment on lines +79 to +84
Copy link
Member Author

Choose a reason for hiding this comment

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

kind of annoying, but we need this workaround for now to land the contract changes (until we have a better way to resolve the user type config with strong types)

Copy link
Member

@holic holic Sep 24, 2023

Choose a reason for hiding this comment

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

hopefully coming in #1561 (but still a lot of work left to do)

export const schemasTableId = resourceIdToHex({
type: schemasTable.offchainOnly ? "offchainTable" : "table",
namespace: storeConfig.namespace,
Expand Down
21 changes: 14 additions & 7 deletions packages/store-sync/src/logToTable.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { hexToSchema, decodeValue } from "@latticexyz/protocol-parser";
import { concatHex, decodeAbiParameters, parseAbiParameters } from "viem";
import { hexToSchema, decodeValue, ValueSchema } from "@latticexyz/protocol-parser";
import { Hex, concatHex, decodeAbiParameters, parseAbiParameters } from "viem";
import { StorageAdapterLog, Table, schemasTable } from "./common";
import { hexToResourceId } from "@latticexyz/common";

Expand All @@ -14,15 +14,22 @@ export function logToTable(log: StorageAdapterLog & { eventName: "Store_SetRecor
const table = hexToResourceId(tableId);

const value = decodeValue(
schemasTable.valueSchema,
// TODO: remove cast when we have strong types for user types
schemasTable.valueSchema as ValueSchema,
concatHex([log.args.staticData, log.args.encodedLengths, log.args.dynamicData])
);

const keySchema = hexToSchema(value.keySchema);
const valueSchema = hexToSchema(value.valueSchema);
// TODO: remove cast when we have strong types for user types
const keySchema = hexToSchema(value.keySchema as Hex);

const keyNames = decodeAbiParameters(parseAbiParameters("string[]"), value.abiEncodedKeyNames)[0];
const fieldNames = decodeAbiParameters(parseAbiParameters("string[]"), value.abiEncodedFieldNames)[0];
// TODO: remove cast when we have strong types for user types
const valueSchema = hexToSchema(value.valueSchema as Hex);

// TODO: remove cast when we have strong types for user types
const keyNames = decodeAbiParameters(parseAbiParameters("string[]"), value.abiEncodedKeyNames as Hex)[0];

// TODO: remove cast when we have strong types for user types
const fieldNames = decodeAbiParameters(parseAbiParameters("string[]"), value.abiEncodedFieldNames as Hex)[0];

const valueAbiTypes = [...valueSchema.staticFields, ...valueSchema.dynamicFields];

Expand Down
24 changes: 12 additions & 12 deletions packages/store/gas-report.json
Original file line number Diff line number Diff line change
Expand Up @@ -603,73 +603,73 @@
"file": "test/StoreCoreDynamic.t.sol",
"test": "testGetDynamicFieldSlice",
"name": "get field slice (cold, 1 slot)",
"gasUsed": 10240
"gasUsed": 5609
Copy link
Member Author

Choose a reason for hiding this comment

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

these gas changes come from not loading the _tableId and _keyTuple from storage in the gas test

Copy link
Member

Choose a reason for hiding this comment

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

same for all tests or just this one? I noticed some other gas improvements below

Copy link
Member Author

Choose a reason for hiding this comment

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

wrapping/unwrapping in itself should not significantly impact gas, so i think most of the gas variation in here comes from the changed gas tests

},
{
"file": "test/StoreCoreDynamic.t.sol",
"test": "testGetDynamicFieldSlice",
"name": "get field slice (warm, 1 slot)",
"gasUsed": 2308
"gasUsed": 1677
},
{
"file": "test/StoreCoreDynamic.t.sol",
"test": "testGetDynamicFieldSlice",
"name": "get field slice (semi-cold, 1 slot)",
"gasUsed": 4313
"gasUsed": 3680
},
{
"file": "test/StoreCoreDynamic.t.sol",
"test": "testGetDynamicFieldSlice",
"name": "get field slice (warm, 2 slots)",
"gasUsed": 4539
"gasUsed": 3907
},
{
"file": "test/StoreCoreDynamic.t.sol",
"test": "testGetSecondFieldLength",
"name": "get field length (cold, 1 slot)",
"gasUsed": 7795
"gasUsed": 3163
},
{
"file": "test/StoreCoreDynamic.t.sol",
"test": "testGetSecondFieldLength",
"name": "get field length (warm, 1 slot)",
"gasUsed": 1790
"gasUsed": 1160
},
{
"file": "test/StoreCoreDynamic.t.sol",
"test": "testGetThirdFieldLength",
"name": "get field length (warm due to , 2 slots)",
"gasUsed": 7794
"gasUsed": 3163
},
{
"file": "test/StoreCoreDynamic.t.sol",
"test": "testGetThirdFieldLength",
"name": "get field length (warm, 2 slots)",
"gasUsed": 1790
"gasUsed": 1160
},
{
"file": "test/StoreCoreDynamic.t.sol",
"test": "testPopFromSecondField",
"name": "pop from field (cold, 1 slot, 1 uint32 item)",
"gasUsed": 18728
"gasUsed": 18097
},
{
"file": "test/StoreCoreDynamic.t.sol",
"test": "testPopFromSecondField",
"name": "pop from field (warm, 1 slot, 1 uint32 item)",
"gasUsed": 12736
"gasUsed": 12104
},
{
"file": "test/StoreCoreDynamic.t.sol",
"test": "testPopFromThirdField",
"name": "pop from field (cold, 2 slots, 10 uint32 items)",
"gasUsed": 16496
"gasUsed": 15865
},
{
"file": "test/StoreCoreDynamic.t.sol",
"test": "testPopFromThirdField",
"name": "pop from field (warm, 2 slots, 10 uint32 items)",
"gasUsed": 12504
"gasUsed": 11872
},
{
"file": "test/StoreCoreGas.t.sol",
Expand Down
33 changes: 25 additions & 8 deletions packages/store/mud.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,35 +6,52 @@ export default mudConfig({
enums: {
ExampleEnum: ["None", "First", "Second", "Third"],
},
userTypes: {
ResourceId: { filePath: "./src/ResourceId.sol", internalType: "bytes32" },
FieldLayout: { filePath: "./src/FieldLayout.sol", internalType: "bytes32" },
Schema: { filePath: "./src/Schema.sol", internalType: "bytes32" },
},
tables: {
StoreHooks: "bytes21[]",
Callbacks: "bytes24[]",
StoreHooks: {
keySchema: {
tableId: "ResourceId",
},
valueSchema: {
hooks: "bytes21[]",
},
},
Tables: {
keySchema: {
tableId: "bytes32",
tableId: "ResourceId",
},
valueSchema: {
fieldLayout: "bytes32",
keySchema: "bytes32",
valueSchema: "bytes32",
fieldLayout: "FieldLayout",
keySchema: "Schema",
valueSchema: "Schema",
abiEncodedKeyNames: "bytes",
abiEncodedFieldNames: "bytes",
},
},
ResourceIds: {
keySchema: {
resourceId: "bytes32",
resourceId: "ResourceId",
},
valueSchema: {
exists: "bool",
},
},
// The Hooks table is a generic table used by the `filterFromList` util in `Hook.sol`
Hooks: {
valueSchema: "bytes21[]",
keySchema: {
resourceId: "ResourceId",
},
valueSchema: {
hooks: "bytes21[]",
},
tableIdArgument: true,
},
// TODO: move these test tables to a separate mud config
Callbacks: "bytes24[]",
Mixed: {
valueSchema: {
u32: "uint32",
Expand Down
4 changes: 2 additions & 2 deletions packages/store/src/Hook.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ library HookLib {
ResourceId tableWithHooks,
address hookAddressToRemove
) internal {
bytes21[] memory currentHooks = Hooks._get(hookTableId, ResourceId.unwrap(tableWithHooks));
bytes21[] memory currentHooks = Hooks._get(hookTableId, tableWithHooks);

// Initialize the new hooks array with the same length because we don't know if the hook is registered yet
bytes21[] memory newHooks = new bytes21[](currentHooks.length);
Expand All @@ -49,7 +49,7 @@ library HookLib {
}

// Set the new hooks table
Hooks._set(hookTableId, ResourceId.unwrap(tableWithHooks), newHooks);
Hooks._set(hookTableId, tableWithHooks, newHooks);
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/store/src/IStore.sol
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ interface IStoreWrite {
ResourceId indexed tableId,
bytes32[] keyTuple,
bytes staticData,
bytes32 encodedLengths,
PackedCounter encodedLengths,
bytes dynamicData
);
event Store_SpliceStaticData(ResourceId indexed tableId, bytes32[] keyTuple, uint48 start, bytes data);
Expand All @@ -133,7 +133,7 @@ interface IStoreWrite {
uint48 start,
uint40 deleteCount,
bytes data,
bytes32 encodedLengths
PackedCounter encodedLengths
);
event Store_DeleteRecord(ResourceId indexed tableId, bytes32[] keyTuple);

Expand Down
Loading