From cc2a79ce12dfbb5c4a419051bc31e197a5454958 Mon Sep 17 00:00:00 2001 From: Jonathan Gimeno Date: Thu, 2 Apr 2020 15:23:45 +0200 Subject: [PATCH] x/staking: fix REST calls panic on some method call (#5906) Improve message validation to prevent REST handlers from panicking. --- CHANGELOG.md | 2 ++ x/staking/types/msg.go | 8 ++++---- x/staking/types/msg_test.go | 17 +++++++++++------ 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19203677ad..2bcbedca0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,6 +96,8 @@ to now accept a `codec.JSONMarshaler` for modular serialization of genesis state when method receivers are offline/multisig keys. * (x/auth) [\#5892](https://github.com/cosmos/cosmos-sdk/pull/5892) Add `RegisterKeyTypeCodec` to register new types (eg. keys) to the `auth` module internal amino codec. +* (rest) [\#5906](https://github.com/cosmos/cosmos-sdk/pull/5906) Fix an issue that make some REST calls panic when sending +invalid or incomplete requests. ### State Machine Breaking diff --git a/x/staking/types/msg.go b/x/staking/types/msg.go index 18e0bac2dc..f7fc713c9e 100644 --- a/x/staking/types/msg.go +++ b/x/staking/types/msg.go @@ -81,7 +81,7 @@ func (msg MsgCreateValidator) ValidateBasic() error { if msg.Pubkey == "" { return ErrEmptyValidatorPubKey } - if !msg.Value.Amount.IsPositive() { + if !msg.Value.IsValid() || !msg.Value.Amount.IsPositive() { return ErrBadDelegationAmount } if msg.Description == (Description{}) { @@ -184,7 +184,7 @@ func (msg MsgDelegate) ValidateBasic() error { if msg.ValidatorAddress.Empty() { return ErrEmptyValidatorAddr } - if !msg.Amount.Amount.IsPositive() { + if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() { return ErrBadDelegationAmount } return nil @@ -230,7 +230,7 @@ func (msg MsgBeginRedelegate) ValidateBasic() error { if msg.ValidatorDstAddress.Empty() { return ErrEmptyValidatorAddr } - if !msg.Amount.Amount.IsPositive() { + if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() { return ErrBadSharesAmount } return nil @@ -268,7 +268,7 @@ func (msg MsgUndelegate) ValidateBasic() error { if msg.ValidatorAddress.Empty() { return ErrEmptyValidatorAddr } - if !msg.Amount.Amount.IsPositive() { + if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() { return ErrBadSharesAmount } return nil diff --git a/x/staking/types/msg_test.go b/x/staking/types/msg_test.go index 99866b11ca..127e80549b 100644 --- a/x/staking/types/msg_test.go +++ b/x/staking/types/msg_test.go @@ -34,6 +34,7 @@ func TestMsgCreateValidator(t *testing.T) { {"empty address", "a", "b", "c", "d", "e", commission2, sdk.OneInt(), emptyAddr, pk1, coinPos, false}, {"empty pubkey", "a", "b", "c", "d", "e", commission1, sdk.OneInt(), valAddr1, emptyPubkey, coinPos, false}, {"empty bond", "a", "b", "c", "d", "e", commission2, sdk.OneInt(), valAddr1, pk1, coinZero, false}, + {"nil bond", "a", "b", "c", "d", "e", commission2, sdk.OneInt(), valAddr1, pk1, sdk.Coin{}, false}, {"zero min self delegation", "a", "b", "c", "d", "e", commission1, sdk.ZeroInt(), valAddr1, pk1, coinPos, false}, {"negative min self delegation", "a", "b", "c", "d", "e", commission1, sdk.NewInt(-1), valAddr1, pk1, coinPos, false}, {"delegation less than min self delegation", "a", "b", "c", "d", "e", commission1, coinPos.Amount.Add(sdk.OneInt()), valAddr1, pk1, coinPos, false}, @@ -56,19 +57,20 @@ func TestMsgEditValidator(t *testing.T) { name, moniker, identity, website, securityContact, details string validatorAddr sdk.ValAddress expectPass bool + minSelfDelegation sdk.Int }{ - {"basic good", "a", "b", "c", "d", "e", valAddr1, true}, - {"partial description", "", "", "c", "", "", valAddr1, true}, - {"empty description", "", "", "", "", "", valAddr1, false}, - {"empty address", "a", "b", "c", "d", "e", emptyAddr, false}, + {"basic good", "a", "b", "c", "d", "e", valAddr1, true, sdk.OneInt()}, + {"partial description", "", "", "c", "", "", valAddr1, true, sdk.OneInt()}, + {"empty description", "", "", "", "", "", valAddr1, false, sdk.OneInt()}, + {"empty address", "a", "b", "c", "d", "e", emptyAddr, false, sdk.OneInt()}, + {"nil int", "a", "b", "c", "d", "e", emptyAddr, false, sdk.Int{}}, } for _, tc := range tests { description := NewDescription(tc.moniker, tc.identity, tc.website, tc.securityContact, tc.details) newRate := sdk.ZeroDec() - newMinSelfDelegation := sdk.OneInt() - msg := NewMsgEditValidator(tc.validatorAddr, description, &newRate, &newMinSelfDelegation) + msg := NewMsgEditValidator(tc.validatorAddr, description, &newRate, &tc.minSelfDelegation) if tc.expectPass { require.Nil(t, msg.ValidateBasic(), "test: %v", tc.name) } else { @@ -91,6 +93,7 @@ func TestMsgDelegate(t *testing.T) { {"empty delegator", sdk.AccAddress(emptyAddr), valAddr1, coinPos, false}, {"empty validator", sdk.AccAddress(valAddr1), emptyAddr, coinPos, false}, {"empty bond", sdk.AccAddress(valAddr1), valAddr2, coinZero, false}, + {"nil bold", sdk.AccAddress(valAddr1), valAddr2, sdk.Coin{}, false}, } for _, tc := range tests { @@ -115,6 +118,7 @@ func TestMsgBeginRedelegate(t *testing.T) { }{ {"regular", sdk.AccAddress(valAddr1), valAddr2, valAddr3, sdk.NewInt64Coin(sdk.DefaultBondDenom, 1), true}, {"zero amount", sdk.AccAddress(valAddr1), valAddr2, valAddr3, sdk.NewInt64Coin(sdk.DefaultBondDenom, 0), false}, + {"nil amount", sdk.AccAddress(valAddr1), valAddr2, valAddr3, sdk.Coin{}, false}, {"empty delegator", sdk.AccAddress(emptyAddr), valAddr1, valAddr3, sdk.NewInt64Coin(sdk.DefaultBondDenom, 1), false}, {"empty source validator", sdk.AccAddress(valAddr1), emptyAddr, valAddr3, sdk.NewInt64Coin(sdk.DefaultBondDenom, 1), false}, {"empty destination validator", sdk.AccAddress(valAddr1), valAddr2, emptyAddr, sdk.NewInt64Coin(sdk.DefaultBondDenom, 1), false}, @@ -141,6 +145,7 @@ func TestMsgUndelegate(t *testing.T) { }{ {"regular", sdk.AccAddress(valAddr1), valAddr2, sdk.NewInt64Coin(sdk.DefaultBondDenom, 1), true}, {"zero amount", sdk.AccAddress(valAddr1), valAddr2, sdk.NewInt64Coin(sdk.DefaultBondDenom, 0), false}, + {"nil amount", sdk.AccAddress(valAddr1), valAddr2, sdk.Coin{}, false}, {"empty delegator", sdk.AccAddress(emptyAddr), valAddr1, sdk.NewInt64Coin(sdk.DefaultBondDenom, 1), false}, {"empty validator", sdk.AccAddress(valAddr1), emptyAddr, sdk.NewInt64Coin(sdk.DefaultBondDenom, 1), false}, }