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.

Sign up for GitHub

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

Remove Redundant Staking Errors #9231

Merged
merged 8 commits into from
May 19, 2021
Merged

Remove Redundant Staking Errors #9231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bc52259
refactor(staking errors): 'invalid' errors: use ErrInvalidRequest, re…
jeebster Apr 29, 2021
6cfd058
refactor(staking errors): fix error registration codes
jeebster Apr 29, 2021
c3b4b62
support(staking errors): add changelog entry
jeebster Apr 29, 2021
b721616
fix(staking test suite): update expected error codes relative to refa…
jeebster Apr 30, 2021
4914118
chore(staking errors): code formatting
jeebster May 13, 2021
d5d99fe
Merge branch 'master' into jeebster/5450-staking-errors-refactor
tac0turtle May 19, 2021
80c36e5
Merge branch 'master' into jeebster/5450-staking-errors-refactor
mergify[bot] May 19, 2021
986fa22
Merge branch 'master' into jeebster/5450-staking-errors-refactor
mergify[bot] May 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
# Changelog

## [Unreleased]

* [\#9231](https://github.com/cosmos/cosmos-sdk/pull/9231) Remove redundant staking errors.
* [\#9205](https://github.com/cosmos/cosmos-sdk/pull/9205) Improve readability in `abci` handleQueryP2P
* [\#9235](https://github.com/cosmos/cosmos-sdk/pull/9235) CreateMembershipProof/CreateNonMembershipProof now returns an error
if input key is empty, or input data contains empty key.
Expand Down
7 changes: 4 additions & 3 deletions x/staking/client/cli/tx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/cosmos/cosmos-sdk/client/tx"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/version"
"github.com/cosmos/cosmos-sdk/x/staking/types"
)
Expand Down Expand Up @@ -122,7 +123,7 @@ func NewEditValidatorCmd() *cobra.Command {
if minSelfDelegationString != "" {
msb, ok := sdk.NewIntFromString(minSelfDelegationString)
if !ok {
return types.ErrMinSelfDelegationInvalid
return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "minimum self delegation must be a positive integer")
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can use the wrap method of the error, it makes the code a bit shorter

sdkerrors.ErrInvalidRequest.Wrap("minimum self delegation must be a positive integer"

Same in other places.

Copy link
Contributor Author

@jeebster jeebster May 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@robert-zaremba Thanks for bringing this to my attention, I'll take a look at the Wrap() implementation. I see both implementation strategies in the repository, please let me know which is preferred and whether refactoring should be executed accordingly

Copy link
Member

Choose a reason for hiding this comment

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

I think this is more of a nit. We use the design currently coded throughout the repo and the one robert mentioned in a few places.

@robert-zaremba want to approve this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not blocking it - I only made a suggestion. The PR already got enough approvals.

Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW - we implemented the Wrap method for the error types 2-3 months ago, that why it's not use that much.

}

newMinSelfDelegation = &msb
Expand Down Expand Up @@ -322,7 +323,7 @@ func newBuildCreateValidatorMsg(clientCtx client.Context, txf tx.Factory, fs *fl

minSelfDelegation, ok := sdk.NewIntFromString(msbStr)
if !ok {
return txf, nil, types.ErrMinSelfDelegationInvalid
return txf, nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "minimum self delegation must be a positive integer")
}

msg, err := types.NewMsgCreateValidator(
Expand Down Expand Up @@ -525,7 +526,7 @@ func BuildCreateValidatorMsg(clientCtx client.Context, config TxCreateValidatorC
minSelfDelegation, ok := sdk.NewIntFromString(msbStr)

if !ok {
return txBldr, nil, types.ErrMinSelfDelegationInvalid
return txBldr, nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "minimum self delegation must be a positive integer")
}

msg, err := types.NewMsgCreateValidator(
Expand Down
4 changes: 2 additions & 2 deletions x/staking/client/testutil/suite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1157,7 +1157,7 @@ func (s *IntegrationTestSuite) TestNewRedelegateCmd() {
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
},
false, 4, &sdk.TxResponse{},
false, 3, &sdk.TxResponse{},
},
{
"with wrong destination validator address",
Expand All @@ -1170,7 +1170,7 @@ func (s *IntegrationTestSuite) TestNewRedelegateCmd() {
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
},
false, 39, &sdk.TxResponse{},
false, 31, &sdk.TxResponse{},
},
{
"valid transaction of delegate",
Expand Down
2 changes: 1 addition & 1 deletion x/staking/keeper/delegation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -923,7 +923,7 @@ func (k Keeper) ValidateUnbondAmount(

delShares := del.GetShares()
if sharesTruncated.GT(delShares) {
return shares, types.ErrBadSharesAmount
return shares, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "invalid shares amount")
}

// Cap the shares at the delegation's shares. Shares being greater could occur
Expand Down
16 changes: 12 additions & 4 deletions x/staking/keeper/msg_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ func (k msgServer) CreateValidator(goCtx context.Context, msg *types.MsgCreateVa

bondDenom := k.BondDenom(ctx)
if msg.Value.Denom != bondDenom {
return nil, sdkerrors.Wrapf(types.ErrBadDenom, "got %s, expected %s", msg.Value.Denom, bondDenom)
return nil, sdkerrors.Wrapf(
sdkerrors.ErrInvalidRequest, "invalid coin denomination: got %s, expected %s", msg.Value.Denom, bondDenom,
)
}

if _, err := msg.Description.EnsureLength(); err != nil {
Expand Down Expand Up @@ -203,7 +205,9 @@ func (k msgServer) Delegate(goCtx context.Context, msg *types.MsgDelegate) (*typ

bondDenom := k.BondDenom(ctx)
if msg.Amount.Denom != bondDenom {
return nil, sdkerrors.Wrapf(types.ErrBadDenom, "got %s, expected %s", msg.Amount.Denom, bondDenom)
return nil, sdkerrors.Wrapf(
sdkerrors.ErrInvalidRequest, "invalid coin denomination: got %s, expected %s", msg.Amount.Denom, bondDenom,
)
}

// NOTE: source funds are always unbonded
Expand Down Expand Up @@ -260,7 +264,9 @@ func (k msgServer) BeginRedelegate(goCtx context.Context, msg *types.MsgBeginRed

bondDenom := k.BondDenom(ctx)
if msg.Amount.Denom != bondDenom {
return nil, sdkerrors.Wrapf(types.ErrBadDenom, "got %s, expected %s", msg.Amount.Denom, bondDenom)
return nil, sdkerrors.Wrapf(
sdkerrors.ErrInvalidRequest, "invalid coin denomination: got %s, expected %s", msg.Amount.Denom, bondDenom,
)
}

valDstAddr, err := sdk.ValAddressFromBech32(msg.ValidatorDstAddress)
Expand Down Expand Up @@ -327,7 +333,9 @@ func (k msgServer) Undelegate(goCtx context.Context, msg *types.MsgUndelegate) (

bondDenom := k.BondDenom(ctx)
if msg.Amount.Denom != bondDenom {
return nil, sdkerrors.Wrapf(types.ErrBadDenom, "got %s, expected %s", msg.Amount.Denom, bondDenom)
return nil, sdkerrors.Wrapf(
sdkerrors.ErrInvalidRequest, "invalid coin denomination: got %s, expected %s", msg.Amount.Denom, bondDenom,
)
}

completionTime, err := k.Keeper.Undelegate(ctx, delegatorAddress, addr, shares)
Expand Down
82 changes: 37 additions & 45 deletions x/staking/types/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,49 +12,41 @@ import (
// REF: https://github.com/cosmos/cosmos-sdk/issues/5450
var (
ErrEmptyValidatorAddr = sdkerrors.Register(ModuleName, 2, "empty validator address")
ErrBadValidatorAddr = sdkerrors.Register(ModuleName, 3, "validator address is invalid")
ErrNoValidatorFound = sdkerrors.Register(ModuleName, 4, "validator does not exist")
ErrValidatorOwnerExists = sdkerrors.Register(ModuleName, 5, "validator already exist for this operator address; must use new validator operator address")
ErrValidatorPubKeyExists = sdkerrors.Register(ModuleName, 6, "validator already exist for this pubkey; must use new validator pubkey")
ErrValidatorPubKeyTypeNotSupported = sdkerrors.Register(ModuleName, 7, "validator pubkey type is not supported")
ErrValidatorJailed = sdkerrors.Register(ModuleName, 8, "validator for this address is currently jailed")
ErrBadRemoveValidator = sdkerrors.Register(ModuleName, 9, "failed to remove validator")
ErrCommissionNegative = sdkerrors.Register(ModuleName, 10, "commission must be positive")
ErrCommissionHuge = sdkerrors.Register(ModuleName, 11, "commission cannot be more than 100%")
ErrCommissionGTMaxRate = sdkerrors.Register(ModuleName, 12, "commission cannot be more than the max rate")
ErrCommissionUpdateTime = sdkerrors.Register(ModuleName, 13, "commission cannot be changed more than once in 24h")
ErrCommissionChangeRateNegative = sdkerrors.Register(ModuleName, 14, "commission change rate must be positive")
ErrCommissionChangeRateGTMaxRate = sdkerrors.Register(ModuleName, 15, "commission change rate cannot be more than the max rate")
ErrCommissionGTMaxChangeRate = sdkerrors.Register(ModuleName, 16, "commission cannot be changed more than max change rate")
ErrSelfDelegationBelowMinimum = sdkerrors.Register(ModuleName, 17, "validator's self delegation must be greater than their minimum self delegation")
ErrMinSelfDelegationInvalid = sdkerrors.Register(ModuleName, 18, "minimum self delegation must be a positive integer")
ErrMinSelfDelegationDecreased = sdkerrors.Register(ModuleName, 19, "minimum self delegation cannot be decrease")
ErrEmptyDelegatorAddr = sdkerrors.Register(ModuleName, 20, "empty delegator address")
ErrBadDenom = sdkerrors.Register(ModuleName, 21, "invalid coin denomination")
ErrBadDelegationAddr = sdkerrors.Register(ModuleName, 22, "invalid address for (address, validator) tuple")
ErrBadDelegationAmount = sdkerrors.Register(ModuleName, 23, "invalid delegation amount")
ErrNoDelegation = sdkerrors.Register(ModuleName, 24, "no delegation for (address, validator) tuple")
ErrBadDelegatorAddr = sdkerrors.Register(ModuleName, 25, "delegator does not exist with address")
ErrNoDelegatorForAddress = sdkerrors.Register(ModuleName, 26, "delegator does not contain delegation")
ErrInsufficientShares = sdkerrors.Register(ModuleName, 27, "insufficient delegation shares")
ErrDelegationValidatorEmpty = sdkerrors.Register(ModuleName, 28, "cannot delegate to an empty validator")
ErrNotEnoughDelegationShares = sdkerrors.Register(ModuleName, 29, "not enough delegation shares")
ErrBadSharesAmount = sdkerrors.Register(ModuleName, 30, "invalid shares amount")
ErrBadSharesPercent = sdkerrors.Register(ModuleName, 31, "Invalid shares percent")
ErrNotMature = sdkerrors.Register(ModuleName, 32, "entry not mature")
ErrNoUnbondingDelegation = sdkerrors.Register(ModuleName, 33, "no unbonding delegation found")
ErrMaxUnbondingDelegationEntries = sdkerrors.Register(ModuleName, 34, "too many unbonding delegation entries for (delegator, validator) tuple")
ErrBadRedelegationAddr = sdkerrors.Register(ModuleName, 35, "invalid address for (address, src-validator, dst-validator) tuple")
ErrNoRedelegation = sdkerrors.Register(ModuleName, 36, "no redelegation found")
ErrSelfRedelegation = sdkerrors.Register(ModuleName, 37, "cannot redelegate to the same validator")
ErrTinyRedelegationAmount = sdkerrors.Register(ModuleName, 38, "too few tokens to redelegate (truncates to zero tokens)")
ErrBadRedelegationDst = sdkerrors.Register(ModuleName, 39, "redelegation destination validator not found")
ErrTransitiveRedelegation = sdkerrors.Register(ModuleName, 40, "redelegation to this validator already in progress; first redelegation to this validator must complete before next redelegation")
ErrMaxRedelegationEntries = sdkerrors.Register(ModuleName, 41, "too many redelegation entries for (delegator, src-validator, dst-validator) tuple")
ErrDelegatorShareExRateInvalid = sdkerrors.Register(ModuleName, 42, "cannot delegate to validators with invalid (zero) ex-rate")
ErrBothShareMsgsGiven = sdkerrors.Register(ModuleName, 43, "both shares amount and shares percent provided")
ErrNeitherShareMsgsGiven = sdkerrors.Register(ModuleName, 44, "neither shares amount nor shares percent provided")
ErrInvalidHistoricalInfo = sdkerrors.Register(ModuleName, 45, "invalid historical info")
ErrNoHistoricalInfo = sdkerrors.Register(ModuleName, 46, "no historical info found")
ErrEmptyValidatorPubKey = sdkerrors.Register(ModuleName, 47, "empty validator public key")
ErrNoValidatorFound = sdkerrors.Register(ModuleName, 3, "validator does not exist")
ErrValidatorOwnerExists = sdkerrors.Register(ModuleName, 4, "validator already exist for this operator address; must use new validator operator address")
ErrValidatorPubKeyExists = sdkerrors.Register(ModuleName, 5, "validator already exist for this pubkey; must use new validator pubkey")
ErrValidatorPubKeyTypeNotSupported = sdkerrors.Register(ModuleName, 6, "validator pubkey type is not supported")
ErrValidatorJailed = sdkerrors.Register(ModuleName, 7, "validator for this address is currently jailed")
ErrBadRemoveValidator = sdkerrors.Register(ModuleName, 8, "failed to remove validator")
ErrCommissionNegative = sdkerrors.Register(ModuleName, 9, "commission must be positive")
ErrCommissionHuge = sdkerrors.Register(ModuleName, 10, "commission cannot be more than 100%")
ErrCommissionGTMaxRate = sdkerrors.Register(ModuleName, 11, "commission cannot be more than the max rate")
ErrCommissionUpdateTime = sdkerrors.Register(ModuleName, 12, "commission cannot be changed more than once in 24h")
ErrCommissionChangeRateNegative = sdkerrors.Register(ModuleName, 13, "commission change rate must be positive")
ErrCommissionChangeRateGTMaxRate = sdkerrors.Register(ModuleName, 14, "commission change rate cannot be more than the max rate")
ErrCommissionGTMaxChangeRate = sdkerrors.Register(ModuleName, 15, "commission cannot be changed more than max change rate")
ErrSelfDelegationBelowMinimum = sdkerrors.Register(ModuleName, 16, "validator's self delegation must be greater than their minimum self delegation")
ErrMinSelfDelegationDecreased = sdkerrors.Register(ModuleName, 17, "minimum self delegation cannot be decrease")
ErrEmptyDelegatorAddr = sdkerrors.Register(ModuleName, 18, "empty delegator address")
ErrNoDelegation = sdkerrors.Register(ModuleName, 19, "no delegation for (address, validator) tuple")
ErrBadDelegatorAddr = sdkerrors.Register(ModuleName, 20, "delegator does not exist with address")
ErrNoDelegatorForAddress = sdkerrors.Register(ModuleName, 21, "delegator does not contain delegation")
ErrInsufficientShares = sdkerrors.Register(ModuleName, 22, "insufficient delegation shares")
ErrDelegationValidatorEmpty = sdkerrors.Register(ModuleName, 23, "cannot delegate to an empty validator")
ErrNotEnoughDelegationShares = sdkerrors.Register(ModuleName, 24, "not enough delegation shares")
ErrNotMature = sdkerrors.Register(ModuleName, 25, "entry not mature")
ErrNoUnbondingDelegation = sdkerrors.Register(ModuleName, 26, "no unbonding delegation found")
ErrMaxUnbondingDelegationEntries = sdkerrors.Register(ModuleName, 27, "too many unbonding delegation entries for (delegator, validator) tuple")
ErrNoRedelegation = sdkerrors.Register(ModuleName, 28, "no redelegation found")
ErrSelfRedelegation = sdkerrors.Register(ModuleName, 29, "cannot redelegate to the same validator")
ErrTinyRedelegationAmount = sdkerrors.Register(ModuleName, 30, "too few tokens to redelegate (truncates to zero tokens)")
ErrBadRedelegationDst = sdkerrors.Register(ModuleName, 31, "redelegation destination validator not found")
ErrTransitiveRedelegation = sdkerrors.Register(ModuleName, 32, "redelegation to this validator already in progress; first redelegation to this validator must complete before next redelegation")
ErrMaxRedelegationEntries = sdkerrors.Register(ModuleName, 33, "too many redelegation entries for (delegator, src-validator, dst-validator) tuple")
ErrDelegatorShareExRateInvalid = sdkerrors.Register(ModuleName, 34, "cannot delegate to validators with invalid (zero) ex-rate")
ErrBothShareMsgsGiven = sdkerrors.Register(ModuleName, 35, "both shares amount and shares percent provided")
ErrNeitherShareMsgsGiven = sdkerrors.Register(ModuleName, 36, "neither shares amount nor shares percent provided")
ErrInvalidHistoricalInfo = sdkerrors.Register(ModuleName, 37, "invalid historical info")
ErrNoHistoricalInfo = sdkerrors.Register(ModuleName, 38, "no historical info found")
ErrEmptyValidatorPubKey = sdkerrors.Register(ModuleName, 39, "empty validator public key")
)
29 changes: 22 additions & 7 deletions x/staking/types/msg.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,15 @@ func (msg MsgCreateValidator) ValidateBasic() error {
return err
}
if !sdk.AccAddress(valAddr).Equals(delAddr) {
return ErrBadValidatorAddr
return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "validator address is invalid")
}

if msg.Pubkey == nil {
return ErrEmptyValidatorPubKey
}

if !msg.Value.IsValid() || !msg.Value.Amount.IsPositive() {
return ErrBadDelegationAmount
return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "invalid delegation amount")
}

if msg.Description == (Description{}) {
Expand All @@ -130,7 +130,10 @@ func (msg MsgCreateValidator) ValidateBasic() error {
}

if !msg.MinSelfDelegation.IsPositive() {
return ErrMinSelfDelegationInvalid
return sdkerrors.Wrap(
sdkerrors.ErrInvalidRequest,
"minimum self delegation must be a positive integer",
)
}

if msg.Value.Amount.LT(msg.MinSelfDelegation) {
Expand Down Expand Up @@ -189,7 +192,10 @@ func (msg MsgEditValidator) ValidateBasic() error {
}

if msg.MinSelfDelegation != nil && !msg.MinSelfDelegation.IsPositive() {
return ErrMinSelfDelegationInvalid
return sdkerrors.Wrap(
sdkerrors.ErrInvalidRequest,
"minimum self delegation must be a positive integer",
)
}

if msg.CommissionRate != nil {
Expand Down Expand Up @@ -243,7 +249,10 @@ func (msg MsgDelegate) ValidateBasic() error {
}

if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() {
return ErrBadDelegationAmount
return sdkerrors.Wrap(
sdkerrors.ErrInvalidRequest,
"invalid delegation amount",
)
}

return nil
Expand Down Expand Up @@ -298,7 +307,10 @@ func (msg MsgBeginRedelegate) ValidateBasic() error {
}

if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() {
return ErrBadSharesAmount
return sdkerrors.Wrap(
sdkerrors.ErrInvalidRequest,
"invalid shares amount",
)
}

return nil
Expand Down Expand Up @@ -346,7 +358,10 @@ func (msg MsgUndelegate) ValidateBasic() error {
}

if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() {
return ErrBadSharesAmount
return sdkerrors.Wrap(
sdkerrors.ErrInvalidRequest,
"invalid shares amount",
)
}

return nil
Expand Down