Skip to content

Commit

Permalink
fix(store): return zero for uninitialised static array elements (#2613)
Browse files Browse the repository at this point in the history
Co-authored-by: alvarius <[email protected]>
Co-authored-by: Kevin Ingersoll <[email protected]>
  • Loading branch information
3 people authored Apr 5, 2024
1 parent e3c3a11 commit b798ccb
Show file tree
Hide file tree
Showing 6 changed files with 210 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .changeset/popular-oranges-end.md
Original file line number Diff line number Diff line change
@@ -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.
32 changes: 32 additions & 0 deletions e2e/packages/contracts/src/codegen/tables/StaticArray.sol

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

80 changes: 80 additions & 0 deletions packages/cli/contracts/src/codegen/tables/Dynamics1.sol

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

48 changes: 48 additions & 0 deletions packages/cli/contracts/src/codegen/tables/Singleton.sol

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 29 additions & 0 deletions packages/cli/contracts/test/Tablegen.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,27 @@ 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";
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();
Expand Down Expand Up @@ -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];
Expand Down
16 changes: 16 additions & 0 deletions packages/store/ts/codegen/field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit b798ccb

Please sign in to comment.