diff --git a/CHANGELOG.md b/CHANGELOG.md
index b840983fd78d63a58164332f38bfbcdad0e8bf5e..276330608a3bb64b2b31be79526f4930719929bb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -40,6 +40,18 @@ 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).
+## v10.2.0
+
+In v10.1.0 a refactor did changed the ordering of certain function calls in a message execution.
+The data read and written to in state were the same, but re-ordered.
+This unknowingly caused gas_used incompatabilities, because you could out-of-gas error at any intermediate step, and end in a different gas_used quantity. cref: https://github.com/cosmos/cosmos-sdk/issues/12788
+
+The v10.2 line reverts the gas limit state incompatabilities introduced due to function ordering.
+Namely this reverts
+
+* [#1937](https://github.com/osmosis-labs/osmosis/pull/1937) Change `lockupKeeper.ExtendLock` to take in lockID instead of the direct lock struct.
+* [#2030](https://github.com/osmosis-labs/osmosis/pull/2030) Rename lockup keeper `ResetAllLocks` to `InitializeAllLocks` and `ResetAllSyntheticLocks` to `InitializeAllSyntheticLocks`.
+
## v10.1.1
#### Improvements
diff --git a/x/lockup/keeper/admin_keeper_test.go b/x/lockup/keeper/admin_keeper_test.go
index 84a4af63362635d0c4fb0f78e6ff39897f578ff5..c10826881e89e467e6f44340cd0281c1e9ad2762 100644
--- a/x/lockup/keeper/admin_keeper_test.go
+++ b/x/lockup/keeper/admin_keeper_test.go
@@ -4,6 +4,7 @@ import (
"time"
"github.com/osmosis-labs/osmosis/v10/x/lockup/keeper"
+ "github.com/osmosis-labs/osmosis/v10/x/lockup/types"
sdk "github.com/cosmos/cosmos-sdk/types"
)
@@ -13,10 +14,11 @@ func (suite *KeeperTestSuite) TestRelock() {
addr1 := sdk.AccAddress([]byte("addr1---------------"))
coins := sdk.Coins{sdk.NewInt64Coin("stake", 10)}
+ lock := types.NewPeriodLock(1, addr1, time.Second, suite.Ctx.BlockTime().Add(time.Second), coins)
// lock with balance
suite.FundAcc(addr1, coins)
- lock, err := suite.App.LockupKeeper.CreateLock(suite.Ctx, addr1, coins, time.Second)
+ err := suite.App.LockupKeeper.Lock(suite.Ctx, lock)
suite.Require().NoError(err)
// lock with balance with same id
@@ -36,12 +38,12 @@ func (suite *KeeperTestSuite) BreakLock() {
addr1 := sdk.AccAddress([]byte("addr1---------------"))
coins := sdk.Coins{sdk.NewInt64Coin("stake", 10)}
+ lock := types.NewPeriodLock(1, addr1, time.Second, suite.Ctx.BlockTime().Add(time.Second), coins)
// lock with balance
suite.FundAcc(addr1, coins)
- lock, err := suite.App.LockupKeeper.CreateLock(suite.Ctx, addr1, coins, time.Second)
-
+ err := suite.App.LockupKeeper.Lock(suite.Ctx, lock)
suite.Require().NoError(err)
// break lock
diff --git a/x/lockup/keeper/export_test.go b/x/lockup/keeper/export_test.go
index 369c6b1812eef4394914bdd3c489cebc679e501e..bbb648e6a1924affb76c7abd600cdf2d1c791f77 100644
--- a/x/lockup/keeper/export_test.go
+++ b/x/lockup/keeper/export_test.go
@@ -25,7 +25,3 @@ func (k Keeper) SyntheticCoins(coins sdk.Coins, suffix string) sdk.Coins {
func (k Keeper) GetCoinsFromLocks(locks []types.PeriodLock) sdk.Coins {
return k.getCoinsFromLocks(locks)
}
-
-func (k Keeper) Lock(ctx sdk.Context, lock types.PeriodLock, tokensToLock sdk.Coins) error {
- return k.lock(ctx, lock, tokensToLock)
-}
diff --git a/x/lockup/keeper/lock.go b/x/lockup/keeper/lock.go
index 3d43af871cd1c7d39c633018092c459cca277adf..826656ec49ab9cfc11188cf84919be10c5780fe3 100644
--- a/x/lockup/keeper/lock.go
+++ b/x/lockup/keeper/lock.go
@@ -66,79 +66,100 @@ func (k Keeper) AddToExistingLock(ctx sdk.Context, owner sdk.AccAddress, coin sd
// Tokens locked are sent and kept in the module account.
// This method alters the lock state in store, thus we do a sanity check to ensure
// lock owner matches the given owner.
-func (k Keeper) AddTokensToLockByID(ctx sdk.Context, lockID uint64, owner sdk.AccAddress, tokensToAdd sdk.Coin) (*types.PeriodLock, error) {
+func (k Keeper) AddTokensToLockByID(ctx sdk.Context, lockID uint64, owner sdk.AccAddress, coin sdk.Coin) (*types.PeriodLock, error) {
lock, err := k.GetLockByID(ctx, lockID)
- if err != nil {
- return nil, err
- }
if lock.GetOwner() != owner.String() {
return nil, types.ErrNotLockOwner
}
- lock.Coins = lock.Coins.Add(tokensToAdd)
- err = k.lock(ctx, *lock, sdk.NewCoins(tokensToAdd))
if err != nil {
return nil, err
}
+ if err := k.bk.SendCoinsFromAccountToModule(ctx, lock.OwnerAddress(), types.ModuleName, sdk.NewCoins(coin)); err != nil {
+ return nil, err
+ }
- for _, synthlock := range k.GetAllSyntheticLockupsByLockup(ctx, lock.ID) {
- k.accumulationStore(ctx, synthlock.SynthDenom).Increase(accumulationKey(synthlock.Duration), tokensToAdd.Amount)
+ err = k.addTokenToLock(ctx, lock, coin)
+ if err != nil {
+ return nil, err
}
if k.hooks == nil {
return lock, nil
}
- k.hooks.AfterAddTokensToLock(ctx, lock.OwnerAddress(), lock.GetID(), sdk.NewCoins(tokensToAdd))
-
+ k.hooks.OnTokenLocked(ctx, lock.OwnerAddress(), lock.ID, sdk.Coins{coin}, lock.Duration, lock.EndTime)
return lock, nil
}
+// addTokenToLock adds token to lock and modifies the state of the lock and the accumulation store.
+func (k Keeper) addTokenToLock(ctx sdk.Context, lock *types.PeriodLock, coin sdk.Coin) error {
+ lock.Coins = lock.Coins.Add(coin)
+
+ err := k.setLock(ctx, *lock)
+ if err != nil {
+ return err
+ }
+
+ // modifications to accumulation store
+ k.accumulationStore(ctx, coin.Denom).Increase(accumulationKey(lock.Duration), coin.Amount)
+
+ // CONTRACT: lock will have synthetic lock only if it has a single coin
+ // accumulation store for its synthetic denom is increased if exists.
+ lockedCoin, err := lock.SingleCoin()
+ if err == nil {
+ for _, synthlock := range k.GetAllSyntheticLockupsByLockup(ctx, lock.ID) {
+ k.accumulationStore(ctx, synthlock.SynthDenom).Increase(accumulationKey(synthlock.Duration), sdk.NewCoins(coin).AmountOf(lockedCoin.Denom))
+ }
+ }
+
+ k.hooks.AfterAddTokensToLock(ctx, lock.OwnerAddress(), lock.GetID(), sdk.NewCoins(coin))
+
+ return nil
+}
+
// CreateLock creates a new lock with the specified duration for the owner.
-// Returns an error in the following conditions:
-// - account does not have enough balance
func (k Keeper) CreateLock(ctx sdk.Context, owner sdk.AccAddress, coins sdk.Coins, duration time.Duration) (types.PeriodLock, error) {
ID := k.GetLastLockID(ctx) + 1
// unlock time is initially set without a value, gets set as unlock start time + duration
// when unlocking starts.
lock := types.NewPeriodLock(ID, owner, duration, time.Time{}, coins)
- err := k.lock(ctx, lock, lock.Coins)
+ err := k.Lock(ctx, lock)
if err != nil {
return lock, err
}
-
- // add lock refs into not unlocking queue
- err = k.addLockRefs(ctx, lock)
- if err != nil {
- return lock, err
- }
-
k.SetLastLockID(ctx, lock.ID)
return lock, nil
}
-// lock is an internal utility to lock coins and set corresponding states.
-// This is only called by either of the two possible entry points to lock tokens.
-// 1. CreateLock
-// 2. AddTokensToLockByID
-func (k Keeper) lock(ctx sdk.Context, lock types.PeriodLock, tokensToLock sdk.Coins) error {
+// Lock is a utility method to lock tokens into the module account. This method includes setting the
+// lock within the state machine and increasing the value of accumulation store.
+func (k Keeper) Lock(ctx sdk.Context, lock types.PeriodLock) error {
owner, err := sdk.AccAddressFromBech32(lock.Owner)
if err != nil {
return err
}
- if err := k.bk.SendCoinsFromAccountToModule(ctx, owner, types.ModuleName, tokensToLock); err != nil {
+ if err := k.bk.SendCoinsFromAccountToModule(ctx, owner, types.ModuleName, lock.Coins); err != nil {
return err
}
// store lock object into the store
- err = k.setLock(ctx, lock)
+ store := ctx.KVStore(k.storeKey)
+ bz, err := proto.Marshal(&lock)
+ if err != nil {
+ return err
+ }
+ store.Set(lockStoreKey(lock.ID), bz)
+
+ // add lock refs into not unlocking queue
+ err = k.addLockRefs(ctx, lock)
if err != nil {
return err
}
// add to accumulation store
- for _, coin := range tokensToLock {
+ for _, coin := range lock.Coins {
k.accumulationStore(ctx, coin.Denom).Increase(accumulationKey(lock.Duration), coin.Amount)
}
@@ -353,16 +374,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 +384,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 +400,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.setLock(ctx, *lock)
+ err = k.addLockRefs(ctx, lock)
+ if err != nil {
+ return err
+ }
+
+ 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 edd22c9df2352b36c58a5e1d778341a26d1524a7..e5ea3ec8e4f680c84da9949da23dcc167838bdeb 100644
--- a/x/lockup/keeper/lock_test.go
+++ b/x/lockup/keeper/lock_test.go
@@ -144,6 +144,35 @@ func (suite *KeeperTestSuite) TestUnlockPeriodLockByID() {
suite.Require().Len(locks, 0)
}
+func (suite *KeeperTestSuite) TestLock() {
+ // test for coin locking
+ suite.SetupTest()
+
+ addr1 := sdk.AccAddress([]byte("addr1---------------"))
+ coins := sdk.Coins{sdk.NewInt64Coin("stake", 10)}
+ lock := types.NewPeriodLock(1, addr1, time.Second, suite.Ctx.BlockTime().Add(time.Second), coins)
+
+ // try lock without balance
+ err := suite.App.LockupKeeper.Lock(suite.Ctx, lock)
+ suite.Require().Error(err)
+
+ // lock with balance
+ suite.FundAcc(addr1, coins)
+ err = suite.App.LockupKeeper.Lock(suite.Ctx, lock)
+ suite.Require().NoError(err)
+
+ // lock with balance with same id
+ suite.FundAcc(addr1, coins)
+ err = suite.App.LockupKeeper.Lock(suite.Ctx, lock)
+ suite.Require().Error(err)
+
+ // lock with balance with different id
+ lock = types.NewPeriodLock(2, addr1, time.Second, suite.Ctx.BlockTime().Add(time.Second), coins)
+ suite.FundAcc(addr1, coins)
+ err = suite.App.LockupKeeper.Lock(suite.Ctx, lock)
+ suite.Require().NoError(err)
+}
+
func (suite *KeeperTestSuite) TestUnlock() {
// test for coin unlocking
suite.SetupTest()
@@ -151,10 +180,11 @@ func (suite *KeeperTestSuite) TestUnlock() {
addr1 := sdk.AccAddress([]byte("addr1---------------"))
coins := sdk.Coins{sdk.NewInt64Coin("stake", 10)}
+ lock := types.NewPeriodLock(1, addr1, time.Second, time.Time{}, coins)
// lock with balance
suite.FundAcc(addr1, coins)
- lock, err := suite.App.LockupKeeper.CreateLock(suite.Ctx, addr1, coins, time.Second)
+ err := suite.App.LockupKeeper.Lock(suite.Ctx, lock)
suite.Require().NoError(err)
// begin unlock with lock object
@@ -290,65 +320,6 @@ func (suite *KeeperTestSuite) TestLocksLongerThanDurationDenom() {
suite.Require().Len(locks, 1)
}
-func (suite *KeeperTestSuite) TestCreateLock() {
- suite.SetupTest()
-
- addr1 := sdk.AccAddress([]byte("addr1---------------"))
- coins := sdk.Coins{sdk.NewInt64Coin("stake", 10)}
-
- // test locking without balance
- _, err := suite.App.LockupKeeper.CreateLock(suite.Ctx, addr1, coins, time.Second)
- suite.Require().Error(err)
-
- suite.FundAcc(addr1, coins)
-
- lock, err := suite.App.LockupKeeper.CreateLock(suite.Ctx, addr1, coins, time.Second)
- suite.Require().NoError(err)
-
- // check new lock
- suite.Require().Equal(coins, lock.Coins)
- suite.Require().Equal(time.Second, lock.Duration)
- suite.Require().Equal(time.Time{}, lock.EndTime)
- suite.Require().Equal(uint64(1), lock.ID)
-
- lockID := suite.App.LockupKeeper.GetLastLockID(suite.Ctx)
- suite.Require().Equal(uint64(1), lockID)
-
- // check accumulation store
- accum := suite.App.LockupKeeper.GetPeriodLocksAccumulation(suite.Ctx, types.QueryCondition{
- LockQueryType: types.ByDuration,
- Denom: "stake",
- Duration: time.Second,
- })
- suite.Require().Equal(accum.String(), "10")
-
- // create new lock
- coins = sdk.Coins{sdk.NewInt64Coin("stake", 20)}
- suite.FundAcc(addr1, coins)
-
- lock, err = suite.App.LockupKeeper.CreateLock(suite.Ctx, addr1, coins, time.Second)
- suite.Require().NoError(err)
-
- lockID = suite.App.LockupKeeper.GetLastLockID(suite.Ctx)
- suite.Require().Equal(uint64(2), lockID)
-
- // check accumulation store
- accum = suite.App.LockupKeeper.GetPeriodLocksAccumulation(suite.Ctx, types.QueryCondition{
- LockQueryType: types.ByDuration,
- Denom: "stake",
- Duration: time.Second,
- })
- suite.Require().Equal(accum.String(), "30")
-
- // check balance
- balance := suite.App.BankKeeper.GetBalance(suite.Ctx, addr1, "stake")
- suite.Require().Equal(sdk.ZeroInt(), balance.Amount)
-
- acc := suite.App.AccountKeeper.GetModuleAccount(suite.Ctx, types.ModuleName)
- balance = suite.App.BankKeeper.GetBalance(suite.Ctx, acc.GetAddress(), "stake")
- suite.Require().Equal(sdk.NewInt(30), balance.Amount)
-}
-
func (suite *KeeperTestSuite) TestAddTokensToLock() {
suite.SetupTest()
@@ -410,52 +381,6 @@ func (suite *KeeperTestSuite) TestAddTokensToLock() {
suite.Require().Error(err)
}
-func (suite *KeeperTestSuite) TestLock() {
- suite.SetupTest()
-
- addr1 := sdk.AccAddress([]byte("addr1---------------"))
- coins := sdk.Coins{sdk.NewInt64Coin("stake", 10)}
-
- lock := types.PeriodLock{
- ID: 1,
- Owner: addr1.String(),
- Duration: time.Second,
- EndTime: time.Time{},
- Coins: coins,
- }
-
- // test locking without balance
- err := suite.App.LockupKeeper.Lock(suite.Ctx, lock, coins)
- suite.Require().Error(err)
-
- // check accumulation store
- accum := suite.App.LockupKeeper.GetPeriodLocksAccumulation(suite.Ctx, types.QueryCondition{
- LockQueryType: types.ByDuration,
- Denom: "stake",
- Duration: time.Second,
- })
- suite.Require().Equal(accum.String(), "0")
-
- suite.FundAcc(addr1, coins)
- err = suite.App.LockupKeeper.Lock(suite.Ctx, lock, coins)
- suite.Require().NoError(err)
-
- // check accumulation store
- accum = suite.App.LockupKeeper.GetPeriodLocksAccumulation(suite.Ctx, types.QueryCondition{
- LockQueryType: types.ByDuration,
- Denom: "stake",
- Duration: time.Second,
- })
- suite.Require().Equal(accum.String(), "10")
-
- balance := suite.App.BankKeeper.GetBalance(suite.Ctx, addr1, "stake")
- suite.Require().Equal(sdk.ZeroInt(), balance.Amount)
-
- acc := suite.App.AccountKeeper.GetModuleAccount(suite.Ctx, types.ModuleName)
- balance = suite.App.BankKeeper.GetBalance(suite.Ctx, acc.GetAddress(), "stake")
- suite.Require().Equal(sdk.NewInt(10), balance.Amount)
-}
-
func (suite *KeeperTestSuite) AddTokensToLockForSynth() {
suite.SetupTest()
@@ -741,14 +666,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 51b704114563cde90bcc96a3b3cdc6c152bb0e28..bb7e478e93fb0873e14131e296f90a2a0cca22f1 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{