Map serde support

[CyberHoward]

Attempt to add serde support to map types. Changes based on serde-json-core

closes #41

[webmaster128]

Beautiful. Diff LGTM. Could you add a CHANGELOG entry to the unreleased section above 0.4.1 for this?

[webmaster128]

I pushed a HashMap test here. This shows it works and has non-deterministic ordering (as expected).

[webmaster128]

The HashMap tests are not very interesting from the CW POV (because random state hashing not being useful in CW I think)

Actually in Wasm the random state hashing is disabled. The point here is that the serialization is as deterministic or undeterministic as the map's iteration order. The serializer DOES NOT sort keys.

[webmaster128]

@CyberHoward do you want to work on the MapKeySerializer issue brought up in https://github.com/CosmWasm/serde-json-wasm/pull/45#pullrequestreview-1183348695? Or should we merge here and I do it myself in a separate PR? Both ways are fine with me.

[hashedone]

Actually in Wasm the random state hashing is disabled

Exactly that was what I meant by that, but I haven't this tweet anywhere in my bookmarks, so I didn't want to make too strong claims not to be wrong.

[CyberHoward]

@CyberHoward do you want to work on the MapKeySerializer issue brought up in https://github.com/CosmWasm/serde-json-wasm/pull/45#pullrequestreview-1183348695? Or should we merge here and I do it myself in a separate PR? Both ways are fine with me.

Sure, I'll try to do it!

[CyberHoward]

The implementation in serde_json parses number types (i32, u64, etc.) into string types through a Formatter while the current implementation refuses numbers as keys. Is this the behavior we want or should I add parsing logic to serialize them into strings?

Also FYI new type Structs and unit Enum variants are considered valid keys. See the added tests.

[webmaster128]

Great stuff. I'd go with best possible serde_json compatibility, such that map created with our lib can be decoded using serde_json and vice versa. But we only need to support a subset of serde_json. Serializing the number types to string does no harm IMO and might be useful for some users.

[CyberHoward]

Deserialization into number key types is missing so this test fails:

#[test]
    fn numbered_keys() {
        let mut ranking: BTreeMap<u64, String> = BTreeMap::new();
        ranking.insert(1, "Elon".to_string());
        ranking.insert(2, "Bazos".to_string());

        assert_eq!(
            from_str::<BTreeMap<u64, String>>(&to_string(&ranking).unwrap()).unwrap(),
            ranking
        );
    }

Also the Deserializer trait of serde does not support 128-bit values so maybe we should not allow serialization of those values?

[webmaster128]

Deserialization into number key types is missing so this test fails:

Could you add that too?

Also the Deserializer trait of serde does not support 128-bit values so maybe we should not allow serialization of those values?

This is just the default implementation which deserializer implementations then override. It is added there for compatibility reasons because the first version of the trait did not have 128 bit. I think we can just implement it and then test deserializing with serde_json in a test.

[CyberHoward]

I think the sanity check is failing due to the outdated Rust version.

[webmaster128]

Right, see #47. Thanks for the hint.

[ethanfrey]

Nice updates @CyberHoward !