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): combine schema and metadata registration, rename getSchema to getValueSchema, change Schema table id #1182

Merged
merged 23 commits into from
Aug 16, 2023

Conversation

alvrs
Copy link
Member

@alvrs alvrs commented Jul 19, 2023

Fixes #1122
Fixes #1043
Fixes #1134

This PR includes a few breaking changes:

  • Store's registerSchema and setMetadata are combined into a single registerTable method. This means metadata (key names, field names) is immutable and indexers can create tables with this metadata when a new table is registered on-chain.
  • World's registerTable method is updated to also include key names and field names, setMetadata is removed
  • The getSchema method is renamed to getValueSchema on all interfaces
  • The internal Schema table is now an actual table instead of receiving special treatment. It is renamed to Tables, and the table id chagned from "mudstore:schema" to "mudstore:Tables"

We need to integrate these changes into our networking stack before merging, so this PR is blocked on replacing v1 networking code with v2 (otherwise we'd have to integrate these chagnes into the deprecated v1 networking code which will be removed very soon).

Consumers who are using default MUD table libraries don't need to do anything to integrate these breaking changes except update to the latest version and regenerate the table libraries (which happens automatically during the build step of projects started from MUD templates)

@changeset-bot
Copy link

changeset-bot bot commented Jul 19, 2023

🦋 Changeset detected

Latest commit: 3ce006c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 26 packages
Name Type
@latticexyz/cli Major
@latticexyz/store Major
@latticexyz/world Major
@latticexyz/store-sync Major
create-mud Major
@latticexyz/std-client Major
@latticexyz/dev-tools Major
@latticexyz/react Major
@latticexyz/store-cache Major
@latticexyz/store-indexer Major
@latticexyz/ecs-browser Major
@latticexyz/block-logs-stream Major
@latticexyz/common Major
@latticexyz/config Major
@latticexyz/gas-report Major
@latticexyz/noise Major
@latticexyz/phaserx Major
@latticexyz/protocol-parser Major
@latticexyz/recs Major
@latticexyz/schema-type Major
@latticexyz/services Major
@latticexyz/solecs Major
solhint-config-mud Major
solhint-plugin-mud Major
@latticexyz/std-contracts Major
@latticexyz/utils Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

bytes32 constant _tableId = bytes32(abi.encodePacked(bytes16("mudstore"), bytes16("TableData")));
bytes32 constant TableDataTableId = _tableId;

struct TableDataData {
Copy link
Member

Choose a reason for hiding this comment

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

not blocking but "data data" is a little weird

would calling this table Table be too weird/confusing?

Copy link
Member

Choose a reason for hiding this comment

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

we'd get TableTableId

Copy link
Member

Choose a reason for hiding this comment

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

what about TableMetadata or StoreMetadata? (since the old table can go away, and the new one can get an extra field - tableName)

abiEncodedKeyNames: "bytes",
abiEncodedValueNames: "bytes",
},
},
StoreMetadata: {
Copy link
Member

Choose a reason for hiding this comment

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

this can go away now, right?

world,
{ table: Type.T },
{ metadata: { componentName: "TableMetadata" } }
{ metadata: { componentName: "Tables" } }
Copy link
Member

Choose a reason for hiding this comment

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

I think this got renamed by mistake!

Copy link
Member

Choose a reason for hiding this comment

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

Might be worth adjusting this name anyway to like RegisteredClientTables or something

Copy link
Member

Choose a reason for hiding this comment

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

That said, I wonder if the now-built-in Tables component (derived from mud config) would have everything I need (minus some nice-to-haves like an already parsed table schema).

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch!

@alvrs alvrs marked this pull request as ready for review August 16, 2023 17:22
@alvrs alvrs requested a review from holic August 16, 2023 17:22
.changeset/modern-hornets-jam.md Outdated Show resolved Hide resolved
.changeset/modern-hornets-jam.md Outdated Show resolved Hide resolved
.changeset/modern-hornets-jam.md Outdated Show resolved Hide resolved
.changeset/modern-hornets-jam.md Outdated Show resolved Hide resolved
docs/pages/store/internals.mdx Outdated Show resolved Hide resolved
StoreMetadata.set(StoreCoreInternal.SCHEMA_TABLE, "schema", abi.encode(fieldNames));
// Register internal tables
Tables.register();
Hooks.register();
Copy link
Member

Choose a reason for hiding this comment

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

:chefkiss:

packages/store/src/StoreCore.sol Outdated Show resolved Hide resolved
alvrs and others added 2 commits August 16, 2023 23:20
Co-authored-by: Kevin Ingersoll <kingersoll@gmail.com>
Co-authored-by: Kevin Ingersoll <kingersoll@gmail.com>
@alvrs
Copy link
Member Author

alvrs commented Aug 16, 2023

merging since only comment changes since last approval

@alvrs alvrs merged commit afaf2f5 into main Aug 16, 2023
10 checks passed
@alvrs alvrs deleted the alvrs/real-schema branch August 16, 2023 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants