Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove osmo multihop discount #6468

Merged
merged 6 commits into from
Sep 26, 2023
Merged

feat: remove osmo multihop discount #6468

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
476e8c8
remove osmo multihop discount
czarcas7ic Sep 20, 2023
036f283
update changelog
devbot-wizard Sep 20, 2023
eeafde4
edit comments
czarcas7ic Sep 20, 2023
7d1c6bf
add sanity check that reduced swap fee is no longer applied
czarcas7ic Sep 20, 2023
bdd1ccc
Merge branch 'main' into adam/remove-osmo-multiho-discount
czarcas7ic Sep 20, 2023
aba729e
Merge branch 'main' into adam/remove-osmo-multiho-discount
mattverse Sep 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

* [#6427](https://github.com/osmosis-labs/osmosis/pull/6427) sdk.Coins Mul and Quo helpers in osmoutils
* [#6437](https://github.com/osmosis-labs/osmosis/pull/6437) mutative version for QuoRoundUp
* [#6468](https://github.com/osmosis-labs/osmosis/pull/6468) feat: remove osmo multihop discount
* [#6261](https://github.com/osmosis-labs/osmosis/pull/6261) mutative and efficient BigDec truncations with arbitrary decimals

### Misc Improvements
Expand Down
5 changes: 0 additions & 5 deletions x/poolmanager/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,11 +309,6 @@ multiple pools in the process.
The most cost-efficient route is determined offline and the list of the pools is provided externally, by user, during the broadcasting of the swapping transaction.
At the moment of execution, the provided route may not be the most cost-efficient one anymore.

When a trade consists of just two OSMO-included routes during a single transaction,
the spread factors on each hop would be automatically halved.
Example: for converting `ATOM -> OSMO -> LUNA` using two pools with spread factors `0.3% + 0.2%`,
instead `0.15% + 0.1%` spread factors will be applied.

[Multi-Hop](https://github.com/osmosis-labs/osmosis/blob/f26ceb958adaaf31510e17ed88f5eab47e2bac03/x/poolmanager/router.go#L16)

## Route Splitting
Expand Down
19 changes: 0 additions & 19 deletions x/poolmanager/export_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,6 @@ func (k Keeper) GetNextPoolIdAndIncrement(ctx sdk.Context) uint64 {
return k.getNextPoolIdAndIncrement(ctx)
}

func (k Keeper) GetOsmoRoutedMultihopTotalSpreadFactor(ctx sdk.Context, route types.MultihopRoute) (
totalPathSpreadFactor osmomath.Dec, sumOfSpreadFactors osmomath.Dec, err error,
) {
return k.getOsmoRoutedMultihopTotalSpreadFactor(ctx, route)
}

// SetPoolRoutesUnsafe sets the given routes to the poolmanager keeper
// to allow routing from a pool type to a certain swap module.
// For example, balancer -> gamm.
Expand All @@ -43,10 +37,6 @@ func (k Keeper) ValidateCreatedPool(ctx sdk.Context, poolId uint64, pool types.P
return k.validateCreatedPool(poolId, pool)
}

func (k Keeper) IsOsmoRoutedMultihop(ctx sdk.Context, route types.MultihopRoute, inDenom, outDenom string) (isRouted bool) {
return k.isOsmoRoutedMultihop(ctx, route, inDenom, outDenom)
}

func (k Keeper) CreateMultihopExpectedSwapOuts(
ctx sdk.Context,
route []types.SwapAmountOutRoute,
Expand All @@ -55,15 +45,6 @@ func (k Keeper) CreateMultihopExpectedSwapOuts(
return k.createMultihopExpectedSwapOuts(ctx, route, tokenOut)
}

func (k Keeper) CreateOsmoMultihopExpectedSwapOuts(
ctx sdk.Context,
route []types.SwapAmountOutRoute,
tokenOut sdk.Coin,
cumulativeRouteSwapFee, sumOfSwapFees osmomath.Dec,
) ([]osmomath.Int, error) {
return k.createOsmoMultihopExpectedSwapOuts(ctx, route, tokenOut, cumulativeRouteSwapFee, sumOfSwapFees)
}

func (k Keeper) CalcTakerFeeExactIn(tokenIn sdk.Coin, takerFee osmomath.Dec) (sdk.Coin, sdk.Coin) {
return k.calcTakerFeeExactIn(tokenIn, takerFee)
}
Expand Down
194 changes: 2 additions & 192 deletions x/poolmanager/router.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import (

sdk "github.com/cosmos/cosmos-sdk/types"

appparams "github.com/osmosis-labs/osmosis/v19/app/params"

"github.com/osmosis-labs/osmosis/osmomath"
"github.com/osmosis-labs/osmosis/osmoutils"
"github.com/osmosis-labs/osmosis/v19/x/poolmanager/types"
Expand All @@ -30,36 +28,12 @@ func (k Keeper) RouteExactAmountIn(
tokenIn sdk.Coin,
tokenOutMinAmount osmomath.Int,
) (tokenOutAmount osmomath.Int, err error) {
var (
isMultiHopRouted bool
routeSpreadFactor osmomath.Dec
sumOfSpreadFactors osmomath.Dec
)

// Ensure that provided route is not empty and has valid denom format.
routeStep := types.SwapAmountInRoutes(route)
if err := routeStep.Validate(); err != nil {
return osmomath.Int{}, err
}

// In this loop (isOsmoRoutedMultihop), we check if:
// - the routeStep is of length 2
// - routeStep 1 and routeStep 2 don't trade via the same pool
// - routeStep 1 contains uosmo
// - both routeStep 1 and routeStep 2 are incentivized pools
//
// If all of the above is true, then we collect the additive and max fee between the
// two pools to later calculate the following:
// total_spread_factor = max(spread_factor1, spread_factor2)
// fee_per_pool = total_spread_factor * ((pool_fee) / (spread_factor1 + spread_factor2))
if k.isOsmoRoutedMultihop(ctx, routeStep, route[0].TokenOutDenom, tokenIn.Denom) {
isMultiHopRouted = true
routeSpreadFactor, sumOfSpreadFactors, err = k.getOsmoRoutedMultihopTotalSpreadFactor(ctx, routeStep)
if err != nil {
return osmomath.Int{}, err
}
}

// Iterate through the route and execute a series of swaps through each pool.
for i, routeStep := range route {
// To prevent the multihop swap from being interrupted prematurely, we keep
Expand Down Expand Up @@ -88,12 +62,6 @@ func (k Keeper) RouteExactAmountIn(

spreadFactor := pool.GetSpreadFactor(ctx)

// If we determined the route is an osmo multi-hop and both routes are incentivized,
// we modify the spread factor accordingly.
if isMultiHopRouted {
spreadFactor = routeSpreadFactor.MulRoundUp((spreadFactor.QuoRoundUp(sumOfSpreadFactors)))
}

tokenInAfterSubTakerFee, err := k.chargeTakerFee(ctx, tokenIn, routeStep.TokenOutDenom, sender, true)
if err != nil {
return osmomath.Int{}, err
Expand Down Expand Up @@ -273,12 +241,6 @@ func (k Keeper) MultihopEstimateOutGivenExactAmountIn(
route []types.SwapAmountInRoute,
tokenIn sdk.Coin,
) (tokenOutAmount osmomath.Int, err error) {
var (
isMultiHopRouted bool
routeSpreadFactor osmomath.Dec
sumOfSpreadFactors osmomath.Dec
)

// recover from panic
defer func() {
if r := recover(); r != nil {
Expand All @@ -292,14 +254,6 @@ func (k Keeper) MultihopEstimateOutGivenExactAmountIn(
return osmomath.Int{}, err
}

if k.isOsmoRoutedMultihop(ctx, routeStep, route[0].TokenOutDenom, tokenIn.Denom) {
isMultiHopRouted = true
routeSpreadFactor, sumOfSpreadFactors, err = k.getOsmoRoutedMultihopTotalSpreadFactor(ctx, routeStep)
if err != nil {
return osmomath.Int{}, err
}
}

for _, routeStep := range route {
swapModule, err := k.GetPoolModule(ctx, routeStep.PoolId)
if err != nil {
Expand All @@ -314,12 +268,6 @@ func (k Keeper) MultihopEstimateOutGivenExactAmountIn(

spreadFactor := poolI.GetSpreadFactor(ctx)

// If we determined the routeStep is an osmo multi-hop and both route are incentivized,
// we modify the swap fee accordingly.
if isMultiHopRouted {
spreadFactor = routeSpreadFactor.Mul((spreadFactor.Quo(sumOfSpreadFactors)))
}

takerFee, err := k.GetTradingPairTakerFee(ctx, routeStep.TokenOutDenom, tokenIn.Denom)
if err != nil {
return osmomath.Int{}, err
Expand Down Expand Up @@ -369,30 +317,8 @@ func (k Keeper) RouteExactAmountOut(ctx sdk.Context,
}
}()

// In this loop (isOsmoRoutedMultihop), we check if:
// - the routeStep is of length 2
// - routeStep 1 and routeStep 2 don't trade via the same pool
// - routeStep 1 contains uosmo
// - both routeStep 1 and routeStep 2 are incentivized pools
//
// if all of the above is true, then we collect the additive and max fee between the two pools to later calculate the following:
// total_spread_factor = total_spread_factor = max(spread_factor1, spread_factor2)
// fee_per_pool = total_spread_factor * ((pool_fee) / (spread_factor1 + spread_factor2))
var insExpected []osmomath.Int
isMultiHopRouted = k.isOsmoRoutedMultihop(ctx, routeStep, route[0].TokenInDenom, tokenOut.Denom)

// Determine what the estimated input would be for each pool along the multi-hop routeStep
// if we determined the routeStep is an osmo multi-hop and both route are incentivized,
// we utilize a separate function that calculates the discounted swap fees
if isMultiHopRouted {
routeSpreadFactor, sumOfSpreadFactors, err = k.getOsmoRoutedMultihopTotalSpreadFactor(ctx, routeStep)
if err != nil {
return osmomath.Int{}, err
}
insExpected, err = k.createOsmoMultihopExpectedSwapOuts(ctx, route, tokenOut, routeSpreadFactor, sumOfSpreadFactors)
} else {
insExpected, err = k.createMultihopExpectedSwapOuts(ctx, route, tokenOut)
}
insExpected, err = k.createMultihopExpectedSwapOuts(ctx, route, tokenOut)

if err != nil {
return osmomath.Int{}, err
Expand Down Expand Up @@ -573,7 +499,6 @@ func (k Keeper) MultihopEstimateInGivenExactAmountOut(
route []types.SwapAmountOutRoute,
tokenOut sdk.Coin,
) (tokenInAmount osmomath.Int, err error) {
isMultiHopRouted, routeSpreadFactor, sumOfSpreadFactors := false, osmomath.Dec{}, osmomath.Dec{}
var insExpected []osmomath.Int

// recover from panic
Expand All @@ -589,22 +514,8 @@ func (k Keeper) MultihopEstimateInGivenExactAmountOut(
return osmomath.Int{}, err
}

if k.isOsmoRoutedMultihop(ctx, routeStep, route[0].TokenInDenom, tokenOut.Denom) {
isMultiHopRouted = true
routeSpreadFactor, sumOfSpreadFactors, err = k.getOsmoRoutedMultihopTotalSpreadFactor(ctx, routeStep)
if err != nil {
return osmomath.Int{}, err
}
}

// Determine what the estimated input would be for each pool along the multi-hop route
// if we determined the route is an osmo multi-hop and both routes are incentivized,
// we utilize a separate function that calculates the discounted spread factors
if isMultiHopRouted {
insExpected, err = k.createOsmoMultihopExpectedSwapOuts(ctx, route, tokenOut, routeSpreadFactor, sumOfSpreadFactors)
} else {
insExpected, err = k.createMultihopExpectedSwapOuts(ctx, route, tokenOut)
}
insExpected, err = k.createMultihopExpectedSwapOuts(ctx, route, tokenOut)
if err != nil {
return osmomath.Int{}, err
}
Expand Down Expand Up @@ -652,63 +563,6 @@ func (k Keeper) AllPools(
return sortedPools, nil
}

// IsOsmoRoutedMultihop determines if a multi-hop swap involves OSMO, as one of the intermediary tokens.
func (k Keeper) isOsmoRoutedMultihop(ctx sdk.Context, route types.MultihopRoute, inDenom, outDenom string) (isRouted bool) {
if route.Length() != 2 {
return false
}
intemediateDenoms := route.IntermediateDenoms()
if len(intemediateDenoms) != 1 || intemediateDenoms[0] != appparams.BaseCoinUnit {
return false
}
if inDenom == outDenom {
return false
}
poolIds := route.PoolIds()
if poolIds[0] == poolIds[1] {
return false
}

route0Incentivized := k.poolIncentivesKeeper.IsPoolIncentivized(ctx, poolIds[0])
route1Incentivized := k.poolIncentivesKeeper.IsPoolIncentivized(ctx, poolIds[1])

return route0Incentivized && route1Incentivized
}

// getOsmoRoutedMultihopTotalSpreadFactor calculates and returns the average swap fee and the sum of swap fees for
// a given route. For the former, it sets a lower bound of the highest swap fee pool in the route to ensure total
// swap fees for a route are never more than halved.
func (k Keeper) getOsmoRoutedMultihopTotalSpreadFactor(ctx sdk.Context, route types.MultihopRoute) (
totalPathSpreadFactor osmomath.Dec, sumOfSpreadFactors osmomath.Dec, err error,
) {
additiveSpreadFactor := osmomath.ZeroDec()
maxSpreadFactor := osmomath.ZeroDec()

for _, poolId := range route.PoolIds() {
swapModule, err := k.GetPoolModule(ctx, poolId)
if err != nil {
return osmomath.Dec{}, osmomath.Dec{}, err
}

pool, poolErr := swapModule.GetPool(ctx, poolId)
if poolErr != nil {
return osmomath.Dec{}, osmomath.Dec{}, poolErr
}
spreadFactor := pool.GetSpreadFactor(ctx)
additiveSpreadFactor = additiveSpreadFactor.Add(spreadFactor)
maxSpreadFactor = sdk.MaxDec(maxSpreadFactor, spreadFactor)
}

// We divide by 2 to get the average since OSMO-routed multihops always have exactly 2 pools.
averageSpreadFactor := additiveSpreadFactor.QuoInt64(2)

// We take the max here as a guardrail to ensure that there is a lowerbound on the swap fee for the
// whole route equivalent to the highest fee pool
routeSpreadFactor := sdk.MaxDec(maxSpreadFactor, averageSpreadFactor)

return routeSpreadFactor, additiveSpreadFactor, nil
}

// createMultihopExpectedSwapOuts defines the output denom and output amount for the last pool in
// the routeStep of pools the caller is intending to hop through in a fixed-output multihop tx. It estimates the input
// amount for this last pool and then chains that input as the output of the previous pool in the routeStep, repeating
Expand Down Expand Up @@ -754,50 +608,6 @@ func (k Keeper) createMultihopExpectedSwapOuts(
return insExpected, nil
}

// createOsmoMultihopExpectedSwapOuts does the same as createMultihopExpectedSwapOuts, however discounts the swap fee.
func (k Keeper) createOsmoMultihopExpectedSwapOuts(
ctx sdk.Context,
route []types.SwapAmountOutRoute,
tokenOut sdk.Coin,
cumulativeRouteSpreadFactor, sumOfSpreadFactors osmomath.Dec,
) ([]osmomath.Int, error) {
insExpected := make([]osmomath.Int, len(route))
for i := len(route) - 1; i >= 0; i-- {
routeStep := route[i]

swapModule, err := k.GetPoolModule(ctx, routeStep.PoolId)
if err != nil {
return nil, err
}

poolI, err := swapModule.GetPool(ctx, routeStep.PoolId)
if err != nil {
return nil, err
}

spreadFactor := poolI.GetSpreadFactor(ctx)

takerFee, err := k.GetTradingPairTakerFee(ctx, routeStep.TokenInDenom, tokenOut.Denom)
if err != nil {
return nil, err
}

osmoDiscountedSpreadFactor := cumulativeRouteSpreadFactor.Mul((spreadFactor.Quo(sumOfSpreadFactors)))

tokenIn, err := swapModule.CalcInAmtGivenOut(ctx, poolI, tokenOut, routeStep.TokenInDenom, osmoDiscountedSpreadFactor)
if err != nil {
return nil, err
}

tokenInAfterTakerFee, _ := k.calcTakerFeeExactOut(tokenIn, takerFee)

insExpected[i] = tokenInAfterTakerFee.Amount
tokenOut = tokenInAfterTakerFee
}

return insExpected, nil
}

// GetTotalPoolLiquidity gets the total liquidity for a given poolId.
func (k Keeper) GetTotalPoolLiquidity(ctx sdk.Context, poolId uint64) (sdk.Coins, error) {
swapModule, err := k.GetPoolModule(ctx, poolId)
Expand Down
Loading