babylonlabs-io / babylon-contract

CosmWasm smart contracts for Babylon integration
Other
10 stars 6 forks source link

Add json support to protobuf structs #6

Closed maurolacy closed 1 month ago

maurolacy commented 3 months ago

@maurolacy cloned issue babylonchain/babylon-contract#71 on 2024-04-24:

That way they can be used as messages for CosmWasm execution handlers, and return types for query handlers; probably more.

For these structs to work with CosmWasm, we would need #[cw_serde]-equivalent support.

That means, supporting / deriving cosmwasm_schema::serde::Serialize, cosmwasm_schema::serde::Deserialize, Clone, Debug, PartialEq, ::cosmwasm_schema::schemars::JsonSchema

Perhaps we can live without JsonSchema, i.e. not published json schemas, at least for internal messages / structs. And query outputs.

But we at least need Serialize for queries, and Deserialize for messages.

And, we would want to use cosmwasm_schema::serde instead of serde, so that serde-json-wasm is used under the hood for serialisation / deserialisation.

This is important for compatibility, but it's something that can perhaps be solved / tackled after stock serde support is achieved.

maurolacy commented 3 months ago

@maurolacy commented on 2024-04-24:

Also, listed here for completeness, another option would be to switch to json for IBC communication, define the structs as part of Babylon, and export those to contracts and customers.

With the aim of using a unified format for both IBC and message handlers, whenever that's possible.

maurolacy commented 3 months ago

@maurolacy commented on 2024-05-29:

If we use Cosmos SDK for Rust types, provided by tendermint-proto and cosmos-sdk-proto, it seems the only missing thing would be:

  • JsonSchema / schemars support. See if we can either, go without this, or add JsonSchema to the needed nested structs somehow.

  • serialisation / deserialisation and schemas for Bitcoin related types.

maurolacy commented 3 months ago

@SebastianElvis commented on 2024-05-30:

Should we ask CometBFT / Cosmos SDK people why json is not supported? It seems that they do not consider use cases in CosmWasm.

maurolacy commented 3 months ago

@maurolacy commented on 2024-07-02:

I would just get rid of JsonSchema altogether.

maurolacy commented 3 months ago

@maurolacy commented on 2024-07-02:

Another issue is, byte array fields (#133). Go encodes them as Base64 (which is not covered by the RFC-7159 mandated format, btw), whereas Rust expect them as JSON arrays.

So, if we want to use naked byte array fields as part of the contracts API, we need to be able to either, encode them as JSON arrays on the Go side or, decode them as Base64 strings on the Rust side.

Was trying to do the later (Add Base64 decoding for byte arrays), to no avail. Perhaps there's a way to encode them as JSON arrays on the Go side. Posted some comments on SO, without success so far.

Seems encoding these byte arrays as Base64 is common practice, which comes from Java. Go json package devs just blindly followed that common practice (which is non-standard) whereas as Rust serde-json devs (sensibly) decided not to.

maurolacy commented 3 months ago

@maurolacy commented on 2024-07-02:

This is the best we can do atm:

Declare byte arrays as Binary on the Rust side, but as []byte on the Go side. That way, Go will encode them as Base64 by default (without us having to do anything). And, they'll be decoded just fine on the Rust side.

I think this is the simplest solution by now. We still cannot re-use the

Protobuf definitions as JSON definitions on the contracts side (because they'll need the Binary wrapper / annotation). But at least, we can get rid of the ugly explicit conversions to Base64 in the Go side.

maurolacy commented 3 months ago

@maurolacy commented on 2024-07-03:

Declare byte arrays as Binary on the Rust side, but as []byte on the Go side.

Just confirmed that this works, by changing the StakingTx type from string to []byte here

https://www.github.com/babylonchain/finality-provider/blob/585aec5b5d444be792ea01bed14e83c4434a3aef/clientcontroller/cosmwasm/msg.go#L114

And adapting the tests, which ran just fine.

Update: See https://www.github.com/babylonchain/finality-provider/pull/462.

maurolacy commented 1 month ago

Closing as obsolete / not doable.

For sending data, the alternative is, if possible, to encode protobuf payload structs as binary non-json data. That is, using Binary::new and Binary::from instead of to_json_binary.