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: correct read mode for storage_has_key #8968

Merged
merged 3 commits into from
Apr 25, 2023

Conversation

Longarithm
Copy link
Member

@Longarithm Longarithm commented Apr 24, 2023

In storage_has_key we were setting KeyLookupMode::FlatStorage by default. This is correct for most reads, but not for reads inside contracts, as it impacts chunk cache and contract call costs.

We must read from flat storage deterministically and derive lookup mode from protocol version.

@Longarithm Longarithm marked this pull request as ready for review April 24, 2023 21:59
@Longarithm Longarithm requested a review from a team as a code owner April 24, 2023 21:59
@akhi3030
Copy link
Collaborator

@pugachAG: can you help review this please?

@akhi3030 akhi3030 requested a review from pugachAG April 25, 2023 06:34
@pugachAG
Copy link
Contributor

LGTM, got only one suggestion:
consider adding mode argument for storage_has_key to be consistent with storage_get

@Longarithm Longarithm self-assigned this Apr 25, 2023
@near-bulldozer near-bulldozer bot merged commit c271196 into near:master Apr 25, 2023
nikurt pushed a commit that referenced this pull request Apr 25, 2023
In `storage_has_key` we were setting `KeyLookupMode::FlatStorage` by default. This is correct for most reads, but not for reads inside contracts, as it impacts chunk cache and contract call costs.

We must read from flat storage deterministically and derive lookup mode from protocol version.
nikurt pushed a commit that referenced this pull request Apr 28, 2023
In `storage_has_key` we were setting `KeyLookupMode::FlatStorage` by default. This is correct for most reads, but not for reads inside contracts, as it impacts chunk cache and contract call costs.

We must read from flat storage deterministically and derive lookup mode from protocol version.
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