diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c0eb54b74..13d326af17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,12 @@ Ref: https://keepachangelog.com/en/1.0.0/ # Changelog -## [Unreleased] +## [v0.53.3](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.53.3) - 2025-07-08 + +### Bug Fixes + +* [GHSA-p22h-3m2v-cmgh](https://github.com/cosmos/cosmos-sdk/security/advisories/GHSA-p22h-3m2v-cmgh) Fix x/distribution can halt when historical rewards overflow. + ## [v0.53.2](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.53.2) - 2025-06-02 diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index f1288c6e46..aa52eba64f 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,17 +1,15 @@ -# Cosmos SDK v0.53.2 Release Notes - -💬 [**Release Discussion**](https://github.com/orgs/cosmos/discussions/58) +# Cosmos SDK v0.53.3 Release Notes ## 🚀 Highlights -Announcing Cosmos SDK v0.53.2 +This patch release fixes [GHSA-p22h-3m2v-cmgh](https://github.com/cosmos/cosmos-sdk/security/advisories/GHSA-p22h-3m2v-cmgh). +It resolves a `x/distribution` module issue that can halt chains when the historical rewards pool overflows. +Chains using the `x/distribution` module are affected by this issue. -This release is a patch update that includes feedback from early users of Cosmos SDK v0.53.0. +We recommended upgrading to this patch release as soon as possible. -Upgrading to this version of the Cosmos SDK from any `v0.53.x` is trivial and does not require a chain upgrade. - -NOTE: `v0.53.1` has been retracted. +This patch is state-breaking; chains must perform a coordinated upgrade. This patch cannot be applied in a rolling upgrade. ## 📝 Changelog -Check out the [changelog](https://github.com/cosmos/cosmos-sdk/blob/v0.53.2/CHANGELOG.md) for an exhaustive list of changes, or [compare changes](https://github.com/cosmos/cosmos-sdk/compare/v0.53.0...v0.53.1) from the last release. +Check out the [changelog](https://github.com/cosmos/cosmos-sdk/blob/v0.53.3/CHANGELOG.md) for an exhaustive list of changes or [compare changes](https://github.com/cosmos/cosmos-sdk/compare/v0.53.2...v0.53.3) from the last release. \ No newline at end of file diff --git a/tests/integration/distribution/keeper/msg_server_test.go b/tests/integration/distribution/keeper/msg_server_test.go index bc0434af88..6ee5fb2a2f 100644 --- a/tests/integration/distribution/keeper/msg_server_test.go +++ b/tests/integration/distribution/keeper/msg_server_test.go @@ -1,11 +1,15 @@ package keeper_test import ( + "encoding/hex" "fmt" "testing" cmtabcitypes "github.com/cometbft/cometbft/abci/types" "github.com/cometbft/cometbft/proto/tendermint/types" + "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + minttypes "github.com/cosmos/cosmos-sdk/x/mint/types" "github.com/stretchr/testify/require" "gotest.tools/v3/assert" @@ -73,6 +77,7 @@ func initFixture(tb testing.TB) *fixture { maccPerms := map[string][]string{ distrtypes.ModuleName: {authtypes.Minter}, + minttypes.ModuleName: {authtypes.Minter}, stakingtypes.BondedPoolName: {authtypes.Burner, authtypes.Staking}, stakingtypes.NotBondedPoolName: {authtypes.Burner, authtypes.Staking}, } @@ -133,11 +138,20 @@ func initFixture(tb testing.TB) *fixture { }) sdkCtx := sdk.UnwrapSDKContext(integrationApp.Context()) + require.NoError(tb, stakingKeeper.SetParams(sdkCtx, stakingtypes.DefaultParams())) + + stakingKeeper.SetHooks( + stakingtypes.NewMultiStakingHooks( + distrKeeper.Hooks(), // Needed for reward distribution on staking events + ), + ) // Register MsgServer and QueryServer distrtypes.RegisterMsgServer(integrationApp.MsgServiceRouter(), distrkeeper.NewMsgServerImpl(distrKeeper)) distrtypes.RegisterQueryServer(integrationApp.QueryHelper(), distrkeeper.NewQuerier(distrKeeper)) + stakingtypes.RegisterMsgServer(integrationApp.MsgServiceRouter(), stakingkeeper.NewMsgServerImpl(stakingKeeper)) + return &fixture{ app: integrationApp, sdkCtx: sdkCtx, @@ -974,3 +988,108 @@ func TestMsgDepositValidatorRewardsPool(t *testing.T) { }) } } + +func TestCannotDepositIfRewardPoolFull(t *testing.T) { + f := initFixture(t) + err := f.distrKeeper.FeePool.Set(f.sdkCtx, distrtypes.FeePool{ + CommunityPool: sdk.NewDecCoins(sdk.DecCoin{Denom: sdk.DefaultBondDenom, Amount: math.LegacyNewDec(10000)}), + }) + assert.NilError(t, err) + assert.NilError(t, f.distrKeeper.Params.Set(f.sdkCtx, distrtypes.DefaultParams())) + _, err = f.distrKeeper.FeePool.Get(f.sdkCtx) + assert.NilError(t, err) + + ctx := f.sdkCtx.WithIsCheckTx(false).WithBlockHeight(1) + populateValidators(t, f) + + valPubKey := newPubKey("0B485CFC0EECC619440448436F8FC9DF40566F2369E72400281454CB552AFB53") + operatorAddr := sdk.ValAddress(valPubKey.Address()) + + tstaking := stakingtestutil.NewHelper(t, ctx, f.stakingKeeper) + + assert.NilError(t, f.bankKeeper.MintCoins(f.sdkCtx, minttypes.ModuleName, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, initAmt)))) + assert.NilError(t, f.bankKeeper.SendCoinsFromModuleToAccount(f.sdkCtx, minttypes.ModuleName, sdk.AccAddress(operatorAddr), sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, initAmt)))) + + tstaking.Commission = stakingtypes.NewCommissionRates(math.LegacyZeroDec(), math.LegacyZeroDec(), math.LegacyZeroDec()) + selfDelegation := math.OneInt() + tstaking.CreateValidator(operatorAddr, valPubKey, selfDelegation, true) + + _, err = f.stakingKeeper.EndBlocker(f.sdkCtx) + assert.NilError(t, err) + + testDenom := "utesttest" + maxSupply, ok := math.NewIntFromString("115792089237316195423570985008687907853269984665640564039457584007913129639934") + assert.Assert(t, ok) + maxCoins := sdk.NewCoins(sdk.NewCoin(testDenom, maxSupply)) + assert.NilError(t, f.bankKeeper.MintCoins(f.sdkCtx, minttypes.ModuleName, maxCoins)) + assert.NilError(t, f.bankKeeper.SendCoinsFromModuleToAccount(f.sdkCtx, minttypes.ModuleName, sdk.AccAddress(operatorAddr), maxCoins)) + + fundValMsg := &distrtypes.MsgDepositValidatorRewardsPool{ + Depositor: sdk.AccAddress(operatorAddr).String(), + ValidatorAddress: operatorAddr.String(), + Amount: maxCoins, + } + + // fund the rewards pool. this will set the current rewards. + _, err = f.app.RunMsg( + fundValMsg, + integration.WithAutomaticFinalizeBlock(), + integration.WithAutomaticCommit(), + ) + assert.NilError(t, err) + + // now we delegate to increment the validator period, setting the current rewards to the previous. + power := int64(1) + delegationAmount := sdk.TokensFromConsensusPower(power, sdk.DefaultPowerReduction) + delMsg := stakingtypes.NewMsgDelegate(sdk.AccAddress(operatorAddr).String(), operatorAddr.String(), sdk.NewCoin(sdk.DefaultBondDenom, delegationAmount)) + _, err = f.app.RunMsg( + delMsg, + integration.WithAutomaticFinalizeBlock(), + integration.WithAutomaticCommit(), + ) + assert.NilError(t, err) + + // this should fail since this amount cannot be added to the previous amount without overflowing. + _, err = f.app.RunMsg( + fundValMsg, + integration.WithAutomaticFinalizeBlock(), + integration.WithAutomaticCommit(), + ) + assert.ErrorContains(t, err, "unable to deposit coins") +} + +var ( + pubkeys = []cryptotypes.PubKey{ + newPubKey("0B485CFC0EECC619440448436F8FC9DF40566F2369E72400281454CB552AFB50"), + newPubKey("0B485CFC0EECC619440448436F8FC9DF40566F2369E72400281454CB552AFB51"), + newPubKey("0B485CFC0EECC619440448436F8FC9DF40566F2369E72400281454CB552AFB52"), + } + + valAddresses = []sdk.ValAddress{ + sdk.ValAddress(pubkeys[0].Address()), + sdk.ValAddress(pubkeys[1].Address()), + sdk.ValAddress(pubkeys[2].Address()), + } + + initAmt = sdk.TokensFromConsensusPower(1000000, sdk.DefaultPowerReduction) + initCoins = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, initAmt)) +) + +func populateValidators(t assert.TestingT, f *fixture) { + totalSupplyAmt := initAmt.MulRaw(int64(len(valAddresses))) + totalSupply := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, totalSupplyAmt)) + assert.NilError(t, f.bankKeeper.MintCoins(f.sdkCtx, distrtypes.ModuleName, totalSupply)) + + for _, addr := range valAddresses { + assert.NilError(t, f.bankKeeper.SendCoinsFromModuleToAccount(f.sdkCtx, distrtypes.ModuleName, (sdk.AccAddress)(addr), initCoins)) + } +} + +func newPubKey(pk string) (res cryptotypes.PubKey) { + pkBytes, err := hex.DecodeString(pk) + if err != nil { + panic(err) + } + pubkey := &ed25519.PubKey{Key: pkBytes} + return pubkey +} diff --git a/x/distribution/keeper/msg_server.go b/x/distribution/keeper/msg_server.go index 5204b282fc..59779d0d2b 100644 --- a/x/distribution/keeper/msg_server.go +++ b/x/distribution/keeper/msg_server.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "fmt" "github.com/hashicorp/go-metrics" @@ -208,6 +209,36 @@ func (k msgServer) DepositValidatorRewardsPool(ctx context.Context, msg *types.M return nil, err } + // make sure the reward pool isn't already full. + if !validator.GetTokens().IsZero() { + rewards, err := k.GetValidatorCurrentRewards(ctx, valAddr) + if err != nil { + return nil, err + } + current := rewards.Rewards + historical, err := k.GetValidatorHistoricalRewards(ctx, valAddr, rewards.Period-1) + if err != nil { + return nil, err + } + if !historical.CumulativeRewardRatio.IsZero() { + rewardRatio := historical.CumulativeRewardRatio + var panicErr error + func() { + defer func() { + if r := recover(); r != nil { + panicErr = fmt.Errorf("deposit is too large: %v", r) + } + }() + rewardRatio.Add(current...) + }() + + // Check if the deferred function caught a panic + if panicErr != nil { + return nil, fmt.Errorf("unable to deposit coins: %w", panicErr) + } + } + } + logger := k.Logger(ctx) logger.Info( "transferred from rewards to validator rewards pool",