Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot parse new () encoding #27

Closed
ethanfrey opened this issue Jan 14, 2021 · 4 comments · Fixed by #28
Closed

Cannot parse new () encoding #27

ethanfrey opened this issue Jan 14, 2021 · 4 comments · Fixed by #28
Assignees

Comments

@ethanfrey
Copy link
Member

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/git.luolix.top-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
Copy link
Member Author

ethanfrey commented Jan 14, 2021

@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
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

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!()
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants