Update to cosmwasm 0.8 #68
Conversation
|
If someone could take a look at help figure out why |
Looking at the error message, there must be an ApiError::ContractError
Maybe this new error case? Update: CosmWasm/cosmwasm@bbe1cbe |
There was a problem hiding this comment.
I had to go somewhere in the middle of my review, so it took longer to wrap up than expected. There are missing documentation comments here and there, and plenty of TODO comments but i'm sure you won't leave those unchecked.
I couldn't find the source of the error you mention by statically analyzing the code, so i'll try running/debugging it now
types/types.go
Outdated
| type ApiError struct { | ||
| Base64Err *SourceErr `json:"base64_err,omitempty"` | ||
| ContractErr *MsgErr `json:"contract_err,omitempty"` | ||
| DynContractErr *MsgErr `json:"dyn_contract_err,omitempty"` | ||
| NotFound *NotFoundErr `json:"not_found,omitempty"` | ||
| NullPointer *struct{} `json:"null_pointer,omitempty"` | ||
| ParseErr *JSONErr `json:"parse_err,omitempty"` | ||
| SerializeErr *JSONErr `json:"serialize_err,omitempty"` | ||
| Unauthorized *struct{} `json:"unauthorized,omitempty"` | ||
| UnderflowErr *UnderflowErr `json:"underflow_err,omitempty"` | ||
| Utf8Err *SourceErr `json:"utf8_err,omitempty"` | ||
| Utf8StringErr *SourceErr `json:"utf8_string_err,omitempty"` | ||
| ValidationErr *ValidationErr `json:"validation_err,omitempty"` | ||
| } |
There was a problem hiding this comment.
I'm not that well versed in Go, but is it maybe better to define this as something akin to a pointer to a trait object in rust? that way the layout of this type won't be so big.
There was a problem hiding this comment.
Yes, I can try to use a (private) interface and then separate types/structs for each of these cases, and use a type-switch. This guarantees 0 or 1 elements (not 0 to N) and will be a smaller struct, but actually a fair bit more code for the structs.
There was a problem hiding this comment.
That probably is more idiomatic Go as well
There was a problem hiding this comment.
I started doing this for errors and it is a lot of code. And they are being overhauled right now..
I will update messages to work like this, and then update errors once CosmWasm/cosmwasm#271 is merged and we update go-cosmwasm to that commit
There was a problem hiding this comment.
Oof... I did this for CosmosMsg, then I realized something...
interface{} types can be json serialized, but not deserialized (missing info on how), so I will need more custom code there.
I was trying the approach used by protobuf, but they use codegen and determine the bytes and cast it to a type manually (in generated code).
I will stick with the current (ugly) solution. But at least I know why I did this
There was a problem hiding this comment.
Sounds reasonable. The overhead here isn't too terrible, and it's definitely fixable in the future since there's zero effect on exposed interfaces afaik.
How/Where do these types propagate to the cosmos sdk and eventually users of the CLI or client libraries?
|
I made PRs for follow-ups. I would like to merge this in to have a solid base, and we can keep making PRs (in parallel) to add features to get 0.8 feature complete. If I forget some other feature, please add it to the list |
Rest server fixes
This updates
go-cosmwasmto the current master ofcosmwasmand updates tocontract_0.8.wasm. This now works to 90%. There is some error with the humanize_address callback that I need to track down and repair.There are also a number of TODOs to get full working version, some of which may be in future PRs. This is tracked in https://github.com/CosmWasm/go-cosmwasm/blob/update-cosmwasm-0.8/TODO
Planned for this PR:
removefor DBPlanned for future PRs (linked to this PR):
[]notnull(transform on Rust side if needed) - need to find sample failure caseQueriercallback to go code