Skip to content

Commit

Permalink
Update x/ibc error handling (#5462)
Browse files Browse the repository at this point in the history
* Merge PR #5428: Add mod, exponentiation for uint

* Modified examples in distribution module (#5441)

* Merge PR #5442: Remove of the client/alias.go

* Merge PR #5445: Mock rpcclient in tests for votes pagination

* Merge PR #5435: Added iterator that allows to read only requested values

* Merge PR #5427: Remove code duplication in x/auth/client/cli

* Merge PR #5421: Refactor Error Handling

* update x/ibc error handling

* update ICS24 and ICS02 errors

* ICS03, ICS23 and common errors

* updates from master and errors from ICS04

* build

* fix ics20 tests

* fix tests

* golangcibot fixes

Co-authored-by: Kevin Davis <karzak@users.noreply.github.com>
Co-authored-by: kaustubhkapatral <54210167+kaustubhkapatral@users.noreply.github.com>
Co-authored-by: Ferenc Fabian <qwer.kocka@gmail.com>
Co-authored-by: Dmitry Shulyak <yashulyak@gmail.com>
Co-authored-by: Alessio Treglia <quadrispro@ubuntu.com>
Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
  • Loading branch information
7 people authored and jackzampolin committed Jan 15, 2020
1 parent 045b021 commit 1e10e1f
Show file tree
Hide file tree
Showing 67 changed files with 1,073 additions and 1,265 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ if the provided arguments are invalid.
the `client/keys` add command.
* `SupportedAlgos` and `SupportedAlgosLedger` functions return a slice of `SigningAlgo`s that are
supported by the keybase and the ledger integration respectively.
* The option introduced in this PR is `WithKeygenFunc` which allows a custom bytes to key implementation to be defined when keys are created.
* (simapp) [\#5419](https://github.com/cosmos/cosmos-sdk/pull/5419) simapp/helpers.GenTx() now accepts a gas argument.
* (baseapp) [\#5455](https://github.com/cosmos/cosmos-sdk/issues/5455) An `sdk.Context` is passed into the `router.Route()`
function.
Expand Down Expand Up @@ -261,8 +262,11 @@ to detail this new feature and how state transitions occur.
* (docs/interfaces/) Add documentation on building interfaces for the Cosmos SDK.
* Redesigned user interface that features new dynamically generated sidebar, build-time code embedding from GitHub, new homepage as well as many other improvements.
* (types) [\#5428](https://github.com/cosmos/cosmos-sdk/pull/5428) Add `Mod` (modulo) method and `RelativePow` (exponentation) function for `Uint`.
<<<<<<< HEAD
* (modules) [\#5506](https://github.com/cosmos/cosmos-sdk/pull/5506) Remove redundancy in `x/distribution`s use of parameters. There
now exists a single `Params` type with a getter and setter along with a getter for each individual parameter.
=======
>>>>>>> Update x/ibc error handling (#5462)
### Bug Fixes
Expand Down Expand Up @@ -2866,3 +2870,5 @@ BUG FIXES:
[v0.37.1]: https://github.com/cosmos/cosmos-sdk/releases/tag/v0.37.1
[v0.37.0]: https://github.com/cosmos/cosmos-sdk/releases/tag/v0.37.0
[v0.36.0]: https://github.com/cosmos/cosmos-sdk/releases/tag/v0.36.0
2 changes: 1 addition & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func NewSimApp(
staking.NewMultiStakingHooks(app.DistrKeeper.Hooks(), app.SlashingKeeper.Hooks()),
)

app.IBCKeeper = ibc.NewKeeper(app.cdc, keys[ibc.StoreKey], ibc.DefaultCodespace, app.BankKeeper, app.SupplyKeeper)
app.IBCKeeper = ibc.NewKeeper(app.cdc, keys[ibc.StoreKey], app.BankKeeper, app.SupplyKeeper)

// NOTE: Any module instantiated in the module manager that is later modified
// must be passed by reference here.
Expand Down
9 changes: 0 additions & 9 deletions types/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,6 @@ type TxResponse struct {
Timestamp string `json:"timestamp,omitempty"`
}

func (res TxResponse) IsOK() bool {
for _, lg := range res.Logs {
if !lg.Success {
return false
}
}
return true
}

// NewResponseResultTx returns a TxResponse given a ResultTx from tendermint
func NewResponseResultTx(res *ctypes.ResultTx, tx Tx, timestamp string) TxResponse {
if res == nil {
Expand Down
29 changes: 9 additions & 20 deletions x/ibc/02-client/alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,15 @@ import (
)

const (
DefaultCodespace = errors.DefaultCodespace
CodeClientExists = errors.CodeClientExists
CodeClientNotFound = errors.CodeClientNotFound
CodeClientFrozen = errors.CodeClientFrozen
CodeConsensusStateNotFound = errors.CodeConsensusStateNotFound
CodeInvalidConsensusState = errors.CodeInvalidConsensusState
CodeClientTypeNotFound = errors.CodeClientTypeNotFound
CodeInvalidClientType = errors.CodeInvalidClientType
CodeRootNotFound = errors.CodeRootNotFound
CodeInvalidHeader = errors.CodeInvalidHeader
CodeInvalidEvidence = errors.CodeInvalidEvidence
AttributeKeyClientID = types.AttributeKeyClientID
SubModuleName = types.SubModuleName
StoreKey = types.StoreKey
RouterKey = types.RouterKey
QuerierRoute = types.QuerierRoute
QueryAllClients = types.QueryAllClients
QueryClientState = types.QueryClientState
QueryConsensusState = types.QueryConsensusState
QueryVerifiedRoot = types.QueryVerifiedRoot
AttributeKeyClientID = types.AttributeKeyClientID
SubModuleName = types.SubModuleName
StoreKey = types.StoreKey
RouterKey = types.RouterKey
QuerierRoute = types.QuerierRoute
QueryAllClients = types.QueryAllClients
QueryClientState = types.QueryClientState
QueryConsensusState = types.QueryConsensusState
QueryVerifiedRoot = types.QueryVerifiedRoot
)

var (
Expand Down
6 changes: 3 additions & 3 deletions x/ibc/02-client/client/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package cli
import (
"github.com/spf13/cobra"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/codec"
)

Expand All @@ -16,7 +16,7 @@ func GetQueryCmd(queryRoute string, cdc *codec.Codec) *cobra.Command {
SuggestionsMinimumDistance: 2,
}

ics02ClientQueryCmd.AddCommand(client.GetCommands(
ics02ClientQueryCmd.AddCommand(flags.GetCommands(
GetCmdQueryClientStates(queryRoute, cdc),
GetCmdQueryClientState(queryRoute, cdc),
GetCmdQueryConsensusState(queryRoute, cdc),
Expand All @@ -37,7 +37,7 @@ func GetTxCmd(storeKey string, cdc *codec.Codec) *cobra.Command {
SuggestionsMinimumDistance: 2,
}

ics02ClientTxCmd.AddCommand(client.PostCommands(
ics02ClientTxCmd.AddCommand(flags.PostCommands(
GetCmdCreateClient(cdc),
GetCmdUpdateClient(cdc),
)...)
Expand Down
7 changes: 3 additions & 4 deletions x/ibc/02-client/client/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
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/x/ibc/02-client/types"
"github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/tendermint"
Expand Down Expand Up @@ -62,10 +61,10 @@ func QueryClientState(
return clientStateRes, nil
}

// QueryConsensusStateProof queries the store to get the consensus state and a
// merkle proof.
// QueryConsensusState queries the store to get the consensus state and a merkle
// proof.
func QueryConsensusState(
cliCtx client.CLIContext, clientID string, prove bool) (types.ConsensusStateResponse, error) {
cliCtx context.CLIContext, clientID string, prove bool) (types.ConsensusStateResponse, error) {
var conStateRes types.ConsensusStateResponse

req := abci.RequestQuery{
Expand Down
26 changes: 13 additions & 13 deletions x/ibc/02-client/handler.go
Original file line number Diff line number Diff line change
@@ -1,27 +1,24 @@
package client

import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/x/evidence"
evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported"
"github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported"
"github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/tendermint"
)

// HandleMsgCreateClient defines the sdk.Handler for MsgCreateClient
func HandleMsgCreateClient(ctx sdk.Context, k Keeper, msg MsgCreateClient) sdk.Result {
func HandleMsgCreateClient(ctx sdk.Context, k Keeper, msg MsgCreateClient) (*sdk.Result, error) {
clientType := exported.ClientTypeFromString(msg.ClientType)
if clientType == 0 {
return sdk.ResultFromError(
ErrInvalidClientType(DefaultCodespace, fmt.Sprintf("invalid client type '%s'", msg.ClientType)),
)
return nil, sdkerrors.Wrap(ErrInvalidClientType, msg.ClientType)
}

_, err := k.CreateClient(ctx, msg.ClientID, clientType, msg.ConsensusState)
if err != nil {
return sdk.ResultFromError(err)
return nil, err
}

ctx.EventManager().EmitEvents(sdk.Events{
Expand All @@ -36,14 +33,16 @@ func HandleMsgCreateClient(ctx sdk.Context, k Keeper, msg MsgCreateClient) sdk.R
),
})

return sdk.Result{Events: ctx.EventManager().Events()}
return &sdk.Result{
Events: ctx.EventManager().Events(),
}, nil
}

// HandleMsgUpdateClient defines the sdk.Handler for MsgUpdateClient
func HandleMsgUpdateClient(ctx sdk.Context, k Keeper, msg MsgUpdateClient) sdk.Result {
func HandleMsgUpdateClient(ctx sdk.Context, k Keeper, msg MsgUpdateClient) (*sdk.Result, error) {
err := k.UpdateClient(ctx, msg.ClientID, msg.Header)
if err != nil {
return sdk.ResultFromError(err)
return nil, err
}

ctx.EventManager().EmitEvents(sdk.Events{
Expand All @@ -58,7 +57,9 @@ func HandleMsgUpdateClient(ctx sdk.Context, k Keeper, msg MsgUpdateClient) sdk.R
),
})

return sdk.Result{Events: ctx.EventManager().Events()}
return &sdk.Result{
Events: ctx.EventManager().Events(),
}, nil
}

// HandlerClientMisbehaviour defines the Evidence module handler for submitting a
Expand All @@ -70,8 +71,7 @@ func HandlerClientMisbehaviour(k Keeper) evidence.Handler {
return k.CheckMisbehaviourAndUpdateState(ctx, evidence)

default:
errMsg := fmt.Sprintf("unrecognized IBC client evidence type: %T", e)
return sdk.ErrUnknownRequest(errMsg)
return sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "unrecognized IBC client evidence type: %T", e)
}
}
}
24 changes: 12 additions & 12 deletions x/ibc/02-client/keeper/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func (k Keeper) CreateClient(
) (types.State, error) {
_, found := k.GetClientState(ctx, clientID)
if found {
return types.State{}, errors.ErrClientExists(k.codespace, clientID)
return types.State{}, sdkerrors.Wrapf(errors.ErrClientExists, "cannot create client with ID %s", clientID)
}

_, found = k.GetClientType(ctx, clientID)
Expand All @@ -41,31 +41,31 @@ func (k Keeper) CreateClient(
func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.Header) error {
clientType, found := k.GetClientType(ctx, clientID)
if !found {
return sdkerrors.Wrap(errors.ErrClientTypeNotFound(k.codespace), "cannot update client")
return sdkerrors.Wrapf(errors.ErrClientTypeNotFound, "cannot update client with ID %s", clientID)
}

// check that the header consensus matches the client one
if header.ClientType() != clientType {
return sdkerrors.Wrap(errors.ErrInvalidConsensus(k.codespace), "cannot update client")
return sdkerrors.Wrapf(errors.ErrInvalidConsensus, "cannot update client with ID %s", clientID)
}

clientState, found := k.GetClientState(ctx, clientID)
if !found {
return sdkerrors.Wrap(errors.ErrClientNotFound(k.codespace, clientID), "cannot update client")
return sdkerrors.Wrapf(errors.ErrClientNotFound, "cannot update client with ID %s", clientID)
}

if clientState.Frozen {
return sdkerrors.Wrap(errors.ErrClientFrozen(k.codespace, clientID), "cannot update client")
return sdkerrors.Wrapf(errors.ErrClientFrozen, "cannot update client with ID %s", clientID)
}

consensusState, found := k.GetConsensusState(ctx, clientID)
if !found {
return sdkerrors.Wrap(errors.ErrConsensusStateNotFound(k.codespace), "cannot update client")
return sdkerrors.Wrapf(errors.ErrConsensusStateNotFound, "cannot update client with ID %s", clientID)
}

consensusState, err := consensusState.CheckValidityAndUpdateState(header)
if err != nil {
return sdkerrors.Wrap(err, "cannot update client")
return sdkerrors.Wrapf(err, "cannot update client with ID %s", clientID)
}

k.SetConsensusState(ctx, clientID, consensusState)
Expand All @@ -83,25 +83,25 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H
func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, evidence evidenceexported.Evidence) error {
misbehaviour, ok := evidence.(tendermint.Misbehaviour)
if !ok {
return errors.ErrInvalidClientType(k.codespace, "consensus type is not Tendermint")
return sdkerrors.Wrap(errors.ErrInvalidClientType, "consensus type is not Tendermint")
}

clientState, found := k.GetClientState(ctx, misbehaviour.ClientID)
if !found {
return errors.ErrClientNotFound(k.codespace, misbehaviour.ClientID)
return sdkerrors.Wrap(errors.ErrClientNotFound, misbehaviour.ClientID)
}

committer, found := k.GetCommitter(ctx, misbehaviour.ClientID, uint64(misbehaviour.GetHeight()))
if !found {
return errors.ErrCommitterNotFound(k.codespace, fmt.Sprintf("committer not found for height %d", misbehaviour.GetHeight()))
return errors.ErrCommitterNotFound
}
tmCommitter, ok := committer.(tendermint.Committer)
if !ok {
return errors.ErrInvalidCommitter(k.codespace, "committer type is not Tendermint")
return sdkerrors.Wrap(errors.ErrInvalidCommitter, "committer type is not Tendermint")
}

if err := tendermint.CheckMisbehaviour(tmCommitter, misbehaviour); err != nil {
return errors.ErrInvalidEvidence(k.codespace, err.Error())
return sdkerrors.Wrap(errors.ErrInvalidEvidence, err.Error())
}

clientState, err := k.freeze(ctx, clientState)
Expand Down
14 changes: 6 additions & 8 deletions x/ibc/02-client/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,15 @@ import (
// Keeper represents a type that grants read and write permissions to any client
// state information
type Keeper struct {
storeKey sdk.StoreKey
cdc *codec.Codec
codespace sdk.CodespaceType
storeKey sdk.StoreKey
cdc *codec.Codec
}

// NewKeeper creates a new NewKeeper instance
func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, codespace sdk.CodespaceType) Keeper {
func NewKeeper(cdc *codec.Codec, key sdk.StoreKey) Keeper {
return Keeper{
storeKey: key,
cdc: cdc,
codespace: sdk.CodespaceType(fmt.Sprintf("%s/%s", codespace, errors.DefaultCodespace)), // "ibc/client",
storeKey: key,
cdc: cdc,
}
}

Expand Down Expand Up @@ -181,7 +179,7 @@ func (k Keeper) initialize(ctx sdk.Context, clientID string, consensusState expo
// 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, clientState.ID)
}

clientState.Frozen = true
Expand Down
13 changes: 4 additions & 9 deletions x/ibc/02-client/keeper/querier.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package keeper

import (
"fmt"

abci "github.com/tendermint/tendermint/abci/types"

"github.com/cosmos/cosmos-sdk/client"
Expand Down Expand Up @@ -48,7 +46,7 @@ func QuerierClientState(ctx sdk.Context, req abci.RequestQuery, k Keeper) ([]byt

clientState, found := k.GetClientState(ctx, params.ClientID)
if !found {
return nil, errors.ErrClientTypeNotFound(k.codespace)
return nil, sdkerrors.Wrap(errors.ErrClientTypeNotFound, params.ClientID)
}

bz, err := codec.MarshalJSONIndent(k.cdc, clientState)
Expand All @@ -69,7 +67,7 @@ func QuerierConsensusState(ctx sdk.Context, req abci.RequestQuery, k Keeper) ([]

consensusState, found := k.GetConsensusState(ctx, params.ClientID)
if !found {
return nil, errors.ErrConsensusStateNotFound(k.codespace)
return nil, errors.ErrConsensusStateNotFound
}

bz, err := codec.MarshalJSONIndent(k.cdc, consensusState)
Expand All @@ -90,7 +88,7 @@ func QuerierVerifiedRoot(ctx sdk.Context, req abci.RequestQuery, k Keeper) ([]by

root, found := k.GetVerifiedRoot(ctx, params.ClientID, params.Height)
if !found {
return nil, errors.ErrRootNotFound(k.codespace)
return nil, errors.ErrRootNotFound
}

bz, err := codec.MarshalJSONIndent(k.cdc, root)
Expand All @@ -111,10 +109,7 @@ func QuerierCommitter(ctx sdk.Context, req abci.RequestQuery, k Keeper) ([]byte,

committer, found := k.GetCommitter(ctx, params.ClientID, params.Height)
if !found {
return nil, errors.ErrCommitterNotFound(
k.codespace,
fmt.Sprintf("committer not found on height: %d", params.Height),
)
return nil, errors.ErrCommitterNotFound
}

bz, err := codec.MarshalJSONIndent(k.cdc, committer)
Expand Down
Loading

0 comments on commit 1e10e1f

Please sign in to comment.