From 617e2417eefbfcb75790e70491337ae0b498f7a4 Mon Sep 17 00:00:00 2001 From: DimitrisJim Date: Wed, 20 Mar 2024 23:56:13 +0200 Subject: [PATCH 1/5] chore: move initialization logic to light client module method. Adjust testing. --- .../06-solomachine/client_state.go | 15 ----- .../06-solomachine/client_state_test.go | 59 ----------------- .../06-solomachine/light_client_module.go | 12 +++- .../light_client_module_test.go | 63 +++++++++++++++++++ 4 files changed, 74 insertions(+), 75 deletions(-) diff --git a/modules/light-clients/06-solomachine/client_state.go b/modules/light-clients/06-solomachine/client_state.go index 24335d0287f..2b816a9dfb7 100644 --- 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 index 5b982aaa407..8bd1fe93891 100644 --- 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 index 4cbf09b3991..35682f9b911 100644 --- 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 index 13331000768..e01bacc41b8 100644 --- a/modules/light-clients/06-solomachine/light_client_module_test.go +++ b/modules/light-clients/06-solomachine/light_client_module_test.go @@ -5,6 +5,7 @@ import ( 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 +14,68 @@ 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 + 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() + + clientStateBz := suite.chainA.Codec.MustMarshal(sm.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) + + 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) TestRecoverClient() { var ( subjectClientID, substituteClientID string From 3ec58b2eb7f00416685d996e45b5811fef40261b Mon Sep 17 00:00:00 2001 From: DimitrisJim Date: Thu, 21 Mar 2024 11:42:48 +0200 Subject: [PATCH 2/5] tests: check for errors in test. --- .../light_client_module_test.go | 40 ++++++++++++++----- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/modules/light-clients/06-solomachine/light_client_module_test.go b/modules/light-clients/06-solomachine/light_client_module_test.go index e01bacc41b8..991d342edc1 100644 --- a/modules/light-clients/06-solomachine/light_client_module_test.go +++ b/modules/light-clients/06-solomachine/light_client_module_test.go @@ -1,6 +1,8 @@ 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" @@ -21,29 +23,46 @@ func (suite *SoloMachineTestSuite) TestInitialize() { malleatedConsensus.Timestamp += 10 testCases := []struct { - name string - consState exported.ConsensusState - expPass bool + name string + consState exported.ConsensusState + clientState exported.ClientState + expErr error }{ { "valid consensus state", sm.ConsensusState(), - true, + sm.ClientState(), + nil, }, { "nil consensus state", nil, - false, + sm.ClientState(), + clienttypes.ErrInvalidConsensus, }, { "invalid consensus state: Tendermint consensus state", &ibctm.ConsensusState{}, - false, + sm.ClientState(), + fmt.Errorf("proto: wrong wireType = 0 for field TypeUrl"), }, { "invalid consensus state: consensus state does not match consensus state in client", malleatedConsensus, - false, + sm.ClientState(), + clienttypes.ErrInvalidConsensus, + }, + { + "invalid client state: sequence is zero", + sm.ConsensusState(), + solomachine.NewClientState(0, sm.ConsensusState()), + clienttypes.ErrInvalidClient, + }, + { + "invalid client state", + sm.ConsensusState(), + &ibctm.ClientState{}, + fmt.Errorf("proto: wrong wireType = 2 for field IsFrozen"), }, } @@ -53,7 +72,7 @@ func (suite *SoloMachineTestSuite) TestInitialize() { suite.Run(tc.name, func() { suite.SetupTest() - clientStateBz := suite.chainA.Codec.MustMarshal(sm.ClientState()) + 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) @@ -64,11 +83,12 @@ func (suite *SoloMachineTestSuite) TestInitialize() { err := lcm.Initialize(suite.chainA.GetContext(), clientID, clientStateBz, consStateBz) store := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), clientID) - if tc.expPass { + 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().Error(err, "invalid testcase: %s passed", tc.name) + suite.Require().ErrorContains(err, tc.expErr.Error()) suite.Require().False(store.Has(host.ClientStateKey())) } }) From 033ff17c12a941dee2837f83f2ffd6ed34bebe3e Mon Sep 17 00:00:00 2001 From: DimitrisJim Date: Thu, 21 Mar 2024 21:54:22 +0200 Subject: [PATCH 3/5] Update modules/light-clients/06-solomachine/light_client_module_test.go Co-authored-by: Carlos Rodriguez --- .../light-clients/06-solomachine/light_client_module_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/light-clients/06-solomachine/light_client_module_test.go b/modules/light-clients/06-solomachine/light_client_module_test.go index 991d342edc1..8db932a3579 100644 --- a/modules/light-clients/06-solomachine/light_client_module_test.go +++ b/modules/light-clients/06-solomachine/light_client_module_test.go @@ -59,7 +59,7 @@ func (suite *SoloMachineTestSuite) TestInitialize() { clienttypes.ErrInvalidClient, }, { - "invalid client state", + "invalid client state: Tendermint client state", sm.ConsensusState(), &ibctm.ClientState{}, fmt.Errorf("proto: wrong wireType = 2 for field IsFrozen"), From 4ec0e97d3fa263dbe7dc0bc0f3e70e70fabc89ed Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Mon, 25 Mar 2024 17:04:14 +0100 Subject: [PATCH 4/5] add migration note --- docs/docs/05-migrations/13-v8-to-v9.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/docs/05-migrations/13-v8-to-v9.md b/docs/docs/05-migrations/13-v8-to-v9.md index 489b88d4f47..1fafc4c8265 100644 --- 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. From e33c37d9ef06e59d67d8be0a2be6df15ad0f5c0a Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Mon, 25 Mar 2024 17:06:49 +0100 Subject: [PATCH 5/5] add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74b300c522b..760c399b8ef 100644 --- 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