Skip to content

Commit

Permalink
chore(06-solomachine, 07-tendermint): make client state methods moved…
Browse files Browse the repository at this point in the history
… to lcm private (#6891)

* chore(06-solomachine, 07-tendermint)make client state methods moved to lcm private.

* docs: add changelog, add to migration doc.

* Update CHANGELOG.md

* Update docs/docs/05-migrations/13-v8-to-v9.md
  • Loading branch information
DimitrisJim committed Jul 22, 2024
1 parent f738c22 commit a6fd4d7
Show file tree
Hide file tree
Showing 10 changed files with 42 additions and 53 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (core/02-client) [\#6777](https://github.com/cosmos/ibc-go/pull/6777) The `NewClientProposalHandler` of `02-client` has been removed.
* (core/types) [\#6794](https://github.com/cosmos/ibc-go/pull/6794) The composite interface `QueryServer` has been removed from package `core/types`. Please use the granular `QueryServer` interfaces provided by each core submodule.
* (light-clients/06-solomachine) [\#6888](https://github.com/cosmos/ibc-go/pull/6888) Remove `TypeClientMisbehaviour` constant and the `Type` method on `Misbehaviour`.
* (light-clients/06-solomachine, light-clients/07-tendermint) [\#6891](https://github.com/cosmos/ibc-go/pull/6891) The `VerifyMembership` and `VerifyNonMembership` functions of solomachine's `ClientState` have been made private. The `VerifyMembership`, `VerifyNonMembership`, `GetTimestampAtHeight`, `Status` and `Initialize` functions of tendermint's `ClientState` have been made private.

### State Machine Breaking

Expand Down
4 changes: 2 additions & 2 deletions docs/docs/05-migrations/13-v8-to-v9.md
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,12 @@ Please check also the [Light client developer guide](../03-light-clients/01-deve

### 06-solomachine

- The `Initialize`, `Status`, `GetTimestampAtHeight` and `UpdateStateOnMisbehaviour` functions in `ClientState` have been removed and all their logic has been moved to functions of the `LightClientModule`.
- The `Initialize`, `Status`, `GetTimestampAtHeight` and `UpdateStateOnMisbehaviour` functions in `ClientState` have been removed and all their logic has been moved to functions of the `LightClientModule`. The `VerifyMembership` and `VerifyNonMembership` functions have been made private.
- The `Type` method on `Misbehaviour` has been removed.

### 07-tendermint

The `IterateConsensusMetadata` function has been removed.
The `IterateConsensusMetadata` function has been removed. The `VerifyMembership`, `VerifyNonMembership`, `GetTimestampAtHeight`, `Status` and `Initialize` functions have been made private.

### 08-wasm

Expand Down
7 changes: 3 additions & 4 deletions modules/core/02-client/keeper/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -682,10 +682,9 @@ func (suite *KeeperTestSuite) TestRecoverClient() {
ibctesting.AssertEvents(&suite.Suite, expectedEvents, ctx.EventManager().Events().ToABCIEvents())

// Assert that client status is now Active
clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID)
tmClientState, ok := subjectPath.EndpointA.GetClientState().(*ibctm.ClientState)
suite.Require().True(ok)
suite.Require().Equal(tmClientState.Status(suite.chainA.GetContext(), clientStore, suite.chainA.App.AppCodec()), exported.Active)
lightClientModule, err := suite.chainA.App.GetIBCKeeper().ClientKeeper.Route(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID)
suite.Require().NoError(err)
suite.Require().Equal(lightClientModule.Status(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID), exported.Active)

} else {
suite.Require().Error(err)
Expand Down
8 changes: 4 additions & 4 deletions modules/core/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,10 +287,10 @@ func (suite *KeeperTestSuite) TestRecoverClient() {
suite.Require().NoError(err)

// Assert that client status is now Active
clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID)
tmClientState, ok := subjectPath.EndpointA.GetClientState().(*ibctm.ClientState)
suite.Require().True(ok)
suite.Require().Equal(tmClientState.Status(suite.chainA.GetContext(), clientStore, suite.chainA.App.AppCodec()), exported.Active)

lightClientModule, err := suite.chainA.App.GetIBCKeeper().ClientKeeper.Route(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID)
suite.Require().NoError(err)
suite.Require().Equal(lightClientModule.Status(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID), exported.Active)
} else {
suite.Require().Error(err)
suite.Require().ErrorIs(err, tc.expErr)
Expand Down
17 changes: 4 additions & 13 deletions modules/light-clients/06-solomachine/client_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (

"github.com/cosmos/cosmos-sdk/codec"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/tx/signing"

clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types"
Expand Down Expand Up @@ -43,15 +42,11 @@ func (cs ClientState) Validate() error {
return cs.ConsensusState.ValidateBasic()
}

// 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.
// 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(
ctx sdk.Context,
func (cs *ClientState) verifyMembership(
clientStore storetypes.KVStore,
cdc codec.BinaryCodec,
_ exported.Height,
delayTimePeriod uint64,
delayBlockPeriod uint64,
proof []byte,
path exported.Path,
value []byte,
Expand Down Expand Up @@ -100,15 +95,11 @@ func (cs *ClientState) VerifyMembership(
return nil
}

// VerifyNonMembership is a generic proof verification method which verifies the absence of a given CommitmentPath at the latest sequence.
// verifyNonMembership is a generic proof verification method which verifies the absence of 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) VerifyNonMembership(
ctx sdk.Context,
func (cs *ClientState) verifyNonMembership(
clientStore storetypes.KVStore,
cdc codec.BinaryCodec,
_ exported.Height,
delayTimePeriod uint64,
delayBlockPeriod uint64,
proof []byte,
path exported.Path,
) error {
Expand Down
8 changes: 4 additions & 4 deletions modules/light-clients/06-solomachine/light_client_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func (l LightClientModule) UpdateState(ctx sdk.Context, clientID string, clientM
return clientState.UpdateState(ctx, l.cdc, clientStore, clientMsg)
}

// VerifyMembership obtains the client state associated with the client identifier and calls into the clientState.VerifyMembership method.
// VerifyMembership obtains the client state associated with the client identifier and calls into the clientState.verifyMembership method.
//
// CONTRACT: clientID is validated in 02-client router, thus clientID is assumed here to have the format 06-solomachine-{n}.
func (l LightClientModule) VerifyMembership(
Expand All @@ -137,10 +137,10 @@ func (l LightClientModule) VerifyMembership(
return errorsmod.Wrap(clienttypes.ErrClientNotFound, clientID)
}

return clientState.VerifyMembership(ctx, clientStore, l.cdc, height, delayTimePeriod, delayBlockPeriod, proof, path, value)
return clientState.verifyMembership(clientStore, l.cdc, proof, path, value)
}

// VerifyNonMembership obtains the client state associated with the client identifier and calls into the clientState.VerifyNonMembership method.
// VerifyNonMembership obtains the client state associated with the client identifier and calls into the clientState.verifyNonMembership method.
//
// CONTRACT: clientID is validated in 02-client router, thus clientID is assumed here to have the format 06-solomachine-{n}.
func (l LightClientModule) VerifyNonMembership(
Expand All @@ -158,7 +158,7 @@ func (l LightClientModule) VerifyNonMembership(
return errorsmod.Wrap(clienttypes.ErrClientNotFound, clientID)
}

return clientState.VerifyNonMembership(ctx, clientStore, l.cdc, height, delayTimePeriod, delayBlockPeriod, proof, path)
return clientState.verifyNonMembership(clientStore, l.cdc, proof, path)
}

// Status returns the status of the solo machine client.
Expand Down
21 changes: 10 additions & 11 deletions modules/light-clients/07-tendermint/client_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,8 @@ func (ClientState) ClientType() string {
return exported.Tendermint
}

// GetTimestampAtHeight returns the timestamp in nanoseconds of the consensus state at the given height.
func (ClientState) GetTimestampAtHeight(
ctx sdk.Context,
// getTimestampAtHeight returns the timestamp in nanoseconds of the consensus state at the given height.
func (ClientState) getTimestampAtHeight(
clientStore storetypes.KVStore,
cdc codec.BinaryCodec,
height exported.Height,
Expand All @@ -69,15 +68,15 @@ func (ClientState) GetTimestampAtHeight(
return consState.GetTimestamp(), nil
}

// Status returns the status of the tendermint client.
// status returns the status of the tendermint client.
// The client may be:
// - Active: FrozenHeight is zero and client is not expired
// - Frozen: Frozen Height is not zero
// - Expired: the latest consensus state timestamp + trusting period <= current time
//
// A frozen client will become expired, so the Frozen status
// has higher precedence.
func (cs ClientState) Status(
func (cs ClientState) status(
ctx sdk.Context,
clientStore storetypes.KVStore,
cdc codec.BinaryCodec,
Expand Down Expand Up @@ -185,9 +184,9 @@ func (cs ClientState) ZeroCustomFields() *ClientState {
}
}

// Initialize checks that the initial consensus state is an 07-tendermint consensus state and
// 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 storetypes.KVStore, consState exported.ConsensusState) error {
func (cs ClientState) initialize(ctx sdk.Context, cdc codec.BinaryCodec, clientStore storetypes.KVStore, consState exported.ConsensusState) error {
consensusState, ok := consState.(*ConsensusState)
if !ok {
return errorsmod.Wrapf(clienttypes.ErrInvalidConsensus, "invalid initial consensus state. expected type: %T, got: %T",
Expand All @@ -201,10 +200,10 @@ func (cs ClientState) Initialize(ctx sdk.Context, cdc codec.BinaryCodec, clientS
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 specified height.
// 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.
// The caller is expected to construct the full CommitmentPath from a CommitmentPrefix and a standardized path (as defined in ICS 24).
// If a zero proof height is passed in, it will fail to retrieve the associated consensus state.
func (cs ClientState) VerifyMembership(
func (cs ClientState) verifyMembership(
ctx sdk.Context,
clientStore storetypes.KVStore,
cdc codec.BinaryCodec,
Expand Down Expand Up @@ -244,10 +243,10 @@ func (cs ClientState) VerifyMembership(
return merkleProof.VerifyMembership(cs.ProofSpecs, consensusState.GetRoot(), merklePath, value)
}

// VerifyNonMembership is a generic proof verification method which verifies the absence of a given CommitmentPath at a specified height.
// verifyNonMembership is a generic proof verification method which verifies the absence of a given CommitmentPath at a specified height.
// The caller is expected to construct the full CommitmentPath from a CommitmentPrefix and a standardized path (as defined in ICS 24).
// If a zero proof height is passed in, it will fail to retrieve the associated consensus state.
func (cs ClientState) VerifyNonMembership(
func (cs ClientState) verifyNonMembership(
ctx sdk.Context,
clientStore storetypes.KVStore,
cdc codec.BinaryCodec,
Expand Down
20 changes: 10 additions & 10 deletions modules/light-clients/07-tendermint/light_client_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func NewLightClientModule(cdc codec.BinaryCodec, storeProvider clienttypes.Store
}

// Initialize unmarshals the provided client and consensus states and performs basic validation. It calls into the
// clientState.Initialize method.
// clientState.initialize method.
//
// CONTRACT: clientID is validated in 02-client router, thus clientID is assumed here to have the format 07-tendermint-{n}.
func (l LightClientModule) Initialize(ctx sdk.Context, clientID string, clientStateBz, consensusStateBz []byte) error {
Expand All @@ -54,7 +54,7 @@ func (l LightClientModule) Initialize(ctx sdk.Context, clientID string, clientSt

clientStore := l.storeProvider.ClientStore(ctx, clientID)

return clientState.Initialize(ctx, l.cdc, clientStore, &consensusState)
return clientState.initialize(ctx, l.cdc, clientStore, &consensusState)
}

// VerifyClientMessage obtains the client state associated with the client identifier and calls into the clientState.VerifyClientMessage method.
Expand Down Expand Up @@ -109,7 +109,7 @@ func (l LightClientModule) UpdateState(ctx sdk.Context, clientID string, clientM
return clientState.UpdateState(ctx, l.cdc, clientStore, clientMsg)
}

// VerifyMembership obtains the client state associated with the client identifier and calls into the clientState.VerifyMembership method.
// VerifyMembership obtains the client state associated with the client identifier and calls into the clientState.verifyMembership method.
//
// CONTRACT: clientID is validated in 02-client router, thus clientID is assumed here to have the format 07-tendermint-{n}.
func (l LightClientModule) VerifyMembership(
Expand All @@ -128,10 +128,10 @@ func (l LightClientModule) VerifyMembership(
return errorsmod.Wrap(clienttypes.ErrClientNotFound, clientID)
}

return clientState.VerifyMembership(ctx, clientStore, l.cdc, height, delayTimePeriod, delayBlockPeriod, proof, path, value)
return clientState.verifyMembership(ctx, clientStore, l.cdc, height, delayTimePeriod, delayBlockPeriod, proof, path, value)
}

// VerifyNonMembership obtains the client state associated with the client identifier and calls into the clientState.VerifyNonMembership method.
// VerifyNonMembership obtains the client state associated with the client identifier and calls into the clientState.verifyNonMembership method.
//
// CONTRACT: clientID is validated in 02-client router, thus clientID is assumed here to have the format 07-tendermint-{n}.
func (l LightClientModule) VerifyNonMembership(
Expand All @@ -149,10 +149,10 @@ func (l LightClientModule) VerifyNonMembership(
return errorsmod.Wrap(clienttypes.ErrClientNotFound, clientID)
}

return clientState.VerifyNonMembership(ctx, clientStore, l.cdc, height, delayTimePeriod, delayBlockPeriod, proof, path)
return clientState.verifyNonMembership(ctx, clientStore, l.cdc, height, delayTimePeriod, delayBlockPeriod, proof, path)
}

// Status obtains the client state associated with the client identifier and calls into the clientState.Status method.
// Status obtains the client state associated with the client identifier and calls into the clientState.status method.
//
// CONTRACT: clientID is validated in 02-client router, thus clientID is assumed here to have the format 07-tendermint-{n}.
func (l LightClientModule) Status(ctx sdk.Context, clientID string) exported.Status {
Expand All @@ -162,7 +162,7 @@ func (l LightClientModule) Status(ctx sdk.Context, clientID string) exported.Sta
return exported.Unknown
}

return clientState.Status(ctx, clientStore, l.cdc)
return clientState.status(ctx, clientStore, l.cdc)
}

// LatestHeight returns the latest height for the client state for the given client identifier.
Expand All @@ -179,7 +179,7 @@ func (l LightClientModule) LatestHeight(ctx sdk.Context, clientID string) export
return clientState.LatestHeight
}

// TimestampAtHeight obtains the client state associated with the client identifier and calls into the clientState.GetTimestampAtHeight method.
// TimestampAtHeight obtains the client state associated with the client identifier and calls into the clientState.getTimestampAtHeight method.
//
// CONTRACT: clientID is validated in 02-client router, thus clientID is assumed here to have the format 07-tendermint-{n}.
func (l LightClientModule) TimestampAtHeight(
Expand All @@ -193,7 +193,7 @@ func (l LightClientModule) TimestampAtHeight(
return 0, errorsmod.Wrap(clienttypes.ErrClientNotFound, clientID)
}

return clientState.GetTimestampAtHeight(ctx, clientStore, l.cdc, height)
return clientState.getTimestampAtHeight(clientStore, l.cdc, height)
}

// RecoverClient asserts that the substitute client is a tendermint client. It obtains the client state associated with the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1026,10 +1026,9 @@ func (suite *TendermintTestSuite) TestRecoverClient() {
suite.Require().NoError(err)

// assert that status of subject client is now Active
clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(ctx, subjectClientID)
tmClientState, ok := subjectPath.EndpointA.GetClientState().(*ibctm.ClientState)
suite.Require().True(ok)
suite.Require().Equal(exported.Active, tmClientState.Status(ctx, clientStore, suite.chainA.App.AppCodec()))
lightClientModule, err := suite.chainA.App.GetIBCKeeper().ClientKeeper.Route(suite.chainA.GetContext(), subjectClientID)
suite.Require().NoError(err)
suite.Require().Equal(lightClientModule.Status(suite.chainA.GetContext(), subjectClientID), exported.Active)
} else {
suite.Require().Error(err)
suite.Require().ErrorIs(err, tc.expErr)
Expand Down
2 changes: 1 addition & 1 deletion modules/light-clients/07-tendermint/proposal_handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (cs ClientState) CheckSubstituteAndUpdateState(
return errorsmod.Wrap(clienttypes.ErrInvalidSubstitute, "subject client state does not match substitute client state")
}

if cs.Status(ctx, subjectClientStore, cdc) == exported.Frozen {
if cs.status(ctx, subjectClientStore, cdc) == exported.Frozen {
// unfreeze the client
cs.FrozenHeight = clienttypes.ZeroHeight()
}
Expand Down

0 comments on commit a6fd4d7

Please sign in to comment.