LaconicNetwork/cosmos-sdk
7
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

fix(x/slashing): Avoid overslashing on redelegation + unbonding in specific situations (#20688)

Browse Source
This commit is contained in:
Facundo Medica 2024-06-24 12:33:46 +02:00 committed by GitHub
parent ab1433bf16
commit 720c1086cb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 215 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

214
tests/integration/slashing/keeper/slash_redelegation_test.go
View File

@ -2,7 +2,6 @@ package keeper_test
import (
"context"
"fmt"
"testing"
"time"
@ -66,30 +65,30 @@ func TestSlashRedelegation(t *testing.T) {
testAcc2 := sdk.AccAddress([]byte("addr2_______________"))
// fund acc 1 and acc 2
testCoins := sdk.NewCoins(sdk.NewCoin(bondDenom, stakingKeeper.TokensFromConsensusPower(ctx, 10)))
fundAccount(t, ctx, bankKeeper, authKeeper, testAcc1, testCoins)
fundAccount(t, ctx, bankKeeper, authKeeper, testAcc2, testCoins)
testCoin := sdk.NewCoin(bondDenom, stakingKeeper.TokensFromConsensusPower(ctx, 10))
fundAccount(t, ctx, bankKeeper, authKeeper, testAcc1, testCoin)
fundAccount(t, ctx, bankKeeper, authKeeper, testAcc2, testCoin)
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())
require.Equal(t, balance1Before.Amount.String(), testCoin.Amount.String())
require.Equal(t, balance2Before.Amount.String(), testCoin.Amount.String())
// creating evil val
evilValAddr := sdk.ValAddress(evilValPubKey.Address())
fundAccount(t, ctx, bankKeeper, authKeeper, sdk.AccAddress(evilValAddr), testCoins)
fundAccount(t, ctx, bankKeeper, authKeeper, sdk.AccAddress(evilValAddr), testCoin)
createValMsg1, _ := stakingtypes.NewMsgCreateValidator(
evilValAddr.String(), evilValPubKey, testCoins[0], stakingtypes.Description{Details: "test"}, stakingtypes.NewCommissionRates(math.LegacyNewDecWithPrec(5, 1), math.LegacyNewDecWithPrec(5, 1), math.LegacyNewDec(0)), math.OneInt())
evilValAddr.String(), evilValPubKey, testCoin, stakingtypes.Description{Details: "test"}, stakingtypes.NewCommissionRates(math.LegacyNewDecWithPrec(5, 1), math.LegacyNewDecWithPrec(5, 1), math.LegacyNewDec(0)), math.OneInt())
_, err = stakingMsgServer.CreateValidator(ctx, createValMsg1)
require.NoError(t, err)
// creating good val
goodValAddr := sdk.ValAddress(goodValPubKey.Address())
fundAccount(t, ctx, bankKeeper, authKeeper, sdk.AccAddress(goodValAddr), testCoins)
fundAccount(t, ctx, bankKeeper, authKeeper, sdk.AccAddress(goodValAddr), testCoin)
createValMsg2, _ := stakingtypes.NewMsgCreateValidator(
goodValAddr.String(), goodValPubKey, testCoins[0], stakingtypes.Description{Details: "test"}, stakingtypes.NewCommissionRates(math.LegacyNewDecWithPrec(5, 1), math.LegacyNewDecWithPrec(5, 1), math.LegacyNewDec(0)), math.OneInt())
goodValAddr.String(), goodValPubKey, testCoin, stakingtypes.Description{Details: "test"}, stakingtypes.NewCommissionRates(math.LegacyNewDecWithPrec(5, 1), math.LegacyNewDecWithPrec(5, 1), math.LegacyNewDec(0)), math.OneInt())
_, err = stakingMsgServer.CreateValidator(ctx, createValMsg2)
require.NoError(t, err)
@ -100,12 +99,12 @@ func TestSlashRedelegation(t *testing.T) {
require.NoError(t, err)
// Acc 2 delegate
delMsg := stakingtypes.NewMsgDelegate(testAcc2.String(), evilValAddr.String(), testCoins[0])
delMsg := stakingtypes.NewMsgDelegate(testAcc2.String(), evilValAddr.String(), testCoin)
_, err = stakingMsgServer.Delegate(ctx, delMsg)
require.NoError(t, err)
// Acc 1 delegate
delMsg = stakingtypes.NewMsgDelegate(testAcc1.String(), evilValAddr.String(), testCoins[0])
delMsg = stakingtypes.NewMsgDelegate(testAcc1.String(), evilValAddr.String(), testCoin)
_, err = stakingMsgServer.Delegate(ctx, delMsg)
require.NoError(t, err)
@ -119,20 +118,19 @@ func TestSlashRedelegation(t *testing.T) {
require.NoError(t, err)
evilPower := stakingKeeper.TokensToConsensusPower(ctx, evilVal.Tokens)
fmt.Println(evilPower)
// Acc 1 redelegate from evil val to good val
redelMsg := stakingtypes.NewMsgBeginRedelegate(testAcc1.String(), evilValAddr.String(), goodValAddr.String(), testCoins[0])
redelMsg := stakingtypes.NewMsgBeginRedelegate(testAcc1.String(), evilValAddr.String(), goodValAddr.String(), testCoin)
_, err = stakingMsgServer.BeginRedelegate(ctx, redelMsg)
require.NoError(t, err)
// Acc 1 undelegate from good val
undelMsg := stakingtypes.NewMsgUndelegate(testAcc1.String(), goodValAddr.String(), testCoins[0])
undelMsg := stakingtypes.NewMsgUndelegate(testAcc1.String(), goodValAddr.String(), testCoin)
_, err = stakingMsgServer.Undelegate(ctx, undelMsg)
require.NoError(t, err)
// Acc 2 undelegate from evil val
undelMsg = stakingtypes.NewMsgUndelegate(testAcc2.String(), evilValAddr.String(), testCoins[0])
undelMsg = stakingtypes.NewMsgUndelegate(testAcc2.String(), evilValAddr.String(), testCoin)
_, err = stakingMsgServer.Undelegate(ctx, undelMsg)
require.NoError(t, err)
@ -175,7 +173,7 @@ func TestSlashRedelegation(t *testing.T) {
require.Equal(t, balance2AfterSlashing.Amount.Mul(math.NewIntFromUint64(10)).String(), balance2Before.Amount.String())
}
func fundAccount(t *testing.T, ctx context.Context, bankKeeper bankkeeper.Keeper, authKeeper authkeeper.AccountKeeper, addr sdk.AccAddress, amount sdk.Coins) {
func fundAccount(t *testing.T, ctx context.Context, bankKeeper bankkeeper.Keeper, authKeeper authkeeper.AccountKeeper, addr sdk.AccAddress, amount ...sdk.Coin) {
t.Helper()
if authKeeper.GetAccount(ctx, addr) == nil {
@ -185,3 +183,185 @@ func fundAccount(t *testing.T, ctx context.Context, bankKeeper bankkeeper.Keeper
require.NoError(t, banktestutil.FundAccount(ctx, bankKeeper, addr, amount))
}
func TestOverSlashing(t *testing.T) {
// slash penalty percentage
slashFraction := "0.45"
// percentage of (undelegation/(undelegation + redelegation))
undelegationPercentageStr := "0.30"
// setting up
var (
stakingKeeper *stakingkeeper.Keeper
bankKeeper bankkeeper.Keeper
slashKeeper slashingkeeper.Keeper
distrKeeper distributionkeeper.Keeper
authKeeper authkeeper.AccountKeeper
)
app, err := simtestutil.Setup(depinject.Configs(
depinject.Supply(log.NewNopLogger()),
slashing.AppConfig,
), &stakingKeeper, &bankKeeper, &slashKeeper, &distrKeeper, &authKeeper)
require.NoError(t, err)
// get sdk context, staking msg server and bond denom
ctx := app.BaseApp.NewContext(false)
stakingMsgServer := stakingkeeper.NewMsgServerImpl(stakingKeeper)
bondDenom, err := stakingKeeper.BondDenom(ctx)
require.NoError(t, err)
// evilVal will be slashed, goodVal won't be slashed
evilValPubKey := secp256k1.GenPrivKey().PubKey()
goodValPubKey := secp256k1.GenPrivKey().PubKey()
/*
all test accs will delegate to evil val, which evil validator will eventually be slashed
- test acc 1: redelegate -> undelegate full amount
- test acc 2: simple undelegation. intended scenario.
- test acc 3: redelegate -> undelegate some amount
*/
testAcc1 := sdk.AccAddress([]byte("addr1new____________"))
testAcc2 := sdk.AccAddress([]byte("addr2new____________"))
testAcc3 := sdk.AccAddress([]byte("addr3new____________"))
// fund all accounts
testCoin := sdk.NewCoin(bondDenom, math.NewInt(1_000_000))
fundAccount(t, ctx, bankKeeper, authKeeper, testAcc1, testCoin)
fundAccount(t, ctx, bankKeeper, authKeeper, testAcc2, testCoin)
fundAccount(t, ctx, bankKeeper, authKeeper, testAcc3, testCoin)
balance1Before := bankKeeper.GetBalance(ctx, testAcc1, bondDenom)
balance2Before := bankKeeper.GetBalance(ctx, testAcc2, bondDenom)
balance3Before := bankKeeper.GetBalance(ctx, testAcc3, bondDenom)
// assert acc 1, 2 and 3 balance
require.Equal(t, testCoin.Amount.String(), balance1Before.Amount.String())
require.Equal(t, testCoin.Amount.String(), balance2Before.Amount.String())
require.Equal(t, testCoin.Amount.String(), balance3Before.Amount.String())
// create evil val
evilValAddr := sdk.ValAddress(evilValPubKey.Address())
fundAccount(t, ctx, bankKeeper, authKeeper, sdk.AccAddress(evilValAddr), testCoin)
createValMsg1, _ := stakingtypes.NewMsgCreateValidator(
evilValAddr.String(), evilValPubKey, testCoin, stakingtypes.Description{Details: "test"}, stakingtypes.NewCommissionRates(math.LegacyNewDecWithPrec(5, 1), math.LegacyNewDecWithPrec(5, 1), math.LegacyNewDec(0)), math.OneInt())
_, err = stakingMsgServer.CreateValidator(ctx, createValMsg1)
require.NoError(t, err)
// create good val 1
goodValAddr := sdk.ValAddress(goodValPubKey.Address())
fundAccount(t, ctx, bankKeeper, authKeeper, sdk.AccAddress(goodValAddr), testCoin)
createValMsg2, _ := stakingtypes.NewMsgCreateValidator(
goodValAddr.String(), goodValPubKey, testCoin, stakingtypes.Description{Details: "test"}, stakingtypes.NewCommissionRates(math.LegacyNewDecWithPrec(5, 1), math.LegacyNewDecWithPrec(5, 1), math.LegacyNewDec(0)), math.OneInt())
_, err = stakingMsgServer.CreateValidator(ctx, createValMsg2)
require.NoError(t, err)
// next block
ctx = ctx.WithBlockHeight(app.LastBlockHeight() + 1).WithHeaderInfo(header.Info{Height: app.LastBlockHeight() + 1})
ctx, err = simtestutil.NextBlock(app, ctx, time.Duration(1))
require.NoError(t, err)
// delegate all accs to evil val
delMsg := stakingtypes.NewMsgDelegate(testAcc1.String(), evilValAddr.String(), testCoin)
_, err = stakingMsgServer.Delegate(ctx, delMsg)
require.NoError(t, err)
delMsg = stakingtypes.NewMsgDelegate(testAcc2.String(), evilValAddr.String(), testCoin)
_, err = stakingMsgServer.Delegate(ctx, delMsg)
require.NoError(t, err)
delMsg = stakingtypes.NewMsgDelegate(testAcc3.String(), evilValAddr.String(), testCoin)
_, err = stakingMsgServer.Delegate(ctx, delMsg)
require.NoError(t, err)
// next block
ctx, err = simtestutil.NextBlock(app, ctx, time.Duration(1))
require.NoError(t, err)
// evilValAddr done something bad
misbehaveHeight := ctx.BlockHeader().Height
evilVal, err := stakingKeeper.GetValidator(ctx, evilValAddr)
require.NoError(t, err)
evilValConsAddr, err := evilVal.GetConsAddr()
require.NoError(t, err)
evilPower := stakingKeeper.TokensToConsensusPower(ctx, evilVal.Tokens)
// next block
ctx, err = simtestutil.NextBlock(app, ctx, time.Duration(1))
require.NoError(t, err)
// acc 1: redelegate to goodval1 and undelegate FULL amount
redelMsg := stakingtypes.NewMsgBeginRedelegate(testAcc1.String(), evilValAddr.String(), goodValAddr.String(), testCoin)
_, err = stakingMsgServer.BeginRedelegate(ctx, redelMsg)
require.NoError(t, err)
undelMsg := stakingtypes.NewMsgUndelegate(testAcc1.String(), goodValAddr.String(), testCoin)
_, err = stakingMsgServer.Undelegate(ctx, undelMsg)
require.NoError(t, err)
// acc 2: undelegate full amount
undelMsg = stakingtypes.NewMsgUndelegate(testAcc2.String(), evilValAddr.String(), testCoin)
_, err = stakingMsgServer.Undelegate(ctx, undelMsg)
require.NoError(t, err)
// acc 3: redelegate to goodval1 and undelegate some amount
redelMsg = stakingtypes.NewMsgBeginRedelegate(testAcc3.String(), evilValAddr.String(), goodValAddr.String(), testCoin)
_, err = stakingMsgServer.BeginRedelegate(ctx, redelMsg)
require.NoError(t, err)
undelegationPercentage := math.LegacyMustNewDecFromStr(undelegationPercentageStr)
undelegationAmountDec := math.LegacyNewDecFromInt(testCoin.Amount).Mul(undelegationPercentage)
amountToUndelegate := undelegationAmountDec.TruncateInt()
// next block
ctx, err = simtestutil.NextBlock(app, ctx, time.Duration(1))
require.NoError(t, err)
portionofTestCoins := sdk.NewCoin(bondDenom, amountToUndelegate)
undelMsg = stakingtypes.NewMsgUndelegate(testAcc3.String(), goodValAddr.String(), portionofTestCoins)
_, err = stakingMsgServer.Undelegate(ctx, undelMsg)
require.NoError(t, err)
// next block
ctx, err = simtestutil.NextBlock(app, ctx, time.Duration(1))
require.NoError(t, err)
// slash the evil val
err = slashKeeper.Slash(ctx, evilValConsAddr, math.LegacyMustNewDecFromStr(slashFraction), evilPower, misbehaveHeight)
require.NoError(t, err)
// assert invariants
_, 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)
// fastforward 2 blocks to complete redelegations and unbondings
for i := 0; i < 2; i++ {
ctx, err = simtestutil.NextBlock(app, ctx, time.Duration(1000000000000000000))
require.NoError(t, err)
}
// we check all accounts should be slashed with the equal amount, and they should end up with same balance including staked amount
stakedAcc1, err := stakingKeeper.GetDelegatorBonded(ctx, testAcc1)
require.NoError(t, err)
stakedAcc2, err := stakingKeeper.GetDelegatorBonded(ctx, testAcc2)
require.NoError(t, err)
stakedAcc3, err := stakingKeeper.GetDelegatorBonded(ctx, testAcc3)
require.NoError(t, err)
balance1AfterSlashing := bankKeeper.GetBalance(ctx, testAcc1, bondDenom).Add(sdk.NewCoin(bondDenom, stakedAcc1))
balance2AfterSlashing := bankKeeper.GetBalance(ctx, testAcc2, bondDenom).Add(sdk.NewCoin(bondDenom, stakedAcc2))
balance3AfterSlashing := bankKeeper.GetBalance(ctx, testAcc3, bondDenom).Add(sdk.NewCoin(bondDenom, stakedAcc3))
require.Equal(t, "550000stake", balance1AfterSlashing.String())
require.Equal(t, "550000stake", balance2AfterSlashing.String())
require.Equal(t, "550000stake", balance3AfterSlashing.String())
}

4
x/staking/CHANGELOG.md
View File

@ -25,6 +25,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
## [Unreleased]
### Bug Fixes
* [#20688](https://github.com/cosmos/cosmos-sdk/pull/20688) Avoid overslashing unbonding delegations after a redelegation.
### Features
* [#19537](https://github.com/cosmos/cosmos-sdk/pull/19537) Changing `MinCommissionRate` in `MsgUpdateParams` now updates the minimum commission rate for all validators.

16
x/staking/keeper/slash.go
View File

@ -365,8 +365,20 @@ func (k Keeper) SlashRedelegation(ctx context.Context, srcValidator types.Valida
// Slash the moved delegation
// Unbond from target validator
sharesToUnbond := slashFactor.Mul(entry.SharesDst)
if sharesToUnbond.IsZero() || slashAmount.IsZero() {
if slashAmount.IsZero() {
continue
}
dstVal, err := k.GetValidator(ctx, valDstAddr)
if err != nil {
return math.ZeroInt(), err
}
sharesToUnbond, err := dstVal.SharesFromTokensTruncated(slashAmount)
if err != nil {
return math.ZeroInt(), err
}
if sharesToUnbond.IsZero() {
continue
}