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

* feat(x/twap): use exp2 in twapPow; add overflow test * Update x/twap/strategy_test.go * tests * clean up * Update x/twap/logic.go Co-authored-by: Dev Ojha <[email protected]> * one hundred years Co-authored-by: Dev Ojha <[email protected]>
osmosis-labs · Dec 21, 2022 · 013dee4 · 013dee4
1 parent 7b056f9
commit 013dee4
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 39 deletions.
diff --git a/x/twap/export_test.go b/x/twap/export_test.go
@@ -15,8 +15,6 @@ type (
 	GeometricTwapStrategy  = geometric
 )
 
-var GeometricTwapMathBase = geometricTwapMathBase
-
 func (k Keeper) StoreNewRecord(ctx sdk.Context, record types.TwapRecord) {
 	k.storeNewRecord(ctx, record)
 }

diff --git a/x/twap/logic.go b/x/twap/logic.go
@@ -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 {
@@ -268,13 +259,15 @@ 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.
+// twapPow exponentiates 2 to the given exponent.
 func twapPow(exponent sdk.Dec) sdk.Dec {
-	return osmomath.PowApprox(geometricTwapMathBase, exponent, geometricTwapPowPrecision)
+	exp2 := osmomath.Exp2(osmomath.BigDecFromSDKDec(exponent.Abs()))
+	if exponent.IsNegative() {
+		return osmomath.OneDec().Quo(exp2).SDKDec()
+	}
+	return exp2.SDKDec()
 }
diff --git a/x/twap/logic_test.go b/x/twap/logic_test.go
@@ -3,7 +3,6 @@ package twap_test
 import (
 	"errors"
 	"fmt"
-	"math"
 	"testing"
 	"time"
 
@@ -1319,30 +1318,34 @@ func (s *TestSuite) TestComputeArithmeticTwapWithSpotPriceError() {
 	}
 }
 
-func (s *TestSuite) TestTwapLog() {
-	var expectedErrTolerance = osmomath.MustNewDecFromStr("0.000000000000000100")
-	// "Twaplog{912648174127941279170121098210.928219201902041311} = 99.525973560175362367"
-	// From: https://www.wolframalpha.com/input?i2d=true&i=log+base+2+of+912648174127941279170121098210.928219201902041311+with+20+digits
-	var priceValue = osmomath.MustNewDecFromStr("912648174127941279170121098210.928219201902041311")
-	var expectedValue = osmomath.MustNewDecFromStr("99.525973560175362367")
-
-	result := twap.TwapLog(priceValue.SDKDec())
-	result_by_customBaseLog := priceValue.CustomBaseLog(osmomath.BigDecFromSDKDec(twap.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))
+// TestTwapLog_CorrectBase tests that the base of 2 is used for the twap log function.
+// log_2{16} = 4
+func (s *TestSuite) TestTwapLog_CorrectBase() {
+	logOf := sdk.NewDec(16)
+	expectedValue := sdk.NewDec(4)
+
+	result := twap.TwapLog(logOf)
+
+	s.Require().Equal(expectedValue, result)
 }
 
-func (s *TestSuite) TestTwapPow() {
-	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")
+// TestTwapPow_CorrectBase tests that the base of 2 is used for the twap power function.
+// 2^3 = 8
+func (s *TestSuite) TestTwapPow_CorrectBase() {
+	exponentValue := osmomath.NewBigDec(3)
+	expectedValue := sdk.NewDec(8)
 
 	result := twap.TwapPow(exponentValue.SDKDec())
-	result_by_mathPow := math.Pow(twap.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))
+
+	s.Require().Equal(expectedValue, result)
+}
+
+// TestTwapPow_NegativeExponent tests that twap pow can handle a negative exponent
+// 2^-1 = 0.5
+func (s *TestSuite) TestTwapPow_NegativeExponent() {
+	expectedResult := sdk.MustNewDecFromStr("0.5")
+	result := twap.TwapPow(oneDec.Neg())
+	s.Require().Equal(expectedResult, result)
 }
 
 func testCaseFromDeltas(s *TestSuite, startAccum, accumDiff sdk.Dec, timeDelta time.Duration, expectedTwap sdk.Dec) computeTwapTestCase {

diff --git a/x/twap/strategy.go b/x/twap/strategy.go
@@ -2,7 +2,11 @@ package twap
 
 import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	"github.com/osmosis-labs/osmosis/v13/osmomath"
 	"github.com/osmosis-labs/osmosis/v13/x/twap/types"
+
+	gammtypes "github.com/osmosis-labs/osmosis/v13/x/gamm/types"
 )
 
 // twapStrategy is an interface for computing TWAPs.
@@ -42,11 +46,14 @@ func (s *geometric) computeTwap(startRecord types.TwapRecord, endRecord types.Tw
 	timeDelta := endRecord.Time.Sub(startRecord.Time)
 	arithmeticMeanOfLogPrices := types.AccumDiffDivDuration(accumDiff, timeDelta)
 
-	geometricMeanDenom0 := twapPow(arithmeticMeanOfLogPrices)
+	result := twapPow(arithmeticMeanOfLogPrices)
 	// N.B.: Geometric mean of recprocals is reciprocal of geometric mean.
 	// https://proofwiki.org/wiki/Geometric_Mean_of_Reciprocals_is_Reciprocal_of_Geometric_Mean
 	if quoteAsset == startRecord.Asset1Denom {
-		return sdk.OneDec().Quo(geometricMeanDenom0)
+		result = sdk.OneDec().Quo(result)
 	}
-	return geometricMeanDenom0
+
+	// N.B. we round because this is the max number of significant figures supported
+	// by the underlying spot price function.
+	return osmomath.SigFigRound(result, gammtypes.SpotPriceSigFigs)
 }
diff --git a/x/twap/strategy_test.go b/x/twap/strategy_test.go
@@ -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"
 )
@@ -21,6 +22,10 @@ type computeTwapTestCase struct {
 	expPanic       bool
 }
 
+var (
+	oneHundredYears = OneSec.MulInt64(60 * 60 * 24 * 365 * 100)
+)
+
 // 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.
@@ -300,3 +305,19 @@ func (s *TestSuite) TestComputeGeometricStrategyTwap_ThreeAsset() {
 		})
 	}
 }
+
+// TestTwapPow_MaxSpotPrice_NoOverflow tests that no overflow occurs at log_2{max spot price values}.
+// and that the epsilon is within the tolerated multiplicative error.
+func (s *TestSuite) TestTwapLogPow_MaxSpotPrice_NoOverflow() {
+	errTolerance := osmomath.ErrTolerance{
+		MultiplicativeTolerance: sdk.OneDec().Quo(sdk.NewDec(10).Power(18)),
+		RoundingDir:             osmomath.RoundDown,
+	}
+
+	oneHundredYearsTimesMaxSpotPrice := oneHundredYears.Mul(gammtypes.MaxSpotPrice)
+
+	exponentValue := twap.TwapLog(oneHundredYearsTimesMaxSpotPrice)
+	finalValue := twap.TwapPow(exponentValue)
+
+	s.Require().Equal(0, errTolerance.CompareBigDec(osmomath.BigDecFromSDKDec(oneHundredYearsTimesMaxSpotPrice), osmomath.BigDecFromSDKDec(finalValue)))
+}