Merge PR #1511: Prevent unrevoked validators from unrevoking

* Prevent unrevoked validators from unrevoking * Update changelog * Update previously incorrect test * 'make format'
cosmos · Jul 3, 2018 · 3f438cd · 3f438cd
1 parent d388036
commit 3f438cd
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -80,6 +80,7 @@ IMPROVEMENTS
 * added contributing guidelines
 
 BUG FIXES
+* [x/slashing] \#1510 Unrevoked validators cannot un-revoke themselves
 * [gaia] Added self delegation for validators in the genesis creation
 * [lcd] tests now don't depend on raw json text
 * [stake] error strings lower case

diff --git a/x/slashing/app_test.go b/x/slashing/app_test.go
@@ -106,7 +106,7 @@ func TestSlashingMsgs(t *testing.T) {
 	// no signing info yet
 	checkValidatorSigningInfo(t, mapp, keeper, addr1, false)
 
-	// unrevoke should fail with unknown validator
+	// unrevoke should fail with validator not revoked
 	res := mock.SignCheck(mapp.BaseApp, []sdk.Msg{unrevokeMsg}, []int64{0}, []int64{1}, priv1)
-	require.Equal(t, sdk.ToABCICode(DefaultCodespace, CodeInvalidValidator), res.Code)
+	require.Equal(t, sdk.ToABCICode(DefaultCodespace, CodeValidatorNotRevoked), res.Code)
 }
diff --git a/x/slashing/errors.go b/x/slashing/errors.go
@@ -12,8 +12,9 @@ const (
 	// Default slashing codespace
 	DefaultCodespace sdk.CodespaceType = 10
 
-	CodeInvalidValidator CodeType = 101
-	CodeValidatorJailed  CodeType = 102
+	CodeInvalidValidator    CodeType = 101
+	CodeValidatorJailed     CodeType = 102
+	CodeValidatorNotRevoked CodeType = 103
 )
 
 func ErrNoValidatorForAddress(codespace sdk.CodespaceType) sdk.Error {
@@ -25,3 +26,6 @@ func ErrBadValidatorAddr(codespace sdk.CodespaceType) sdk.Error {
 func ErrValidatorJailed(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeValidatorJailed, "validator jailed, cannot yet be unrevoked")
 }
+func ErrValidatorNotRevoked(codespace sdk.CodespaceType) sdk.Error {
+	return sdk.NewError(codespace, CodeValidatorNotRevoked, "validator not revoked, cannot be unrevoked")
+}
diff --git a/x/slashing/handler.go b/x/slashing/handler.go
@@ -26,6 +26,10 @@ func handleMsgUnrevoke(ctx sdk.Context, msg MsgUnrevoke, k Keeper) sdk.Result {
 		return ErrNoValidatorForAddress(k.codespace).Result()
 	}
 
+	if !validator.GetRevoked() {
+		return ErrValidatorNotRevoked(k.codespace).Result()
+	}
+
 	addr := validator.GetPubKey().Address()
 
 	// Signing info must exist

diff --git a/x/slashing/handler_test.go b/x/slashing/handler_test.go
@@ -0,0 +1,28 @@
+package slashing
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/stake"
+)
+
+func TestCannotUnrevokeUnlessRevoked(t *testing.T) {
+	// initial setup
+	ctx, ck, sk, keeper := createTestInput(t)
+	slh := NewHandler(keeper)
+	amtInt := int64(100)
+	addr, val, amt := addrs[0], pks[0], sdk.NewInt(amtInt)
+	got := stake.NewHandler(sk)(ctx, newTestMsgCreateValidator(addr, val, amt))
+	require.True(t, got.IsOK())
+	stake.EndBlocker(ctx, sk)
+	require.Equal(t, ck.GetCoins(ctx, addr), sdk.Coins{{sk.GetParams(ctx).BondDenom, initCoins.Sub(amt)}})
+	require.True(t, sdk.NewRatFromInt(amt).Equal(sk.Validator(ctx, addr).GetPower()))
+
+	// assert non-revoked validator can't be unrevoked
+	got = slh(ctx, NewMsgUnrevoke(addr))
+	require.False(t, got.IsOK(), "allowed unrevoke of non-revoked validator")
+	require.Equal(t, sdk.ToABCICode(DefaultCodespace, CodeValidatorNotRevoked), got.Code)
+}