Fix: Fix valset pref unbonding bugs (#5967) (#6696)

* Add helper to get existing delegations along with valset pref

* Lay out more of the logic nuance

* Add pseudo-code needed

* algorithm v1

* added algorithm docs

* fixed all test

* removed unwanted files

* remove more code

* added more tests

* update changelog

* added test and addressed feedback

* Update x/valset-pref/validator_set.go

* Update x/valset-pref/validator_set.go

* Update x/valset-pref/README.md

* Minor cleanup

* re-use validator gets

* Refactor

* Highlight bug

* fixed the issue

* added comments

* update changelog

* fixed the issue

* fixed go test

* fixed test

* lint

* Finish ValSet Pref Unbonding (#6630)

* Add Test cases and fix

* Update x/valset-pref/keeper.go

* Adams comment

* add error types and tidy test

---------

Co-authored-by: Adam Tucker <adam@osmosis.team>
Co-authored-by: Adam Tucker <adamleetucker@outlook.com>

* feat: unbond with rebalanced val set weights (#6685)

* initial push

* add todo

* msg server test

* add godoc

* custom error

* update godoc

* update proto and remove duplication

* add more TODOs

* use min when calculating amt to withdraw from last val

* Update x/valset-pref/validator_set.go

---------

Co-authored-by: stackman27 <sis1001@berkeley.edu>
Co-authored-by: devbot-wizard <141283918+devbot-wizard@users.noreply.github.com>
Co-authored-by: Adam Tucker <adam@osmosis.team>
Co-authored-by: roman <roman@osmosis.team>
Co-authored-by: Matt, Park <45252226+mattverse@users.noreply.github.com>
Co-authored-by: Adam Tucker <adamleetucker@outlook.com>
(cherry picked from commit e12011b)

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>

CHANGELOG.md

@@ -57,6 +57,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * [#6309](https://github.com/osmosis-labs/osmosis/pull/6309) Add  Cosmwasm Pool Queries to Stargate Query
 * [#6493](https://github.com/osmosis-labs/osmosis/pull/6493) Add PoolManager Params query to Stargate Whitelist
 * [#6421](https://github.com/osmosis-labs/osmosis/pull/6421) Moves ValidatePermissionlessPoolCreationEnabled out of poolmanager module
+* [#5967](https://github.com/osmosis-labs/osmosis/pull/5967) fix ValSet undelegate API out of sync with existing staking
 * [#6627](https://github.com/osmosis-labs/osmosis/pull/6627) Limit pow iterations in osmomath.
 * [#6586](https://github.com/osmosis-labs/osmosis/pull/6586) add auth.moduleaccounts to the stargate whitelist
 * [#6680](https://github.com/osmosis-labs/osmosis/pull/6680) Add Taker Fee query and add it to stargate whitelist

proto/osmosis/valset-pref/v1beta1/tx.proto

@@ -27,6 +27,13 @@ service Msg {
   rpc UndelegateFromValidatorSet(MsgUndelegateFromValidatorSet)
       returns (MsgUndelegateFromValidatorSetResponse);
 
+  // UndelegateFromRebalancedValidatorSet undelegates the proivded amount from
+  // the validator set, but takes into consideration the current delegations
+  // to the user's validator set to determine the weights assigned to each.
+  rpc UndelegateFromRebalancedValidatorSet(
+      MsgUndelegateFromRebalancedValidatorSet)
+      returns (MsgUndelegateFromRebalancedValidatorSetResponse);
+
   // RedelegateValidatorSet takes the existing validator set and redelegates to
   // a new set.
   rpc RedelegateValidatorSet(MsgRedelegateValidatorSet)
@@ -98,6 +105,27 @@ message MsgUndelegateFromValidatorSet {
 
 message MsgUndelegateFromValidatorSetResponse {}
 
+message MsgUndelegateFromRebalancedValidatorSet {
+  option (amino.name) = "osmosis/MsgUndelegateFromRebalancedValidatorSet";
+
+  // delegator is the user who is trying to undelegate.
+  string delegator = 1 [ (gogoproto.moretags) = "yaml:\"delegator\"" ];
+
+  // the amount the user wants to undelegate
+  // For ex: Undelegate 50 osmo with validator-set {ValA -> 0.5, ValB -> 0.5}
+  // Our undelegate logic would first check the current delegation balance.
+  // If the user has 90 osmo delegated to ValA and 10 osmo delegated to ValB,
+  // the rebalanced validator set would be {ValA -> 0.9, ValB -> 0.1}
+  // So now the 45 osmo would be undelegated from ValA and 5 osmo would be
+  // undelegated from ValB.
+  cosmos.base.v1beta1.Coin coin = 2 [
+    (gogoproto.nullable) = false,
+    (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coin"
+  ];
+}
+
+message MsgUndelegateFromRebalancedValidatorSetResponse {}
+
 message MsgRedelegateValidatorSet {
   option (amino.name) = "osmosis/MsgRedelegateValidatorSet";
 

x/valset-pref/README.md

@@ -25,15 +25,54 @@ How does this module work?
 - If the delegator has not set a validator-set preference list then the validator set, then it defaults to their current validator set.
 - If a user has no preference list and has not staked, then these messages / queries return errors.
 
-## Calculations
-
-Staking Calculation 
-
-- The user provides an amount to delegate and our `MsgDelegateToValidatorSet` divides the amount based on validator weight distribution.
-  For example: Stake 100osmo with validator-set {ValA -> 0.5, ValB -> 0.3, ValC -> 0.2}
-  our delegate logic will attempt to delegate (100 * 0.5) 50osmo for ValA , (100 * 0.3) 30osmo from ValB and (100 * 0.2) 20osmo from ValC.
-
-UnStaking Calculation 
+## Logic
+
+### Delegate to validator set
+
+This is pretty straight-forward, theres not really any edge cases here.
+
+The user provides an amount to delegate and our `MsgDelegateToValidatorSet` divides the amount based on validator weight distribution.
+
+For example: Stake 100osmo with validator-set {ValA -> 0.5, ValB -> 0.3, ValC -> 0.2}
+our delegate logic will attempt to delegate (100 * 0.5) 50osmo for ValA , (100 * 0.3) 30osmo from ValB and (100 * 0.2) 20osmo from ValC.
+
+### Undelegate from validator set 
+
+We can imagine describing undelegate from validator set in two cases:
+- Users existing delegation distribution matches their validator-set preference distribution.
+- Users existing delegation distribution does not match their validator-set preference distribution.
+
+Algorithm for undelegation; 
+unbond as true to valset ratios as possible. Undelegation should be possible.
+Idea of what we should be doing for Undelegate(valset, amount):
+1. Calculate the amount to undelegate from each validator under full valset usage
+2. If all amounts are less than amount staked to validator, undelegate all
+3. If any amount is greater than amount staked to validator (S,V),
+   fully unstake S from that validator. Recursively proceed with undelegating the
+   remaining amount from the remaining validators.
+   Undelegate(valset - V, amount - S)
+
+The above algorithm would take O(V^2) worst case, so we instead do something better
+to be O(V). 
+
+1. Calculate the amount to undelegate from each validator under full valset usage
+2. For each validator, compute V.ratio = undelegate_amount / amount_staked_to_val
+3. Sort validators by V_ratio descending. If V_ratio <= 1, there is no need to re-calculate amount to undelegate for each validator, undelegate and end algorithm.
+4. If V_ratio <= 1, undelegate target amount from each validator. (happy path)  
+5. Set target_ratio = 1, amount_remaining_to_unbond = amount
+6. While greatest V_ratio > target_ratio:
+   - Fully undelegate validator with greatest V_ratio. (Amount S)
+   - remove validator from list
+   - recalculate target_ratio = target_ratio * (1 - removed_V.target_percent) 
+     - this works, because if you recalculated target ratio scaled to 1
+		   every val's ratio would just differ by the constant factor of 1 / (1 - removed_V.target_percent)
+       doing that would take O(V), hence we change the target.
+7. Normal undelegate the remainder.
+
+
+#### Case 1: Users existing delegation distribution matches their validator-set preference distribution
+
+{Old docs below}
 
 - The user provides an amount to undelegate and our `MsgUnDelegateToValidatorSet` divides the amount based on validator weight distribution.
 - Here, the user can either undelegate the entire amount or partial amount 

x/valset-pref/client/cli/tx.go

@@ -19,7 +19,10 @@ func GetTxCmd() *cobra.Command {
 	txCmd := osmocli.TxIndexCmd(types.ModuleName)
 	osmocli.AddTxCmd(txCmd, NewSetValSetCmd)
 	osmocli.AddTxCmd(txCmd, NewDelValSetCmd)
-	osmocli.AddTxCmd(txCmd, NewUnDelValSetCmd)
+	// TODO: Uncomment when undelegate is implemented
+	// https://github.com/osmosis-labs/osmosis/issues/6686
+	//osmocli.AddTxCmd(txCmd, NewUnDelValSetCmd)
+	osmocli.AddTxCmd(txCmd, NewUndelRebalancedValSetCmd)
 	osmocli.AddTxCmd(txCmd, NewReDelValSetCmd)
 	osmocli.AddTxCmd(txCmd, NewWithRewValSetCmd)
 	return txCmd
@@ -44,13 +47,25 @@ func NewDelValSetCmd() (*osmocli.TxCliDesc, *types.MsgDelegateToValidatorSet) {
 	}, &types.MsgDelegateToValidatorSet{}
 }
 
-func NewUnDelValSetCmd() (*osmocli.TxCliDesc, *types.MsgUndelegateFromValidatorSet) {
+// TODO: Uncomment when undelegate is implemented
+// https://github.com/osmosis-labs/osmosis/issues/6686
+// func NewUnDelValSetCmd() (*osmocli.TxCliDesc, *types.MsgUndelegateFromValidatorSet) {
+// 	return &osmocli.TxCliDesc{
+// 		Use:     "undelegate-valset",
+// 		Short:   "UnDelegate tokens from existing valset using delegatorAddress and tokenAmount.",
+// 		Example: "osmosisd tx valset-pref undelegate-valset osmo1... 100stake",
+// 		NumArgs: 2,
+// 	}, &types.MsgUndelegateFromValidatorSet{}
+// }
+
+func NewUndelRebalancedValSetCmd() (*osmocli.TxCliDesc, *types.MsgUndelegateFromRebalancedValidatorSet) {
 	return &osmocli.TxCliDesc{
-		Use:     "undelegate-valset",
-		Short:   "UnDelegate tokens from existing valset using delegatorAddress and tokenAmount.",
-		Example: "osmosisd tx valset-pref undelegate-valset osmo1... 100stake",
+		Use:     "undelegate-rebalanced-valset",
+		Short:   "Undelegate tokens from rebalanced valset using delegatorAddress and tokenAmount.",
+		Long:    "Undelegates from an existing valset, but calculates the valset weights based on current user delegations.",
+		Example: "osmosisd tx valset-pref undelegate-rebalanced-valset osmo1... 100stake",
 		NumArgs: 2,
-	}, &types.MsgUndelegateFromValidatorSet{}
+	}, &types.MsgUndelegateFromRebalancedValidatorSet{}
 }
 
 func NewReDelValSetCmd() (*osmocli.TxCliDesc, *types.MsgRedelegateValidatorSet) {

x/valset-pref/export_test.go

@@ -2,9 +2,11 @@ package keeper
 
 import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 
 	"github.com/osmosis-labs/osmosis/osmomath"
 	lockuptypes "github.com/osmosis-labs/osmosis/v20/x/lockup/types"
+	"github.com/osmosis-labs/osmosis/v20/x/valset-pref/types"
 )
 
 type (
@@ -14,3 +16,12 @@ type (
 func (k Keeper) ValidateLockForForceUnlock(ctx sdk.Context, lockID uint64, delegatorAddr string) (*lockuptypes.PeriodLock, osmomath.Int, error) {
 	return k.validateLockForForceUnlock(ctx, lockID, delegatorAddr)
 }
+
+func (k Keeper) GetValsetRatios(ctx sdk.Context, delegator sdk.AccAddress,
+	prefs []types.ValidatorPreference, undelegateAmt osmomath.Int) ([]ValRatio, map[string]stakingtypes.Validator, osmomath.Dec, error) {
+	return k.getValsetRatios(ctx, delegator, prefs, undelegateAmt)
+}
+
+func FormatToValPrefArr(delegations []stakingtypes.Delegation) []types.ValidatorPreference {
+	return formatToValPrefArr(delegations)
+}

x/valset-pref/keeper.go

@@ -6,10 +6,11 @@ import (
 
 	"github.com/tendermint/tendermint/libs/log"
 
+	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
 
-	"github.com/osmosis-labs/osmosis/osmomath"
 	"github.com/osmosis-labs/osmosis/v20/x/valset-pref/types"
 )
 
@@ -49,42 +50,63 @@ func (k Keeper) GetDelegationPreferences(ctx sdk.Context, delegator string) (typ
 		if err != nil {
 			return types.ValidatorSetPreferences{}, err
 		}
-		existingDelsValSetFormatted, err := k.GetExistingStakingDelegations(ctx, delAddr)
-		if err != nil {
-			return types.ValidatorSetPreferences{}, err
+		existingDelegations := k.stakingKeeper.GetDelegatorDelegations(ctx, delAddr, math.MaxUint16)
+		if len(existingDelegations) == 0 {
+			return types.ValidatorSetPreferences{}, types.ErrNoDelegation
 		}
 
-		return types.ValidatorSetPreferences{Preferences: existingDelsValSetFormatted}, nil
+		return types.ValidatorSetPreferences{Preferences: formatToValPrefArr(existingDelegations)}, nil
 	}
 
 	return valSet, nil
 }
 
-// GetExistingStakingDelegations returns the existing delegation that's not valset.
-// This function also formats the output into ValidatorSetPreference struct {valAddr, weight}.
-// The weight is calculated based on (valDelegation / totalDelegations) for each validator.
-// This method erros when given address does not have any existing delegations.
-func (k Keeper) GetExistingStakingDelegations(ctx sdk.Context, delAddr sdk.AccAddress) ([]types.ValidatorPreference, error) {
-	var existingDelsValSetFormatted []types.ValidatorPreference
+// GetValSetPreferencesWithDelegations fetches the delegator's validator set preferences
+// considering their existing delegations.
+// -If validator set preference does not exist and there are no existing delegations, it returns an error.
+// -If validator set preference exists and there are no existing delegations, it returns the existing preference.
+// -If there is any existing delegation:
+// calculates the delegator's shares in each delegation
+// as a ratio of the total shares and returns it as part of ValidatorSetPreferences.
+func (k Keeper) GetValSetPreferencesWithDelegations(ctx sdk.Context, delegator string) (types.ValidatorSetPreferences, error) {
+	delAddr, err := sdk.AccAddressFromBech32(delegator)
+	if err != nil {
+		return types.ValidatorSetPreferences{}, err
+	}
 
+	valSet, exists := k.GetValidatorSetPreference(ctx, delegator)
 	existingDelegations := k.stakingKeeper.GetDelegatorDelegations(ctx, delAddr, math.MaxUint16)
-	if len(existingDelegations) == 0 {
-		return nil, types.ErrNoDelegation
+
+	// No existing delegations for a delegator when valSet does not exist
+	if !exists && len(existingDelegations) == 0 {
+		return types.ValidatorSetPreferences{}, fmt.Errorf("No Existing delegation to unbond from")
 	}
 
-	existingTotalShares := osmomath.NewDec(0)
-	// calculate total shares that currently exists
-	for _, existingDelegation := range existingDelegations {
-		existingTotalShares = existingTotalShares.Add(existingDelegation.Shares)
+	// Returning existing valSet when there are no existing delegations
+	if exists && len(existingDelegations) == 0 {
+		return valSet, nil
 	}
 
-	// for each delegation format it in types.ValidatorSetPreferences format
-	for _, existingDelegation := range existingDelegations {
-		existingDelsValSetFormatted = append(existingDelsValSetFormatted, types.ValidatorPreference{
-			ValOperAddress: existingDelegation.ValidatorAddress,
-			Weight:         existingDelegation.Shares.Quo(existingTotalShares),
-		})
+	// when existing delegation exists, have it based upon the existing delegation
+	// regardless of the delegator having valset pref or not
+	return types.ValidatorSetPreferences{Preferences: formatToValPrefArr(existingDelegations)}, nil
+}
+
+// formatToValPrefArr iterates over given delegations array, formats it into ValidatorPreference array.
+// Used to calculate weights for the each delegation towards validator.
+// CONTRACT: This method assumes no duplicated ValOperAddress exists in the given delegation.
+func formatToValPrefArr(delegations []stakingtypes.Delegation) []types.ValidatorPreference {
+	totalShares := sdk.NewDec(0)
+	for _, existingDelegation := range delegations {
+		totalShares = totalShares.Add(existingDelegation.Shares)
 	}
 
-	return existingDelsValSetFormatted, nil
+	valPrefs := make([]types.ValidatorPreference, len(delegations))
+	for i, delegation := range delegations {
+		valPrefs[i] = types.ValidatorPreference{
+			ValOperAddress: delegation.ValidatorAddress,
+			Weight:         delegation.Shares.Quo(totalShares),
+		}
+	}
+	return valPrefs
 }