-
Notifications
You must be signed in to change notification settings - Fork 185
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
feat(store): expose getStaticField
and getDynamicField
on IStore
and use it in codegen tables
#1521
Conversation
🦋 Changeset detectedLatest commit: 66522b7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 28 packages
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 |
@@ -572,12 +572,49 @@ library StoreCore { | |||
FieldLayout fieldLayout | |||
) internal view returns (bytes memory) { | |||
if (fieldIndex < fieldLayout.numStaticFields()) { | |||
return StoreCoreInternal._getStaticField(tableId, keyTuple, fieldIndex, fieldLayout); | |||
return StoreCoreInternal._getStaticFieldBytes(tableId, keyTuple, fieldIndex, fieldLayout); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need a method that returns the field as dynamic bytes memory
with the correct length here
bytes32 tableId, | ||
bytes32[] calldata keyTuple, | ||
uint8 fieldIndex, | ||
FieldLayout fieldLayout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you think it would make sense to flip these last two args? (might be a bigger refactor for other methods though)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added an issue to get back to this: #1521 (comment) (i remember we had talked about moving the Schema
argument to another location too before it got replaced by FieldLayout
)
Fixes #1519
Followup to #1512
Storage.loadField
utility to separategetField
intogetStaticField
andgetDynamicField
for better gas performancegetStaticField
andgetDynamicField
onIStore
thanks @Boffee for finding this optimization