fix: add validation for potential slashing evasion during re-delegati…

…on (#1306)

* Add slash redelegation test

* Add validation

* Update CHANGELOG

* Fix for lint

* Fix for lint

CHANGELOG.md

@@ -65,6 +65,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (types) [\#1299](https://github.com/Finschia/finschia-sdk/pull/1299) add missing nil checks
 * (x/staking) [\#1301](https://github.com/Finschia/finschia-sdk/pull/1301) Use bytes instead of string comparison in delete validator queue (backport cosmos/cosmos-sdk#12303)
 * (x/gov) [\#1304](https://github.com/Finschia/finschia-sdk/pull/1304) fetch a failed proposal tally from proposal.FinalTallyResult in the gprc query
+* (x/staking) [\#1306](https://github.com/Finschia/finschia-sdk/pull/1306) add validation for potential slashing evasion during re-delegation
 
 ### Removed
 

simapp/app.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"time"
 
 	"github.com/gorilla/mux"
 	"github.com/spf13/cast"
@@ -597,6 +598,21 @@ func (app *SimApp) GetTxConfig() client.TxConfig {
 	return MakeTestEncodingConfig().TxConfig
 }
 
+// NextBlock starts a new block.
+func (app *SimApp) NextBlock(ctx sdk.Context, jumpTime time.Duration) sdk.Context {
+	app.EndBlock(abci.RequestEndBlock{Height: ctx.BlockHeight()})
+
+	app.Commit()
+
+	newBlockTime := ctx.BlockTime().Add(jumpTime)
+	nextHeight := ctx.BlockHeight() + 1
+	newHeader := tmproto.Header{Height: nextHeight, Time: newBlockTime}
+
+	app.BeginBlock(abci.RequestBeginBlock{Header: newHeader})
+
+	return app.NewUncachedContext(false, newHeader)
+}
+
 // AppCodec returns SimApp's app codec.
 //
 // NOTE: This is solely to be used for testing purposes as it may be desirable

x/slashing/keeper/slash_redelegation_test.go

@@ -0,0 +1,151 @@
+package keeper_test
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
+
+	"github.com/Finschia/finschia-sdk/crypto/keys/secp256k1"
+	"github.com/Finschia/finschia-sdk/simapp"
+	sdk "github.com/Finschia/finschia-sdk/types"
+	bankkeeper "github.com/Finschia/finschia-sdk/x/bank/keeper"
+	banktestutil "github.com/Finschia/finschia-sdk/x/bank/testutil"
+	distributionkeeper "github.com/Finschia/finschia-sdk/x/distribution/keeper"
+	stakingkeeper "github.com/Finschia/finschia-sdk/x/staking/keeper"
+	stakingtypes "github.com/Finschia/finschia-sdk/x/staking/types"
+)
+
+func TestSlashRedelegation(t *testing.T) {
+	// setting up
+	app := simapp.Setup(false)
+
+	stakingKeeper := app.StakingKeeper
+	bankKeeper := app.BankKeeper
+	slashKeeper := app.SlashingKeeper
+	distrKeeper := app.DistrKeeper
+
+	// get sdk context, staking msg server and bond denom
+	ctx := app.BaseApp.NewContext(false, tmproto.Header{Height: app.LastBlockHeight() + 1})
+	stakingMsgServer := stakingkeeper.NewMsgServerImpl(stakingKeeper)
+	bondDenom := stakingKeeper.BondDenom(ctx)
+
+	// evilVal will be slashed, goodVal won't be slashed
+	evilValPubKey := secp256k1.GenPrivKey().PubKey()
+	goodValPubKey := secp256k1.GenPrivKey().PubKey()
+
+	// both test acc 1 and 2 delegated to evil val, both acc should be slashed when evil val is slashed
+	// test acc 1 use the "undelegation after redelegation" trick (redelegate to good val and then undelegate) to avoid slashing
+	// test acc 2 only undelegate from evil val
+	testAcc1 := sdk.AccAddress([]byte("addr1_______________"))
+	testAcc2 := sdk.AccAddress([]byte("addr2_______________"))
+
+	// fund acc 1 and acc 2
+	testCoins := sdk.NewCoins(sdk.NewCoin(bondDenom, stakingKeeper.TokensFromConsensusPower(ctx, 10)))
+	err := banktestutil.FundAccount(bankKeeper, ctx, testAcc1, testCoins)
+	require.NoError(t, err)
+	err = banktestutil.FundAccount(bankKeeper, ctx, testAcc2, testCoins)
+	require.NoError(t, err)
+
+	balance1Before := bankKeeper.GetBalance(ctx, testAcc1, bondDenom)
+	balance2Before := bankKeeper.GetBalance(ctx, testAcc2, bondDenom)
+
+	// assert acc 1 and acc 2 balance
+	require.Equal(t, balance1Before.Amount.String(), testCoins[0].Amount.String())
+	require.Equal(t, balance2Before.Amount.String(), testCoins[0].Amount.String())
+
+	// creating evil val
+	evilValAddr := sdk.ValAddress(evilValPubKey.Address())
+	err = banktestutil.FundAccount(bankKeeper, ctx, sdk.AccAddress(evilValAddr), testCoins)
+	require.NoError(t, err)
+	createValMsg1, _ := stakingtypes.NewMsgCreateValidator(
+		evilValAddr, evilValPubKey, testCoins[0], stakingtypes.Description{Details: "test"}, stakingtypes.NewCommissionRates(sdk.NewDecWithPrec(5, 1), sdk.NewDecWithPrec(5, 1), sdk.NewDec(0)), sdk.OneInt())
+	_, err = stakingMsgServer.CreateValidator(sdk.WrapSDKContext(ctx), createValMsg1)
+	require.NoError(t, err)
+
+	// creating good val
+	goodValAddr := sdk.ValAddress(goodValPubKey.Address())
+	err = banktestutil.FundAccount(bankKeeper, ctx, sdk.AccAddress(goodValAddr), testCoins)
+	require.NoError(t, err)
+	createValMsg2, _ := stakingtypes.NewMsgCreateValidator(
+		goodValAddr, goodValPubKey, testCoins[0], stakingtypes.Description{Details: "test"}, stakingtypes.NewCommissionRates(sdk.NewDecWithPrec(5, 1), sdk.NewDecWithPrec(5, 1), sdk.NewDec(0)), sdk.OneInt())
+	_, err = stakingMsgServer.CreateValidator(sdk.WrapSDKContext(ctx), createValMsg2)
+	require.NoError(t, err)
+
+	// next block, commit height 1, move to height 2
+	// acc 1 and acc 2 delegate to evil val
+	ctx = app.NextBlock(ctx, time.Duration(1))
+	require.NoError(t, err)
+
+	// Acc 2 delegate
+	delMsg := stakingtypes.NewMsgDelegate(testAcc2, evilValAddr, testCoins[0])
+	_, err = stakingMsgServer.Delegate(sdk.WrapSDKContext(ctx), delMsg)
+	require.NoError(t, err)
+
+	// Acc 1 delegate
+	delMsg = stakingtypes.NewMsgDelegate(testAcc1, evilValAddr, testCoins[0])
+	_, err = stakingMsgServer.Delegate(sdk.WrapSDKContext(ctx), delMsg)
+	require.NoError(t, err)
+
+	// next block, commit height 2, move to height 3
+	// with the new delegations, evil val increases in voting power and commit byzantine behavior at height 3 consensus
+	// at the same time, acc 1 and acc 2 withdraw delegation from evil val
+	ctx = app.NextBlock(ctx, time.Duration(1))
+	require.NoError(t, err)
+
+	evilVal, found := stakingKeeper.GetValidator(ctx, evilValAddr)
+	require.True(t, found)
+
+	evilPower := stakingKeeper.TokensToConsensusPower(ctx, evilVal.Tokens)
+
+	// Acc 1 redelegate from evil val to good val
+	redelMsg := stakingtypes.NewMsgBeginRedelegate(testAcc1, evilValAddr, goodValAddr, testCoins[0])
+	_, err = stakingMsgServer.BeginRedelegate(sdk.WrapSDKContext(ctx), redelMsg)
+	require.NoError(t, err)
+
+	// Acc 1 undelegate from good val
+	undelMsg := stakingtypes.NewMsgUndelegate(testAcc1, goodValAddr, testCoins[0])
+	_, err = stakingMsgServer.Undelegate(sdk.WrapSDKContext(ctx), undelMsg)
+	require.NoError(t, err)
+
+	// Acc 2 undelegate from evil val
+	undelMsg = stakingtypes.NewMsgUndelegate(testAcc2, evilValAddr, testCoins[0])
+	_, err = stakingMsgServer.Undelegate(sdk.WrapSDKContext(ctx), undelMsg)
+	require.NoError(t, err)
+
+	// next block, commit height 3, move to height 4
+	// Slash evil val for byzantine behavior at height 3 consensus,
+	// at which acc 1 and acc 2 still contributed to evil val voting power
+	// even tho they undelegate at block 3, the valset update is applied after committed block 3 when height 3 consensus already passes
+	ctx = app.NextBlock(ctx, time.Duration(1))
+	require.NoError(t, err)
+
+	// slash evil val with slash factor = 0.9, leaving only 10% of stake after slashing
+	evilVal, _ = stakingKeeper.GetValidator(ctx, evilValAddr)
+	evilValConsAddr, err := evilVal.GetConsAddr()
+	require.NoError(t, err)
+
+	slashKeeper.Slash(ctx, evilValConsAddr, sdk.MustNewDecFromStr("0.9"), evilPower, 3)
+
+	// assert invariant to make sure we conduct slashing correctly
+	_, stop := stakingkeeper.AllInvariants(stakingKeeper)(ctx)
+	require.False(t, stop)
+
+	_, stop = bankkeeper.AllInvariants(bankKeeper)(ctx)
+	require.False(t, stop)
+
+	_, stop = distributionkeeper.AllInvariants(distrKeeper)(ctx)
+	require.False(t, stop)
+
+	// one eternity later
+	ctx = app.NextBlock(ctx, time.Duration(1000000000000000000))
+	ctx = app.NextBlock(ctx, time.Duration(1))
+
+	// confirm that account 1 and account 2 has been slashed, and the slash amount is correct
+	balance1AfterSlashing := bankKeeper.GetBalance(ctx, testAcc1, bondDenom)
+	balance2AfterSlashing := bankKeeper.GetBalance(ctx, testAcc2, bondDenom)
+
+	require.Equal(t, balance1AfterSlashing.Amount.Mul(sdk.NewIntFromUint64(10)).String(), balance1Before.Amount.String())
+	require.Equal(t, balance2AfterSlashing.Amount.Mul(sdk.NewIntFromUint64(10)).String(), balance2Before.Amount.String())
+}

x/staking/keeper/slash.go

@@ -248,9 +248,46 @@ func (k Keeper) SlashRedelegation(ctx sdk.Context, srcValidator types.Validator,
 		slashAmount := slashAmountDec.TruncateInt()
 		totalSlashAmount = totalSlashAmount.Add(slashAmount)
 
+		validatorDstAddress, err := sdk.ValAddressFromBech32(redelegation.ValidatorDstAddress)
+		if err != nil {
+			panic(err)
+		}
+		// Handle undelegation after redelegation
+		// Prioritize slashing unbondingDelegation than delegation
+		unbondingDelegation, found := k.GetUnbondingDelegation(ctx, sdk.MustAccAddressFromBech32(redelegation.DelegatorAddress), validatorDstAddress)
+		if found {
+			for i, entry := range unbondingDelegation.Entries {
+				// slash with the amount of `slashAmount` if possible, else slash all unbonding token
+				unbondingSlashAmount := sdk.MinInt(slashAmount, entry.Balance)
+
+				switch {
+				// There's no token to slash
+				case unbondingSlashAmount.IsZero():
+					continue
+				// If unbonding started before this height, stake didn't contribute to infraction
+				case entry.CreationHeight < infractionHeight:
+					continue
+				// Unbonding delegation no longer eligible for slashing, skip it
+				case entry.IsMature(now):
+					continue
+				// Slash the unbonding delegation
+				default:
+					// update remaining slashAmount
+					slashAmount = slashAmount.Sub(unbondingSlashAmount)
+
+					notBondedBurnedAmount = notBondedBurnedAmount.Add(unbondingSlashAmount)
+					entry.Balance = entry.Balance.Sub(unbondingSlashAmount)
+					unbondingDelegation.Entries[i] = entry
+					k.SetUnbondingDelegation(ctx, unbondingDelegation)
+				}
+			}
+		}
+
+		// Slash the moved delegation
+
 		// Unbond from target validator
 		sharesToUnbond := slashFactor.Mul(entry.SharesDst)
-		if sharesToUnbond.IsZero() {
+		if sharesToUnbond.IsZero() || slashAmount.IsZero() {
 			continue
 		}