02-client-refactor: rename update to UpdateState for 07-tendermint

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

@@ -28,14 +28,6 @@ import (
 // - header timestamp is past the trusting period in relation to the consensus state
 // - header timestamp is less than or equal to the consensus state timestamp
 //
-// UpdateClient may be used to either create a consensus state for:
-// - a future height greater than the latest client state height
-// - a past height that was skipped during bisection
-// If we are updating to a past height, a consensus state is created for that height to be persisted in client store
-// If we are updating to a future height, the consensus state is created and the client state is updated to reflect
-// the new latest height
-// UpdateClient 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
 // Tendermint client validity checking uses the bisection algorithm described
 // in the [Tendermint spec](https://github.com/tendermint/spec/blob/master/spec/consensus/light-client.md).
 //
@@ -45,10 +37,6 @@ import (
 // 2. Any valid update that breaks time monotonicity with respect to its neighboring consensus states is evidence of misbehaviour and will freeze client.
 // Misbehaviour sets frozen height to {0, 1} since it is only used as a boolean value (zero or non-zero).
 //
-// Pruning:
-// UpdateClient will additionally retrieve the earliest consensus state for this clientID and check if it is expired. If it is,
-// that consensus state will be pruned from store along with all associated metadata. This will prevent the client store from
-// becoming bloated with expired consensus states that can no longer be used for updates and packet verification.
 func (cs ClientState) CheckHeaderAndUpdateState(
 	ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore,
 	header exported.ClientMessage,
@@ -110,35 +98,10 @@ func (cs ClientState) CheckHeaderAndUpdateState(
 		return &cs, consState, nil
 	}
 
-	// Check the earliest consensus state to see if it is expired, if so then set the prune height
-	// so that we can delete consensus state and all associated metadata.
-	var (
-		pruneHeight exported.Height
-		pruneError  error
-	)
-	pruneCb := func(height exported.Height) bool {
-		consState, err := GetConsensusState(clientStore, cdc, height)
-		// this error should never occur
-		if err != nil {
-			pruneError = err
-			return true
-		}
-		if cs.IsExpired(consState.Timestamp, ctx.BlockTime()) {
-			pruneHeight = height
-		}
-		return true
-	}
-	IterateConsensusStateAscending(clientStore, pruneCb)
-	if pruneError != nil {
-		return nil, nil, pruneError
-	}
-	// if pruneHeight is set, delete consensus state and metadata
-	if pruneHeight != nil {
-		deleteConsensusState(clientStore, pruneHeight)
-		deleteConsensusMetadata(clientStore, pruneHeight)
+	newClientState, consensusState, err := cs.UpdateState(ctx, cdc, clientStore, tmHeader)
+	if err != nil {
+		return nil, nil, err
 	}
-
-	newClientState, consensusState := update(ctx, clientStore, &cs, tmHeader)
 	return newClientState, consensusState, nil
 }
 
@@ -244,11 +207,32 @@ func checkValidity(
 	return nil
 }
 
-// update the consensus state from a new header and set processed time metadata
-func update(ctx sdk.Context, clientStore sdk.KVStore, clientState *ClientState, header *Header) (*ClientState, *ConsensusState) {
+// UpdateState may be used to either create a consensus state for:
+// - a future height greater than the latest client state height
+// - a past height that was skipped during bisection
+// If we are updating to a past height, a consensus state is created for that height to be persisted in client store
+// If we are updating to a future height, the consensus state is created and the client state is updated to reflect
+// the new latest height
+// 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) {
+	header, ok := clientMsg.(*Header)
+	if !ok {
+		return nil, nil, 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
+	}
+
+	cs.pruneOldestConsensusState(ctx, cdc, clientStore)
+
 	height := header.GetHeight().(clienttypes.Height)
-	if height.GT(clientState.LatestHeight) {
-		clientState.LatestHeight = height
+	if height.GT(cs.LatestHeight) {
+		cs.LatestHeight = height
 	}
 	consensusState := &ConsensusState{
 		Timestamp:          header.GetTime(),
@@ -259,5 +243,43 @@ func update(ctx sdk.Context, clientStore sdk.KVStore, clientState *ClientState,
 	// set metadata for this consensus state
 	setConsensusMetadata(ctx, clientStore, header.GetHeight())
 
-	return clientState, consensusState
+	return &cs, consensusState, nil
+}
+
+// pruneOldestConsensusState will retrieve the earliest consensus state for this clientID and check if it is expired. If it is,
+// that consensus state will be pruned from store along with all associated metadata. This will prevent the client store from
+// becoming bloated with expired consensus states that can no longer be used for updates and packet verification.
+func (cs ClientState) pruneOldestConsensusState(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore) {
+	// Check the earliest consensus state to see if it is expired, if so then set the prune height
+	// so that we can delete consensus state and all associated metadata.
+	var (
+		pruneHeight exported.Height
+		pruneError  error
+	)
+
+	pruneCb := func(height exported.Height) bool {
+		consState, err := GetConsensusState(clientStore, cdc, height)
+		// this error should never occur
+		if err != nil {
+			pruneError = err
+			return true
+		}
+
+		if cs.IsExpired(consState.Timestamp, ctx.BlockTime()) {
+			pruneHeight = height
+		}
+
+		return true
+	}
+
+	IterateConsensusStateAscending(clientStore, pruneCb)
+	if pruneError != nil {
+		panic(pruneError)
+	}
+
+	// if pruneHeight is set, delete consensus state and metadata
+	if pruneHeight != nil {
+		deleteConsensusState(clientStore, pruneHeight)
+		deleteConsensusMetadata(clientStore, pruneHeight)
+	}
 }

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

@@ -367,6 +367,149 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() {
 	}
 }
 
+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
+	)
+
+	testCases := []struct {
+		name      string
+		malleate  func()
+		expResult func()
+		expPass   bool
+	}{
+		{
+			"success with height later than latest height", func() {
+				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
+			}, true,
+		},
+		{
+			"success with height earlier than latest height", func() {
+				// commit a block so the pre-created ClientMessage
+				// isn't used to update the client to a newer height
+				suite.coordinator.CommitBlock(suite.chainB)
+				err := path.EndpointA.UpdateClient()
+				suite.Require().NoError(err)
+
+				suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().GT(clientMessage.GetHeight()))
+			},
+			func() {
+				suite.Require().Equal(path.EndpointA.GetClientState(), updatedClientState) // fill in height, no change to client state
+			}, true,
+		},
+		{
+			"success with duplicate header", func() {
+				// update client in advance
+				err := path.EndpointA.UpdateClient()
+				suite.Require().NoError(err)
+
+				// use the same header which just updated the client
+				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())
+			},
+			func() {
+				suite.Require().Equal(path.EndpointA.GetClientState(), updatedClientState)
+				suite.Require().Equal(path.EndpointA.GetConsensusState(clientMessage.GetHeight()), updatedConsensusState)
+			}, true,
+		},
+		{
+			"success with pruned consensus state", func() {
+				// this height will be expired and pruned
+				err := path.EndpointA.UpdateClient()
+				suite.Require().NoError(err)
+				pruneHeight = path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height)
+
+				// Increment the time by a week
+				suite.coordinator.IncrementTimeBy(7 * 24 * time.Hour)
+
+				// create the consensus state that can be used as trusted height for next update
+				err = path.EndpointA.UpdateClient()
+				suite.Require().NoError(err)
+
+				// Increment the time by another week, then update the client.
+				// This will cause the first two consensus states to become expired.
+				suite.coordinator.IncrementTimeBy(7 * 24 * time.Hour)
+				err = path.EndpointA.UpdateClient()
+				suite.Require().NoError(err)
+
+				// ensure counterparty state is committed
+				suite.coordinator.CommitBlock(suite.chainB)
+				clientMessage, err = path.EndpointA.Chain.ConstructUpdateTMClientHeader(path.EndpointA.Counterparty.Chain, path.EndpointA.ClientID)
+				suite.Require().NoError(err)
+			},
+			func() {
+				suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().LT(updatedClientState.GetLatestHeight())) // new update, updated client state should have changed
+
+				// ensure consensus state was pruned
+				_, found := path.EndpointA.Chain.GetConsensusState(path.EndpointA.ClientID, pruneHeight)
+				suite.Require().False(found)
+			}, true,
+		},
+		{
+			"invalid ClientMessage type", func() {
+				clientMessage = &types.Misbehaviour{}
+			},
+			func() {}, false,
+		},
+	}
+	for _, tc := range testCases {
+		tc := tc
+		suite.Run(tc.name, func() {
+			suite.SetupTest() // reset
+			pruneHeight = clienttypes.ZeroHeight()
+			path = ibctesting.NewPath(suite.chainA, suite.chainB)
+
+			err := path.EndpointA.CreateClient()
+			suite.Require().NoError(err)
+
+			// ensure counterparty state is committed
+			suite.coordinator.CommitBlock(suite.chainB)
+			clientMessage, err = path.EndpointA.Chain.ConstructUpdateTMClientHeader(path.EndpointA.Counterparty.Chain, path.EndpointA.ClientID)
+			suite.Require().NoError(err)
+
+			tc.malleate()
+
+			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)
+			updatedClientState, updatedConsensusState, err = tmClientState.UpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore, clientMessage)
+
+			if tc.expPass {
+				suite.Require().NoError(err)
+
+				header := clientMessage.(*types.Header)
+				expConsensusState := &types.ConsensusState{
+					Timestamp:          header.GetTime(),
+					Root:               commitmenttypes.NewMerkleRoot(header.Header.GetAppHash()),
+					NextValidatorsHash: header.Header.NextValidatorsHash,
+				}
+				suite.Require().Equal(expConsensusState, updatedConsensusState)
+
+			} else {
+				suite.Require().Error(err)
+				suite.Require().Nil(updatedClientState)
+				suite.Require().Nil(updatedConsensusState)
+
+			}
+
+			// perform custom checks
+			tc.expResult()
+		})
+	}
+}
+
 func (suite *TendermintTestSuite) TestPruneConsensusState() {
 	// create path and setup clients
 	path := ibctesting.NewPath(suite.chainA, suite.chainB)