chore: adding initial localhost client testing #3085
Conversation
| merklePath, ok := path.(commitmenttypes.MerklePath) | ||
| if !ok { | ||
| return sdkerrors.Wrapf(clienttypes.ErrFailedChannelStateVerification, "todo: update error -- not found for path %s", path) | ||
| } | ||
|
|
||
| // todo: revisit this | ||
| if len(merklePath.GetKeyPath()) != 2 && merklePath.KeyPath[0] != exported.StoreKey { | ||
| return sdkerrors.Wrapf(clienttypes.ErrFailedChannelStateVerification, "todo: update error -- not found for path %s", path) | ||
| } | ||
|
|
||
| bz := store.Get([]byte(merklePath.KeyPath[1])) |
There was a problem hiding this comment.
could be a good candidate for an unexported function.
func readPathBytes(path exported.Path,) ([]byte, error) {...}There was a problem hiding this comment.
I think its more or less ok as is, given its a relatively small method compared to solomachine or tm
| merklePath, ok := path.(commitmenttypes.MerklePath) | ||
| if !ok { | ||
| return sdkerrors.Wrapf(clienttypes.ErrFailedChannelStateVerification, "todo: update error -- not found for path %s", path) | ||
| } | ||
|
|
||
| // todo: revisit this | ||
| if len(merklePath.GetKeyPath()) != 2 && merklePath.KeyPath[0] != exported.StoreKey { | ||
| return sdkerrors.Wrapf(clienttypes.ErrFailedChannelStateVerification, "todo: update error -- not found for path %s", path) | ||
| } | ||
|
|
||
| bz := store.Get([]byte(merklePath.KeyPath[1])) |
There was a problem hiding this comment.
could use the function I suggested above here too
damiannolan
requested review from
colin-axner and
AdityaSripal
as code owners
|
Created #3099 to address error returns in a follow-up PR. |
| @@ -23,11 +24,6 @@ func NewClientState(chainID string, height clienttypes.Height) exported.ClientSt | |||
| } | |||
| } | |||
|
|
|||
| // GetChainID returns the client state chain ID. | |||
| func (cs ClientState) GetChainID() string { | |||
There was a problem hiding this comment.
seems unnecessary and unused
| return sdkerrors.Wrapf(clienttypes.ErrFailedChannelStateVerification, "todo: update error -- not found for path %s", path) | ||
| } | ||
|
|
||
| bz := store.Get([]byte(merklePath.KeyPath[1])) |
There was a problem hiding this comment.
merklePath.KeyPath is a []string where the first element of the key path "/ibc/" (the prefix that is applied here), but here we pass the core IBC store for verification, so we can trim the prefix or use keyPath[1] (i.e. the key path without the prefix)
There was a problem hiding this comment.
I feel like we should include this reasoning in a comment alongside these checks.
There was a problem hiding this comment.
good call, done!
There was a problem hiding this comment.
Should we verify that the store is prefixed by KeyPath[0]? I don't know if there's an easy way to do that
| return sdkerrors.Wrapf(clienttypes.ErrFailedChannelStateVerification, "todo: update error -- not found for path %s", path) | ||
| } | ||
|
|
||
| bz := store.Get([]byte(merklePath.KeyPath[1])) |
There was a problem hiding this comment.
I feel like we should include this reasoning in a comment alongside these checks.
| @@ -153,8 +169,10 @@ func (cs ClientState) UpdateStateOnMisbehaviour(_ sdk.Context, _ codec.BinaryCod | |||
| func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, clientMsg exported.ClientMessage) []exported.Height { | |||
| height := clienttypes.GetSelfHeight(ctx) | |||
|
|
|||
| clientState := NewClientState(ctx.ChainID(), height) | |||
| clientStore.Set([]byte(host.KeyClientState), clienttypes.MustMarshalClientState(cdc, clientState)) | |||
| cs.ChainId = ctx.ChainID() | |||
There was a problem hiding this comment.
Maybe should be a separate PR. But do we need the chainID if the function got removed above
Description
closes: #XXXX
Commit Message / Changelog Entry
see the guidelines for commit messages. (view raw markdown for examples)
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.