From ecfa8a044464bbe1c24d62fb2661517b86da6597 Mon Sep 17 00:00:00 2001 From: stackman27 Date: Fri, 25 Nov 2022 17:50:35 -0800 Subject: [PATCH 1/7] refactored twap api.go for geometric TWAP --- x/twap/api.go | 40 +++++++++++++++++++++++++++++++--------- x/twap/strategy.go | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 9 deletions(-) create mode 100644 x/twap/strategy.go diff --git a/x/twap/api.go b/x/twap/api.go index 49fe42ddcea..75d215e5cb1 100644 --- a/x/twap/api.go +++ b/x/twap/api.go @@ -51,12 +51,38 @@ func (k Keeper) GetArithmeticTwap( quoteAssetDenom string, startTime time.Time, endTime time.Time, +) (sdk.Dec, error) { + arithmeticStrategy := &arithmetic{k} + return k.getTwap(ctx, poolId, baseAssetDenom, quoteAssetDenom, startTime, endTime, arithmeticStrategy) +} + +// GetArithmeticTwapToNow returns GetArithmeticTwap on the input, with endTime being fixed to ctx.BlockTime() +// This function does not mutate records. +func (k Keeper) GetArithmeticTwapToNow( + ctx sdk.Context, + poolId uint64, + baseAssetDenom string, + quoteAssetDenom string, + startTime time.Time, +) (sdk.Dec, error) { + arithmeticStrategy := &arithmetic{k} + return k.getTwapToNow(ctx, poolId, baseAssetDenom, quoteAssetDenom, startTime, arithmeticStrategy) +} + +func (k Keeper) getTwap( + ctx sdk.Context, + poolId uint64, + baseAssetDenom string, + quoteAssetDenom string, + startTime time.Time, + endTime time.Time, + strategy twapStrategy, ) (sdk.Dec, error) { if startTime.After(endTime) { return sdk.Dec{}, types.StartTimeAfterEndTimeError{StartTime: startTime, EndTime: endTime} } if endTime.Equal(ctx.BlockTime()) { - return k.GetArithmeticTwapToNow(ctx, poolId, baseAssetDenom, quoteAssetDenom, startTime) + return k.getTwapToNow(ctx, poolId, baseAssetDenom, quoteAssetDenom, startTime, strategy) } else if endTime.After(ctx.BlockTime()) { return sdk.Dec{}, types.EndTimeInFutureError{EndTime: endTime, BlockTime: ctx.BlockTime()} } @@ -68,17 +94,16 @@ func (k Keeper) GetArithmeticTwap( if err != nil { return sdk.Dec{}, err } - return computeTwap(startRecord, endRecord, quoteAssetDenom, arithmeticTwapType) + return strategy.computeTwap(startRecord, endRecord, quoteAssetDenom) } -// GetArithmeticTwapToNow returns GetArithmeticTwap on the input, with endTime being fixed to ctx.BlockTime() -// This function does not mutate records. -func (k Keeper) GetArithmeticTwapToNow( +func (k Keeper) getTwapToNow( ctx sdk.Context, poolId uint64, baseAssetDenom string, quoteAssetDenom string, startTime time.Time, + strategy twapStrategy, ) (sdk.Dec, error) { if startTime.After(ctx.BlockTime()) { return sdk.Dec{}, types.StartTimeAfterEndTimeError{StartTime: startTime, EndTime: ctx.BlockTime()} @@ -92,14 +117,11 @@ func (k Keeper) GetArithmeticTwapToNow( if err != nil { return sdk.Dec{}, err } - return computeTwap(startRecord, endRecord, quoteAssetDenom, arithmeticTwapType) + return strategy.computeTwap(startRecord, endRecord, quoteAssetDenom) } // GetBeginBlockAccumulatorRecord returns a TwapRecord struct corresponding to the state of pool `poolId` // as of the beginning of the block this is called on. -// This uses the state of the beginning of the block, as if there were swaps since the block has started, -// these swaps have had no time to be arbitraged back. -// This accumulator can be stored, to compute wider ranged twaps. func (k Keeper) GetBeginBlockAccumulatorRecord(ctx sdk.Context, poolId uint64, asset0Denom string, asset1Denom string) (types.TwapRecord, error) { return k.getMostRecentRecord(ctx, poolId, asset0Denom, asset1Denom) } diff --git a/x/twap/strategy.go b/x/twap/strategy.go new file mode 100644 index 00000000000..49fa7ebc1c8 --- /dev/null +++ b/x/twap/strategy.go @@ -0,0 +1,40 @@ +package twap + +import ( + "time" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/osmosis-labs/osmosis/v13/x/twap/types" +) + +type twapStrategy interface { + getTwapToNow( + ctx sdk.Context, + poolId uint64, + baseAssetDenom string, + quoteAssetDenom string, + startTime time.Time) (sdk.Dec, error) + computeTwap(startRecord types.TwapRecord, endRecord types.TwapRecord, quoteAsset string) (sdk.Dec, error) +} + +type arithmetic struct { + keeper Keeper +} + +var _ twapStrategy = &arithmetic{} + +func (s *arithmetic) getTwapToNow( + ctx sdk.Context, + poolId uint64, + baseAssetDenom string, + quoteAssetDenom string, + startTime time.Time, +) (sdk.Dec, error) { + // decide whether we want to use arithmetic or geometric twap + return s.keeper.GetArithmeticTwapToNow(ctx, poolId, baseAssetDenom, quoteAssetDenom, startTime) +} + +func (s *arithmetic) computeTwap(startRecord types.TwapRecord, endRecord types.TwapRecord, quoteAsset string) (sdk.Dec, error) { + // decide whether we want to use arithmetic or geometric twap + return computeArithmeticTwap(startRecord, endRecord, quoteAsset) +} From f46e4d5ce3a97eb4d66a085707e4f62a74512528 Mon Sep 17 00:00:00 2001 From: stackman27 Date: Fri, 25 Nov 2022 21:52:52 -0800 Subject: [PATCH 2/7] added barebon docs --- x/twap/api.go | 2 ++ x/twap/strategy.go | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/x/twap/api.go b/x/twap/api.go index 75d215e5cb1..b9313f54ecd 100644 --- a/x/twap/api.go +++ b/x/twap/api.go @@ -97,6 +97,8 @@ func (k Keeper) getTwap( return strategy.computeTwap(startRecord, endRecord, quoteAssetDenom) } +// getTwapToNow extracts the computation from GetArithmeticTwap and allows TWAP computation either +// arithmetic or geometric mean based on the strategy provided. func (k Keeper) getTwapToNow( ctx sdk.Context, poolId uint64, diff --git a/x/twap/strategy.go b/x/twap/strategy.go index 49fa7ebc1c8..e995aefa81b 100644 --- a/x/twap/strategy.go +++ b/x/twap/strategy.go @@ -7,6 +7,8 @@ import ( "github.com/osmosis-labs/osmosis/v13/x/twap/types" ) +// Since we have two methods of computing TWAP; airthmetic and geometric. +// We expose a common TWAP API to reduce duplication and avoid complexity. type twapStrategy interface { getTwapToNow( ctx sdk.Context, @@ -23,6 +25,7 @@ type arithmetic struct { var _ twapStrategy = &arithmetic{} +// getTwapToNow calculates the TWAP with endRecord as currentBlocktime. func (s *arithmetic) getTwapToNow( ctx sdk.Context, poolId uint64, @@ -30,10 +33,11 @@ func (s *arithmetic) getTwapToNow( quoteAssetDenom string, startTime time.Time, ) (sdk.Dec, error) { - // decide whether we want to use arithmetic or geometric twap + // decide whether we want to use arithmetic or geometric twap. return s.keeper.GetArithmeticTwapToNow(ctx, poolId, baseAssetDenom, quoteAssetDenom, startTime) } +// getTwapToNow calculates the TWAP with specific startRecord and endRecord. func (s *arithmetic) computeTwap(startRecord types.TwapRecord, endRecord types.TwapRecord, quoteAsset string) (sdk.Dec, error) { // decide whether we want to use arithmetic or geometric twap return computeArithmeticTwap(startRecord, endRecord, quoteAsset) From d808d8110b285ee92baa294e6837b4ec9a0e5ca4 Mon Sep 17 00:00:00 2001 From: stackman27 Date: Sat, 26 Nov 2022 21:58:30 -0800 Subject: [PATCH 3/7] romans feedback --- x/twap/api.go | 8 ++++---- x/twap/strategy.go | 9 ++++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/x/twap/api.go b/x/twap/api.go index b9313f54ecd..29ae3504c68 100644 --- a/x/twap/api.go +++ b/x/twap/api.go @@ -56,8 +56,8 @@ func (k Keeper) GetArithmeticTwap( return k.getTwap(ctx, poolId, baseAssetDenom, quoteAssetDenom, startTime, endTime, arithmeticStrategy) } -// GetArithmeticTwapToNow returns GetArithmeticTwap on the input, with endTime being fixed to ctx.BlockTime() -// This function does not mutate records. +// GetArithmeticTwapToNow returns arithmetic twap from start time until the current block time for quote and base +// assets in a given pool. func (k Keeper) GetArithmeticTwapToNow( ctx sdk.Context, poolId uint64, @@ -97,8 +97,8 @@ func (k Keeper) getTwap( return strategy.computeTwap(startRecord, endRecord, quoteAssetDenom) } -// getTwapToNow extracts the computation from GetArithmeticTwap and allows TWAP computation either -// arithmetic or geometric mean based on the strategy provided. +// getTwapToNow computes and returns twap from the start time until the current block time. The type +// of twap returned depends on the strategy given and can be either arithmetic or geometric. func (k Keeper) getTwapToNow( ctx sdk.Context, poolId uint64, diff --git a/x/twap/strategy.go b/x/twap/strategy.go index e995aefa81b..6fd0d90803f 100644 --- a/x/twap/strategy.go +++ b/x/twap/strategy.go @@ -7,15 +7,18 @@ import ( "github.com/osmosis-labs/osmosis/v13/x/twap/types" ) -// Since we have two methods of computing TWAP; airthmetic and geometric. +// twapStrategy is an interface for computing TWAPs. +// We have two strategies implementing the interface - arithmetic and geometric. // We expose a common TWAP API to reduce duplication and avoid complexity. type twapStrategy interface { + // getTwapToNow calculates the TWAP with endRecord as currentBlocktime. getTwapToNow( ctx sdk.Context, poolId uint64, baseAssetDenom string, quoteAssetDenom string, startTime time.Time) (sdk.Dec, error) + // computeTwap calculates the TWAP with specific startRecord and endRecord. computeTwap(startRecord types.TwapRecord, endRecord types.TwapRecord, quoteAsset string) (sdk.Dec, error) } @@ -25,7 +28,6 @@ type arithmetic struct { var _ twapStrategy = &arithmetic{} -// getTwapToNow calculates the TWAP with endRecord as currentBlocktime. func (s *arithmetic) getTwapToNow( ctx sdk.Context, poolId uint64, @@ -33,12 +35,9 @@ func (s *arithmetic) getTwapToNow( quoteAssetDenom string, startTime time.Time, ) (sdk.Dec, error) { - // decide whether we want to use arithmetic or geometric twap. return s.keeper.GetArithmeticTwapToNow(ctx, poolId, baseAssetDenom, quoteAssetDenom, startTime) } -// getTwapToNow calculates the TWAP with specific startRecord and endRecord. func (s *arithmetic) computeTwap(startRecord types.TwapRecord, endRecord types.TwapRecord, quoteAsset string) (sdk.Dec, error) { - // decide whether we want to use arithmetic or geometric twap return computeArithmeticTwap(startRecord, endRecord, quoteAsset) } From 7a54da8a938e05e193e427d9c2effe5412b7cf4e Mon Sep 17 00:00:00 2001 From: stackman27 Date: Mon, 28 Nov 2022 19:16:02 -0800 Subject: [PATCH 4/7] new --- x/twap/api.go | 8 ++++++-- x/twap/strategy.go | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/x/twap/api.go b/x/twap/api.go index 29ae3504c68..d6133f06e75 100644 --- a/x/twap/api.go +++ b/x/twap/api.go @@ -69,6 +69,8 @@ func (k Keeper) GetArithmeticTwapToNow( return k.getTwapToNow(ctx, poolId, baseAssetDenom, quoteAssetDenom, startTime, arithmeticStrategy) } +// getTwap computes and returns twap from the start time until the end time. The type +// of twap returned depends on the strategy given and can be either arithmetic or geometric. func (k Keeper) getTwap( ctx sdk.Context, poolId uint64, @@ -94,7 +96,8 @@ func (k Keeper) getTwap( if err != nil { return sdk.Dec{}, err } - return strategy.computeTwap(startRecord, endRecord, quoteAssetDenom) + + return computeArithmeticTwap(startRecord, endRecord, quoteAssetDenom), nil } // getTwapToNow computes and returns twap from the start time until the current block time. The type @@ -119,7 +122,8 @@ func (k Keeper) getTwapToNow( if err != nil { return sdk.Dec{}, err } - return strategy.computeTwap(startRecord, endRecord, quoteAssetDenom) + + return computeArithmeticTwap(startRecord, endRecord, quoteAssetDenom), nil } // GetBeginBlockAccumulatorRecord returns a TwapRecord struct corresponding to the state of pool `poolId` diff --git a/x/twap/strategy.go b/x/twap/strategy.go index 6fd0d90803f..e5c1e0d045d 100644 --- a/x/twap/strategy.go +++ b/x/twap/strategy.go @@ -19,7 +19,7 @@ type twapStrategy interface { quoteAssetDenom string, startTime time.Time) (sdk.Dec, error) // computeTwap calculates the TWAP with specific startRecord and endRecord. - computeTwap(startRecord types.TwapRecord, endRecord types.TwapRecord, quoteAsset string) (sdk.Dec, error) + computeTwap(startRecord types.TwapRecord, endRecord types.TwapRecord, quoteAsset string) sdk.Dec } type arithmetic struct { @@ -38,6 +38,6 @@ func (s *arithmetic) getTwapToNow( return s.keeper.GetArithmeticTwapToNow(ctx, poolId, baseAssetDenom, quoteAssetDenom, startTime) } -func (s *arithmetic) computeTwap(startRecord types.TwapRecord, endRecord types.TwapRecord, quoteAsset string) (sdk.Dec, error) { +func (s *arithmetic) computeTwap(startRecord types.TwapRecord, endRecord types.TwapRecord, quoteAsset string) sdk.Dec { return computeArithmeticTwap(startRecord, endRecord, quoteAsset) } From 03eb8aea2931730f33fa7c14e4f0faa114bd72d0 Mon Sep 17 00:00:00 2001 From: stackman27 Date: Mon, 28 Nov 2022 19:29:54 -0800 Subject: [PATCH 5/7] fix --- x/twap/api.go | 4 ++-- x/twap/strategy.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/x/twap/api.go b/x/twap/api.go index d6133f06e75..05b07874502 100644 --- a/x/twap/api.go +++ b/x/twap/api.go @@ -97,7 +97,7 @@ func (k Keeper) getTwap( return sdk.Dec{}, err } - return computeArithmeticTwap(startRecord, endRecord, quoteAssetDenom), nil + return strategy.computeTwap(startRecord, endRecord, quoteAssetDenom) } // getTwapToNow computes and returns twap from the start time until the current block time. The type @@ -123,7 +123,7 @@ func (k Keeper) getTwapToNow( return sdk.Dec{}, err } - return computeArithmeticTwap(startRecord, endRecord, quoteAssetDenom), nil + return strategy.computeTwap(startRecord, endRecord, quoteAssetDenom) } // GetBeginBlockAccumulatorRecord returns a TwapRecord struct corresponding to the state of pool `poolId` diff --git a/x/twap/strategy.go b/x/twap/strategy.go index e5c1e0d045d..affcb1268d9 100644 --- a/x/twap/strategy.go +++ b/x/twap/strategy.go @@ -19,7 +19,7 @@ type twapStrategy interface { quoteAssetDenom string, startTime time.Time) (sdk.Dec, error) // computeTwap calculates the TWAP with specific startRecord and endRecord. - computeTwap(startRecord types.TwapRecord, endRecord types.TwapRecord, quoteAsset string) sdk.Dec + computeTwap(startRecord types.TwapRecord, endRecord types.TwapRecord, quoteAsset string) (sdk.Dec, error) } type arithmetic struct { @@ -38,6 +38,6 @@ func (s *arithmetic) getTwapToNow( return s.keeper.GetArithmeticTwapToNow(ctx, poolId, baseAssetDenom, quoteAssetDenom, startTime) } -func (s *arithmetic) computeTwap(startRecord types.TwapRecord, endRecord types.TwapRecord, quoteAsset string) sdk.Dec { - return computeArithmeticTwap(startRecord, endRecord, quoteAsset) +func (s *arithmetic) computeTwap(startRecord types.TwapRecord, endRecord types.TwapRecord, quoteAsset string) (sdk.Dec, error) { + return computeTwap(startRecord, endRecord, quoteAsset, arithmeticTwapType) } From 5ae2eaa751a1a6c42c494cd6d05c44491ca435b3 Mon Sep 17 00:00:00 2001 From: stackman27 Date: Wed, 30 Nov 2022 14:49:50 -0800 Subject: [PATCH 6/7] nichola feedback --- x/twap/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/twap/api.go b/x/twap/api.go index 05b07874502..0fb3b7d87c5 100644 --- a/x/twap/api.go +++ b/x/twap/api.go @@ -84,7 +84,7 @@ func (k Keeper) getTwap( return sdk.Dec{}, types.StartTimeAfterEndTimeError{StartTime: startTime, EndTime: endTime} } if endTime.Equal(ctx.BlockTime()) { - return k.getTwapToNow(ctx, poolId, baseAssetDenom, quoteAssetDenom, startTime, strategy) + return strategy.getTwapToNow(ctx, poolId, baseAssetDenom, quoteAssetDenom, startTime) } else if endTime.After(ctx.BlockTime()) { return sdk.Dec{}, types.EndTimeInFutureError{EndTime: endTime, BlockTime: ctx.BlockTime()} } From a2e66daaf0657de9942e153085963dfa6a954d7c Mon Sep 17 00:00:00 2001 From: stackman27 Date: Thu, 1 Dec 2022 08:29:06 -0800 Subject: [PATCH 7/7] final roman comments --- x/twap/api.go | 2 +- x/twap/strategy.go | 19 ------------------- 2 files changed, 1 insertion(+), 20 deletions(-) diff --git a/x/twap/api.go b/x/twap/api.go index 0fb3b7d87c5..05b07874502 100644 --- a/x/twap/api.go +++ b/x/twap/api.go @@ -84,7 +84,7 @@ func (k Keeper) getTwap( return sdk.Dec{}, types.StartTimeAfterEndTimeError{StartTime: startTime, EndTime: endTime} } if endTime.Equal(ctx.BlockTime()) { - return strategy.getTwapToNow(ctx, poolId, baseAssetDenom, quoteAssetDenom, startTime) + return k.getTwapToNow(ctx, poolId, baseAssetDenom, quoteAssetDenom, startTime, strategy) } else if endTime.After(ctx.BlockTime()) { return sdk.Dec{}, types.EndTimeInFutureError{EndTime: endTime, BlockTime: ctx.BlockTime()} } diff --git a/x/twap/strategy.go b/x/twap/strategy.go index affcb1268d9..426454a163a 100644 --- a/x/twap/strategy.go +++ b/x/twap/strategy.go @@ -1,8 +1,6 @@ package twap import ( - "time" - sdk "github.com/cosmos/cosmos-sdk/types" "github.com/osmosis-labs/osmosis/v13/x/twap/types" ) @@ -11,13 +9,6 @@ import ( // We have two strategies implementing the interface - arithmetic and geometric. // We expose a common TWAP API to reduce duplication and avoid complexity. type twapStrategy interface { - // getTwapToNow calculates the TWAP with endRecord as currentBlocktime. - getTwapToNow( - ctx sdk.Context, - poolId uint64, - baseAssetDenom string, - quoteAssetDenom string, - startTime time.Time) (sdk.Dec, error) // computeTwap calculates the TWAP with specific startRecord and endRecord. computeTwap(startRecord types.TwapRecord, endRecord types.TwapRecord, quoteAsset string) (sdk.Dec, error) } @@ -28,16 +19,6 @@ type arithmetic struct { var _ twapStrategy = &arithmetic{} -func (s *arithmetic) getTwapToNow( - ctx sdk.Context, - poolId uint64, - baseAssetDenom string, - quoteAssetDenom string, - startTime time.Time, -) (sdk.Dec, error) { - return s.keeper.GetArithmeticTwapToNow(ctx, poolId, baseAssetDenom, quoteAssetDenom, startTime) -} - func (s *arithmetic) computeTwap(startRecord types.TwapRecord, endRecord types.TwapRecord, quoteAsset string) (sdk.Dec, error) { return computeTwap(startRecord, endRecord, quoteAsset, arithmeticTwapType) }