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

Remove duplicated one minus spread factor calculations (backport #8040) #8066

Merged
merged 1 commit into from
Apr 17, 2024
Merged

Remove duplicated one minus spread factor calculations (backport #8040) #8066

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 5 additions & 2 deletions x/concentrated-liquidity/swapstrategy/export_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@ package swapstrategy
import "github.com/osmosis-labs/osmosis/osmomath"

func ComputeSpreadRewardChargePerSwapStepOutGivenIn(hasReachedTarget bool, amountIn, amountSpecifiedRemaining, spreadFactor osmomath.Dec) osmomath.Dec {
return computeSpreadRewardChargePerSwapStepOutGivenIn(hasReachedTarget, amountIn, amountSpecifiedRemaining, spreadFactor)
oneMinusSpreadFactorGetter := func() osmomath.Dec {
return osmomath.OneDec().Sub(spreadFactor)
}
return computeSpreadRewardChargePerSwapStepOutGivenIn(hasReachedTarget, amountIn, amountSpecifiedRemaining, spreadFactor, oneMinusSpreadFactorGetter)
}

func ComputeSpreadRewardChargeFromAmountIn(amountIn, spreadFactor osmomath.Dec) osmomath.Dec {
return computeSpreadRewardChargeFromAmountIn(amountIn, spreadFactor)
return computeSpreadRewardChargeFromAmountIn(amountIn, spreadFactor, oneDec.Sub(spreadFactor))
}
16 changes: 13 additions & 3 deletions x/concentrated-liquidity/swapstrategy/one_for_zero.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ type oneForZeroStrategy struct {
sqrtPriceLimit osmomath.BigDec
storeKey storetypes.StoreKey
spreadFactor osmomath.Dec

// oneMinusSpreadFactor is 1 - spreadFactor
oneMinusSpreadFactor osmomath.Dec
}

var _ SwapStrategy = (*oneForZeroStrategy)(nil)
Expand Down Expand Up @@ -64,7 +67,7 @@ func (s oneForZeroStrategy) ComputeSwapWithinBucketOutGivenIn(sqrtPriceCurrent,
amountOneIn := math.CalcAmount1Delta(liquidity, sqrtPriceTarget, sqrtPriceCurrent, true)

// Calculate sqrtPriceNext on the amount of token remaining after spread reward.
oneMinusTakerFee := oneDec.Sub(s.spreadFactor)
oneMinusTakerFee := s.getOneMinusSpreadFactor()
amountOneInRemainingLessSpreadReward := osmomath.NewBigDecFromDecMulDec(amountOneInRemaining, oneMinusTakerFee)

var sqrtPriceNext osmomath.BigDec
Expand Down Expand Up @@ -94,7 +97,7 @@ func (s oneForZeroStrategy) ComputeSwapWithinBucketOutGivenIn(sqrtPriceCurrent,

// Handle spread rewards.
// Note that spread reward is always charged on the amount in.
spreadRewardChargeTotal := computeSpreadRewardChargePerSwapStepOutGivenIn(hasReachedTarget, amountInDecFinal, amountOneInRemaining, s.spreadFactor)
spreadRewardChargeTotal := computeSpreadRewardChargePerSwapStepOutGivenIn(hasReachedTarget, amountInDecFinal, amountOneInRemaining, s.spreadFactor, s.getOneMinusSpreadFactor)

// Round down amount out to give user less in pool's favor.
return sqrtPriceNext, amountInDecFinal, amountZeroOut.Dec(), spreadRewardChargeTotal
Expand Down Expand Up @@ -159,7 +162,7 @@ func (s oneForZeroStrategy) ComputeSwapWithinBucketInGivenOut(sqrtPriceCurrent,

// Handle spread rewards.
// Note that spread reward is always charged on the amount in.
spreadRewardChargeTotal := computeSpreadRewardChargeFromAmountIn(amountOneInFinal, s.spreadFactor)
spreadRewardChargeTotal := computeSpreadRewardChargeFromAmountIn(amountOneInFinal, s.spreadFactor, s.getOneMinusSpreadFactor())

// Cap the output amount to not exceed the remaining output amount.
// The reason why we must do this for in given out and NOT out given in is the following:
Expand All @@ -180,6 +183,13 @@ func (s oneForZeroStrategy) ComputeSwapWithinBucketInGivenOut(sqrtPriceCurrent,
return sqrtPriceNext, amountZeroOut.Dec(), amountOneInFinal, spreadRewardChargeTotal
}

func (s oneForZeroStrategy) getOneMinusSpreadFactor() osmomath.Dec {
if s.oneMinusSpreadFactor.IsNil() {
s.oneMinusSpreadFactor = oneDec.Sub(s.spreadFactor)
}
return s.oneMinusSpreadFactor
}

// InitializeNextTickIterator returns iterator that seeks to the next tick from the given tickIndex.
// In one for zero direction, the search is EXCLUSIVE of the current tick index.
// If next tick relative to currentTickIndex is not initialized (does not exist in the store),
Expand Down
10 changes: 6 additions & 4 deletions x/concentrated-liquidity/swapstrategy/spread_rewards.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"github.com/osmosis-labs/osmosis/osmomath"
)

type oneMinusSpreadFactorGetter func() osmomath.Dec

// computeSpreadRewardChargePerSwapStepOutGivenIn returns the total spread factor charge per swap step given the parameters.
// Assumes swapping for token out given token in.
//
Expand All @@ -22,7 +24,7 @@ import (
//
// If spread factor is negative, it panics.
// If spread factor is 0, returns 0. Otherwise, computes and returns the spread factor charge per step.
func computeSpreadRewardChargePerSwapStepOutGivenIn(hasReachedTarget bool, amountIn, amountSpecifiedRemaining, spreadFactor osmomath.Dec) osmomath.Dec {
func computeSpreadRewardChargePerSwapStepOutGivenIn(hasReachedTarget bool, amountIn, amountSpecifiedRemaining, spreadFactor osmomath.Dec, oneMinSf oneMinusSpreadFactorGetter) osmomath.Dec {
if spreadFactor.IsZero() {
return osmomath.ZeroDec()
} else if spreadFactor.IsNegative() {
Expand All @@ -37,7 +39,7 @@ func computeSpreadRewardChargePerSwapStepOutGivenIn(hasReachedTarget bool, amoun
// 2) or sqrtPriceLimit is reached
// In both cases, we charge the spread factor on the amount in actually consumed before
// hitting the target.
spreadRewardChargeTotal = computeSpreadRewardChargeFromAmountIn(amountIn, spreadFactor)
spreadRewardChargeTotal = computeSpreadRewardChargeFromAmountIn(amountIn, spreadFactor, oneMinSf())
} else {
// Otherwise, the current tick had enough liquidity to fulfill the swap
// and we ran out of amount remaining before reaching either the next tick or the limit.
Expand All @@ -59,6 +61,6 @@ func computeSpreadRewardChargePerSwapStepOutGivenIn(hasReachedTarget bool, amoun
// at precision end. This is necessary to ensure that the spread factor charge is always
// rounded in favor of the pool.
// TODO: Change this fn to take in 1 - spreadFactor as it should already have been computed.
func computeSpreadRewardChargeFromAmountIn(amountIn osmomath.Dec, spreadFactor osmomath.Dec) osmomath.Dec {
return amountIn.MulRoundUp(spreadFactor).QuoRoundupMut(osmomath.OneDec().SubMut(spreadFactor))
func computeSpreadRewardChargeFromAmountIn(amountIn osmomath.Dec, spreadFactor, oneMinusSpreadFactor osmomath.Dec) osmomath.Dec {
return amountIn.MulRoundUp(spreadFactor).QuoRoundupMut(oneMinusSpreadFactor)
}
16 changes: 13 additions & 3 deletions x/concentrated-liquidity/swapstrategy/zero_for_one.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ type zeroForOneStrategy struct {
sqrtPriceLimit osmomath.BigDec
storeKey storetypes.StoreKey
spreadFactor osmomath.Dec

// oneMinusSpreadFactor is 1 - spreadFactor
oneMinusSpreadFactor osmomath.Dec
}

var _ SwapStrategy = (*zeroForOneStrategy)(nil)
Expand Down Expand Up @@ -66,7 +69,7 @@ func (s zeroForOneStrategy) ComputeSwapWithinBucketOutGivenIn(sqrtPriceCurrent,
amountZeroIn := math.CalcAmount0Delta(liquidity, sqrtPriceTarget, sqrtPriceCurrent, true) // N.B.: if this is false, causes infinite loop

// Calculate sqrtPriceNext on the amount of token remaining after spread reward.
oneMinusTakerFee := oneDec.Sub(s.spreadFactor)
oneMinusTakerFee := s.getOneMinusSpreadFactor()
amountZeroInRemainingLessSpreadReward := osmomath.NewBigDecFromDecMulDec(amountZeroInRemaining, oneMinusTakerFee)

var sqrtPriceNext osmomath.BigDec
Expand Down Expand Up @@ -96,7 +99,7 @@ func (s zeroForOneStrategy) ComputeSwapWithinBucketOutGivenIn(sqrtPriceCurrent,

// Handle spread rewards.
// Note that spread reward is always charged on the amount in.
spreadRewardChargeTotal := computeSpreadRewardChargePerSwapStepOutGivenIn(hasReachedTarget, amountZeroInFinal, amountZeroInRemaining, s.spreadFactor)
spreadRewardChargeTotal := computeSpreadRewardChargePerSwapStepOutGivenIn(hasReachedTarget, amountZeroInFinal, amountZeroInRemaining, s.spreadFactor, s.getOneMinusSpreadFactor)

// Round down amount out to give user less in pool's favor.
return sqrtPriceNext, amountZeroInFinal, amountOneOut.Dec(), spreadRewardChargeTotal
Expand Down Expand Up @@ -158,7 +161,7 @@ func (s zeroForOneStrategy) ComputeSwapWithinBucketInGivenOut(sqrtPriceCurrent,

// Handle spread rewards.
// Note that spread reward is always charged on the amount in.
spreadRewardChargeTotal := computeSpreadRewardChargeFromAmountIn(amountZeroInFinal, s.spreadFactor)
spreadRewardChargeTotal := computeSpreadRewardChargeFromAmountIn(amountZeroInFinal, s.spreadFactor, s.getOneMinusSpreadFactor())

// Cap the output amount to not exceed the remaining output amount.
// The reason why we must do this for in given out and NOT out given in is the following:
Expand All @@ -179,6 +182,13 @@ func (s zeroForOneStrategy) ComputeSwapWithinBucketInGivenOut(sqrtPriceCurrent,
return sqrtPriceNext, amountOneOut.Dec(), amountZeroInFinal, spreadRewardChargeTotal
}

func (s zeroForOneStrategy) getOneMinusSpreadFactor() osmomath.Dec {
if s.oneMinusSpreadFactor.IsNil() {
s.oneMinusSpreadFactor = oneDec.Sub(s.spreadFactor)
}
return s.oneMinusSpreadFactor
}

// InitializeNextTickIterator returns iterator that searches for the next tick given currentTickIndex.
// In zero for one direction, the search is INCLUSIVE of the current tick index.
// If next tick relative to currentTickIndex is not initialized (does not exist in the store),
Expand Down