feat(store): never allow empty FieldLayout (#2122)

latticexyz · Jan 12, 2024 · 3ac68ad · 3ac68ad
1 parent b1f5816
commit 3ac68ad
Show file tree

Hide file tree

Showing 7 changed files with 32 additions and 27 deletions.
diff --git a/.changeset/tidy-ants-ring.md b/.changeset/tidy-ants-ring.md
@@ -0,0 +1,5 @@
+---
+"@latticexyz/store": minor
+---
+
+Removed `allowEmpty` option from `FieldLayout.validate()` as field layouts should never be empty.
diff --git a/packages/store/gas-report.json b/packages/store/gas-report.json
@@ -135,7 +135,7 @@
     "file": "test/FieldLayout.t.sol",
     "test": "testValidate",
     "name": "validate field layout",
-    "gasUsed": 3993
+    "gasUsed": 3954
   },
   {
     "file": "test/Gas.t.sol",
@@ -369,7 +369,7 @@
     "file": "test/KeyEncoding.t.sol",
     "test": "testRegisterAndGetFieldLayout",
     "name": "register KeyEncoding table",
-    "gasUsed": 719062
+    "gasUsed": 719023
   },
   {
     "file": "test/Mixed.t.sol",
@@ -813,7 +813,7 @@
     "file": "test/StoreCoreGas.t.sol",
     "test": "testRegisterAndGetFieldLayout",
     "name": "StoreCore: register table",
-    "gasUsed": 640944
+    "gasUsed": 640905
   },
   {
     "file": "test/StoreCoreGas.t.sol",
@@ -1155,7 +1155,7 @@
     "file": "test/Vector2.t.sol",
     "test": "testRegisterAndGetFieldLayout",
     "name": "register Vector2 field layout",
-    "gasUsed": 442447
+    "gasUsed": 442408
   },
   {
     "file": "test/Vector2.t.sol",

diff --git a/packages/store/src/FieldLayout.sol b/packages/store/src/FieldLayout.sol
@@ -148,22 +148,22 @@ library FieldLayoutInstance {
    * @notice Validate the field layout with various checks on the length and size of the fields.
    * @dev Reverts if total fields, static field length, or static byte length exceed allowed limits.
    * @param fieldLayout The FieldLayout to validate.
-   * @param allowEmpty A flag to determine if empty field layouts are allowed.
    */
-  function validate(FieldLayout fieldLayout, bool allowEmpty) internal pure {
-    // FieldLayout must not be empty
-    if (!allowEmpty && fieldLayout.isEmpty()) revert FieldLayoutLib.FieldLayoutLib_Empty();
+  function validate(FieldLayout fieldLayout) internal pure {
+    if (fieldLayout.isEmpty()) {
+      revert FieldLayoutLib.FieldLayoutLib_Empty();
+    }
 
-    // FieldLayout must have no more than MAX_DYNAMIC_FIELDS
     uint256 _numDynamicFields = fieldLayout.numDynamicFields();
-    if (_numDynamicFields > MAX_DYNAMIC_FIELDS)
+    if (_numDynamicFields > MAX_DYNAMIC_FIELDS) {
       revert FieldLayoutLib.FieldLayoutLib_TooManyDynamicFields(_numDynamicFields, MAX_DYNAMIC_FIELDS);
+    }
 
     uint256 _numStaticFields = fieldLayout.numStaticFields();
-    // FieldLayout must not have more than MAX_TOTAL_FIELDS in total
     uint256 _numTotalFields = _numStaticFields + _numDynamicFields;
-    if (_numTotalFields > MAX_TOTAL_FIELDS)
+    if (_numTotalFields > MAX_TOTAL_FIELDS) {
       revert FieldLayoutLib.FieldLayoutLib_TooManyFields(_numTotalFields, MAX_TOTAL_FIELDS);
+    }
 
     // Static lengths must be valid
     for (uint256 i; i < _numStaticFields; ) {

diff --git a/packages/store/src/StoreCore.sol b/packages/store/src/StoreCore.sol
@@ -185,7 +185,7 @@ library StoreCore {
     }
 
     // Verify the field layout is valid
-    fieldLayout.validate({ allowEmpty: false });
+    fieldLayout.validate();
 
     // Verify the schema is valid
     keySchema.validate({ allowEmpty: true });

diff --git a/packages/store/test/FieldLayout.t.sol b/packages/store/test/FieldLayout.t.sol
@@ -176,12 +176,12 @@ contract FieldLayoutTest is Test, GasReporter {
     FieldLayout encodedFieldLayout = FieldLayoutLib.encode(fieldLayout, 5);
 
     startGasReport("validate field layout");
-    encodedFieldLayout.validate({ allowEmpty: false });
+    encodedFieldLayout.validate();
     endGasReport();
   }
 
   function testFailValidate() public pure {
-    FieldLayout.wrap(keccak256("some invalid field layout")).validate({ allowEmpty: false });
+    FieldLayout.wrap(keccak256("some invalid field layout")).validate();
   }
 
   function testIsEmptyTrue() public {

diff --git a/packages/world-modules/gas-report.json b/packages/world-modules/gas-report.json
@@ -75,13 +75,13 @@
     "file": "test/KeysInTableModule.t.sol",
     "test": "testInstallComposite",
     "name": "install keys in table module",
-    "gasUsed": 1413433
+    "gasUsed": 1413357
   },
   {
     "file": "test/KeysInTableModule.t.sol",
     "test": "testInstallGas",
     "name": "install keys in table module",
-    "gasUsed": 1413433
+    "gasUsed": 1413357
   },
   {
     "file": "test/KeysInTableModule.t.sol",
@@ -93,13 +93,13 @@
     "file": "test/KeysInTableModule.t.sol",
     "test": "testInstallSingleton",
     "name": "install keys in table module",
-    "gasUsed": 1413433
+    "gasUsed": 1413357
   },
   {
     "file": "test/KeysInTableModule.t.sol",
     "test": "testSetAndDeleteRecordHookCompositeGas",
     "name": "install keys in table module",
-    "gasUsed": 1413433
+    "gasUsed": 1413357
   },
   {
     "file": "test/KeysInTableModule.t.sol",
@@ -117,7 +117,7 @@
     "file": "test/KeysInTableModule.t.sol",
     "test": "testSetAndDeleteRecordHookGas",
     "name": "install keys in table module",
-    "gasUsed": 1413433
+    "gasUsed": 1413357
   },
   {
     "file": "test/KeysInTableModule.t.sol",
@@ -135,7 +135,7 @@
     "file": "test/KeysWithValueModule.t.sol",
     "test": "testGetKeysWithValueGas",
     "name": "install keys with value module",
-    "gasUsed": 668206
+    "gasUsed": 668168
   },
   {
     "file": "test/KeysWithValueModule.t.sol",
@@ -153,7 +153,7 @@
     "file": "test/KeysWithValueModule.t.sol",
     "test": "testInstall",
     "name": "install keys with value module",
-    "gasUsed": 668206
+    "gasUsed": 668168
   },
   {
     "file": "test/KeysWithValueModule.t.sol",
@@ -165,7 +165,7 @@
     "file": "test/KeysWithValueModule.t.sol",
     "test": "testSetAndDeleteRecordHook",
     "name": "install keys with value module",
-    "gasUsed": 668206
+    "gasUsed": 668168
   },
   {
     "file": "test/KeysWithValueModule.t.sol",
@@ -183,7 +183,7 @@
     "file": "test/KeysWithValueModule.t.sol",
     "test": "testSetField",
     "name": "install keys with value module",
-    "gasUsed": 668206
+    "gasUsed": 668168
   },
   {
     "file": "test/KeysWithValueModule.t.sol",
@@ -303,7 +303,7 @@
     "file": "test/UniqueEntityModule.t.sol",
     "test": "testInstall",
     "name": "install unique entity module",
-    "gasUsed": 694917
+    "gasUsed": 694879
   },
   {
     "file": "test/UniqueEntityModule.t.sol",
@@ -315,7 +315,7 @@
     "file": "test/UniqueEntityModule.t.sol",
     "test": "testInstallRoot",
     "name": "installRoot unique entity module",
-    "gasUsed": 663866
+    "gasUsed": 663827
   },
   {
     "file": "test/UniqueEntityModule.t.sol",

diff --git a/packages/world/gas-report.json b/packages/world/gas-report.json
@@ -117,7 +117,7 @@
     "file": "test/World.t.sol",
     "test": "testRegisterTable",
     "name": "Register a new table in the namespace",
-    "gasUsed": 528422
+    "gasUsed": 528384
   },
   {
     "file": "test/World.t.sol",