From 6973c564572908c780becf74ffaad1ff04ae13e1 Mon Sep 17 00:00:00 2001 From: colin axner Date: Fri, 22 May 2020 14:17:52 -0700 Subject: [PATCH] x/ibc: fix internal errors messages (#6268) * update errors in ibc client and localhost * update code 1 errors * fix misisng import * add import * fix compile and lint issues * fix lint Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> --- types/errors/errors.go | 6 ++++++ x/ibc/02-client/exported/exported.go | 4 ++-- x/ibc/07-tendermint/misbehaviour.go | 12 +++++++----- x/ibc/07-tendermint/types/client_state.go | 16 +++++++++------- x/ibc/07-tendermint/types/errors.go | 3 +++ x/ibc/07-tendermint/update.go | 7 +++++-- x/ibc/09-localhost/types/client_state.go | 5 ++--- x/ibc/23-commitment/types/errors.go | 5 +++-- x/ibc/23-commitment/types/merkle.go | 8 ++++---- x/ibc/24-host/utils.go | 7 ++++--- 10 files changed, 45 insertions(+), 28 deletions(-) diff --git a/types/errors/errors.go b/types/errors/errors.go index 684186b861..1ec52fbe72 100644 --- a/types/errors/errors.go +++ b/types/errors/errors.go @@ -100,6 +100,12 @@ var ( // ErrInvalidVersion defines a general error for an invalid version ErrInvalidVersion = Register(RootCodespace, 27, "invalid version") + // ErrInvalidChainID defines an error when the chain-id is invalid. + ErrInvalidChainID = Register(RootCodespace, 28, "invalid chain-id") + + // ErrInvalidType defines an error an invalid type. + ErrInvalidType = Register(RootCodespace, 29, "invalid type") + // ErrPanic is only set when we recover from a panic, so we know to // redact potentially sensitive system info ErrPanic = Register(UndefinedCodespace, 111222, "panic") diff --git a/x/ibc/02-client/exported/exported.go b/x/ibc/02-client/exported/exported.go index 50f0183cba..304a62430c 100644 --- a/x/ibc/02-client/exported/exported.go +++ b/x/ibc/02-client/exported/exported.go @@ -2,9 +2,9 @@ package exported import ( "encoding/json" - "fmt" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/codec" evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported" @@ -188,7 +188,7 @@ func (ct *ClientType) UnmarshalJSON(data []byte) error { clientType := ClientTypeFromString(s) if clientType == 0 { - return fmt.Errorf("invalid client type '%s'", s) + return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "invalid client type '%s'", s) } *ct = clientType diff --git a/x/ibc/07-tendermint/misbehaviour.go b/x/ibc/07-tendermint/misbehaviour.go index c2c3892f3e..d7e0d1e320 100644 --- a/x/ibc/07-tendermint/misbehaviour.go +++ b/x/ibc/07-tendermint/misbehaviour.go @@ -1,8 +1,6 @@ package tendermint import ( - "errors" - "fmt" "time" lite "github.com/tendermint/tendermint/lite2" @@ -76,7 +74,11 @@ func checkMisbehaviour( // assert that the timestamp is not from more than an unbonding period ago if currentTimestamp.Sub(consensusState.Timestamp) >= clientState.UnbondingPeriod { - return errors.New("unbonding period since last consensus state timestamp is over") + return sdkerrors.Wrapf( + types.ErrUnbondingPeriodExpired, + "current timestamp minus the latest consensus state timestamp is greater than or equal to the unbonding period (%s >= %s)", + currentTimestamp.Sub(consensusState.Timestamp), clientState.UnbondingPeriod, + ) } // TODO: Evidence must be within trusting period @@ -88,14 +90,14 @@ func checkMisbehaviour( evidence.ChainID, evidence.Header1.Commit.BlockID, evidence.Header1.Height, evidence.Header1.Commit, lite.DefaultTrustLevel, ); err != nil { - return fmt.Errorf("validator set in header 1 has too much change from last known validator set: %v", err) + return sdkerrors.Wrapf(clienttypes.ErrInvalidEvidence, "validator set in header 1 has too much change from last known validator set: %v", err) } if err := consensusState.ValidatorSet.VerifyCommitTrusting( evidence.ChainID, evidence.Header2.Commit.BlockID, evidence.Header2.Height, evidence.Header2.Commit, lite.DefaultTrustLevel, ); err != nil { - return fmt.Errorf("validator set in header 2 has too much change from last known validator set: %v", err) + return sdkerrors.Wrapf(clienttypes.ErrInvalidEvidence, "validator set in header 2 has too much change from last known validator set: %v", err) } return nil diff --git a/x/ibc/07-tendermint/types/client_state.go b/x/ibc/07-tendermint/types/client_state.go index 7bcb6129e2..dec469d5a5 100644 --- a/x/ibc/07-tendermint/types/client_state.go +++ b/x/ibc/07-tendermint/types/client_state.go @@ -1,7 +1,6 @@ package types import ( - "errors" "fmt" "time" @@ -68,7 +67,10 @@ func Initialize( ) (ClientState, error) { if trustingPeriod >= ubdPeriod { - return ClientState{}, errors.New("trusting period should be < unbonding period") + return ClientState{}, sdkerrors.Wrapf( + ErrInvalidTrustingPeriod, + "trusting period (%s) should be < unbonding period (%s)", trustingPeriod, ubdPeriod, + ) } clientState := NewClientState(id, trustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header) @@ -134,13 +136,13 @@ func (cs ClientState) Validate() error { return err } if cs.TrustingPeriod == 0 { - return errors.New("trusting period cannot be zero") + return sdkerrors.Wrap(ErrInvalidTrustingPeriod, "trusting period cannot be zero") } if cs.UnbondingPeriod == 0 { - return errors.New("unbonding period cannot be zero") + return sdkerrors.Wrap(ErrInvalidUnbondingPeriod, "unbonding period cannot be zero") } if cs.MaxClockDrift == 0 { - return errors.New("max clock drift cannot be zero") + return sdkerrors.Wrap(ErrInvalidMaxClockDrift, "max clock drift cannot be zero") } return cs.LastHeader.ValidateBasic(cs.GetChainID()) } @@ -203,7 +205,7 @@ func (cs ClientState) VerifyConnectionState( connection, ok := connectionEnd.(connectiontypes.ConnectionEnd) if !ok { - return fmt.Errorf("invalid connection type %T", connectionEnd) + return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "invalid connection type %T", connectionEnd) } bz, err := cdc.MarshalBinaryBare(&connection) @@ -242,7 +244,7 @@ func (cs ClientState) VerifyChannelState( channelEnd, ok := channel.(channeltypes.Channel) if !ok { - return fmt.Errorf("invalid channel type %T", channel) + return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "invalid channel type %T", channel) } bz, err := cdc.MarshalBinaryBare(&channelEnd) diff --git a/x/ibc/07-tendermint/types/errors.go b/x/ibc/07-tendermint/types/errors.go index fd29dd6e2a..0c659acbdd 100644 --- a/x/ibc/07-tendermint/types/errors.go +++ b/x/ibc/07-tendermint/types/errors.go @@ -13,4 +13,7 @@ var ( ErrInvalidTrustingPeriod = sdkerrors.Register(SubModuleName, 2, "invalid trusting period") ErrInvalidUnbondingPeriod = sdkerrors.Register(SubModuleName, 3, "invalid unbonding period") ErrInvalidHeader = sdkerrors.Register(SubModuleName, 4, "invalid header") + ErrInvalidMaxClockDrift = sdkerrors.Register(SubModuleName, 5, "invalid max clock drift") + ErrTrustingPeriodExpired = sdkerrors.Register(SubModuleName, 6, "time since latest trusted state has passed the trusting period") + ErrUnbondingPeriodExpired = sdkerrors.Register(SubModuleName, 7, "time since latest trusted state has passed the unbonding period") ) diff --git a/x/ibc/07-tendermint/update.go b/x/ibc/07-tendermint/update.go index 414d4bc071..3e95489a48 100644 --- a/x/ibc/07-tendermint/update.go +++ b/x/ibc/07-tendermint/update.go @@ -1,7 +1,6 @@ package tendermint import ( - "errors" "time" lite "github.com/tendermint/tendermint/lite2" @@ -56,7 +55,11 @@ func checkValidity( ) error { // assert trusting period has not yet passed if currentTimestamp.Sub(clientState.GetLatestTimestamp()) >= clientState.TrustingPeriod { - return errors.New("trusting period since last client timestamp already passed") + return sdkerrors.Wrapf( + types.ErrTrustingPeriodExpired, + "current timestamp minus the latest trusted client state timestamp is greater than or equal to the trusting period (%s >= %s)", + currentTimestamp.Sub(clientState.GetLatestTimestamp()), clientState.TrustingPeriod, + ) } // assert header timestamp is not past the trusting period diff --git a/x/ibc/09-localhost/types/client_state.go b/x/ibc/09-localhost/types/client_state.go index 61d7bcd299..ab613be1dd 100644 --- a/x/ibc/09-localhost/types/client_state.go +++ b/x/ibc/09-localhost/types/client_state.go @@ -3,7 +3,6 @@ package types import ( "bytes" "encoding/binary" - "errors" "fmt" "strings" @@ -67,10 +66,10 @@ func (cs ClientState) IsFrozen() bool { // Validate performs a basic validation of the client state fields. func (cs ClientState) Validate() error { if strings.TrimSpace(cs.ChainID) == "" { - return errors.New("chain id cannot be blank") + return sdkerrors.Wrap(sdkerrors.ErrInvalidChainID, "chain id cannot be blank") } if cs.Height <= 0 { - return fmt.Errorf("height must be positive: %d", cs.Height) + return sdkerrors.Wrapf(sdkerrors.ErrInvalidHeight, "height must be positive: %d", cs.Height) } return host.ClientIdentifierValidator(cs.ID) } diff --git a/x/ibc/23-commitment/types/errors.go b/x/ibc/23-commitment/types/errors.go index e8194d58d8..7191baef1c 100644 --- a/x/ibc/23-commitment/types/errors.go +++ b/x/ibc/23-commitment/types/errors.go @@ -9,6 +9,7 @@ const SubModuleName string = "commitment" // IBC connection sentinel errors var ( - ErrInvalidProof = sdkerrors.Register(SubModuleName, 2, "invalid proof") - ErrInvalidPrefix = sdkerrors.Register(SubModuleName, 3, "invalid prefix") + ErrInvalidProof = sdkerrors.Register(SubModuleName, 2, "invalid proof") + ErrInvalidPrefix = sdkerrors.Register(SubModuleName, 3, "invalid prefix") + ErrInvalidMerkleProof = sdkerrors.Register(SubModuleName, 4, "invalid merkle proof") ) diff --git a/x/ibc/23-commitment/types/merkle.go b/x/ibc/23-commitment/types/merkle.go index 4f54c2341b..2040b1179d 100644 --- a/x/ibc/23-commitment/types/merkle.go +++ b/x/ibc/23-commitment/types/merkle.go @@ -1,10 +1,10 @@ package types import ( - "errors" "net/url" "github.com/cosmos/cosmos-sdk/store/rootmulti" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/exported" host "github.com/cosmos/cosmos-sdk/x/ibc/24-host" @@ -115,7 +115,7 @@ func ApplyPrefix(prefix exported.Prefix, path string) (MerklePath, error) { } if prefix == nil || prefix.IsEmpty() { - return MerklePath{}, errors.New("prefix can't be empty") + return MerklePath{}, sdkerrors.Wrap(ErrInvalidPrefix, "prefix can't be empty") } return NewMerklePath([]string{string(prefix.Bytes()), path}), nil } @@ -130,7 +130,7 @@ func (MerkleProof) GetCommitmentType() exported.Type { // VerifyMembership verifies the membership pf a merkle proof against the given root, path, and value. func (proof MerkleProof) VerifyMembership(root exported.Root, path exported.Path, value []byte) error { if proof.IsEmpty() || root == nil || root.IsEmpty() || path == nil || path.IsEmpty() || len(value) == 0 { - return errors.New("empty params or proof") + return sdkerrors.Wrap(ErrInvalidMerkleProof, "empty params or proof") } runtime := rootmulti.DefaultProofRuntime() @@ -140,7 +140,7 @@ func (proof MerkleProof) VerifyMembership(root exported.Root, path exported.Path // VerifyNonMembership verifies the absence of a merkle proof against the given root and path. func (proof MerkleProof) VerifyNonMembership(root exported.Root, path exported.Path) error { if proof.IsEmpty() || root == nil || root.IsEmpty() || path == nil || path.IsEmpty() { - return errors.New("empty params or proof") + return sdkerrors.Wrap(ErrInvalidMerkleProof, "empty params or proof") } runtime := rootmulti.DefaultProofRuntime() diff --git a/x/ibc/24-host/utils.go b/x/ibc/24-host/utils.go index 2e9c91b183..b0d06c9728 100644 --- a/x/ibc/24-host/utils.go +++ b/x/ibc/24-host/utils.go @@ -1,8 +1,9 @@ package host import ( - "fmt" "strings" + + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) // RemovePath is an util function to remove a path from a set. @@ -20,11 +21,11 @@ func RemovePath(paths []string, path string) ([]string, bool) { func ParseChannelPath(path string) (string, string, error) { split := strings.Split(path, "/") if len(split) < 5 { - return "", "", fmt.Errorf("cannot parse channel path %s", path) + return "", "", sdkerrors.Wrapf(ErrInvalidPath, "cannot parse channel path %s", path) } if split[1] != "ports" || split[3] != "channels" { - return "", "", fmt.Errorf("cannot parse channel path %s", path) + return "", "", sdkerrors.Wrapf(ErrInvalidPath, "cannot parse channel path %s", path) } return split[2], split[4], nil