From 88aa966ba1b162b8f19fd676bf06a4b7fafd508f Mon Sep 17 00:00:00 2001 From: Matthias <97468149+matthiasmatt@users.noreply.github.com> Date: Wed, 28 Dec 2022 13:24:31 -0300 Subject: [PATCH] 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 --- CHANGELOG.md | 9 +++++-- x/dex/keeper/keeper.go | 4 +-- x/dex/keeper/swap_test.go | 51 +++++++++++++++++++++++++++++++++++++++ x/dex/types/errors.go | 24 +++++++++--------- x/dex/types/pool.go | 28 +++++++++++++-------- x/dex/types/pool_test.go | 3 ++- x/dex/types/shares.go | 15 +++++++++--- 7 files changed, 105 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74ddbcea7..7599d1c94 100644 --- 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 index 4e9cfa7eb..79539fec7 100644 --- 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 index fd2180819..c91c7a799 100644 --- 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 index f6962c337..6c21d4738 100644 --- 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 index 8e952b75a..a370be5f4 100644 --- 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 index 1747a7153..5cd7b7461 100644 --- 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 index 2e6cfff56..65bc31022 100644 --- 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