fix(store): reorder core table registration (#2164)

latticexyz · Jan 30, 2024 · 05b3e88 · 05b3e88
1 parent 55a05fd
commit 05b3e88
Show file tree

Hide file tree

Showing 7 changed files with 658 additions and 623 deletions.
diff --git a/.changeset/breezy-days-greet.md b/.changeset/breezy-days-greet.md
@@ -0,0 +1,5 @@
+---
+"@latticexyz/store": patch
+---
+
+Fixed a race condition when registering core tables, where we would set a record in the `ResourceIds` table before the table was registered.
diff --git a/packages/store-sync/src/postgres-decoded/createStorageAdapter.test.ts b/packages/store-sync/src/postgres-decoded/createStorageAdapter.test.ts
@@ -50,7 +50,7 @@ describe("createStorageAdapter", async () => {
     expect(await db.select().from(storageAdapter.tables.configTable)).toMatchInlineSnapshot(`
       [
         {
-          "blockNumber": 19n,
+          "blockNumber": 21n,
           "chainId": 31337,
           "version": "0.0.4",
         },
@@ -70,8 +70,8 @@ describe("createStorageAdapter", async () => {
     ).toMatchInlineSnapshot(`
       [
         {
-          "address": "0xf599E8F01A5Ca469CDa10711C3f2Ffa4Eeed755E",
-          "blockNumber": 19n,
+          "address": "0x1A33F58dE9acC2645630B4EdC3d3116c755016e0",
+          "blockNumber": 21n,
           "dynamicData": "0x000001a400000045",
           "encodedLengths": "0x0000000000000000000000000000000000000000000000000800000000000008",
           "isDeleted": false,
@@ -89,7 +89,7 @@ describe("createStorageAdapter", async () => {
     expect(tables).toMatchInlineSnapshot(`
       [
         {
-          "address": "0xf599E8F01A5Ca469CDa10711C3f2Ffa4Eeed755E",
+          "address": "0x1A33F58dE9acC2645630B4EdC3d3116c755016e0",
           "keySchema": {},
           "name": "NumberList",
           "namespace": "",
@@ -106,7 +106,7 @@ describe("createStorageAdapter", async () => {
       [
         {
           "__keyBytes": "0x",
-          "__lastUpdatedBlockNumber": 19n,
+          "__lastUpdatedBlockNumber": 21n,
           "value": [
             420,
             69,

diff --git a/packages/store-sync/src/postgres/createStorageAdapter.test.ts b/packages/store-sync/src/postgres/createStorageAdapter.test.ts
@@ -48,7 +48,7 @@ describe("createStorageAdapter", async () => {
     expect(await db.select().from(storageAdapter.tables.configTable)).toMatchInlineSnapshot(`
       [
         {
-          "blockNumber": 19n,
+          "blockNumber": 21n,
           "chainId": 31337,
           "version": "0.0.4",
         },
@@ -68,8 +68,8 @@ describe("createStorageAdapter", async () => {
     ).toMatchInlineSnapshot(`
       [
         {
-          "address": "0xf599E8F01A5Ca469CDa10711C3f2Ffa4Eeed755E",
-          "blockNumber": 19n,
+          "address": "0x1A33F58dE9acC2645630B4EdC3d3116c755016e0",
+          "blockNumber": 21n,
           "dynamicData": "0x000001a400000045",
           "encodedLengths": "0x0000000000000000000000000000000000000000000000000800000000000008",
           "isDeleted": false,

diff --git a/packages/store-sync/src/sqlite/sqliteStorage.test.ts b/packages/store-sync/src/sqlite/sqliteStorage.test.ts
@@ -64,7 +64,7 @@ describe("sqliteStorage", async () => {
         {
           "chainId": 31337,
           "lastError": null,
-          "lastUpdatedBlockNumber": 19n,
+          "lastUpdatedBlockNumber": 21n,
           "schemaVersion": 1,
         },
       ]
@@ -73,11 +73,11 @@ describe("sqliteStorage", async () => {
     expect(db.select().from(mudStoreTables).where(eq(mudStoreTables.name, "NumberList")).all()).toMatchInlineSnapshot(`
       [
         {
-          "address": "0xf599E8F01A5Ca469CDa10711C3f2Ffa4Eeed755E",
-          "id": "0xf599E8F01A5Ca469CDa10711C3f2Ffa4Eeed755E____NumberList",
+          "address": "0x1A33F58dE9acC2645630B4EdC3d3116c755016e0",
+          "id": "0x1A33F58dE9acC2645630B4EdC3d3116c755016e0____NumberList",
           "keySchema": {},
           "lastError": null,
-          "lastUpdatedBlockNumber": 19n,
+          "lastUpdatedBlockNumber": 21n,
           "name": "NumberList",
           "namespace": "",
           "schemaVersion": 1,
@@ -93,11 +93,11 @@ describe("sqliteStorage", async () => {
     expect(tables).toMatchInlineSnapshot(`
       [
         {
-          "address": "0xf599E8F01A5Ca469CDa10711C3f2Ffa4Eeed755E",
-          "id": "0xf599E8F01A5Ca469CDa10711C3f2Ffa4Eeed755E____NumberList",
+          "address": "0x1A33F58dE9acC2645630B4EdC3d3116c755016e0",
+          "id": "0x1A33F58dE9acC2645630B4EdC3d3116c755016e0____NumberList",
           "keySchema": {},
           "lastError": null,
-          "lastUpdatedBlockNumber": 19n,
+          "lastUpdatedBlockNumber": 21n,
           "name": "NumberList",
           "namespace": "",
           "schemaVersion": 1,
@@ -117,7 +117,7 @@ describe("sqliteStorage", async () => {
           "__encodedLengths": "0x0000000000000000000000000000000000000000000000000800000000000008",
           "__isDeleted": false,
           "__key": "0x",
-          "__lastUpdatedBlockNumber": 19n,
+          "__lastUpdatedBlockNumber": 21n,
           "__staticData": null,
           "value": [
             420,

diff --git a/packages/store/src/StoreCore.sol b/packages/store/src/StoreCore.sol
@@ -9,7 +9,7 @@ import { FieldLayout, FieldLayoutLib } from "./FieldLayout.sol";
 import { Schema, SchemaLib } from "./Schema.sol";
 import { PackedCounter } from "./PackedCounter.sol";
 import { Slice, SliceLib } from "./Slice.sol";
-import { StoreHooks, Tables, TablesTableId, ResourceIds, StoreHooksTableId } from "./codegen/index.sol";
+import { Tables, TablesTableId, ResourceIds, ResourceIdsTableId, StoreHooks, StoreHooksTableId } from "./codegen/index.sol";
 import { _fieldLayout as TablesTableFieldLayout } from "./codegen/tables/Tables.sol";
 import { IStoreErrors } from "./IStoreErrors.sol";
 import { IStoreHook } from "./IStoreHook.sol";
@@ -93,10 +93,40 @@ library StoreCore {
    * any table data to allow indexers to decode table events.
    */
   function registerCoreTables() internal {
-    // Register core tables
-    Tables.register();
+    // Because `registerTable` writes to both `Tables` and `ResourceIds`, we can't use it
+    // directly here without creating a race condition, where we'd write to one or the other
+    // before they exist (depending on the order of registration).
+    //
+    // Instead, we'll register them manually, writing everything to the `Tables` table first,
+    // then the `ResourceIds` table. The logic here ought to be kept in sync with the internals
+    // of the `registerTable` function below.
+    if (ResourceIds._getExists(TablesTableId)) {
+      revert IStoreErrors.Store_TableAlreadyExists(TablesTableId, string(abi.encodePacked(TablesTableId)));
+    }
+    if (ResourceIds._getExists(ResourceIdsTableId)) {
+      revert IStoreErrors.Store_TableAlreadyExists(ResourceIdsTableId, string(abi.encodePacked(ResourceIdsTableId)));
+    }
+    Tables._set(
+      TablesTableId,
+      Tables.getFieldLayout(),
+      Tables.getKeySchema(),
+      Tables.getValueSchema(),
+      abi.encode(Tables.getKeyNames()),
+      abi.encode(Tables.getFieldNames())
+    );
+    Tables._set(
+      ResourceIdsTableId,
+      ResourceIds.getFieldLayout(),
+      ResourceIds.getKeySchema(),
+      ResourceIds.getValueSchema(),
+      abi.encode(ResourceIds.getKeyNames()),
+      abi.encode(ResourceIds.getFieldNames())
+    );
+    ResourceIds._setExists(TablesTableId, true);
+    ResourceIds._setExists(ResourceIdsTableId, true);
+
+    // Now we can register the rest of the core tables as regular tables.
     StoreHooks.register();
-    ResourceIds.register();
   }
 
   /************************************************************************

diff --git a/packages/world/gas-report.json b/packages/world/gas-report.json
@@ -63,7 +63,7 @@
     "file": "test/Factories.t.sol",
     "test": "testWorldFactory",
     "name": "deploy world via WorldFactory",
-    "gasUsed": 13045360
+    "gasUsed": 13018541
   },
   {
     "file": "test/World.t.sol",