chore(x/gov): cap SensitivityTargetDistance for dynamic deposit

x/gov/keeper/min_deposit.go

@@ -78,6 +78,8 @@ func (keeper Keeper) GetLastMinDeposit(ctx sdk.Context) (sdk.Coins, time.Time) {
 
 // GetMinDeposit returns the (dynamic) minimum deposit currently required for a proposal
 func (keeper Keeper) GetMinDeposit(ctx sdk.Context) sdk.Coins {
+	logger := keeper.Logger(ctx)
+
 	params := keeper.GetParams(ctx)
 	minDepositFloor := sdk.Coins(params.MinDepositThrottler.FloorValue)
 	tick := params.MinDepositThrottler.UpdatePeriod
@@ -99,10 +101,20 @@ func (keeper Keeper) GetMinDeposit(ctx sdk.Context) sdk.Coins {
 	distance := numActiveProposals.Sub(targetActiveProposals)
 	percChange := math.LegacyOneDec()
 	if !distance.IsZero() {
+		// ApproxRoot is here being called on a relatively small positive
+		// integer (even if distance < 0, ApproxRoot will return
+		// `|distance|.ApproxRoot(k) * -1`) with a value of k expected to also
+		// be relatively small (<= 100).
+		// This is a safe operation and should not error.
 		b, err := distance.ToLegacyDec().ApproxRoot(k)
 		if err != nil {
 			// in case of error bypass the sensitivity, i.e. assume k = 1
 			b = distance.ToLegacyDec()
+			logger.Error("failed to calculate ApproxRoot for min deposit",
+				"error", err,
+				"distance", distance.String(),
+				"k", k,
+				"fallback", "using k=1")
 		}
 		c := a.Mul(b)
 		percChange = percChange.Add(c)

x/gov/keeper/min_initial_deposit.go

@@ -80,6 +80,8 @@ func (keeper Keeper) GetLastMinInitialDeposit(ctx sdk.Context) (sdk.Coins, time.
 // GetMinInitialDeposit returns the (dynamic) minimum initial deposit currently required for
 // proposal submission
 func (keeper Keeper) GetMinInitialDeposit(ctx sdk.Context) sdk.Coins {
+	logger := keeper.Logger(ctx)
+
 	params := keeper.GetParams(ctx)
 	minInitialDepositFloor := sdk.Coins(params.MinInitialDepositThrottler.FloorValue)
 	tick := params.MinInitialDepositThrottler.UpdatePeriod
@@ -101,10 +103,20 @@ func (keeper Keeper) GetMinInitialDeposit(ctx sdk.Context) sdk.Coins {
 	distance := numInactiveProposals.Sub(targetInactiveProposals)
 	percChange := math.LegacyOneDec()
 	if !distance.IsZero() {
+		// ApproxRoot is here being called on a relatively small positive
+		// integer (even if distance < 0, ApproxRoot will return
+		// `|distance|.ApproxRoot(k) * -1`) with a value of k expected to also
+		// be relatively small (<= 100).
+		// This is a safe operation and should not error.
 		b, err := distance.ToLegacyDec().ApproxRoot(k)
 		if err != nil {
 			// in case of error bypass the sensitivity, i.e. assume k = 1
 			b = distance.ToLegacyDec()
+			logger.Error("failed to calculate ApproxRoot for min initial deposit",
+				"error", err,
+				"distance", distance.String(),
+				"k", k,
+				"fallback", "using k=1")
 		}
 		c := a.Mul(b)
 		percChange = percChange.Add(c)

x/gov/types/v1/params.go

@@ -9,10 +9,17 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
-// Default period for deposits & voting and min voting period
 const (
+	// Default period for deposits & voting and min voting period
 	DefaultVotingPeriod  time.Duration = time.Hour * 24 * 21 // 21 days
 	DefaultDepositPeriod time.Duration = time.Hour * 24 * 14 // 14 days
+
+	// MaxSensitivityTargetDistanceDepositThrottler is the maximum value that
+	// can be set for the sensitivity to target distance for dynamic initial
+	// and total deposit. This value has been empirically found to
+	// be sufficient for realistic usage. A higher value would make the
+	// throttler too little sensitive to the distance from the target.
+	MaxSensitivityTargetDistanceDepositThrottler = 100
 )
 
 // MinVotingPeriod is set in stone by the constitution at 21 days, but it can
@@ -351,6 +358,10 @@ func (p Params) ValidateBasic() error {
 		return fmt.Errorf("minimum deposit sensitivity target distance must be positive: %d", p.MinDepositThrottler.SensitivityTargetDistance)
 	}
 
+	if p.MinDepositThrottler.SensitivityTargetDistance > MaxSensitivityTargetDistanceDepositThrottler {
+		return fmt.Errorf("minimum deposit sensitivity target distance must be less than or equal to %d: %d", MaxSensitivityTargetDistanceDepositThrottler, p.MinDepositThrottler.SensitivityTargetDistance)
+	}
+
 	minDepositIncreaseRatio, err := sdk.NewDecFromStr(p.MinDepositThrottler.IncreaseRatio)
 	if err != nil {
 		return fmt.Errorf("invalid minimum deposit increase ratio: %w", err)
@@ -397,6 +408,10 @@ func (p Params) ValidateBasic() error {
 		return fmt.Errorf("minimum initial deposit sensitivity target distance must be positive: %d", p.MinInitialDepositThrottler.SensitivityTargetDistance)
 	}
 
+	if p.MinInitialDepositThrottler.SensitivityTargetDistance > MaxSensitivityTargetDistanceDepositThrottler {
+		return fmt.Errorf("minimum initial deposit sensitivity target distance must be less than or equal to %d: %d", MaxSensitivityTargetDistanceDepositThrottler, p.MinInitialDepositThrottler.SensitivityTargetDistance)
+	}
+
 	minInitialDepositIncreaseRatio, err := sdk.NewDecFromStr(p.MinInitialDepositThrottler.IncreaseRatio)
 	if err != nil {
 		return fmt.Errorf("invalid minimum initial deposit increase ratio: %w", err)