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): return zero for uninitialised static array elements #2613

Merged
merged 22 commits into from
Apr 5, 2024

Conversation

yonadaa
Copy link
Contributor

@yonadaa yonadaa commented Apr 4, 2024

Changes the behaviour of static arrays to return default values on "uninitialised" values. If an array index is less than the static length, but greater than the underlying dynamic length, we return zero.

Copy link

changeset-bot bot commented Apr 4, 2024

🦋 Changeset detected

Latest commit: 5663590

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 24 packages
Name Type
@latticexyz/store Patch
@latticexyz/cli Patch
@latticexyz/dev-tools Patch
@latticexyz/query Patch
@latticexyz/react Patch
@latticexyz/store-indexer Patch
@latticexyz/store-sync Patch
@latticexyz/world-modules Patch
@latticexyz/world Patch
@latticexyz/abi-ts Patch
@latticexyz/block-logs-stream Patch
@latticexyz/common Patch
@latticexyz/config Patch
create-mud Patch
@latticexyz/faucet Patch
@latticexyz/gas-report Patch
@latticexyz/protocol-parser Patch
@latticexyz/recs Patch
@latticexyz/schema-type Patch
@latticexyz/services Patch
solhint-config-mud Patch
solhint-plugin-mud Patch
@latticexyz/utils Patch
mock-game-contracts Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@yonadaa yonadaa changed the title fix: return zero for initialised static array elements fix(store): return zero for uninitialised static array elements Apr 4, 2024
"@latticexyz/cli": patch
---

Fixed the behaviour of static arrays, so that they always return zero for uninitialised values. Previously they would revert with `Store_IndexOutOfBounds` if the index had not been set yet.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make a note about this matching native solidity behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that! lemme think

alvrs
alvrs previously approved these changes Apr 4, 2024
// but ahead of the dynamic length, return zero
typeWrappingData && typeWrappingData.kind === "staticArray" && field.arrayElement
? `
uint256 _byteLength = ${_store}.getDynamicFieldLength(_tableId, _keyTuple, ${dynamicSchemaIndex});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect this to add gas but not seeing a gas report change. Are we not testing for gas on static-length array fields?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wondered that also. It seems like we don't report table libraries for this (or at all?)

@holic holic merged commit b798ccb into main Apr 5, 2024
6 of 12 checks passed
@holic holic deleted the yonadaaa/static-array-oob-length branch April 5, 2024 03:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants