CosmWasm / serde-json-wasm

serde_json for Wasm programs (small, deterministic, no floats)
Apache License 2.0
30 stars 15 forks source link

Cannot parse new `()` encoding #27

Closed ethanfrey closed 3 years ago

ethanfrey commented 3 years ago

As of #24 we now support encoding (), such as in ContractResult<()>

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
#[serde(rename_all = "snake_case")]
pub enum ContractResult<S> {
    Ok(S),
    #[serde(rename = "error")]
    Err(String),
}

In my tests, ContractResult::Ok(()) will encode to {"ok":null} properly. However, when I try to deserialize it, I get an error:

use cosmwasm_std::from_slice;
let ack: ContractResult<()> = from_slice(&res.acknowledgement).unwrap();

Error:

thread 'contract::tests::handle_packet' panicked at 'internal error: entered unreachable code', /home/ethan/.cargo/registry/src/github.com-1ecc6299db9ec823/serde-json-wasm-0.2.2/src/de/mod.rs:414:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Note: Tested with v0.2.2

ethanfrey commented 3 years ago

@webmaster128 looks like using () here instead of bool has caused a few headaches. I think if parsing is fixed, then all tests should work. I will try to parse with serde-json as well

webmaster128 commented 3 years ago

Right, deserializing into () was not tested. It should be easy to add.

I wonder where you need this. For contract -> VM communication (like ContractResult) we use serde-json-wasm for serializing and serde-json for deserializing.

ethanfrey commented 3 years ago

It shows up in the unit tests: https://github.com/CosmWasm/cosmwasm/blob/bcd465b3e1307c21a7d23ac5d9f67908301df71b/contracts/ibc-reflect/src/contract.rs#L398-L404

It is not blocking the PR, but one step of the unit test is disabled. I guess I could directly use serde_json::from_slice there, but that seems rather ugly to mix. The same test passes in the integration test: https://github.com/CosmWasm/cosmwasm/blob/bcd465b3e1307c21a7d23ac5d9f67908301df71b/contracts/ibc-reflect/tests/integration.rs#L218-L220

webmaster128 commented 3 years ago

Ahh, okay! Good to know this is used.

Not implemented:

    /// Unsupported. Use a more specific deserialize_* method
    fn deserialize_unit<V>(self, _visitor: V) -> Result<V::Value>
    where
        V: Visitor<'de>,
    {
        unreachable!()
    }