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

[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 2 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)
}
24 changes: 7 additions & 17 deletions x/superfluid/keeper/migrate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,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 +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
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
63 changes: 0 additions & 63 deletions x/superfluid/keeper/migrate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,37 +68,19 @@ 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,
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 Down Expand Up @@ -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,
Expand Down Expand Up @@ -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"},
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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))),
Expand Down Expand Up @@ -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"),
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
160 changes: 0 additions & 160 deletions x/superfluid/keeper/stake_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down