IBC alpha general cleanup (#5291)

* remove prefix from keeper; update client queries; address ICS02 comments from @cwgoes

* add proof for root query

* golangci

* remove hardcoded bind port logic

* space

* WIP: register errors

* register errors; make format

* use new instead of register; unescape path

* golangci

client/context/context.go

@@ -99,7 +99,7 @@ func NewCLIContextWithFrom(from string) CLIContext {
 	return ctx.WithVerifier(verifier)
 }
 
-// NewCLIContextIBC takes additional arguements
+// NewCLIContextIBC takes additional arguments
 func NewCLIContextIBC(from string, chainID string, nodeURI string) CLIContext {
 	var rpc rpcclient.Client
 

client/context/query.go

@@ -129,19 +129,18 @@ func (ctx CLIContext) GetFromName() string {
 	return ctx.FromName
 }
 
-func (ctx CLIContext) queryABCI(req abci.RequestQuery) (resp abci.ResponseQuery, err error) {
-
+func (ctx CLIContext) queryABCI(req abci.RequestQuery) (abci.ResponseQuery, error) {
 	node, err := ctx.GetNode()
 	if err != nil {
-		return resp, err
+		return abci.ResponseQuery{}, err
 	}
 
 	// When a client did not provide a query height, manually query for it so it can
 	// be injected downstream into responses.
 	if ctx.Height == 0 {
 		status, err := node.Status()
 		if err != nil {
-			return resp, err
+			return abci.ResponseQuery{}, err
 		}
 		ctx = ctx.WithHeight(status.SyncInfo.LatestBlockHeight)
 	}
@@ -153,26 +152,25 @@ func (ctx CLIContext) queryABCI(req abci.RequestQuery) (resp abci.ResponseQuery,
 
 	result, err := node.ABCIQueryWithOptions(req.Path, req.Data, opts)
 	if err != nil {
-		return
+		return abci.ResponseQuery{}, err
 	}
 
-	resp = result.Response
-	if !resp.IsOK() {
-		err = errors.New(resp.Log)
-		return
+	if !result.Response.IsOK() {
+		err = errors.New(result.Response.Log)
+		return abci.ResponseQuery{}, err
 	}
 
 	// data from trusted node or subspace query doesn't need verification
 	if ctx.TrustNode || !isQueryStoreWithProof(req.Path) {
-		return resp, nil
+		return result.Response, nil
 	}
 
-	err = ctx.verifyProof(req.Path, resp)
+	err = ctx.verifyProof(req.Path, result.Response)
 	if err != nil {
-		return
+		return abci.ResponseQuery{}, err
 	}
 
-	return
+	return result.Response, nil
 }
 
 // query performs a query to a Tendermint node with the provided store name

x/ibc/02-client/client/cli/query.go

@@ -7,15 +7,17 @@ import (
 	"strings"
 
 	"github.com/spf13/cobra"
+	"github.com/spf13/viper"
 
+	abci "github.com/tendermint/tendermint/abci/types"
 	tmtypes "github.com/tendermint/tendermint/types"
 
 	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/client/context"
+	"github.com/cosmos/cosmos-sdk/client/flags"
 	"github.com/cosmos/cosmos-sdk/codec"
 	"github.com/cosmos/cosmos-sdk/version"
 	"github.com/cosmos/cosmos-sdk/x/ibc/02-client/client/utils"
-	"github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported"
 	"github.com/cosmos/cosmos-sdk/x/ibc/02-client/types"
 	"github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/tendermint"
 	commitment "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment"
@@ -44,7 +46,7 @@ func GetQueryCmd(queryRoute string, cdc *codec.Codec) *cobra.Command {
 // GetCmdQueryClientState defines the command to query the state of a client with
 // a given id as defined in https://github.com/cosmos/ics/tree/master/spec/ics-002-client-semantics#query
 func GetCmdQueryClientState(queryRoute string, cdc *codec.Codec) *cobra.Command {
-	return &cobra.Command{
+	cmd := &cobra.Command{
 		Use:   "state [client-id]",
 		Short: "Query a client state",
 		Long: strings.TrimSpace(
@@ -67,24 +69,33 @@ $ %s query ibc client state [client-id]
 				return err
 			}
 
-			res, _, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/%s", queryRoute, types.QueryClientState), bz)
+			req := abci.RequestQuery{
+				Path:  fmt.Sprintf("custom/%s/%s", queryRoute, types.QueryClientState),
+				Data:  bz,
+				Prove: viper.GetBool(flags.FlagProve),
+			}
+
+			res, err := cliCtx.QueryABCI(req)
 			if err != nil {
 				return err
 			}
 
 			var clientState types.State
-			if err := cdc.UnmarshalJSON(res, &clientState); err != nil {
+			if err := cdc.UnmarshalJSON(res.Value, &clientState); err != nil {
 				return err
 			}
 
-			return cliCtx.PrintOutput(clientState)
+			clientStateRes := types.NewClientStateResponse(clientID, clientState, res.Proof, res.Height)
+			return cliCtx.PrintOutput(clientStateRes)
 		},
 	}
+	cmd.Flags().Bool(flags.FlagProve, true, "show proofs for the query results")
+	return cmd
 }
 
-// GetCmdQueryRoot defines the command to query
+// GetCmdQueryRoot defines the command to query a verified commitment root
 func GetCmdQueryRoot(queryRoute string, cdc *codec.Codec) *cobra.Command {
-	return &cobra.Command{
+	cmd := &cobra.Command{
 		Use:   "root [client-id] [height]",
 		Short: "Query a verified commitment root",
 		Long: strings.TrimSpace(
@@ -112,25 +123,34 @@ $ %s query ibc client root [client-id] [height]
 				return err
 			}
 
-			res, _, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/%s", queryRoute, types.QueryVerifiedRoot), bz)
+			req := abci.RequestQuery{
+				Path:  fmt.Sprintf("custom/%s/%s", queryRoute, types.QueryVerifiedRoot),
+				Data:  bz,
+				Prove: viper.GetBool(flags.FlagProve),
+			}
+
+			res, err := cliCtx.QueryABCI(req)
 			if err != nil {
 				return err
 			}
 
-			var root commitment.RootI
-			if err := cdc.UnmarshalJSON(res, &root); err != nil {
+			var root commitment.Root
+			if err := cdc.UnmarshalJSON(res.Value, &root); err != nil {
 				return err
 			}
 
-			return cliCtx.PrintOutput(root)
+			rootRes := types.NewRootResponse(clientID, height, root, res.Proof, res.Height)
+			return cliCtx.PrintOutput(rootRes)
 		},
 	}
+	cmd.Flags().Bool(flags.FlagProve, true, "show proofs for the query results")
+	return cmd
 }
 
 // GetCmdQueryConsensusState defines the command to query the consensus state of
 // the chain as defined in https://github.com/cosmos/ics/tree/master/spec/ics-002-client-semantics#query
 func GetCmdQueryConsensusState(queryRoute string, cdc *codec.Codec) *cobra.Command {
-	return &cobra.Command{
+	cmd := &cobra.Command{
 		Use:   "consensus-state [client-id]",
 		Short: "Query the latest consensus state of the client",
 		Long: strings.TrimSpace(
@@ -148,24 +168,16 @@ $ %s query ibc client consensus-state [client-id]
 				return errors.New("client ID can't be blank")
 			}
 
-			bz, err := cdc.MarshalJSON(types.NewQueryClientStateParams(clientID))
+			csRes, err := utils.QueryConsensusStateProof(cliCtx, cdc, queryRoute, clientID)
 			if err != nil {
 				return err
 			}
 
-			res, _, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/%s", queryRoute, types.QueryConsensusState), bz)
-			if err != nil {
-				return err
-			}
-
-			var consensusState exported.ConsensusState
-			if err := cdc.UnmarshalJSON(res, &consensusState); err != nil {
-				return err
-			}
-
-			return cliCtx.PrintOutput(consensusState)
+			return cliCtx.PrintOutput(csRes)
 		},
 	}
+	cmd.Flags().Bool(flags.FlagProve, true, "show proofs for the query results")
+	return cmd
 }
 
 // GetCmdQueryHeader defines the command to query the latest header on the chain

x/ibc/02-client/client/utils/utils.go

@@ -3,23 +3,36 @@ package utils
 import (
 	"fmt"
 
+	"github.com/spf13/viper"
+
 	abci "github.com/tendermint/tendermint/abci/types"
 	tmtypes "github.com/tendermint/tendermint/types"
 
 	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/client/context"
+	"github.com/cosmos/cosmos-sdk/client/flags"
+	"github.com/cosmos/cosmos-sdk/codec"
 	"github.com/cosmos/cosmos-sdk/x/ibc/02-client/types"
 	"github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/tendermint"
 )
 
 // QueryConsensusStateProof queries the store to get the consensus state and a
 // merkle proof.
-func QueryConsensusStateProof(cliCtx client.CLIContext, clientID string) (types.ConsensusStateResponse, error) {
+func QueryConsensusStateProof(
+	cliCtx client.CLIContext, cdc *codec.Codec,
+	queryRoute, clientID string,
+) (types.ConsensusStateResponse, error) {
 	var conStateRes types.ConsensusStateResponse
+
+	bz, err := cdc.MarshalJSON(types.NewQueryClientStateParams(clientID))
+	if err != nil {
+		return conStateRes, err
+	}
+
 	req := abci.RequestQuery{
-		Path:  "store/ibc/key",
-		Data:  []byte(fmt.Sprintf("clients/%s/consensusState", clientID)),
-		Prove: true,
+		Path:  fmt.Sprintf("custom/%s/%s", queryRoute, types.QueryClientState),
+		Data:  bz,
+		Prove: viper.GetBool(flags.FlagProve),
 	}
 
 	res, err := cliCtx.QueryABCI(req)
@@ -28,9 +41,10 @@ func QueryConsensusStateProof(cliCtx client.CLIContext, clientID string) (types.
 	}
 
 	var cs tendermint.ConsensusState
-	if err := cliCtx.Codec.UnmarshalBinaryLengthPrefixed(res.Value, &cs); err != nil {
+	if err := cliCtx.Codec.UnmarshalJSON(res.Value, &cs); err != nil {
 		return conStateRes, err
 	}
+
 	return types.NewConsensusStateResponse(clientID, cs, res.Proof, res.Height), nil
 }
 

x/ibc/02-client/doc.go

@@ -1,5 +1,5 @@
 /*
-Package client implements the ICS 02 - Client Semenatics specification
+Package client implements the ICS 02 - Client Semantics specification
 https://github.com/cosmos/ics/tree/master/spec/ics-002-client-semantics.  This
 concrete implementations defines types and method to store and update light
 clients which tracks on other chain's state.

x/ibc/02-client/exported/exported.go

@@ -6,17 +6,10 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
-	commitment "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment"
 	cmn "github.com/tendermint/tendermint/libs/common"
-)
 
-// Blockchain is consensus algorithm which generates valid Headers. It generates
-// a unique list of headers starting from a genesis ConsensusState with arbitrary messages.
-// This interface is implemented as defined in https://github.com/cosmos/ics/tree/master/spec/ics-002-client-semantics#blockchain.
-type Blockchain interface {
-	Genesis() ConsensusState // Consensus state defined in the genesis
-	Consensus() Header       // Header generating function
-}
+	commitment "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment"
+)
 
 // ConsensusState is the state of the consensus process
 type ConsensusState interface {

x/ibc/02-client/keeper/client.go

@@ -60,15 +60,6 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H
 		return sdkerrors.Wrap(errors.ErrConsensusStateNotFound(k.codespace), "cannot update client")
 	}
 
-	if header.GetHeight() < consensusState.GetHeight() {
-		return sdkerrors.Wrap(
-			sdk.ErrInvalidSequence(
-				fmt.Sprintf("header height < consensus height (%d < %d)", header.GetHeight(), consensusState.GetHeight()),
-			),
-			"cannot update client",
-		)
-	}
-
 	consensusState, err := consensusState.CheckValidityAndUpdateState(header)
 	if err != nil {
 		return sdkerrors.Wrap(err, "cannot update client")

x/ibc/02-client/keeper/keeper.go

@@ -23,7 +23,6 @@ type Keeper struct {
 	storeKey  sdk.StoreKey
 	cdc       *codec.Codec
 	codespace sdk.CodespaceType
-	prefix    []byte // prefix bytes for accessing the store
 }
 
 // NewKeeper creates a new NewKeeper instance
@@ -32,8 +31,6 @@ func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, codespace sdk.CodespaceType)
 		storeKey:  key,
 		cdc:       cdc,
 		codespace: sdk.CodespaceType(fmt.Sprintf("%s/%s", codespace, errors.DefaultCodespace)), // "ibc/client",
-		prefix:    []byte{},
-		// prefix:    []byte(types.SubModuleName + "/"),                                          // "client/"
 	}
 }
 
@@ -44,7 +41,7 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger {
 
 // GetClientState gets a particular client from the store
 func (k Keeper) GetClientState(ctx sdk.Context, clientID string) (types.State, bool) {
-	store := prefix.NewStore(ctx.KVStore(k.storeKey), k.prefix)
+	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixClient)
 	bz := store.Get(types.KeyClientState(clientID))
 	if bz == nil {
 		return types.State{}, false
@@ -57,14 +54,14 @@ func (k Keeper) GetClientState(ctx sdk.Context, clientID string) (types.State, b
 
 // SetClientState sets a particular Client to the store
 func (k Keeper) SetClientState(ctx sdk.Context, clientState types.State) {
-	store := prefix.NewStore(ctx.KVStore(k.storeKey), k.prefix)
+	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixClient)
 	bz := k.cdc.MustMarshalBinaryLengthPrefixed(clientState)
-	store.Set(types.KeyClientState(clientState.ID()), bz)
+	store.Set(types.KeyClientState(clientState.ID), bz)
 }
 
 // GetClientType gets the consensus type for a specific client
 func (k Keeper) GetClientType(ctx sdk.Context, clientID string) (exported.ClientType, bool) {
-	store := prefix.NewStore(ctx.KVStore(k.storeKey), k.prefix)
+	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixClient)
 	bz := store.Get(types.KeyClientType(clientID))
 	if bz == nil {
 		return 0, false
@@ -75,13 +72,13 @@ func (k Keeper) GetClientType(ctx sdk.Context, clientID string) (exported.Client
 
 // SetClientType sets the specific client consensus type to the provable store
 func (k Keeper) SetClientType(ctx sdk.Context, clientID string, clientType exported.ClientType) {
-	store := prefix.NewStore(ctx.KVStore(k.storeKey), k.prefix)
+	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixClient)
 	store.Set(types.KeyClientType(clientID), []byte{byte(clientType)})
 }
 
 // GetConsensusState creates a new client state and populates it with a given consensus state
 func (k Keeper) GetConsensusState(ctx sdk.Context, clientID string) (exported.ConsensusState, bool) {
-	store := prefix.NewStore(ctx.KVStore(k.storeKey), k.prefix)
+	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixClient)
 	bz := store.Get(types.KeyConsensusState(clientID))
 	if bz == nil {
 		return nil, false
@@ -94,15 +91,15 @@ func (k Keeper) GetConsensusState(ctx sdk.Context, clientID string) (exported.Co
 
 // SetConsensusState sets a ConsensusState to a particular client
 func (k Keeper) SetConsensusState(ctx sdk.Context, clientID string, consensusState exported.ConsensusState) {
-	store := prefix.NewStore(ctx.KVStore(k.storeKey), k.prefix)
+	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixClient)
 	bz := k.cdc.MustMarshalBinaryLengthPrefixed(consensusState)
 	store.Set(types.KeyConsensusState(clientID), bz)
 }
 
 // GetVerifiedRoot gets a verified commitment Root from a particular height to
 // a client
 func (k Keeper) GetVerifiedRoot(ctx sdk.Context, clientID string, height uint64) (commitment.RootI, bool) {
-	store := prefix.NewStore(ctx.KVStore(k.storeKey), k.prefix)
+	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixClient)
 
 	bz := store.Get(types.KeyRoot(clientID, height))
 	if bz == nil {
@@ -117,7 +114,7 @@ func (k Keeper) GetVerifiedRoot(ctx sdk.Context, clientID string, height uint64)
 // SetVerifiedRoot sets a verified commitment Root from a particular height to
 // a client
 func (k Keeper) SetVerifiedRoot(ctx sdk.Context, clientID string, height uint64, root commitment.RootI) {
-	store := prefix.NewStore(ctx.KVStore(k.storeKey), k.prefix)
+	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixClient)
 	bz := k.cdc.MustMarshalBinaryLengthPrefixed(root)
 	store.Set(types.KeyRoot(clientID, height), bz)
 }
@@ -151,7 +148,7 @@ func (k Keeper) checkMisbehaviour(ctx sdk.Context, evidence exported.Evidence) e
 // freeze updates the state of the client in the event of a misbehaviour
 func (k Keeper) freeze(ctx sdk.Context, clientState types.State) (types.State, error) {
 	if clientState.Frozen {
-		return types.State{}, sdkerrors.Wrap(errors.ErrClientFrozen(k.codespace, clientState.ID()), "already frozen")
+		return types.State{}, sdkerrors.Wrap(errors.ErrClientFrozen(k.codespace, clientState.ID), "already frozen")
 	}
 
 	clientState.Frozen = true