Full deserialization for range

[maurolacy]

Closes #198.

First (working) attempt at deserializing range result keys.

TODO:

• Naming
  • Proper naming for the new range and Prefix functionality. Perhaps range_de, PrefixDe?
  • Proper naming for Pair2. Perhaps consolidating Pair2 and Pair into one (with default args for both key and value). (See Make Pair more extensible cosmwasm#1101)
  • Better internal naming of deserialization functions, associated types, etc. Perhaps renaming deserialize_kv - > deserialize_v, and deserialize2_kv -> deserialize_kv.
  • Better naming for Deserializable. Perhaps DeserializeOwned (although confusing with serde).
  • Put Prefix2 into its own module? Consolidate Prefix and Prefix2 (default K = Vec<u8>) (Follow-up PR Prefix consolidation #439)?
• keys() deserialization (keys_de).
• Implement range_de for the other iterables (IndexedMap, IndexedSnapshotMap, and the indexes). (Add range_de to Map-like structs #461)
• Make prefix_de deserialization (i.e. suffixes deserialization) work. (Follow-up PR Working prefix_de #434).
• Lifetimes / references. Currently only owned approach (Follow / take a look at serde's approach here). (Add non-owned range_de #463)
• Implement Deserializable for other types.
  • String types.
  • Integer types.
  • Tuples.
  • ...
• Unit testing. (wip)

[maurolacy]

This is now "rebased", and properly supporting the no-iterator feature.

[ethanfrey]

Seems like a good implementation.

I would prefer improved names and some other details, but I think you have a few PRs coming on top that will fix that. Approving for merge, with the condition these comments are addressed in a future PR.

[ethanfrey]

Please take a look at #446
This adds type-safe ranges.

I think using Bound<Vec<u8>> with no connection to the Map keys. And then using joined_keys and such manually is just a recipe for disaster. You may be able to clean up that PR, but I think the idea of creating Bounds with some K: PrimaryKey or K: Prefixer will catch a whole lot of issues when writing code.

I know the crux of your PR is to add more deserialisation (of K), but I think while doing some of you work, and removing EmptyPrefix, you went more in the direction of people just manually appending prefixes.

[ethanfrey]

Is this ready for a deep review?
When all this stuff is updated, ping me, and I can do one pass over this. But I don't want to while it is still a WIP

[maurolacy]

Is this ready for a deep review?

I think so, yes.

[ethanfrey]

This is a big one!

First, I like the type changes - both KeyDeserializer (nicer naming) and the additions to PrimaryKey.

I also like the addition of key information to Prefix.

Added some cleanup comments, and maybe some redundancy can be removed, but I think just one pass with some polish and this can be merged.

I think there is more than can be done to enhance it (in other PRs).

• properly handling prefix_range_de
• deprecating range and promoting range_de as the standard case (I would almost be in favor of just renaming them now and break everything once... range -> range_raw and range_de -> range.
• replace KeyDeserializer::from_slice(&[u8]) with KeyDeserializer::from_vec(Vec<u8>). When we call it, we can consume the Vec and save a clone. See deserialize_kv
• Add range_de types to "Map-like" structs (Indexed, Snapshot)
• Simplify usage in many contracts where we use range and custom parsing (but could now use range_de)

No need to handle that in this PR. Tackle what you can and make an issue for the items you will put into other PRs.

[maurolacy]

I think there is more than can be done to enhance it (in other PRs).

• properly handling prefix_range_de

Yes. Will do in a follow-up. (done)

• deprecating range and promoting range_de as the standard case (I would almost be in favor of just renaming them now and break everything once... range -> range_raw and range_de -> range.

I would love to do that renaming. Let's do it in a follow-up, just to better handle the breaking change. (#460).

• replace KeyDeserializer::from_slice(&[u8]) with KeyDeserializer::from_vec(Vec<u8>). When we call it, we can consume the Vec and save a clone. See deserialize_kv

Will do in a follow-up. (#464)

• Add range_de types to "Map-like" structs (Indexed, Snapshot)

Another follow-up. (#461)

• Simplify usage in many contracts where we use range and custom parsing (but could now use range_de)

For sure. I would do this as part of the breaking change from range -> range_raw. (#461)

No need to handle that in this PR. Tackle what you can and make an issue for the items you will put into other PRs.

Good review, thanks. Will create the follow-up issues and link them here. (done)

[maurolacy]

Will add unit tests for key deserialization, and I think then we can merge this.

[ethanfrey]

Sounds good.

If you can get a 🟢 from the CI, I'm happy to merge it. Let's do this and

Replace KeyDeserializer::from_slice(&[u8]) with KeyDeserializer::from_vec(Vec). When we call it, we can consume the Vec and save a clone. See deserialize_kv. (From #464)

And update to final cosmwasm release. Then we can cut a v0.10.0

[ethanfrey]

looks good.
let's merge

[ethanfrey]

nice

outdated, changes already made

[ethanfrey]

Another approval in case the last one was marked as stale