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 👍

6 changes: 5 additions & 1 deletion x/stake/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cli
import (
"fmt"

"github.com/pkg/errors"
"github.com/spf13/cobra"
"github.com/spf13/viper"
"github.com/tendermint/tendermint/libs/cli"
Expand Down Expand Up @@ -139,7 +140,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 errors.Errorf("cannot unmarshal delegation: %v", err)
Copy link
Collaborator

@fedekunze fedekunze Aug 13, 2018

Choose a reason for hiding this comment

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

Wouldn't it be better to add a new error function in stake/types/errors.go and use that instead ? you can use the CodeInternal as the CodeType

}

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(), errors.Errorf("cannot unmarshal delegation: %v", err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same

}
sharesAmount = sharesPercent.Mul(delegation.Shares)
}

return
}

Expand Down