Skip to content

Commit

Permalink
Merge pull request #202 from yihuang/release/v0.47.x
Browse files Browse the repository at this point in the history
Merge pull request from GHSA-86h5-xcpx-cfqc
  • Loading branch information
mmsqe authored Feb 28, 2024
2 parents 0f7197f + 7d76e2f commit 9bd1e03
Show file tree
Hide file tree
Showing 5 changed files with 221 additions and 7 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ Ref: https://keepachangelog.com/en/1.0.0/

## [Unreleased]

* (x/staking) Fix a possible bypass of delagator slashing: [GHSA-86h5-xcpx-cfqc](https://github.com/cosmos/cosmos-sdk/security/advisories/GHSA-86h5-xcpx-cfqc)

## [v0.47.9](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.47.9) - 2024-02-19

### Bug Fixes
Expand Down
13 changes: 7 additions & 6 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
# Cosmos SDK v0.47.9 Release Notes
# Cosmos SDK v0.47.10 Release Notes

💬 [**Release Discussion**](https://github.com/orgs/cosmos/discussions/6)

## 🚀 Highlights

This patch release includes a fix in baseapp in `DefaultProposalHandler` and fixes [GHSA-4j93-fm92-rp4m](https://github.com/cosmos/cosmos-sdk/security/advisories/GHSA-4j93-fm92-rp4m).
This early monthly patch release fixes [GHSA-86h5-xcpx-cfqc](https://github.com/cosmos/cosmos-sdk/security/advisories/GHSA-86h5-xcpx-cfqc).

We recommended to upgrade to this patch release as soon as possible.
When upgrading from <= v0.47.8, please ensure that 2/3 of the validator power upgrade to v0.47.9.
When upgrading from <= v0.47.9, please ensure that 2/3 of the validator power upgrade to v0.47.10.

Curious? Check out the [changelog](https://github.com/cosmos/cosmos-sdk/blob/v0.47.9/CHANGELOG.md) for an exhaustive list of changes or [compare changes](https://github.com/cosmos/cosmos-sdk/compare/v0.47.8...v0.47.9) from last release.
Curious? Check out the [changelog](https://github.com/cosmos/cosmos-sdk/blob/v0.47.10/CHANGELOG.md) for an exhaustive list of changes or [compare changes](https://github.com/cosmos/cosmos-sdk/compare/v0.47.9...v0.47.10) from last release.

Refer to the [upgrading guide](https://github.com/cosmos/cosmos-sdk/blob/release/v0.50.x/UPGRADING.md) when migrating from `v0.47.x` to `v0.50.x`.

## Maintenance Policy

v0.50 has been released which means the v0.47.x line is now supported for bug fixes only, as per our release policy.
Start integrating with [Cosmos SDK Eden (v0.50)](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.4) and enjoy and the new features and performance improvements.
v0.50 has been released which means the v0.47.x line is now supported for bug fixes only, as per our release policy. Earlier versions are not maintained.

Start integrating with [Cosmos SDK Eden (v0.50)](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.5) and enjoy and the new features and performance improvements.
15 changes: 15 additions & 0 deletions testutil/sims/app_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,21 @@ func SetupAtGenesis(appConfig depinject.Config, extraOutputs ...interface{}) (*r
return SetupWithConfiguration(appConfig, cfg, extraOutputs...)
}

// NextBlock starts a new block.
func NextBlock(app *runtime.App, 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)
}

// SetupWithConfiguration initializes a new runtime.App. A Nop logger is set in runtime.App.
// appConfig defines the application configuration (f.e. app_config.go).
// extraOutputs defines the extra outputs to be assigned by the dependency injector (depinject).
Expand Down
159 changes: 159 additions & 0 deletions x/slashing/keeper/slash_redelegation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
package keeper_test

import (
"testing"
"time"

"cosmossdk.io/depinject"
"cosmossdk.io/log"
"cosmossdk.io/math"
tmproto "github.com/cometbft/cometbft/proto/tendermint/types"
bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper"
banktestutil "github.com/cosmos/cosmos-sdk/x/bank/testutil"
slashingkeeper "github.com/cosmos/cosmos-sdk/x/slashing/keeper"
"github.com/cosmos/cosmos-sdk/x/slashing/testutil"
stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"

distributionkeeper "github.com/cosmos/cosmos-sdk/x/distribution/keeper"

"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/stretchr/testify/require"

simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
)

func TestSlashRedelegation(t *testing.T) {
// setting up
var stakingKeeper *stakingkeeper.Keeper
var bankKeeper bankkeeper.Keeper
var slashKeeper slashingkeeper.Keeper
var distrKeeper distributionkeeper.Keeper

app, err := simtestutil.Setup(depinject.Configs(
depinject.Supply(log.NewNopLogger()),
testutil.AppConfig,
), &stakingKeeper, &bankKeeper, &slashKeeper, &distrKeeper)
require.NoError(t, err)

// 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)
require.NoError(t, err)

// 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)))
banktestutil.FundAccount(bankKeeper, ctx, testAcc1, testCoins)
banktestutil.FundAccount(bankKeeper, ctx, testAcc2, testCoins)

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())
banktestutil.FundAccount(bankKeeper, ctx, sdk.AccAddress(evilValAddr), testCoins)
createValMsg1, _ := stakingtypes.NewMsgCreateValidator(
evilValAddr, evilValPubKey, testCoins[0], 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())
banktestutil.FundAccount(bankKeeper, ctx, sdk.AccAddress(goodValAddr), testCoins)
createValMsg2, _ := stakingtypes.NewMsgCreateValidator(
goodValAddr, goodValPubKey, testCoins[0], 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, commit height 2, move to height 3
// acc 1 and acc 2 delegate to evil val
ctx = simtestutil.NextBlock(app, ctx, time.Duration(1))
require.NoError(t, err)

// Acc 2 delegate
delMsg := stakingtypes.NewMsgDelegate(testAcc2, evilValAddr, testCoins[0])
_, err = stakingMsgServer.Delegate(ctx, delMsg)
require.NoError(t, err)

// Acc 1 delegate
delMsg = stakingtypes.NewMsgDelegate(testAcc1, evilValAddr, testCoins[0])
_, err = stakingMsgServer.Delegate(ctx, delMsg)
require.NoError(t, err)

// next block, commit height 3, move to height 4
// with the new delegations, evil val increases in voting power and commit byzantine behaviour at height 4 consensus
// at the same time, acc 1 and acc 2 withdraw delegation from evil val
ctx = simtestutil.NextBlock(app, 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(ctx, redelMsg)
require.NoError(t, err)

// Acc 1 undelegate from good val
undelMsg := stakingtypes.NewMsgUndelegate(testAcc1, goodValAddr, testCoins[0])
_, err = stakingMsgServer.Undelegate(ctx, undelMsg)
require.NoError(t, err)

// Acc 2 undelegate from evil val
undelMsg = stakingtypes.NewMsgUndelegate(testAcc2, evilValAddr, testCoins[0])
_, err = stakingMsgServer.Undelegate(ctx, undelMsg)
require.NoError(t, err)

// next block, commit height 4, move to height 5
// Slash evil val for byzantine behaviour at height 4 consensus,
// at which acc 1 and acc 2 still contributed to evil val voting power
// even tho they undelegate at block 4, the valset update is applied after commited block 4 when height 4 consensus already passes
ctx = simtestutil.NextBlock(app, 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, math.LegacyMustNewDecFromStr("0.9"), evilPower, 4)

// 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 = simtestutil.NextBlock(app, ctx, time.Duration(1000000000000000000))
ctx = simtestutil.NextBlock(app, 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(math.NewIntFromUint64(10)).String(), balance1Before.Amount.String())
require.Equal(t, balance2AfterSlashing.Amount.Mul(math.NewIntFromUint64(10)).String(), balance2Before.Amount.String())
}
39 changes: 38 additions & 1 deletion x/staking/keeper/slash.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,9 +260,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 := math.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) && !entry.OnHold():
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
}

Expand Down

0 comments on commit 9bd1e03

Please sign in to comment.