perf: Remove TakerFee from protorev estimation (#7562)

* Remove TakerFee from protorev estimation * Add Changelog
osmosis-labs · Feb 20, 2024 · 9f1eb42 · 9f1eb42
1 parent 1559790
commit 9f1eb42
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -47,6 +47,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * [#7250](https://github.com/osmosis-labs/osmosis/pull/7250) Further filter spam gauges from epoch distribution.
 * [#7472](https://github.com/osmosis-labs/osmosis/pull/7472) Refactor TWAP keys to only require a single key format. Significantly lowers TWAP-caused writes
 * [#7499](https://github.com/osmosis-labs/osmosis/pull/7499) Slight speed/gas improvements to CL CreatePosition and AddToPosition
+* [#7562](https://github.com/osmosis-labs/osmosis/pull/7562) Speedup Protorev estimation logic by removing unnecessary taker fee simulations.
 
 ## v23.0.0
 

diff --git a/x/poolmanager/router.go b/x/poolmanager/router.go
@@ -225,10 +225,27 @@ func (k Keeper) SwapExactAmountInNoTakerFee(
 	return tokenOutAmount, nil
 }
 
+func (k Keeper) MultihopEstimateOutGivenExactAmountInNoTakerFee(
+	ctx sdk.Context,
+	route []types.SwapAmountInRoute,
+	tokenIn sdk.Coin,
+) (tokenOutAmount osmomath.Int, err error) {
+	return k.multihopEstimateOutGivenExactAmountInInternal(ctx, route, tokenIn, false)
+}
+
 func (k Keeper) MultihopEstimateOutGivenExactAmountIn(
 	ctx sdk.Context,
 	route []types.SwapAmountInRoute,
 	tokenIn sdk.Coin,
+) (tokenOutAmount osmomath.Int, err error) {
+	return k.multihopEstimateOutGivenExactAmountInInternal(ctx, route, tokenIn, true)
+}
+
+func (k Keeper) multihopEstimateOutGivenExactAmountInInternal(
+	ctx sdk.Context,
+	route []types.SwapAmountInRoute,
+	tokenIn sdk.Coin,
+	applyTakerFee bool,
 ) (tokenOutAmount osmomath.Int, err error) {
 	// recover from panic
 	defer func() {
@@ -254,14 +271,18 @@ func (k Keeper) MultihopEstimateOutGivenExactAmountIn(
 
 		spreadFactor := poolI.GetSpreadFactor(ctx)
 
-		takerFee, err := k.GetTradingPairTakerFee(ctx, routeStep.TokenOutDenom, tokenIn.Denom)
-		if err != nil {
-			return osmomath.Int{}, err
-		}
+		actualTokenIn := tokenIn
+		// apply taker fee if applicable
+		if applyTakerFee {
+			takerFee, err := k.GetTradingPairTakerFee(ctx, routeStep.TokenOutDenom, tokenIn.Denom)
+			if err != nil {
+				return osmomath.Int{}, err
+			}
 
-		tokenInAfterSubTakerFee, _ := CalcTakerFeeExactIn(tokenIn, takerFee)
+			actualTokenIn, _ = CalcTakerFeeExactIn(tokenIn, takerFee)
+		}
 
-		tokenOut, err := swapModule.CalcOutAmtGivenIn(ctx, poolI, tokenInAfterSubTakerFee, routeStep.TokenOutDenom, spreadFactor)
+		tokenOut, err := swapModule.CalcOutAmtGivenIn(ctx, poolI, actualTokenIn, routeStep.TokenOutDenom, spreadFactor)
 		if err != nil {
 			return osmomath.Int{}, err
 		}

diff --git a/x/protorev/keeper/rebalance.go b/x/protorev/keeper/rebalance.go
@@ -98,7 +98,7 @@ func (k Keeper) ConvertProfits(ctx sdk.Context, inputCoin sdk.Coin, profit osmom
 // and then subtracting the amount in from the amount out to get the profit
 func (k Keeper) EstimateMultihopProfit(ctx sdk.Context, inputDenom string, amount osmomath.Int, route poolmanagertypes.SwapAmountInRoutes) (sdk.Coin, osmomath.Int, error) {
 	tokenIn := sdk.Coin{Denom: inputDenom, Amount: amount}
-	amtOut, err := k.poolmanagerKeeper.MultihopEstimateOutGivenExactAmountIn(ctx, route, tokenIn)
+	amtOut, err := k.poolmanagerKeeper.MultihopEstimateOutGivenExactAmountInNoTakerFee(ctx, route, tokenIn)
 	if err != nil {
 		return sdk.Coin{}, osmomath.ZeroInt(), err
 	}

diff --git a/x/protorev/types/expected_keepers.go b/x/protorev/types/expected_keepers.go
@@ -39,7 +39,7 @@ type PoolManagerKeeper interface {
 		tokenIn sdk.Coin,
 		tokenOutMinAmount osmomath.Int) (tokenOutAmount osmomath.Int, err error)
 
-	MultihopEstimateOutGivenExactAmountIn(
+	MultihopEstimateOutGivenExactAmountInNoTakerFee(
 		ctx sdk.Context,
 		routes []poolmanagertypes.SwapAmountInRoute,
 		tokenIn sdk.Coin,