Fix: Flip existing twapRecords base/quote price denoms

CHANGELOG.md

@@ -56,6 +56,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * [#5874](https://github.com/osmosis-labs/osmosis/pull/5874) Remove Partial Migration from superfluid migration to CL
 * [#5901](https://github.com/osmosis-labs/osmosis/pull/5901) Adding support for CW pools in ProtoRev
 * [#5937](https://github.com/osmosis-labs/osmosis/pull/5937) feat: add SetScalingFactorController gov prop
+* [#5939](https://github.com/osmosis-labs/osmosis/pull/5939) Fix: Flip existing twapRecords base/quote price denoms 
+* [#5938](https://github.com/osmosis-labs/osmosis/pull/5938) Chore: Fix valset amino codec
 
 ### BugFix
 

app/upgrades/v17/upgrades.go

@@ -17,6 +17,8 @@ import (
 	"github.com/osmosis-labs/osmosis/v17/app/keepers"
 	"github.com/osmosis-labs/osmosis/v17/app/upgrades"
 	"github.com/osmosis-labs/osmosis/v17/x/protorev/types"
+
+	poolmanagertypes "github.com/osmosis-labs/osmosis/v17/x/poolmanager/types"
 )
 
 func CreateUpgradeHandler(
@@ -33,6 +35,18 @@ func CreateUpgradeHandler(
 			return nil, err
 		}
 
+		// get all the existing CL pools
+		pools, err := keepers.ConcentratedLiquidityKeeper.GetPools(ctx)
+		if err != nil {
+			return nil, err
+		}
+
+		// migrate twap records for CL Pools
+		err = FlipTwapSpotPriceRecords(ctx, pools, keepers)
+		if err != nil {
+			return nil, err
+		}
+
 		// Get community pool address.
 		communityPoolAddress := keepers.AccountKeeper.GetModuleAddress(distrtypes.ModuleName)
 
@@ -128,3 +142,41 @@ func CreateUpgradeHandler(
 		return migrations, nil
 	}
 }
+
+// FlipTwapSpotPriceRecords flips the denoms and spot price of twap record of a given pool.
+func FlipTwapSpotPriceRecords(ctx sdk.Context, pools []poolmanagertypes.PoolI, keepers *keepers.AppKeepers) error {
+	for _, pool := range pools {
+		twapRecordHistoricalPoolIndexed, err := keepers.TwapKeeper.GetAllHistoricalPoolIndexedTWAPsForPoolId(ctx, pool.GetId())
+		if err != nil {
+			return err
+		}
+
+		for _, historicalTwapRecord := range twapRecordHistoricalPoolIndexed {
+			oldRecord := historicalTwapRecord
+			historicalTwapRecord.Asset0Denom, historicalTwapRecord.Asset1Denom = oldRecord.Asset1Denom, oldRecord.Asset0Denom
+			historicalTwapRecord.P0LastSpotPrice, historicalTwapRecord.P1LastSpotPrice = oldRecord.P1LastSpotPrice, oldRecord.P0LastSpotPrice
+
+			keepers.TwapKeeper.StoreHistoricalTWAP(ctx, historicalTwapRecord)
+			keepers.TwapKeeper.DeleteHistoricalRecord(ctx, oldRecord)
+		}
+
+		// check that the twap record exists
+		clPoolTwapRecords, err := keepers.TwapKeeper.GetAllMostRecentRecordsForPool(ctx, pool.GetId())
+		if err != nil {
+			return err
+		}
+
+		for _, twapRecord := range clPoolTwapRecords {
+			twapRecord.LastErrorTime = time.Time{}
+			oldRecord := twapRecord
+
+			twapRecord.Asset0Denom, twapRecord.Asset1Denom = oldRecord.Asset1Denom, oldRecord.Asset0Denom
+			twapRecord.P0LastSpotPrice, twapRecord.P1LastSpotPrice = oldRecord.P1LastSpotPrice, oldRecord.P0LastSpotPrice
+
+			keepers.TwapKeeper.StoreNewRecord(ctx, twapRecord)
+			keepers.TwapKeeper.DeleteOldRecord(ctx, oldRecord)
+		}
+	}
+
+	return nil
+}

app/upgrades/v17/upgrades_test.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"sort"
 	"testing"
+	"time"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
@@ -19,6 +20,7 @@ import (
 	v17 "github.com/osmosis-labs/osmosis/v17/app/upgrades/v17"
 	cltypes "github.com/osmosis-labs/osmosis/v17/x/concentrated-liquidity/types"
 	poolmanagertypes "github.com/osmosis-labs/osmosis/v17/x/poolmanager/types"
+	"github.com/osmosis-labs/osmosis/v17/x/twap/types"
 )
 
 type UpgradeTestSuite struct {
@@ -54,6 +56,21 @@ func dummyUpgrade(suite *UpgradeTestSuite) {
 	suite.Ctx = suite.Ctx.WithBlockHeight(dummyUpgradeHeight)
 }
 
+func dummyTwapRecord(poolId uint64, t time.Time, asset0 string, asset1 string, sp0, accum0, accum1, geomAccum sdk.Dec) types.TwapRecord {
+	return types.TwapRecord{
+		PoolId:      poolId,
+		Time:        t,
+		Asset0Denom: asset0,
+		Asset1Denom: asset1,
+
+		P0LastSpotPrice:             sp0,
+		P1LastSpotPrice:             sdk.OneDec().Quo(sp0),
+		P0ArithmeticTwapAccumulator: accum0,
+		P1ArithmeticTwapAccumulator: accum1,
+		GeometricTwapAccumulator:    geomAccum,
+	}
+}
+
 func (suite *UpgradeTestSuite) TestUpgrade() {
 	upgradeSetup := func() {
 		// This is done to ensure that we run the InitGenesis() logic for the new modules
@@ -70,12 +87,12 @@ func (suite *UpgradeTestSuite) TestUpgrade() {
 
 	testCases := []struct {
 		name        string
-		pre_upgrade func(sdk.Context, *keepers.AppKeepers) (sdk.Coins, uint64)
-		upgrade     func(sdk.Context, *keepers.AppKeepers, sdk.Coins, uint64)
+		pre_upgrade func(sdk.Context, *keepers.AppKeepers) (sdk.Coins, uint64, []types.TwapRecord)
+		upgrade     func(sdk.Context, *keepers.AppKeepers, sdk.Coins, uint64, []types.TwapRecord)
 	}{
 		{
 			"Test that the upgrade succeeds",
-			func(ctx sdk.Context, keepers *keepers.AppKeepers) (sdk.Coins, uint64) {
+			func(ctx sdk.Context, keepers *keepers.AppKeepers) (sdk.Coins, uint64, []types.TwapRecord) {
 				upgradeSetup()
 
 				var lastPoolID uint64 // To keep track of the last assigned pool ID
@@ -125,10 +142,24 @@ func (suite *UpgradeTestSuite) TestUpgrade() {
 					lastPoolID = poolID
 				}
 
-				return expectedCoinsUsedInUpgradeHandler, lastPoolID
+				existingPool := suite.PrepareConcentratedPoolWithCoins("ibc/1480B8FD20AD5FCAE81EA87584D269547DD4D436843C1D20F15E00EB64743EF4", "uosmo")
+
+				// create few TWAP records for the pools
+				t1 := dummyTwapRecord(existingPool.GetId(), time.Now().Add(-time.Hour*24), "ibc/1480B8FD20AD5FCAE81EA87584D269547DD4D436843C1D20F15E00EB64743EF4", "uosmo", sdk.NewDec(10),
+					sdk.OneDec().MulInt64(10*10),
+					sdk.OneDec().MulInt64(3),
+					sdk.ZeroDec())
+
+				suite.App.TwapKeeper.StoreNewRecord(suite.Ctx, t1)
+
+				clPoolTwapRecordPreUpgrade, err := keepers.TwapKeeper.GetAllMostRecentRecordsForPool(ctx, existingPool.GetId())
+				suite.Require().NoError(err)
+
+				return expectedCoinsUsedInUpgradeHandler, lastPoolID, clPoolTwapRecordPreUpgrade
 
 			},
-			func(ctx sdk.Context, keepers *keepers.AppKeepers, expectedCoinsUsedInUpgradeHandler sdk.Coins, lastPoolID uint64) {
+			func(ctx sdk.Context, keepers *keepers.AppKeepers, expectedCoinsUsedInUpgradeHandler sdk.Coins, lastPoolID uint64, clPoolTwapRecordPreUpgrade []types.TwapRecord) {
+				lastPoolID = clPoolTwapRecordPreUpgrade[0].GetPoolId()
 				stakingParams := suite.App.StakingKeeper.GetParams(suite.Ctx)
 				stakingParams.BondDenom = "uosmo"
 				suite.App.StakingKeeper.SetParams(suite.Ctx, stakingParams)
@@ -137,12 +168,31 @@ func (suite *UpgradeTestSuite) TestUpgrade() {
 				communityPoolAddress := suite.App.AccountKeeper.GetModuleAddress(distrtypes.ModuleName)
 				communityPoolBalancePre := suite.App.BankKeeper.GetAllBalances(suite.Ctx, communityPoolAddress)
 
+				clPoolTwapRecordHistoricalPoolIndexPreUpgrade, err := keepers.TwapKeeper.GetAllHistoricalPoolIndexedTWAPsForPoolId(ctx, lastPoolID)
+				suite.Require().NoError(err)
+
 				// Run upgrade handler.
 				dummyUpgrade(suite)
 				suite.Require().NotPanics(func() {
 					suite.App.BeginBlocker(suite.Ctx, abci.RequestBeginBlock{})
 				})
 
+				clPoolTwapRecordPostUpgrade, err := keepers.TwapKeeper.GetAllMostRecentRecordsForPool(ctx, lastPoolID)
+				suite.Require().NoError(err)
+
+				clPoolTwapRecordHistoricalPoolIndexPostUpgrade, err := keepers.TwapKeeper.GetAllHistoricalPoolIndexedTWAPsForPoolId(ctx, lastPoolID)
+				suite.Require().NoError(err)
+
+				for i := range clPoolTwapRecordHistoricalPoolIndexPostUpgrade {
+					suite.Require().Equal(clPoolTwapRecordHistoricalPoolIndexPreUpgrade[i].Asset0Denom, clPoolTwapRecordHistoricalPoolIndexPostUpgrade[i].Asset1Denom)
+					suite.Require().Equal(clPoolTwapRecordHistoricalPoolIndexPreUpgrade[i].Asset1Denom, clPoolTwapRecordHistoricalPoolIndexPostUpgrade[i].Asset0Denom)
+				}
+
+				for i := range clPoolTwapRecordPostUpgrade {
+					suite.Require().Equal(clPoolTwapRecordPreUpgrade[i].Asset0Denom, clPoolTwapRecordPostUpgrade[i].Asset1Denom)
+					suite.Require().Equal(clPoolTwapRecordPreUpgrade[i].Asset1Denom, clPoolTwapRecordPostUpgrade[i].Asset0Denom)
+				}
+
 				// Retrieve the community pool balance (and the feePool balance) after the upgrade
 				communityPoolBalancePost := suite.App.BankKeeper.GetAllBalances(suite.Ctx, communityPoolAddress)
 				feePoolCommunityPoolPost := suite.App.DistrKeeper.GetFeePool(suite.Ctx).CommunityPool
@@ -205,11 +255,11 @@ func (suite *UpgradeTestSuite) TestUpgrade() {
 		},
 		{
 			"Fails because CFMM pool is not found",
-			func(ctx sdk.Context, keepers *keepers.AppKeepers) (sdk.Coins, uint64) {
+			func(ctx sdk.Context, keepers *keepers.AppKeepers) (sdk.Coins, uint64, []types.TwapRecord) {
 				upgradeSetup()
-				return sdk.NewCoins(), 0
+				return sdk.NewCoins(), 0, nil
 			},
-			func(ctx sdk.Context, keepers *keepers.AppKeepers, expectedCoinsUsedInUpgradeHandler sdk.Coins, lastPoolID uint64) {
+			func(ctx sdk.Context, keepers *keepers.AppKeepers, expectedCoinsUsedInUpgradeHandler sdk.Coins, lastPoolID uint64, clPoolTwapRecordPreUpgrade []types.TwapRecord) {
 				dummyUpgrade(suite)
 				suite.Require().Panics(func() {
 					suite.App.BeginBlocker(suite.Ctx, abci.RequestBeginBlock{})
@@ -222,8 +272,8 @@ func (suite *UpgradeTestSuite) TestUpgrade() {
 		suite.Run(fmt.Sprintf("Case %s", tc.name), func() {
 			suite.SetupTest() // reset
 
-			expectedCoinsUsedInUpgradeHandler, lastPoolID := tc.pre_upgrade(suite.Ctx, &suite.App.AppKeepers)
-			tc.upgrade(suite.Ctx, &suite.App.AppKeepers, expectedCoinsUsedInUpgradeHandler, lastPoolID)
+			expectedCoinsUsedInUpgradeHandler, lastPoolID, clPoolTwapRecordPreUpgrade := tc.pre_upgrade(suite.Ctx, &suite.App.AppKeepers)
+			tc.upgrade(suite.Ctx, &suite.App.AppKeepers, expectedCoinsUsedInUpgradeHandler, lastPoolID, clPoolTwapRecordPreUpgrade)
 		})
 	}
 }

x/twap/export_test.go

@@ -24,14 +24,6 @@ func (k Keeper) GetRecordAtOrBeforeTime(ctx sdk.Context, poolId uint64, time tim
 	return k.getRecordAtOrBeforeTime(ctx, poolId, time, asset0Denom, asset1Denom)
 }
 
-func (k Keeper) GetAllHistoricalTimeIndexedTWAPs(ctx sdk.Context) ([]types.TwapRecord, error) {
-	return k.getAllHistoricalTimeIndexedTWAPs(ctx)
-}
-
-func (k Keeper) GetAllHistoricalPoolIndexedTWAPs(ctx sdk.Context) ([]types.TwapRecord, error) {
-	return k.getAllHistoricalPoolIndexedTWAPs(ctx)
-}
-
 func (k Keeper) TrackChangedPool(ctx sdk.Context, poolId uint64) {
 	k.trackChangedPool(ctx, poolId)
 }

x/twap/keeper.go

@@ -73,7 +73,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) {
 func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
 	// These are ordered in increasing order, guaranteed by the iterator
 	// that is prefixed by time.
-	twapRecords, err := k.getAllHistoricalTimeIndexedTWAPs(ctx)
+	twapRecords, err := k.GetAllHistoricalTimeIndexedTWAPs(ctx)
 	if err != nil {
 		panic(err)
 	}

x/twap/keeper_test.go

@@ -383,7 +383,7 @@ func (s *TestSuite) createTestRecordsFromTime(t time.Time) (types.TwapRecord, ty
 // - 3 records at time t
 // - 3 records t time t + 1 seconds
 // all returned records belong to the same pool with poolId
-func (s *TestSuite) createTestRecordsFromTimeInPool(t time.Time, poolId uint64) (types.TwapRecord, types.TwapRecord, types.TwapRecord, types.TwapRecord, types.TwapRecord, types.TwapRecord,
+func (s *TestSuite) CreateTestRecordsFromTimeInPool(t time.Time, poolId uint64) (types.TwapRecord, types.TwapRecord, types.TwapRecord, types.TwapRecord, types.TwapRecord, types.TwapRecord,
 	types.TwapRecord, types.TwapRecord, types.TwapRecord, types.TwapRecord, types.TwapRecord, types.TwapRecord,
 ) {
 	baseRecordAB := newEmptyPriceRecord(poolId, t, denom0, denom1)

x/twap/store.go

@@ -52,7 +52,7 @@ func (k Keeper) getChangedPools(ctx sdk.Context) []uint64 {
 }
 
 // storeHistoricalTWAP writes a twap to the store, in all needed indexing.
-func (k Keeper) storeHistoricalTWAP(ctx sdk.Context, twap types.TwapRecord) {
+func (k Keeper) StoreHistoricalTWAP(ctx sdk.Context, twap types.TwapRecord) {
 	store := ctx.KVStore(k.storeKey)
 	key1 := types.FormatHistoricalTimeIndexTWAPKey(twap.Time, twap.PoolId, twap.Asset0Denom, twap.Asset1Denom)
 	key2 := types.FormatHistoricalPoolIndexTWAPKey(twap.PoolId, twap.Asset0Denom, twap.Asset1Denom, twap.Time)
@@ -110,12 +110,12 @@ func (k Keeper) pruneRecordsBeforeTimeButNewest(ctx sdk.Context, lastKeptTime ti
 			continue
 		}
 
-		k.deleteHistoricalRecord(ctx, twapToRemove)
+		k.DeleteHistoricalRecord(ctx, twapToRemove)
 	}
 	return nil
 }
 
-func (k Keeper) deleteHistoricalRecord(ctx sdk.Context, twap types.TwapRecord) {
+func (k Keeper) DeleteHistoricalRecord(ctx sdk.Context, twap types.TwapRecord) {
 	store := ctx.KVStore(k.storeKey)
 	key1 := types.FormatHistoricalTimeIndexTWAPKey(twap.Time, twap.PoolId, twap.Asset0Denom, twap.Asset1Denom)
 	key2 := types.FormatHistoricalPoolIndexTWAPKey(twap.PoolId, twap.Asset0Denom, twap.Asset1Denom, twap.Time)
@@ -152,22 +152,33 @@ func (k Keeper) GetAllMostRecentRecordsForPool(ctx sdk.Context, poolId uint64) (
 }
 
 // getAllHistoricalTimeIndexedTWAPs returns all historical TWAPs indexed by time.
-func (k Keeper) getAllHistoricalTimeIndexedTWAPs(ctx sdk.Context) ([]types.TwapRecord, error) {
+func (k Keeper) GetAllHistoricalTimeIndexedTWAPs(ctx sdk.Context) ([]types.TwapRecord, error) {
 	return osmoutils.GatherValuesFromStorePrefix(ctx.KVStore(k.storeKey), []byte(types.HistoricalTWAPTimeIndexPrefix), types.ParseTwapFromBz)
 }
 
 // getAllHistoricalPoolIndexedTWAPs returns all historical TWAPs indexed by pool id.
-// nolint: unused
-func (k Keeper) getAllHistoricalPoolIndexedTWAPs(ctx sdk.Context) ([]types.TwapRecord, error) {
+func (k Keeper) GetAllHistoricalPoolIndexedTWAPs(ctx sdk.Context) ([]types.TwapRecord, error) {
 	return osmoutils.GatherValuesFromStorePrefix(ctx.KVStore(k.storeKey), []byte(types.HistoricalTWAPPoolIndexPrefix), types.ParseTwapFromBz)
 }
 
+// GetAllHistoricalPoolIndexedTWAPsForPoolId returns HistoricalTwapRecord for a pool give poolId.
+func (k Keeper) GetAllHistoricalPoolIndexedTWAPsForPoolId(ctx sdk.Context, poolId uint64) ([]types.TwapRecord, error) {
+	return osmoutils.GatherValuesFromStorePrefixWithKeyParser(ctx.KVStore(k.storeKey), types.FormatKeyPoolTwapRecords(poolId), types.ParseTwapHistoricalPoolIndexedRecordFromBz)
+}
+
 // StoreNewRecord stores a record, in both the most recent record store and historical stores.
 func (k Keeper) StoreNewRecord(ctx sdk.Context, twap types.TwapRecord) {
 	store := ctx.KVStore(k.storeKey)
 	key := types.FormatMostRecentTWAPKey(twap.PoolId, twap.Asset0Denom, twap.Asset1Denom)
 	osmoutils.MustSet(store, key, &twap)
-	k.storeHistoricalTWAP(ctx, twap)
+	k.StoreHistoricalTWAP(ctx, twap)
+}
+
+// DeleteOldRecord deletes a record, in most recent record store
+func (k Keeper) DeleteOldRecord(ctx sdk.Context, twap types.TwapRecord) {
+	store := ctx.KVStore(k.storeKey)
+	key := types.FormatMostRecentTWAPKey(twap.PoolId, twap.Asset0Denom, twap.Asset1Denom)
+	store.Delete(key)
 }
 
 // getRecordAtOrBeforeTime on a given input (id, t, asset0, asset1)

x/twap/store_test.go

@@ -273,13 +273,13 @@ func (s *TestSuite) TestPruneRecordsBeforeTimeButNewest() {
 	pool5Min2SBaseMsAB, pool5Min2SBaseMsAC, pool5Min2SBaseMsBC,
 		pool5Min1SBaseMsAB, pool5Min1SBaseMsAC, pool5Min1SBaseMsBC,
 		pool5BaseSecBaseMsAB, pool5BaseSecBaseMsAC, pool5BaseSecBaseMsBC,
-		pool5Plus1SBaseMsAB, pool5Plus1SBaseMsAC, pool5Plus1SBaseMsBC := s.createTestRecordsFromTimeInPool(baseTime, 5)
+		pool5Plus1SBaseMsAB, pool5Plus1SBaseMsAC, pool5Plus1SBaseMsBC := s.CreateTestRecordsFromTimeInPool(baseTime, 5)
 
 	// Create 12 records in the same pool from base time - 1 ms, each record with the difference of 1 second between them
 	pool5Min2SMin1MsAB, pool5Min2SMin1MsAC, pool5Min2SMin1MsBC,
 		pool5Min1SMin1MsAB, pool5Min1SMin1MsAC, pool5Min1SMin1MsBC,
 		pool5BaseSecMin1MsAB, pool5BaseSecMin1MsAC, pool5BaseSecMin1MsBC,
-		pool5Plus1SMin1MsAB, pool5Plus1SMin1MsAC, pool5Plus1SMin1MsBC := s.createTestRecordsFromTimeInPool(baseTime.Add(-time.Millisecond), 5)
+		pool5Plus1SMin1MsAB, pool5Plus1SMin1MsAC, pool5Plus1SMin1MsBC := s.CreateTestRecordsFromTimeInPool(baseTime.Add(-time.Millisecond), 5)
 
 	tests := map[string]struct {
 		// order does not follow any specific pattern
@@ -574,3 +574,47 @@ func (s *TestSuite) TestAccumulatorOverflow() {
 		})
 	}
 }
+
+func (s *TestSuite) TestGetAllHistoricalPoolIndexedTWAPsForPooId() {
+	baseRecord := newEmptyPriceRecord(1, baseTime, denom0, denom1)
+	tPlusOneRecord := newEmptyPriceRecord(1, tPlusOne, denom0, denom1)
+	tests := map[string]struct {
+		recordsToSet    []types.TwapRecord
+		poolId          uint64
+		expectedRecords []types.TwapRecord
+	}{
+		"set single record": {
+			poolId:          1,
+			expectedRecords: []types.TwapRecord{baseRecord},
+		},
+		"query non-existent pool": {
+			poolId:          2,
+			expectedRecords: []types.TwapRecord{},
+		},
+		"set single record, different pool ID": {
+			poolId:          2,
+			expectedRecords: []types.TwapRecord{newEmptyPriceRecord(2, baseTime, denom0, denom1)},
+		},
+		"set two records": {
+			poolId:          1,
+			expectedRecords: []types.TwapRecord{baseRecord, tPlusOneRecord},
+		},
+	}
+
+	for name, test := range tests {
+		s.Run(name, func() {
+			s.SetupTest()
+			twapKeeper := s.twapkeeper
+			s.preSetRecords(test.expectedRecords)
+
+			// System under test.
+			actualRecords, err := twapKeeper.GetAllHistoricalPoolIndexedTWAPsForPoolId(s.Ctx, test.poolId)
+			s.NoError(err)
+
+			// Assertions.
+			s.Equal(test.expectedRecords, actualRecords)
+
+		})
+	}
+
+}