chore: 07-tendermint set client/consensus states in UpdateState (#1199)

* adding storage ops to 07-tendermint UpdateState, updating tests

* use clientMessage.GetHeight() as per review suggestions

* updating tests to obtain previous client/consensus state in malleate

modules/light-clients/07-tendermint/types/proposal_handle.go

@@ -67,7 +67,7 @@ func (cs ClientState) CheckSubstituteAndUpdateState(
 		return nil, sdkerrors.Wrap(err, "unable to retrieve latest consensus state for substitute client")
 	}
 
-	SetConsensusState(subjectClientStore, cdc, consensusState, height)
+	setConsensusState(subjectClientStore, cdc, consensusState, height)
 
 	// set metadata stored for the substitute consensus state
 	processedHeight, found := GetProcessedHeight(substituteClientStore, height)

modules/light-clients/07-tendermint/types/store.go

@@ -50,8 +50,8 @@ func setClientState(clientStore sdk.KVStore, cdc codec.BinaryCodec, clientState
 	clientStore.Set(key, val)
 }
 
-// SetConsensusState stores the consensus state at the given height.
-func SetConsensusState(clientStore sdk.KVStore, cdc codec.BinaryCodec, consensusState *ConsensusState, height exported.Height) {
+// setConsensusState stores the consensus state at the given height.
+func setConsensusState(clientStore sdk.KVStore, cdc codec.BinaryCodec, consensusState *ConsensusState, height exported.Height) {
 	key := host.ConsensusStateKey(height)
 	val := clienttypes.MustMarshalConsensusState(cdc, consensusState)
 	clientStore.Set(key, val)

modules/light-clients/07-tendermint/types/update.go

@@ -262,7 +262,9 @@ func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, client
 		NextValidatorsHash: header.Header.NextValidatorsHash,
 	}
 
-	// set metadata for this consensus state
+	// set client state, consensus state and asssociated metadata
+	setClientState(clientStore, cdc, &cs)
+	setConsensusState(clientStore, cdc, consensusState, header.GetHeight())
 	setConsensusMetadata(ctx, clientStore, header.GetHeight())
 
 	return &cs, consensusState, nil

modules/light-clients/07-tendermint/types/update_test.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"time"
 
+	sdk "github.com/cosmos/cosmos-sdk/types"
 	clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
 	commitmenttypes "github.com/cosmos/ibc-go/v3/modules/core/23-commitment/types"
 	host "github.com/cosmos/ibc-go/v3/modules/core/24-host"
@@ -394,11 +395,12 @@ func (suite *TendermintTestSuite) TestVerifyHeader() {
 
 func (suite *TendermintTestSuite) TestUpdateState() {
 	var (
-		path                  *ibctesting.Path
-		clientMessage         exported.ClientMessage
-		pruneHeight           clienttypes.Height
-		updatedClientState    *types.ClientState    // TODO: retrieve from state after 'UpdateState' call
-		updatedConsensusState *types.ConsensusState // TODO: retrieve from state after 'UpdateState' call
+		path               *ibctesting.Path
+		clientMessage      exported.ClientMessage
+		clientStore        sdk.KVStore
+		pruneHeight        clienttypes.Height
+		prevClientState    exported.ClientState
+		prevConsensusState exported.ConsensusState
 	)
 
 	testCases := []struct {
@@ -412,7 +414,7 @@ func (suite *TendermintTestSuite) TestUpdateState() {
 				suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().LT(clientMessage.GetHeight()))
 			},
 			func() {
-				suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().LT(updatedClientState.GetLatestHeight())) // new update, updated client state should have changed
+				suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().EQ(clientMessage.GetHeight())) // new update, updated client state should have changed
 			}, true,
 		},
 		{
@@ -424,9 +426,11 @@ func (suite *TendermintTestSuite) TestUpdateState() {
 				suite.Require().NoError(err)
 
 				suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().GT(clientMessage.GetHeight()))
+
+				prevClientState = path.EndpointA.GetClientState()
 			},
 			func() {
-				suite.Require().Equal(path.EndpointA.GetClientState(), updatedClientState) // fill in height, no change to client state
+				suite.Require().Equal(path.EndpointA.GetClientState(), prevClientState) // fill in height, no change to client state
 			}, true,
 		},
 		{
@@ -439,10 +443,13 @@ func (suite *TendermintTestSuite) TestUpdateState() {
 				clientMessage, err = path.EndpointA.Chain.ConstructUpdateTMClientHeader(path.EndpointA.Counterparty.Chain, path.EndpointA.ClientID)
 				suite.Require().NoError(err)
 				suite.Require().Equal(path.EndpointA.GetClientState().GetLatestHeight(), clientMessage.GetHeight())
+
+				prevClientState = path.EndpointA.GetClientState()
+				prevConsensusState = path.EndpointA.GetConsensusState(clientMessage.GetHeight())
 			},
 			func() {
-				suite.Require().Equal(path.EndpointA.GetClientState(), updatedClientState)
-				suite.Require().Equal(path.EndpointA.GetConsensusState(clientMessage.GetHeight()), updatedConsensusState)
+				suite.Require().Equal(path.EndpointA.GetClientState(), prevClientState)
+				suite.Require().Equal(path.EndpointA.GetConsensusState(clientMessage.GetHeight()), prevConsensusState)
 			}, true,
 		},
 		{
@@ -471,7 +478,7 @@ func (suite *TendermintTestSuite) TestUpdateState() {
 				suite.Require().NoError(err)
 			},
 			func() {
-				suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().LT(updatedClientState.GetLatestHeight())) // new update, updated client state should have changed
+				suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().EQ(clientMessage.GetHeight())) // new update, updated client state should have changed
 
 				// ensure consensus state was pruned
 				_, found := path.EndpointA.Chain.GetConsensusState(path.EndpointA.ClientID, pruneHeight)
@@ -508,8 +515,8 @@ func (suite *TendermintTestSuite) TestUpdateState() {
 			tmClientState, ok := clientState.(*types.ClientState)
 			suite.Require().True(ok)
 
-			clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID)
-			updatedClientState, updatedConsensusState, err = tmClientState.UpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore, clientMessage)
+			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)
 
 			if tc.expPass {
 				suite.Require().NoError(err)
@@ -520,13 +527,14 @@ func (suite *TendermintTestSuite) TestUpdateState() {
 					Root:               commitmenttypes.NewMerkleRoot(header.Header.GetAppHash()),
 					NextValidatorsHash: header.Header.NextValidatorsHash,
 				}
+
+				bz := clientStore.Get(host.ConsensusStateKey(header.GetHeight()))
+				updatedConsensusState := clienttypes.MustUnmarshalConsensusState(suite.chainA.App.AppCodec(), bz)
+
 				suite.Require().Equal(expConsensusState, updatedConsensusState)
 
 			} else {
 				suite.Require().Error(err)
-				suite.Require().Nil(updatedClientState)
-				suite.Require().Nil(updatedConsensusState)
-
 			}
 
 			// perform custom checks

modules/light-clients/07-tendermint/types/upgrade.go

@@ -120,7 +120,7 @@ func (cs ClientState) VerifyUpgradeAndUpdateState(
 	)
 
 	setClientState(clientStore, cdc, newClientState)
-	SetConsensusState(clientStore, cdc, newConsState, newClientState.LatestHeight)
+	setConsensusState(clientStore, cdc, newConsState, newClientState.LatestHeight)
 	setConsensusMetadata(ctx, clientStore, tmUpgradeClient.LatestHeight)
 
 	return nil