From 987776783f9025296403da6bf9b2e3a98c07efad Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 30 Mar 2022 16:46:12 +0200 Subject: [PATCH 1/5] adding UpdateState to ClientState interface and updating surrounds --- .../core/02-client/legacy/v100/solomachine.go | 5 +++++ modules/core/exported/client.go | 4 ++++ .../06-solomachine/types/update.go | 14 ++++++++++---- .../06-solomachine/types/update_test.go | 6 +----- .../light-clients/07-tendermint/types/update.go | 17 ++++++++++------- .../07-tendermint/types/update_test.go | 6 +----- .../09-localhost/types/client_state.go | 6 +++--- 7 files changed, 34 insertions(+), 24 deletions(-) diff --git a/modules/core/02-client/legacy/v100/solomachine.go b/modules/core/02-client/legacy/v100/solomachine.go index 9888814dee4..b5a44ff9ba4 100644 --- a/modules/core/02-client/legacy/v100/solomachine.go +++ b/modules/core/02-client/legacy/v100/solomachine.go @@ -95,6 +95,11 @@ func (cs *ClientState) VerifyClientMessage( panic("legacy solo machine is deprecated!") } +// UpdateState panis! +func (cs *ClientState) UpdateState(_ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore, _ exported.ClientMessage) error { + panic("legacy solo machine is deprecated!") +} + // CheckHeaderAndUpdateState panics! func (cs *ClientState) CheckHeaderAndUpdateState( _ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore, _ exported.ClientMessage, diff --git a/modules/core/exported/client.go b/modules/core/exported/client.go index 00ebc1e418c..1af73354124 100644 --- a/modules/core/exported/client.go +++ b/modules/core/exported/client.go @@ -59,6 +59,10 @@ type ClientState interface { // VerifyClientMessage verifies a ClientMessage. A ClientMessage could be a Header, Misbehaviour, or batch update. VerifyClientMessage(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, clientMsg ClientMessage) error + // UpdateState updates and stores the ClientState and associated ConsensusState using the provided ClientMessage Header + // An error is returned if ClientMessage is of type Misbehaviour + UpdateState(sdk.Context, codec.BinaryCodec, sdk.KVStore, ClientMessage) error + // Update and Misbehaviour functions CheckHeaderAndUpdateState(sdk.Context, codec.BinaryCodec, sdk.KVStore, ClientMessage) (ClientState, ConsensusState, error) CheckMisbehaviourAndUpdateState(sdk.Context, codec.BinaryCodec, sdk.KVStore, ClientMessage) (ClientState, error) diff --git a/modules/light-clients/06-solomachine/types/update.go b/modules/light-clients/06-solomachine/types/update.go index b6dfc44eac1..188987988a6 100644 --- a/modules/light-clients/06-solomachine/types/update.go +++ b/modules/light-clients/06-solomachine/types/update.go @@ -29,7 +29,13 @@ func (cs ClientState) CheckHeaderAndUpdateState( return cs.UpdateStateOnMisbehaviour(ctx, cdc, clientStore) } - return cs.UpdateState(ctx, cdc, clientStore, msg) + if err := cs.UpdateState(ctx, cdc, clientStore, msg); err != nil { + return nil, nil, err + } + + newClientState := clienttypes.MustUnmarshalClientState(cdc, clientStore.Get(host.ClientStateKey())).(*ClientState) + + return newClientState, newClientState.ConsensusState, nil } // VerifyClientMessage introspects the provided ClientMessage and checks its validity @@ -103,10 +109,10 @@ func (cs ClientState) verifyMisbehaviour(ctx sdk.Context, cdc codec.BinaryCodec, } // UpdateState updates the consensus state to the new public key and an incremented sequence. -func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, clientMsg exported.ClientMessage) (exported.ClientState, exported.ConsensusState, error) { +func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, clientMsg exported.ClientMessage) error { smHeader, ok := clientMsg.(*Header) if !ok { - return nil, nil, sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "expected %T got %T", Header{}, clientMsg) + return sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "expected %T got %T", Header{}, clientMsg) } // create new solomachine ConsensusState @@ -121,7 +127,7 @@ func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, client clientStore.Set(host.ClientStateKey(), clienttypes.MustMarshalClientState(cdc, &cs)) - return &cs, consensusState, nil + return nil } // CheckForMisbehaviour returns true for type Misbehaviour (passed VerifyClientMessage check), otherwise returns false diff --git a/modules/light-clients/06-solomachine/types/update_test.go b/modules/light-clients/06-solomachine/types/update_test.go index 53b6f014890..0078c4718df 100644 --- a/modules/light-clients/06-solomachine/types/update_test.go +++ b/modules/light-clients/06-solomachine/types/update_test.go @@ -612,11 +612,7 @@ func (suite *SoloMachineTestSuite) TestUpdateState() { suite.Run(tc.name, func() { tc.setup() // setup test - // TODO: remove casting when 'UpdateState' is an interface function. - clientState, ok := clientState.(*types.ClientState) - suite.Require().True(ok) - - _, _, err := clientState.UpdateState(suite.chainA.GetContext(), suite.chainA.Codec, suite.store, clientMsg) + err := clientState.UpdateState(suite.chainA.GetContext(), suite.chainA.Codec, suite.store, clientMsg) if tc.expPass { suite.Require().NoError(err) diff --git a/modules/light-clients/07-tendermint/types/update.go b/modules/light-clients/07-tendermint/types/update.go index ecd88312882..5b854730992 100644 --- a/modules/light-clients/07-tendermint/types/update.go +++ b/modules/light-clients/07-tendermint/types/update.go @@ -90,11 +90,14 @@ func (cs ClientState) CheckHeaderAndUpdateState( return &cs, consState, nil } - newClientState, consensusState, err := cs.UpdateState(ctx, cdc, clientStore, tmHeader) - if err != nil { + if err := cs.UpdateState(ctx, cdc, clientStore, tmHeader); err != nil { return nil, nil, err } - return newClientState, consensusState, nil + + newClientState := clienttypes.MustUnmarshalClientState(cdc, clientStore.Get(host.ClientStateKey())) + newConsensusState := clienttypes.MustUnmarshalConsensusState(cdc, clientStore.Get(host.ConsensusStateKey(header.GetHeight()))) + + return newClientState, newConsensusState, nil } // checkTrustedHeader checks that consensus state matches trusted fields of Header @@ -238,16 +241,16 @@ func (cs *ClientState) verifyHeader( // UpdateState must only be used to update within a single revision, thus header revision number and trusted height's revision // number must be the same. To update to a new revision, use a separate upgrade path // UpdateState will prune the oldest consensus state if it is expired. -func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, clientMsg exported.ClientMessage) (*ClientState, *ConsensusState, error) { +func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, clientMsg exported.ClientMessage) error { header, ok := clientMsg.(*Header) if !ok { - return nil, nil, sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "expected type %T, got %T", &Header{}, header) + return sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "expected type %T, got %T", &Header{}, header) } // check for duplicate update if consensusState, _ := GetConsensusState(clientStore, cdc, header.GetHeight()); consensusState != nil { // perform no-op - return &cs, consensusState, nil + return nil } cs.pruneOldestConsensusState(ctx, cdc, clientStore) @@ -267,7 +270,7 @@ func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, client setConsensusState(clientStore, cdc, consensusState, header.GetHeight()) setConsensusMetadata(ctx, clientStore, header.GetHeight()) - return &cs, consensusState, nil + return nil } // pruneOldestConsensusState will retrieve the earliest consensus state for this clientID and check if it is expired. If it is, diff --git a/modules/light-clients/07-tendermint/types/update_test.go b/modules/light-clients/07-tendermint/types/update_test.go index 4f9648bd74a..bdc38e2486c 100644 --- a/modules/light-clients/07-tendermint/types/update_test.go +++ b/modules/light-clients/07-tendermint/types/update_test.go @@ -511,12 +511,8 @@ func (suite *TendermintTestSuite) TestUpdateState() { clientState := path.EndpointA.GetClientState() - // TODO: remove casting when 'UpdateState' is an interface function. - tmClientState, ok := clientState.(*types.ClientState) - suite.Require().True(ok) - clientStore = suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) - _, _, err = tmClientState.UpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore, clientMessage) + err = clientState.UpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore, clientMessage) if tc.expPass { suite.Require().NoError(err) diff --git a/modules/light-clients/09-localhost/types/client_state.go b/modules/light-clients/09-localhost/types/client_state.go index 1ac05f3af64..f052c77e92e 100644 --- a/modules/light-clients/09-localhost/types/client_state.go +++ b/modules/light-clients/09-localhost/types/client_state.go @@ -81,7 +81,7 @@ func (cs ClientState) ExportMetadata(_ sdk.KVStore) []exported.GenesisMetadata { func (cs *ClientState) CheckHeaderAndUpdateState( ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, header exported.ClientMessage, ) (exported.ClientState, exported.ConsensusState, error) { - return cs.UpdateState(ctx, cdc, clientStore, header) + return cs, nil, cs.UpdateState(ctx, cdc, clientStore, header) } // VerifyHeader is a no-op. @@ -101,12 +101,12 @@ func (cs *ClientState) CheckForMisbehaviour( // UpdateState updates the localhost client. It only needs access to the context func (cs *ClientState) UpdateState( ctx sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore, _ exported.ClientMessage, -) (exported.ClientState, exported.ConsensusState, error) { +) error { // use the chain ID from context since the localhost client is from the running chain (i.e self). cs.ChainId = ctx.ChainID() revision := clienttypes.ParseChainID(cs.ChainId) cs.Height = clienttypes.NewHeight(revision, uint64(ctx.BlockHeight())) - return cs, nil, nil + return nil } // UpdateStateOnMisbehaviour returns an error (no misbehaviour case). From e340f0de7d9f872b302ece46c1c0492069175bb2 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 30 Mar 2022 16:48:47 +0200 Subject: [PATCH 2/5] updating godoc --- modules/core/exported/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/core/exported/client.go b/modules/core/exported/client.go index 1af73354124..c3057c260a7 100644 --- a/modules/core/exported/client.go +++ b/modules/core/exported/client.go @@ -59,7 +59,7 @@ type ClientState interface { // VerifyClientMessage verifies a ClientMessage. A ClientMessage could be a Header, Misbehaviour, or batch update. VerifyClientMessage(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, clientMsg ClientMessage) error - // UpdateState updates and stores the ClientState and associated ConsensusState using the provided ClientMessage Header + // UpdateState updates and stores the ClientState and associated ConsensusState using the provided KVStore and ClientMessage Header // An error is returned if ClientMessage is of type Misbehaviour UpdateState(sdk.Context, codec.BinaryCodec, sdk.KVStore, ClientMessage) error From d0627b124f2d8678f375611d2daaad9b3bd073fa Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Thu, 31 Mar 2022 14:05:13 +0200 Subject: [PATCH 3/5] adding changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bd1a631b45..ce7c0787ab9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [\#1186](https://github.com/cosmos/ibc-go/pull/1186/files) Removing `GetRoot` function from ConsensusState interface in `02-client`. `GetRoot` is unused by core IBC. * (modules/core/02-client) [\#1196](https://github.com/cosmos/ibc-go/pull/1196) Adding VerifyClientMessage to ClientState interface. * (modules/core/02-client) [\#1170](https://github.com/cosmos/ibc-go/pull/1170) Updating `ClientUpdateProposal` to set client state in lightclient implementations `CheckSubstituteAndUpdateState` methods. +* *(modules/core/exported) [\#1206](https://github.com/cosmos/ibc-go/pull/1206) Adding new method `UpdateState` to `ClientState` interface. ### Features From 4bf6afd5a56853f6188654143e9e7b0cdd3bfa26 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Thu, 31 Mar 2022 14:06:53 +0200 Subject: [PATCH 4/5] Update modules/core/exported/client.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> --- modules/core/exported/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/core/exported/client.go b/modules/core/exported/client.go index c3057c260a7..f39d1930b11 100644 --- a/modules/core/exported/client.go +++ b/modules/core/exported/client.go @@ -59,7 +59,7 @@ type ClientState interface { // VerifyClientMessage verifies a ClientMessage. A ClientMessage could be a Header, Misbehaviour, or batch update. VerifyClientMessage(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, clientMsg ClientMessage) error - // UpdateState updates and stores the ClientState and associated ConsensusState using the provided KVStore and ClientMessage Header + // UpdateState updates and stores as necessary any associated information for an IBC client, such as the ClientState and corresponding ConsensusState. // An error is returned if ClientMessage is of type Misbehaviour UpdateState(sdk.Context, codec.BinaryCodec, sdk.KVStore, ClientMessage) error From f8be5c415a5f472bd9ce98b37e5e6580baf8fe83 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Thu, 31 Mar 2022 14:07:37 +0200 Subject: [PATCH 5/5] fixing typo in changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce7c0787ab9..6fe12a3e641 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,7 +46,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [\#1186](https://github.com/cosmos/ibc-go/pull/1186/files) Removing `GetRoot` function from ConsensusState interface in `02-client`. `GetRoot` is unused by core IBC. * (modules/core/02-client) [\#1196](https://github.com/cosmos/ibc-go/pull/1196) Adding VerifyClientMessage to ClientState interface. * (modules/core/02-client) [\#1170](https://github.com/cosmos/ibc-go/pull/1170) Updating `ClientUpdateProposal` to set client state in lightclient implementations `CheckSubstituteAndUpdateState` methods. -* *(modules/core/exported) [\#1206](https://github.com/cosmos/ibc-go/pull/1206) Adding new method `UpdateState` to `ClientState` interface. +* (modules/core/exported) [\#1206](https://github.com/cosmos/ibc-go/pull/1206) Adding new method `UpdateState` to `ClientState` interface. ### Features