diff --git a/CHANGELOG.md b/CHANGELOG.md index aefce01f30c..8d02710d253 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased - v10.1.0 * [2011](https://github.com/osmosis-labs/osmosis/pull/2011) Fix bug in TokenFactory initGenesis, relating to denom creation fee param. +#### Golang API breaks +* [#1937](https://github.com/osmosis-labs/osmosis/pull/1937) Change `lockupKeeper.ExtendLock` to take in lockID instead of the direct lock struct. ## v10.0.1 diff --git a/x/lockup/keeper/lock.go b/x/lockup/keeper/lock.go index 2ac861bda89..a1445723b53 100644 --- 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 index 854b024dde4..edd22c9df23 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, 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 index bb7e478e93f..51b70411456 100644 --- 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{