Merge pull request #1997: Handle unmarshalling failures gracefully in…

… x/stake commands

* Handle panic gracefully when unbond begin fails

See #1831

* Handle failure to query delegation gracefully.

Closes #1907

* Update PENDING.md

* Reuse stake's error functions

* New ErrBadValidatorAddr error

UnmarshalValidator() checks the address length first;
it does not make sense to attempt unmarshalling if the
address is wrong.

* New ErrBadDelegationAddr error

* Introduce ErrBad{Redelegation,UnbondingDelegation}Addr custom errors to replace errors.New() calls

* Replace ErrBadUnbondingDelegationAddr with ErrBadDelegationAddr to avoid duplication

Thanks: @melekes for pointing this out

* Use sdk.AddrLen instead of hardcoded address length

* s/triple/tuple/ ## mention PR id in PENDING.md

PENDING.md

@@ -101,3 +101,4 @@ BUG FIXES
 *  \#1787 Fixed bug where Tally fails due to revoked/unbonding validator
 *  \#1787 Fixed bug where Tally fails due to revoked/unbonding validator
 * [basecoin] Fixes coin transaction failure and account query [discussion](https://forum.cosmos.network/t/unmarshalbinarybare-expected-to-read-prefix-bytes-75fbfab8-since-it-is-registered-concrete-but-got-0a141dfa/664/6)
+* [cli] \#1997 Handle panics gracefully when `gaiacli stake {delegation,unbond}` fail to unmarshal delegation.

x/stake/client/cli/query.go

@@ -139,7 +139,10 @@ func GetCmdQueryDelegation(storeName string, cdc *wire.Codec) *cobra.Command {
 			}
 
 			// parse out the delegation
-			delegation := types.MustUnmarshalDelegation(cdc, key, res)
+			delegation, err := types.UnmarshalDelegation(cdc, key, res)
+			if err != nil {
+				return err
+			}
 
 			switch viper.Get(cli.OutputFlag) {
 			case "text":

x/stake/client/cli/tx.go

@@ -273,11 +273,12 @@ func getShares(
 		if err != nil {
 			return sharesAmount, errors.Errorf("cannot find delegation to determine percent Error: %v", err)
 		}
-
-		delegation := types.MustUnmarshalDelegation(cdc, key, resQuery)
+		delegation, err := types.UnmarshalDelegation(cdc, key, resQuery)
+		if err != nil {
+			return sdk.ZeroRat(), err
+		}
 		sharesAmount = sharesPercent.Mul(delegation.Shares)
 	}
-
 	return
 }
 

x/stake/types/delegation.go

@@ -2,7 +2,6 @@ package types
 
 import (
 	"bytes"
-	"errors"
 	"fmt"
 	"time"
 
@@ -48,12 +47,13 @@ func UnmarshalDelegation(cdc *wire.Codec, key, value []byte) (delegation Delegat
 	var storeValue delegationValue
 	err = cdc.UnmarshalBinary(value, &storeValue)
 	if err != nil {
+		err = fmt.Errorf("%v: %v", ErrNoDelegation(DefaultCodespace).Data(), err)
 		return
 	}
 
 	addrs := key[1:] // remove prefix bytes
 	if len(addrs) != 2*sdk.AddrLen {
-		err = errors.New("unexpected key length")
+		err = fmt.Errorf("%v", ErrBadDelegationAddr(DefaultCodespace).Data())
 		return
 	}
 	delAddr := sdk.AccAddress(addrs[:sdk.AddrLen])
@@ -143,7 +143,7 @@ func UnmarshalUBD(cdc *wire.Codec, key, value []byte) (ubd UnbondingDelegation,
 
 	addrs := key[1:] // remove prefix bytes
 	if len(addrs) != 2*sdk.AddrLen {
-		err = errors.New("unexpected key length")
+		err = fmt.Errorf("%v", ErrBadDelegationAddr(DefaultCodespace).Data())
 		return
 	}
 	delAddr := sdk.AccAddress(addrs[:sdk.AddrLen])
@@ -235,7 +235,7 @@ func UnmarshalRED(cdc *wire.Codec, key, value []byte) (red Redelegation, err err
 
 	addrs := key[1:] // remove prefix bytes
 	if len(addrs) != 3*sdk.AddrLen {
-		err = errors.New("unexpected key length")
+		err = fmt.Errorf("%v", ErrBadRedelegationAddr(DefaultCodespace).Data())
 		return
 	}
 	delAddr := sdk.AccAddress(addrs[:sdk.AddrLen])

x/stake/types/errors.go

@@ -17,6 +17,7 @@ const (
 	CodeInvalidDelegation CodeType = 102
 	CodeInvalidInput      CodeType = 103
 	CodeValidatorJailed   CodeType = 104
+	CodeInvalidAddress    CodeType = sdk.CodeInvalidAddress
 	CodeUnauthorized      CodeType = sdk.CodeUnauthorized
 	CodeInternal          CodeType = sdk.CodeInternal
 	CodeUnknownRequest    CodeType = sdk.CodeUnknownRequest
@@ -27,6 +28,10 @@ func ErrNilValidatorAddr(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidInput, "validator address is nil")
 }
 
+func ErrBadValidatorAddr(codespace sdk.CodespaceType) sdk.Error {
+	return sdk.NewError(codespace, CodeInvalidAddress, "validator address is invalid")
+}
+
 func ErrNoValidatorFound(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidValidator, "validator does not exist for that address")
 }
@@ -68,6 +73,10 @@ func ErrBadDenom(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation, "invalid coin denomination")
 }
 
+func ErrBadDelegationAddr(codespace sdk.CodespaceType) sdk.Error {
+	return sdk.NewError(codespace, CodeInvalidInput, "unexpected address length for this (address, validator) pair")
+}
+
 func ErrBadDelegationAmount(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation, "amount must be > 0")
 }
@@ -118,6 +127,10 @@ func ErrExistingUnbondingDelegation(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation, "existing unbonding delegation found")
 }
 
+func ErrBadRedelegationAddr(codespace sdk.CodespaceType) sdk.Error {
+	return sdk.NewError(codespace, CodeInvalidInput, "unexpected address length for this (address, srcValidator, dstValidator) tuple")
+}
+
 func ErrNoRedelegation(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation, "no redelegation found")
 }

x/stake/types/validator.go

@@ -2,7 +2,6 @@ package types
 
 import (
 	"bytes"
-	"errors"
 	"fmt"
 
 	abci "github.com/tendermint/tendermint/abci/types"
@@ -115,17 +114,16 @@ func MustUnmarshalValidator(cdc *wire.Codec, ownerAddr, value []byte) Validator
 
 // unmarshal a redelegation from a store key and value
 func UnmarshalValidator(cdc *wire.Codec, ownerAddr, value []byte) (validator Validator, err error) {
+	if len(ownerAddr) != sdk.AddrLen {
+		err = fmt.Errorf("%v", ErrBadValidatorAddr(DefaultCodespace).Data())
+		return
+	}
 	var storeValue validatorValue
 	err = cdc.UnmarshalBinary(value, &storeValue)
 	if err != nil {
 		return
 	}
 
-	if len(ownerAddr) != 20 {
-		err = errors.New("unexpected address length")
-		return
-	}
-
 	return Validator{
 		Owner:                 ownerAddr,
 		PubKey:                storeValue.PubKey,