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

untagged enum causes F64Load vm gatekeeper's error #1443

Closed
iboss-ptk opened this issue Sep 29, 2022 · 10 comments
Closed

untagged enum causes F64Load vm gatekeeper's error #1443

iboss-ptk opened this issue Sep 29, 2022 · 10 comments

Comments

@iboss-ptk
Copy link

When introducing #[serde(untagged)] to a contract, it causes:

"failed to execute message; message index: 0: Error calling the VM: Error compiling Wasm: Could not compile: WebAssembly translation error: Error in middleware Gatekeeper: Float operator detected: F64Load { memarg: MemoryImmediate { align: 3, offset: 0, memory: 0 } }. The use of floats is not supported.: create wasm contract failed"

Even with simple enum like:

#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(untagged)]
enum Simple {
    A(String),
}

so even with serde_json_wasm supports untagged it's currently not working with cosmwasm contract.

cosmwasm_std = "1.1.2"
wasmd = "https://github.com/osmosis-labs/wasmd/tree/v0.28.0-osmo-v12.1"
@chris-ricketts
Copy link
Contributor

I came up against this too after following the nested example in the cosmwasm_schema::QueryResponses docs.

It works if you remove #[serde(untagged)].

@tubackkhoa
Copy link

You can use cfg macro to fix this, one for building wasm and one for generating schema

#[cfg(not(target_arch = "wasm32"))]
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(untagged)]
enum Simple {
    A(String),
}

#[cfg(target_arch = "wasm32")]
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
enum Simple {
    A(String),
}

@Art3miX
Copy link

Art3miX commented Jun 14, 2023

Its an old issue, but I think this feature is very needed in CW, and can make contracts much nicer to look at and work with.
Here is one example: cosmos/ibc-go#3796
Does anyone have a solution? or the team is working on a fix?
@ethanfrey, not sure who is the best person to tag on this.

@webmaster128
Copy link
Member

I'm not sure about the state of untagged support. Would be great to test this heaviliy in serde_json_wasm. But float support is on the agenda for this year, avoiding all of those issues.

@ethanfrey
Copy link
Member

cosmos/ibc-go#3796

Reading this issue, I see: "In this implementation, our main goal is to support json messages coming from cosmwasm. Communicating go modules should use protobuf encoding."

The interesting point is that (1) there are nice libraries (from Osmosis) to generate protobuf sdk types (for StargateMsg) and (2) the proposed JSON doesn't work at all in CosmWasm.

If the main point of this ibc-go PR is to for easier support from CosmWasm contracts, then someone (like @Art3miX who flagged this) should talk with them so they modify their format to produce something that is CosmWasm compatible. And also ask about their process, so they check this actually works on the Rust side. In particular, the existence of a unique type_url for each entry would make Adjacent tagging a distinct possibility, which may well work in wasm (someone should verify that).

@srdtrk
Copy link
Contributor

srdtrk commented Jun 14, 2023

Hi @ethanfrey, I've opened that PR in ibc-go, and we've been in communication with @Art3miX. I was aware that the rust implementation was not optimal but wasn't aware that it would panic in wasmvm. We're in communication and will update that example once we come to a good resolution

@Art3miX
Copy link

Art3miX commented Jun 14, 2023

Hey Ethan, Thank you for your suggestion, unfortunately tags also uses floats, so that doesn't work:

#[derive(Serialize, Deserialize, Clone, Debug)]
#[serde(tag = "@type")]
pub enum CosmosMessages {
  #[serde(rename = "/cosmos.gov.v1beta1.MsgSubmitProposal")]
    MsgSubmitProposalV1 {
        content: Box<CosmosMessages>,
        initial_deposit: Vec<Coin>,
        proposer: String,
    },
    #[serde(rename = "/cosmos.gov.v1beta1.TextProposal")]
    TextProposal {
        title: String,
        description: String,
    },
}

I gave the ibc-go as an example of a use case for untagged support, another example would be for receive/callback msgs, you could use the same msg to receive multiple types of receive msgs, a specific example would be of receiveNft, cw721 and ics721 could utilize the same ReceiveNft msg, and a contract can handle those as it see fit, for example:

#[cw_serde]
#[serde(untagged)]
pub enum AcceptedReceiveNftMsgs {
  Cw721 (Cw721ReceiveMsg)
  Ics721 (Ics721ReceiveMsg)
  Other (OtherReceiveMsg)
}

#[cw_serde]
pub enum ExecuteMsg {
  ReceiveNft (AcceptedReceiveNftMsgs)
}

// contract.rs
match msg {
  ExecuteMsg::ReceiveNft (AcceptedReceiveNftMsgs::Other (other)) => handle_other_receive(other),
  ExecuteMsg::ReceiveNft (receiveMsg) => handle_receive(receiveMsg)
}

The alternative is to use something like this:

pub enum ExecuteMsg {
  ReceiveNft (cw721::Cw721ReceiveMsg),
  ReceiveNftIcs721 (ics721::Ics721ReceiveMsg),
  ReceiveNftOther (other::OtherReceiveMsg)
}

@srdtrk
Copy link
Contributor

srdtrk commented Jun 15, 2023

For the interchain accounts feature we discussed, we only need serialization to work (and deserialization is not a priority). The tags don't work only if you use deserialize. So, this suggestion actually works!

We didn't go with a more cosmwasm friendly json format because we simply wanted to import the sdk's JSONCodec methods rather than implementing our own. (I did actually write a cosmwasm friendly json encoder/decoder but the team and I have agreed that using the sdk's codec makes more sense). I think relying on the sdk and expecting cosmwasm to produce the same format is acceptable as long as there is a simple solution like @Art3miX posted. Since we don't need deserialization at the moment, I think this is enough for our feature.

@ethanfrey
Copy link
Member

Thank you two for working this out.

I agree float support (or better untagged) is a nice longer-term goal, but glad this is not blocking your work.

@webmaster128
Copy link
Member

Float support is done in #1845 to be released with CosmWasm 1.5.

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

No branches or pull requests

7 participants