fix: unexpected panic in stableswap calcs (#1124)

* feat: fix unexpected panic in stableswap calcs * chore: changelog * CHANGELOG update for v0.16.3 Co-authored-by: Unique-Divine <[email protected]>
NibiruChain · Dec 28, 2022 · 88aa966 · 88aa966
1 parent fdd9dbd
commit 88aa966
Show file tree

Hide file tree

Showing 7 changed files with 105 additions and 29 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -38,7 +38,11 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [v0.16.2](https://github.com/NibiruChain/nibiru/releases/tag/v0.16.2) - Dec 13, 2022
+## Unreleased
+
+* ...
+
+## [v0.16.3](https://github.com/NibiruChain/nibiru/releases/tag/v0.16.3)
 
 ### Features
 
@@ -50,8 +54,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * [#1113](https://github.com/NibiruChain/nibiru/pull/1113) - fix: fix quick simulation issue
 * [#1114](https://github.com/NibiruChain/nibiru/pull/1114) - fix(dex): fix single asset join
 * [#1116](https://github.com/NibiruChain/nibiru/pull/1116) - fix(dex): unfroze pool when LP share supply of 0
+* [#1124](https://github.com/NibiruChain/nibiru/pull/1124) - fix(dex): fix unexpected panic in stableswap calcs
 
-## [v0.16.0](https://github.com/NibiruChain/nibiru/releases/tag/v0.16.0) -
+## [v0.16.2](https://github.com/NibiruChain/nibiru/releases/tag/v0.16.2) - Dec 13, 2022
 
 ### Features
 

diff --git a/x/dex/keeper/keeper.go b/x/dex/keeper/keeper.go
@@ -396,7 +396,7 @@ func (k Keeper) NewPool(
 		PoolId:  poolId,
 	})
 	if err != nil {
-		panic(err)
+		return
 	}
 
 	return poolId, nil
@@ -497,7 +497,7 @@ func (k Keeper) JoinPool(
 		RemCoins:      remCoins,
 	})
 	if err != nil {
-		panic(err)
+		return
 	}
 
 	return pool, poolSharesOut, remCoins, nil

diff --git a/x/dex/keeper/swap_test.go b/x/dex/keeper/swap_test.go
@@ -33,6 +33,57 @@ func TestSwapExactAmountIn(t *testing.T) {
 		expectedUserFinalFunds sdk.Coins
 		expectedFinalPool      types.Pool
 	}{
+		{
+			name: "testnet 2 BUG, should not panic",
+			userInitialFunds: sdk.NewCoins(
+				sdk.NewInt64Coin("unibi", 236534500),
+				sdk.NewInt64Coin("unusd", 1700000000),
+				sdk.NewInt64Coin("uusdt", 701785070),
+			),
+			initialPool: types.Pool{
+				Id: 1,
+				PoolParams: types.PoolParams{
+					SwapFee:  sdk.MustNewDecFromStr("0.01"),
+					ExitFee:  sdk.MustNewDecFromStr("0.01"),
+					PoolType: types.PoolType_STABLESWAP,
+					A:        sdk.NewInt(10),
+				},
+				PoolAssets: []types.PoolAsset{
+					{Token: sdk.NewInt64Coin("unusd", 1_510_778_598),
+						Weight: sdk.NewInt(1)},
+					{Token: sdk.NewInt64Coin("uusdt", 7_712_056),
+						Weight: sdk.NewInt(1)},
+				},
+				TotalWeight: sdk.NewInt(2),
+				TotalShares: sdk.NewInt64Coin("nibiru/pool/1", 100),
+			},
+			tokenIn:          sdk.NewInt64Coin("unusd", 1_500_000_000),
+			tokenOutDenom:    "uusdt",
+			expectedTokenOut: sdk.NewInt64Coin("uusdt", 1_019_823),
+			expectedUserFinalFunds: sdk.NewCoins(
+				sdk.NewInt64Coin("unibi", 236_534_500),
+				sdk.NewInt64Coin("unusd", 200_000_000),
+				sdk.NewInt64Coin("uusdt", 702_804_893),
+			),
+			expectedFinalPool: types.Pool{
+				Id: 1,
+				PoolParams: types.PoolParams{
+					SwapFee:  sdk.MustNewDecFromStr("0.01"),
+					ExitFee:  sdk.MustNewDecFromStr("0.01"),
+					PoolType: types.PoolType_STABLESWAP,
+					A:        sdk.NewInt(10),
+				},
+				PoolAssets: []types.PoolAsset{
+					{Token: sdk.NewInt64Coin("unusd", 3_010_778_598),
+						Weight: sdk.NewInt(1)},
+					{Token: sdk.NewInt64Coin("uusdt", 6_692_233),
+						Weight: sdk.NewInt(1)},
+				},
+				TotalWeight: sdk.NewInt(2),
+				TotalShares: sdk.NewInt64Coin("nibiru/pool/1", 100),
+			},
+			expectedError: nil,
+		},
 		{
 			name: "regular swap",
 			userInitialFunds: sdk.NewCoins(

diff --git a/x/dex/types/errors.go b/x/dex/types/errors.go
@@ -8,17 +8,19 @@ import (
 
 // x/dex module sentinel errors
 var (
-	ErrTooFewPoolAssets         = sdkerrors.Register(ModuleName, 1, "pool should have at least 2 assets, as they must be swapping between at least two assets")
-	ErrTooManyPoolAssets        = sdkerrors.Register(ModuleName, 2, "pool has too many assets (currently capped at 2 assets per pool)")
-	ErrInvalidSwapFee           = sdkerrors.Register(ModuleName, 3, "invalid pool swap fee, must be between [0, 1]")
-	ErrInvalidExitFee           = sdkerrors.Register(ModuleName, 4, "invalid pool exit fee, must be between [0, 1]")
-	ErrInvalidTokenWeight       = sdkerrors.Register(ModuleName, 5, "token weight must be greater than zero")
-	ErrTokenNotAllowed          = sdkerrors.Register(ModuleName, 8, "token not allowed")
-	ErrInvalidPoolType          = sdkerrors.Register(ModuleName, 15, "pool_type needs to be either `balancer` or `stableswap`")
-	ErrAmplificationMissing     = sdkerrors.Register(ModuleName, 16, "amplification parameter is missing")
-	ErrAmplificationTooLow      = sdkerrors.Register(ModuleName, 17, "amplification parameter a needs to be greater than 1")
-	ErrInitialDeposit           = sdkerrors.Register(ModuleName, 19, "initial deposit requires all coins deposited")
-	ErrPoolWithSameAssetsExists = sdkerrors.Register(ModuleName, 20, "a pool with the same denoms already exists")
+	ErrTooFewPoolAssets           = sdkerrors.Register(ModuleName, 1, "pool should have at least 2 assets, as they must be swapping between at least two assets")
+	ErrTooManyPoolAssets          = sdkerrors.Register(ModuleName, 2, "pool has too many assets (currently capped at 2 assets per pool)")
+	ErrInvalidSwapFee             = sdkerrors.Register(ModuleName, 3, "invalid pool swap fee, must be between [0, 1]")
+	ErrInvalidExitFee             = sdkerrors.Register(ModuleName, 4, "invalid pool exit fee, must be between [0, 1]")
+	ErrInvalidTokenWeight         = sdkerrors.Register(ModuleName, 5, "token weight must be greater than zero")
+	ErrTokenNotAllowed            = sdkerrors.Register(ModuleName, 8, "token not allowed")
+	ErrInvalidPoolType            = sdkerrors.Register(ModuleName, 15, "pool_type needs to be either `balancer` or `stableswap`")
+	ErrAmplificationMissing       = sdkerrors.Register(ModuleName, 16, "amplification parameter is missing")
+	ErrAmplificationTooLow        = sdkerrors.Register(ModuleName, 17, "amplification parameter a needs to be greater than 1")
+	ErrInitialDeposit             = sdkerrors.Register(ModuleName, 19, "initial deposit requires all coins deposited")
+	ErrPoolWithSameAssetsExists   = sdkerrors.Register(ModuleName, 20, "a pool with the same denoms already exists")
+	ErrBorkedPool                 = sdkerrors.Register(ModuleName, 21, "the pool is borked")
+	ErrInvariantLowerAfterJoining = sdkerrors.Register(ModuleName, 22, "the invariant was unexpectedly lower after joining")
 
 	// create-pool tx cli errors
 	ErrMissingPoolFileFlag   = sdkerrors.Register(ModuleName, 6, "must pass in a pool json using the --pool-file flag")

diff --git a/x/dex/types/pool.go b/x/dex/types/pool.go
@@ -278,7 +278,7 @@ func (pool *Pool) setInitialPoolAssets(poolAssets []PoolAsset) (err error) {
 // A * sum(x_i) * n**n + D = A * D * n**n + D**(n+1) / (n**n * prod(x_i))
 // Converging solution:
 // D[j+1] = (A * n**n * sum(x_i) - D[j]**(n+1) / (n**n prod(x_i))) / (A * n**n - 1)
-func (pool Pool) getD(poolAssets []PoolAsset) *uint256.Int {
+func (pool Pool) getD(poolAssets []PoolAsset) (*uint256.Int, error) {
 	nCoins := uint256.NewInt().SetUint64(uint64(len(poolAssets)))
 
 	S := uint256.NewInt()
@@ -338,15 +338,14 @@ func (pool Pool) getD(poolAssets []PoolAsset) *uint256.Int {
 		D.Div(num, denom)
 
 		absDifference.Abs(uint256.NewInt().Sub(D, previousD))
-		if absDifference.Lt(uint256.NewInt().SetUint64(1)) {
-			return D
+		if absDifference.Lt(uint256.NewInt().SetUint64(2)) { // absDifference LTE 1 -> absDifference LT 2
+			return D, nil
 		}
 	}
 
-	// # convergence typically occurs in 4 rounds or less, this should be unreachable!
-	// # if it does happen the pool is borked and LPs can withdraw via `remove_liquidity`
-	// panic
-	panic(nil)
+	// convergence typically occurs in 4 rounds or less, this should be unreachable!
+	// if it does happen the pool is borked and LPs can withdraw via `remove_liquidity`
+	return uint256.NewInt(), ErrBorkedPool
 }
 
 // getA returns the amplification factor of the pool
@@ -381,7 +380,10 @@ func MustSdkIntToUint256(num sdk.Int) *uint256.Int {
 // x_1 = (x_1**2 + c) / (2*x_1 + b - D)
 func (pool Pool) SolveStableswapInvariant(tokenIn sdk.Coin, tokenOutDenom string) (yAmount sdk.Int, err error) {
 	A := pool.getA()
-	D := pool.getD(pool.PoolAssets)
+	D, err := pool.getD(pool.PoolAssets)
+	if err != nil {
+		return
+	}
 
 	Ann := uint256.NewInt()
 	nCoins := uint256.NewInt().SetUint64(uint64(len(pool.PoolAssets)))
@@ -453,11 +455,17 @@ func (pool Pool) SolveStableswapInvariant(tokenIn sdk.Coin, tokenOutDenom string
 
 		absDifference := uint256.NewInt()
 		absDifference.Abs(uint256.NewInt().Sub(y, y_prev))
-		if absDifference.Lt(uint256.NewInt().SetUint64(1)) {
+		if absDifference.Lt(uint256.NewInt().SetUint64(2)) { // LTE 1
 			return sdk.NewIntFromUint64(y.Uint64()), nil
 		}
 	}
 
+	errvals := fmt.Sprintf(
+		"y=%v\ny_prev=%v\nb=%v\nD=%v\nc=%v\nS=%v\n",
+		y, y_prev, b, D, c, S,
+	)
+
 	// Should converge in a couple of round unless pool is borked
-	panic(nil)
+	err = fmt.Errorf("%w: unable to compute the SolveStableswapInvariant for values %s", ErrBorkedPool, errvals)
+	return
 }
diff --git a/x/dex/types/pool_test.go b/x/dex/types/pool_test.go
@@ -766,7 +766,8 @@ func TestGetD(t *testing.T) {
 				PoolParams: PoolParams{A: tc.amplificationParameter},
 			}
 
-			D := pool.getD(pool.PoolAssets)
+			D, err := pool.getD(pool.PoolAssets)
+			require.NoError(t, err)
 			require.EqualValues(t, tc.expectedD, D.Uint64())
 		})
 	}

diff --git a/x/dex/types/shares.go b/x/dex/types/shares.go
@@ -116,7 +116,11 @@ func (pool Pool) numSharesOutFromTokensInStableSwap(tokensIn sdk.Coins) (
 ) {
 	tokenSupply := pool.TotalShares.Amount
 
-	D0 := sdk.NewInt(int64(pool.getD(pool.PoolAssets).Uint64()))
+	D, err := pool.getD(pool.PoolAssets)
+	if err != nil {
+		return
+	}
+	D0 := sdk.NewInt(int64(D.Uint64()))
 
 	var newPoolAssets []PoolAsset
 
@@ -132,10 +136,15 @@ func (pool Pool) numSharesOutFromTokensInStableSwap(tokensIn sdk.Coins) (
 		}
 	}
 
-	D1 := sdk.NewInt(int64(pool.getD(newPoolAssets).Uint64()))
+	newD, err := pool.getD(newPoolAssets)
+	if err != nil {
+		return
+	}
+	D1 := sdk.NewInt(int64(newD.Uint64()))
 	if D1.LT(D0) {
 		// Should not happen
-		panic(nil)
+		err = ErrInvariantLowerAfterJoining
+		return
 	}
 
 	// Calculate, how much pool tokens to mint