Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 6 commits into from
Jul 26, 2023
Merged

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

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9073bdc
remove partial sf migration
stackman27 Jul 18, 2023
d70e4e1
added changelog
stackman27 Jul 18, 2023
a914d71
adams feedback
stackman27 Jul 25, 2023
152f6a9
fixed test
stackman27 Jul 25, 2023
6829d30
adams comment
stackman27 Jul 25, 2023
aa5d7e3
Update x/superfluid/keeper/migrate.go
czarcas7ic Jul 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 0 additions & 5 deletions x/superfluid/keeper/export_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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)
}
51 changes: 20 additions & 31 deletions x/superfluid/keeper/migrate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down Expand Up @@ -84,8 +88,6 @@ func (k Keeper) migrateSuperfluidBondedBalancerToConcentrated(ctx sdk.Context,
return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, 0, err
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should just take in the lock ID and not the sharesToMigrate, and instead just pull the shares directly in this method. This API only made sense when shares to migrate was chooseable which it no longer is

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one of the thought process i had was since we only call the migration functions using RouteLockedBalancerToConcentratedMigration and MigrateUnlockedPositionFromBalancerToConcentrated does allow partial migration, i wanted to keep sharesToMigrate. For all the private migration functions like migrateSuperfluidBondedBalancerToConcentrated we can just default it to the entire lock coins which we do in validateSharesToMigrateUnlockAndExitBalancerPool

Copy link
Member

@czarcas7ic czarcas7ic Jul 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see yeah, that makes sense, thanks!

}

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)
Expand All @@ -97,21 +99,13 @@ func (k Keeper) migrateSuperfluidBondedBalancerToConcentrated(ctx sdk.Context,
// 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
}

// Note that lock's id is the same as the originalLockId since all shares are being migrated
// and old lock is deleted
gammLockToMigrate = preMigrationLock
czarcas7ic marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand Down Expand Up @@ -326,20 +320,15 @@ 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()}
}

// If migrating the entire lock, force unlock.
// This breaks and deletes associated synthetic locks.
err = k.lk.ForceUnlock(ctx, *lock)
czarcas7ic marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return sdk.Coins{}, err
}

// Exit the balancer pool position.
Expand Down
76 changes: 8 additions & 68 deletions x/superfluid/keeper/migrate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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": {
Expand All @@ -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,
},
Expand All @@ -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,
},
Expand Down Expand Up @@ -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"},
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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))),
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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"),
Expand Down
8 changes: 0 additions & 8 deletions x/superfluid/keeper/stake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading