Merge pull request #1200 from cosmos/cwgoes/slashing-bugfixes

Slashing bugfixes (start height, handler registration)

CHANGELOG.md

@@ -17,6 +17,8 @@ FIXES
 * [lcd] Switch to bech32 for addresses on all human readable inputs and outputs
 * [lcd] fixed tx indexing/querying
 * [cli] Added `--gas` flag to specify transaction gas limit
+* [gaia] Registered slashing message handler
+* [x/slashing] Set signInfo.StartHeight correctly for newly bonded validators
 
 ## 0.18.0
 

cmd/gaia/app/app.go

@@ -82,7 +82,8 @@ func NewGaiaApp(logger log.Logger, db dbm.DB) *GaiaApp {
 	app.Router().
 		AddRoute("bank", bank.NewHandler(app.coinKeeper)).
 		AddRoute("ibc", ibc.NewHandler(app.ibcMapper, app.coinKeeper)).
-		AddRoute("stake", stake.NewHandler(app.stakeKeeper))
+		AddRoute("stake", stake.NewHandler(app.stakeKeeper)).
+		AddRoute("slashing", slashing.NewHandler(app.slashingKeeper))
 
 	// initialize BaseApp
 	app.SetInitChainer(app.initChainer)

x/auth/mock/simulate_block.go

@@ -53,6 +53,13 @@ func GenTx(msg sdk.Msg, accnums []int64, seq []int64, priv ...crypto.PrivKeyEd25
 	return auth.NewStdTx(msg, fee, sigs)
 }
 
+// check a transaction result
+func SignCheck(t *testing.T, app *baseapp.BaseApp, msg sdk.Msg, seq []int64, priv ...crypto.PrivKeyEd25519) sdk.Result {
+	tx := GenTx(msg, seq, priv...)
+	res := app.Check(tx)
+	return res
+}
+
 // simulate a block
 func SignCheckDeliver(t *testing.T, app *baseapp.BaseApp, msg sdk.Msg, accnums []int64, seq []int64, expPass bool, priv ...crypto.PrivKeyEd25519) {
 

x/slashing/app_test.go

@@ -0,0 +1,111 @@
+package slashing
+
+import (
+	"testing"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/auth"
+	"github.com/cosmos/cosmos-sdk/x/auth/mock"
+	"github.com/cosmos/cosmos-sdk/x/bank"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/cosmos/cosmos-sdk/x/stake"
+	abci "github.com/tendermint/abci/types"
+	crypto "github.com/tendermint/go-crypto"
+)
+
+var (
+	priv1 = crypto.GenPrivKeyEd25519()
+	addr1 = priv1.PubKey().Address()
+	coins = sdk.Coins{{"foocoin", 10}}
+)
+
+// initialize the mock application for this module
+func getMockApp(t *testing.T) (*mock.App, stake.Keeper, Keeper) {
+	mapp := mock.NewApp()
+
+	RegisterWire(mapp.Cdc)
+	keyStake := sdk.NewKVStoreKey("stake")
+	keySlashing := sdk.NewKVStoreKey("slashing")
+	coinKeeper := bank.NewKeeper(mapp.AccountMapper)
+	stakeKeeper := stake.NewKeeper(mapp.Cdc, keyStake, coinKeeper, mapp.RegisterCodespace(stake.DefaultCodespace))
+	keeper := NewKeeper(mapp.Cdc, keySlashing, stakeKeeper, mapp.RegisterCodespace(DefaultCodespace))
+	mapp.Router().AddRoute("stake", stake.NewHandler(stakeKeeper))
+	mapp.Router().AddRoute("slashing", NewHandler(keeper))
+
+	mapp.SetEndBlocker(getEndBlocker(stakeKeeper))
+	mapp.SetInitChainer(getInitChainer(mapp, stakeKeeper))
+	mapp.CompleteSetup(t, []*sdk.KVStoreKey{keyStake, keySlashing})
+
+	return mapp, stakeKeeper, keeper
+}
+
+// stake endblocker
+func getEndBlocker(keeper stake.Keeper) sdk.EndBlocker {
+	return func(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock {
+		validatorUpdates := stake.EndBlocker(ctx, keeper)
+		return abci.ResponseEndBlock{
+			ValidatorUpdates: validatorUpdates,
+		}
+	}
+}
+
+// overwrite the mock init chainer
+func getInitChainer(mapp *mock.App, keeper stake.Keeper) sdk.InitChainer {
+	return func(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain {
+		mapp.InitChainer(ctx, req)
+		stake.InitGenesis(ctx, keeper, stake.DefaultGenesisState())
+		return abci.ResponseInitChain{}
+	}
+}
+
+func checkValidator(t *testing.T, mapp *mock.App, keeper stake.Keeper,
+	addr sdk.Address, expFound bool) stake.Validator {
+	ctxCheck := mapp.BaseApp.NewContext(true, abci.Header{})
+	validator, found := keeper.GetValidator(ctxCheck, addr1)
+	assert.Equal(t, expFound, found)
+	return validator
+}
+
+func checkValidatorSigningInfo(t *testing.T, mapp *mock.App, keeper Keeper,
+	addr sdk.Address, expFound bool) ValidatorSigningInfo {
+	ctxCheck := mapp.BaseApp.NewContext(true, abci.Header{})
+	signingInfo, found := keeper.getValidatorSigningInfo(ctxCheck, addr)
+	assert.Equal(t, expFound, found)
+	return signingInfo
+}
+
+func TestSlashingMsgs(t *testing.T) {
+	mapp, stakeKeeper, keeper := getMockApp(t)
+
+	genCoin := sdk.Coin{"steak", 42}
+	bondCoin := sdk.Coin{"steak", 10}
+
+	acc1 := &auth.BaseAccount{
+		Address: addr1,
+		Coins:   sdk.Coins{genCoin},
+	}
+	accs := []auth.Account{acc1}
+	mock.SetGenesis(mapp, accs)
+	description := stake.NewDescription("foo_moniker", "", "", "")
+	createValidatorMsg := stake.NewMsgCreateValidator(
+		addr1, priv1.PubKey(), bondCoin, description,
+	)
+	mock.SignCheckDeliver(t, mapp.BaseApp, createValidatorMsg, []int64{0}, true, priv1)
+	mock.CheckBalance(t, mapp, addr1, sdk.Coins{genCoin.Minus(bondCoin)})
+	mapp.BeginBlock(abci.RequestBeginBlock{})
+
+	validator := checkValidator(t, mapp, stakeKeeper, addr1, true)
+	require.Equal(t, addr1, validator.Owner)
+	require.Equal(t, sdk.Bonded, validator.Status())
+	require.True(sdk.RatEq(t, sdk.NewRat(10), validator.PoolShares.Bonded()))
+	unrevokeMsg := MsgUnrevoke{ValidatorAddr: validator.PubKey.Address()}
+
+	// no signing info yet
+	checkValidatorSigningInfo(t, mapp, keeper, addr1, false)
+
+	// unrevoke should fail with unknown validator
+	res := mock.SignCheck(t, mapp.BaseApp, unrevokeMsg, []int64{1}, priv1)
+	require.Equal(t, sdk.ToABCICode(DefaultCodespace, CodeInvalidValidator), res.Code)
+}

x/slashing/keeper.go

@@ -55,8 +55,12 @@ func (k Keeper) handleValidatorSignature(ctx sdk.Context, pubkey crypto.PubKey,
 	address := pubkey.Address()
 
 	// Local index, so counts blocks validator *should* have signed
-	// Will use the 0-value default signing info if not present
-	signInfo, _ := k.getValidatorSigningInfo(ctx, address)
+	// Will use the 0-value default signing info if not present, except for start height
+	signInfo, found := k.getValidatorSigningInfo(ctx, address)
+	if !found {
+		// If this validator has never been seen before, construct a new SigningInfo with the correct start height
+		signInfo = NewValidatorSigningInfo(height, 0, 0, 0)
+	}
 	index := signInfo.IndexOffset % SignedBlocksWindow
 	signInfo.IndexOffset++
 

x/slashing/keeper_test.go

@@ -11,6 +11,8 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/stake"
 )
 
+// Test that a validator is slashed correctly
+// when we discover evidence of equivocation
 func TestHandleDoubleSign(t *testing.T) {
 
 	// initial setup
@@ -32,6 +34,8 @@ func TestHandleDoubleSign(t *testing.T) {
 	require.Equal(t, sdk.NewRat(amt).Mul(sdk.NewRat(19).Quo(sdk.NewRat(20))), sk.Validator(ctx, addr).GetPower())
 }
 
+// Test a validator through uptime, downtime, revocation,
+// unrevocation, starting height reset, and revocation again
 func TestHandleAbsentValidator(t *testing.T) {
 
 	// initial setup
@@ -129,3 +133,39 @@ func TestHandleAbsentValidator(t *testing.T) {
 	validator, _ = sk.GetValidatorByPubKey(ctx, val)
 	require.Equal(t, sdk.Unbonded, validator.GetStatus())
 }
+
+// Test a new validator entering the validator set
+// Ensure that SigningInfo.StartHeight is set correctly
+// and that they are not immediately revoked
+func TestHandleNewValidator(t *testing.T) {
+	// initial setup
+	ctx, ck, sk, keeper := createTestInput(t)
+	addr, val, amt := addrs[0], pks[0], int64(100)
+	sh := stake.NewHandler(sk)
+	got := sh(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 - amt}})
+	require.Equal(t, sdk.NewRat(amt), sk.Validator(ctx, addr).GetPower())
+
+	// 1000 first blocks not a validator
+	ctx = ctx.WithBlockHeight(1001)
+
+	// Now a validator, for two blocks
+	keeper.handleValidatorSignature(ctx, val, true)
+	ctx = ctx.WithBlockHeight(1002)
+	keeper.handleValidatorSignature(ctx, val, false)
+
+	info, found := keeper.getValidatorSigningInfo(ctx, val.Address())
+	require.True(t, found)
+	require.Equal(t, int64(1001), info.StartHeight)
+	require.Equal(t, int64(2), info.IndexOffset)
+	require.Equal(t, int64(1), info.SignedBlocksCounter)
+	require.Equal(t, int64(0), info.JailedUntil)
+
+	// validator should be bonded still, should not have been revoked or slashed
+	validator, _ := sk.GetValidatorByPubKey(ctx, val)
+	require.Equal(t, sdk.Bonded, validator.GetStatus())
+	pool := sk.GetPool(ctx)
+	require.Equal(t, int64(100), pool.BondedTokens)
+}

x/slashing/signing_info.go

@@ -47,6 +47,16 @@ func (k Keeper) setValidatorSigningBitArray(ctx sdk.Context, address sdk.Address
 	store.Set(GetValidatorSigningBitArrayKey(address, index), bz)
 }
 
+// Construct a new `ValidatorSigningInfo` struct
+func NewValidatorSigningInfo(startHeight int64, indexOffset int64, jailedUntil int64, signedBlocksCounter int64) ValidatorSigningInfo {
+	return ValidatorSigningInfo{
+		StartHeight:         startHeight,
+		IndexOffset:         indexOffset,
+		JailedUntil:         jailedUntil,
+		SignedBlocksCounter: signedBlocksCounter,
+	}
+}
+
 // Signing info for a validator
 type ValidatorSigningInfo struct {
 	StartHeight         int64 `json:"start_height"`          // height at which validator was first a candidate OR was unrevoked