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

fix(store): reorder core table registration #2164

Merged
merged 11 commits into from
Jan 30, 2024
5 changes: 5 additions & 0 deletions .changeset/breezy-days-greet.md
Original file line number Diff line number Diff line change
@@ -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.
38 changes: 34 additions & 4 deletions packages/store/src/StoreCore.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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();
}

/************************************************************************
Expand Down
2 changes: 1 addition & 1 deletion packages/world/gas-report.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
"file": "test/Factories.t.sol",
"test": "testWorldFactory",
"name": "deploy world via WorldFactory",
"gasUsed": 12512902
"gasUsed": 12485582
},
{
"file": "test/World.t.sol",
Expand Down
Loading