From 54f8d89cafef9a4092e7136d28892059a91049ec Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Mon, 1 Aug 2022 13:00:24 -0500 Subject: [PATCH] Revert "x/lock: Fix `ExtendLockup` API (backport #1937) (#2030)" This reverts commit bd32316c9f4d2ad63ca66ae2928489fcbc2b3b70. --- CHANGELOG.md | 3 +++ x/lockup/keeper/lock.go | 33 ++++++++++++--------------------- x/lockup/keeper/lock_test.go | 6 +++--- x/lockup/keeper/msg_server.go | 13 ++++++------- 4 files changed, 24 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b840983fd78..71edeb6f6c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 #### Bug Fixes * [2011](https://github.com/osmosis-labs/osmosis/pull/2011) Fix bug in TokenFactory initGenesis, relating to denom creation fee param. +<<<<<<< HEAD #### Improvements * [#2130](https://github.com/osmosis-labs/osmosis/pull/2130) Introduce errors in mint types. * [#2000](https://github.com/osmosis-labs/osmosis/pull/2000) Update import paths from v9 to v10. @@ -66,6 +67,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Bring back the cliff vesting command: https://github.com/osmosis-labs/cosmos-sdk/pull/272 * Allow ScheduleUpgrade to come from same block: https://github.com/osmosis-labs/cosmos-sdk/pull/261 +======= +>>>>>>> parent of bd32316c (x/lock: Fix `ExtendLockup` API (backport #1937) (#2030)) ## v10.0.1 diff --git a/x/lockup/keeper/lock.go b/x/lockup/keeper/lock.go index 3d43af871cd..03408013700 100644 --- a/x/lockup/keeper/lock.go +++ b/x/lockup/keeper/lock.go @@ -353,16 +353,7 @@ 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, 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 - } - +func (k Keeper) ExtendLockup(ctx sdk.Context, lock types.PeriodLock, newDuration time.Duration) error { if lock.IsUnlocking() { return fmt.Errorf("cannot edit unlocking lockup for lock %d", lock.ID) } @@ -372,15 +363,10 @@ func (k Keeper) ExtendLockup(ctx sdk.Context, lockID uint64, owner sdk.AccAddres return fmt.Errorf("cannot edit lockup with synthetic lock %d", lock.ID) } - // completely delete existing lock refs - err = k.deleteLockRefs(ctx, unlockingPrefix(lock.IsUnlocking()), *lock) - if err != nil { - return err - } + oldLock := lock - oldDuration := lock.GetDuration() if newDuration != 0 { - if newDuration <= oldDuration { + if newDuration <= lock.Duration { return fmt.Errorf("new duration should be greater than the original") } @@ -393,20 +379,25 @@ func (k Keeper) ExtendLockup(ctx sdk.Context, lockID uint64, owner sdk.AccAddres lock.Duration = newDuration } - // add lock refs with the new duration - err = k.addLockRefs(ctx, *lock) + // update lockup + err := k.deleteLockRefs(ctx, unlockingPrefix(oldLock.IsUnlocking()), oldLock) + if err != nil { + return err + } + + 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(), - oldDuration, + oldLock.GetDuration(), lock.GetDuration(), ) diff --git a/x/lockup/keeper/lock_test.go b/x/lockup/keeper/lock_test.go index edd22c9df23..854b024dde4 100644 --- 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.ID, addr, time.Second/2) + err = suite.App.LockupKeeper.ExtendLockup(suite.Ctx, *lock, time.Second/2) suite.Require().Error(err) // extending lock with same duration should fail - err = suite.App.LockupKeeper.ExtendLockup(suite.Ctx, lock.ID, addr, time.Second) + err = suite.App.LockupKeeper.ExtendLockup(suite.Ctx, *lock, time.Second) suite.Require().Error(err) // duration increase should success - err = suite.App.LockupKeeper.ExtendLockup(suite.Ctx, lock.ID, addr, time.Second*2) + err = suite.App.LockupKeeper.ExtendLockup(suite.Ctx, *lock, 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 index 51b70411456..bb7e478e93f 100644 --- a/x/lockup/keeper/msg_server.go +++ b/x/lockup/keeper/msg_server.go @@ -155,19 +155,18 @@ func createBeginUnlockEvent(lock *types.PeriodLock) sdk.Event { func (server msgServer) ExtendLockup(goCtx context.Context, msg *types.MsgExtendLockup) (*types.MsgExtendLockupResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) - owner, err := sdk.AccAddressFromBech32(msg.Owner) + lock, err := server.keeper.GetLockByID(ctx, msg.ID) if err != nil { - return nil, err + return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, err.Error()) } - err = server.keeper.ExtendLockup(ctx, msg.ID, owner, msg.Duration) - if err != nil { - return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, err.Error()) + 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)) } - lock, err := server.keeper.GetLockByID(ctx, msg.ID) + err = server.keeper.ExtendLockup(ctx, *lock, msg.Duration) if err != nil { - return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, err.Error()) + return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, err.Error()) } ctx.EventManager().EmitEvents(sdk.Events{