chore: move initialization logic to light client module method (#6037)

* chore: move initialization logic to light client module method. Adjust testing. * tests: check for errors in test. * Update modules/light-clients/06-solomachine/light_client_module_test.go Co-authored-by: Carlos Rodriguez <carlos@interchain.io> * add migration note * add changelog --------- Co-authored-by: Carlos Rodriguez <carlos@interchain.io> Co-authored-by: Cian Hatton <cian@interchain.io>
cosmos · Mar 25, 2024 · d18a661 · d18a661
1 parent 572d8e8
commit d18a661
Show file tree

Hide file tree

Showing 6 changed files with 99 additions and 75 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -42,6 +42,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 * (core/02-client, light-clients) [\#5806](https://github.com/cosmos/ibc-go/pull/5806) Decouple light client routing from their encoding structure.
 * (core/04-channel) [\#5991](https://github.com/cosmos/ibc-go/pull/5991) The client CLI `QueryLatestConsensusState` has been removed.
+* (light-clients/06-solomachine) [\#6037](https://github.com/cosmos/ibc-go/pull/6037) Remove `Initialize` function from `ClientState` and move logic to `Initialize` function of `LightClientModule`.
 
 ### State Machine Breaking
 

diff --git a/docs/docs/05-migrations/13-v8-to-v9.md b/docs/docs/05-migrations/13-v8-to-v9.md
@@ -80,6 +80,10 @@ The following functions have also been removed from the `ClientState` interface:
 
 Please check also the [Light client developer guide](../03-light-clients/01-developer-guide/01-overview.md) for more information. The light client module implementation for `07-tendermint` may also be useful as reference.
 
+### 06-solomachine
+
+The `Initialize` function in `ClientState` has been removed and all its logic has been moved to the implemention of the `LightClientModule` interface `Initialize` function.
+
 ### 07-tendermint
 
 The `IterateConsensusMetadata` function has been removed.

diff --git a/modules/light-clients/06-solomachine/client_state.go b/modules/light-clients/06-solomachine/client_state.go
@@ -1,8 +1,6 @@
 package solomachine
 
 import (
-	"reflect"
-
 	errorsmod "cosmossdk.io/errors"
 	storetypes "cosmossdk.io/store/types"
 
@@ -67,19 +65,6 @@ func (cs ClientState) Validate() error {
 	return cs.ConsensusState.ValidateBasic()
 }
 
-// Initialize checks that the initial consensus state is equal to the latest consensus state of the initial client and
-// sets the client state in the provided client store.
-func (cs ClientState) Initialize(_ sdk.Context, cdc codec.BinaryCodec, clientStore storetypes.KVStore, consState exported.ConsensusState) error {
-	if !reflect.DeepEqual(cs.ConsensusState, consState) {
-		return errorsmod.Wrapf(clienttypes.ErrInvalidConsensus, "consensus state in initial client does not equal initial consensus state. expected: %s, got: %s",
-			cs.ConsensusState, consState)
-	}
-
-	setClientState(clientStore, cdc, &cs)
-
-	return nil
-}
-
 // VerifyMembership is a generic proof verification method which verifies a proof of the existence of a value at a given CommitmentPath at the latest sequence.
 // The caller is expected to construct the full CommitmentPath from a CommitmentPrefix and a standardized path (as defined in ICS 24).
 func (cs *ClientState) VerifyMembership(

diff --git a/modules/light-clients/06-solomachine/client_state_test.go b/modules/light-clients/06-solomachine/client_state_test.go
@@ -8,10 +8,8 @@ import (
 	clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
 	channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
 	commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types"
-	host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
 	"github.com/cosmos/ibc-go/v8/modules/core/exported"
 	solomachine "github.com/cosmos/ibc-go/v8/modules/light-clients/06-solomachine"
-	ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint"
 	ibctesting "github.com/cosmos/ibc-go/v8/testing"
 	ibcmock "github.com/cosmos/ibc-go/v8/testing/mock"
 )
@@ -92,63 +90,6 @@ func (suite *SoloMachineTestSuite) TestClientStateValidateBasic() {
 	}
 }
 
-func (suite *SoloMachineTestSuite) TestInitialize() {
-	// test singlesig and multisig public keys
-	for _, sm := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {
-		malleatedConsensus := sm.ClientState().ConsensusState
-		malleatedConsensus.Timestamp += 10
-
-		testCases := []struct {
-			name      string
-			consState exported.ConsensusState
-			expPass   bool
-		}{
-			{
-				"valid consensus state",
-				sm.ConsensusState(),
-				true,
-			},
-			{
-				"nil consensus state",
-				nil,
-				false,
-			},
-			{
-				"invalid consensus state: Tendermint consensus state",
-				&ibctm.ConsensusState{},
-				false,
-			},
-			{
-				"invalid consensus state: consensus state does not match consensus state in client",
-				malleatedConsensus,
-				false,
-			},
-		}
-
-		for _, tc := range testCases {
-			tc := tc
-
-			suite.Run(tc.name, func() {
-				suite.SetupTest()
-
-				store := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), "solomachine")
-				err := sm.ClientState().Initialize(
-					suite.chainA.GetContext(), suite.chainA.Codec,
-					store, tc.consState,
-				)
-
-				if tc.expPass {
-					suite.Require().NoError(err, "valid testcase: %s failed", tc.name)
-					suite.Require().True(store.Has(host.ClientStateKey()))
-				} else {
-					suite.Require().Error(err, "invalid testcase: %s passed", tc.name)
-					suite.Require().False(store.Has(host.ClientStateKey()))
-				}
-			})
-		}
-	}
-}
-
 func (suite *SoloMachineTestSuite) TestVerifyMembership() {
 	// test singlesig and multisig public keys
 	for _, sm := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {

diff --git a/modules/light-clients/06-solomachine/light_client_module.go b/modules/light-clients/06-solomachine/light_client_module.go
@@ -1,6 +1,8 @@
 package solomachine
 
 import (
+	"reflect"
+
 	errorsmod "cosmossdk.io/errors"
 
 	"github.com/cosmos/cosmos-sdk/codec"
@@ -56,7 +58,15 @@ func (l LightClientModule) Initialize(ctx sdk.Context, clientID string, clientSt
 	}
 
 	clientStore := l.storeProvider.ClientStore(ctx, clientID)
-	return clientState.Initialize(ctx, l.cdc, clientStore, &consensusState)
+
+	if !reflect.DeepEqual(clientState.ConsensusState, &consensusState) {
+		return errorsmod.Wrapf(clienttypes.ErrInvalidConsensus, "consensus state in initial client does not equal initial consensus state. expected: %s, got: %s",
+			clientState.ConsensusState, &consensusState)
+	}
+
+	setClientState(clientStore, l.cdc, &clientState)
+
+	return nil
 }
 
 // VerifyClientMessage obtains the client state associated with the client identifier and calls into the clientState.VerifyClientMessage method.

diff --git a/modules/light-clients/06-solomachine/light_client_module_test.go b/modules/light-clients/06-solomachine/light_client_module_test.go
@@ -1,10 +1,13 @@
 package solomachine_test
 
 import (
+	fmt "fmt"
+
 	clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
 	host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
 	"github.com/cosmos/ibc-go/v8/modules/core/exported"
 	solomachine "github.com/cosmos/ibc-go/v8/modules/light-clients/06-solomachine"
+	ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint"
 	ibctesting "github.com/cosmos/ibc-go/v8/testing"
 )
 
@@ -13,6 +16,86 @@ const (
 	wasmClientID = "08-wasm-0"
 )
 
+func (suite *SoloMachineTestSuite) TestInitialize() {
+	// test singlesig and multisig public keys
+	for _, sm := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {
+		malleatedConsensus := sm.ClientState().ConsensusState
+		malleatedConsensus.Timestamp += 10
+
+		testCases := []struct {
+			name        string
+			consState   exported.ConsensusState
+			clientState exported.ClientState
+			expErr      error
+		}{
+			{
+				"valid consensus state",
+				sm.ConsensusState(),
+				sm.ClientState(),
+				nil,
+			},
+			{
+				"nil consensus state",
+				nil,
+				sm.ClientState(),
+				clienttypes.ErrInvalidConsensus,
+			},
+			{
+				"invalid consensus state: Tendermint consensus state",
+				&ibctm.ConsensusState{},
+				sm.ClientState(),
+				fmt.Errorf("proto: wrong wireType = 0 for field TypeUrl"),
+			},
+			{
+				"invalid consensus state: consensus state does not match consensus state in client",
+				malleatedConsensus,
+				sm.ClientState(),
+				clienttypes.ErrInvalidConsensus,
+			},
+			{
+				"invalid client state: sequence is zero",
+				sm.ConsensusState(),
+				solomachine.NewClientState(0, sm.ConsensusState()),
+				clienttypes.ErrInvalidClient,
+			},
+			{
+				"invalid client state: Tendermint client state",
+				sm.ConsensusState(),
+				&ibctm.ClientState{},
+				fmt.Errorf("proto: wrong wireType = 2 for field IsFrozen"),
+			},
+		}
+
+		for _, tc := range testCases {
+			tc := tc
+
+			suite.Run(tc.name, func() {
+				suite.SetupTest()
+
+				clientStateBz := suite.chainA.Codec.MustMarshal(tc.clientState)
+				consStateBz := suite.chainA.Codec.MustMarshal(tc.consState)
+
+				clientID := suite.chainA.App.GetIBCKeeper().ClientKeeper.GenerateClientIdentifier(suite.chainA.GetContext(), exported.Solomachine)
+
+				lcm, found := suite.chainA.GetSimApp().IBCKeeper.ClientKeeper.Route(clientID)
+				suite.Require().True(found)
+
+				err := lcm.Initialize(suite.chainA.GetContext(), clientID, clientStateBz, consStateBz)
+				store := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), clientID)
+
+				expPass := tc.expErr == nil
+				if expPass {
+					suite.Require().NoError(err, "valid testcase: %s failed", tc.name)
+					suite.Require().True(store.Has(host.ClientStateKey()))
+				} else {
+					suite.Require().ErrorContains(err, tc.expErr.Error())
+					suite.Require().False(store.Has(host.ClientStateKey()))
+				}
+			})
+		}
+	}
+}
+
 func (suite *SoloMachineTestSuite) TestRecoverClient() {
 	var (
 		subjectClientID, substituteClientID       string