refactor: uint256 tableId -> bytes32 tableId #613

alvrs · 2023-04-12T13:06:59Z

We originally used bytes32 for tableIds, but switched to uint256 in refactor(store): change tableId type from bytes32 to uint256 #416 because bytes32 had a type overlap with string, which caused issues in the first version of the World contract (where we used strings for routes instead of the current namespace/name approach, see feat(world): simplify access control to namespaces instead of routes #467 for context on this change).
Now that we don't use strings for routes in the World anymore, there is no reason for tableId to be uint256 anymore, while resourceSelector in World is bytes32. A resourceSelector is a superset of a tableId (also including systems), and converting from bytes32 to uint256 whenever we use a resourceSelector as a tableId is unnecessary overhead.

alvrs · 2023-04-12T14:50:08Z

packages/services/pkg/mode/storecore/storecore.go

@@ -255,7 +255,7 @@ type StorecoreStoreDeleteRecord struct {



cc @authcall - I edited this file manually when changing all the references of uint256 table to bytes32 table, but the correct approach would be to regenerate this file. What's the script to do so?

alvrs · 2023-04-12T14:51:13Z

packages/store/mud.config.mts

@@ -8,7 +8,7 @@ export default storeConfig({
    Callbacks: "bytes24[]",
    StoreMetadata: {
      primaryKeys: {
-        tableId: "uint256",


cc @authcall / @holic: does any networking code on MODE/client hardcode the schema of the StoreMetadata table or is it inferred dynamically?

in client, StoreMetadata is inferred since its a proper table

alvrs · 2023-04-12T14:58:34Z

packages/store/package.json

    "tablegen": "../cli/dist/mud.js tablegen",
    "test": "yarn codegen && forge test",
-    "build": "yarn codegen && rimraf out && forge build && yarn dist && yarn types && tsup",
+    "build": "rimraf out && forge build && yarn dist && yarn types && tsup",


Using tablegen here caused an error in the CI, because ../cli/dist/mud.js does not exist yet while store is being built (CLI can't be built first because it depends on World's ABI, which depends on Store). There are (at least) two approaches to solve this:

Move the tablegen scripts into Store (and worldgen scripts into World), so these packages can use them locally (and consumers of these packages don't have to install the MUD CLI if they only want to use eg Store without the rest of the framework). CLI can still expose a tablegen command by importing the scripts from Store (and a worldgen command by importing from World)

Check in the World/Store ABIs, so CLI can be built independently from building World/Store (this alone doesn't fix the circular dependency though)

since this has come up a bunch, mind opening an issue for this?

alvrs · 2023-04-12T15:03:00Z

packages/utils/src/v2/TableId.ts

    // assumes tableId is a 32-byte hex string, otherwise it left-pads with zeros (for numbers)
    // this is scary, since zero padding is different depending on the type (bytes types vs number types)
    // TODO: fix this after we move tableIds to bytes32 instead of uint256
+    // TODO: check if this is still relevant
    const tableIdBytes = new Uint8Array(32);
    tableIdBytes.set(tableId.slice().reverse());
    tableIdBytes.reverse();


@holic can you help me figure out how to change this logic to make it work with tableIds that are bytes32? Do we assume tableId to be uint256 anywhere else in the client code?

I think we can just change this to

const tableIdBytes = new Uint8Array(32); tableIdBytes.set(tableId);

which ensures right-zero padding. But if ethers is already passing bytes32 values as zero padded hex, then we won't even need this.

(before it would reverse, then right-pad, then reverse again, so that the result is left-padded)

We could also use the viem pad helpers for this to make it more clear what's going on.

holic · 2023-04-12T18:33:56Z

packages/network/src/v2/schemas/tableSchemas.ts

@@ -45,7 +46,7 @@ export function registerSchema(world: Contract, table: TableId, rawSchema?: stri
  // TODO: populate from ECS cache before fetching from RPC

  console.log("fetching schema for table", { table: table.toString(), world: world.address });
-  const schema = world.getSchema(table).then((rawSchema: string) => {
+  const schema = (world as IStore).getSchema(table.toHexString()).then((rawSchema: string) => {


good catch!

should we make the world argument an IStore here too?

I tried that first but it requires to change it all the way upstream too, so I waited for now bc it's mostly unrelated to this PR. Just thought having type hints where we call the World methods is useful

alvrs marked this pull request as ready for review April 12, 2023 14:44

alvrs requested a review from a user April 12, 2023 14:44

alvrs requested a review from holic as a code owner April 12, 2023 14:44

alvrs commented Apr 12, 2023

View reviewed changes

alvrs mentioned this pull request Apr 12, 2023

Move tablegen to Store and worldgen to World to resolve circular dependencies #616

Closed

holic reviewed Apr 12, 2023

View reviewed changes

alvrs added 2 commits April 12, 2023 20:40

refactor: rename fileSelector to name

e92f5bb

fix(std-client): update tsgen

ced5245

alvrs force-pushed the alvrs/renamefileselector branch from 81edeee to ced5245 Compare April 12, 2023 18:40

alvrs added 5 commits April 12, 2023 20:43

refactor: uint256 tableId -> bytes32 tableId

22704e8

fix(cli): regenerate cli tables

5751642

chore(world, store): update gas-report

8779883

fix(utils): remove left padding of TableId

e72a2fe

fix(network): type issues

3f5bd43

alvrs force-pushed the alvrs/tableidbytes32 branch from 17a70f1 to 3f5bd43 Compare April 12, 2023 18:44

holic approved these changes Apr 12, 2023

View reviewed changes

Base automatically changed from alvrs/renamefileselector to main April 12, 2023 19:25

Merge remote-tracking branch 'origin/main' into alvrs/tableidbytes32

8a10802

holic merged commit 26195b8 into main Apr 12, 2023

holic deleted the alvrs/tableidbytes32 branch April 12, 2023 19:59

holic mentioned this pull request Apr 17, 2023

move back to bytes32 (instead of uint256) for tableIds #484

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: uint256 tableId -> bytes32 tableId #613

refactor: uint256 tableId -> bytes32 tableId #613

alvrs commented Apr 12, 2023 •

edited

Loading

alvrs Apr 12, 2023

alvrs Apr 12, 2023

holic Apr 12, 2023

alvrs Apr 12, 2023

holic Apr 12, 2023

alvrs Apr 12, 2023

alvrs Apr 12, 2023

holic Apr 12, 2023

holic Apr 12, 2023

holic Apr 12, 2023

alvrs Apr 12, 2023

		@@ -255,7 +255,7 @@ type StorecoreStoreDeleteRecord struct {

refactor: uint256 tableId -> bytes32 tableId #613

refactor: uint256 tableId -> bytes32 tableId #613

Conversation

alvrs commented Apr 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alvrs commented Apr 12, 2023 •

edited

Loading