chore: add solomachine client storage (#1144)

* WIP: solomachine UpdateState now handles storage of ClientState and ConsensusState

* applying suggestions for review

* removing unnecessary consensus state storage

* updating tests to use store in favour of return values

modules/light-clients/06-solomachine/types/misbehaviour_handle.go

@@ -34,12 +34,12 @@ func (cs ClientState) CheckMisbehaviourAndUpdateState(
 	// misbehaviour.ValidateBasic which is called by the 02-client keeper.
 
 	// verify first signature
-	if err := verifySignatureAndData(cdc, cs, soloMisbehaviour, soloMisbehaviour.SignatureOne); err != nil {
+	if err := cs.verifySignatureAndData(cdc, soloMisbehaviour, soloMisbehaviour.SignatureOne); err != nil {
 		return nil, sdkerrors.Wrap(err, "failed to verify signature one")
 	}
 
 	// verify second signature
-	if err := verifySignatureAndData(cdc, cs, soloMisbehaviour, soloMisbehaviour.SignatureTwo); err != nil {
+	if err := cs.verifySignatureAndData(cdc, soloMisbehaviour, soloMisbehaviour.SignatureTwo); err != nil {
 		return nil, sdkerrors.Wrap(err, "failed to verify signature two")
 	}
 
@@ -50,7 +50,7 @@ func (cs ClientState) CheckMisbehaviourAndUpdateState(
 // verifySignatureAndData verifies that the currently registered public key has signed
 // over the provided data and that the data is valid. The data is valid if it can be
 // unmarshaled into the specified data type.
-func verifySignatureAndData(cdc codec.BinaryCodec, clientState ClientState, misbehaviour *Misbehaviour, sigAndData *SignatureAndData) error {
+func (cs ClientState) verifySignatureAndData(cdc codec.BinaryCodec, misbehaviour *Misbehaviour, sigAndData *SignatureAndData) error {
 
 	// do not check misbehaviour timestamp since we want to allow processing of past misbehaviour
 
@@ -62,7 +62,7 @@ func verifySignatureAndData(cdc codec.BinaryCodec, clientState ClientState, misb
 	data, err := MisbehaviourSignBytes(
 		cdc,
 		misbehaviour.Sequence, sigAndData.Timestamp,
-		clientState.ConsensusState.Diversifier,
+		cs.ConsensusState.Diversifier,
 		sigAndData.DataType,
 		sigAndData.Data,
 	)
@@ -75,7 +75,7 @@ func verifySignatureAndData(cdc codec.BinaryCodec, clientState ClientState, misb
 		return err
 	}
 
-	publicKey, err := clientState.ConsensusState.GetPubKey()
+	publicKey, err := cs.ConsensusState.GetPubKey()
 	if err != nil {
 		return err
 	}

modules/light-clients/06-solomachine/types/update.go

@@ -6,6 +6,7 @@ import (
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 
 	clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
+	host "github.com/cosmos/ibc-go/v3/modules/core/24-host"
 	"github.com/cosmos/ibc-go/v3/modules/core/exported"
 )
 
@@ -89,12 +90,12 @@ func (cs ClientState) verifyMisbehaviour(ctx sdk.Context, cdc codec.BinaryCodec,
 	// NOTE: a check that the misbehaviour message data are not equal is done by
 	// misbehaviour.ValidateBasic which is called by the 02-client keeper.
 	// verify first signature
-	if err := verifySignatureAndData(cdc, cs, misbehaviour, misbehaviour.SignatureOne); err != nil {
+	if err := cs.verifySignatureAndData(cdc, misbehaviour, misbehaviour.SignatureOne); err != nil {
 		return sdkerrors.Wrap(err, "failed to verify signature one")
 	}
 
 	// verify second signature
-	if err := verifySignatureAndData(cdc, cs, misbehaviour, misbehaviour.SignatureTwo); err != nil {
+	if err := cs.verifySignatureAndData(cdc, misbehaviour, misbehaviour.SignatureTwo); err != nil {
 		return sdkerrors.Wrap(err, "failed to verify signature two")
 	}
 
@@ -103,7 +104,12 @@ 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) {
-	smHeader := clientMsg.(*Header)
+	smHeader, ok := clientMsg.(*Header)
+	if !ok {
+		return nil, nil, sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "expected %T got %T", Header{}, clientMsg)
+	}
+
+	// create new solomachine ConsensusState
 	consensusState := &ConsensusState{
 		PublicKey:   smHeader.NewPublicKey,
 		Diversifier: smHeader.NewDiversifier,
@@ -112,6 +118,9 @@ func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, client
 
 	cs.Sequence++
 	cs.ConsensusState = consensusState
+
+	clientStore.Set(host.ClientStateKey(), clienttypes.MustMarshalClientState(cdc, &cs))
+
 	return &cs, consensusState, nil
 }
 
@@ -126,9 +135,10 @@ func (cs ClientState) CheckForMisbehaviour(_ sdk.Context, _ codec.BinaryCodec, _
 
 // UpdateStateOnMisbehaviour updates state upon misbehaviour. This method should only be called on misbehaviour
 // as it does not perform any misbehaviour checks.
-func (cs ClientState) UpdateStateOnMisbehaviour(
-	_ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore, // prematurely include args for self storage of consensus state
-) (*ClientState, exported.ConsensusState, error) {
+func (cs ClientState) UpdateStateOnMisbehaviour(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore) (*ClientState, exported.ConsensusState, error) {
 	cs.IsFrozen = true
+
+	clientStore.Set(host.ClientStateKey(), clienttypes.MustMarshalClientState(cdc, &cs))
+
 	return &cs, cs.ConsensusState, nil
 }

modules/light-clients/06-solomachine/types/update_test.go

@@ -4,6 +4,8 @@ import (
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
+	clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
+	host "github.com/cosmos/ibc-go/v3/modules/core/24-host"
 	"github.com/cosmos/ibc-go/v3/modules/core/exported"
 	"github.com/cosmos/ibc-go/v3/modules/light-clients/06-solomachine/types"
 	ibctmtypes "github.com/cosmos/ibc-go/v3/modules/light-clients/07-tendermint/types"
@@ -594,30 +596,43 @@ func (suite *SoloMachineTestSuite) TestUpdateState() {
 				},
 				true,
 			},
+			{
+				"invalid type misbehaviour",
+				func() {
+					clientState = solomachine.ClientState()
+					clientMsg = solomachine.CreateMisbehaviour()
+				},
+				false,
+			},
 		}
 
 		for _, tc := range testCases {
 			tc := tc
 
 			suite.Run(tc.name, func() {
-				// setup test
-				tc.setup()
+				tc.setup() // setup test
 
+				// TODO: remove casting when 'UpdateState' is an interface function.
 				clientState, ok := clientState.(*types.ClientState)
-				if ok {
-					cs, consensusState, err := clientState.UpdateState(suite.chainA.GetContext(), suite.chainA.Codec, suite.store, clientMsg)
-
-					if tc.expPass {
-						suite.Require().NoError(err)
-						suite.Require().Equal(clientMsg.(*types.Header).NewPublicKey, cs.(*types.ClientState).ConsensusState.PublicKey)
-						suite.Require().Equal(false, cs.(*types.ClientState).IsFrozen)
-						suite.Require().Equal(clientMsg.(*types.Header).Sequence+1, cs.(*types.ClientState).Sequence)
-						suite.Require().Equal(consensusState, cs.(*types.ClientState).ConsensusState)
-					} else {
-						suite.Require().Error(err)
-						suite.Require().Nil(clientState)
-						suite.Require().Nil(consensusState)
-					}
+				suite.Require().True(ok)
+
+				_, _, err := clientState.UpdateState(suite.chainA.GetContext(), suite.chainA.Codec, suite.store, clientMsg)
+
+				if tc.expPass {
+					suite.Require().NoError(err)
+
+					clientStateBz := suite.store.Get(host.ClientStateKey())
+					suite.Require().NotEmpty(clientStateBz)
+
+					newClientState := clienttypes.MustUnmarshalClientState(suite.chainA.Codec, clientStateBz)
+
+					suite.Require().False(newClientState.(*types.ClientState).IsFrozen)
+					suite.Require().Equal(clientMsg.(*types.Header).Sequence+1, newClientState.(*types.ClientState).Sequence)
+					suite.Require().Equal(clientMsg.(*types.Header).NewPublicKey, newClientState.(*types.ClientState).ConsensusState.PublicKey)
+					suite.Require().Equal(clientMsg.(*types.Header).NewDiversifier, newClientState.(*types.ClientState).ConsensusState.Diversifier)
+					suite.Require().Equal(clientMsg.(*types.Header).Timestamp, newClientState.(*types.ClientState).ConsensusState.Timestamp)
+				} else {
+					suite.Require().Error(err)
 				}
 
 			})
@@ -697,10 +712,15 @@ func (suite *SoloMachineTestSuite) TestUpdateStateOnMisbehaviour() {
 
 				tc.malleate()
 
-				cs, _, _ := clientState.UpdateStateOnMisbehaviour(suite.chainA.GetContext(), suite.chainA.Codec, suite.store)
+				clientState.UpdateStateOnMisbehaviour(suite.chainA.GetContext(), suite.chainA.Codec, suite.store)
 
 				if tc.expPass {
-					suite.Require().True(cs.IsFrozen)
+					clientStateBz := suite.store.Get(host.ClientStateKey())
+					suite.Require().NotEmpty(clientStateBz)
+
+					newClientState := clienttypes.MustUnmarshalClientState(suite.chainA.Codec, clientStateBz)
+
+					suite.Require().True(newClientState.(*types.ClientState).IsFrozen)
 				}
 			})
 		}