Skip to content

Commit

Permalink
refactor: remove localhost client implementation (cosmos#1187)
Browse files Browse the repository at this point in the history
* refactor: remove localhost light client implementation

* chore: readding CreateLocalhost field

* chore: readding createLocalhost field

* chore: changelog

* fix: BeginBlocker

* fix: removing CreateLocalhost

* chore: remove unused code

* refactor: keeper tests

* refactor: genesis type test add case for invalid client type

* fix: add back tests

* fix: remove unncessary if statement
  • Loading branch information
seantking authored and seunlanlege committed Aug 9, 2022
1 parent 7ed7171 commit fce284c
Show file tree
Hide file tree
Showing 15 changed files with 112 additions and 683 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### State Machine Breaking

### Improvements
* (modules/light-clients/09-localhost) [\#1187](https://github.com/cosmos/ibc-go/pull/1187/) Removing localhost light client implementation as it is not functional.
* [\#1186](https://github.com/cosmos/ibc-go/pull/1186/files) Removing `GetRoot` function from ConsensusState interface in `02-client`. `GetRoot` is unused by core IBC.
* (modules/core/02-client) [\#1196](https://github.com/cosmos/ibc-go/pull/1196) Adding VerifyClientMessage to ClientState interface.
* (modules/core/02-client) [\#1198](https://github.com/cosmos/ibc-go/pull/1198) Adding UpdateStateOnMisbehaviour to ClientState interface.
Expand Down
36 changes: 0 additions & 36 deletions docs/ibc/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,6 @@
- [ibc/core/types/v1/genesis.proto](#ibc/core/types/v1/genesis.proto)
- [GenesisState](#ibc.core.types.v1.GenesisState)

- [ibc/lightclients/localhost/v1/localhost.proto](#ibc/lightclients/localhost/v1/localhost.proto)
- [ClientState](#ibc.lightclients.localhost.v1.ClientState)

- [ibc/lightclients/solomachine/v1/solomachine.proto](#ibc/lightclients/solomachine/v1/solomachine.proto)
- [ChannelStateData](#ibc.lightclients.solomachine.v1.ChannelStateData)
- [ClientState](#ibc.lightclients.solomachine.v1.ClientState)
Expand Down Expand Up @@ -3242,39 +3239,6 @@ GenesisState defines the ibc module's genesis state.



<!-- end messages -->

<!-- end enums -->

<!-- end HasExtensions -->

<!-- end services -->



<a name="ibc/lightclients/localhost/v1/localhost.proto"></a>
<p align="right"><a href="#top">Top</a></p>

## ibc/lightclients/localhost/v1/localhost.proto



<a name="ibc.lightclients.localhost.v1.ClientState"></a>

### ClientState
ClientState defines a loopback (localhost) client. It requires (read-only)
access to keys outside the client prefix.


| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `chain_id` | [string](#string) | | self chain ID |
| `height` | [ibc.core.client.v1.Height](#ibc.core.client.v1.Height) | | self latest block height |





<!-- end messages -->

<!-- end enums -->
Expand Down
11 changes: 4 additions & 7 deletions modules/core/02-client/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ package client
import (
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/cosmos/ibc-go/v5/modules/core/02-client/keeper"
ibctm "github.com/cosmos/ibc-go/v5/modules/light-clients/07-tendermint"
"github.com/cosmos/ibc-go/v3/modules/core/02-client/keeper"
ibctmtypes "github.com/cosmos/ibc-go/v3/modules/light-clients/07-tendermint/types"
)

// BeginBlocker is used to perform IBC client upgrades
Expand All @@ -19,16 +19,13 @@ func BeginBlocker(ctx sdk.Context, k keeper.Keeper) {
// within the trusting period of the last block time on this chain.
_, exists := k.GetUpgradedClient(ctx, plan.Height)
if exists && ctx.BlockHeight() == plan.Height-1 {
upgradedConsState := &ibctm.ConsensusState{
upgradedConsState := &ibctmtypes.ConsensusState{
Timestamp: ctx.BlockTime(),
NextValidatorsHash: ctx.BlockHeader().NextValidatorsHash,
}
bz := k.MustMarshalConsensusState(upgradedConsState)

// SetUpgradedConsensusState always returns nil, hence the blank here.
_ = k.SetUpgradedConsensusState(ctx, plan.Height, bz)

keeper.EmitUpgradeChainEvent(ctx, plan.Height)
k.SetUpgradedConsensusState(ctx, plan.Height, bz)
}
}
}
66 changes: 6 additions & 60 deletions modules/core/02-client/abci_test.go
Original file line number Diff line number Diff line change
@@ -1,19 +1,17 @@
package client_test

import (
"strings"
"testing"

sdk "github.com/cosmos/cosmos-sdk/types"
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
"github.com/stretchr/testify/suite"
abci "github.com/tendermint/tendermint/abci/types"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"

client "github.com/cosmos/ibc-go/v5/modules/core/02-client"
"github.com/cosmos/ibc-go/v5/modules/core/02-client/types"
ibctm "github.com/cosmos/ibc-go/v5/modules/light-clients/07-tendermint"
ibctesting "github.com/cosmos/ibc-go/v5/testing"
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
client "github.com/cosmos/ibc-go/v3/modules/core/02-client"
"github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
ibctmtypes "github.com/cosmos/ibc-go/v3/modules/light-clients/07-tendermint/types"
ibctesting "github.com/cosmos/ibc-go/v3/testing"
)

type ClientTestSuite struct {
Expand Down Expand Up @@ -72,59 +70,7 @@ func (suite *ClientTestSuite) TestBeginBlockerConsensusState() {
// plan Height is at ctx.BlockHeight+1
consState, found := suite.chainA.GetSimApp().UpgradeKeeper.GetUpgradedConsensusState(newCtx, plan.Height)
suite.Require().True(found)
bz, err = types.MarshalConsensusState(suite.chainA.App.AppCodec(), &ibctm.ConsensusState{Timestamp: newCtx.BlockTime(), NextValidatorsHash: nextValsHash})
bz, err = types.MarshalConsensusState(suite.chainA.App.AppCodec(), &ibctmtypes.ConsensusState{Timestamp: newCtx.BlockTime(), NextValidatorsHash: nextValsHash})
suite.Require().NoError(err)
suite.Require().Equal(bz, consState)
}

func (suite *ClientTestSuite) TestBeginBlockerUpgradeEvents() {
plan := &upgradetypes.Plan{
Name: "test",
Height: suite.chainA.GetContext().BlockHeight() + 1,
}
// set upgrade plan in the upgrade store
store := suite.chainA.GetContext().KVStore(suite.chainA.GetSimApp().GetKey(upgradetypes.StoreKey))
bz := suite.chainA.App.AppCodec().MustMarshal(plan)
store.Set(upgradetypes.PlanKey(), bz)

nextValsHash := []byte("nextValsHash")
newCtx := suite.chainA.GetContext().WithBlockHeader(tmproto.Header{
Height: suite.chainA.GetContext().BlockHeight(),
NextValidatorsHash: nextValsHash,
})

err := suite.chainA.GetSimApp().UpgradeKeeper.SetUpgradedClient(newCtx, plan.Height, []byte("client state"))
suite.Require().NoError(err)

cacheCtx, writeCache := suite.chainA.GetContext().CacheContext()

client.BeginBlocker(cacheCtx, suite.chainA.App.GetIBCKeeper().ClientKeeper)
writeCache()

suite.requireContainsEvent(cacheCtx.EventManager().Events(), types.EventTypeUpgradeChain, true)
}

func (suite *ClientTestSuite) TestBeginBlockerUpgradeEventsAbsence() {
cacheCtx, writeCache := suite.chainA.GetContext().CacheContext()
client.BeginBlocker(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper)
writeCache()
suite.requireContainsEvent(cacheCtx.EventManager().Events(), types.EventTypeUpgradeChain, false)
}

// requireContainsEvent verifies if an event of a specific type was emitted.
func (suite *ClientTestSuite) requireContainsEvent(events sdk.Events, eventType string, shouldContain bool) {
found := false
var eventTypes []string
for _, e := range events {
eventTypes = append(eventTypes, e.Type)
if e.Type == eventType {
found = true
break
}
}
if shouldContain {
suite.Require().True(found, "event type %s was not found in %s", eventType, strings.Join(eventTypes, ","))
} else {
suite.Require().False(found, "event type %s was found in %s", eventType, strings.Join(eventTypes, ","))
}
}
12 changes: 2 additions & 10 deletions modules/core/02-client/keeper/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,7 @@ func (k Keeper) CreateClient(
return "", err
}

// check if consensus state is nil in case the created client is Localhost
if consensusState != nil {
k.SetClientConsensusState(ctx, clientID, clientState.GetLatestHeight(), consensusState)
}
k.SetClientConsensusState(ctx, clientID, clientState.GetLatestHeight(), consensusState)

k.Logger(ctx).Info("client created at height", "client-id", clientID, "height", clientState.GetLatestHeight().String())

Expand Down Expand Up @@ -98,12 +95,7 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.C
// Else the update was proof of misbehaviour and we must emit appropriate misbehaviour events.
if status := newClientState.Status(ctx, clientStore, k.cdc); status != exported.Frozen {
// if update is not misbehaviour then update the consensus state
// we don't set consensus state for localhost client
if header != nil && clientID != exported.Localhost {
k.SetClientConsensusState(ctx, clientID, header.GetHeight(), newConsensusState)
} else {
consensusHeight = types.GetSelfHeight(ctx)
}
k.SetClientConsensusState(ctx, clientID, header.GetHeight(), newConsensusState)

k.Logger(ctx).Info("client state updated", "client-id", clientID, "height", consensusHeight.String())

Expand Down
17 changes: 2 additions & 15 deletions modules/core/02-client/keeper/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import (
clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
commitmenttypes "github.com/cosmos/ibc-go/v3/modules/core/23-commitment/types"
"github.com/cosmos/ibc-go/v3/modules/core/exported"
solomachinetypes "github.com/cosmos/ibc-go/v3/modules/light-clients/06-solomachine/types"
ibctmtypes "github.com/cosmos/ibc-go/v3/modules/light-clients/07-tendermint/types"
localhosttypes "github.com/cosmos/ibc-go/v3/modules/light-clients/09-localhost/types"
ibctesting "github.com/cosmos/ibc-go/v3/testing"
ibctestingmock "github.com/cosmos/ibc-go/v3/testing/mock"
)
Expand All @@ -25,7 +25,7 @@ func (suite *KeeperTestSuite) TestCreateClient() {
expPass bool
}{
{"success", ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), true},
{"client type not supported", localhosttypes.NewClientState(testChainID, clienttypes.NewHeight(0, 1)), false},
{"client type not supported", solomachinetypes.NewClientState(0, &solomachinetypes.ConsensusState{suite.solomachine.ConsensusState().PublicKey, suite.solomachine.Diversifier, suite.solomachine.Time}, false), false},
}

for i, tc := range cases {
Expand Down Expand Up @@ -252,19 +252,6 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() {
}
}

func (suite *KeeperTestSuite) TestUpdateClientLocalhost() {
revision := types.ParseChainID(suite.chainA.ChainID)
var localhostClient exported.ClientState = localhosttypes.NewClientState(suite.chainA.ChainID, types.NewHeight(revision, uint64(suite.chainA.GetContext().BlockHeight())))

ctx := suite.chainA.GetContext().WithBlockHeight(suite.chainA.GetContext().BlockHeight() + 1)
err := suite.chainA.App.GetIBCKeeper().ClientKeeper.UpdateClient(ctx, exported.Localhost, nil)
suite.Require().NoError(err)

clientState, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientState(ctx, exported.Localhost)
suite.Require().True(found)
suite.Require().Equal(localhostClient.GetLatestHeight().(types.Height).Increment(), clientState.GetLatestHeight())
}

func (suite *KeeperTestSuite) TestUpgradeClient() {
var (
path *ibctesting.Path
Expand Down
29 changes: 9 additions & 20 deletions modules/core/02-client/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import (
"github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
commitmenttypes "github.com/cosmos/ibc-go/v3/modules/core/23-commitment/types"
"github.com/cosmos/ibc-go/v3/modules/core/exported"
solomachinetypes "github.com/cosmos/ibc-go/v3/modules/light-clients/06-solomachine/types"
ibctmtypes "github.com/cosmos/ibc-go/v3/modules/light-clients/07-tendermint/types"
localhosttypes "github.com/cosmos/ibc-go/v3/modules/light-clients/09-localhost/types"
ibctesting "github.com/cosmos/ibc-go/v3/testing"
ibctestingmock "github.com/cosmos/ibc-go/v3/testing/mock"
"github.com/cosmos/ibc-go/v3/testing/simapp"
Expand Down Expand Up @@ -65,8 +65,8 @@ type KeeperTestSuite struct {
privVal tmtypes.PrivValidator
now time.Time
past time.Time

signers map[string]tmtypes.PrivValidator
solomachine *ibctesting.Solomachine
signers map[string]tmtypes.PrivValidator

// TODO: deprecate
queryClient types.QueryClient
Expand Down Expand Up @@ -122,12 +122,7 @@ func (suite *KeeperTestSuite) SetupTest() {
app.StakingKeeper.SetHistoricalInfo(suite.ctx, int64(i), &hi)
}

// add localhost client
revision := types.ParseChainID(suite.chainA.ChainID)
localHostClient := localhosttypes.NewClientState(
suite.chainA.ChainID, types.NewHeight(revision, uint64(suite.chainA.GetContext().BlockHeight())),
)
suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), exported.Localhost, localHostClient)
suite.solomachine = ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "solomachinesingle", "testing", 1)

// TODO: deprecate
queryHelper := baseapp.NewQueryServerTestHelper(suite.ctx, app.InterfaceRegistry())
Expand Down Expand Up @@ -177,11 +172,6 @@ func (suite *KeeperTestSuite) TestValidateSelfClient() {
ibctmtypes.NewClientState(suite.chainA.ChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), nil, false, false),
true,
},
{
"invalid client type",
localhosttypes.NewClientState(suite.chainA.ChainID, testClientHeight),
false,
},
{
"frozen client",
&ibctmtypes.ClientState{suite.chainA.ChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false},
Expand All @@ -197,6 +187,11 @@ func (suite *KeeperTestSuite) TestValidateSelfClient() {
ibctmtypes.NewClientState(suite.chainA.ChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, types.NewHeight(0, uint64(suite.chainA.GetContext().BlockHeight())), commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false),
false,
},
{
"invalid client type",
solomachinetypes.NewClientState(0, &solomachinetypes.ConsensusState{suite.solomachine.ConsensusState().PublicKey, suite.solomachine.Diversifier, suite.solomachine.Time}, false),
false,
},
{
"invalid client revision",
ibctmtypes.NewClientState(suite.chainA.ChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeightRevision1, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false),
Expand Down Expand Up @@ -256,11 +251,6 @@ func (suite KeeperTestSuite) TestGetAllGenesisClients() {
expGenClients[i] = types.NewIdentifiedClientState(clientIDs[i], expClients[i])
}

// add localhost client
localHostClient, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientState(suite.chainA.GetContext(), exported.Localhost)
suite.Require().True(found)
expGenClients = append(expGenClients, types.NewIdentifiedClientState(exported.Localhost, localHostClient))

genClients := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetAllGenesisClients(suite.chainA.GetContext())

suite.Require().Equal(expGenClients.Sort(), genClients)
Expand All @@ -287,7 +277,6 @@ func (suite KeeperTestSuite) TestGetAllGenesisMetadata() {

genClients := []types.IdentifiedClientState{
types.NewIdentifiedClientState("07-tendermint-1", &ibctmtypes.ClientState{}), types.NewIdentifiedClientState("clientB", &ibctmtypes.ClientState{}),
types.NewIdentifiedClientState("clientC", &ibctmtypes.ClientState{}), types.NewIdentifiedClientState("clientD", &localhosttypes.ClientState{}),
}

suite.chainA.App.GetIBCKeeper().ClientKeeper.SetAllClientMetadata(suite.chainA.GetContext(), expectedGenMetadata)
Expand Down
15 changes: 8 additions & 7 deletions modules/core/02-client/types/codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ package types_test
import (
codectypes "github.com/cosmos/cosmos-sdk/codec/types"

"github.com/cosmos/ibc-go/v5/modules/core/02-client/types"
commitmenttypes "github.com/cosmos/ibc-go/v5/modules/core/23-commitment/types"
"github.com/cosmos/ibc-go/v5/modules/core/exported"
ibctm "github.com/cosmos/ibc-go/v5/modules/light-clients/07-tendermint"
ibctesting "github.com/cosmos/ibc-go/v5/testing"
"github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
commitmenttypes "github.com/cosmos/ibc-go/v3/modules/core/23-commitment/types"
"github.com/cosmos/ibc-go/v3/modules/core/exported"
ibctmtypes "github.com/cosmos/ibc-go/v3/modules/light-clients/07-tendermint/types"
ibctesting "github.com/cosmos/ibc-go/v3/testing"
)

type caseAny struct {
Expand All @@ -17,6 +17,7 @@ type caseAny struct {
}

func (suite *TypesTestSuite) TestPackClientState() {

testCases := []struct {
name string
clientState exported.ClientState
Expand All @@ -29,7 +30,7 @@ func (suite *TypesTestSuite) TestPackClientState() {
},
{
"tendermint client",
ibctm.NewClientState(suite.chainA.ChainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false),
ibctmtypes.NewClientState(chainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false),
true,
},
{
Expand Down Expand Up @@ -117,7 +118,7 @@ func (suite *TypesTestSuite) TestPackClientMessage() {
}{
{
"solo machine header",
ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "solomachine", "", 2).CreateHeader("solomachine"),
ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "solomachine", "", 2).CreateHeader(),
true,
},
{
Expand Down
Loading

0 comments on commit fce284c

Please sign in to comment.