diff --git a/x/ibc/core/02-client/abci_test.go b/x/ibc/core/02-client/abci_test.go index 33774b7c53..fe9016750f 100644 --- a/x/ibc/core/02-client/abci_test.go +++ b/x/ibc/core/02-client/abci_test.go @@ -8,6 +8,7 @@ import ( client "github.com/cosmos/cosmos-sdk/x/ibc/core/02-client" "github.com/cosmos/cosmos-sdk/x/ibc/core/02-client/types" "github.com/cosmos/cosmos-sdk/x/ibc/core/exported" + localhosttypes "github.com/cosmos/cosmos-sdk/x/ibc/light-clients/09-localhost/types" ibctesting "github.com/cosmos/cosmos-sdk/x/ibc/testing" ) @@ -25,6 +26,13 @@ func (suite *ClientTestSuite) SetupTest() { suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(0)) suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(1)) + + // set localhost client + version := types.ParseChainID(suite.chainA.GetContext().ChainID()) + localHostClient := localhosttypes.NewClientState( + suite.chainA.GetContext().ChainID(), types.NewHeight(version, uint64(suite.chainA.GetContext().BlockHeight())), + ) + suite.chainA.App.IBCKeeper.ClientKeeper.SetClientState(suite.chainA.GetContext(), exported.Localhost, localHostClient) } func TestClientTestSuite(t *testing.T) { diff --git a/x/ibc/core/02-client/keeper/client.go b/x/ibc/core/02-client/keeper/client.go index 463df9ef46..49e9c62c88 100644 --- a/x/ibc/core/02-client/keeper/client.go +++ b/x/ibc/core/02-client/keeper/client.go @@ -22,6 +22,7 @@ func (k Keeper) CreateClient( return sdkerrors.Wrapf(types.ErrClientExists, "cannot create client with ID %s", clientID) } + // check if consensus state is nil in case the created client is Localhost if consensusState != nil { k.SetClientConsensusState(ctx, clientID, clientState.GetLatestHeight(), consensusState) } @@ -52,20 +53,15 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H return sdkerrors.Wrapf(types.ErrClientFrozen, "cannot update client with ID %s", clientID) } - var ( - consensusState exported.ConsensusState - consensusHeight exported.Height - err error - ) - - clientState, consensusState, err = clientState.CheckHeaderAndUpdateState(ctx, k.cdc, k.ClientStore(ctx, clientID), header) - + clientState, consensusState, err := clientState.CheckHeaderAndUpdateState(ctx, k.cdc, k.ClientStore(ctx, clientID), header) if err != nil { return sdkerrors.Wrapf(err, "cannot update client with ID %s", clientID) } k.SetClientState(ctx, clientID, clientState) + var consensusHeight exported.Height + // we don't set consensus state for localhost client if header != nil && clientID != exported.Localhost { k.SetClientConsensusState(ctx, clientID, header.GetHeight(), consensusState) @@ -116,7 +112,7 @@ func (k Keeper) UpgradeClient(ctx sdk.Context, clientID string, upgradedClient e err := clientState.VerifyUpgrade(ctx, k.cdc, k.ClientStore(ctx, clientID), upgradedClient, upgradeHeight, proofUpgrade) if err != nil { - return sdkerrors.Wrapf(err, "cannot upgrade client with ID: %s", clientID) + return sdkerrors.Wrapf(err, "cannot upgrade client with ID %s", clientID) } k.SetClientState(ctx, clientID, upgradedClient) @@ -155,6 +151,10 @@ func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, misbehaviour ex return sdkerrors.Wrapf(types.ErrClientNotFound, "cannot check misbehaviour for client with ID %s", misbehaviour.GetClientID()) } + if clientState.IsFrozen() && clientState.GetFrozenHeight().LTE(misbehaviour.GetHeight()) { + return sdkerrors.Wrapf(types.ErrInvalidMisbehaviour, "client is already frozen at height ≤ misbehaviour height (%s ≤ %s)", clientState.GetFrozenHeight(), misbehaviour.GetHeight()) + } + clientState, err := clientState.CheckMisbehaviourAndUpdateState(ctx, k.cdc, k.ClientStore(ctx, misbehaviour.GetClientID()), misbehaviour) if err != nil { return err diff --git a/x/ibc/core/02-client/keeper/client_test.go b/x/ibc/core/02-client/keeper/client_test.go index d0e8a06254..31cf5d80fe 100644 --- a/x/ibc/core/02-client/keeper/client_test.go +++ b/x/ibc/core/02-client/keeper/client_test.go @@ -222,14 +222,14 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() { } func (suite *KeeperTestSuite) TestUpdateClientLocalhost() { - var localhostClient exported.ClientState = localhosttypes.NewClientState(suite.header.Header.GetChainID(), types.NewHeight(0, uint64(suite.ctx.BlockHeight()))) + version := types.ParseChainID(suite.chainA.ChainID) + var localhostClient exported.ClientState = localhosttypes.NewClientState(suite.chainA.ChainID, types.NewHeight(version, uint64(suite.chainA.GetContext().BlockHeight()))) - suite.ctx = suite.ctx.WithBlockHeight(suite.ctx.BlockHeight() + 1) + ctx := suite.chainA.GetContext().WithBlockHeight(suite.chainA.GetContext().BlockHeight() + 1) + err := suite.chainA.App.IBCKeeper.ClientKeeper.UpdateClient(ctx, exported.Localhost, nil) + suite.Require().NoError(err) - err := suite.keeper.UpdateClient(suite.ctx, exported.Localhost, nil) - suite.Require().NoError(err, err) - - clientState, found := suite.keeper.GetClientState(suite.ctx, exported.Localhost) + clientState, found := suite.chainA.App.IBCKeeper.ClientKeeper.GetClientState(ctx, exported.Localhost) suite.Require().True(found) suite.Require().Equal(localhostClient.GetLatestHeight().(types.Height).Increment(), clientState.GetLatestHeight()) } diff --git a/x/ibc/core/02-client/keeper/keeper_test.go b/x/ibc/core/02-client/keeper/keeper_test.go index 9679353422..11db64a4d6 100644 --- a/x/ibc/core/02-client/keeper/keeper_test.go +++ b/x/ibc/core/02-client/keeper/keeper_test.go @@ -113,6 +113,13 @@ func (suite *KeeperTestSuite) SetupTest() { app.StakingKeeper.SetHistoricalInfo(suite.ctx, int64(i), &hi) } + // add localhost client + version := types.ParseChainID(suite.chainA.ChainID) + localHostClient := localhosttypes.NewClientState( + suite.chainA.ChainID, types.NewHeight(version, uint64(suite.chainA.GetContext().BlockHeight())), + ) + suite.chainA.App.IBCKeeper.ClientKeeper.SetClientState(suite.chainA.GetContext(), exported.Localhost, localHostClient) + queryHelper := baseapp.NewQueryServerTestHelper(suite.ctx, app.InterfaceRegistry()) types.RegisterQueryServer(queryHelper, app.IBCKeeper.ClientKeeper) suite.queryClient = types.NewQueryClient(queryHelper) @@ -245,15 +252,15 @@ func (suite KeeperTestSuite) TestGetAllClients() { } for i := range expClients { - suite.keeper.SetClientState(suite.ctx, clientIDs[i], expClients[i]) + suite.chainA.App.IBCKeeper.ClientKeeper.SetClientState(suite.chainA.GetContext(), clientIDs[i], expClients[i]) } // add localhost client - localHostClient, found := suite.keeper.GetClientState(suite.ctx, exported.Localhost) + localHostClient, found := suite.chainA.App.IBCKeeper.ClientKeeper.GetClientState(suite.chainA.GetContext(), exported.Localhost) suite.Require().True(found) expClients = append(expClients, localHostClient) - clients := suite.keeper.GetAllClients(suite.ctx) + clients := suite.chainA.App.IBCKeeper.ClientKeeper.GetAllClients(suite.chainA.GetContext()) suite.Require().Len(clients, len(expClients)) suite.Require().Equal(expClients, clients) } @@ -271,16 +278,16 @@ func (suite KeeperTestSuite) TestGetAllGenesisClients() { expGenClients := make([]types.IdentifiedClientState, len(expClients)) for i := range expClients { - suite.keeper.SetClientState(suite.ctx, clientIDs[i], expClients[i]) + suite.chainA.App.IBCKeeper.ClientKeeper.SetClientState(suite.chainA.GetContext(), clientIDs[i], expClients[i]) expGenClients[i] = types.NewIdentifiedClientState(clientIDs[i], expClients[i]) } // add localhost client - localHostClient, found := suite.keeper.GetClientState(suite.ctx, exported.Localhost) + localHostClient, found := suite.chainA.App.IBCKeeper.ClientKeeper.GetClientState(suite.chainA.GetContext(), exported.Localhost) suite.Require().True(found) expGenClients = append(expGenClients, types.NewIdentifiedClientState(exported.Localhost, localHostClient)) - genClients := suite.keeper.GetAllGenesisClients(suite.ctx) + genClients := suite.chainA.App.IBCKeeper.ClientKeeper.GetAllGenesisClients(suite.chainA.GetContext()) suite.Require().Equal(expGenClients, genClients) } diff --git a/x/ibc/core/02-client/types/genesis.go b/x/ibc/core/02-client/types/genesis.go index 77efae936e..c9be22f821 100644 --- a/x/ibc/core/02-client/types/genesis.go +++ b/x/ibc/core/02-client/types/genesis.go @@ -80,7 +80,7 @@ func DefaultGenesisState() GenesisState { return GenesisState{ Clients: []IdentifiedClientState{}, ClientsConsensus: ClientsConsensusStates{}, - CreateLocalhost: true, + CreateLocalhost: false, } } diff --git a/x/ibc/core/02-client/types/proposal.go b/x/ibc/core/02-client/types/proposal.go index b91fd1d06c..edbb4a8063 100644 --- a/x/ibc/core/02-client/types/proposal.go +++ b/x/ibc/core/02-client/types/proposal.go @@ -2,6 +2,7 @@ package types import ( govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" + host "github.com/cosmos/cosmos-sdk/x/ibc/core/24-host" "github.com/cosmos/cosmos-sdk/x/ibc/core/exported" ) @@ -33,7 +34,7 @@ func (cup *ClientUpdateProposal) GetTitle() string { return cup.Title } // GetDescription returns the description of a client update proposal. func (cup *ClientUpdateProposal) GetDescription() string { return cup.Description } -// GetDescription returns the routing key of a client update proposal. +// ProposalRoute returns the routing key of a client update proposal. func (cup *ClientUpdateProposal) ProposalRoute() string { return RouterKey } // ProposalType returns the type of a client update proposal. @@ -46,6 +47,10 @@ func (cup *ClientUpdateProposal) ValidateBasic() error { return err } + if err := host.ClientIdentifierValidator(cup.ClientId); err != nil { + return err + } + header, err := UnpackHeader(cup.Header) if err != nil { return err diff --git a/x/ibc/core/24-host/keys.go b/x/ibc/core/24-host/keys.go index 50249552c0..c2f344028e 100644 --- a/x/ibc/core/24-host/keys.go +++ b/x/ibc/core/24-host/keys.go @@ -39,11 +39,6 @@ const ( KeyPacketReceiptPrefix = "receipts" ) -// KeyPrefixBytes return the key prefix bytes from a URL string format -func KeyPrefixBytes(prefix int) []byte { - return []byte(fmt.Sprintf("%d/", prefix)) -} - // FullClientPath returns the full path of a specific client path in the format: // "clients/{clientID}/{path}" as a string. func FullClientPath(clientID string, path string) string { @@ -87,7 +82,7 @@ func KeyConsensusState(height exported.Height) []byte { // ClientConnectionsPath defines a reverse mapping from clients to a set of connections func ClientConnectionsPath(clientID string) string { - return fmt.Sprintf("clients/%s/connections", clientID) + return fmt.Sprintf("%s/%s/connections", KeyClientStorePrefix, clientID) } // ConnectionPath defines the path under which connection paths are stored @@ -95,7 +90,7 @@ func ConnectionPath(connectionID string) string { return fmt.Sprintf("%s/%s", KeyConnectionPrefix, connectionID) } -// KeyClientConnections returns the store key for the connectios of a given client +// KeyClientConnections returns the store key for the connections of a given client func KeyClientConnections(clientID string) []byte { return []byte(ClientConnectionsPath(clientID)) } @@ -108,11 +103,6 @@ func KeyConnection(connectionID string) []byte { // ICS04 // The following paths are the keys to the store as defined in https://github.com/cosmos/ics/tree/master/spec/ics-004-channel-and-packet-semantics#store-paths -// GetChannelPortsKeysPrefix returns the prefix bytes for ICS04 and ICS05 iterators -func GetChannelPortsKeysPrefix(prefix int) []byte { - return []byte(fmt.Sprintf("%d/ports/", prefix)) -} - // ChannelPath defines the path under which channels are stored func ChannelPath(portID, channelID string) string { return fmt.Sprintf("%s/", KeyChannelPrefix) + channelPath(portID, channelID) diff --git a/x/ibc/core/24-host/parse.go b/x/ibc/core/24-host/parse.go index 31500c3463..7200c4ff00 100644 --- a/x/ibc/core/24-host/parse.go +++ b/x/ibc/core/24-host/parse.go @@ -6,16 +6,6 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) -// RemovePath is an util function to remove a path from a set. -func RemovePath(paths []string, path string) ([]string, bool) { - for i, p := range paths { - if p == path { - return append(paths[:i], paths[i+1:]...), true - } - } - return paths, false -} - // ParseConnectionPath returns the connection ID from a full path. It returns // an error if the provided path is invalid. func ParseConnectionPath(path string) (string, error) { diff --git a/x/ibc/core/24-host/validate.go b/x/ibc/core/24-host/validate.go index 0a8db56067..6d512fbe0c 100644 --- a/x/ibc/core/24-host/validate.go +++ b/x/ibc/core/24-host/validate.go @@ -56,29 +56,29 @@ func defaultIdentifierValidator(id string, min, max int) error { //nolint:unpara } // ClientIdentifierValidator is the default validator function for Client identifiers. -// A valid Identifier must be between 9-20 characters and only contain lowercase -// alphabetic characters, +// A valid Identifier must be between 9-64 characters and only contain lowercase +// alphabetic characters. func ClientIdentifierValidator(id string) error { return defaultIdentifierValidator(id, 9, DefaultMaxCharacterLength) } // ConnectionIdentifierValidator is the default validator function for Connection identifiers. -// A valid Identifier must be between 10-20 characters and only contain lowercase -// alphabetic characters, +// A valid Identifier must be between 10-64 characters and only contain lowercase +// alphabetic characters. func ConnectionIdentifierValidator(id string) error { return defaultIdentifierValidator(id, 10, DefaultMaxCharacterLength) } // ChannelIdentifierValidator is the default validator function for Channel identifiers. -// A valid Identifier must be between 10-20 characters and only contain lowercase -// alphabetic characters, +// A valid Identifier must be between 10-64 characters and only contain lowercase +// alphabetic characters. func ChannelIdentifierValidator(id string) error { return defaultIdentifierValidator(id, 10, DefaultMaxCharacterLength) } // PortIdentifierValidator is the default validator function for Port identifiers. -// A valid Identifier must be between 2-20 characters and only contain lowercase -// alphabetic characters, +// A valid Identifier must be between 2-64 characters and only contain lowercase +// alphabetic characters. func PortIdentifierValidator(id string) error { return defaultIdentifierValidator(id, 2, DefaultMaxCharacterLength) } diff --git a/x/ibc/core/handler.go b/x/ibc/core/handler.go index d2ef7f7b61..c8e4dfc898 100644 --- a/x/ibc/core/handler.go +++ b/x/ibc/core/handler.go @@ -24,6 +24,10 @@ func NewHandler(k keeper.Keeper) sdk.Handler { res, err := k.UpdateClient(sdk.WrapSDKContext(ctx), msg) return sdk.WrapServiceResult(ctx, res, err) + case *clienttypes.MsgUpgradeClient: + res, err := k.UpgradeClient(sdk.WrapSDKContext(ctx), msg) + return sdk.WrapServiceResult(ctx, res, err) + case *clienttypes.MsgSubmitMisbehaviour: res, err := k.SubmitMisbehaviour(sdk.WrapSDKContext(ctx), msg) return sdk.WrapServiceResult(ctx, res, err) diff --git a/x/ibc/core/keeper/msg_server.go b/x/ibc/core/keeper/msg_server.go index d448aca8b6..d26ea627b0 100644 --- a/x/ibc/core/keeper/msg_server.go +++ b/x/ibc/core/keeper/msg_server.go @@ -84,10 +84,6 @@ func (k Keeper) UpgradeClient(goCtx context.Context, msg *clienttypes.MsgUpgrade return nil, err } - if err := upgradedClient.Validate(); err != nil { - return nil, err - } - if err = k.ClientKeeper.UpgradeClient(ctx, msg.ClientId, upgradedClient, msg.UpgradeHeight, msg.ProofUpgrade); err != nil { return nil, err } diff --git a/x/ibc/core/keeper/msg_server_test.go b/x/ibc/core/keeper/msg_server_test.go index 2f3910fe71..0e1f6aab77 100644 --- a/x/ibc/core/keeper/msg_server_test.go +++ b/x/ibc/core/keeper/msg_server_test.go @@ -688,34 +688,6 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { }, expPass: false, }, - { - name: "invalid clientstate", - setup: func() { - - upgradedClient = ibctmtypes.NewClientState("", ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod+ibctesting.TrustingPeriod, ibctesting.MaxClockDrift, newClientHeight, ibctesting.DefaultConsensusParams, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) - - // upgrade Height is at next block - upgradeHeight = clienttypes.NewHeight(0, uint64(suite.chainB.GetContext().BlockHeight()+1)) - - // zero custom fields and store in upgrade store - suite.chainB.App.UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(upgradeHeight.GetVersionHeight()), upgradedClient) - - // commit upgrade store changes and update clients - - suite.coordinator.CommitBlock(suite.chainB) - err := suite.coordinator.UpdateClient(suite.chainA, suite.chainB, clientA, ibctesting.Tendermint) - suite.Require().NoError(err) - - cs, found := suite.chainA.App.IBCKeeper.ClientKeeper.GetClientState(suite.chainA.GetContext(), clientA) - suite.Require().True(found) - - proofUpgrade, _ := suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(upgradeHeight.GetVersionHeight())), cs.GetLatestHeight().GetVersionHeight()) - - msg, err = clienttypes.NewMsgUpgradeClient(clientA, upgradedClient, upgradeHeight, proofUpgrade, suite.chainA.SenderAccount.GetAddress()) - suite.Require().NoError(err) - }, - expPass: false, - }, { name: "VerifyUpgrade fails", setup: func() {