[Chore] Remove SF partial migration from balancer to CL (#5874)

* remove partial sf migration * added changelog * adams feedback * fixed test * adams comment * Update x/superfluid/keeper/migrate.go --------- Co-authored-by: Adam Tucker <[email protected]>
osmosis-labs · Jul 26, 2023 · b65a184 · b65a184
1 parent c75ad9f
commit b65a184
Show file tree

Hide file tree

Showing 7 changed files with 35 additions and 274 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -45,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### State Breaking
 
 * [#5532](https://github.com/osmosis-labs/osmosis/pull/5532) fix: Fix x/tokenfactory genesis import denoms reset x/bank existing denom metadata
+* [#5874](https://github.com/osmosis-labs/osmosis/pull/5874) Remove Partial Migration from superfluid migration to CL
 
 ### BugFix
 

diff --git a/x/superfluid/keeper/export_test.go b/x/superfluid/keeper/export_test.go
@@ -6,7 +6,6 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	lockuptypes "github.com/osmosis-labs/osmosis/v16/x/lockup/types"
-	"github.com/osmosis-labs/osmosis/v16/x/superfluid/types"
 )
 
 var (
@@ -58,10 +57,6 @@ func (k Keeper) GetExistingLockRemainingDuration(ctx sdk.Context, lock *lockupty
 	return k.getExistingLockRemainingDuration(ctx, lock)
 }
 
-func (k Keeper) PartialSuperfluidUndelegateToConcentratedPosition(ctx sdk.Context, sender string, lockID uint64, amountToUndelegate sdk.Coin) (intermediaryAcc types.SuperfluidIntermediaryAccount, newlock *lockuptypes.PeriodLock, err error) {
-	return k.partialSuperfluidUndelegateToConcentratedPosition(ctx, sender, lockID, amountToUndelegate)
-}
-
 func (k Keeper) DistributeSuperfluidGauges(ctx sdk.Context) {
 	k.distributeSuperfluidGauges(ctx)
 }
diff --git a/x/superfluid/keeper/migrate.go b/x/superfluid/keeper/migrate.go
@@ -26,18 +26,22 @@ const (
 // RouteLockedBalancerToConcentratedMigration routes the provided lock to the proper migration function based on the lock status.
 // The testing conditions and scope for the different lock status are as follows:
 // Lock Status = Superfluid delegated
+// - cannot migrate partial shares
 // - Instantly undelegate which will bypass unbonding time.
 // - Create new CL Lock and Re-delegate it as a concentrated liquidity position.
 //
 // Lock Status = Superfluid undelegating
+// - cannot migrate partial shares
 // - Continue undelegating as superfluid unbonding CL Position.
 // - Lock the tokens and create an unlocking syntheticLock (to handle cases of slashing)
 //
 // Lock Status = Locked or unlocking (no superfluid delegation/undelegation)
+// - cannot migrate partial shares
 // - Force unlock tokens from gamm shares.
 // - Create new CL lock and starts unlocking or unlocking where it left off.
 //
 // Lock Status = Unlocked
+// - can migrate partial shares
 // - For ex: LP shares
 // - Create new CL lock and starts unlocking or unlocking where it left off.
 //
@@ -84,8 +88,6 @@ func (k Keeper) migrateSuperfluidBondedBalancerToConcentrated(ctx sdk.Context,
 		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, 0, err
 	}
 
-	isPartialMigration := sharesToMigrate.Amount.LT(preMigrationLock.Coins[0].Amount)
-
 	// Get the validator address from the synth denom and ensure it is a valid address.
 	valAddr := strings.Split(synthDenomBeforeMigration, "/")[4]
 	_, err = sdk.ValAddressFromBech32(valAddr)
@@ -96,28 +98,16 @@ func (k Keeper) migrateSuperfluidBondedBalancerToConcentrated(ctx sdk.Context,
 	// Superfluid undelegate the portion of shares the user is migrating from the superfluid delegated position.
 	// If all shares are being migrated, this deletes the connection between the gamm lock and the intermediate account, deletes the synthetic lock, and burns the synthetic osmo.
 	intermediateAccount := types.SuperfluidIntermediaryAccount{}
-	var gammLockToMigrate *lockuptypes.PeriodLock
-	if isPartialMigration {
-		// Note that lock's id is different from the originalLockId since it was split.
-		// The original lock id stays in gamm.
-		intermediateAccount, gammLockToMigrate, err = k.partialSuperfluidUndelegateToConcentratedPosition(ctx, sender.String(), originalLockId, sharesToMigrate)
-		if err != nil {
-			return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, 0, err
-		}
-	} else {
-		// Note that lock's id is the same as the originalLockId since all shares are being migrated
-		// and old lock is deleted
-		gammLockToMigrate = preMigrationLock
-		intermediateAccount, err = k.SuperfluidUndelegateToConcentratedPosition(ctx, sender.String(), originalLockId)
-		if err != nil {
-			return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, 0, err
-		}
+
+	intermediateAccount, err = k.SuperfluidUndelegateToConcentratedPosition(ctx, sender.String(), originalLockId)
+	if err != nil {
+		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, 0, err
 	}
 
 	// Force unlock, validate the provided sharesToMigrate, and exit the balancer pool.
 	// This will return the coins that will be used to create the concentrated liquidity position.
 	// It also returns the lock object that contains the remaining shares that were not used in this migration.
-	exitCoins, err := k.validateSharesToMigrateUnlockAndExitBalancerPool(ctx, sender, poolIdLeaving, gammLockToMigrate, sharesToMigrate, tokenOutMins)
+	exitCoins, err := k.validateSharesToMigrateUnlockAndExitBalancerPool(ctx, sender, poolIdLeaving, preMigrationLock, sharesToMigrate, tokenOutMins)
 	if err != nil {
 		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, 0, err
 	}
@@ -326,20 +316,14 @@ func (k Keeper) validateSharesToMigrateUnlockAndExitBalancerPool(ctx sdk.Context
 	}
 
 	// Finish unlocking directly for locked or unlocking locks
-	if sharesToMigrate.Equal(gammSharesInLock) {
-		// If migrating the entire lock, force unlock.
-		// This breaks and deletes associated synthetic locks.
-		err = k.lk.ForceUnlock(ctx, *lock)
-		if err != nil {
-			return sdk.Coins{}, err
-		}
-	} else {
-		// Otherwise, we must split the lock and force unlock the partial shares to migrate.
-		// This breaks and deletes associated synthetic locks.
-		err = k.lk.PartialForceUnlock(ctx, *lock, sdk.NewCoins(sharesToMigrate))
-		if err != nil {
-			return sdk.Coins{}, err
-		}
+	if !sharesToMigrate.Equal(gammSharesInLock) {
+		return sdk.Coins{}, types.MigratePartialSharesError{SharesToMigrate: sharesToMigrate.Amount.String(), SharesInLock: gammSharesInLock.Amount.String()}
+	}
+
+	// Force migrate, which breaks and deletes associated synthetic locks.
+	err = k.lk.ForceUnlock(ctx, *lock)
+	if err != nil {
+		return sdk.Coins{}, err
 	}
 
 	// Exit the balancer pool position.

diff --git a/x/superfluid/keeper/migrate_test.go b/x/superfluid/keeper/migrate_test.go
@@ -57,48 +57,32 @@ func (s *KeeperTestSuite) TestRouteLockedBalancerToConcentratedMigration() {
 		"lock that is not superfluid delegated, not unlocking": {
 			// migrateNonSuperfluidLockBalancerToConcentrated
 			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.9"),
+			expectedError:            types.MigratePartialSharesError{SharesToMigrate: "45000000000000000000", SharesInLock: "50000000000000000000"},
 		},
 		"lock that is not superfluid delegated, unlocking": {
 			// migrateNonSuperfluidLockBalancerToConcentrated
 			unlocking:                true,
 			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.6"),
+			expectedError:            types.MigratePartialSharesError{SharesToMigrate: "30000000000000000000", SharesInLock: "50000000000000000000"},
 		},
 		"lock that is superfluid delegated, not unlocking (full shares)": {
 			// migrateSuperfluidBondedBalancerToConcentrated
 			superfluidDelegated:      true,
 			percentOfSharesToMigrate: sdk.MustNewDecFromStr("1"),
 		},
-		"lock that is superfluid delegated, not unlocking (partial shares)": {
-			// migrateSuperfluidBondedBalancerToConcentrated
-			superfluidDelegated:      true,
-			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.4"),
-		},
 		"lock that is superfluid undelegating, not unlocking (full shares)": {
 			// migrateSuperfluidUnbondingBalancerToConcentrated
 			superfluidDelegated:      true,
 			superfluidUndelegating:   true,
 			percentOfSharesToMigrate: sdk.MustNewDecFromStr("1"),
 		},
-		"lock that is superfluid undelegating, not unlocking (partial shares)": {
-			// migrateSuperfluidUnbondingBalancerToConcentrated
-			superfluidDelegated:      true,
-			superfluidUndelegating:   true,
-			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.4"),
-		},
 		"lock that is superfluid undelegating, unlocking (full shares)": {
 			// migrateSuperfluidUnbondingBalancerToConcentrated
 			superfluidDelegated:      true,
 			superfluidUndelegating:   true,
 			unlocking:                true,
 			percentOfSharesToMigrate: sdk.MustNewDecFromStr("1"),
 		},
-		"lock that is superfluid undelegating, unlocking (partial shares)": {
-			// migrateSuperfluidUnbondingBalancerToConcentrated
-			superfluidDelegated:      true,
-			superfluidUndelegating:   true,
-			unlocking:                true,
-			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.3"),
-		},
 		"no lock (partial shares)": {
 			// MigrateUnlockedPositionFromBalancerToConcentrated
 			noLock:                   true,
@@ -116,15 +100,15 @@ func (s *KeeperTestSuite) TestRouteLockedBalancerToConcentratedMigration() {
 		},
 		"error: lock that is not superfluid delegated, not unlocking, min exit coins more than being exitted": {
 			// migrateNonSuperfluidLockBalancerToConcentrated
-			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.9"),
+			percentOfSharesToMigrate: sdk.MustNewDecFromStr("1"),
 			minExitCoins:             sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(5000)), sdk.NewCoin("stake", sdk.NewInt(5000))),
 			expectedError:            gammtypes.ErrLimitMinAmount,
 		},
 		"error: lock that is not superfluid delegated, unlocking, min exit coins more than being exitted": {
 			// migrateNonSuperfluidLockBalancerToConcentrated
 			unlocking:                true,
-			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.6"),
-			minExitCoins:             sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(4000))),
+			percentOfSharesToMigrate: sdk.MustNewDecFromStr("1"),
+			minExitCoins:             sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(5000))),
 			expectedError:            gammtypes.ErrLimitMinAmount,
 		},
 		"error: lock that is superfluid delegated, not unlocking (full shares), min exit coins more than being exitted": {
@@ -134,18 +118,11 @@ func (s *KeeperTestSuite) TestRouteLockedBalancerToConcentratedMigration() {
 			minExitCoins:             sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(10000))),
 			expectedError:            gammtypes.ErrLimitMinAmount,
 		},
-		"error: lock that is superfluid delegated, not unlocking (partial shares, min exit coins more than being exitted": {
-			// migrateSuperfluidBondedBalancerToConcentrated
-			superfluidDelegated:      true,
-			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.5"),
-			minExitCoins:             sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(3000))),
-			expectedError:            gammtypes.ErrLimitMinAmount,
-		},
 		"error: lock that is superfluid undelegating, not unlocking, min exit coins more than being exitted": {
 			// migrateSuperfluidUnbondingBalancerToConcentrated
 			superfluidDelegated:      true,
 			superfluidUndelegating:   true,
-			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.5"),
+			percentOfSharesToMigrate: sdk.MustNewDecFromStr("1"),
 			minExitCoins:             sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(40000))),
 			expectedError:            gammtypes.ErrLimitMinAmount,
 		},
@@ -154,7 +131,7 @@ func (s *KeeperTestSuite) TestRouteLockedBalancerToConcentratedMigration() {
 			superfluidDelegated:      true,
 			superfluidUndelegating:   true,
 			unlocking:                true,
-			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.3"),
+			percentOfSharesToMigrate: sdk.MustNewDecFromStr("1"),
 			minExitCoins:             sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(40000))),
 			expectedError:            gammtypes.ErrLimitMinAmount,
 		},
@@ -288,9 +265,6 @@ func (s *KeeperTestSuite) TestMigrateSuperfluidBondedBalancerToConcentrated() {
 		"lock that is superfluid delegated, not unlocking (full shares)": {
 			percentOfSharesToMigrate: sdk.MustNewDecFromStr("1"),
 		},
-		"lock that is superfluid delegated, not unlocking (partial shares)": {
-			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.5"),
-		},
 		"error: migrate more shares than lock has": {
 			percentOfSharesToMigrate: sdk.MustNewDecFromStr("1.1"),
 			expectedError:            types.MigrateMoreSharesThanLockHasError{SharesToMigrate: "55000000000000000000", SharesInLock: "50000000000000000000"},
@@ -450,17 +424,10 @@ func (s *KeeperTestSuite) TestMigrateSuperfluidUnbondingBalancerToConcentrated()
 		"lock that is superfluid undelegating, not unlocking (full shares)": {
 			percentOfSharesToMigrate: sdk.MustNewDecFromStr("1"),
 		},
-		"lock that is superfluid undelegating, not unlocking (partial shares)": {
-			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.5"),
-		},
 		"lock that is superfluid undelegating, unlocking (full shares)": {
 			unlocking:                true,
 			percentOfSharesToMigrate: sdk.MustNewDecFromStr("1"),
 		},
-		"lock that is superfluid undelegating, unlocking (partial shares)": {
-			unlocking:                true,
-			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.3"),
-		},
 		"error: invalid validator address": {
 			overwriteValidatorAddress: true,
 			percentOfSharesToMigrate:  sdk.MustNewDecFromStr("1"),
@@ -580,17 +547,10 @@ func (s *KeeperTestSuite) TestMigrateNonSuperfluidLockBalancerToConcentrated() {
 		"lock that is not superfluid delegated, not unlocking (full shares)": {
 			percentOfSharesToMigrate: sdk.MustNewDecFromStr("1"),
 		},
-		"lock that is not superfluid delegated, not unlocking (partial shares)": {
-			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.9"),
-		},
 		"lock that is not superfluid delegated, unlocking (full shares)": {
 			unlocking:                true,
 			percentOfSharesToMigrate: sdk.MustNewDecFromStr("1"),
 		},
-		"lock that is not superfluid delegated, unlocking (partial shares)": {
-			unlocking:                true,
-			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.6"),
-		},
 		"error: lock that is not superfluid delegated, not unlocking (full shares), token out mins is more than exit coins": {
 			percentOfSharesToMigrate: sdk.MustNewDecFromStr("1"),
 			tokenOutMins:             sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(10000))),
@@ -754,42 +714,21 @@ func (s *KeeperTestSuite) TestValidateMigration() {
 			isSuperfluidDelegated:    false,
 			isSuperfluidUndelegating: false,
 		},
-		"lock that is not superfluid delegated, unlocking (partial shares)": {
-			unlocking:                true,
-			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.6"),
-			isSuperfluidDelegated:    false,
-			isSuperfluidUndelegating: false,
-		},
 		"lock that is superfluid undelegating, not unlocking (full shares)": {
 			percentOfSharesToMigrate: sdk.MustNewDecFromStr("1"),
 			isSuperfluidDelegated:    true,
 			isSuperfluidUndelegating: true,
 		},
-		"lock that is superfluid undelegating, not unlocking (partial shares)": {
-			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.5"),
-			isSuperfluidDelegated:    true,
-			isSuperfluidUndelegating: true,
-		},
 		"lock that is superfluid undelegating, unlocking (full shares)": {
 			unlocking:                true,
 			percentOfSharesToMigrate: sdk.MustNewDecFromStr("1"),
 			isSuperfluidDelegated:    true,
 			isSuperfluidUndelegating: true,
 		},
-		"lock that is superfluid undelegating, unlocking (partial shares)": {
-			unlocking:                true,
-			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.3"),
-			isSuperfluidDelegated:    true,
-			isSuperfluidUndelegating: true,
-		},
 		"lock that is superfluid delegated, not unlocking (full shares)": {
 			percentOfSharesToMigrate: sdk.MustNewDecFromStr("1"),
 			isSuperfluidDelegated:    true,
 		},
-		"lock that is superfluid delegated, not unlocking (partial shares)": {
-			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.5"),
-			isSuperfluidDelegated:    true,
-		},
 		"error: denom prefix error": {
 			overwriteSharesDenomValue: "cl/pool/2",
 			percentOfSharesToMigrate:  sdk.MustNewDecFromStr("1"),
@@ -875,6 +814,7 @@ func (s *KeeperTestSuite) TestValidateSharesToMigrateUnlockAndExitBalancerPool()
 		},
 		"happy path (partial shares)": {
 			percentOfSharesToMigrate: sdk.MustNewDecFromStr("0.4"),
+			expectedError:            types.MigratePartialSharesError{SharesToMigrate: "20000000000000000000", SharesInLock: "50000000000000000000"},
 		},
 		"error: lock does not exist": {
 			percentOfSharesToMigrate:  sdk.MustNewDecFromStr("1"),

diff --git a/x/superfluid/keeper/stake.go b/x/superfluid/keeper/stake.go
@@ -364,14 +364,6 @@ func (k Keeper) partialSuperfluidUndelegate(ctx sdk.Context, sender string, lock
 	return k.createSyntheticLockup(ctx, newLock.ID, intermediaryAcc, unlockingStatus)
 }
 
-// partialSuperfluidUndelegateToConcentratedPosition starts undelegating a portion of a superfluid delegated position for the given lock. It behaves similarly to partialSuperfluidUndelegate,
-// however it does not create a new synthetic lockup representing the unstaking side. This is because after the time this function is called, we might
-// want to perform more operations prior to creating a lock. Once the actual lock is created, the synthetic lockup representing the unstaking side
-// should eventually be created as well. Use this function with caution to avoid accidentally missing synthetic lock creation.
-func (k Keeper) partialSuperfluidUndelegateToConcentratedPosition(ctx sdk.Context, sender string, gammLockID uint64, amountToUndelegate sdk.Coin) (types.SuperfluidIntermediaryAccount, *lockuptypes.PeriodLock, error) {
-	return k.partialUndelegateCommon(ctx, sender, gammLockID, amountToUndelegate)
-}
-
 // SuperfluidUnbondLock unbonds the lock that has been used for superfluid staking.
 // This method would return an error if the underlying lock is not superfluid undelegating.
 func (k Keeper) SuperfluidUnbondLock(ctx sdk.Context, underlyingLockId uint64, sender string) error {