From 9073bdc085c8328296105e0241071b370f42204a Mon Sep 17 00:00:00 2001 From: stackman27 Date: Tue, 18 Jul 2023 12:55:43 -0700 Subject: [PATCH 1/6] remove partial sf migration --- x/superfluid/keeper/export_test.go | 5 - x/superfluid/keeper/migrate.go | 24 ++--- x/superfluid/keeper/migrate_test.go | 63 ----------- x/superfluid/keeper/stake.go | 8 -- x/superfluid/keeper/stake_test.go | 160 ---------------------------- 5 files changed, 7 insertions(+), 253 deletions(-) diff --git a/x/superfluid/keeper/export_test.go b/x/superfluid/keeper/export_test.go index 7c30ff05c7c..aca5594501e 100644 --- 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 index 47257b4bfa6..1a95842deb2 100644 --- a/x/superfluid/keeper/migrate.go +++ b/x/superfluid/keeper/migrate.go @@ -84,8 +84,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) @@ -97,21 +95,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 + 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. diff --git a/x/superfluid/keeper/migrate_test.go b/x/superfluid/keeper/migrate_test.go index b4068bd2f6b..1a0d76bb1c3 100644 --- a/x/superfluid/keeper/migrate_test.go +++ b/x/superfluid/keeper/migrate_test.go @@ -68,23 +68,12 @@ func (s *KeeperTestSuite) TestRouteLockedBalancerToConcentratedMigration() { 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, @@ -92,13 +81,6 @@ func (s *KeeperTestSuite) TestRouteLockedBalancerToConcentratedMigration() { 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, @@ -134,13 +116,6 @@ 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, @@ -288,9 +263,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 +422,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 +545,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 +712,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"), diff --git a/x/superfluid/keeper/stake.go b/x/superfluid/keeper/stake.go index bd40b110a77..a239517e18e 100644 --- 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 { diff --git a/x/superfluid/keeper/stake_test.go b/x/superfluid/keeper/stake_test.go index 9060cca2163..631f8c0bee5 100644 --- a/x/superfluid/keeper/stake_test.go +++ b/x/superfluid/keeper/stake_test.go @@ -1028,166 +1028,6 @@ func (s *KeeperTestSuite) TestRefreshIntermediaryDelegationAmounts() { } } -func (suite *KeeperTestSuite) TestPartialSuperfluidUndelegateToConcentratedPosition() { - testCases := []struct { - name string - validatorStats []stakingtypes.BondStatus - superDelegations []superfluidDelegation - undelegateAmounts []sdk.Coin - superUnbondingLockIds []uint64 - expSuperUnbondingErr []bool - // expected amount of delegation to intermediary account - expInterDelegation []sdk.Dec - }{ - { - "with single validator and single superfluid delegation and single undelegation", - []stakingtypes.BondStatus{stakingtypes.Bonded}, - []superfluidDelegation{{0, 0, 0, 1000000}}, - []sdk.Coin{sdk.NewCoin("gamm/pool/1", sdk.NewInt(400000))}, - []uint64{1}, - []bool{false}, - []sdk.Dec{sdk.NewDec(6000000)}, - }, - { - "with single validator and additional superfluid delegations and single undelegation", - []stakingtypes.BondStatus{stakingtypes.Bonded}, - []superfluidDelegation{{0, 0, 0, 1000000}, {0, 0, 0, 1000000}}, - []sdk.Coin{sdk.NewCoin("gamm/pool/1", sdk.NewInt(900000))}, - []uint64{1}, - []bool{false}, - []sdk.Dec{sdk.NewDec(11000000)}, - }, - { - "with multiple validators and multiple superfluid delegations and multiple undelegations", - []stakingtypes.BondStatus{stakingtypes.Bonded, stakingtypes.Bonded}, - []superfluidDelegation{{0, 0, 0, 1000000}, {1, 1, 0, 1000000}}, - []sdk.Coin{sdk.NewCoin("gamm/pool/1", sdk.NewInt(600000)), sdk.NewCoin("gamm/pool/1", sdk.NewInt(400000))}, - []uint64{1, 2}, - []bool{false, false}, - []sdk.Dec{sdk.NewDec(4000000), sdk.NewDec(6000000)}, - }, - { - "add unbonding validator", - []stakingtypes.BondStatus{stakingtypes.Bonded, stakingtypes.Unbonding}, - []superfluidDelegation{{0, 0, 0, 1000000}, {1, 1, 0, 1000000}}, - []sdk.Coin{sdk.NewCoin("gamm/pool/1", sdk.NewInt(600000)), sdk.NewCoin("gamm/pool/1", sdk.NewInt(400000))}, - []uint64{1, 2}, - []bool{false, false}, - []sdk.Dec{sdk.NewDec(4000000), sdk.NewDec(6000000)}, - }, - { - "add unbonded validator", - []stakingtypes.BondStatus{stakingtypes.Bonded, stakingtypes.Unbonded}, - []superfluidDelegation{{0, 0, 0, 1000000}, {1, 1, 0, 1000000}}, - []sdk.Coin{sdk.NewCoin("gamm/pool/1", sdk.NewInt(600000)), sdk.NewCoin("gamm/pool/1", sdk.NewInt(400000))}, - []uint64{1, 2}, - []bool{false, false}, - []sdk.Dec{sdk.NewDec(4000000), sdk.NewDec(6000000)}, - }, - { - "undelegating not available lock id", - []stakingtypes.BondStatus{stakingtypes.Bonded}, - []superfluidDelegation{{0, 0, 0, 1000000}}, - []sdk.Coin{sdk.NewCoin("gamm/pool/1", sdk.NewInt(600000))}, - []uint64{2}, - []bool{true}, - []sdk.Dec{}, - }, - } - - for _, tc := range testCases { - tc := tc - suite.Run(tc.name, func() { - suite.SetupTest() - - bondDenom := suite.App.StakingKeeper.GetParams(suite.Ctx).BondDenom - - // setup validators - valAddrs := suite.SetupValidators(tc.validatorStats) - - denoms, _ := suite.SetupGammPoolsAndSuperfluidAssets([]sdk.Dec{sdk.NewDec(20), sdk.NewDec(20)}) - - // setup superfluid delegations - _, intermediaryAccs, _ := suite.setupSuperfluidDelegations(valAddrs, tc.superDelegations, denoms) - suite.checkIntermediaryAccountDelegations(intermediaryAccs) - - suite.Greater(len(tc.superUnbondingLockIds), 0) - for index, lockId := range tc.superUnbondingLockIds { - // get intermediary account - accAddr := suite.App.SuperfluidKeeper.GetLockIdIntermediaryAccountConnection(suite.Ctx, lockId) - intermediaryAcc := suite.App.SuperfluidKeeper.GetIntermediaryAccount(suite.Ctx, accAddr) - valAddr := intermediaryAcc.ValAddr - - lock, err := suite.App.LockupKeeper.GetLockByID(suite.Ctx, lockId) - if tc.expSuperUnbondingErr[index] { - // manually set the lock to nil if we expect an error so we don't fail early - suite.Require().Error(err) - lock = &lockuptypes.PeriodLock{} - } else { - suite.Require().NoError(err) - } - - // get pre-superfluid delgations osmo supply and supplyWithOffset - presupply := suite.App.BankKeeper.GetSupply(suite.Ctx, bondDenom) - presupplyWithOffset := suite.App.BankKeeper.GetSupplyWithOffset(suite.Ctx, bondDenom) - - // superfluid undelegate - intermediaryAcc, newLock, err := suite.App.SuperfluidKeeper.PartialSuperfluidUndelegateToConcentratedPosition(suite.Ctx, lock.Owner, lockId, tc.undelegateAmounts[index]) - if tc.expSuperUnbondingErr[index] { - suite.Require().Error(err) - continue - } - suite.Require().NoError(err) - - // the new lock should be equal to the amount we partially undelegated - suite.Require().Equal(tc.undelegateAmounts[index].Amount.String(), newLock.Coins[0].Amount.String()) - - // ensure post-superfluid delegations osmo supplywithoffset is the same while supply is not - postsupply := suite.App.BankKeeper.GetSupply(suite.Ctx, bondDenom) - postsupplyWithOffset := suite.App.BankKeeper.GetSupplyWithOffset(suite.Ctx, bondDenom) - - suite.Require().Equal(presupply.Amount.Sub(tc.undelegateAmounts[index].Amount.Mul(sdk.NewInt(10))).String(), postsupply.Amount.String()) - suite.Require().Equal(presupplyWithOffset, postsupplyWithOffset) - - // check lockId and intermediary account connection is not deleted - addr := suite.App.SuperfluidKeeper.GetLockIdIntermediaryAccountConnection(suite.Ctx, lockId) - suite.Require().Equal(intermediaryAcc.GetAccAddress().String(), addr.String()) - - // check bonding synthetic lockup is not deleted - _, err = suite.App.LockupKeeper.GetSyntheticLockup(suite.Ctx, lockId, keeper.StakingSyntheticDenom(lock.Coins[0].Denom, valAddr)) - suite.Require().NoError(err) - - // check unbonding synthetic lockup creation - // since this is the concentrated liquidity path, no new synthetic lockup should be created - synthLock, err := suite.App.LockupKeeper.GetSyntheticLockup(suite.Ctx, lockId, keeper.UnstakingSyntheticDenom(lock.Coins[0].Denom, valAddr)) - suite.Require().Error(err) - suite.Require().Nil(synthLock) - synthLock, err = suite.App.LockupKeeper.GetSyntheticLockup(suite.Ctx, newLock.ID, keeper.UnstakingSyntheticDenom(lock.Coins[0].Denom, valAddr)) - suite.Require().Error(err) - suite.Require().Nil(synthLock) - } - - // check invariant is fine - reason, broken := keeper.AllInvariants(*suite.App.SuperfluidKeeper)(suite.Ctx) - suite.Require().False(broken, reason) - - // check remaining intermediary account delegation - for index, expDelegation := range tc.expInterDelegation { - acc := intermediaryAccs[index] - valAddr, err := sdk.ValAddressFromBech32(acc.ValAddr) - suite.Require().NoError(err) - delegation, found := suite.App.StakingKeeper.GetDelegation(suite.Ctx, acc.GetAccAddress(), valAddr) - if expDelegation.IsZero() { - suite.Require().False(found, "expected no delegation, found delegation w/ %d shares", delegation.Shares) - } else { - suite.Require().True(found) - suite.Require().Equal(expDelegation, delegation.Shares) - } - } - }) - } -} - // type superfluidRedelegation struct { // lockId uint64 // oldValIndex int64 From d70e4e1059087e6eebbe62d05cdb247edad2de49 Mon Sep 17 00:00:00 2001 From: stackman27 Date: Tue, 18 Jul 2023 13:32:59 -0700 Subject: [PATCH 2/6] added changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1777d6906f..d8678f9f891 100644 --- 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 From a914d7125b14c67796758d15982de8a556d87390 Mon Sep 17 00:00:00 2001 From: stackman27 Date: Tue, 25 Jul 2023 01:01:15 -0700 Subject: [PATCH 3/6] adams feedback --- x/superfluid/keeper/migrate.go | 23 +++++++++-------------- x/superfluid/keeper/migrate_test.go | 1 + x/superfluid/types/errors.go | 9 +++++++++ 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/x/superfluid/keeper/migrate.go b/x/superfluid/keeper/migrate.go index 1a95842deb2..04e05b20171 100644 --- a/x/superfluid/keeper/migrate.go +++ b/x/superfluid/keeper/migrate.go @@ -316,20 +316,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) + 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 index 1a0d76bb1c3..5d4691c3d93 100644 --- a/x/superfluid/keeper/migrate_test.go +++ b/x/superfluid/keeper/migrate_test.go @@ -812,6 +812,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/types/errors.go b/x/superfluid/types/errors.go index 2e752968210..015e8d2f1ae 100644 --- a/x/superfluid/types/errors.go +++ b/x/superfluid/types/errors.go @@ -73,6 +73,15 @@ func (e MigrateMoreSharesThanLockHasError) Error() string { return fmt.Sprintf("cannot migrate more shares (%s) than lock has (%s)", e.SharesToMigrate, e.SharesInLock) } +type MigratePartialSharesError struct { + SharesToMigrate string + SharesInLock string +} + +func (e MigratePartialSharesError) Error() string { + return fmt.Sprintf("cannot partial migrate shares (%s). The lock has (%s)", e.SharesToMigrate, e.SharesInLock) +} + type TwoTokenBalancerPoolError struct { NumberOfTokens int } From 152f6a932e55c0df3bd81cf1a4bdff064c2a2c8d Mon Sep 17 00:00:00 2001 From: stackman27 Date: Tue, 25 Jul 2023 02:50:13 -0700 Subject: [PATCH 4/6] fixed test --- x/superfluid/keeper/migrate.go | 4 ++++ x/superfluid/keeper/migrate_test.go | 12 +++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/x/superfluid/keeper/migrate.go b/x/superfluid/keeper/migrate.go index 04e05b20171..a785f44d5a5 100644 --- 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. // diff --git a/x/superfluid/keeper/migrate_test.go b/x/superfluid/keeper/migrate_test.go index 5d4691c3d93..e8201b96c74 100644 --- a/x/superfluid/keeper/migrate_test.go +++ b/x/superfluid/keeper/migrate_test.go @@ -57,11 +57,13 @@ 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 @@ -98,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": { @@ -120,7 +122,7 @@ func (s *KeeperTestSuite) TestRouteLockedBalancerToConcentratedMigration() { // 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, }, @@ -129,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, }, From 6829d3055c733dcaaf8e8fb6d0063090dc3ee1fe Mon Sep 17 00:00:00 2001 From: stackman27 Date: Tue, 25 Jul 2023 13:01:00 -0700 Subject: [PATCH 5/6] adams comment --- x/superfluid/keeper/migrate.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/x/superfluid/keeper/migrate.go b/x/superfluid/keeper/migrate.go index a785f44d5a5..19557a59ab4 100644 --- a/x/superfluid/keeper/migrate.go +++ b/x/superfluid/keeper/migrate.go @@ -98,11 +98,7 @@ 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 - // 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 @@ -111,7 +107,7 @@ func (k Keeper) migrateSuperfluidBondedBalancerToConcentrated(ctx sdk.Context, // 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 } From aa5d7e3a112987f71064ce4b3d8a2168570d027f Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Tue, 25 Jul 2023 21:01:32 -0500 Subject: [PATCH 6/6] Update x/superfluid/keeper/migrate.go --- x/superfluid/keeper/migrate.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/superfluid/keeper/migrate.go b/x/superfluid/keeper/migrate.go index 19557a59ab4..e093f19397a 100644 --- a/x/superfluid/keeper/migrate.go +++ b/x/superfluid/keeper/migrate.go @@ -320,8 +320,7 @@ func (k Keeper) validateSharesToMigrateUnlockAndExitBalancerPool(ctx sdk.Context 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. + // Force migrate, which breaks and deletes associated synthetic locks. err = k.lk.ForceUnlock(ctx, *lock) if err != nil { return sdk.Coins{}, err