Add unlocking lock id to BeginUnlocking response (#4489)

* added unlocking lock id

* added changelog

(cherry picked from commit d5f8210)

# Conflicts:
#	x/superfluid/keeper/migrate_test.go

CHANGELOG.md

@@ -40,6 +40,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### API Breaks
+
+ * [#4489](https://github.com/osmosis-labs/osmosis/pull/4489) Add unlockingLockId to BeginUnlocking response
+
 ## v15.0.0
 
 This release containts the following new modules:

proto/osmosis/lockup/tx.proto

@@ -51,7 +51,10 @@ message MsgBeginUnlocking {
     (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"
   ];
 }
-message MsgBeginUnlockingResponse { bool success = 1; }
+message MsgBeginUnlockingResponse {
+  bool success = 1;
+  uint64 unlockingLockID = 2;
+}
 
 // MsgExtendLockup extends the existing lockup's duration.
 // The new duration is longer than the original.

x/lockup/keeper/iterator.go

@@ -218,7 +218,7 @@ func (k Keeper) beginUnlockFromIterator(ctx sdk.Context, iterator db.Iterator) (
 
 	locks := k.getLocksFromIterator(ctx, iterator)
 	for _, lock := range locks {
-		err := k.BeginUnlock(ctx, lock.ID, nil)
+		_, err := k.BeginUnlock(ctx, lock.ID, nil)
 		if err != nil {
 			return locks, err
 		}

x/lockup/keeper/lock.go

@@ -161,19 +161,19 @@ func (k Keeper) lock(ctx sdk.Context, lock types.PeriodLock, tokensToLock sdk.Co
 
 // BeginUnlock is a utility to start unlocking coins from NotUnlocking queue.
 // Returns an error if the lock has a synthetic lock.
-func (k Keeper) BeginUnlock(ctx sdk.Context, lockID uint64, coins sdk.Coins) error {
+func (k Keeper) BeginUnlock(ctx sdk.Context, lockID uint64, coins sdk.Coins) (uint64, error) {
 	// prohibit BeginUnlock if synthetic locks are referring to this
 	// TODO: In the future, make synthetic locks only get partial restrictions on the main lock.
 	lock, err := k.GetLockByID(ctx, lockID)
 	if err != nil {
-		return err
+		return 0, err
 	}
 	if k.HasAnySyntheticLockups(ctx, lock.ID) {
-		return fmt.Errorf("cannot BeginUnlocking a lock with synthetic lockup")
+		return 0, fmt.Errorf("cannot BeginUnlocking a lock with synthetic lockup")
 	}
 
-	_, err = k.beginUnlock(ctx, *lock, coins)
-	return err
+	unlockingLock, err := k.beginUnlock(ctx, *lock, coins)
+	return unlockingLock, err
 }
 
 // BeginForceUnlock begins force unlock of the given lock.
@@ -356,7 +356,7 @@ func (k Keeper) ForceUnlock(ctx sdk.Context, lock types.PeriodLock) error {
 	}
 
 	if !lock.IsUnlocking() {
-		err := k.BeginUnlock(ctx, lock.ID, nil)
+		_, err := k.BeginUnlock(ctx, lock.ID, nil)
 		if err != nil {
 			return err
 		}

x/lockup/keeper/lock_test.go

@@ -97,7 +97,7 @@ func (suite *KeeperTestSuite) TestBeginForceUnlock() {
 				if tc.expectSameLockID {
 					suite.Require().Equal(lockID, lock.ID)
 				} else {
-					suite.Require().Equal(lockID, lock.ID + 1)
+					suite.Require().Equal(lockID, lock.ID+1)
 				}
 
 				// new or updated lock
@@ -139,6 +139,7 @@ func (suite *KeeperTestSuite) TestUnlock() {
 		passedTime                    time.Duration
 		expectedUnlockMaturedLockPass bool
 		balanceAfterUnlock            sdk.Coins
+		isPartial                     bool
 	}{
 		{
 			name:                          "normal unlocking case",
@@ -177,6 +178,7 @@ func (suite *KeeperTestSuite) TestUnlock() {
 			passedTime:                    time.Second,
 			expectedUnlockMaturedLockPass: true,
 			balanceAfterUnlock:            sdk.Coins{sdk.NewInt64Coin("stake", 5)},
+			isPartial:                     true,
 		},
 		{
 			name:                          "partial unlocking unknown tokens",
@@ -213,11 +215,15 @@ func (suite *KeeperTestSuite) TestUnlock() {
 		partialUnlocking := tc.unlockingCoins.IsAllLT(initialLockCoins) && tc.unlockingCoins != nil
 
 		// begin unlocking
-		err = lockupKeeper.BeginUnlock(ctx, lock.ID, tc.unlockingCoins)
+		unlockingLock, err := lockupKeeper.BeginUnlock(ctx, lock.ID, tc.unlockingCoins)
 
 		if tc.expectedBeginUnlockPass {
 			suite.Require().NoError(err)
 
+			if tc.isPartial {
+				suite.Require().Equal(unlockingLock, lock.ID+1)
+			}
+
 			// check unlocking coins. When a lock is a partial lock
 			// (i.e. tc.unlockingCoins is not nit and less than initialLockCoins),
 			// we only unlock the partial amount of tc.unlockingCoins

x/lockup/keeper/msg_server.go

@@ -91,14 +91,14 @@ func (server msgServer) BeginUnlocking(goCtx context.Context, msg *types.MsgBegi
 		return nil, sdkerrors.Wrap(types.ErrNotLockOwner, fmt.Sprintf("msg sender (%s) and lock owner (%s) does not match", msg.Owner, lock.Owner))
 	}
 
-	err = server.keeper.BeginUnlock(ctx, lock.ID, msg.Coins)
+	unlockingLock, err := server.keeper.BeginUnlock(ctx, lock.ID, msg.Coins)
 	if err != nil {
 		return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, err.Error())
 	}
 
 	// N.B. begin unlock event is emitted downstream in the keeper method.
 
-	return &types.MsgBeginUnlockingResponse{}, nil
+	return &types.MsgBeginUnlockingResponse{Success: true, UnlockingLockID: unlockingLock}, nil
 }
 
 // BeginUnlockingAll begins unlocking for all the locks that the account has by iterating all the not-unlocking locks the account holds.

x/lockup/keeper/msg_server_test.go

@@ -115,6 +115,7 @@ func (suite *KeeperTestSuite) TestMsgBeginUnlocking() {
 		name       string
 		param      param
 		expectPass bool
+		isPartial  bool
 	}{
 		{
 			name: "unlock full amount of tokens via begin unlock",
@@ -138,6 +139,7 @@ func (suite *KeeperTestSuite) TestMsgBeginUnlocking() {
 				duration:            time.Second,
 				coinsInOwnerAddress: sdk.Coins{sdk.NewInt64Coin("stake", 10)},
 			},
+			isPartial:  true,
 			expectPass: true,
 		},
 		{
@@ -163,6 +165,7 @@ func (suite *KeeperTestSuite) TestMsgBeginUnlocking() {
 				coinsInOwnerAddress: sdk.Coins{sdk.NewInt64Coin("stake", 10)},
 			},
 			expectPass: false,
+			isPartial:  true,
 		},
 	}
 
@@ -181,11 +184,17 @@ func (suite *KeeperTestSuite) TestMsgBeginUnlocking() {
 			suite.Require().NoError(err)
 		}
 
-		_, err = msgServer.BeginUnlocking(goCtx, types.NewMsgBeginUnlocking(test.param.lockOwner, resp.ID, test.param.coinsToUnlock))
+		unlockingResponse, err := msgServer.BeginUnlocking(goCtx, types.NewMsgBeginUnlocking(test.param.lockOwner, resp.ID, test.param.coinsToUnlock))
 
 		if test.expectPass {
 			suite.Require().NoError(err)
 			suite.AssertEventEmitted(suite.Ctx, types.TypeEvtBeginUnlock, 1)
+			suite.Require().True(unlockingResponse.Success)
+			if test.isPartial {
+				suite.Require().Equal(unlockingResponse.UnlockingLockID, resp.ID+1)
+			} else {
+				suite.Require().Equal(unlockingResponse.UnlockingLockID, resp.ID)
+			}
 		} else {
 			suite.Require().Error(err)
 			suite.AssertEventEmitted(suite.Ctx, types.TypeEvtBeginUnlock, 0)

x/superfluid/keeper/edge_case_test.go

@@ -136,7 +136,7 @@ func (suite *KeeperTestSuite) TestTryUnbondingSuperfluidLockupDirectly() {
 			_, _, locks := suite.setupSuperfluidDelegations(valAddrs, tc.superDelegations, denoms)
 
 			for _, lock := range locks {
-				err := suite.App.LockupKeeper.BeginUnlock(suite.Ctx, lock.ID, sdk.Coins{})
+				_, err := suite.App.LockupKeeper.BeginUnlock(suite.Ctx, lock.ID, sdk.Coins{})
 				suite.Require().Error(err)
 			}
 		})