chore: adding UpdateState to ClientState interface (#1206)

* adding UpdateState to ClientState interface and updating surrounds * updating godoc * adding changelog entry * Update modules/core/exported/client.go Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> * fixing typo in changelog Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
cosmos · Mar 31, 2022 · eb48e54 · eb48e54
1 parent 5cf6528
commit eb48e54
Show file tree

Hide file tree

Showing 7 changed files with 32 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -48,6 +48,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (modules/core/02-client) [\#1196](https://github.com/cosmos/ibc-go/pull/1196) Adding VerifyClientMessage to ClientState interface. 
 * (modules/core/02-client) [\#1198](https://github.com/cosmos/ibc-go/pull/1198) Adding UpdateStateOnMisbehaviour 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

diff --git a/modules/core/02-client/legacy/v100/solomachine.go b/modules/core/02-client/legacy/v100/solomachine.go
@@ -102,6 +102,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
@@ -58,6 +58,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 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
+
 	// 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
@@ -30,7 +30,13 @@ func (cs ClientState) CheckHeaderAndUpdateState(
 		return &cs, cs.ConsensusState, nil
 	}
 
-	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
@@ -104,10 +110,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
@@ -122,7 +128,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
@@ -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
@@ -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
@@ -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)