Skip to content
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.

Sign up for GitHub

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(x/twap): use exp2 in twapPow; add overflow test #3809

Merged
merged 6 commits into from
Dec 21, 2022
Merged

feat(x/twap): use exp2 in twapPow; add overflow test #3809

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
654b2a5
feat(x/twap): use exp2 in twapPow; add overflow test
p0mvn Dec 21, 2022
f9c3e06
Update x/twap/strategy_test.go
p0mvn Dec 21, 2022
4934eb8
tests
p0mvn Dec 21, 2022
7fc0223
clean up
p0mvn Dec 21, 2022
f1db44d
Update x/twap/logic.go
p0mvn Dec 21, 2022
a2c653c
one hundred years
p0mvn Dec 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions x/twap/export_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ type (
GeometricTwapStrategy = geometric
)

var GeometricTwapMathBase = geometricTwapMathBase

func (k Keeper) StoreNewRecord(ctx sdk.Context, record types.TwapRecord) {
k.storeNewRecord(ctx, record)
}
Expand Down
13 changes: 1 addition & 12 deletions x/twap/logic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,6 @@ import (
"github.com/osmosis-labs/osmosis/v13/x/twap/types"
)

// geometricTwapMathBase is the base used for geometric twap calculation
// in logarithm and power math functions.
// See twapLog and computeGeometricTwap functions for more details.
var (
geometricTwapMathBase = sdk.NewDec(2)
// TODO: analyze choice.
geometricTwapPowPrecision = sdk.MustNewDecFromStr("0.00000001")
)

func newTwapRecord(k types.AmmInterface, ctx sdk.Context, poolId uint64, denom0, denom1 string) (types.TwapRecord, error) {
denom0, denom1, err := types.LexicographicalOrderDenoms(denom0, denom1)
if err != nil {
Expand Down Expand Up @@ -268,13 +259,11 @@ func computeTwap(startRecord types.TwapRecord, endRecord types.TwapRecord, quote
}

// twapLog returns the logarithm of the given spot price, base 2.
// TODO: basic test
func twapLog(price sdk.Dec) sdk.Dec {
return osmomath.BigDecFromSDKDec(price).LogBase2().SDKDec()
}

// twapPow exponentiates the geometricTwapMathBase to the given exponent.
// TODO: basic test and benchmark.
func twapPow(exponent sdk.Dec) sdk.Dec {
return osmomath.PowApprox(geometricTwapMathBase, exponent, geometricTwapPowPrecision)
return osmomath.Exp2(osmomath.BigDecFromSDKDec(exponent)).SDKDec()
p0mvn marked this conversation as resolved.
Show resolved Hide resolved
}
7 changes: 4 additions & 3 deletions x/twap/logic_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1327,20 +1327,21 @@ func (s *TestSuite) TestTwapLog() {
var expectedValue = osmomath.MustNewDecFromStr("99.525973560175362367")

result := twap.TwapLog(priceValue.SDKDec())
result_by_customBaseLog := priceValue.CustomBaseLog(osmomath.BigDecFromSDKDec(twap.GeometricTwapMathBase))
result_by_customBaseLog := priceValue.CustomBaseLog(geometricTwapMathBase)
s.Require().True(expectedValue.Sub(osmomath.BigDecFromSDKDec(result)).Abs().LTE(expectedErrTolerance))
s.Require().True(result_by_customBaseLog.Sub(osmomath.BigDecFromSDKDec(result)).Abs().LTE(expectedErrTolerance))
}

func (s *TestSuite) TestTwapPow() {
// TestTwapPow_CorrectBase tests that the right base is used for the twap power function.
func (s *TestSuite) TestTwapPow_CorrectBase() {
var expectedErrTolerance = osmomath.MustNewDecFromStr("0.00000100")
// "TwapPow(0.5) = 1.41421356"
// From: https://www.wolframalpha.com/input?i2d=true&i=power+base+2+exponent+0.5+with+9+digits
exponentValue := osmomath.MustNewDecFromStr("0.5")
expectedValue := osmomath.MustNewDecFromStr("1.41421356")

result := twap.TwapPow(exponentValue.SDKDec())
result_by_mathPow := math.Pow(twap.GeometricTwapMathBase.MustFloat64(), exponentValue.SDKDec().MustFloat64())
result_by_mathPow := math.Pow(geometricTwapMathBase.MustFloat64(), exponentValue.SDKDec().MustFloat64())
s.Require().True(expectedValue.Sub(osmomath.BigDecFromSDKDec(result)).Abs().LTE(expectedErrTolerance))
s.Require().True(osmomath.MustNewDecFromStr(fmt.Sprint(result_by_mathPow)).Sub(osmomath.BigDecFromSDKDec(result)).Abs().LTE(expectedErrTolerance))
}
Expand Down
26 changes: 26 additions & 0 deletions x/twap/strategy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/osmosis-labs/osmosis/v13/app/apptesting/osmoassert"
"github.com/osmosis-labs/osmosis/v13/osmomath"
gammtypes "github.com/osmosis-labs/osmosis/v13/x/gamm/types"
"github.com/osmosis-labs/osmosis/v13/x/twap"
"github.com/osmosis-labs/osmosis/v13/x/twap/types"
)
Expand All @@ -21,6 +22,13 @@ type computeTwapTestCase struct {
expPanic bool
}

// geometricTwapMathBase is the base used for geometric twap calculation
// in logarithm and power math functions.
// See twapLog and computeGeometricTwap functions for more details.
var (
geometricTwapMathBase = osmomath.NewBigDec(2)
)

// TestComputeArithmeticTwap tests computeTwap on various inputs.
// The test vectors are structured by setting up different start and records,
// based on time interval, and their accumulator values.
Expand Down Expand Up @@ -300,3 +308,21 @@ func (s *TestSuite) TestComputeGeometricStrategyTwap_ThreeAsset() {
})
}
}

// TestTwapPow_MaxSpotPrice_NoOverflow tests that no overflow occurs at log_2{max spot price values}.
// and that the epsilot is within the tolerated multiplicative error.
p0mvn marked this conversation as resolved.
Show resolved Hide resolved
func (s *TestSuite) TestTwapLogPow_MaxSpotPrice_NoOverflow() {
errTolerance := osmomath.ErrTolerance{
MultiplicativeTolerance: sdk.OneDec().Quo(sdk.NewDec(10).Power(18)),
RoundingDir: osmomath.RoundDown,
}

oneYear := OneSec.MulInt64(60 * 60 * 24 * 365)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we test w/ 100 years 👀

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also need to test min spot price, since this can get underflow? (or is that a future PR?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(not blocking merge, as long as we track in an issue otherwise)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#3815

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with 100 years - will address underflow in #3815


oneYearTimesMaxSpotPrice := oneYear.Mul(gammtypes.MaxSpotPrice)

exponentValue := twap.TwapLog(oneYearTimesMaxSpotPrice)
finalValue := twap.TwapPow(exponentValue)

s.Equal(0, errTolerance.CompareBigDec(osmomath.BigDecFromSDKDec(oneYearTimesMaxSpotPrice), osmomath.BigDecFromSDKDec(finalValue)))
}