From ad72b805da2783270c08326863172597cd575248 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 14 Dec 2022 10:53:32 +0000 Subject: [PATCH 1/9] set the initial client and consensus state via the client state Initialize method. update godocs --- modules/core/02-client/keeper/client.go | 15 +++++---------- modules/core/exported/client.go | 5 ++--- .../light-clients/06-solomachine/client_state.go | 8 ++++++-- .../light-clients/07-tendermint/client_state.go | 14 +++++++++----- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/modules/core/02-client/keeper/client.go b/modules/core/02-client/keeper/client.go index 1e7bbe9c26a..37f33df1079 100644 --- a/modules/core/02-client/keeper/client.go +++ b/modules/core/02-client/keeper/client.go @@ -10,8 +10,9 @@ import ( "github.com/cosmos/ibc-go/v6/modules/core/exported" ) -// CreateClient creates a new client state and populates it with a given consensus -// state as defined in https://github.com/cosmos/ibc/tree/master/spec/core/ics-002-client-semantics#create +// CreateClient generates a new client identifier and isolated prefix store for the provided client state. +// The client state is responsible for setting any client-specific data in the store via the Initialize method. +// This includes the client state, initial consensus state and associated metadata. func (k Keeper) CreateClient( ctx sdk.Context, clientState exported.ClientState, consensusState exported.ConsensusState, ) (string, error) { @@ -24,18 +25,12 @@ func (k Keeper) CreateClient( } clientID := k.GenerateClientIdentifier(ctx, clientState.ClientType()) + clientStore := k.ClientStore(ctx, clientID) - k.Logger(ctx).Info("client created at height", "client-id", clientID, "height", clientState.GetLatestHeight().String()) - - // verifies initial consensus state against client state and initializes client store with any client-specific metadata - // e.g. set ProcessedTime in Tendermint clients - if err := clientState.Initialize(ctx, k.cdc, k.ClientStore(ctx, clientID), consensusState); err != nil { + if err := clientState.Initialize(ctx, k.cdc, clientStore, consensusState); err != nil { return "", err } - k.SetClientState(ctx, clientID, clientState) - k.SetClientConsensusState(ctx, clientID, clientState.GetLatestHeight(), consensusState) - k.Logger(ctx).Info("client created at height", "client-id", clientID, "height", clientState.GetLatestHeight().String()) defer telemetry.IncrCounterWithLabels( diff --git a/modules/core/exported/client.go b/modules/core/exported/client.go index 81dea3373c5..3b2305beb71 100644 --- a/modules/core/exported/client.go +++ b/modules/core/exported/client.go @@ -59,9 +59,8 @@ type ClientState interface { height Height, ) (uint64, error) - // Initialization function - // Clients must validate the initial consensus state, and may store any client-specific metadata - // necessary for correct light client operation + // Initialize is called upon client creation, it allows the client to perform validation on the initial consensus state and set the + // client state, consensus state and any client-specific metadata necessary for correct light client operation in the provided client store. Initialize(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, consensusState ConsensusState) error // VerifyMembership is a generic proof verification method which verifies a proof of the existence of a value at a given CommitmentPath at the specified height. diff --git a/modules/light-clients/06-solomachine/client_state.go b/modules/light-clients/06-solomachine/client_state.go index ed2bf259418..fc7b8057f65 100644 --- a/modules/light-clients/06-solomachine/client_state.go +++ b/modules/light-clients/06-solomachine/client_state.go @@ -77,12 +77,16 @@ func (cs ClientState) ZeroCustomFields() exported.ClientState { panic("ZeroCustomFields is not implemented as the solo machine implementation does not support upgrades.") } -// Initialize will check that initial consensus state is equal to the latest consensus state of the initial client. -func (cs ClientState) Initialize(_ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore, consState exported.ConsensusState) error { +// Initialize checks that the initial consensus state is equal to the latest consensus state of the initial client and +// set the client state in the provided client store. +func (cs ClientState) Initialize(_ sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, consState exported.ConsensusState) error { if !reflect.DeepEqual(cs.ConsensusState, consState) { return sdkerrors.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 } diff --git a/modules/light-clients/07-tendermint/client_state.go b/modules/light-clients/07-tendermint/client_state.go index 9f6da36bf00..7b40dba24c4 100644 --- a/modules/light-clients/07-tendermint/client_state.go +++ b/modules/light-clients/07-tendermint/client_state.go @@ -188,15 +188,19 @@ func (cs ClientState) ZeroCustomFields() exported.ClientState { } } -// Initialize will check that initial consensus state is a Tendermint consensus state -// and will store ProcessedTime for initial consensus state as ctx.BlockTime() -func (cs ClientState) Initialize(ctx sdk.Context, _ codec.BinaryCodec, clientStore sdk.KVStore, consState exported.ConsensusState) error { - if _, ok := consState.(*ConsensusState); !ok { +// Initialize checks that the initial consensus state is a Tendermint consensus state and +// sets the client state, consensus state and associated metatdata in the provided client store. +func (cs ClientState) Initialize(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, consState exported.ConsensusState) error { + consensusState, ok := consState.(*ConsensusState) + if !ok { return sdkerrors.Wrapf(clienttypes.ErrInvalidConsensus, "invalid initial consensus state. expected type: %T, got: %T", &ConsensusState{}, consState) } - // set metadata for initial consensus state. + + setClientState(clientStore, cdc, &cs) + setConsensusState(clientStore, cdc, consensusState, cs.GetLatestHeight()) setConsensusMetadata(ctx, clientStore, cs.GetLatestHeight()) + return nil } From 53e2ffcea9faa6d52a20eee05b639344fc7e2329 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 14 Dec 2022 10:56:02 +0000 Subject: [PATCH 2/9] updating godocs --- modules/core/02-client/keeper/client.go | 2 +- modules/light-clients/06-solomachine/client_state.go | 2 +- modules/light-clients/07-tendermint/client_state.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/core/02-client/keeper/client.go b/modules/core/02-client/keeper/client.go index 37f33df1079..a593c682fd9 100644 --- a/modules/core/02-client/keeper/client.go +++ b/modules/core/02-client/keeper/client.go @@ -12,7 +12,7 @@ import ( // CreateClient generates a new client identifier and isolated prefix store for the provided client state. // The client state is responsible for setting any client-specific data in the store via the Initialize method. -// This includes the client state, initial consensus state and associated metadata. +// This includes the client state, initial consensus state and any associated metadata. func (k Keeper) CreateClient( ctx sdk.Context, clientState exported.ClientState, consensusState exported.ConsensusState, ) (string, error) { diff --git a/modules/light-clients/06-solomachine/client_state.go b/modules/light-clients/06-solomachine/client_state.go index fc7b8057f65..5148a35aa62 100644 --- a/modules/light-clients/06-solomachine/client_state.go +++ b/modules/light-clients/06-solomachine/client_state.go @@ -78,7 +78,7 @@ func (cs ClientState) ZeroCustomFields() exported.ClientState { } // Initialize checks that the initial consensus state is equal to the latest consensus state of the initial client and -// set the client state in the provided client store. +// sets the client state in the provided client store. func (cs ClientState) Initialize(_ sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, consState exported.ConsensusState) error { if !reflect.DeepEqual(cs.ConsensusState, consState) { return sdkerrors.Wrapf(clienttypes.ErrInvalidConsensus, "consensus state in initial client does not equal initial consensus state. expected: %s, got: %s", diff --git a/modules/light-clients/07-tendermint/client_state.go b/modules/light-clients/07-tendermint/client_state.go index 7b40dba24c4..aad7cfd4586 100644 --- a/modules/light-clients/07-tendermint/client_state.go +++ b/modules/light-clients/07-tendermint/client_state.go @@ -188,7 +188,7 @@ func (cs ClientState) ZeroCustomFields() exported.ClientState { } } -// Initialize checks that the initial consensus state is a Tendermint consensus state and +// Initialize checks that the initial consensus state is a 07-tendermint consensus state and // sets the client state, consensus state and associated metatdata in the provided client store. func (cs ClientState) Initialize(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, consState exported.ConsensusState) error { consensusState, ok := consState.(*ConsensusState) From 12dbd3d38cb09b8fb015c1f0c775eaab024b03f7 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 14 Dec 2022 11:02:10 +0000 Subject: [PATCH 3/9] updating migration doc --- docs/migrations/v6-to-v7.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/migrations/v6-to-v7.md b/docs/migrations/v6-to-v7.md index 64cd0d54f28..95025ccf0df 100644 --- a/docs/migrations/v6-to-v7.md +++ b/docs/migrations/v6-to-v7.md @@ -106,7 +106,9 @@ The `VerifyUpgradeAndUpdateState` function has been modified. The client state a Light clients **must** handle all management of client and consensus states including the setting of updated client state and consensus state in the client store. -The `CheckHeaderAndUpdateState` function has been split into 4 new functions: +The `Initialize` method is now expected to set the initial client state, consensus state and any client-specific metadata in the provided store upon client creation. + +The `CheckHeaderAndUpdateState` method has been split into 4 new functions: - `VerifyClientMessage` verifies a `ClientMessage`. A `ClientMessage` could be a `Header`, `Misbehaviour`, or batch update. Calls to `CheckForMisbehaviour`, `UpdateState`, and `UpdateStateOnMisbehaviour` will assume that the content of the `ClientMessage` has been verified and can be trusted. An error should be returned if the `ClientMessage` fails to verify. From ea4066959b783c9807631791e48a244cff9da3ed Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 14 Dec 2022 11:09:36 +0000 Subject: [PATCH 4/9] updating light client guide docs for Initialize method --- docs/ibc/light-clients/client-state.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/ibc/light-clients/client-state.md b/docs/ibc/light-clients/client-state.md index fa32e90fe8a..3173726c1a2 100644 --- a/docs/ibc/light-clients/client-state.md +++ b/docs/ibc/light-clients/client-state.md @@ -48,9 +48,10 @@ This value is used to facilitate timeouts by checking the packet timeout timesta ## `Initialize` method -Clients must validate the initial consensus state, and may store any client-specific metadata necessary. +Clients must validate the initial consensus state, and set the initial client state and consensus state in the provided client store. +Clients may also store any client-specific metadata necessary. -`Initialize` gets called when a [client is created](https://github.com/cosmos/ibc-go/blob/main/modules/core/02-client/keeper/client.go#L32). +`Initialize` is called when a [client is created](https://github.com/cosmos/ibc-go/blob/main/modules/core/02-client/keeper/client.go#L32). ## `VerifyMembership` method From 8a5b5bc71cb24602dbda677f243de3c6c2a63de2 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 14 Dec 2022 11:16:32 +0000 Subject: [PATCH 5/9] updating migration doc --- docs/migrations/v6-to-v7.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/migrations/v6-to-v7.md b/docs/migrations/v6-to-v7.md index 95025ccf0df..6649b216f6e 100644 --- a/docs/migrations/v6-to-v7.md +++ b/docs/migrations/v6-to-v7.md @@ -108,7 +108,7 @@ Light clients **must** handle all management of client and consensus states incl The `Initialize` method is now expected to set the initial client state, consensus state and any client-specific metadata in the provided store upon client creation. -The `CheckHeaderAndUpdateState` method has been split into 4 new functions: +The `CheckHeaderAndUpdateState` method has been split into 4 new methods: - `VerifyClientMessage` verifies a `ClientMessage`. A `ClientMessage` could be a `Header`, `Misbehaviour`, or batch update. Calls to `CheckForMisbehaviour`, `UpdateState`, and `UpdateStateOnMisbehaviour` will assume that the content of the `ClientMessage` has been verified and can be trusted. An error should be returned if the `ClientMessage` fails to verify. From ad7d7cedfc89591bd9c29decd43962d330549d4e Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 14 Dec 2022 11:46:44 +0000 Subject: [PATCH 6/9] Apply suggestions from code review Co-authored-by: Charly --- modules/light-clients/07-tendermint/client_state.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/light-clients/07-tendermint/client_state.go b/modules/light-clients/07-tendermint/client_state.go index aad7cfd4586..cc6fbe9c482 100644 --- a/modules/light-clients/07-tendermint/client_state.go +++ b/modules/light-clients/07-tendermint/client_state.go @@ -188,8 +188,8 @@ func (cs ClientState) ZeroCustomFields() exported.ClientState { } } -// Initialize checks that the initial consensus state is a 07-tendermint consensus state and -// sets the client state, consensus state and associated metatdata in the provided client store. +// Initialize checks that the initial consensus state is an 07-tendermint consensus state and +// sets the client state, consensus state and associated metadata in the provided client store. func (cs ClientState) Initialize(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, consState exported.ConsensusState) error { consensusState, ok := consState.(*ConsensusState) if !ok { From 3a42a6a4bec1efd2a1ab96f67bec9611d9f8c76d Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 14 Dec 2022 20:33:28 +0000 Subject: [PATCH 7/9] Update docs/ibc/light-clients/client-state.md Co-authored-by: Carlos Rodriguez --- docs/ibc/light-clients/client-state.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/ibc/light-clients/client-state.md b/docs/ibc/light-clients/client-state.md index 3173726c1a2..024da207fbb 100644 --- a/docs/ibc/light-clients/client-state.md +++ b/docs/ibc/light-clients/client-state.md @@ -49,7 +49,7 @@ This value is used to facilitate timeouts by checking the packet timeout timesta ## `Initialize` method Clients must validate the initial consensus state, and set the initial client state and consensus state in the provided client store. -Clients may also store any client-specific metadata necessary. +Clients may also store any necessary client-specific metadata. `Initialize` is called when a [client is created](https://github.com/cosmos/ibc-go/blob/main/modules/core/02-client/keeper/client.go#L32). From 5e896eb171e57ab6ed70e8c0d70f27f19248070f Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Thu, 15 Dec 2022 16:43:37 +0000 Subject: [PATCH 8/9] adding tests to lightclients --- .../06-solomachine/client_state_test.go | 9 ++++++-- .../07-tendermint/client_state_test.go | 23 ++++++++++++++----- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/modules/light-clients/06-solomachine/client_state_test.go b/modules/light-clients/06-solomachine/client_state_test.go index e494a484ec8..dbb958ae7ba 100644 --- a/modules/light-clients/06-solomachine/client_state_test.go +++ b/modules/light-clients/06-solomachine/client_state_test.go @@ -8,6 +8,7 @@ import ( clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types" commitmenttypes "github.com/cosmos/ibc-go/v6/modules/core/23-commitment/types" + host "github.com/cosmos/ibc-go/v6/modules/core/24-host" "github.com/cosmos/ibc-go/v6/modules/core/exported" solomachine "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine" ibctm "github.com/cosmos/ibc-go/v6/modules/light-clients/07-tendermint" @@ -125,16 +126,20 @@ func (suite *SoloMachineTestSuite) TestInitialize() { } for _, tc := range testCases { + suite.SetupTest() + + store := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), "solomachine") err := sm.ClientState().Initialize( suite.chainA.GetContext(), suite.chainA.Codec, - suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), "solomachine"), - tc.consState, + 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())) } } } diff --git a/modules/light-clients/07-tendermint/client_state_test.go b/modules/light-clients/07-tendermint/client_state_test.go index 64ffff0fd50..be7cccf2ce4 100644 --- a/modules/light-clients/07-tendermint/client_state_test.go +++ b/modules/light-clients/07-tendermint/client_state_test.go @@ -192,19 +192,30 @@ func (suite *TendermintTestSuite) TestInitialize() { }, } - path := ibctesting.NewPath(suite.chainA, suite.chainB) - err := path.EndpointA.CreateClient() - suite.Require().NoError(err) + for _, tc := range testCases { + suite.SetupTest() + path := ibctesting.NewPath(suite.chainA, suite.chainB) - clientState := suite.chainA.GetClientState(path.EndpointA.ClientID) - store := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) + tmConfig, ok := path.EndpointB.ClientConfig.(*ibctesting.TendermintConfig) + suite.Require().True(ok) - for _, tc := range testCases { + clientState := ibctm.NewClientState( + path.EndpointB.Chain.ChainID, + tmConfig.TrustLevel, tmConfig.TrustingPeriod, tmConfig.UnbondingPeriod, tmConfig.MaxClockDrift, + suite.chainB.LastHeader.GetTrustedHeight(), commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, + ) + + store := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) err := clientState.Initialize(suite.chainA.GetContext(), suite.chainA.Codec, store, tc.consensusState) + if tc.expPass { suite.Require().NoError(err, "valid case returned an error") + suite.Require().True(store.Has(host.ClientStateKey())) + suite.Require().True(store.Has(host.ConsensusStateKey(suite.chainB.LastHeader.GetTrustedHeight()))) } else { suite.Require().Error(err, "invalid case didn't return an error") + suite.Require().False(store.Has(host.ClientStateKey())) + suite.Require().False(store.Has(host.ConsensusStateKey(suite.chainB.LastHeader.GetTrustedHeight()))) } } } From e2d2544dd8b063dc7ec4913e071e203ad50bad76 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Thu, 15 Dec 2022 16:48:14 +0000 Subject: [PATCH 9/9] updating 02-client tests --- modules/core/02-client/keeper/client_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/core/02-client/keeper/client_test.go b/modules/core/02-client/keeper/client_test.go index 12acddb4ff1..f20ba1dcdc1 100644 --- a/modules/core/02-client/keeper/client_test.go +++ b/modules/core/02-client/keeper/client_test.go @@ -10,6 +10,7 @@ import ( "github.com/cosmos/ibc-go/v6/modules/core/02-client/types" clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types" commitmenttypes "github.com/cosmos/ibc-go/v6/modules/core/23-commitment/types" + host "github.com/cosmos/ibc-go/v6/modules/core/24-host" "github.com/cosmos/ibc-go/v6/modules/core/exported" solomachine "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine" ibctm "github.com/cosmos/ibc-go/v6/modules/light-clients/07-tendermint" @@ -32,9 +33,11 @@ func (suite *KeeperTestSuite) TestCreateClient() { if tc.expPass { suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.msg) suite.Require().NotNil(clientID, "valid test case %d failed: %s", i, tc.msg) + suite.Require().True(suite.keeper.ClientStore(suite.ctx, clientID).Has(host.ClientStateKey())) } else { suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.msg) suite.Require().Equal("", clientID, "invalid test case %d passed: %s", i, tc.msg) + suite.Require().False(suite.keeper.ClientStore(suite.ctx, clientID).Has(host.ClientStateKey())) } } }