refactor: remove localhost client implementation (#1187)

* 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
cosmos · Mar 31, 2022 · 5cf6528 · 5cf6528
1 parent 3c7358b
commit 5cf6528
Show file tree

Hide file tree

Showing 34 changed files with 70 additions and 1,694 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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. 

diff --git a/README.md b/README.md
@@ -59,8 +59,6 @@ The Inter-Blockchain Communication protocol (IBC) allows blockchains to talk to
 
     3.2 [ICS 06 Solo Machine](https://github.com/cosmos/ibc-go/tree/main/modules/light-clients/06-solomachine)
 
-Note: The localhost client is currently non-functional. 
-
 ## Roadmap
 
 For an overview of upcoming changes to ibc-go take a look at the [roadmap](./docs/roadmap/roadmap.md).

diff --git a/docs/OLD_README.md b/docs/OLD_README.md
@@ -28,8 +28,6 @@ For the general specification please refer to the [Interchain Standards](https:/
 
     3.2 [Tendermint Client](./../light-clients/07-tendermint/spec/README.md)
 
-    3.3 [Localhost Client](./../light-clients/09-localhost/spec/README.md)
-
 ## Implementation Details
 
 As stated above, the IBC implementation on the Cosmos SDK introduces some changes
@@ -114,4 +112,4 @@ x/ibc
 │   └── 09-localhost/
 └── testing/
 ```
-<!-- markdown-link-check-enable-->
+<!-- markdown-link-check-enable-->
diff --git a/docs/ibc/integration.md b/docs/ibc/integration.md
@@ -175,17 +175,6 @@ at each height during the `BeginBlock` call. The historical info is required to
 past historical info at any given height in order to verify the light client `ConsensusState` during the
 connection handhake.
 
-The IBC module also has
-[`BeginBlock`](https://github.com/cosmos/ibc-go/blob/main/modules/core/02-client/abci.go) logic as
-well. This is optional as it is only required if your application uses the [localhost
-client](https://github.com/cosmos/ibc/blob/master/spec/client/ics-009-loopback-client) to connect two
-different modules from the same chain.
-
-::: tip
-Only register the ibc module to the `SetOrderBeginBlockers` if your application will use the
-localhost (_aka_ loopback) client.
-:::
-
 ```go
 // app.go
 func NewApp(...args) *App {

diff --git a/docs/ibc/proto-docs.md b/docs/ibc/proto-docs.md
@@ -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)
@@ -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 -->

diff --git a/modules/core/02-client/abci.go b/modules/core/02-client/abci.go
@@ -4,11 +4,10 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	"github.com/cosmos/ibc-go/v3/modules/core/02-client/keeper"
-	"github.com/cosmos/ibc-go/v3/modules/core/exported"
 	ibctmtypes "github.com/cosmos/ibc-go/v3/modules/light-clients/07-tendermint/types"
 )
 
-// BeginBlocker updates an existing localhost client with the latest block height.
+// BeginBlocker is used to perform IBC client upgrades
 func BeginBlocker(ctx sdk.Context, k keeper.Keeper) {
 	plan, found := k.GetUpgradePlan(ctx)
 	if found {
@@ -29,14 +28,4 @@ func BeginBlocker(ctx sdk.Context, k keeper.Keeper) {
 			k.SetUpgradedConsensusState(ctx, plan.Height, bz)
 		}
 	}
-
-	_, found = k.GetClientState(ctx, exported.Localhost)
-	if !found {
-		return
-	}
-
-	// update the localhost client with the latest block height
-	if err := k.UpdateClient(ctx, exported.Localhost, nil); err != nil {
-		panic(err)
-	}
 }
diff --git a/modules/core/02-client/abci_test.go b/modules/core/02-client/abci_test.go
@@ -3,16 +3,14 @@ package client_test
 import (
 	"testing"
 
-	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"
 
+	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"
-	"github.com/cosmos/ibc-go/v3/modules/core/exported"
 	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"
 )
 
@@ -30,36 +28,20 @@ func (suite *ClientTestSuite) SetupTest() {
 
 	suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1))
 	suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(2))
-
-	// set localhost client
-	revision := types.ParseChainID(suite.chainA.GetContext().ChainID())
-	localHostClient := localhosttypes.NewClientState(
-		suite.chainA.GetContext().ChainID(), types.NewHeight(revision, uint64(suite.chainA.GetContext().BlockHeight())),
-	)
-	suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), exported.Localhost, localHostClient)
 }
 
 func TestClientTestSuite(t *testing.T) {
 	suite.Run(t, new(ClientTestSuite))
 }
 
 func (suite *ClientTestSuite) TestBeginBlocker() {
-	prevHeight := types.GetSelfHeight(suite.chainA.GetContext())
-
-	localHostClient := suite.chainA.GetClientState(exported.Localhost)
-	suite.Require().Equal(prevHeight, localHostClient.GetLatestHeight())
-
 	for i := 0; i < 10; i++ {
 		// increment height
 		suite.coordinator.CommitBlock(suite.chainA, suite.chainB)
 
 		suite.Require().NotPanics(func() {
 			client.BeginBlocker(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper)
 		}, "BeginBlocker shouldn't panic")
-
-		localHostClient = suite.chainA.GetClientState(exported.Localhost)
-		suite.Require().Equal(prevHeight.Increment(), localHostClient.GetLatestHeight())
-		prevHeight = localHostClient.GetLatestHeight().(types.Height)
 	}
 }
 

diff --git a/modules/core/02-client/genesis.go b/modules/core/02-client/genesis.go
@@ -46,25 +46,21 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs types.GenesisState) {
 	}
 
 	k.SetNextClientSequence(ctx, gs.NextClientSequence)
-
-	// NOTE: localhost creation is specifically disallowed for the time being.
-	// Issue: https://github.com/cosmos/cosmos-sdk/issues/7871
 }
 
 // ExportGenesis returns the ibc client submodule's exported genesis.
-// NOTE: CreateLocalhost should always be false on export since a
-// created localhost will be included in the exported clients.
 func ExportGenesis(ctx sdk.Context, k keeper.Keeper) types.GenesisState {
 	genClients := k.GetAllGenesisClients(ctx)
 	clientsMetadata, err := k.GetAllClientMetadata(ctx, genClients)
 	if err != nil {
 		panic(err)
 	}
 	return types.GenesisState{
-		Clients:            genClients,
-		ClientsMetadata:    clientsMetadata,
-		ClientsConsensus:   k.GetAllConsensusStates(ctx),
-		Params:             k.GetParams(ctx),
+		Clients:          genClients,
+		ClientsMetadata:  clientsMetadata,
+		ClientsConsensus: k.GetAllConsensusStates(ctx),
+		Params:           k.GetParams(ctx),
+		// Warning: CreateLocalhost is deprecated
 		CreateLocalhost:    false,
 		NextClientSequence: k.GetNextClientSequence(ctx),
 	}

diff --git a/modules/core/02-client/keeper/client.go b/modules/core/02-client/keeper/client.go
@@ -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())
 
@@ -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())
 

diff --git a/modules/core/02-client/keeper/client_test.go b/modules/core/02-client/keeper/client_test.go
@@ -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"
 )
@@ -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 {
@@ -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

diff --git a/modules/core/02-client/keeper/grpc_query_test.go b/modules/core/02-client/keeper/grpc_query_test.go
@@ -140,7 +140,7 @@ func (suite *KeeperTestSuite) TestQueryClientStates() {
 				idcs := types.NewIdentifiedClientState(path1.EndpointA.ClientID, clientStateA1)
 				idcs2 := types.NewIdentifiedClientState(path2.EndpointA.ClientID, clientStateA2)
 
-				// order is sorted by client id, localhost is last
+				// order is sorted by client id
 				expClientStates = types.IdentifiedClientStates{idcs, idcs2}.Sort()
 				req = &types.QueryClientStatesRequest{
 					Pagination: &query.PageRequest{
@@ -156,13 +156,7 @@ func (suite *KeeperTestSuite) TestQueryClientStates() {
 	for _, tc := range testCases {
 		suite.Run(fmt.Sprintf("Case %s", tc.msg), func() {
 			suite.SetupTest() // reset
-			expClientStates = nil
-
 			tc.malleate()
-			// always add localhost which is created by default in init genesis
-			localhostClientState := suite.chainA.GetClientState(exported.Localhost)
-			identifiedLocalhost := types.NewIdentifiedClientState(exported.Localhost, localhostClientState)
-			expClientStates = append(expClientStates, identifiedLocalhost)
 
 			ctx := sdk.WrapSDKContext(suite.chainA.GetContext())
 

diff --git a/modules/core/02-client/keeper/keeper_test.go b/modules/core/02-client/keeper/keeper_test.go
@@ -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"
@@ -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
@@ -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())
@@ -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},
@@ -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),
@@ -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)
@@ -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)

diff --git a/modules/core/02-client/keeper/proposal.go b/modules/core/02-client/keeper/proposal.go
@@ -13,16 +13,11 @@ import (
 // ClientUpdateProposal will retrieve the subject and substitute client.
 // A callback will occur to the subject client state with the client
 // prefixed store being provided for both the subject and the substitute client.
-// The localhost client is not allowed to be modified with a proposal. The IBC
-// client implementations are responsible for validating the parameters of the
+// The IBC client implementations are responsible for validating the parameters of the
 // subtitute (enusring they match the subject's parameters) as well as copying
 // the necessary consensus states from the subtitute to the subject client
 // store. The substitute must be Active and the subject must not be Active.
 func (k Keeper) ClientUpdateProposal(ctx sdk.Context, p *types.ClientUpdateProposal) error {
-	if p.SubjectClientId == exported.Localhost || p.SubstituteClientId == exported.Localhost {
-		return sdkerrors.Wrap(types.ErrInvalidUpdateClientProposal, "cannot update localhost client with proposal")
-	}
-
 	subjectClientState, found := k.GetClientState(ctx, p.SubjectClientId)
 	if !found {
 		return sdkerrors.Wrapf(types.ErrClientNotFound, "subject client with ID %s", p.SubjectClientId)