fix(08-wasm)!: use concrete types for json serialization #4909
Conversation
|
With this PR, I could successfully get a test scaffolding together like this for |
damiannolan
requested review from
colin-axner and
AdityaSripal
as code owners
damiannolan
requested review from
crodriguezvega,
chatton,
DimitrisJim,
charleenfei and
srdtrk
| @@ -86,9 +86,14 @@ func (cs ClientState) GetTimestampAtHeight( | |||
| cdc codec.BinaryCodec, | |||
| height exported.Height, | |||
| ) (uint64, error) { | |||
| timestampHeight, ok := height.(clienttypes.Height) | |||
| if !ok { | |||
| return 0, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected %T, got %T", clienttypes.Height{}, height) | |||
There was a problem hiding this comment.
| return 0, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected %T, got %T", clienttypes.Height{}, height) | |
| return 0, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected %T, got %T", (*clienttypes.Height)(nil), height) |
There was a problem hiding this comment.
we mostly use the approach used here. We should open a larger issue about fixing the way we do this tho since I see we're not consistent all over the place, not just 08-wasm.
There was a problem hiding this comment.
Relayers provide proof heights as values and not as pointers https://github.com/cosmos/ibc-go/blob/main/proto/ibc/core/channel/v1/tx.proto#L185
| if !ok { | ||
| return errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected %T, got %T", commitmenttypes.MerklePath{}, path) | ||
| } | ||
|
|
||
| proofHeight, ok := height.(clienttypes.Height) | ||
| if !ok { | ||
| return errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected %T, got %T", clienttypes.Height{}, height) |
There was a problem hiding this comment.
| return errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected %T, got %T", clienttypes.Height{}, height) | |
| return errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected %T, got %T", (*clienttypes.Height)(nil), height) |
I think there are other places in the code where this is done. So I might just open a separate PR for this.
| if !ok { | ||
| return errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected %T, got %T", commitmenttypes.MerklePath{}, path) | ||
| } | ||
|
|
||
| proofHeight, ok := height.(clienttypes.Height) | ||
| if !ok { | ||
| return errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected %T, got %T", clienttypes.Height{}, height) |
| UpgradeClientState: *wasmUpgradeClientState, | ||
| UpgradeConsensusState: *wasmUpgradeConsState, |
There was a problem hiding this comment.
This will panic if wasmUpgradeClientState or wasmUpgradeConsState is nil. Is it possible to assert this earlier and possibly handle it as an error there?
Not a big deal though as it's probably ok for it to panic. Or it is somehow not possible for it to reach here if it were nil.
There was a problem hiding this comment.
nil is handled by type assertion. Type assertions yield false if the underlying value of interface you're trying to cast from is nil or != T (target type).
Description
Removes usage of
exportedinterface types in the contract api types to allow clean codec usage with native Go implementations.We should run an e2e regression with this but encoding should be fine.
closes: #4908
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/).godoccomments.Files changedin the Github PR explorer.Codecov Reportin the comment section below once CI passes.