-
Notifications
You must be signed in to change notification settings - Fork 608
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: twap key refactor #7472
feat: twap key refactor #7472
Changes from all commits
18aaec3
cd2da82
2128437
be2c96f
12348a9
0aa2e97
9151583
bfa5c3d
1172f26
ccb2a5e
45e7485
da70f02
b189f52
b18ecdd
c73b96c
e2fd17e
1385f60
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,10 @@ func CreateUpgradeHandler( | |
return nil, err | ||
} | ||
|
||
// Now that the TWAP keys are refactored, we can delete all time indexed TWAPs | ||
// since we only need the pool indexed TWAPs. | ||
keepers.TwapKeeper.DeleteAllHistoricalTimeIndexedTWAPs(ctx) | ||
|
||
Comment on lines
+26
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note to selves, we may have to make these deletes smoothed over several blocks There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it depends on what else exists in the upgrade handler. If there are some other larger changes then yeah I think we should, otherwise itll be just like any other epoch, but less. |
||
return migrations, nil | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
package v24_test | ||
|
||
import ( | ||
"bytes" | ||
"fmt" | ||
"testing" | ||
"time" | ||
|
||
"github.com/stretchr/testify/suite" | ||
|
||
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" | ||
|
||
abci "github.com/cometbft/cometbft/abci/types" | ||
|
||
"github.com/osmosis-labs/osmosis/osmomath" | ||
"github.com/osmosis-labs/osmosis/osmoutils" | ||
"github.com/osmosis-labs/osmosis/v23/app/apptesting" | ||
|
||
"github.com/osmosis-labs/osmosis/v23/x/twap/types" | ||
twaptypes "github.com/osmosis-labs/osmosis/v23/x/twap/types" | ||
) | ||
|
||
const ( | ||
v24UpgradeHeight = int64(10) | ||
HistoricalTWAPTimeIndexPrefix = "historical_time_index" | ||
KeySeparator = "|" | ||
) | ||
|
||
type UpgradeTestSuite struct { | ||
apptesting.KeeperTestHelper | ||
} | ||
|
||
func TestUpgradeTestSuite(t *testing.T) { | ||
suite.Run(t, new(UpgradeTestSuite)) | ||
} | ||
|
||
func (s *UpgradeTestSuite) TestUpgrade() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great job on the test |
||
s.Setup() | ||
|
||
// Manually set up TWAP records indexed by both pool ID and time. | ||
twapStoreKey := s.App.GetKey(twaptypes.ModuleName) | ||
store := s.Ctx.KVStore(twapStoreKey) | ||
twap := twaptypes.TwapRecord{ | ||
PoolId: 1, | ||
Asset0Denom: "foo", | ||
Asset1Denom: "bar", | ||
Height: 1, | ||
Time: time.Date(2023, 0o2, 1, 0, 0, 0, 0, time.UTC), | ||
P0LastSpotPrice: osmomath.OneDec(), | ||
P1LastSpotPrice: osmomath.OneDec(), | ||
P0ArithmeticTwapAccumulator: osmomath.ZeroDec(), | ||
P1ArithmeticTwapAccumulator: osmomath.ZeroDec(), | ||
GeometricTwapAccumulator: osmomath.ZeroDec(), | ||
LastErrorTime: time.Time{}, // no previous error | ||
} | ||
poolIndexKey := types.FormatHistoricalPoolIndexTWAPKey(twap.PoolId, twap.Asset0Denom, twap.Asset1Denom, twap.Time) | ||
osmoutils.MustSet(store, poolIndexKey, &twap) | ||
|
||
// The time index key is a bit manual since we removed the old code that did this programmatically. | ||
var buffer bytes.Buffer | ||
timeS := osmoutils.FormatTimeString(twap.Time) | ||
fmt.Fprintf(&buffer, "%s%d%s%s%s%s%s%s", HistoricalTWAPTimeIndexPrefix, twap.PoolId, KeySeparator, twap.Asset0Denom, KeySeparator, twap.Asset1Denom, KeySeparator, timeS) | ||
timeIndexKey := buffer.Bytes() | ||
osmoutils.MustSet(store, timeIndexKey, &twap) | ||
|
||
// TWAP records indexed by time should exist | ||
twapRecords, err := osmoutils.GatherValuesFromStorePrefix(store, []byte(HistoricalTWAPTimeIndexPrefix), types.ParseTwapFromBz) | ||
s.Require().NoError(err) | ||
s.Require().Len(twapRecords, 1) | ||
s.Require().Equal(twap, twapRecords[0]) | ||
|
||
// TWAP records indexed by pool ID should exist. | ||
twapRecords, err = s.App.TwapKeeper.GetAllHistoricalPoolIndexedTWAPsForPoolId(s.Ctx, twap.PoolId) | ||
s.Require().NoError(err) | ||
s.Require().Len(twapRecords, 1) | ||
s.Require().Equal(twap, twapRecords[0]) | ||
|
||
dummyUpgrade(s) | ||
s.Require().NotPanics(func() { | ||
s.App.BeginBlocker(s.Ctx, abci.RequestBeginBlock{}) | ||
}) | ||
|
||
// TWAP records indexed by time should be completely removed. | ||
twapRecords, err = osmoutils.GatherValuesFromStorePrefix(store, []byte(HistoricalTWAPTimeIndexPrefix), types.ParseTwapFromBz) | ||
s.Require().NoError(err) | ||
s.Require().Len(twapRecords, 0) | ||
|
||
// TWAP records indexed by pool ID should be untouched. | ||
twapRecords, err = s.App.TwapKeeper.GetAllHistoricalPoolIndexedTWAPsForPoolId(s.Ctx, twap.PoolId) | ||
s.Require().NoError(err) | ||
s.Require().Len(twapRecords, 1) | ||
s.Require().Equal(twap, twapRecords[0]) | ||
} | ||
|
||
func dummyUpgrade(s *UpgradeTestSuite) { | ||
s.Ctx = s.Ctx.WithBlockHeight(v24UpgradeHeight - 1) | ||
plan := upgradetypes.Plan{Name: "v24", Height: v24UpgradeHeight} | ||
err := s.App.UpgradeKeeper.ScheduleUpgrade(s.Ctx, plan) | ||
s.Require().NoError(err) | ||
_, exists := s.App.UpgradeKeeper.GetUpgradePlan(s.Ctx) | ||
s.Require().True(exists) | ||
|
||
s.Ctx = s.Ctx.WithBlockHeight(v24UpgradeHeight) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drive by change, this folder no longer lives in Osmosis repo