From 298ad00dc9f792dfccf10e864002a02bcdd0b3fb Mon Sep 17 00:00:00 2001 From: Roman <34196718+p0mvn@users.noreply.github.com> Date: Mon, 18 Apr 2022 13:25:14 -0400 Subject: [PATCH] refactor: ExitSwapExternAmountOut (#1244) * refactor: ExitSwapExternAmountOut * better name for interface extension * implement types.PoolExitSwapExternAmountOutExtension * update changelog * interface assertion for PoolI in balancer pool * err check on extendedPool.ExitSwapExternAmountOut * ZeroDec() * comma in calcPoolInGivenSingleOut call * redundnet type in calcPoolInGivenSingleOut * comment for feeRatio * fix comment for calcPoolInGivenSingleOut and the converse * whitespace in ExitSwapExternAmountOut * fix changelog entry * godoc for PoolExitSwapExternAmountOutExtension * fmt * gofumpt * deduct shares on exit, make exit pool logic shared, improve names * rename to ExitSwapExactAmountOutExtension * comment out TestNewExitSwapShareAmountInCmd * fix ExitSwapExactAmountOut by truncating int on return from calcPoolSharesInGivenSingleAssetOut * revert x/gamm/cli_test.go to original state with TestNewExitSwapExternAmountOutCmd uncommented * uncomment ExitSwapExactAmountOut in TestActiveBalancerPool * change variable name to exitingCoins * fmt * Update x/gamm/keeper/pool_service.go * Update x/gamm/pool-models/balancer/amm.go * Update x/gamm/keeper/pool_service.go Co-authored-by: Aleksandr Bezobchuk --- CHANGELOG.md | 1 + x/gamm/client/cli/cli_test.go | 81 +++++++++--------- x/gamm/keeper/msg_server.go | 2 +- x/gamm/keeper/pool_service.go | 31 ++++--- x/gamm/keeper/pool_service_test.go | 6 +- x/gamm/pool-models/balancer/amm.go | 88 ++++++++++++++++++-- x/gamm/pool-models/balancer/balancer_pool.go | 5 +- x/gamm/types/pool.go | 16 ++++ 8 files changed, 165 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 030d4e11fca..79275becce3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Features +* [#1244](https://github.com/osmosis-labs/osmosis/pull/1244) Refactor `x/gamm`'s `ExitSwapExternAmountOut`. * [#1107](https://github.com/osmosis-labs/osmosis/pull/1107) Update to wasmvm v0.24.0, re-enabling building on M1 macs! ### Minor improvements & Bug Fixes diff --git a/x/gamm/client/cli/cli_test.go b/x/gamm/client/cli/cli_test.go index 9c4498af2e7..e001dbd5a35 100644 --- a/x/gamm/client/cli/cli_test.go +++ b/x/gamm/client/cli/cli_test.go @@ -608,52 +608,51 @@ func (s IntegrationTestSuite) TestNewJoinSwapExternAmountInCmd() { } } -// Todo: Re-add once implemented -// func (s IntegrationTestSuite) TestNewExitSwapExternAmountOutCmd() { -// val := s.network.Validators[0] +func (s IntegrationTestSuite) TestNewExitSwapExternAmountOutCmd() { + val := s.network.Validators[0] -// testCases := []struct { -// name string -// args []string -// expectErr bool -// respType proto.Message -// expectedCode uint32 -// }{ -// { -// "exit swap extern amount out", // osmosisd tx gamm exit-swap-extern-amount-out --pool-id=1 10stake 1 --from=validator --keyring-backend=test --chain-id=testing --yes -// []string{ -// "10stake", "10000000000000000000", -// fmt.Sprintf("--%s=%d", cli.FlagPoolId, 1), -// fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()), -// // common args -// fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), -// fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), -// fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10))).String()), -// }, -// false, &sdk.TxResponse{}, 0, -// }, -// } + testCases := []struct { + name string + args []string + expectErr bool + respType proto.Message + expectedCode uint32 + }{ + { + "exit swap extern amount out", // osmosisd tx gamm exit-swap-extern-amount-out --pool-id=1 10stake 1 --from=validator --keyring-backend=test --chain-id=testing --yes + []string{ + "10stake", "10000000000000000000", + fmt.Sprintf("--%s=%d", cli.FlagPoolId, 1), + fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()), + // common args + fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), + fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), + fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10))).String()), + }, + false, &sdk.TxResponse{}, 0, + }, + } -// for _, tc := range testCases { -// tc := tc + for _, tc := range testCases { + tc := tc -// s.Run(tc.name, func() { -// cmd := cli.NewExitSwapExternAmountOut() -// clientCtx := val.ClientCtx + s.Run(tc.name, func() { + cmd := cli.NewExitSwapExternAmountOut() + clientCtx := val.ClientCtx -// out, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, tc.args) -// if tc.expectErr { -// s.Require().Error(err) -// } else { -// s.Require().NoError(err, out.String()) -// s.Require().NoError(clientCtx.Codec.UnmarshalJSON(out.Bytes(), tc.respType), out.String()) + out, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, tc.args) + if tc.expectErr { + s.Require().Error(err) + } else { + s.Require().NoError(err, out.String()) + s.Require().NoError(clientCtx.Codec.UnmarshalJSON(out.Bytes(), tc.respType), out.String()) -// txResp := tc.respType.(*sdk.TxResponse) -// s.Require().Equal(tc.expectedCode, txResp.Code, out.String()) -// } -// }) -// } -// } + txResp := tc.respType.(*sdk.TxResponse) + s.Require().Equal(tc.expectedCode, txResp.Code, out.String()) + } + }) + } +} // TODO: Re-add once implemented // func (s IntegrationTestSuite) TestNewJoinSwapShareAmountOutCmd() { diff --git a/x/gamm/keeper/msg_server.go b/x/gamm/keeper/msg_server.go index fb4749eaa90..ccf32342cf5 100644 --- a/x/gamm/keeper/msg_server.go +++ b/x/gamm/keeper/msg_server.go @@ -224,7 +224,7 @@ func (server msgServer) ExitSwapExternAmountOut(goCtx context.Context, msg *type return nil, err } - shareInAmount, err := server.keeper.ExitSwapExternAmountOut(ctx, sender, msg.PoolId, msg.TokenOut, msg.ShareInMaxAmount) + shareInAmount, err := server.keeper.ExitSwapExactAmountOut(ctx, sender, msg.PoolId, msg.TokenOut, msg.ShareInMaxAmount) if err != nil { return nil, err } diff --git a/x/gamm/keeper/pool_service.go b/x/gamm/keeper/pool_service.go index aafea76d787..cb6e4e20458 100644 --- a/x/gamm/keeper/pool_service.go +++ b/x/gamm/keeper/pool_service.go @@ -395,20 +395,31 @@ func (k Keeper) ExitSwapShareAmountIn( return tokenOutAmount, nil } -func (k Keeper) ExitSwapExternAmountOut( +func (k Keeper) ExitSwapExactAmountOut( ctx sdk.Context, sender sdk.AccAddress, poolId uint64, tokenOut sdk.Coin, shareInMaxAmount sdk.Int, ) (shareInAmount sdk.Int, err error) { - // Basically what we have to do is: - // estimate how many LP shares this would take to do. - // We do so by calculating how much a swap of half of tokenOut to TokenIn would be. - // Then we calculate how many LP shares that'd be worth. - // We should have code for that once we implement JoinPoolNoSwap. - // Then check if the number of shares is LTE to shareInMaxAmount. - // if so, use the needed number of shares, do exit pool, and the swap. - - panic("To implement later") + pool, err := k.getPoolForSwap(ctx, poolId) + if err != nil { + return sdk.Int{}, err + } + + extendedPool, ok := pool.(types.PoolExitSwapExactAmountOutExtension) + if !ok { + return sdk.Int{}, fmt.Errorf("pool with id %d does not support this kind of exit", poolId) + } + + shareInAmount, err = extendedPool.ExitSwapExactAmountOut(ctx, tokenOut, shareInMaxAmount) + if err != nil { + return sdk.Int{}, err + } + + if err := k.applyExitPoolStateChange(ctx, pool, sender, shareInAmount, sdk.Coins{tokenOut}); err != nil { + return sdk.Int{}, err + } + + return shareInAmount, nil } diff --git a/x/gamm/keeper/pool_service_test.go b/x/gamm/keeper/pool_service_test.go index e9dbdf7a446..4b5af5b799e 100644 --- a/x/gamm/keeper/pool_service_test.go +++ b/x/gamm/keeper/pool_service_test.go @@ -482,15 +482,15 @@ func (suite *KeeperTestSuite) TestActiveBalancerPool() { // suite.Require().NoError(err) _, err = suite.app.GAMMKeeper.ExitSwapShareAmountIn(suite.ctx, acc1, poolId, "foo", types.OneShare.MulRaw(10), sdk.ZeroInt()) suite.Require().NoError(err) - // _, err = suite.app.GAMMKeeper.ExitSwapExternAmountOut(suite.ctx, acc1, poolId, foocoin, sdk.NewInt(1000000000000000000)) - // suite.Require().NoError(err) + _, err = suite.app.GAMMKeeper.ExitSwapExactAmountOut(suite.ctx, acc1, poolId, foocoin, sdk.NewInt(1000000000000000000)) + suite.Require().NoError(err) } else { suite.Require().Error(err) _, err = suite.app.GAMMKeeper.JoinSwapShareAmountOut(suite.ctx, acc1, poolId, "foo", types.OneShare.MulRaw(10), sdk.NewInt(1000000000000000000)) suite.Require().Error(err) _, err = suite.app.GAMMKeeper.ExitSwapShareAmountIn(suite.ctx, acc1, poolId, "foo", types.OneShare.MulRaw(10), sdk.ZeroInt()) suite.Require().Error(err) - _, err = suite.app.GAMMKeeper.ExitSwapExternAmountOut(suite.ctx, acc1, poolId, foocoin, sdk.NewInt(1000000000000000000)) + _, err = suite.app.GAMMKeeper.ExitSwapExactAmountOut(suite.ctx, acc1, poolId, foocoin, sdk.NewInt(1000000000000000000)) suite.Require().Error(err) } } diff --git a/x/gamm/pool-models/balancer/amm.go b/x/gamm/pool-models/balancer/amm.go index f93149fae5f..9e3c33de7ea 100644 --- a/x/gamm/pool-models/balancer/amm.go +++ b/x/gamm/pool-models/balancer/amm.go @@ -185,9 +185,9 @@ func (p Pool) SpotPrice(ctx sdk.Context, baseAsset, quoteAsset string) (sdk.Dec, return ratio, nil } -// balancer notation: pAo - poolshares amount out, given single asset in +// balancer notation: pAo - pool shares amount out, given single asset in // the second argument requires the tokenWeightIn / total token weight. -func calcPoolOutGivenSingleIn( +func calcPoolSharesOutGivenSingleAssetIn( tokenBalanceIn, normalizedTokenWeightIn, poolShares, @@ -226,7 +226,7 @@ func (p *Pool) calcSingleAssetJoin(tokenIn sdk.Coin, swapFee sdk.Dec, tokenInPoo return sdk.ZeroInt(), errors.New("pool misconfigured, total weight = 0") } normalizedWeight := tokenInPoolAsset.Weight.ToDec().Quo(totalWeight.ToDec()) - return calcPoolOutGivenSingleIn( + return calcPoolSharesOutGivenSingleAssetIn( tokenInPoolAsset.Token.Amount.ToDec(), normalizedWeight, totalShares.ToDec(), @@ -330,22 +330,31 @@ func (p *Pool) CalcJoinPoolShares(_ctx sdk.Context, tokensIn sdk.Coins, swapFee return numShares, newLiquidity, nil } -func (p *Pool) ExitPool(ctx sdk.Context, exitingShares sdk.Int, exitFee sdk.Dec) (exitedCoins sdk.Coins, err error) { - exitedCoins, err = p.CalcExitPoolShares(ctx, exitingShares, exitFee) +func (p *Pool) ExitPool(ctx sdk.Context, exitingShares sdk.Int, exitFee sdk.Dec) (exitingCoins sdk.Coins, err error) { + exitingCoins, err = p.CalcExitPoolShares(ctx, exitingShares, exitFee) if err != nil { return sdk.Coins{}, err } - balances := p.GetTotalPoolLiquidity(ctx).Sub(exitedCoins) - err = p.UpdatePoolAssetBalances(balances) - if err != nil { + if err := p.exitPool(ctx, exitingCoins, exitingShares); err != nil { return sdk.Coins{}, err } + return exitingCoins, nil +} + +// exitPool exits the pool given exitingCoins and exitingShares. +// updates the pool's liquidity and totalShares. +func (p *Pool) exitPool(ctx sdk.Context, exitingCoins sdk.Coins, exitingShares sdk.Int) error { + balances := p.GetTotalPoolLiquidity(ctx).Sub(exitingCoins) + if err := p.UpdatePoolAssetBalances(balances); err != nil { + return err + } + totalShares := p.GetTotalShares() p.TotalShares = sdk.NewCoin(p.TotalShares.Denom, totalShares.Sub(exitingShares)) - return exitedCoins, nil + return nil } func (p *Pool) CalcExitPoolShares(ctx sdk.Context, exitingShares sdk.Int, exitFee sdk.Dec) (exitedCoins sdk.Coins, err error) { @@ -375,3 +384,64 @@ func (p *Pool) CalcExitPoolShares(ctx sdk.Context, exitingShares sdk.Int, exitFe } return exitedCoins, nil } + +// balancer notation: pAi - pool shares amount in, given single asset out. +// the returned shares in have the fee included in them. +// the second argument requires the tokenWeightOut / total token weight. +func calcPoolSharesInGivenSingleAssetOut( + tokenBalanceOut, + normalizedTokenWeightOut, + poolSupply, + tokenAmountOut, + swapFee, + exitFee sdk.Dec, +) sdk.Dec { + // feeRatio is defined as follows: + // 1 - ((1 - normalizedTokenWeightOut) * swapFee) + feeRatio := sdk.OneDec().Sub((sdk.OneDec().Sub(normalizedTokenWeightOut)).Mul(swapFee)) + + tokenAmountOutBeforeFee := tokenAmountOut.Quo(feeRatio) + + // delta poolSupply is positive(total pool shares decreases) + // pool weight is always 1 + sharesIn := solveConstantFunctionInvariant(tokenBalanceOut.Sub(tokenAmountOutBeforeFee), tokenBalanceOut, normalizedTokenWeightOut, poolSupply, sdk.OneDec()) + + // charge exit fee on the pool token side + // pAi = pAiAfterExitFee/(1-exitFee) + sharesInFeeIncluded := sharesIn.Quo(sdk.OneDec().Sub(exitFee)) + return sharesInFeeIncluded +} + +func (p *Pool) ExitSwapExactAmountOut( + ctx sdk.Context, + tokenOut sdk.Coin, + shareInMaxAmount sdk.Int, +) (shareInAmount sdk.Int, err error) { + _, pAsset, err := p.getPoolAssetAndIndex(tokenOut.Denom) + if err != nil { + return sdk.Int{}, err + } + + sharesIn := calcPoolSharesInGivenSingleAssetOut( + pAsset.Token.Amount.ToDec(), + pAsset.Weight.ToDec().Quo(p.TotalWeight.ToDec()), + p.GetTotalShares().ToDec(), + tokenOut.Amount.ToDec(), + p.GetSwapFee(ctx), + p.GetExitFee(ctx), + ).TruncateInt() + + if sharesIn.LTE(sdk.ZeroInt()) { + return sdk.Int{}, sdkerrors.Wrapf(types.ErrInvalidMathApprox, "token amount is zero or negative") + } + + if sharesIn.GT(shareInMaxAmount) { + return sdk.Int{}, sdkerrors.Wrapf(types.ErrLimitMaxAmount, "%s token is larger than max amount", pAsset.Token.Denom) + } + + if err := p.exitPool(ctx, sdk.NewCoins(tokenOut), sharesIn); err != nil { + return sdk.Int{}, err + } + + return sharesIn, nil +} diff --git a/x/gamm/pool-models/balancer/balancer_pool.go b/x/gamm/pool-models/balancer/balancer_pool.go index 9fc62c844fd..82932209840 100644 --- a/x/gamm/pool-models/balancer/balancer_pool.go +++ b/x/gamm/pool-models/balancer/balancer_pool.go @@ -13,7 +13,10 @@ import ( "github.com/osmosis-labs/osmosis/v7/x/gamm/types" ) -var _ types.PoolI = &Pool{} +var ( + _ types.PoolI = &Pool{} + _ types.PoolExitSwapExactAmountOutExtension = &Pool{} +) // NewPool returns a weighted CPMM pool with the provided parameters, and initial assets. // Invariants that are assumed to be satisfied and not checked: diff --git a/x/gamm/types/pool.go b/x/gamm/types/pool.go index 38d8bfa31a6..2a7b8dc0c5b 100644 --- a/x/gamm/types/pool.go +++ b/x/gamm/types/pool.go @@ -75,6 +75,22 @@ type PoolI interface { PokePool(blockTime time.Time) } +// PoolExitSwapExactAmountOutExtension is an extension of the PoolI +// interface definiting an abstraction for pools that hold tokens. +// In addition, it supports ExitSwapExactAmountOut method. +// See definition below. +type PoolExitSwapExactAmountOutExtension interface { + PoolI + + // ExitSwapExactAmountOut removes liquidity from a specified pool with a maximum amount of LP shares (shareInMaxAmount) + // and swaps to an exact amount of one of the token pairs (tokenOut) + ExitSwapExactAmountOut( + ctx sdk.Context, + tokenOut sdk.Coin, + shareInMaxAmount sdk.Int, + ) (shareInAmount sdk.Int, err error) +} + func NewPoolAddress(poolId uint64) sdk.AccAddress { key := append([]byte("pool"), sdk.Uint64ToBigEndian(poolId)...) return address.Module(ModuleName, key)