From 18f4bd7e81d8dd238fc541fa4d9cd840ca82d0ed Mon Sep 17 00:00:00 2001 From: mattverse Date: Fri, 11 Aug 2023 22:27:08 +0900 Subject: [PATCH] Roman and Adam --- x/superfluid/keeper/stake.go | 16 ++++++++++----- x/superfluid/keeper/stake_test.go | 33 +++++++++++++++++++++++++++---- x/superfluid/types/msg_test.go | 10 +++++++++- 3 files changed, 49 insertions(+), 10 deletions(-) diff --git a/x/superfluid/keeper/stake.go b/x/superfluid/keeper/stake.go index 2c23745fed6..623eeebbe59 100644 --- a/x/superfluid/keeper/stake.go +++ b/x/superfluid/keeper/stake.go @@ -610,10 +610,12 @@ func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn // Delegation is done in the following logic: // - If valAddr provided, single delegate. // - If valAddr not provided and valset exists, valsetpref.Delegate -// - If valAddr not provided and valset delegation is not possible, refer back to original lock's superfluid validator +// - If valAddr not provided and valset delegation is not possible, refer back to original lock's superfluid validator if it was a superfluid lock // - Else: error func (k Keeper) UnbondConvertAndStake(ctx sdk.Context, lockID uint64, sender, valAddr string, minAmtToStake sdk.Int, sharesToConvert sdk.Coin) (totalAmtConverted sdk.Int, err error) { + ctx.Logger().Error(sharesToConvert.String()) + ctx.Logger().Error(minAmtToStake.String()) senderAddr, err := sdk.AccAddressFromBech32(sender) if err != nil { return sdk.Int{}, err @@ -635,9 +637,9 @@ func (k Keeper) UnbondConvertAndStake(ctx sdk.Context, lockID uint64, sender, va if migrationType == SuperfluidBonded || migrationType == SuperfluidUnbonding || migrationType == NonSuperfluid { totalAmtConverted, err = k.convertLockToStake(ctx, senderAddr, valAddr, lockID, minAmtToStake) - } else if migrationType == Unlocked { + } else if migrationType == Unlocked { // liquid gamm shares without locks totalAmtConverted, err = k.convertUnlockedToStake(ctx, senderAddr, valAddr, sharesToConvert, minAmtToStake) - } else { // liquid gamm shares without locks are not supported + } else { // any other types of migration should fail return sdk.Int{}, fmt.Errorf("unsupported staking conversion type") } @@ -699,6 +701,8 @@ func (k Keeper) convertLockToStake(ctx sdk.Context, sender sdk.AccAddress, valAd return totalAmtConverted, nil } +// convertUnlockedToStake converts liquid gamm shares to staking delegation. +// minAmtToStake works as slippage bound for the conversion process. func (k Keeper) convertUnlockedToStake(ctx sdk.Context, sender sdk.AccAddress, valAddr string, sharesToStake sdk.Coin, minAmtToStake sdk.Int) (totalAmtConverted sdk.Int, err error) { if !strings.HasPrefix(sharesToStake.Denom, gammtypes.GAMMTokenPrefix) { @@ -729,7 +733,8 @@ func (k Keeper) convertUnlockedToStake(ctx sdk.Context, sender sdk.AccAddress, v // convertGammSharesToOsmoAndStake converts given gamm shares to osmo by swapping in the given pool // then stakes it to the designated validator. // minAmtToStake works as slippage bound, and would error if total amount being staked is less than min amount to stake. -// Actual delegation is done based on input parameters of delegateBaseOnValsetPref. +// Depending on user inputs, valAddr and originalSuperfluidValAddr could be an empty string, +// each leading to a different delegation scenario. func (k Keeper) convertGammSharesToOsmoAndStake( ctx sdk.Context, sender sdk.AccAddress, valAddr string, @@ -780,10 +785,11 @@ func (k Keeper) convertGammSharesToOsmoAndStake( } // delegateBaseOnValsetPref delegates based on given input parameters. +// valAddr and originalSuperfluidValAddr can be an empty string depending on user input and original lock's status. // Delegation is done in the following logic: // - If valAddr provided, single delegate. // - If valAddr not provided and valset exists, valsetpref.Delegate -// - If valAddr not provided and valset delegation is not possible, refer back to original lock's superfluid validator +// - If valAddr not provided and valset delegation is not possible, refer back to original lock's superfluid validator if it was a superfluid lock // - Else: error func (k Keeper) delegateBaseOnValsetPref(ctx sdk.Context, sender sdk.AccAddress, valAddr, originalSuperfluidValAddr string, totalAmtToStake sdk.Int) error { bondDenom := k.sk.BondDenom(ctx) diff --git a/x/superfluid/keeper/stake_test.go b/x/superfluid/keeper/stake_test.go index 14776115f1e..183f7632d4e 100644 --- a/x/superfluid/keeper/stake_test.go +++ b/x/superfluid/keeper/stake_test.go @@ -1038,6 +1038,7 @@ func (s *KeeperTestSuite) TestUnbondConvertAndStake() { superfluidUndelegating bool unlocking bool unlocked bool + testCLLock bool expectedError error } testCases := map[string]tc{ @@ -1056,16 +1057,40 @@ func (s *KeeperTestSuite) TestUnbondConvertAndStake() { notSuperfluidDelegated: true, unlocked: true, }, + "error: concentrated lock should fail": { + testCLLock: true, + expectedError: types.SharesToMigrateDenomPrefixError{ + Denom: "cl/pool/2", + ExpectedDenomPrefix: "gamm/pool/", + }, + }, } for name, tc := range testCases { s.Run(name, func() { s.SetupTest() s.Ctx = s.Ctx.WithBlockTime(defaultJoinTime) - // We bundle all migration setup into a single function to avoid repeating the same code for each test case. - _, _, lock, _, joinPoolAcc, _, balancerShareOut, originalValAddr := s.SetupUnbondConvertAndStakeTest(s.Ctx, !tc.notSuperfluidDelegated, tc.superfluidUndelegating, tc.unlocking, tc.unlocked) - // testing params + var ( + lock *lockuptypes.PeriodLock + lockId uint64 + joinPoolAcc sdk.AccAddress + originalValAddr sdk.ValAddress + balancerShareOut sdk.Coin + ) + // we use migration setup for testing with cl lock + if tc.testCLLock { + _, _, lock, _, joinPoolAcc, _, _, balancerShareOut, originalValAddr = s.SetupMigrationTest(s.Ctx, !tc.notSuperfluidDelegated, tc.superfluidUndelegating, tc.unlocking, tc.unlocked, sdk.MustNewDecFromStr("1")) + synthLockBeforeMigration, _, err := s.App.SuperfluidKeeper.GetMigrationType(s.Ctx, joinPoolAcc, int64(lock.ID)) + s.Require().NoError(err) + _, lockId, _, _, err = s.App.SuperfluidKeeper.MigrateSuperfluidBondedBalancerToConcentrated(s.Ctx, joinPoolAcc, lock.ID, lock.Coins[0], synthLockBeforeMigration.SynthDenom, sdk.NewCoins()) + s.Require().NoError(err) + } else { + // We bundle all migration setup into a single function to avoid repeating the same code for each test case. + _, _, lock, _, joinPoolAcc, _, balancerShareOut, originalValAddr = s.SetupUnbondConvertAndStakeTest(s.Ctx, !tc.notSuperfluidDelegated, tc.superfluidUndelegating, tc.unlocking, tc.unlocked) + lockId = lock.ID + } + sender := sdk.MustAccAddressFromBech32(joinPoolAcc.String()) valAddr := s.SetupValidator(stakingtypes.Bonded) minAmountToStake := sdk.ZeroInt() @@ -1078,7 +1103,7 @@ func (s *KeeperTestSuite) TestUnbondConvertAndStake() { balanceBeforeConvertLockToStake := s.App.BankKeeper.GetAllBalances(s.Ctx, sender).FilterDenoms([]string{"foo", "stake", "uosmo"}) // system under test - totalAmtConverted, err := s.App.SuperfluidKeeper.UnbondConvertAndStake(s.Ctx, lock.ID, sender.String(), valAddr.String(), minAmountToStake, sharesToConvert) + totalAmtConverted, err := s.App.SuperfluidKeeper.UnbondConvertAndStake(s.Ctx, lockId, sender.String(), valAddr.String(), minAmountToStake, sharesToConvert) if tc.expectedError != nil { s.Require().Equal(err.Error(), tc.expectedError.Error()) s.Require().Error(err) diff --git a/x/superfluid/types/msg_test.go b/x/superfluid/types/msg_test.go index cf302f68fd9..761a1792d46 100644 --- a/x/superfluid/types/msg_test.go +++ b/x/superfluid/types/msg_test.go @@ -108,6 +108,15 @@ func TestUnbondConvertAndStakeMsg(t *testing.T) { SharesToConvert: sdk.NewInt64Coin("foo", 10), }, }, + { + name: "no val address should not fail", + msg: &types.MsgUnbondConvertAndStake{ + LockId: 0, + Sender: addr1, + MinAmtToStake: sdk.NewInt(10), + SharesToConvert: sdk.NewInt64Coin("foo", 10), + }, + }, { name: "err: sender is invalid", msg: &types.MsgUnbondConvertAndStake{ @@ -124,7 +133,6 @@ func TestUnbondConvertAndStakeMsg(t *testing.T) { msg: &types.MsgUnbondConvertAndStake{ LockId: 0, Sender: addr1, - ValAddr: "abcd", MinAmtToStake: sdk.NewInt(10).Neg(), SharesToConvert: sdk.NewInt64Coin("foo", 10), },