fix: improve dust handling in EstimateTradeBasedOnPriceImpact

CHANGELOG.md

@@ -76,6 +76,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * [#6674](https://github.com/osmosis-labs/osmosis/pull/6674) fix: remove dragonberry replace directive
 * [#6692](https://github.com/osmosis-labs/osmosis/pull/6692) chore: add cur sqrt price to LiquidityNetInDirection return value
 * [#6710](https://github.com/osmosis-labs/osmosis/pull/6710) fix: `{overflow}` bug when querying cosmwasmpool spot price
+* [#6769](https://github.com/osmosis-labs/osmosis/pull/6769) fix: improve dust handling in EstimateTradeBasedOnPriceImpact
 
 ## v19.2.0
 
@@ -128,7 +129,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * [#6368](https://github.com/osmosis-labs/osmosis/pull/6368) Convert priceLimit API in CL swaps to BigDec 
 * [#6371](https://github.com/osmosis-labs/osmosis/pull/6371) Change PoolI.SpotPrice API from Dec (18 decimals) to BigDec (36 decimals), maintain state-compatibility. 
 * [#6388](https://github.com/osmosis-labs/osmosis/pull/6388) Make cosmwasmpool's create pool cli generic
-* [#6238] switch osmomath to sdkmath types and rename BigDec constructors to contain "Big" in the name.
+* [#6238](https://github.com/osmosis-labs/osmosis/pull/6238) switch osmomath to sdkmath types and rename BigDec constructors to contain "Big" in the name.
 
 Note: with the update, the Dec and Int do not get initialized to zero values by default in proto marhaling/unmarshaling. Instead, they get set to nil values.
 maxDecBitLen has changed by one bit so overflow panic can be triggerred sooner.

x/poolmanager/README.md

@@ -340,8 +340,7 @@ by the routes.
 
 The `EstimateTradeBasedOnPriceImpact` query allows users to estimate a trade for all pool types given the following parameters are provided for this request `EstimateTradeBasedOnPriceImpactRequest`:
 
-
-- **FromCoin**: (`sdk.Coin`): the total amount of tokens one wants to sell.
+- **FromCoin**: (`sdk.Coin`): is the total amount of tokens one wants to sell.
 - **ToCoinDenom**: (`string`): is the denom they want to buy with the tokens being sold.
 - **PoolId**: (`uint64`): is the identifier of the pool that the trade will happen on.
 - **MaxPriceImpact**: (`sdk.Dec`): is the maximum percentage that the user is willing to affect the price of the pool.
@@ -367,20 +366,19 @@ The following is the process in which the query finds a trade that will stay bel
    1. If the `adjustedMaxPriceImpact` was calculated to be `0` or negative it means that the `SpotPrice` is more expensive than the `ExternalPrice` and has already exceeded the possible `MaxPriceImpact`. We return a `sdk.ZeroInt()` input and output for the input and output coins indicating that no trade is viable.
 6. Then according to the pool type we attempt to find a viable trade, we must process each pool type differently as they return different results for different scenarios. The sections below explain the different pool types and how they each handle input.
 
-
 #### Balancer Pool Type Process
 
 The following is the example input/output when executing `CalcOutAmtGivenIn` on balancer pools:
 
 - If the input is greater than the total liquidity of the pool, the output will be the total liquidity of the target token.
 - If the input is an amount that is reasonably within the range of liquidity of the pool, the output will be a tolerable slippage amount based on pool data.
-- If the input is a small amount for which the pool cannot calculate a viable swap output e.g `1`, the output will be `1`, regardless of slippage.
+- If the input is a small amount for which the pool cannot calculate a viable swap output e.g `1`, the output will be a small value which can be either positive (greater or equal to 1) or zero, depending on the pool's weights. In the latter case an `ErrInvalidMathApprox` is returned.
 
-Here is the following process for the `estimateTradeBasedOnPriceImpactBalancerPool` function:
+Here is the following process for the `EstimateTradeBasedOnPriceImpactBalancerPool` function:
 
 1. The function initially calculates the output amount (`tokenOut`) using the input amount (`FromCoin`) without including a swap fee using the `CalcOutAmtGivenIn` function.
 
-   1. If `tokenOut` is zero, the function returns zero for both the input and output coin, signifying that trading a negligible amount yields no output. It is not likely that this pool type returns a zero but it is still catered for.
+   1. If `tokenOut` is zero or an `ErrInvalidMathApprox` is returned, the function returns zero for both the input and output coin, signifying that trading a negligible amount yields no output.
 
 2. The function calculates the current trade price (`currTradePrice`) using the initially estimated `tokenOut`. Following that, it calculates the deviation of this price from the spot price (`priceDeviation`).
 
@@ -402,7 +400,7 @@ The following is the example input/output when executing `CalcOutAmtGivenIn` on
 - If the input is an amount that is reasonably within the range of liquidity of the pool, the output will be a tolerable slippage amount based on pool data.
 - If the input is a small amount for which the pool cannot calculate a viable swap output e.g `1`, the function will throw an error.
 
-Here is the following process for the `estimateTradeBasedOnPriceImpactStableSwapPool` function:
+Here is the following process for the `EstimateTradeBasedOnPriceImpactStableSwapPool` function:
 
 1. The function begins by attempting to estimate the output amount (`tokenOut`) for a given input amount (`req.FromCoin`). This calculation is done without accounting for the swap fee.
 
@@ -424,10 +422,9 @@ Here is the following process for the `estimateTradeBasedOnPriceImpactStableSwap
 
 7. If the new trade amount does not cause an error or panic, and its `priceDeviation` is within limits, the function adjusts the `lowAmount` upwards to continue the search.
 
-8.  If the loop completes without finding an acceptable trade amount, the function returns zero coins for both the input and the output.
-
-9.  If a viable trade is found, the function performs a final recalculation considering the swap fee and returns the estimated trade.
+8. If the loop completes without finding an acceptable trade amount, the function returns zero coins for both the input and the output.
 
+9. If a viable trade is found, the function performs a final recalculation considering the swap fee and returns the estimated trade.
 
 #### Concentrated Liquidity Pool Type Process
 
@@ -437,7 +434,7 @@ The following is the example input/output when executing `CalcOutAmtGivenIn` on
 - If the input is an amount that is reasonably within the range of liquidity of the pool, the output will be a tolerable slippage amount based on pool data.
 - f the input is a small amount for which the pool cannot calculate a viable swap output e.g `1`, the function will return a zero.
 
-Here is the following process for the `estimateTradeBasedOnPriceImpactConcentratedLiquidity` function:
+Here is the following process for the `EstimateTradeBasedOnPriceImpactConcentratedLiquidity` function:
 
 1. The function starts by attempting to estimate the output amount (`tokenOut`) for a given input amount (`req.FromCoin`), using the `CalcOutAmtGivenIn` method of the `swapModule`.
 

x/poolmanager/router.go

@@ -3,18 +3,18 @@ package poolmanager
 import (
 	"errors"
 	"fmt"
-
 	"math/big"
 	"strings"
 
-	"github.com/osmosis-labs/osmosis/v20/x/poolmanager/client/queryproto"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	"github.com/osmosis-labs/osmosis/osmomath"
 	"github.com/osmosis-labs/osmosis/osmoutils"
+	gammtypes "github.com/osmosis-labs/osmosis/v20/x/gamm/types"
+	"github.com/osmosis-labs/osmosis/v20/x/poolmanager/client/queryproto"
 	"github.com/osmosis-labs/osmosis/v20/x/poolmanager/types"
 )
 
@@ -744,10 +744,14 @@ func (k Keeper) EstimateTradeBasedOnPriceImpactBalancerPool(
 	swapModule types.PoolModuleI,
 	poolI types.PoolI,
 ) (*queryproto.EstimateTradeBasedOnPriceImpactResponse, error) {
-	// There isn't a case where the tokenOut could be zero or an error is received but those possibilities are handled
-	// anyway.
 	tokenOut, err := swapModule.CalcOutAmtGivenIn(ctx, poolI, req.FromCoin, req.ToCoinDenom, sdk.ZeroDec())
 	if err != nil {
+		if errors.Is(err, gammtypes.ErrInvalidMathApprox) {
+			return &queryproto.EstimateTradeBasedOnPriceImpactResponse{
+				InputCoin:  sdk.NewCoin(req.FromCoin.Denom, sdk.ZeroInt()),
+				OutputCoin: sdk.NewCoin(req.ToCoinDenom, sdk.ZeroInt()),
+			}, nil
+		}
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 	if tokenOut.IsZero() {
@@ -765,6 +769,12 @@ func (k Keeper) EstimateTradeBasedOnPriceImpactBalancerPool(
 			ctx, poolI, req.FromCoin, req.ToCoinDenom, poolI.GetSpreadFactor(ctx),
 		)
 		if err != nil {
+			if errors.Is(err, gammtypes.ErrInvalidMathApprox) {
+				return &queryproto.EstimateTradeBasedOnPriceImpactResponse{
+					InputCoin:  sdk.NewCoin(req.FromCoin.Denom, sdk.ZeroInt()),
+					OutputCoin: sdk.NewCoin(req.ToCoinDenom, sdk.ZeroInt()),
+				}, nil
+			}
 			return nil, status.Error(codes.Internal, err.Error())
 		}
 
@@ -798,15 +808,18 @@ func (k Keeper) EstimateTradeBasedOnPriceImpactBalancerPool(
 		midAmount := lowAmount.Add(highAmount).Quo(sdk.NewInt(2))
 		currFromCoin = sdk.NewCoin(req.FromCoin.Denom, midAmount)
 
-		// There isn't a case where the tokenOut could be zero or an error is received but those possibilities are
-		// handled anyway.
 		tokenOut, err := swapModule.CalcOutAmtGivenIn(
 			ctx, poolI, currFromCoin, req.ToCoinDenom, sdk.ZeroDec(),
 		)
 		if err != nil {
+			if errors.Is(err, gammtypes.ErrInvalidMathApprox) {
+				return &queryproto.EstimateTradeBasedOnPriceImpactResponse{
+					InputCoin:  sdk.NewCoin(req.FromCoin.Denom, sdk.ZeroInt()),
+					OutputCoin: sdk.NewCoin(req.ToCoinDenom, sdk.ZeroInt()),
+				}, nil
+			}
 			return nil, status.Error(codes.Internal, err.Error())
 		}
-
 		if tokenOut.IsZero() {
 			return &queryproto.EstimateTradeBasedOnPriceImpactResponse{
 				InputCoin:  sdk.NewCoin(req.FromCoin.Denom, sdk.ZeroInt()),

x/poolmanager/router_test.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"reflect"
 
+	"cosmossdk.io/math"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/golang/mock/gomock"
 
@@ -1559,9 +1560,10 @@ func (s *KeeperTestSuite) TestEstimateTradeBasedOnPriceImpact() {
 	maxPriceImpactHalved := sdk.MustNewDecFromStr("0.005") // 0.5%
 	maxPriceImpactTiny := sdk.MustNewDecFromStr("0.0005")  // 0.05%
 
-	externalPriceOneBalancer := sdk.MustNewDecFromStr("0.666666667")   // Spot Price
-	externalPriceTwoBalancer := sdk.MustNewDecFromStr("0.622222222")   // Cheaper than spot price
-	externalPriceThreeBalancer := sdk.MustNewDecFromStr("0.663349917") // Transform adjusted max price impact by 50%
+	externalPriceOneBalancer := sdk.MustNewDecFromStr("0.666666667")                 // Spot Price
+	externalPriceOneBalancerInv := math.LegacyOneDec().Quo(externalPriceOneBalancer) // Inverse of externalPriceOneBalancer
+	externalPriceTwoBalancer := sdk.MustNewDecFromStr("0.622222222")                 // Cheaper than spot price
+	externalPriceThreeBalancer := sdk.MustNewDecFromStr("0.663349917")               // Transform adjusted max price impact by 50%
 
 	externalPriceOneStableSwap := sdk.MustNewDecFromStr("1.00000002")             // Spot Price
 	externalPriceTwoStableSwap := sdk.MustNewDecFromStr("0.98989903")             // Cheaper than spot price
@@ -1624,6 +1626,19 @@ func (s *KeeperTestSuite) TestEstimateTradeBasedOnPriceImpact() {
 			expectedInputCoin:  sdk.NewCoin(assetBaz, sdk.NewInt(39_947)),
 			expectedOutputCoin: sdk.NewCoin(assetBar, sdk.NewInt(59_327)),
 		},
+		"valid balancer pool - estimate trying to trade 1 token": {
+			preCreatePoolType: types.Balancer,
+			poolId:            poolId,
+			req: queryproto.EstimateTradeBasedOnPriceImpactRequest{
+				FromCoin:       sdk.NewCoin(assetBar, sdk.NewInt(1)),
+				ToCoinDenom:    assetBaz,
+				PoolId:         poolId,
+				MaxPriceImpact: maxPriceImpact,
+				ExternalPrice:  externalPriceOneBalancerInv,
+			},
+			expectedInputCoin:  sdk.NewCoin(assetBar, sdk.NewInt(0)),
+			expectedOutputCoin: sdk.NewCoin(assetBaz, sdk.NewInt(0)),
+		},
 		"valid balancer pool - estimate trying to trade dust": {
 			preCreatePoolType: types.Balancer,
 			poolId:            poolId,
@@ -1944,6 +1959,21 @@ func (s *KeeperTestSuite) TestEstimateTradeBasedOnPriceImpact() {
 			expectedInputCoin:    sdk.NewCoin(assetUsdc, sdk.NewInt(0)),
 			expectedOutputCoin:   sdk.NewCoin(assetEth, sdk.NewInt(0)),
 		},
+		"valid concentrated pool - estimate trying to trade one unit": {
+			preCreatePoolType: types.Concentrated,
+			poolId:            poolId,
+			req: queryproto.EstimateTradeBasedOnPriceImpactRequest{
+				FromCoin:       sdk.NewCoin(assetUsdc, math.OneInt()),
+				ToCoinDenom:    assetEth,
+				PoolId:         poolId,
+				MaxPriceImpact: maxPriceImpact,
+				ExternalPrice:  externalPriceOneConcentratedInv,
+			},
+			setPositionForCLPool: true,
+			setClTokens:          clCoinsLiquid,
+			expectedInputCoin:    sdk.NewCoin(assetUsdc, sdk.NewInt(0)),
+			expectedOutputCoin:   sdk.NewCoin(assetEth, sdk.NewInt(0)),
+		},
 		"valid concentrated pool - external price much greater than spot price do not trade": {
 			preCreatePoolType: types.Concentrated,
 			poolId:            poolId,