desmos-labs / desmos-contracts

A collection of cosmWASM contracts for desmos network
Apache License 2.0
10 stars 5 forks source link

Improve `DesmosQuery` and `DesmosMsg` with serde tag #21

Closed dadamu closed 2 years ago

dadamu commented 2 years ago

Currently the structure of DesmosMsg is like:

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
#[serde(rename_all = "snake_case")]
pub struct DesmosMsg {
    pub route: DesmosRoute,
    pub msg_data: DesmosMsgRoute,
}

It could be simplified with serde tag attribute, say:

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
#[serde(rename_all = "snake_case", tag = "route", content = "msg_data")]
pub enum DesmosMsg {
    Profiles(ProfilesMsg),
    Subspaces(SubspacesMsg),
}

To produce the json like:

{
    "route": "subspace",
    "msg_data": {
        "create_subspace": {
            "name": "test",
            "description": "test",
            "......"
        }
    }
}

We can do the same way to DesmosQuery as well. It will reduce some code related to DesmosRoute. What do you think? @bragaz

Note:

it will break the current wasm part of modules in the desmos chain since the json format is different from the current one.

Reference: https://serde.rs/enum-representations.html#adjacently-tagged

dadamu commented 2 years ago

BTW, I have created a branch for testing it. https://github.com/desmos-labs/desmos-contracts/tree/paul/refactor-route

leobragaz commented 2 years ago

Nice one! This rename feature from serde is very powerful! I guess we can make this change, but we need to add also some docs lines in the file to let others that are not familiar with serde to properly understand the signature (maybe you can put the reference you put in the note directly?).

dadamu commented 2 years ago

@bragaz Sure, will do it with a PR.