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

refactor: remove localhost client implementation #1187

Merged
merged 13 commits into from
Mar 31, 2022
Merged
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
4 changes: 1 addition & 3 deletions docs/OLD_README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -114,4 +112,4 @@ x/ibc
│   └── 09-localhost/
└── testing/
```
<!-- markdown-link-check-enable-->
<!-- markdown-link-check-enable-->
11 changes: 0 additions & 11 deletions docs/ibc/integration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
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
42 changes: 0 additions & 42 deletions modules/core/02-client/abci.go

This file was deleted.

65 changes: 0 additions & 65 deletions modules/core/02-client/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,8 @@ 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"

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"
)

Expand All @@ -30,65 +22,8 @@ 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)
}
}

func (suite *ClientTestSuite) TestBeginBlockerConsensusState() {
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
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)

req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()}
suite.chainA.App.BeginBlock(req)

// 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(), &ibctmtypes.ConsensusState{Timestamp: newCtx.BlockTime(), NextValidatorsHash: nextValsHash})
suite.Require().NoError(err)
suite.Require().Equal(bz, consState)
}
6 changes: 0 additions & 6 deletions modules/core/02-client/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,9 @@ 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)
Expand All @@ -65,7 +60,6 @@ func ExportGenesis(ctx sdk.Context, k keeper.Keeper) types.GenesisState {
ClientsMetadata: clientsMetadata,
ClientsConsensus: k.GetAllConsensusStates(ctx),
Params: k.GetParams(ctx),
CreateLocalhost: false,
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
NextClientSequence: k.GetNextClientSequence(ctx),
}
}
8 changes: 2 additions & 6 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,8 +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 {
if header != nil {
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
k.SetClientConsensusState(ctx, clientID, header.GetHeight(), newConsensusState)
} else {
consensusHeight = types.GetSelfHeight(ctx)
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
15 changes: 0 additions & 15 deletions modules/core/02-client/keeper/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
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"
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 +24,6 @@ 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},
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
}

for i, tc := range cases {
Expand Down Expand Up @@ -252,19 +250,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
8 changes: 1 addition & 7 deletions modules/core/02-client/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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())

Expand Down
19 changes: 0 additions & 19 deletions modules/core/02-client/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
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"
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 @@ -122,13 +121,6 @@ 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)

// TODO: deprecate
queryHelper := baseapp.NewQueryServerTestHelper(suite.ctx, app.InterfaceRegistry())
types.RegisterQueryServer(queryHelper, app.IBCKeeper.ClientKeeper)
Expand Down Expand Up @@ -177,11 +169,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,
},
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
{
"frozen client",
&ibctmtypes.ClientState{suite.chainA.ChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false},
Expand Down Expand Up @@ -256,11 +243,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 +269,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
7 changes: 1 addition & 6 deletions modules/core/02-client/keeper/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading