Skip to content

Commit

Permalink
[CL Message Audit] MsgSwapExactAmountOut (#5179)
Browse files Browse the repository at this point in the history 
* progress

* more checks

* checked logic

* moved all fmterror to errors

* added test

* cleanup

* cli tested

* resolved feedbacks

* resolved feedbacks

* fixed ci

* Update x/poolmanager/router_test.go

* Update x/poolmanager/router.go

---------

Co-authored-by: Roman <[email protected]>
  • Loading branch information
@stackman27 @p0mvn @pysel
2 people authored and pysel committed Jun 6, 2023
1 parent 2f2fcfe commit dbba957
Show file tree
Hide file tree
Showing 6 changed files with 208 additions and 24 deletions.
4 changes: 2 additions & 2 deletions x/concentrated-liquidity/client/cli/tx.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ func NewCreateConcentratedPoolCmd() (*osmocli.TxCliDesc, *clmodel.MsgCreateConce

func NewCreatePositionCmd() (*osmocli.TxCliDesc, *types.MsgCreatePosition) {
return &osmocli.TxCliDesc{
Use: "create-position [pool-id] [lower-tick] [upper-tick] [coins] [token-0-min-amount] [token-1-min-amount]",
Use: "create-position [pool-id] [lower-tick] [upper-tick] [tokensProvided] [token-0-min-amount] [token-1-min-amount]",
Short: "create or add to existing concentrated liquidity position",
Example: "create-position 1 \"[-69082]\" 69082 1000000000uosmo,10000000uion 0 0 --from val --chain-id osmosis-1",
Example: "create-position 1 \"[-69082]\" 69082 10000uosmo,10000uion 0 0 --from val --chain-id osmosis-1",
}, &types.MsgCreatePosition{}
}

Expand Down
13 changes: 9 additions & 4 deletions x/concentrated-liquidity/swaps.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ func (k Keeper) SwapExactAmountIn(
return tokenOutAmount, nil
}

// SwapExactAmountOut allows users to specify the output token amount they want to receive from a swap and get the exact
// input token amount they need to provide based on the current pool prices and any applicable fees.
func (k Keeper) SwapExactAmountOut(
ctx sdk.Context,
sender sdk.AccAddress,
Expand All @@ -138,6 +140,7 @@ func (k Keeper) SwapExactAmountOut(
zeroForOne := tokenOut.Denom == asset1

// change priceLimit based on which direction we are swapping
// if zeroForOne == true, use MinSpotPrice else use MaxSpotPrice
priceLimit := swapstrategy.GetPriceLimit(zeroForOne)
tokenIn, tokenOut, _, _, _, err := k.swapInAmtGivenOut(ctx, sender, pool, tokenOut, tokenInDenom, swapFee, priceLimit)
if err != nil {
Expand All @@ -153,7 +156,7 @@ func (k Keeper) SwapExactAmountOut(
return tokenInAmount, nil
}

// SwapOutAmtGivenIn is the internal mutative method for CalcOutAmtGivenIn. Utilizing CalcOutAmtGivenIn's output, this function applies the
// swapOutAmtGivenIn is the internal mutative method for CalcOutAmtGivenIn. Utilizing CalcOutAmtGivenIn's output, this function applies the
// new tick, liquidity, and sqrtPrice to the respective pool
func (k Keeper) swapOutAmtGivenIn(
ctx sdk.Context,
Expand Down Expand Up @@ -182,6 +185,8 @@ func (k Keeper) swapOutAmtGivenIn(
return tokenIn, tokenOut, newCurrentTick, newLiquidity, newSqrtPrice, nil
}

// swapInAmtGivenOut is the internal mutative method for calcInAmtGivenOut. Utilizing calcInAmtGivenOut's output, this function applies the
// new tick, liquidity, and sqrtPrice to the respective pool.
func (k *Keeper) swapInAmtGivenOut(
ctx sdk.Context,
sender sdk.AccAddress,
Expand Down Expand Up @@ -491,7 +496,7 @@ func (k Keeper) computeInAmtGivenOut(
// take provided price limit and turn this into a sqrt price limit since formulas use sqrtPrice
sqrtPriceLimit, err := priceLimit.ApproxSqrt()
if err != nil {
return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, fmt.Errorf("issue calculating square root of price limit")
return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, types.SqrtRootCalculationError{SqrtPriceLimit: sqrtPriceLimit}
}

// set the swap strategy
Expand Down Expand Up @@ -542,13 +547,13 @@ func (k Keeper) computeInAmtGivenOut(
// if no ticks are initialized (no users have created liquidity positions) then we return an error
nextTick, ok := swapStrategy.NextInitializedTick(ctx, poolId, swapState.tick)
if !ok {
return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, fmt.Errorf("there are no more ticks initialized to fill the swap")
return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, types.InvalidTickError{}
}

// utilizing the next initialized tick, we find the corresponding nextPrice (the target price)
_, sqrtPriceNextTick, err := math.TickToSqrtPrice(nextTick)
if err != nil {
return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, fmt.Errorf("could not convert next tick (%v) to nextSqrtPrice", nextTick)
return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, types.TickToSqrtPriceConversionError{NextTick: nextTick}
}

sqrtPriceTarget := swapStrategy.GetSqrtTargetPrice(sqrtPriceNextTick)
Expand Down
16 changes: 16 additions & 0 deletions x/concentrated-liquidity/types/errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -815,3 +815,19 @@ type RanOutOfTicksForPoolError struct {
func (e RanOutOfTicksForPoolError) Error() string {
return fmt.Sprintf("ran out of ticks for pool (%d) during swap", e.PoolId)
}

type SqrtRootCalculationError struct {
SqrtPriceLimit sdk.Dec
}

func (e SqrtRootCalculationError) Error() string {
return fmt.Sprintf("issue calculating square root of price limit %s", e.SqrtPriceLimit)
}

type TickToSqrtPriceConversionError struct {
NextTick int64
}

func (e TickToSqrtPriceConversionError) Error() string {
return fmt.Sprintf("could not convert next tick to nextSqrtPrice (%v)", e.NextTick)
}
17 changes: 17 additions & 0 deletions x/poolmanager/export_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,20 @@ func (k Keeper) ValidateCreatedPool(ctx sdk.Context, poolId uint64, pool types.P
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,
tokenOut sdk.Coin,
) ([]sdk.Int, error) {
return k.createMultihopExpectedSwapOuts(ctx, route, tokenOut)
}

func (k Keeper) CreateOsmoMultihopExpectedSwapOuts(
ctx sdk.Context,
route []types.SwapAmountOutRoute,
tokenOut sdk.Coin,
cumulativeRouteSwapFee, sumOfSwapFees sdk.Dec,
) ([]sdk.Int, error) {
return k.createOsmoMultihopExpectedSwapOuts(ctx, route, tokenOut, cumulativeRouteSwapFee, sumOfSwapFees)
}
42 changes: 24 additions & 18 deletions x/poolmanager/router.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,17 +282,20 @@ func (k Keeper) MultihopEstimateOutGivenExactAmountIn(
return tokenOutAmount, err
}

// MultihopSwapExactAmountOut defines the output denom and output amount for the last pool.
// Calculation starts by providing the tokenOutAmount of the final pool to calculate the required tokenInAmount
// the calculated tokenInAmount is used as defined tokenOutAmount of the previous pool, calculating in reverse order of the swap
// Transaction succeeds if the calculated tokenInAmount of the first pool is less than the defined tokenInMaxAmount defined.
// 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 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,
tokenInMaxAmount sdk.Int,
tokenOut sdk.Coin,
) (tokenInAmount sdk.Int, err error) {
isMultiHopRouted, routeSwapFee, sumOfSwapFees := false, sdk.Dec{}, sdk.Dec{}
// Ensure that provided route is not empty and has valid denom format.
routeStep := types.SwapAmountOutRoutes(route)
if err := routeStep.Validate(); err != nil {
return sdk.Int{}, err
Expand All @@ -305,44 +308,44 @@ func (k Keeper) RouteExactAmountOut(ctx sdk.Context,
}
}()

// in this loop, we check if:
// 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_swap_fee = total_swap_fee = max(swapfee1, swapfee2)
// total_swap_fee = max(swapfee1, swapfee2)
// fee_per_pool = total_swap_fee * ((pool_fee) / (swapfee1 + swapfee2))
if k.isOsmoRoutedMultihop(ctx, routeStep, route[0].TokenInDenom, tokenOut.Denom) {
isMultiHopRouted = true
routeSwapFee, sumOfSwapFees, err = k.getOsmoRoutedMultihopTotalSwapFee(ctx, routeStep)
if err != nil {
return sdk.Int{}, err
}
}
var insExpected []sdk.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
var insExpected []sdk.Int
if isMultiHopRouted {
routeSwapFee, sumOfSwapFees, err = k.getOsmoRoutedMultihopTotalSwapFee(ctx, routeStep)
if err != nil {
return sdk.Int{}, err
}
insExpected, err = k.createOsmoMultihopExpectedSwapOuts(ctx, route, tokenOut, routeSwapFee, sumOfSwapFees)
} else {
insExpected, err = k.createMultihopExpectedSwapOuts(ctx, route, tokenOut)
}

if err != nil {
return sdk.Int{}, err
}
if len(insExpected) == 0 {
return sdk.Int{}, nil
}

insExpected[0] = tokenInMaxAmount

// Iterates through each routed pool and executes their respective swaps. Note that all of the work to get the return
// value of this method is done when we calculate insExpected – this for loop primarily serves to execute the actual
// swaps on each pool.
for i, routeStep := range route {
// Get underlying pool type corresponding to the pool ID at the current routeStep.
swapModule, err := k.GetPoolModule(ctx, routeStep.PoolId)
if err != nil {
return sdk.Int{}, err
Expand All @@ -364,10 +367,12 @@ func (k Keeper) RouteExactAmountOut(ctx sdk.Context,

// check if pool is active, if not error
if !pool.IsActive(ctx) {
return sdk.Int{}, fmt.Errorf("pool %d is not active", pool.GetId())
return sdk.Int{}, types.InactivePoolError{PoolId: pool.GetId()}
}

swapFee := pool.GetSwapFee(ctx)
// If we determined the routeStep is an osmo multi-hop and both route are incentivized,
// we modify the swap fee accordingly.
if isMultiHopRouted {
swapFee = routeSwapFee.Mul((swapFee.Quo(sumOfSwapFees)))
}
Expand Down Expand Up @@ -577,6 +582,7 @@ 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
Expand Down Expand Up @@ -638,7 +644,6 @@ func (k Keeper) getOsmoRoutedMultihopTotalSwapFee(ctx sdk.Context, route types.M
// amount for this last pool and then chains that input as the output of the previous pool in the routeStep, repeating
// until the first pool is reached. It returns an array of inputs, each of which correspond to a pool ID in the
// routeStep of pools for the original multihop transaction.
// TODO: test this.
func (k Keeper) createMultihopExpectedSwapOuts(
ctx sdk.Context,
route []types.SwapAmountOutRoute,
Expand Down Expand Up @@ -670,7 +675,7 @@ func (k Keeper) createMultihopExpectedSwapOuts(
return insExpected, nil
}

// createOsmoMultihopExpectedSwapOuts does the same as createMultihopExpectedSwapOuts, however discounts the swap fee
// createOsmoMultihopExpectedSwapOuts does the same as createMultihopExpectedSwapOuts, however discounts the swap fee.
func (k Keeper) createOsmoMultihopExpectedSwapOuts(
ctx sdk.Context,
route []types.SwapAmountOutRoute,
Expand Down Expand Up @@ -704,6 +709,7 @@ func (k Keeper) createOsmoMultihopExpectedSwapOuts(
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)
if err != nil {
Expand Down
140 changes: 140 additions & 0 deletions x/poolmanager/router_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2442,3 +2442,143 @@ func (s *KeeperTestSuite) TestGetOsmoRoutedMultihopTotalSwapFee() {
})
}
}

func (suite *KeeperTestSuite) TestCreateMultihopExpectedSwapOuts() {
tests := map[string]struct {
route []types.SwapAmountOutRoute
tokenOut sdk.Coin
balancerPoolCoins []sdk.Coins
concentratedPoolDenoms [][]string
poolCoins []sdk.Coins
cumulativeRouteSwapFee sdk.Dec
sumOfSwapFees sdk.Dec

expectedSwapIns []sdk.Int
expectedError bool
}{
"happy path: one route": {
route: []types.SwapAmountOutRoute{
{
PoolId: 1,
TokenInDenom: bar,
},
},
poolCoins: []sdk.Coins{sdk.NewCoins(sdk.NewCoin(foo, sdk.NewInt(100)), sdk.NewCoin(bar, sdk.NewInt(100)))},

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{
{
PoolId: 1,
TokenInDenom: foo,
},
{
PoolId: 2,
TokenInDenom: bar,
},
},

poolCoins: []sdk.Coins{
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(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{
{
PoolId: 1,
TokenInDenom: bar,
},
},
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, sdk.NewInt(10)),
expectedSwapIns: []sdk.Int{sdk.NewInt(12)},
},
"happy path: two route with swap Fee": {
route: []types.SwapAmountOutRoute{
{
PoolId: 1,
TokenInDenom: foo,
},
{
PoolId: 2,
TokenInDenom: bar,
},
},

poolCoins: []sdk.Coins{
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(10)),
expectedSwapIns: []sdk.Int{sdk.NewInt(14), sdk.NewInt(12)},
},
"error: Invalid Pool": {
route: []types.SwapAmountOutRoute{
{
PoolId: 100,
TokenInDenom: foo,
},
},
poolCoins: []sdk.Coins{
sdk.NewCoins(sdk.NewCoin(foo, sdk.NewInt(100)), sdk.NewCoin(bar, sdk.NewInt(100))), // pool 1.
},
tokenOut: sdk.NewCoin(baz, sdk.NewInt(10)),
expectedError: true,
},
"error: calculating in given out": {
route: []types.SwapAmountOutRoute{
{
PoolId: 1,
TokenInDenom: uosmo,
},
},

poolCoins: []sdk.Coins{
sdk.NewCoins(sdk.NewCoin(foo, sdk.NewInt(100)), sdk.NewCoin(bar, sdk.NewInt(100))), // pool 1.
},
tokenOut: sdk.NewCoin(baz, sdk.NewInt(10)),
expectedSwapIns: []sdk.Int{},

expectedError: true,
},
}

for name, tc := range tests {
suite.Run(name, func() {
suite.SetupTest()

suite.createBalancerPoolsFromCoins(tc.poolCoins)

var actualSwapOuts []sdk.Int
var err error

if !tc.sumOfSwapFees.IsNil() && !tc.cumulativeRouteSwapFee.IsNil() {
actualSwapOuts, err = suite.App.PoolManagerKeeper.CreateOsmoMultihopExpectedSwapOuts(suite.Ctx, tc.route, tc.tokenOut, tc.cumulativeRouteSwapFee, tc.sumOfSwapFees)
} else {
actualSwapOuts, err = suite.App.PoolManagerKeeper.CreateMultihopExpectedSwapOuts(suite.Ctx, tc.route, tc.tokenOut)
}
if tc.expectedError {
suite.Require().Error(err)
} else {
suite.Require().NoError(err)
suite.Require().Equal(tc.expectedSwapIns, actualSwapOuts)
}
})
}
}

0 comments on commit dbba957

Please sign in to comment.