perf(x/staking/keeper): make Slash return early iff zero tokens to burn

This change makes Keeper.Slash return early if there are no tokens
to burn! This change also skips invoking the
.Hooks().BeforeValidatorSlashed hook because literally there is no
slashing that can happen if there are no tokens to burn.

Given that the result of burning zero tokens SHOULD be a no-operation
(noop) but we go through the whole routine, assuming no zero tokens
would be burnt, one could argue on a protocol implementation/conformation
that with zero tokens to burn, invoking Keeper.RemoveValidatorTokens which
invokes:

  Keeper.DeleteValidatorByPowerIndex
    -> (Keeper.DeleteValidatorByPowerIndex)
    -> validator.RemoveTokens
    -> Keeper.SetValidator
    -> Keeper.SetValidatorByPowerIndex

was causing a potential self inflicted Byzantine risk because that operation
is non-atomic (it ran through a series of operations that could fail on
their own yet could not be rolled back and not idempotent), will rely
on network operations yet the actual result will have the operations back
to the original: more investigation is needed to examine the risk/impact
of previously letting zero tokens be burnt and run through that process.

Also while here, employed a Go pattern to reuse condition variables just
as a code hygiene improvement and also given that as a Go reviewer, the
unnecessary allocation of variables however small must be avoided.

Fixes #18034

CHANGELOG.md

@@ -51,6 +51,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 
+* (x/staking/keeper) [#]18049(https://github.com/cosmos/cosmos-sdk/pull/18049) return early if Slash encounters zero tokens to burn.
 * (x/staking/keeper) [#18035](https://github.com/cosmos/cosmos-sdk/pull/18035) Hoisted out of the redelegation loop, the non-changing validator and delegator addresses parsing.
 * (keyring) [#17913](https://github.com/cosmos/cosmos-sdk/pull/17913) Add `NewAutoCLIKeyring` for creating an AutoCLI keyring from a SDK keyring.
 * (codec) [#17913](https://github.com/cosmos/cosmos-sdk/pull/17913) `codectypes.NewAnyWithValue` supports proto v2 messages.

x/staking/keeper/slash.go

@@ -147,12 +147,26 @@ func (k Keeper) Slash(ctx context.Context, consAddr sdk.ConsAddress, infractionH
 	tokensToBurn := math.MinInt(remainingSlashAmount, validator.Tokens)
 	tokensToBurn = math.MaxInt(tokensToBurn, math.ZeroInt()) // defensive.
 
+	if tokensToBurn.IsZero() {
+		// Nothing to burn, we can end this route immediately! We also don't
+		// need to call the k.Hooks().BeforeValidatorSlashed hook as we won't
+		// be slashing at all.
+		logger.Info(
+			"no validator slashing because slash amount is zero",
+			"validator", validator.GetOperator(),
+			"slash_factor", slashFactor.String(),
+			"burned", tokensToBurn,
+			"validatorTokens", validator.Tokens,
+		)
+		return math.NewInt(0), nil
+	}
+
 	// we need to calculate the *effective* slash fraction for distribution
 	if validator.Tokens.IsPositive() {
 		effectiveFraction := math.LegacyNewDecFromInt(tokensToBurn).QuoRoundUp(math.LegacyNewDecFromInt(validator.Tokens))
 		// possible if power has changed
-		if effectiveFraction.GT(math.LegacyOneDec()) {
-			effectiveFraction = math.LegacyOneDec()
+		if oneDec := math.LegacyOneDec(); effectiveFraction.GT(oneDec) {
+			effectiveFraction = oneDec
 		}
 		// call the before-slashed hook
 		if err := k.Hooks().BeforeValidatorSlashed(ctx, operatorAddress, effectiveFraction); err != nil {