Update WasmMsg types #213
Update WasmMsg types #213
Conversation
| @@ -209,6 +209,8 @@ type InstantiateMsg struct { | |||
| Send Coins `json:"send"` | |||
| // Label is optional metadata to be stored with a contract instance. | |||
| Label string `json:"label"` | |||
| // Admin (optional) may be set here to allow future migrations from this address | |||
| Admin string `json:"admin,omitempty"` | |||
There was a problem hiding this comment.
Let's make this a *string to match the Rust type Option<String>
There was a problem hiding this comment.
Label is also optional. *string is kind of ugly in Go, so I avoid it. Go standard is to treat "" as nil. Unless an empty string and nil have different actions (eg. a patch for a profile may use this, so *description == "" clears the description while description == nil leaves the description unchanged)
We try to stick with the most idiomatic structs in each language. And there is no valid "" for Admin (it must be a long bech32 string)
There was a problem hiding this comment.
The difference for Label is that we decided to make it a simple String in Rust as well.
I think using plain string works in this particular case, but only we throw away the empty/nil differentiation and because we only use this from Rust -> Go.
It would be nice if we could establish a type pattern that is consistent over all places. With omitempty we send undefined from Go to Rust where string or null is expected. This probably fails when decoding. Also the Go side should error for unexpected JSON values (as this is a JSON interface):
nulland bech32 is valid- Empty string is an error
- All other JSON types are an error
- Unset is an error
There was a problem hiding this comment.
It would be nice if we could establish a type pattern that is consistent over all places. With
omitemptywe send undefined from Go to Rust where string or null is expected. This probably fails when decoding.
No, I have tested this quite a bit last year. Option<T> happy converts null and a missing field to None.
I had issues when Go converted an empty array to null and then we tried to parse into Vec<T>. In that case I overrode GO serialization to produce [] for empty arrays and keep the Rust type sane.
I would like @alpe feedback here, as this is really about keeping canonical Go types. In generally if there is no strong reason to differentiate between "zero-values" and "null", you treat them the same in Go. There is no simple way to differentiate null from undefined (missing field), even if you use *string.
There was a problem hiding this comment.
I need to second Ethan in this discussion. The*string types are very uncommon in Go and only when nil and empty string are treated differently. omitempty describes it well as optional field (for any Go developer).
For this field we use the string type already in other places. For example: https://github.com/CosmWasm/wasmd/blob/master/proto/cosmwasm/wasm/v1beta1/tx.proto#L55
|
Let's merge/cherry pick this as part of a beta5 upgrade to avoid polluting git history with binary files. |
Yes, I definitely wish to include this with the other updates as one PR |
|
Merged as part of #215 |
fixed recursive queries losing context
maybe we can combine this with other in-progress cosmwasm changes.
Also see CosmWasm/cosmwasm#889