Repository: osmosis-labs/osmosis
Pull request #2777 (2022-09-19 05:20:41)

[stableswap]: Convert all core stableswap arithmetic to use `BigDec`
<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺
v                               ✰  Thanks for creating a PR! ✰    
v    Before smashing the submit button please review the checkboxes.
v    If a checkbox is n/a - please still include it but + a little note why
v    If your PR doesn't close an issue, that's OK!  Just remove the Closes: #XXX line!
☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->

Closes: #2776 

## What is the purpose of the change

This PR updates all core arithmetic operations in stableswap to use our `BigDec` type instead of the regular `sdk.Dec` type.

## Brief Changelog

- Update all uses of `sdk.Dec` for internal calculations to use `BigDec`
- Add methods for converting from `sdk.Dec` to `BigDec` and vice versa to our `BigDec` implementation

## Testing and Verifying

- The core change is currently a trivial refactor as `sdk.Dec` and `BigDec` have the same precision, but this change will require more thorough testing once we increase precision bounds in `BigDec`
- The conversion methods for `sdk.Dec` <-> `BigDec` only have basic sanity tests, as testing them more thoroughly requires finalizing the specific decimal precision we want for `BigDec` (left for a future PR to not block this one)

## Documentation and Release Note

  - Does this pull request introduce a new feature or user-facing behavior changes? (no)
  - Is a relevant changelog entry added to the `Unreleased` section in `CHANGELOG.md`? (yes / no)
  - How is the feature or change documented? (not documented)


Patch:
diff --git a/osmomath/decimal.go b/osmomath/decimal.go
index 527bab9ca..3125d64fa 100644
--- a/osmomath/decimal.go
+++ b/osmomath/decimal.go
@@ -498,7 +498,7 @@ func (d BigDec) MustFloat64() float64 {
 
 // SdkDec returns the Sdk.Dec representation of a BigDec.
 // Values in any additional decimal places are truncated.
-func (d BigDec) SdkDec() sdk.Dec {
+func (d BigDec) SDKDec() sdk.Dec {
 	precisionDiff := Precision - sdk.Precision
 	precisionFactor := new(big.Int).Exp(big.NewInt(10), big.NewInt(int64(precisionDiff)), nil)
 
@@ -506,14 +506,19 @@ func (d BigDec) SdkDec() sdk.Dec {
 		panic("invalid decimal precision")
 	}
 
-	truncatedDec := sdk.NewDecFromBigIntWithPrec(new(big.Int).Quo(d.BigInt(), precisionFactor), sdk.Precision)
+	// Truncate any additional decimal values that exist due to BigDec's additional precision
+	// This relies on big.Int's Quo function doing floor division
+	intRepresentation := new(big.Int).Quo(d.BigInt(), precisionFactor)
+
+	// convert int representation back to SDK Dec precision
+	truncatedDec := sdk.NewDecFromBigIntWithPrec(intRepresentation, sdk.Precision)
 
 	return truncatedDec
 }
 
 // BigDecFromSdkDec returns the BigDec representation of an SdkDec.
 // Values in any additional decimal places are truncated.
-func BigDecFromSdkDec(d sdk.Dec) BigDec {
+func BigDecFromSDKDec(d sdk.Dec) BigDec {
 	return NewDecFromBigIntWithPrec(d.BigInt(), sdk.Precision)
 }
 
diff --git a/x/gamm/pool-models/stableswap/amm.go b/x/gamm/pool-models/stableswap/amm.go
index 186c39b4a..b6c9e9100 100644
--- a/x/gamm/pool-models/stableswap/amm.go
+++ b/x/gamm/pool-models/stableswap/amm.go
@@ -360,9 +360,9 @@ func (p *Pool) calcOutAmtGivenIn(tokenIn sdk.Coin, tokenOutDenom string, swapFee
 	}
 	tokenInSupply, tokenOutSupply := reserves[0], reserves[1]
 	// We are solving for the amount of token out, hence x = tokenOutSupply, y = tokenInSupply
-	cfmmOut := solveCfmm(osmomath.BigDecFromSdkDec(tokenOutSupply), osmomath.BigDecFromSdkDec(tokenInSupply), osmomath.BigDecFromSdkDec(tokenIn.Amount.ToDec()))
+	cfmmOut := solveCfmm(osmomath.BigDecFromSDKDec(tokenOutSupply), osmomath.BigDecFromSDKDec(tokenInSupply), osmomath.BigDecFromSDKDec(tokenIn.Amount.ToDec()))
 	outAmt := p.getDescaledPoolAmt(tokenOutDenom, cfmmOut)
-	return outAmt.SdkDec(), nil
+	return outAmt.SDKDec(), nil
 }
 
 // returns inAmt as a decimal
@@ -374,9 +374,9 @@ func (p *Pool) calcInAmtGivenOut(tokenOut sdk.Coin, tokenInDenom string, swapFee
 	tokenInSupply, tokenOutSupply := reserves[0], reserves[1]
 	// We are solving for the amount of token in, cfmm(x,y) = cfmm(x + x_in, y - y_out)
 	// x = tokenInSupply, y = tokenOutSupply, yIn = -tokenOutAmount
-	cfmmIn := solveCfmm(osmomath.BigDecFromSdkDec(tokenInSupply), osmomath.BigDecFromSdkDec(tokenOutSupply), osmomath.BigDecFromSdkDec(tokenOut.Amount.ToDec().Neg()))
+	cfmmIn := solveCfmm(osmomath.BigDecFromSDKDec(tokenInSupply), osmomath.BigDecFromSDKDec(tokenOutSupply), osmomath.BigDecFromSDKDec(tokenOut.Amount.ToDec().Neg()))
 	inAmt := p.getDescaledPoolAmt(tokenInDenom, cfmmIn.Neg())
-	return inAmt.SdkDec(), nil
+	return inAmt.SDKDec(), nil
 }
 
 func (p *Pool) calcSingleAssetJoinShares(tokenIn sdk.Coin, swapFee sdk.Dec) (sdk.Int, error) {
diff --git a/x/gamm/pool-models/stableswap/pool.go b/x/gamm/pool-models/stableswap/pool.go
index d88e63ab2..9a3c4d5b0 100644
--- a/x/gamm/pool-models/stableswap/pool.go
+++ b/x/gamm/pool-models/stableswap/pool.go
@@ -243,9 +243,9 @@ func (p Pool) SpotPrice(ctx sdk.Context, baseAssetDenom string, quoteAssetDenom
 		return sdk.Dec{}, err
 	}
 
-	scaledSpotPrice := spotPrice(osmomath.BigDecFromSdkDec(reserves[0]), osmomath.BigDecFromSdkDec(reserves[1]))
+	scaledSpotPrice := spotPrice(osmomath.BigDecFromSDKDec(reserves[0]), osmomath.BigDecFromSDKDec(reserves[1]))
 	spotPrice := p.getDescaledPoolAmt(baseAssetDenom, scaledSpotPrice)
-	spotPriceSdkDec := spotPrice.SdkDec()
+	spotPriceSdkDec := spotPrice.SDKDec()
 
 	return spotPriceSdkDec, nil
 }
diff --git a/x/twap/types/utils.go b/x/twap/types/utils.go
index 18ce483ec..2674aa861 100644
--- a/x/twap/types/utils.go
+++ b/x/twap/types/utils.go
@@ -6,31 +6,28 @@ import (
 	"time"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
-
-	"github.com/osmosis-labs/osmosis/v12/osmoutils"
 )
 
 var MaxSpotPrice = sdk.NewDec(2).Power(128).Sub(sdk.OneDec())
 
 // GetAllUniqueDenomPairs returns all unique pairs of denoms, where for every pair
-// (X, Y), X > Y.
+// (X, Y), X < Y.
 // The pair (X,Y) should only appear once in the list
 // Panics if finds duplicate pairs.
 //
 // NOTE: Sorts the input denoms slice.
 func GetAllUniqueDenomPairs(denoms []string) ([]string, []string) {
-	// get denoms in descending order
+	// get denoms in ascending order
 	sort.Strings(denoms)
-	reverseDenoms := osmoutils.ReverseSlice(denoms)
 
 	numPairs := len(denoms) * (len(denoms) - 1) / 2
 	pairGT := make([]string, 0, numPairs)
 	pairLT := make([]string, 0, numPairs)
 
-	for i := 0; i < len(reverseDenoms); i++ {
-		for j := i + 1; j < len(reverseDenoms); j++ {
-			pairGT = append(pairGT, reverseDenoms[i])
-			pairLT = append(pairLT, reverseDenoms[j])
+	for i := 0; i < len(denoms); i++ {
+		for j := i + 1; j < len(denoms); j++ {
+			pairGT = append(pairGT, denoms[i])
+			pairLT = append(pairLT, denoms[j])
 		}
 	}
 	// sanity check


Test patch:
diff --git a/osmomath/decimal_test.go b/osmomath/decimal_test.go
index b5441930e..09a419e41 100644
--- a/osmomath/decimal_test.go
+++ b/osmomath/decimal_test.go
@@ -167,9 +167,9 @@ func (s *decimalTestSuite) TestSdkDec() {
 	}
 	for tcIndex, tc := range tests {
 		if tc.expPanic {
-			s.Require().Panics(func() { tc.d.SdkDec() })
+			s.Require().Panics(func() { tc.d.SDKDec() })
 		} else {
-			value := tc.d.SdkDec()
+			value := tc.d.SDKDec()
 			s.Require().Equal(tc.want, value, "bad SdkDec(), index: %v", tcIndex)
 		}
 	}
@@ -192,9 +192,9 @@ func (s *decimalTestSuite) TestBigDecFromSdkDec() {
 	}
 	for tcIndex, tc := range tests {
 		if tc.expPanic {
-			s.Require().Panics(func() { BigDecFromSdkDec(tc.d) })
+			s.Require().Panics(func() { BigDecFromSDKDec(tc.d) })
 		} else {
-			value := BigDecFromSdkDec(tc.d)
+			value := BigDecFromSDKDec(tc.d)
 			s.Require().Equal(tc.want, value, "bad BigDecFromSdkDec(), index: %v", tcIndex)
 		}
 	}
diff --git a/x/twap/types/utils_test.go b/x/twap/types/utils_test.go
index d757faf0e..082344a33 100644
--- a/x/twap/types/utils_test.go
+++ b/x/twap/types/utils_test.go
@@ -21,11 +21,11 @@ func TestGetAllUniqueDenomPairs(t *testing.T) {
 		wantedPairLT []string
 		panics       bool
 	}{
-		"basic":    {[]string{"A", "B"}, []string{"B"}, []string{"A"}, false},
-		"basicRev": {[]string{"B", "A"}, []string{"B"}, []string{"A"}, false},
+		"basic":    {[]string{"A", "B"}, []string{"A"}, []string{"B"}, false},
+		"basicRev": {[]string{"B", "A"}, []string{"A"}, []string{"B"}, false},
 		// AB > A
-		"prefixed": {[]string{"A", "AB"}, []string{"AB"}, []string{"A"}, false},
-		"basic-3":  {[]string{"A", "B", "C"}, []string{"C", "C", "B"}, []string{"B", "A", "A"}, false},
+		"prefixed": {[]string{"A", "AB"}, []string{"A"}, []string{"AB"}, false},
+		"basic-3":  {[]string{"A", "B", "C"}, []string{"A", "A", "B"}, []string{"B", "C", "C"}, false},
 		"panics":   {[]string{"A", "A"}, []string{}, []string{}, true},
 	}
 	for name, tt := range tests {