Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle unmarshalling failures gracefully in x/stake commands #1997

Merged
merged 10 commits into from
Aug 15, 2018
1 change: 1 addition & 0 deletions PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,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] Handle panics gracefully when `gaiacli stake {delegation,unbond}` fail to unmarshal delegation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's reference the PR here 👍

5 changes: 4 additions & 1 deletion x/stake/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand Down
7 changes: 4 additions & 3 deletions x/stake/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
8 changes: 4 additions & 4 deletions x/stake/types/delegation.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package types

import (
"bytes"
"errors"
"fmt"
"time"

Expand Down Expand Up @@ -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])
Expand Down Expand Up @@ -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])
Expand Down Expand Up @@ -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])
Expand Down
13 changes: 13 additions & 0 deletions x/stake/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")
}
Expand Down Expand Up @@ -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")
}
Expand Down Expand Up @@ -125,6 +134,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) triple")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tuple 🤷‍♂️ ?

}

func ErrNoRedelegation(codespace sdk.CodespaceType) sdk.Error {
return sdk.NewError(codespace, CodeInvalidDelegation, "no redelegation found")
}
Expand Down
10 changes: 4 additions & 6 deletions x/stake/types/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package types

import (
"bytes"
"errors"
"fmt"

abci "github.com/tendermint/tendermint/abci/types"
Expand Down Expand Up @@ -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,
Expand Down