Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
Mar 25, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 4 additions & 0 deletions docs/docs/05-migrations/13-v8-to-v9.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
15 changes: 0 additions & 15 deletions modules/light-clients/06-solomachine/client_state.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package solomachine

import (
"reflect"

errorsmod "cosmossdk.io/errors"
storetypes "cosmossdk.io/store/types"

Expand Down Expand Up @@ -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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changelog?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it doesn't hurt. Also, even though the migration docs already mention that the interface function has been removed, maybe it's worth adding a separate section in the migration docs to document that this function has also been removed from the concrete type.

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(
Expand Down
59 changes: 0 additions & 59 deletions modules/light-clients/06-solomachine/client_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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} {
Expand Down
12 changes: 11 additions & 1 deletion modules/light-clients/06-solomachine/light_client_module.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package solomachine

import (
"reflect"

Check notice

Code scanning / CodeQL

Sensitive package import Note

Certain system packages contain functions which may be a possible source of non-determinism

errorsmod "cosmossdk.io/errors"

"github.com/cosmos/cosmos-sdk/codec"
Expand Down Expand Up @@ -56,7 +58,15 @@
}

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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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"
)

Expand All @@ -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
Expand Down
Loading