From b798ccb2b19bdda2995f188912258c7563747e42 Mon Sep 17 00:00:00 2001 From: yonada Date: Fri, 5 Apr 2024 04:32:21 +0100 Subject: [PATCH] fix(store): return zero for uninitialised static array elements (#2613) Co-authored-by: alvarius Co-authored-by: Kevin Ingersoll --- .changeset/popular-oranges-end.md | 5 ++ .../src/codegen/tables/StaticArray.sol | 32 ++++++++ .../src/codegen/tables/Dynamics1.sol | 80 +++++++++++++++++++ .../src/codegen/tables/Singleton.sol | 48 +++++++++++ packages/cli/contracts/test/Tablegen.t.sol | 29 +++++++ packages/store/ts/codegen/field.ts | 16 ++++ 6 files changed, 210 insertions(+) create mode 100644 .changeset/popular-oranges-end.md diff --git a/.changeset/popular-oranges-end.md b/.changeset/popular-oranges-end.md new file mode 100644 index 0000000000..b899727d0a --- /dev/null +++ b/.changeset/popular-oranges-end.md @@ -0,0 +1,5 @@ +--- +"@latticexyz/store": patch +--- + +Fixed the behaviour of static arrays, so that they return zero for uninitialised values, to mirror the native Solidity behavior. Previously they reverted with `Store_IndexOutOfBounds` if the index had not been set yet. diff --git a/e2e/packages/contracts/src/codegen/tables/StaticArray.sol b/e2e/packages/contracts/src/codegen/tables/StaticArray.sol index b9ba6ad861..1ee2570cff 100644 --- a/e2e/packages/contracts/src/codegen/tables/StaticArray.sol +++ b/e2e/packages/contracts/src/codegen/tables/StaticArray.sol @@ -148,6 +148,14 @@ library StaticArray { function getItemValue(uint256 _index) internal view returns (uint256) { bytes32[] memory _keyTuple = new bytes32[](0); + uint256 _byteLength = StoreSwitch.getDynamicFieldLength(_tableId, _keyTuple, 0); + uint256 dynamicLength = _byteLength / 32; + uint256 staticLength = 3; + + if (_index < staticLength && _index >= dynamicLength) { + return (uint256(bytes32(new bytes(0)))); + } + unchecked { bytes memory _blob = StoreSwitch.getDynamicFieldSlice(_tableId, _keyTuple, 0, _index * 32, (_index + 1) * 32); return (uint256(bytes32(_blob))); @@ -161,6 +169,14 @@ library StaticArray { function _getItemValue(uint256 _index) internal view returns (uint256) { bytes32[] memory _keyTuple = new bytes32[](0); + uint256 _byteLength = StoreCore.getDynamicFieldLength(_tableId, _keyTuple, 0); + uint256 dynamicLength = _byteLength / 32; + uint256 staticLength = 3; + + if (_index < staticLength && _index >= dynamicLength) { + return (uint256(bytes32(new bytes(0)))); + } + unchecked { bytes memory _blob = StoreCore.getDynamicFieldSlice(_tableId, _keyTuple, 0, _index * 32, (_index + 1) * 32); return (uint256(bytes32(_blob))); @@ -174,6 +190,14 @@ library StaticArray { function getItem(uint256 _index) internal view returns (uint256) { bytes32[] memory _keyTuple = new bytes32[](0); + uint256 _byteLength = StoreSwitch.getDynamicFieldLength(_tableId, _keyTuple, 0); + uint256 dynamicLength = _byteLength / 32; + uint256 staticLength = 3; + + if (_index < staticLength && _index >= dynamicLength) { + return (uint256(bytes32(new bytes(0)))); + } + unchecked { bytes memory _blob = StoreSwitch.getDynamicFieldSlice(_tableId, _keyTuple, 0, _index * 32, (_index + 1) * 32); return (uint256(bytes32(_blob))); @@ -187,6 +211,14 @@ library StaticArray { function _getItem(uint256 _index) internal view returns (uint256) { bytes32[] memory _keyTuple = new bytes32[](0); + uint256 _byteLength = StoreCore.getDynamicFieldLength(_tableId, _keyTuple, 0); + uint256 dynamicLength = _byteLength / 32; + uint256 staticLength = 3; + + if (_index < staticLength && _index >= dynamicLength) { + return (uint256(bytes32(new bytes(0)))); + } + unchecked { bytes memory _blob = StoreCore.getDynamicFieldSlice(_tableId, _keyTuple, 0, _index * 32, (_index + 1) * 32); return (uint256(bytes32(_blob))); diff --git a/packages/cli/contracts/src/codegen/tables/Dynamics1.sol b/packages/cli/contracts/src/codegen/tables/Dynamics1.sol index 45df0941b4..96f8d3469e 100644 --- a/packages/cli/contracts/src/codegen/tables/Dynamics1.sol +++ b/packages/cli/contracts/src/codegen/tables/Dynamics1.sol @@ -125,6 +125,14 @@ library Dynamics1 { bytes32[] memory _keyTuple = new bytes32[](1); _keyTuple[0] = key; + uint256 _byteLength = StoreSwitch.getDynamicFieldLength(_tableId, _keyTuple, 0); + uint256 dynamicLength = _byteLength / 32; + uint256 staticLength = 1; + + if (_index < staticLength && _index >= dynamicLength) { + return (bytes32(new bytes(0))); + } + unchecked { bytes memory _blob = StoreSwitch.getDynamicFieldSlice(_tableId, _keyTuple, 0, _index * 32, (_index + 1) * 32); return (bytes32(_blob)); @@ -139,6 +147,14 @@ library Dynamics1 { bytes32[] memory _keyTuple = new bytes32[](1); _keyTuple[0] = key; + uint256 _byteLength = StoreCore.getDynamicFieldLength(_tableId, _keyTuple, 0); + uint256 dynamicLength = _byteLength / 32; + uint256 staticLength = 1; + + if (_index < staticLength && _index >= dynamicLength) { + return (bytes32(new bytes(0))); + } + unchecked { bytes memory _blob = StoreCore.getDynamicFieldSlice(_tableId, _keyTuple, 0, _index * 32, (_index + 1) * 32); return (bytes32(_blob)); @@ -224,6 +240,14 @@ library Dynamics1 { bytes32[] memory _keyTuple = new bytes32[](1); _keyTuple[0] = key; + uint256 _byteLength = StoreSwitch.getDynamicFieldLength(_tableId, _keyTuple, 1); + uint256 dynamicLength = _byteLength / 4; + uint256 staticLength = 2; + + if (_index < staticLength && _index >= dynamicLength) { + return (int32(uint32(bytes4(new bytes(0))))); + } + unchecked { bytes memory _blob = StoreSwitch.getDynamicFieldSlice(_tableId, _keyTuple, 1, _index * 4, (_index + 1) * 4); return (int32(uint32(bytes4(_blob)))); @@ -238,6 +262,14 @@ library Dynamics1 { bytes32[] memory _keyTuple = new bytes32[](1); _keyTuple[0] = key; + uint256 _byteLength = StoreCore.getDynamicFieldLength(_tableId, _keyTuple, 1); + uint256 dynamicLength = _byteLength / 4; + uint256 staticLength = 2; + + if (_index < staticLength && _index >= dynamicLength) { + return (int32(uint32(bytes4(new bytes(0))))); + } + unchecked { bytes memory _blob = StoreCore.getDynamicFieldSlice(_tableId, _keyTuple, 1, _index * 4, (_index + 1) * 4); return (int32(uint32(bytes4(_blob)))); @@ -323,6 +355,14 @@ library Dynamics1 { bytes32[] memory _keyTuple = new bytes32[](1); _keyTuple[0] = key; + uint256 _byteLength = StoreSwitch.getDynamicFieldLength(_tableId, _keyTuple, 2); + uint256 dynamicLength = _byteLength / 16; + uint256 staticLength = 3; + + if (_index < staticLength && _index >= dynamicLength) { + return (uint128(bytes16(new bytes(0)))); + } + unchecked { bytes memory _blob = StoreSwitch.getDynamicFieldSlice(_tableId, _keyTuple, 2, _index * 16, (_index + 1) * 16); return (uint128(bytes16(_blob))); @@ -337,6 +377,14 @@ library Dynamics1 { bytes32[] memory _keyTuple = new bytes32[](1); _keyTuple[0] = key; + uint256 _byteLength = StoreCore.getDynamicFieldLength(_tableId, _keyTuple, 2); + uint256 dynamicLength = _byteLength / 16; + uint256 staticLength = 3; + + if (_index < staticLength && _index >= dynamicLength) { + return (uint128(bytes16(new bytes(0)))); + } + unchecked { bytes memory _blob = StoreCore.getDynamicFieldSlice(_tableId, _keyTuple, 2, _index * 16, (_index + 1) * 16); return (uint128(bytes16(_blob))); @@ -422,6 +470,14 @@ library Dynamics1 { bytes32[] memory _keyTuple = new bytes32[](1); _keyTuple[0] = key; + uint256 _byteLength = StoreSwitch.getDynamicFieldLength(_tableId, _keyTuple, 3); + uint256 dynamicLength = _byteLength / 20; + uint256 staticLength = 4; + + if (_index < staticLength && _index >= dynamicLength) { + return (address(bytes20(new bytes(0)))); + } + unchecked { bytes memory _blob = StoreSwitch.getDynamicFieldSlice(_tableId, _keyTuple, 3, _index * 20, (_index + 1) * 20); return (address(bytes20(_blob))); @@ -436,6 +492,14 @@ library Dynamics1 { bytes32[] memory _keyTuple = new bytes32[](1); _keyTuple[0] = key; + uint256 _byteLength = StoreCore.getDynamicFieldLength(_tableId, _keyTuple, 3); + uint256 dynamicLength = _byteLength / 20; + uint256 staticLength = 4; + + if (_index < staticLength && _index >= dynamicLength) { + return (address(bytes20(new bytes(0)))); + } + unchecked { bytes memory _blob = StoreCore.getDynamicFieldSlice(_tableId, _keyTuple, 3, _index * 20, (_index + 1) * 20); return (address(bytes20(_blob))); @@ -521,6 +585,14 @@ library Dynamics1 { bytes32[] memory _keyTuple = new bytes32[](1); _keyTuple[0] = key; + uint256 _byteLength = StoreSwitch.getDynamicFieldLength(_tableId, _keyTuple, 4); + uint256 dynamicLength = _byteLength / 1; + uint256 staticLength = 5; + + if (_index < staticLength && _index >= dynamicLength) { + return (_toBool(uint8(bytes1(new bytes(0))))); + } + unchecked { bytes memory _blob = StoreSwitch.getDynamicFieldSlice(_tableId, _keyTuple, 4, _index * 1, (_index + 1) * 1); return (_toBool(uint8(bytes1(_blob)))); @@ -535,6 +607,14 @@ library Dynamics1 { bytes32[] memory _keyTuple = new bytes32[](1); _keyTuple[0] = key; + uint256 _byteLength = StoreCore.getDynamicFieldLength(_tableId, _keyTuple, 4); + uint256 dynamicLength = _byteLength / 1; + uint256 staticLength = 5; + + if (_index < staticLength && _index >= dynamicLength) { + return (_toBool(uint8(bytes1(new bytes(0))))); + } + unchecked { bytes memory _blob = StoreCore.getDynamicFieldSlice(_tableId, _keyTuple, 4, _index * 1, (_index + 1) * 1); return (_toBool(uint8(bytes1(_blob)))); diff --git a/packages/cli/contracts/src/codegen/tables/Singleton.sol b/packages/cli/contracts/src/codegen/tables/Singleton.sol index 2982b00c4a..20d508cdf6 100644 --- a/packages/cli/contracts/src/codegen/tables/Singleton.sol +++ b/packages/cli/contracts/src/codegen/tables/Singleton.sol @@ -148,6 +148,14 @@ library Singleton { function getItemV2(uint256 _index) internal view returns (uint32) { bytes32[] memory _keyTuple = new bytes32[](0); + uint256 _byteLength = StoreSwitch.getDynamicFieldLength(_tableId, _keyTuple, 0); + uint256 dynamicLength = _byteLength / 4; + uint256 staticLength = 2; + + if (_index < staticLength && _index >= dynamicLength) { + return (uint32(bytes4(new bytes(0)))); + } + unchecked { bytes memory _blob = StoreSwitch.getDynamicFieldSlice(_tableId, _keyTuple, 0, _index * 4, (_index + 1) * 4); return (uint32(bytes4(_blob))); @@ -161,6 +169,14 @@ library Singleton { function _getItemV2(uint256 _index) internal view returns (uint32) { bytes32[] memory _keyTuple = new bytes32[](0); + uint256 _byteLength = StoreCore.getDynamicFieldLength(_tableId, _keyTuple, 0); + uint256 dynamicLength = _byteLength / 4; + uint256 staticLength = 2; + + if (_index < staticLength && _index >= dynamicLength) { + return (uint32(bytes4(new bytes(0)))); + } + unchecked { bytes memory _blob = StoreCore.getDynamicFieldSlice(_tableId, _keyTuple, 0, _index * 4, (_index + 1) * 4); return (uint32(bytes4(_blob))); @@ -239,6 +255,14 @@ library Singleton { function getItemV3(uint256 _index) internal view returns (uint32) { bytes32[] memory _keyTuple = new bytes32[](0); + uint256 _byteLength = StoreSwitch.getDynamicFieldLength(_tableId, _keyTuple, 1); + uint256 dynamicLength = _byteLength / 4; + uint256 staticLength = 2; + + if (_index < staticLength && _index >= dynamicLength) { + return (uint32(bytes4(new bytes(0)))); + } + unchecked { bytes memory _blob = StoreSwitch.getDynamicFieldSlice(_tableId, _keyTuple, 1, _index * 4, (_index + 1) * 4); return (uint32(bytes4(_blob))); @@ -252,6 +276,14 @@ library Singleton { function _getItemV3(uint256 _index) internal view returns (uint32) { bytes32[] memory _keyTuple = new bytes32[](0); + uint256 _byteLength = StoreCore.getDynamicFieldLength(_tableId, _keyTuple, 1); + uint256 dynamicLength = _byteLength / 4; + uint256 staticLength = 2; + + if (_index < staticLength && _index >= dynamicLength) { + return (uint32(bytes4(new bytes(0)))); + } + unchecked { bytes memory _blob = StoreCore.getDynamicFieldSlice(_tableId, _keyTuple, 1, _index * 4, (_index + 1) * 4); return (uint32(bytes4(_blob))); @@ -330,6 +362,14 @@ library Singleton { function getItemV4(uint256 _index) internal view returns (uint32) { bytes32[] memory _keyTuple = new bytes32[](0); + uint256 _byteLength = StoreSwitch.getDynamicFieldLength(_tableId, _keyTuple, 2); + uint256 dynamicLength = _byteLength / 4; + uint256 staticLength = 1; + + if (_index < staticLength && _index >= dynamicLength) { + return (uint32(bytes4(new bytes(0)))); + } + unchecked { bytes memory _blob = StoreSwitch.getDynamicFieldSlice(_tableId, _keyTuple, 2, _index * 4, (_index + 1) * 4); return (uint32(bytes4(_blob))); @@ -343,6 +383,14 @@ library Singleton { function _getItemV4(uint256 _index) internal view returns (uint32) { bytes32[] memory _keyTuple = new bytes32[](0); + uint256 _byteLength = StoreCore.getDynamicFieldLength(_tableId, _keyTuple, 2); + uint256 dynamicLength = _byteLength / 4; + uint256 staticLength = 1; + + if (_index < staticLength && _index >= dynamicLength) { + return (uint32(bytes4(new bytes(0)))); + } + unchecked { bytes memory _blob = StoreCore.getDynamicFieldSlice(_tableId, _keyTuple, 2, _index * 4, (_index + 1) * 4); return (uint32(bytes4(_blob))); diff --git a/packages/cli/contracts/test/Tablegen.t.sol b/packages/cli/contracts/test/Tablegen.t.sol index 49f354c0b5..2469b05386 100644 --- a/packages/cli/contracts/test/Tablegen.t.sol +++ b/packages/cli/contracts/test/Tablegen.t.sol @@ -4,6 +4,7 @@ pragma solidity >=0.8.24; import "forge-std/Test.sol"; import { StoreMock } from "@latticexyz/store/test/StoreMock.sol"; import { IStoreErrors } from "@latticexyz/store/src/IStoreErrors.sol"; +import { StoreSwitch } from "@latticexyz/store/src/StoreSwitch.sol"; import { Statics, StaticsData, Dynamics1, Dynamics1Data, Dynamics2, Dynamics2Data, Singleton, Offchain, UserTyped, UserTypedData } from "../src/codegen/index.sol"; import { TestTypeAddress, TestTypeInt64, TestTypeLibrary } from "../src/types.sol"; @@ -11,6 +12,19 @@ import { ResourceId } from "@latticexyz/store/src/ResourceId.sol"; import { Enum1, Enum2 } from "../src/codegen/common.sol"; +/** + * @title GetItemValueWrapper + * @dev For testing that calling getItemValue properly reverts + * We use a seperate contract to ensure `expectRevert` does not only check the first external call + */ +contract GetItemValueWrapper { + function getItemValue(address worldAddress, bytes32 key, uint256 _index) public { + StoreSwitch.setStoreAddress(worldAddress); + + Dynamics1.getItemStaticU128(key, _index); + } +} + contract TablegenTest is Test, StoreMock { function testStaticsSetAndGet() public { Statics.register(); @@ -42,6 +56,21 @@ contract TablegenTest is Test, StoreMock { assertEq(abi.encode(Dynamics1.get(key)), abi.encode(emptyData1)); assertEq(abi.encode(Dynamics2.get(key)), abi.encode(emptyData2)); + assertEq(Dynamics1.getStaticU128(key)[0], 0); + assertEq(Dynamics1.getItemStaticU128(key, 0), 0); + assertEq(Dynamics1.getStaticU128(key)[1], 0); + assertEq(Dynamics1.getItemStaticU128(key, 1), 0); + assertEq(Dynamics1.getStaticU128(key)[2], 0); + assertEq(Dynamics1.getItemStaticU128(key, 2), 0); + + // using `get` with indices beyond the static length should revert + GetItemValueWrapper wrapper = new GetItemValueWrapper(); + vm.expectRevert(abi.encodeWithSelector(IStoreErrors.Store_IndexOutOfBounds.selector, 0, 48)); + wrapper.getItemValue(address(this), key, 3); + + vm.expectRevert(abi.encodeWithSelector(IStoreErrors.Store_IndexOutOfBounds.selector, 0, 64)); + wrapper.getItemValue(address(this), key, 4); + // initialize values bytes32[1] memory staticB32 = [keccak256("value")]; int32[2] memory staticI32 = [int32(-123), 123]; diff --git a/packages/store/ts/codegen/field.ts b/packages/store/ts/codegen/field.ts index 2e26efba7e..61bfda4372 100644 --- a/packages/store/ts/codegen/field.ts +++ b/packages/store/ts/codegen/field.ts @@ -137,6 +137,22 @@ export function renderFieldMethods(options: RenderTableOptions): string { "uint256 _index", ])}) internal view returns (${portionData.typeWithLocation}) { ${_keyTupleDefinition} + + ${ + // If the index is within the static length, + // but ahead of the dynamic length, return zero + typeWrappingData && typeWrappingData.kind === "staticArray" && field.arrayElement + ? ` + uint256 _byteLength = ${_store}.getDynamicFieldLength(_tableId, _keyTuple, ${dynamicSchemaIndex}); + uint256 dynamicLength = _byteLength / ${portionData.elementLength}; + uint256 staticLength = ${typeWrappingData.staticLength}; + + if (_index < staticLength && _index >= dynamicLength) { + return ${renderCastStaticBytesToType(field.arrayElement, `bytes${field.arrayElement.staticByteLength}(new bytes(0))`)}; + }` + : `` + } + unchecked { bytes memory _blob = ${_store}.getDynamicFieldSlice( _tableId,