fix: ensure withdraw_rewards events are always emitted on reward withdrawal (backport #13323) (#13340)

* fix: ensure withdraw_rewards events are always emitted on reward withdrawal (#13323)

(cherry picked from commit c1c23a7)

# Conflicts:
#	CHANGELOG.md
#	tests/integration/distribution/keeper/delegation_test.go
#	testutil/sims/app_helpers.go
#	x/distribution/keeper/delegation.go
#	x/distribution/keeper/delegation_test.go
#	x/distribution/keeper/keeper.go

* fix conflicts

* move changelog to right place

* fix typo

Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Co-authored-by: Julien Robert <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -37,6 +37,11 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## [Unreleased]
 
+### Improvements
+
+* [#13323](https://github.com/cosmos/cosmos-sdk/pull/13323) Ensure `withdraw_rewards` rewards are emitted from all actions that result in rewards being withdrawn.
+* [#13321](https://github.com/cosmos/cosmos-sdk/pull/13321) Add flag to disable fast node migration and usage. 
+
 ### API Breaking Changes
 
 - (cli) [#13089](https://github.com/cosmos/cosmos-sdk/pull/13089) Fix rollback command don't actually delete multistore versions, added method `RollbackToVersion` to interface `CommitMultiStore` and added method `CommitMultiStore` to `Application` interface.
@@ -51,7 +56,6 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * [#12693](https://github.com/cosmos/cosmos-sdk/pull/12693) Make sure the order of each node is consistent when emitting proto events.
 * (simapp) [#13107](https://github.com/cosmos/cosmos-sdk/pull/13107) Call `SetIAVLCacheSize` with the configured value in simapp.
 * (cli) [#12742](https://github.com/cosmos/cosmos-sdk/pull/12742) Add the `prune` CLI cmd to manually prune app store history versions based on the pruning options.
-* [#13321](https://github.com/cosmos/cosmos-sdk/pull/13321) Add flag to disable fast node migration and usage. 
 
 ### Bug Fixes
 

server/config/config.go

@@ -73,8 +73,8 @@ type BaseConfig struct {
 	// IavlCacheSize set the size of the iavl tree cache.
 	IAVLCacheSize uint64 `mapstructure:"iavl-cache-size"`
 
-	// IAVLDisableFastnNode enables or disables the fast sync node.
-	IAVLDisableFastnNode bool `mapstructure:"iavl-disable-fastnode"`
+	// IAVLDisableFastNode enables or disables the fast sync node.
+	IAVLDisableFastNode bool `mapstructure:"iavl-disable-fastnode"`
 }
 
 // APIConfig defines the API listener configuration.
@@ -207,16 +207,16 @@ func (c *Config) GetMinGasPrices() sdk.DecCoins {
 func DefaultConfig() *Config {
 	return &Config{
 		BaseConfig: BaseConfig{
-			MinGasPrices:         defaultMinGasPrices,
-			InterBlockCache:      true,
-			Pruning:              storetypes.PruningOptionDefault,
-			PruningKeepRecent:    "0",
-			PruningKeepEvery:     "0",
-			PruningInterval:      "0",
-			MinRetainBlocks:      0,
-			IndexEvents:          make([]string, 0),
-			IAVLCacheSize:        781250, // 50 MB
-			IAVLDisableFastnNode: false,
+			MinGasPrices:        defaultMinGasPrices,
+			InterBlockCache:     true,
+			Pruning:             storetypes.PruningOptionDefault,
+			PruningKeepRecent:   "0",
+			PruningKeepEvery:    "0",
+			PruningInterval:     "0",
+			MinRetainBlocks:     0,
+			IndexEvents:         make([]string, 0),
+			IAVLCacheSize:       781250, // 50 MB
+			IAVLDisableFastNode: false,
 		},
 		Telemetry: telemetry.Config{
 			Enabled:      false,
@@ -273,17 +273,17 @@ func GetConfig(v *viper.Viper) (Config, error) {
 
 	return Config{
 		BaseConfig: BaseConfig{
-			MinGasPrices:         v.GetString("minimum-gas-prices"),
-			InterBlockCache:      v.GetBool("inter-block-cache"),
-			Pruning:              v.GetString("pruning"),
-			PruningKeepRecent:    v.GetString("pruning-keep-recent"),
-			PruningInterval:      v.GetString("pruning-interval"),
-			HaltHeight:           v.GetUint64("halt-height"),
-			HaltTime:             v.GetUint64("halt-time"),
-			IndexEvents:          v.GetStringSlice("index-events"),
-			MinRetainBlocks:      v.GetUint64("min-retain-blocks"),
-			IAVLCacheSize:        v.GetUint64("iavl-cache-size"),
-			IAVLDisableFastnNode: v.GetBool("iavl-disable-fastnode"),
+			MinGasPrices:        v.GetString("minimum-gas-prices"),
+			InterBlockCache:     v.GetBool("inter-block-cache"),
+			Pruning:             v.GetString("pruning"),
+			PruningKeepRecent:   v.GetString("pruning-keep-recent"),
+			PruningInterval:     v.GetString("pruning-interval"),
+			HaltHeight:          v.GetUint64("halt-height"),
+			HaltTime:            v.GetUint64("halt-time"),
+			IndexEvents:         v.GetStringSlice("index-events"),
+			MinRetainBlocks:     v.GetUint64("min-retain-blocks"),
+			IAVLCacheSize:       v.GetUint64("iavl-cache-size"),
+			IAVLDisableFastNode: v.GetBool("iavl-disable-fastnode"),
 		},
 		Telemetry: telemetry.Config{
 			ServiceName:             v.GetString("telemetry.service-name"),

server/config/toml.go

@@ -74,9 +74,9 @@ index-events = {{ .BaseConfig.IndexEvents }}
 # Default cache size is 50mb.
 iavl-cache-size = {{ .BaseConfig.IAVLCacheSize }}
 
-# IavlDisableFastnNode enables or disables the fast node feature of IAVL. 
+# IAVLDisableFastNode enables or disables the fast node feature of IAVL. 
 # Default is false.
-iavl-disable-fastnode = {{ .BaseConfig.IAVLDisableFastnNode }}
+iavl-disable-fastnode = {{ .BaseConfig.IAVLDisableFastNode }}
 
 ###############################################################################
 ###                         Telemetry Configuration                         ###

simapp/simd/cmd/root.go

@@ -118,7 +118,7 @@ func initAppConfig() (string, interface{}) {
 	//
 	// In simapp, we set the min gas prices to 0.
 	srvCfg.MinGasPrices = "0stake"
-	// srvCfg.BaseConfig.IAVLDisableFastnNode = true // disable fastnode by default
+	// srvCfg.BaseConfig.IAVLDisableFastNode = true // disable fastnode by default
 
 	customAppConfig := CustomAppConfig{
 		Config: *srvCfg,

x/distribution/keeper/delegation.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
-
 	"github.com/cosmos/cosmos-sdk/x/distribution/types"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 )
@@ -162,13 +161,13 @@ func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val stakingtypes.Vali
 		)
 	}
 
-	// truncate coins, return remainder to community pool
-	coins, remainder := rewards.TruncateDecimal()
+	// truncate reward dec coins, return remainder to community pool
+	finalRewards, remainder := rewards.TruncateDecimal()
 
 	// add coins to user account
-	if !coins.IsZero() {
+	if !finalRewards.IsZero() {
 		withdrawAddr := k.GetDelegatorWithdrawAddr(ctx, del.GetDelegatorAddr())
-		err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, withdrawAddr, coins)
+		err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, withdrawAddr, finalRewards)
 		if err != nil {
 			return nil, err
 		}
@@ -189,5 +188,24 @@ func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val stakingtypes.Vali
 	// remove delegator starting info
 	k.DeleteDelegatorStartingInfo(ctx, del.GetValidatorAddr(), del.GetDelegatorAddr())
 
-	return coins, nil
+	if finalRewards.IsZero() {
+		baseDenom, _ := sdk.GetBaseDenom()
+		if baseDenom == "" {
+			baseDenom = sdk.DefaultBondDenom
+		}
+
+		// Note, we do not call the NewCoins constructor as we do not want the zero
+		// coin removed.
+		finalRewards = sdk.Coins{sdk.NewCoin(baseDenom, sdk.ZeroInt())}
+	}
+
+	ctx.EventManager().EmitEvent(
+		sdk.NewEvent(
+			types.EventTypeWithdrawRewards,
+			sdk.NewAttribute(sdk.AttributeKeyAmount, finalRewards.String()),
+			sdk.NewAttribute(types.AttributeKeyValidator, val.GetOperator().String()),
+		),
+	)
+
+	return finalRewards, nil
 }

x/distribution/keeper/keeper.go

@@ -98,14 +98,6 @@ func (k Keeper) WithdrawDelegationRewards(ctx sdk.Context, delAddr sdk.AccAddres
 		return nil, err
 	}
 
-	ctx.EventManager().EmitEvent(
-		sdk.NewEvent(
-			types.EventTypeWithdrawRewards,
-			sdk.NewAttribute(sdk.AttributeKeyAmount, rewards.String()),
-			sdk.NewAttribute(types.AttributeKeyValidator, valAddr.String()),
-		),
-	)
-
 	// reinitialize the delegation
 	k.initializeDelegation(ctx, valAddr, delAddr)
 	return rewards, nil