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

range to range raw #576

Merged
merged 47 commits into from
Dec 14, 2021
Merged

range to range raw #576

merged 47 commits into from
Dec 14, 2021

Conversation

maurolacy
Copy link
Contributor

@maurolacy maurolacy commented Dec 13, 2021

Closes #460.

Lots of changes, but basically renaming range to range_raw, and range_de to range.

  • Deprecates range in favour of range_de, keeping range as range_raw.
  • Keeps prefix_de as prefix, and remove the currents prefix. That is, no prefix_raw impl, which will be confusing. That implies making range_raw work with / over prefix_de.
  • Migrates the code in contracts to take advantage of the new key deserialization when useful.
  • Improve traits segregation over Map and Prefix range-related methods, so that trait bounds are enforced only when strictly needed. (out of scope for this PR)

Could have chosen to provide two prefix functions, prefix and prefix_raw, but opted instead to provide one prefix with two ranges over it, range and range_raw, for simplicity of use. The only drawback is that if you want to prefix, you need to provide key deserialization impls for the involved keys, even if / when using only range_raw. This is reasonable IMO, as we are providing many of those impls already.

Most interesting stuff is in the contracts, where we leverage the new key deserialization functionality for clarity and convenience.

Remove MultiIndex::prefix()
Adjust MultiIndex::no_prefix() trait bounds
Copy link
Member

@ethanfrey ethanfrey left a comment

Choose a reason for hiding this comment

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

I like the idea.
Have not done a thorough review

contracts/cw1-subkeys/src/contract.rs Show resolved Hide resolved
packages/storage-plus/src/indexes/multi.rs Show resolved Hide resolved
packages/storage-plus/src/indexes/multi.rs Show resolved Hide resolved
Copy link
Contributor

@ueco-jb ueco-jb left a comment

Choose a reason for hiding this comment

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

lgtm

@maurolacy maurolacy merged commit 90c49f4 into main Dec 14, 2021
@maurolacy maurolacy deleted the 460-range-to-range_raw branch December 14, 2021 13:34
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.

Deprecate range to range_raw
3 participants