From 103f635ebc20ac1aecc5c526c4bcb928e860a7ed Mon Sep 17 00:00:00 2001 From: Kevin Ingersoll Date: Thu, 11 Jan 2024 14:53:55 +0000 Subject: [PATCH] feat(store): improve FieldLayout errors [N-03] (#2114) --- .changeset/young-poets-beam.md | 12 ++++++++++++ packages/store/src/FieldLayout.sol | 17 +++++++++++------ packages/store/test/StoreCore.t.sol | 5 +++-- packages/store/ts/codegen/renderFieldLayout.ts | 4 ++-- packages/world/gas-report.json | 2 +- 5 files changed, 29 insertions(+), 11 deletions(-) create mode 100644 .changeset/young-poets-beam.md diff --git a/.changeset/young-poets-beam.md b/.changeset/young-poets-beam.md new file mode 100644 index 0000000000..0430659a3c --- /dev/null +++ b/.changeset/young-poets-beam.md @@ -0,0 +1,12 @@ +--- +"@latticexyz/store": minor +--- + +Improved error messages for invalid `FieldLayout`s + +```diff +-error FieldLayoutLib_InvalidLength(uint256 length); ++error FieldLayoutLib_TooManyFields(uint256 numFields, uint256 maxFields); ++error FieldLayoutLib_TooManyDynamicFields(uint256 numFields, uint256 maxFields); ++error FieldLayoutLib_Empty(); +``` diff --git a/packages/store/src/FieldLayout.sol b/packages/store/src/FieldLayout.sol index 7e93bc28b1..bf88c76dd2 100644 --- a/packages/store/src/FieldLayout.sol +++ b/packages/store/src/FieldLayout.sol @@ -25,7 +25,9 @@ using FieldLayoutInstance for FieldLayout global; * various constraints regarding the length and size of the fields. */ library FieldLayoutLib { - error FieldLayoutLib_InvalidLength(uint256 length); + error FieldLayoutLib_TooManyFields(uint256 numFields, uint256 maxFields); + error FieldLayoutLib_TooManyDynamicFields(uint256 numFields, uint256 maxFields); + error FieldLayoutLib_Empty(); error FieldLayoutLib_StaticLengthIsZero(); error FieldLayoutLib_StaticLengthDoesNotFitInAWord(); @@ -41,8 +43,9 @@ library FieldLayoutLib { uint256 fieldLayout; uint256 totalLength; uint256 totalFields = _staticFields.length + numDynamicFields; - if (totalFields > MAX_TOTAL_FIELDS) revert FieldLayoutLib_InvalidLength(totalFields); - if (numDynamicFields > MAX_DYNAMIC_FIELDS) revert FieldLayoutLib_InvalidLength(numDynamicFields); + if (totalFields > MAX_TOTAL_FIELDS) revert FieldLayoutLib_TooManyFields(totalFields, MAX_TOTAL_FIELDS); + if (numDynamicFields > MAX_DYNAMIC_FIELDS) + revert FieldLayoutLib_TooManyDynamicFields(numDynamicFields, MAX_DYNAMIC_FIELDS); // Compute the total static length and store the field lengths in the encoded fieldLayout for (uint256 i = 0; i < _staticFields.length; ) { @@ -149,16 +152,18 @@ library FieldLayoutInstance { */ function validate(FieldLayout fieldLayout, bool allowEmpty) internal pure { // FieldLayout must not be empty - if (!allowEmpty && fieldLayout.isEmpty()) revert FieldLayoutLib.FieldLayoutLib_InvalidLength(0); + if (!allowEmpty && fieldLayout.isEmpty()) revert FieldLayoutLib.FieldLayoutLib_Empty(); // FieldLayout must have no more than MAX_DYNAMIC_FIELDS uint256 _numDynamicFields = fieldLayout.numDynamicFields(); - if (_numDynamicFields > MAX_DYNAMIC_FIELDS) revert FieldLayoutLib.FieldLayoutLib_InvalidLength(_numDynamicFields); + 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) revert FieldLayoutLib.FieldLayoutLib_InvalidLength(_numTotalFields); + 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/test/StoreCore.t.sol b/packages/store/test/StoreCore.t.sol index fea7a9054e..7638fac612 100644 --- a/packages/store/test/StoreCore.t.sol +++ b/packages/store/test/StoreCore.t.sol @@ -131,8 +131,9 @@ contract StoreCoreTest is Test, StoreMock { vm.expectRevert( abi.encodeWithSelector( - FieldLayoutLib.FieldLayoutLib_InvalidLength.selector, - invalidFieldLayout.numDynamicFields() + FieldLayoutLib.FieldLayoutLib_TooManyDynamicFields.selector, + invalidFieldLayout.numDynamicFields(), + 5 ) ); IStore(this).registerTable( diff --git a/packages/store/ts/codegen/renderFieldLayout.ts b/packages/store/ts/codegen/renderFieldLayout.ts index cb112a875c..b40cc998cb 100644 --- a/packages/store/ts/codegen/renderFieldLayout.ts +++ b/packages/store/ts/codegen/renderFieldLayout.ts @@ -14,8 +14,8 @@ export function encodeFieldLayout(fields: RenderType[]) { let totalLength = 0; const totalFields = fields.length; - if (totalFields > MAX_TOTAL_FIELDS) throw new Error(`FieldLayout: invalid length ${totalFields}`); - if (numDynamicFields > MAX_DYNAMIC_FIELDS) throw new Error(`FieldLayout: invalid length ${numDynamicFields}`); + if (totalFields > MAX_TOTAL_FIELDS) throw new Error("FieldLayout: too many fields"); + if (numDynamicFields > MAX_DYNAMIC_FIELDS) throw new Error("FieldLayout: too many dynamic fields"); for (let i = 0; i < staticFields.length; i++) { const { isDynamic, staticByteLength } = fields[i]; diff --git a/packages/world/gas-report.json b/packages/world/gas-report.json index d1cb3e329a..9caba2f8e9 100644 --- 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": 528361 + "gasUsed": 528393 }, { "file": "test/World.t.sol",