resolved feedbacks

osmosis-labs · May 17, 2023 · 81c8a0f · 81c8a0f
1 parent 413e165
commit 81c8a0f
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 29 deletions.
diff --git a/x/poolmanager/router.go b/x/poolmanager/router.go
@@ -273,12 +273,12 @@ func (k Keeper) MultihopEstimateOutGivenExactAmountIn(
 	return tokenOutAmount, err
 }
 
-// RouteExactAmountIn processes a swap along the given route using the swap function corresponding
-// to poolID's pool type. This function is responsible for computing the optimal input amount
-// for a given output amount when swapping tokens, taking into account the current price of the
+// RouteExactAmountOut processes a swap along the given route using the swap function corresponding
+// to poolID's pool type. This function is responsible for computing the optimal output amount
+// for a given input amount when swapping tokens, taking into account the current price of the
 // tokens in the pool and any slippage.
-// Transaction succeeds if final amount out is greater than tokenOutMinAmount defined
-// and no errors are encountered along the way.
+// Transaction succeeds if the calculated tokenInAmount of the first pool is less than the defined
+// tokenInMaxAmount defined.
 func (k Keeper) RouteExactAmountOut(ctx sdk.Context,
 	sender sdk.AccAddress,
 	route []types.SwapAmountOutRoute,
@@ -292,7 +292,6 @@ func (k Keeper) RouteExactAmountOut(ctx sdk.Context,
 		return sdk.Int{}, err
 	}
 
-	// ? should we have defer in RouteExactAmountIn too?
 	defer func() {
 		if r := recover(); r != nil {
 			tokenInAmount = sdk.Int{}

diff --git a/x/poolmanager/router_test.go b/x/poolmanager/router_test.go
@@ -2452,8 +2452,8 @@ func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() {
 		cumulativeRouteSwapFee sdk.Dec
 		sumOfSwapFees          sdk.Dec
 
-		expectedSwapOuts []sdk.Int
-		expectedError    bool
+		expectedSwapIns []sdk.Int
+		expectedError   bool
 	}{
 		"happy path: one route": {
 			route: []types.SwapAmountOutRoute{
@@ -2462,10 +2462,12 @@ func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() {
 					TokenInDenom: bar,
 				},
 			},
-			poolCoins: []sdk.Coins{sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount))},
+			poolCoins: []sdk.Coins{sdk.NewCoins(sdk.NewCoin(foo, sdk.NewInt(100)), sdk.NewCoin(bar, sdk.NewInt(100)))},
 
-			tokenOut:         sdk.NewCoin(foo, defaultSwapAmount),
-			expectedSwapOuts: []sdk.Int{sdk.NewInt(1000002)},
+			tokenOut: sdk.NewCoin(foo, sdk.NewInt(10)),
+			// expectedSwapIns = (tokenOut * (poolTokenOutBalance / poolPostSwapOutBalance)).ceil()
+			// foo token = 10 * (100 / 90) ~ 12
+			expectedSwapIns: []sdk.Int{sdk.NewInt(12)},
 		},
 		"happy path: two route": {
 			route: []types.SwapAmountOutRoute{
@@ -2480,11 +2482,14 @@ func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() {
 			},
 
 			poolCoins: []sdk.Coins{
-				sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount)), // pool 1.
-				sdk.NewCoins(sdk.NewCoin(bar, defaultInitPoolAmount), sdk.NewCoin(baz, defaultInitPoolAmount)), // pool 2.
+				sdk.NewCoins(sdk.NewCoin(foo, sdk.NewInt(100)), sdk.NewCoin(bar, sdk.NewInt(100))), // pool 1.
+				sdk.NewCoins(sdk.NewCoin(bar, sdk.NewInt(100)), sdk.NewCoin(baz, sdk.NewInt(100))), // pool 2.
 			},
-			tokenOut:         sdk.NewCoin(baz, sdk.NewInt(100000)),
-			expectedSwapOuts: []sdk.Int{sdk.NewInt(100002), sdk.NewInt(100001)},
+			tokenOut: sdk.NewCoin(baz, sdk.NewInt(10)),
+			// expectedSwapIns = (tokenOut * (poolTokenOutBalance / poolPostSwapOutBalance)).ceil()
+			// foo token = 10 * (100 / 90) ~ 12
+			// bar token = 12 * (100 / 88) ~ 14
+			expectedSwapIns: []sdk.Int{sdk.NewInt(14), sdk.NewInt(12)},
 		},
 		"happy path: one route with swap Fee": {
 			route: []types.SwapAmountOutRoute{
@@ -2493,12 +2498,12 @@ func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() {
 					TokenInDenom: bar,
 				},
 			},
-			poolCoins:              []sdk.Coins{sdk.NewCoins(sdk.NewCoin(uosmo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount))},
+			poolCoins:              []sdk.Coins{sdk.NewCoins(sdk.NewCoin(uosmo, sdk.NewInt(100)), sdk.NewCoin(bar, sdk.NewInt(100)))},
 			cumulativeRouteSwapFee: sdk.NewDec(100),
 			sumOfSwapFees:          sdk.NewDec(500),
 
-			tokenOut:         sdk.NewCoin(uosmo, defaultSwapAmount),
-			expectedSwapOuts: []sdk.Int{sdk.NewInt(1000002)},
+			tokenOut:        sdk.NewCoin(uosmo, sdk.NewInt(10)),
+			expectedSwapIns: []sdk.Int{sdk.NewInt(12)},
 		},
 		"happy path: two route with swap Fee": {
 			route: []types.SwapAmountOutRoute{
@@ -2513,14 +2518,14 @@ func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() {
 			},
 
 			poolCoins: []sdk.Coins{
-				sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount)),   // pool 1.
-				sdk.NewCoins(sdk.NewCoin(bar, defaultInitPoolAmount), sdk.NewCoin(uosmo, defaultInitPoolAmount)), // pool 2.
+				sdk.NewCoins(sdk.NewCoin(foo, sdk.NewInt(100)), sdk.NewCoin(bar, sdk.NewInt(100))),   // pool 1.
+				sdk.NewCoins(sdk.NewCoin(bar, sdk.NewInt(100)), sdk.NewCoin(uosmo, sdk.NewInt(100))), // pool 2.
 			},
 			cumulativeRouteSwapFee: sdk.NewDec(100),
 			sumOfSwapFees:          sdk.NewDec(500),
 
-			tokenOut:         sdk.NewCoin(uosmo, sdk.NewInt(100000)),
-			expectedSwapOuts: []sdk.Int{sdk.NewInt(100002), sdk.NewInt(100001)},
+			tokenOut:        sdk.NewCoin(uosmo, sdk.NewInt(10)),
+			expectedSwapIns: []sdk.Int{sdk.NewInt(14), sdk.NewInt(12)},
 		},
 		"error: Invalid Pool": {
 			route: []types.SwapAmountOutRoute{
@@ -2530,9 +2535,9 @@ func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() {
 				},
 			},
 			poolCoins: []sdk.Coins{
-				sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount)), // pool 1.
+				sdk.NewCoins(sdk.NewCoin(foo, sdk.NewInt(100)), sdk.NewCoin(bar, sdk.NewInt(100))), // pool 1.
 			},
-			tokenOut:      sdk.NewCoin(baz, sdk.NewInt(100000)),
+			tokenOut:      sdk.NewCoin(baz, sdk.NewInt(10)),
 			expectedError: true,
 		},
 		"error: calculating in given out": {
@@ -2544,10 +2549,10 @@ func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() {
 			},
 
 			poolCoins: []sdk.Coins{
-				sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount), sdk.NewCoin(bar, defaultInitPoolAmount)), // pool 1.
+				sdk.NewCoins(sdk.NewCoin(foo, sdk.NewInt(100)), sdk.NewCoin(bar, sdk.NewInt(100))), // pool 1.
 			},
-			tokenOut:         sdk.NewCoin(baz, sdk.NewInt(100000)),
-			expectedSwapOuts: []sdk.Int{},
+			tokenOut:        sdk.NewCoin(baz, sdk.NewInt(10)),
+			expectedSwapIns: []sdk.Int{},
 
 			expectedError: true,
 		},
@@ -2567,12 +2572,12 @@ func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() {
 			} else {
 				actualSwapOuts, err = suite.App.PoolManagerKeeper.CreateMultihopExpectedSwapOuts(suite.Ctx, tc.route, tc.tokenOut)
 			}
-
+			fmt.Println("SISHIR", actualSwapOuts)
 			if tc.expectedError {
 				suite.Require().Error(err)
 			} else {
 				suite.Require().NoError(err)
-				suite.Require().Equal(tc.expectedSwapOuts, actualSwapOuts)
+				suite.Require().Equal(tc.expectedSwapIns, actualSwapOuts)
 			}
 		})
 	}