Skip to content

Commit c4a14fa

Browse files
Merge commit from fork
* distr bug * add changelog and release notes
1 parent c1ece30 commit c4a14fa

File tree

4 files changed

+163
-10
lines changed

4 files changed

+163
-10
lines changed

CHANGELOG.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,12 @@ Ref: https://keepachangelog.com/en/1.0.0/
3636

3737
# Changelog
3838

39-
## [Unreleased]
39+
## [v0.53.3](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.53.3) - 2025-07-08
40+
41+
### Bug Fixes
42+
43+
* [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.
44+
4045

4146
## [v0.53.2](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.53.2) - 2025-06-02
4247

RELEASE_NOTES.md

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,15 @@
1-
# Cosmos SDK v0.53.2 Release Notes
2-
3-
💬 [**Release Discussion**](https://github.com/orgs/cosmos/discussions/58)
1+
# Cosmos SDK v0.53.3 Release Notes
42

53
## 🚀 Highlights
64

7-
Announcing Cosmos SDK v0.53.2
8-
9-
This release is a patch update that includes feedback from early users of Cosmos SDK v0.53.0.
5+
This patch release fixes [GHSA-p22h-3m2v-cmgh](https://github.com/cosmos/cosmos-sdk/security/advisories/GHSA-p22h-3m2v-cmgh).
6+
It resolves a `x/distribution` module issue that can halt chains when the historical rewards pool overflows.
7+
Chains using the `x/distribution` module are affected by this issue.
108

11-
Upgrading to this version of the Cosmos SDK from any `v0.53.x` is trivial and does not require a chain upgrade.
9+
We recommended upgrading to this patch release as soon as possible.
1210

13-
NOTE: `v0.53.1` has been retracted.
11+
This patch is state-breaking; chains must perform a coordinated upgrade. This patch cannot be applied in a rolling upgrade.
1412

1513
## 📝 Changelog
1614

17-
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.
15+
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.

tests/integration/distribution/keeper/msg_server_test.go

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
package keeper_test
22

33
import (
4+
"encoding/hex"
45
"fmt"
56
"testing"
67

78
cmtabcitypes "github.com/cometbft/cometbft/abci/types"
89
"github.com/cometbft/cometbft/proto/tendermint/types"
10+
"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
11+
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
12+
minttypes "github.com/cosmos/cosmos-sdk/x/mint/types"
913
"github.com/stretchr/testify/require"
1014
"gotest.tools/v3/assert"
1115

@@ -73,6 +77,7 @@ func initFixture(tb testing.TB) *fixture {
7377

7478
maccPerms := map[string][]string{
7579
distrtypes.ModuleName: {authtypes.Minter},
80+
minttypes.ModuleName: {authtypes.Minter},
7681
stakingtypes.BondedPoolName: {authtypes.Burner, authtypes.Staking},
7782
stakingtypes.NotBondedPoolName: {authtypes.Burner, authtypes.Staking},
7883
}
@@ -133,11 +138,20 @@ func initFixture(tb testing.TB) *fixture {
133138
})
134139

135140
sdkCtx := sdk.UnwrapSDKContext(integrationApp.Context())
141+
require.NoError(tb, stakingKeeper.SetParams(sdkCtx, stakingtypes.DefaultParams()))
142+
143+
stakingKeeper.SetHooks(
144+
stakingtypes.NewMultiStakingHooks(
145+
distrKeeper.Hooks(), // Needed for reward distribution on staking events
146+
),
147+
)
136148

137149
// Register MsgServer and QueryServer
138150
distrtypes.RegisterMsgServer(integrationApp.MsgServiceRouter(), distrkeeper.NewMsgServerImpl(distrKeeper))
139151
distrtypes.RegisterQueryServer(integrationApp.QueryHelper(), distrkeeper.NewQuerier(distrKeeper))
140152

153+
stakingtypes.RegisterMsgServer(integrationApp.MsgServiceRouter(), stakingkeeper.NewMsgServerImpl(stakingKeeper))
154+
141155
return &fixture{
142156
app: integrationApp,
143157
sdkCtx: sdkCtx,
@@ -974,3 +988,108 @@ func TestMsgDepositValidatorRewardsPool(t *testing.T) {
974988
})
975989
}
976990
}
991+
992+
func TestCannotDepositIfRewardPoolFull(t *testing.T) {
993+
f := initFixture(t)
994+
err := f.distrKeeper.FeePool.Set(f.sdkCtx, distrtypes.FeePool{
995+
CommunityPool: sdk.NewDecCoins(sdk.DecCoin{Denom: sdk.DefaultBondDenom, Amount: math.LegacyNewDec(10000)}),
996+
})
997+
assert.NilError(t, err)
998+
assert.NilError(t, f.distrKeeper.Params.Set(f.sdkCtx, distrtypes.DefaultParams()))
999+
_, err = f.distrKeeper.FeePool.Get(f.sdkCtx)
1000+
assert.NilError(t, err)
1001+
1002+
ctx := f.sdkCtx.WithIsCheckTx(false).WithBlockHeight(1)
1003+
populateValidators(t, f)
1004+
1005+
valPubKey := newPubKey("0B485CFC0EECC619440448436F8FC9DF40566F2369E72400281454CB552AFB53")
1006+
operatorAddr := sdk.ValAddress(valPubKey.Address())
1007+
1008+
tstaking := stakingtestutil.NewHelper(t, ctx, f.stakingKeeper)
1009+
1010+
assert.NilError(t, f.bankKeeper.MintCoins(f.sdkCtx, minttypes.ModuleName, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, initAmt))))
1011+
assert.NilError(t, f.bankKeeper.SendCoinsFromModuleToAccount(f.sdkCtx, minttypes.ModuleName, sdk.AccAddress(operatorAddr), sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, initAmt))))
1012+
1013+
tstaking.Commission = stakingtypes.NewCommissionRates(math.LegacyZeroDec(), math.LegacyZeroDec(), math.LegacyZeroDec())
1014+
selfDelegation := math.OneInt()
1015+
tstaking.CreateValidator(operatorAddr, valPubKey, selfDelegation, true)
1016+
1017+
_, err = f.stakingKeeper.EndBlocker(f.sdkCtx)
1018+
assert.NilError(t, err)
1019+
1020+
testDenom := "utesttest"
1021+
maxSupply, ok := math.NewIntFromString("115792089237316195423570985008687907853269984665640564039457584007913129639934")
1022+
assert.Assert(t, ok)
1023+
maxCoins := sdk.NewCoins(sdk.NewCoin(testDenom, maxSupply))
1024+
assert.NilError(t, f.bankKeeper.MintCoins(f.sdkCtx, minttypes.ModuleName, maxCoins))
1025+
assert.NilError(t, f.bankKeeper.SendCoinsFromModuleToAccount(f.sdkCtx, minttypes.ModuleName, sdk.AccAddress(operatorAddr), maxCoins))
1026+
1027+
fundValMsg := &distrtypes.MsgDepositValidatorRewardsPool{
1028+
Depositor: sdk.AccAddress(operatorAddr).String(),
1029+
ValidatorAddress: operatorAddr.String(),
1030+
Amount: maxCoins,
1031+
}
1032+
1033+
// fund the rewards pool. this will set the current rewards.
1034+
_, err = f.app.RunMsg(
1035+
fundValMsg,
1036+
integration.WithAutomaticFinalizeBlock(),
1037+
integration.WithAutomaticCommit(),
1038+
)
1039+
assert.NilError(t, err)
1040+
1041+
// now we delegate to increment the validator period, setting the current rewards to the previous.
1042+
power := int64(1)
1043+
delegationAmount := sdk.TokensFromConsensusPower(power, sdk.DefaultPowerReduction)
1044+
delMsg := stakingtypes.NewMsgDelegate(sdk.AccAddress(operatorAddr).String(), operatorAddr.String(), sdk.NewCoin(sdk.DefaultBondDenom, delegationAmount))
1045+
_, err = f.app.RunMsg(
1046+
delMsg,
1047+
integration.WithAutomaticFinalizeBlock(),
1048+
integration.WithAutomaticCommit(),
1049+
)
1050+
assert.NilError(t, err)
1051+
1052+
// this should fail since this amount cannot be added to the previous amount without overflowing.
1053+
_, err = f.app.RunMsg(
1054+
fundValMsg,
1055+
integration.WithAutomaticFinalizeBlock(),
1056+
integration.WithAutomaticCommit(),
1057+
)
1058+
assert.ErrorContains(t, err, "unable to deposit coins")
1059+
}
1060+
1061+
var (
1062+
pubkeys = []cryptotypes.PubKey{
1063+
newPubKey("0B485CFC0EECC619440448436F8FC9DF40566F2369E72400281454CB552AFB50"),
1064+
newPubKey("0B485CFC0EECC619440448436F8FC9DF40566F2369E72400281454CB552AFB51"),
1065+
newPubKey("0B485CFC0EECC619440448436F8FC9DF40566F2369E72400281454CB552AFB52"),
1066+
}
1067+
1068+
valAddresses = []sdk.ValAddress{
1069+
sdk.ValAddress(pubkeys[0].Address()),
1070+
sdk.ValAddress(pubkeys[1].Address()),
1071+
sdk.ValAddress(pubkeys[2].Address()),
1072+
}
1073+
1074+
initAmt = sdk.TokensFromConsensusPower(1000000, sdk.DefaultPowerReduction)
1075+
initCoins = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, initAmt))
1076+
)
1077+
1078+
func populateValidators(t assert.TestingT, f *fixture) {
1079+
totalSupplyAmt := initAmt.MulRaw(int64(len(valAddresses)))
1080+
totalSupply := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, totalSupplyAmt))
1081+
assert.NilError(t, f.bankKeeper.MintCoins(f.sdkCtx, distrtypes.ModuleName, totalSupply))
1082+
1083+
for _, addr := range valAddresses {
1084+
assert.NilError(t, f.bankKeeper.SendCoinsFromModuleToAccount(f.sdkCtx, distrtypes.ModuleName, (sdk.AccAddress)(addr), initCoins))
1085+
}
1086+
}
1087+
1088+
func newPubKey(pk string) (res cryptotypes.PubKey) {
1089+
pkBytes, err := hex.DecodeString(pk)
1090+
if err != nil {
1091+
panic(err)
1092+
}
1093+
pubkey := &ed25519.PubKey{Key: pkBytes}
1094+
return pubkey
1095+
}

x/distribution/keeper/msg_server.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package keeper
22

33
import (
44
"context"
5+
"fmt"
56

67
"github.com/hashicorp/go-metrics"
78

@@ -208,6 +209,36 @@ func (k msgServer) DepositValidatorRewardsPool(ctx context.Context, msg *types.M
208209
return nil, err
209210
}
210211

212+
// make sure the reward pool isn't already full.
213+
if !validator.GetTokens().IsZero() {
214+
rewards, err := k.GetValidatorCurrentRewards(ctx, valAddr)
215+
if err != nil {
216+
return nil, err
217+
}
218+
current := rewards.Rewards
219+
historical, err := k.GetValidatorHistoricalRewards(ctx, valAddr, rewards.Period-1)
220+
if err != nil {
221+
return nil, err
222+
}
223+
if !historical.CumulativeRewardRatio.IsZero() {
224+
rewardRatio := historical.CumulativeRewardRatio
225+
var panicErr error
226+
func() {
227+
defer func() {
228+
if r := recover(); r != nil {
229+
panicErr = fmt.Errorf("deposit is too large: %v", r)
230+
}
231+
}()
232+
rewardRatio.Add(current...)
233+
}()
234+
235+
// Check if the deferred function caught a panic
236+
if panicErr != nil {
237+
return nil, fmt.Errorf("unable to deposit coins: %w", panicErr)
238+
}
239+
}
240+
}
241+
211242
logger := k.Logger(ctx)
212243
logger.Info(
213244
"transferred from rewards to validator rewards pool",

0 commit comments

Comments
 (0)