refactor(x/twap): handle spot price error case in the context of geom…

…etric twap (#3845) * refactor(x/twap): handle spot price error case * supporting test cases * table-driven log tests
osmosis-labs · Dec 23, 2022 · 678ef58 · 678ef58
1 parent 5e3ca00
commit 678ef58
Show file tree

Hide file tree

Showing 4 changed files with 97 additions and 4 deletions.
diff --git a/osmomath/sigfig_round_test.go b/osmomath/sigfig_round_test.go
@@ -70,6 +70,13 @@ func TestSigFigRound(t *testing.T) {
 			tenToSigFig:    sdk.NewInt(100),
 			expectedResult: sdk.MustNewDecFromStr("0.087"),
 		},
+
+		{
+			name:           "minimum decimal is still kept",
+			decimal:        sdk.NewDecWithPrec(1, 18),
+			tenToSigFig:    sdk.NewInt(10),
+			expectedResult: sdk.NewDecWithPrec(1, 18),
+		},
 	}
 
 	for i, tc := range testCases {

diff --git a/x/gamm/pool-models/balancer/pool_suite_test.go b/x/gamm/pool-models/balancer/pool_suite_test.go
@@ -766,6 +766,22 @@ func (suite *KeeperTestSuite) TestBalancerSpotPriceBounds() {
 			baseDenomWeight:     sdk.NewInt(100),
 			expectError:         true,
 		},
+		{
+			name:                "internal error due to spot price precision being too small, resulting in 0 spot price",
+			quoteDenomPoolInput: sdk.NewCoin(baseDenom, sdk.OneInt()),
+			quoteDenomWeight:    sdk.NewInt(100),
+			baseDenomPoolInput:  sdk.NewCoin(quoteDenom, sdk.NewDec(10).PowerMut(19).TruncateInt().Sub(sdk.NewInt(2))),
+			baseDenomWeight:     sdk.NewInt(100),
+			expectError:         true,
+		},
+		{
+			name:                "at min spot price",
+			quoteDenomPoolInput: sdk.NewCoin(baseDenom, sdk.OneInt()),
+			quoteDenomWeight:    sdk.NewInt(100),
+			baseDenomPoolInput:  sdk.NewCoin(quoteDenom, sdk.NewDec(10).PowerMut(18).TruncateInt()),
+			baseDenomWeight:     sdk.NewInt(100),
+			expectedOutput:      sdk.OneDec().Quo(sdk.NewDec(10).PowerMut(18)),
+		},
 	}
 
 	for _, tc := range tests {

diff --git a/x/twap/logic.go b/x/twap/logic.go
@@ -194,6 +194,14 @@ func recordWithUpdatedAccumulators(record types.TwapRecord, newTime time.Time) t
 	p1NewAccum := types.SpotPriceMulDuration(record.P1LastSpotPrice, timeDelta)
 	newRecord.P1ArithmeticTwapAccumulator = newRecord.P1ArithmeticTwapAccumulator.Add(p1NewAccum)
 
+	// If the last spot price is zero, then the logarithm is undefined.
+	// As a result, we cannot update the geometric accumulator.
+	// We set the last error time to be the new time, and return the record.
+	if record.P0LastSpotPrice.IsZero() {
+		newRecord.LastErrorTime = newTime
+		return newRecord
+	}
+
 	// logP0SpotPrice = log_{2}{P_0}
 	logP0SpotPrice := twapLog(record.P0LastSpotPrice)
 	// p0NewGeomAccum = log_{2}{P_0} * timeDelta
@@ -259,7 +267,12 @@ func computeTwap(startRecord types.TwapRecord, endRecord types.TwapRecord, quote
 }
 
 // twapLog returns the logarithm of the given spot price, base 2.
+// Panics if zero is given.
 func twapLog(price sdk.Dec) sdk.Dec {
+	if price.IsZero() {
+		panic("twap: cannot take logarithm of zero")
+	}
+
 	return osmomath.BigDecFromSDKDec(price).LogBase2().SDKDec()
 }
 

diff --git a/x/twap/logic_test.go b/x/twap/logic_test.go
@@ -270,10 +270,20 @@ func TestRecordWithUpdatedAccumulators(t *testing.T) {
 			newTime:   time.Unix(1, 0),
 			expRecord: newExpRecord(oneDec, twoDec, pointFiveDec),
 		},
-		"zero spot price - panic": {
-			record:      withPrice0Set(defaultRecord, sdk.ZeroDec()),
-			newTime:     defaultRecord.Time.Add(time.Second),
-			expectPanic: true,
+		"sp0 - zero spot price - accum0 unchanged, accum1 updated, geom accum unchanged, last err time set": {
+			record:    withPrice0Set(defaultRecord, sdk.ZeroDec()),
+			newTime:   defaultRecord.Time.Add(time.Second),
+			expRecord: withLastErrTime(newExpRecord(oneDec, twoDec.Add(sdk.NewDecWithPrec(1, 1).Mul(OneSec)), pointFiveDec), defaultRecord.Time.Add(time.Second)),
+		},
+		"sp1 - zero spot price - accum0 updated, accum1 unchanged, geom accum updated correctly": {
+			record:    withPrice1Set(defaultRecord, sdk.ZeroDec()),
+			newTime:   defaultRecord.Time.Add(time.Second),
+			expRecord: newExpRecord(tenSecAccum.Add(oneDec), twoDec, pointFiveDec.Add(geometricTenSecAccum)),
+		},
+		"both sp - zero spot price - accum0 unchange, accum1 unchanged, geom accum unchanged": {
+			record:    withPrice1Set(withPrice0Set(defaultRecord, sdk.ZeroDec()), sdk.ZeroDec()),
+			newTime:   defaultRecord.Time.Add(time.Second),
+			expRecord: withLastErrTime(newExpRecord(oneDec, twoDec, pointFiveDec), defaultRecord.Time.Add(time.Second)),
 		},
 		"spot price of one - geom accumulator 0": {
 			record:    withPrice1Set(withPrice0Set(defaultRecord, sdk.OneDec()), sdk.OneDec()),
@@ -1329,6 +1339,53 @@ func (s *TestSuite) TestTwapLog_CorrectBase() {
 	s.Require().Equal(expectedValue, result)
 }
 
+func (s *TestSuite) TestTwapLog() {
+	smallestAdditiveTolerance := osmomath.ErrTolerance{
+		AdditiveTolerance: sdk.SmallestDec(),
+	}
+
+	testcases := []struct {
+		name        string
+		price       sdk.Dec
+		expected    sdk.Dec
+		expectPanic bool
+	}{
+		{
+			"max spot price",
+			gammtypes.MaxSpotPrice,
+			// log_2{2^128 - 1} = 128
+			sdk.MustNewDecFromStr("127.999999999999999999"),
+			false,
+		},
+		{
+			"zero price - panic",
+			sdk.ZeroDec(),
+			sdk.Dec{},
+			true,
+		},
+		{
+			"smallest dec",
+			sdk.SmallestDec(),
+			// https://www.wolframalpha.com/input?i=log+base+2+of+%2810%5E-18%29+with+20+digits
+			sdk.MustNewDecFromStr("59.794705707972522262").Neg(),
+			false,
+		},
+	}
+
+	for _, tc := range testcases {
+		s.Run(tc.name, func() {
+			osmoassert.ConditionalPanic(s.T(), tc.expectPanic, func() {
+				result := twap.TwapLog(tc.price)
+
+				smallestAdditiveTolerance.CompareBigDec(
+					osmomath.BigDecFromSDKDec(tc.expected),
+					osmomath.BigDecFromSDKDec(result),
+				)
+			})
+		})
+	}
+}
+
 // TestTwapPow_CorrectBase tests that the base of 2 is used for the twap power function.
 // 2^3 = 8
 func (s *TestSuite) TestTwapPow_CorrectBase() {