Take storage keys as reference (K -> &K)

[webmaster128]

This change encourages the use of ownes types as PrimaryKey, which makes it very easy to deserialize bytes into owned keys. This can be implemented for Addr.

Since you cannot deserialize borrowed bytes into an owned object, a copy is created in parse_key. However, parse_key is never used.

In order to avoid copying around owned keys when thes are needed in multiple locations, the key arguments of all the storage functions were turned into references.

This is basically how the type system of standard library maps like https://doc.rust-lang.org/std/collections/struct.HashMap.html works: You have an owned key type K that is used plain in a few places. But get takes a reference. In HashMap the K can be of unknown size (e.g. [u8]). This is something we cannot do as long as we want to keep parse_key, which deserializes into an owned K.

---

Do not merge as is. Target branch is a copy of the fork from #260.

[maurolacy]

This is nice in that it really "forces" the use of references on the user.

But, it's not different from the case in which you define the key as a reference.
In the original case, the & is flexible / optional, and can go inside the K, whereas in this case the & is fixed (outside the K). And, you can still put it inside of it (leading to double references).

I prefer the former version, for simplicity. Even when you could be using values by defining a value key, that can be easily fixed (by changing the key definition in one place). Plus, you have the freedom to do that if you need to, for whatever reason.

That said, both versions are OK with me. The fact of parse_key deserializing to an owned value is irrelevant / minor, in my opinion.

[webmaster128]

The fact of parse_key deserializing to an owned value is irrelevant, in my opinion.

This is crucial. Before you had K in and K out. If K was a reference, you created the requirement described by Ethan here: CosmWasm/cosmwasm#868 (comment) which is almost impossible to fulfill by anything other than renamed &[u8]s. You cannot put K in and *K out.

But what you can do is put &K in and get K out, which is shown here.

This makes a few things a bit less fancy. But I think those get nicer over time when you migrate K from refs to values consistently. One problem you run into is that the literal b"foo" gives you an &[u8; 3] instead of an &[u8], which makes some things slightly less pretty than before. But this is really just a problem of test code.

[maurolacy]

Agreed. But as you mentioned, parse_key is currently not being used. I think we're creating our own parsers when we need them.

Anyway, both options are OK with me.

[ethanfrey]

Let's revisit this when we tackle #198

For now, going for the much more straightforward #264