x/lock: Fix ExtendLockup API (#1937)

* Fix extendlock api * Apply suggestions from code review * Add changelog Co-authored-by: Dev Ojha <[email protected]> Co-authored-by: Dev Ojha <[email protected]> (cherry picked from commit 99304f2) # Conflicts: # CHANGELOG.md
osmosis-labs · Jul 12, 2022 · 9b06110 · 9b06110
1 parent d733f64
commit 9b06110
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -50,7 +50,17 @@ This release contains minor CLI bug fixes.
 * Restores vesting by duration command
 * Fixes pagination in x/incentives module queries
 
+<<<<<<< HEAD
 ## v10.0.0
+=======
+* [#1937](https://github.com/osmosis-labs/osmosis/pull/1937) Change `lockupKeeper.ExtendLock` to take in lockID instead of the direct lock struct.
+* [#1893](https://github.com/osmosis-labs/osmosis/pull/1893) Change `EpochsKeeper.SetEpochInfo` to `AddEpochInfo`, which has more safety checks with it. (Makes it suitable to be called within upgrades)
+* [#1671](https://github.com/osmosis-labs/osmosis/pull/1671) Remove methods that constitute AppModuleSimulation APIs for several modules' AppModules, which implemented no-ops
+* [#1671](https://github.com/osmosis-labs/osmosis/pull/1671) Add hourly epochs to `x/epochs` DefaultGenesis.
+* [#1665](https://github.com/osmosis-labs/osmosis/pull/1665) Delete app/App interface, instead use simapp.App
+* [#1630](https://github.com/osmosis-labs/osmosis/pull/1630) Delete the v043_temp module, now that we're on an updated SDK version.
+* [#1667](https://github.com/osmosis-labs/osmosis/pull/1673) Move wasm-bindings code out of app .
+>>>>>>> 99304f2b (x/lock: Fix `ExtendLockup` API  (#1937))
 
 
 ## v9.0.1

diff --git a/x/lockup/keeper/lock.go b/x/lockup/keeper/lock.go
@@ -353,7 +353,16 @@ func (k Keeper) unlockMaturedLockInternalLogic(ctx sdk.Context, lock types.Perio
 // 2. Locks that are unlokcing are not allowed to change duration.
 // 3. Locks that have synthetic lockup are not allowed to change.
 // 4. Provided duration should be greater than the original duration.
-func (k Keeper) ExtendLockup(ctx sdk.Context, lock types.PeriodLock, newDuration time.Duration) error {
+func (k Keeper) ExtendLockup(ctx sdk.Context, lockID uint64, owner sdk.AccAddress, newDuration time.Duration) error {
+	lock, err := k.GetLockByID(ctx, lockID)
+	if err != nil {
+		return err
+	}
+
+	if lock.GetOwner() != owner.String() {
+		return types.ErrNotLockOwner
+	}
+
 	if lock.IsUnlocking() {
 		return fmt.Errorf("cannot edit unlocking lockup for lock %d", lock.ID)
 	}
@@ -363,10 +372,15 @@ func (k Keeper) ExtendLockup(ctx sdk.Context, lock types.PeriodLock, newDuration
 		return fmt.Errorf("cannot edit lockup with synthetic lock %d", lock.ID)
 	}
 
-	oldLock := lock
+	// completely delete existing lock refs
+	err = k.deleteLockRefs(ctx, unlockingPrefix(lock.IsUnlocking()), *lock)
+	if err != nil {
+		return err
+	}
 
+	oldDuration := lock.GetDuration()
 	if newDuration != 0 {
-		if newDuration <= lock.Duration {
+		if newDuration <= oldDuration {
 			return fmt.Errorf("new duration should be greater than the original")
 		}
 
@@ -379,25 +393,20 @@ func (k Keeper) ExtendLockup(ctx sdk.Context, lock types.PeriodLock, newDuration
 		lock.Duration = newDuration
 	}
 
-	// update lockup
-	err := k.deleteLockRefs(ctx, unlockingPrefix(oldLock.IsUnlocking()), oldLock)
-	if err != nil {
-		return err
-	}
-
-	err = k.addLockRefs(ctx, lock)
+	// add lock refs with the new duration
+	err = k.addLockRefs(ctx, *lock)
 	if err != nil {
 		return err
 	}
 
-	err = k.setLock(ctx, lock)
+	err = k.setLock(ctx, *lock)
 	if err != nil {
 		return err
 	}
 
 	k.hooks.OnLockupExtend(ctx,
 		lock.GetID(),
-		oldLock.GetDuration(),
+		oldDuration,
 		lock.GetDuration(),
 	)
 

diff --git a/x/lockup/keeper/lock_test.go b/x/lockup/keeper/lock_test.go
@@ -741,14 +741,14 @@ func (suite *KeeperTestSuite) TestEditLockup() {
 	lock, _ := suite.App.LockupKeeper.GetLockByID(suite.Ctx, 1)
 
 	// duration decrease should fail
-	err = suite.App.LockupKeeper.ExtendLockup(suite.Ctx, *lock, time.Second/2)
+	err = suite.App.LockupKeeper.ExtendLockup(suite.Ctx, lock.ID, addr, time.Second/2)
 	suite.Require().Error(err)
 	// extending lock with same duration should fail
-	err = suite.App.LockupKeeper.ExtendLockup(suite.Ctx, *lock, time.Second)
+	err = suite.App.LockupKeeper.ExtendLockup(suite.Ctx, lock.ID, addr, time.Second)
 	suite.Require().Error(err)
 
 	// duration increase should success
-	err = suite.App.LockupKeeper.ExtendLockup(suite.Ctx, *lock, time.Second*2)
+	err = suite.App.LockupKeeper.ExtendLockup(suite.Ctx, lock.ID, addr, time.Second*2)
 	suite.Require().NoError(err)
 
 	// check queries

diff --git a/x/lockup/keeper/msg_server.go b/x/lockup/keeper/msg_server.go
@@ -155,18 +155,19 @@ func createBeginUnlockEvent(lock *types.PeriodLock) sdk.Event {
 func (server msgServer) ExtendLockup(goCtx context.Context, msg *types.MsgExtendLockup) (*types.MsgExtendLockupResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
-	lock, err := server.keeper.GetLockByID(ctx, msg.ID)
+	owner, err := sdk.AccAddressFromBech32(msg.Owner)
 	if err != nil {
-		return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, err.Error())
+		return nil, err
 	}
 
-	if msg.Owner != lock.Owner {
-		return nil, sdkerrors.Wrapf(types.ErrNotLockOwner, fmt.Sprintf("msg sender (%s) and lock owner (%s) does not match", msg.Owner, lock.Owner))
+	err = server.keeper.ExtendLockup(ctx, msg.ID, owner, msg.Duration)
+	if err != nil {
+		return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, err.Error())
 	}
 
-	err = server.keeper.ExtendLockup(ctx, *lock, msg.Duration)
+	lock, err := server.keeper.GetLockByID(ctx, msg.ID)
 	if err != nil {
-		return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, err.Error())
+		return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, err.Error())
 	}
 
 	ctx.EventManager().EmitEvents(sdk.Events{