From 3a505afea27722d4fc6e1e166d47cb35f248a01c Mon Sep 17 00:00:00 2001 From: Roman Date: Tue, 12 Apr 2022 20:16:05 -0400 Subject: [PATCH 01/27] refactor: ExitSwapExternAmountOut --- x/gamm/client/cli/cli_test.go | 81 +++++++++++++++--------------- x/gamm/keeper/pool_service.go | 25 +++++---- x/gamm/keeper/pool_service_test.go | 4 +- x/gamm/pool-models/balancer/amm.go | 59 ++++++++++++++++++++++ x/gamm/types/pool.go | 9 ++++ 5 files changed, 126 insertions(+), 52 deletions(-) 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/pool_service.go b/x/gamm/keeper/pool_service.go index aafea76d787..121ea8ea17d 100644 --- a/x/gamm/keeper/pool_service.go +++ b/x/gamm/keeper/pool_service.go @@ -402,13 +402,20 @@ func (k Keeper) ExitSwapExternAmountOut( 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.PoolExternExitSwapExternAmountOutExtension) + if !ok { + return sdk.Int{}, fmt.Errorf("pool with id %d does not support this kind of exit", poolId) + } + + shareInAmount, err = extendedPool.ExitSwapExternAmountOut(ctx, tokenOut, shareInMaxAmount) + + 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..4e2e8529d85 100644 --- a/x/gamm/keeper/pool_service_test.go +++ b/x/gamm/keeper/pool_service_test.go @@ -482,8 +482,8 @@ 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.ExitSwapExternAmountOut(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)) diff --git a/x/gamm/pool-models/balancer/amm.go b/x/gamm/pool-models/balancer/amm.go index f93149fae5f..22068d42a40 100644 --- a/x/gamm/pool-models/balancer/amm.go +++ b/x/gamm/pool-models/balancer/amm.go @@ -375,3 +375,62 @@ func (p *Pool) CalcExitPoolShares(ctx sdk.Context, exitingShares sdk.Int, exitFe } return exitedCoins, nil } + +// balancer notation: pAi - poolshares amount in, given single out +// the second argument requires the tokenWeightOut / total token weight. +func calcPoolInGivenSingleOut( + tokenBalanceOut, + normalizedTokenWeightOut, + poolSupply, + tokenAmountOut, + swapFee sdk.Dec, + exitFee sdk.Dec, +) sdk.Dec { + 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 + poolAmountIn := solveConstantFunctionInvariant(tokenBalanceOut.Sub(tokenAmountOutBeforeFee), tokenBalanceOut, normalizedTokenWeightOut, poolSupply, sdk.OneDec()) + + // charge exit fee on the pool token side + // pAi = pAiAfterExitFee/(1-exitFee) + poolAmountInBeforeFee := poolAmountIn.Quo(sdk.OneDec().Sub(exitFee)) + return poolAmountInBeforeFee +} + +func (p *Pool) ExitSwapExternAmountOut( + 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 + } + + poolAmountInBeforeFee := calcPoolInGivenSingleOut( + pAsset.Token.Amount.ToDec(), + pAsset.Weight.ToDec().Quo(p.TotalWeight.ToDec()), + p.GetTotalShares().ToDec(), + tokenOut.Amount.ToDec(), + p.GetSwapFee(ctx), + p.GetExitFee(ctx)) + + if poolAmountInBeforeFee.LTE(sdk.ZeroInt().ToDec()) { + return sdk.Int{}, sdkerrors.Wrapf(types.ErrInvalidMathApprox, "token amount is zero or negative") + } + + if poolAmountInBeforeFee.GT(shareInMaxAmount.ToDec()) { + return sdk.Int{}, sdkerrors.Wrapf(types.ErrLimitMaxAmount, "%s token is larger than max amount", pAsset.Token.Denom) + } + + pAsset.Token.Amount = pAsset.Token.Amount.Sub(tokenOut.Amount) + err = p.UpdatePoolAssetBalance(pAsset.Token) + if err != nil { + return sdk.Int{}, err + } + + return poolAmountInBeforeFee.TruncateInt(), nil +} diff --git a/x/gamm/types/pool.go b/x/gamm/types/pool.go index 38d8bfa31a6..f7b62ba084e 100644 --- a/x/gamm/types/pool.go +++ b/x/gamm/types/pool.go @@ -75,6 +75,15 @@ type PoolI interface { PokePool(blockTime time.Time) } +type PoolExternExitSwapExternAmountOutExtension interface { + PoolI + ExitSwapExternAmountOut( + 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) From 118bd855c9a0c8686adcd8a2e2971100764dd264 Mon Sep 17 00:00:00 2001 From: Roman Date: Tue, 12 Apr 2022 20:20:17 -0400 Subject: [PATCH 02/27] better name for interface extension --- x/gamm/keeper/pool_service.go | 2 +- x/gamm/types/pool.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x/gamm/keeper/pool_service.go b/x/gamm/keeper/pool_service.go index 121ea8ea17d..4711bfbc582 100644 --- a/x/gamm/keeper/pool_service.go +++ b/x/gamm/keeper/pool_service.go @@ -407,7 +407,7 @@ func (k Keeper) ExitSwapExternAmountOut( return sdk.Int{}, err } - extendedPool, ok := pool.(types.PoolExternExitSwapExternAmountOutExtension) + extendedPool, ok := pool.(types.PoolExitSwapExternAmountOutExtension) if !ok { return sdk.Int{}, fmt.Errorf("pool with id %d does not support this kind of exit", poolId) } diff --git a/x/gamm/types/pool.go b/x/gamm/types/pool.go index f7b62ba084e..0968ee5297d 100644 --- a/x/gamm/types/pool.go +++ b/x/gamm/types/pool.go @@ -75,7 +75,7 @@ type PoolI interface { PokePool(blockTime time.Time) } -type PoolExternExitSwapExternAmountOutExtension interface { +type PoolExitSwapExternAmountOutExtension interface { PoolI ExitSwapExternAmountOut( ctx sdk.Context, From cedc684f6d2f6e5e6d4e347994a9414d6f400570 Mon Sep 17 00:00:00 2001 From: Roman Date: Tue, 12 Apr 2022 20:30:02 -0400 Subject: [PATCH 03/27] implement types.PoolExitSwapExternAmountOutExtension --- x/gamm/pool-models/balancer/balancer_pool.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/gamm/pool-models/balancer/balancer_pool.go b/x/gamm/pool-models/balancer/balancer_pool.go index 9fc62c844fd..98bb0d964b8 100644 --- a/x/gamm/pool-models/balancer/balancer_pool.go +++ b/x/gamm/pool-models/balancer/balancer_pool.go @@ -13,7 +13,7 @@ import ( "github.com/osmosis-labs/osmosis/v7/x/gamm/types" ) -var _ types.PoolI = &Pool{} +var _ types.PoolExitSwapExternAmountOutExtension = &Pool{} // NewPool returns a weighted CPMM pool with the provided parameters, and initial assets. // Invariants that are assumed to be satisfied and not checked: From b9af906b00bbb38b8f5ebb26e9e8100e1dd06167 Mon Sep 17 00:00:00 2001 From: Roman Date: Tue, 12 Apr 2022 20:31:27 -0400 Subject: [PATCH 04/27] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 030d4e11fca..183a88be8ca 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: 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 From 3cee732db360d2890245c95338c53f4cab538a4d Mon Sep 17 00:00:00 2001 From: Roman Date: Thu, 14 Apr 2022 10:24:44 -0400 Subject: [PATCH 05/27] interface assertion for PoolI in balancer pool --- x/gamm/pool-models/balancer/balancer_pool.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/gamm/pool-models/balancer/balancer_pool.go b/x/gamm/pool-models/balancer/balancer_pool.go index 98bb0d964b8..ea4f13014d6 100644 --- a/x/gamm/pool-models/balancer/balancer_pool.go +++ b/x/gamm/pool-models/balancer/balancer_pool.go @@ -13,6 +13,7 @@ import ( "github.com/osmosis-labs/osmosis/v7/x/gamm/types" ) +var _ types.PoolI = &Pool{} var _ types.PoolExitSwapExternAmountOutExtension = &Pool{} // NewPool returns a weighted CPMM pool with the provided parameters, and initial assets. From 0426eec3e4f05c19b39a4976b3930b8f47475d77 Mon Sep 17 00:00:00 2001 From: Roman Date: Thu, 14 Apr 2022 10:25:23 -0400 Subject: [PATCH 06/27] err check on extendedPool.ExitSwapExternAmountOut --- x/gamm/keeper/pool_service.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/x/gamm/keeper/pool_service.go b/x/gamm/keeper/pool_service.go index 4711bfbc582..c6e27b6a3af 100644 --- a/x/gamm/keeper/pool_service.go +++ b/x/gamm/keeper/pool_service.go @@ -413,6 +413,9 @@ func (k Keeper) ExitSwapExternAmountOut( } shareInAmount, err = extendedPool.ExitSwapExternAmountOut(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 From 5f3e4b992df1e4f071752e6b5287a0d9ea5fa34a Mon Sep 17 00:00:00 2001 From: Roman Date: Thu, 14 Apr 2022 10:26:00 -0400 Subject: [PATCH 07/27] ZeroDec() --- x/gamm/pool-models/balancer/amm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/gamm/pool-models/balancer/amm.go b/x/gamm/pool-models/balancer/amm.go index 22068d42a40..30f7003bb0b 100644 --- a/x/gamm/pool-models/balancer/amm.go +++ b/x/gamm/pool-models/balancer/amm.go @@ -418,7 +418,7 @@ func (p *Pool) ExitSwapExternAmountOut( p.GetSwapFee(ctx), p.GetExitFee(ctx)) - if poolAmountInBeforeFee.LTE(sdk.ZeroInt().ToDec()) { + if poolAmountInBeforeFee.LTE(sdk.ZeroDec()) { return sdk.Int{}, sdkerrors.Wrapf(types.ErrInvalidMathApprox, "token amount is zero or negative") } From a8fd3eb38c9b77a51fb207499fcc0d8c626eb62d Mon Sep 17 00:00:00 2001 From: Roman Date: Thu, 14 Apr 2022 10:27:08 -0400 Subject: [PATCH 08/27] comma in calcPoolInGivenSingleOut call --- x/gamm/pool-models/balancer/amm.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/gamm/pool-models/balancer/amm.go b/x/gamm/pool-models/balancer/amm.go index 30f7003bb0b..10bd710cd06 100644 --- a/x/gamm/pool-models/balancer/amm.go +++ b/x/gamm/pool-models/balancer/amm.go @@ -416,7 +416,8 @@ func (p *Pool) ExitSwapExternAmountOut( p.GetTotalShares().ToDec(), tokenOut.Amount.ToDec(), p.GetSwapFee(ctx), - p.GetExitFee(ctx)) + p.GetExitFee(ctx), + ) if poolAmountInBeforeFee.LTE(sdk.ZeroDec()) { return sdk.Int{}, sdkerrors.Wrapf(types.ErrInvalidMathApprox, "token amount is zero or negative") From c7f3728b5c5b43cbb4ff778348de1ccdd6b87456 Mon Sep 17 00:00:00 2001 From: Roman Date: Thu, 14 Apr 2022 10:28:13 -0400 Subject: [PATCH 09/27] redundnet type in calcPoolInGivenSingleOut --- x/gamm/pool-models/balancer/amm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/gamm/pool-models/balancer/amm.go b/x/gamm/pool-models/balancer/amm.go index 10bd710cd06..f6cedfb89fd 100644 --- a/x/gamm/pool-models/balancer/amm.go +++ b/x/gamm/pool-models/balancer/amm.go @@ -383,7 +383,7 @@ func calcPoolInGivenSingleOut( normalizedTokenWeightOut, poolSupply, tokenAmountOut, - swapFee sdk.Dec, + swapFee, exitFee sdk.Dec, ) sdk.Dec { feeRatio := sdk.OneDec().Sub((sdk.OneDec().Sub(normalizedTokenWeightOut)).Mul(swapFee)) From a3c2e580fe851f46966165cf27fe08389dd229d3 Mon Sep 17 00:00:00 2001 From: Roman Date: Thu, 14 Apr 2022 10:28:51 -0400 Subject: [PATCH 10/27] comment for feeRatio --- x/gamm/pool-models/balancer/amm.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x/gamm/pool-models/balancer/amm.go b/x/gamm/pool-models/balancer/amm.go index f6cedfb89fd..103be608e67 100644 --- a/x/gamm/pool-models/balancer/amm.go +++ b/x/gamm/pool-models/balancer/amm.go @@ -386,6 +386,8 @@ func calcPoolInGivenSingleOut( 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) From 10d2e1fef476b1713f4ae8b264e298dfc077d236 Mon Sep 17 00:00:00 2001 From: Roman Date: Thu, 14 Apr 2022 10:33:49 -0400 Subject: [PATCH 11/27] fix comment for calcPoolInGivenSingleOut and the converse --- x/gamm/pool-models/balancer/amm.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/gamm/pool-models/balancer/amm.go b/x/gamm/pool-models/balancer/amm.go index 103be608e67..45e6550f944 100644 --- a/x/gamm/pool-models/balancer/amm.go +++ b/x/gamm/pool-models/balancer/amm.go @@ -185,7 +185,7 @@ 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( tokenBalanceIn, @@ -376,7 +376,7 @@ func (p *Pool) CalcExitPoolShares(ctx sdk.Context, exitingShares sdk.Int, exitFe return exitedCoins, nil } -// balancer notation: pAi - poolshares amount in, given single out +// balancer notation: pAi - pool shares amount in, given single asset out // the second argument requires the tokenWeightOut / total token weight. func calcPoolInGivenSingleOut( tokenBalanceOut, From b5660036726cecc0c15ad21cea78c58f85b111cb Mon Sep 17 00:00:00 2001 From: Roman Date: Thu, 14 Apr 2022 10:34:51 -0400 Subject: [PATCH 12/27] whitespace in ExitSwapExternAmountOut --- x/gamm/keeper/pool_service.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/gamm/keeper/pool_service.go b/x/gamm/keeper/pool_service.go index c6e27b6a3af..99f8b84faeb 100644 --- a/x/gamm/keeper/pool_service.go +++ b/x/gamm/keeper/pool_service.go @@ -420,5 +420,6 @@ func (k Keeper) ExitSwapExternAmountOut( if err := k.applyExitPoolStateChange(ctx, pool, sender, shareInAmount, sdk.Coins{tokenOut}); err != nil { return sdk.Int{}, err } + return shareInAmount, nil } From 955831d79a525a964c7c808205be405947f1f678 Mon Sep 17 00:00:00 2001 From: Roman Date: Thu, 14 Apr 2022 10:35:32 -0400 Subject: [PATCH 13/27] fix changelog entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 183a88be8ca..79275becce3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,7 +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: ExitSwapExternAmountOut +* [#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 From f2c428a6a656ab382fb845c9337fd7a94bc55aaf Mon Sep 17 00:00:00 2001 From: Roman Date: Thu, 14 Apr 2022 10:39:21 -0400 Subject: [PATCH 14/27] godoc for PoolExitSwapExternAmountOutExtension --- x/gamm/types/pool.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/x/gamm/types/pool.go b/x/gamm/types/pool.go index 0968ee5297d..380a3a85b28 100644 --- a/x/gamm/types/pool.go +++ b/x/gamm/types/pool.go @@ -75,8 +75,15 @@ type PoolI interface { PokePool(blockTime time.Time) } +// PoolExitSwapExternAmountOutExtension is an extension of the PoolI +// interface definiting an abstraction for pools that hold tokens. +// In addition, it supports ExitSwapExternAmountOut method. +// See definition below. type PoolExitSwapExternAmountOutExtension interface { PoolI + + // ExitSwapExternAmountOut 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) ExitSwapExternAmountOut( ctx sdk.Context, tokenOut sdk.Coin, From ebb8b963c401177d28e866183903ceae23a346bb Mon Sep 17 00:00:00 2001 From: Roman Date: Thu, 14 Apr 2022 11:00:18 -0400 Subject: [PATCH 15/27] fmt --- x/gamm/types/pool.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/gamm/types/pool.go b/x/gamm/types/pool.go index 380a3a85b28..d2ff46bea08 100644 --- a/x/gamm/types/pool.go +++ b/x/gamm/types/pool.go @@ -78,11 +78,11 @@ type PoolI interface { // PoolExitSwapExternAmountOutExtension is an extension of the PoolI // interface definiting an abstraction for pools that hold tokens. // In addition, it supports ExitSwapExternAmountOut method. -// See definition below. +// See definition below. type PoolExitSwapExternAmountOutExtension interface { PoolI - // ExitSwapExternAmountOut removes liquidity from a specified pool with a maximum amount of LP shares (shareInMaxAmount) + // ExitSwapExternAmountOut 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) ExitSwapExternAmountOut( ctx sdk.Context, From 135fc28d257b47af6eada9f7bbddbc78473ca23b Mon Sep 17 00:00:00 2001 From: Roman Date: Thu, 14 Apr 2022 11:23:39 -0400 Subject: [PATCH 16/27] gofumpt --- x/gamm/pool-models/balancer/balancer_pool.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/x/gamm/pool-models/balancer/balancer_pool.go b/x/gamm/pool-models/balancer/balancer_pool.go index ea4f13014d6..c822030c5b7 100644 --- a/x/gamm/pool-models/balancer/balancer_pool.go +++ b/x/gamm/pool-models/balancer/balancer_pool.go @@ -13,8 +13,10 @@ import ( "github.com/osmosis-labs/osmosis/v7/x/gamm/types" ) -var _ types.PoolI = &Pool{} -var _ types.PoolExitSwapExternAmountOutExtension = &Pool{} +var ( + _ types.PoolI = &Pool{} + _ types.PoolExitSwapExternAmountOutExtension = &Pool{} +) // NewPool returns a weighted CPMM pool with the provided parameters, and initial assets. // Invariants that are assumed to be satisfied and not checked: From 023928013a8c3b32171f68694013cdab084d2dcc Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 15 Apr 2022 11:55:31 -0400 Subject: [PATCH 17/27] deduct shares on exit, make exit pool logic shared, improve names --- x/gamm/keeper/pool_service_test.go | 4 +-- x/gamm/pool-models/balancer/amm.go | 44 ++++++++++++++++++------------ 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/x/gamm/keeper/pool_service_test.go b/x/gamm/keeper/pool_service_test.go index 4e2e8529d85..e9dbdf7a446 100644 --- a/x/gamm/keeper/pool_service_test.go +++ b/x/gamm/keeper/pool_service_test.go @@ -482,8 +482,8 @@ 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.ExitSwapExternAmountOut(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)) diff --git a/x/gamm/pool-models/balancer/amm.go b/x/gamm/pool-models/balancer/amm.go index 45e6550f944..f86e14d0430 100644 --- a/x/gamm/pool-models/balancer/amm.go +++ b/x/gamm/pool-models/balancer/amm.go @@ -187,7 +187,7 @@ func (p Pool) SpotPrice(ctx sdk.Context, baseAsset, quoteAsset string) (sdk.Dec, // 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(), @@ -336,16 +336,25 @@ func (p *Pool) ExitPool(ctx sdk.Context, exitingShares sdk.Int, exitFee sdk.Dec) return sdk.Coins{}, err } - balances := p.GetTotalPoolLiquidity(ctx).Sub(exitedCoins) - err = p.UpdatePoolAssetBalances(balances) - if err != nil { + if err := p.exitPool(ctx, exitedCoins, exitingShares); err != nil { return sdk.Coins{}, err } + return exitedCoins, nil +} + +// exitPool exits the pool given exitedCoins and exitingShares. +// updates the pool's liquidity and totalShares +func (p *Pool) exitPool(ctx sdk.Context, exitedCoins sdk.Coins, exitingShares sdk.Int) error { + balances := p.GetTotalPoolLiquidity(ctx).Sub(exitedCoins) + 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) { @@ -376,9 +385,10 @@ 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 +// 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 calcPoolInGivenSingleOut( +func calcPoolSharesInGivenSingleAssetOut( tokenBalanceOut, normalizedTokenWeightOut, poolSupply, @@ -394,12 +404,12 @@ func calcPoolInGivenSingleOut( // delta poolSupply is positive(total pool shares decreases) // pool weight is always 1 - poolAmountIn := solveConstantFunctionInvariant(tokenBalanceOut.Sub(tokenAmountOutBeforeFee), tokenBalanceOut, normalizedTokenWeightOut, poolSupply, sdk.OneDec()) + sharesIn := solveConstantFunctionInvariant(tokenBalanceOut.Sub(tokenAmountOutBeforeFee), tokenBalanceOut, normalizedTokenWeightOut, poolSupply, sdk.OneDec()) // charge exit fee on the pool token side // pAi = pAiAfterExitFee/(1-exitFee) - poolAmountInBeforeFee := poolAmountIn.Quo(sdk.OneDec().Sub(exitFee)) - return poolAmountInBeforeFee + sharesInFeeIncluded := sharesIn.Quo(sdk.OneDec().Sub(exitFee)) + return sharesInFeeIncluded } func (p *Pool) ExitSwapExternAmountOut( @@ -412,7 +422,7 @@ func (p *Pool) ExitSwapExternAmountOut( return sdk.Int{}, err } - poolAmountInBeforeFee := calcPoolInGivenSingleOut( + sharesIn := calcPoolSharesInGivenSingleAssetOut( pAsset.Token.Amount.ToDec(), pAsset.Weight.ToDec().Quo(p.TotalWeight.ToDec()), p.GetTotalShares().ToDec(), @@ -421,19 +431,17 @@ func (p *Pool) ExitSwapExternAmountOut( p.GetExitFee(ctx), ) - if poolAmountInBeforeFee.LTE(sdk.ZeroDec()) { + if sharesIn.LTE(sdk.ZeroDec()) { return sdk.Int{}, sdkerrors.Wrapf(types.ErrInvalidMathApprox, "token amount is zero or negative") } - if poolAmountInBeforeFee.GT(shareInMaxAmount.ToDec()) { + if sharesIn.GT(shareInMaxAmount.ToDec()) { return sdk.Int{}, sdkerrors.Wrapf(types.ErrLimitMaxAmount, "%s token is larger than max amount", pAsset.Token.Denom) } - pAsset.Token.Amount = pAsset.Token.Amount.Sub(tokenOut.Amount) - err = p.UpdatePoolAssetBalance(pAsset.Token) - if err != nil { + if err := p.exitPool(ctx, sdk.NewCoins(tokenOut), sdk.Int(sharesIn)); err != nil { return sdk.Int{}, err } - return poolAmountInBeforeFee.TruncateInt(), nil + return sharesIn.TruncateInt(), nil } From e8966c232c50a2eb068409afe6172e643d8ae934 Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 15 Apr 2022 12:37:06 -0400 Subject: [PATCH 18/27] rename to ExitSwapExactAmountOutExtension --- x/gamm/client/cli/cli_test.go | 108 +++++++++---------- x/gamm/keeper/msg_server.go | 2 +- x/gamm/keeper/pool_service.go | 6 +- x/gamm/keeper/pool_service_test.go | 2 +- x/gamm/pool-models/balancer/amm.go | 2 +- x/gamm/pool-models/balancer/balancer_pool.go | 2 +- x/gamm/types/pool.go | 10 +- 7 files changed, 66 insertions(+), 66 deletions(-) diff --git a/x/gamm/client/cli/cli_test.go b/x/gamm/client/cli/cli_test.go index e001dbd5a35..ed28b36706c 100644 --- a/x/gamm/client/cli/cli_test.go +++ b/x/gamm/client/cli/cli_test.go @@ -1069,69 +1069,69 @@ func (s *IntegrationTestSuite) TestGetCmdSpotPrice() { // } // } -func (s IntegrationTestSuite) TestNewSwapExactAmountInCmd() { - val := s.network.Validators[0] +// func (s IntegrationTestSuite) TestNewSwapExactAmountInCmd() { +// val := s.network.Validators[0] - info, _, err := val.ClientCtx.Keyring.NewMnemonic("NewSwapExactAmountIn", - keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1) - s.Require().NoError(err) +// info, _, err := val.ClientCtx.Keyring.NewMnemonic("NewSwapExactAmountIn", +// keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1) +// s.Require().NoError(err) - newAddr := sdk.AccAddress(info.GetPubKey().Address()) +// newAddr := sdk.AccAddress(info.GetPubKey().Address()) - _, err = banktestutil.MsgSendExec( - val.ClientCtx, - val.Address, - newAddr, - sdk.NewCoins(sdk.NewInt64Coin(s.cfg.BondDenom, 20000), sdk.NewInt64Coin("node0token", 20000)), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), - osmoutils.DefaultFeeString(s.cfg), - ) - s.Require().NoError(err) +// _, err = banktestutil.MsgSendExec( +// val.ClientCtx, +// val.Address, +// newAddr, +// sdk.NewCoins(sdk.NewInt64Coin(s.cfg.BondDenom, 20000), sdk.NewInt64Coin("node0token", 20000)), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), +// fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), +// osmoutils.DefaultFeeString(s.cfg), +// ) +// s.Require().NoError(err) - testCases := []struct { - name string - args []string +// testCases := []struct { +// name string +// args []string - expectErr bool - respType proto.Message - expectedCode uint32 - }{ - { - "swap exact amount in", // osmosisd tx gamm swap-exact-amount-in 10stake 3 --swap-route-pool-ids=1 --swap-route-denoms=node0token --from=validator --keyring-backend=test --chain-id=testing --yes - []string{ - "10stake", "3", - fmt.Sprintf("--%s=%d", cli.FlagSwapRoutePoolIds, 1), - fmt.Sprintf("--%s=%s", cli.FlagSwapRouteDenoms, "node0token"), - fmt.Sprintf("--%s=%s", flags.FlagFrom, newAddr), - // 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, - }, - } +// expectErr bool +// respType proto.Message +// expectedCode uint32 +// }{ +// { +// "swap exact amount in", // osmosisd tx gamm swap-exact-amount-in 10stake 3 --swap-route-pool-ids=1 --swap-route-denoms=node0token --from=validator --keyring-backend=test --chain-id=testing --yes +// []string{ +// "10stake", "3", +// fmt.Sprintf("--%s=%d", cli.FlagSwapRoutePoolIds, 1), +// fmt.Sprintf("--%s=%s", cli.FlagSwapRouteDenoms, "node0token"), +// fmt.Sprintf("--%s=%s", flags.FlagFrom, newAddr), +// // 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.NewSwapExactAmountInCmd() - clientCtx := val.ClientCtx +// s.Run(tc.name, func() { +// cmd := cli.NewSwapExactAmountInCmd() +// 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()) +// } +// }) +// } +// } func TestIntegrationTestSuite(t *testing.T) { suite.Run(t, new(IntegrationTestSuite)) 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 99f8b84faeb..cb6e4e20458 100644 --- a/x/gamm/keeper/pool_service.go +++ b/x/gamm/keeper/pool_service.go @@ -395,7 +395,7 @@ func (k Keeper) ExitSwapShareAmountIn( return tokenOutAmount, nil } -func (k Keeper) ExitSwapExternAmountOut( +func (k Keeper) ExitSwapExactAmountOut( ctx sdk.Context, sender sdk.AccAddress, poolId uint64, @@ -407,12 +407,12 @@ func (k Keeper) ExitSwapExternAmountOut( return sdk.Int{}, err } - extendedPool, ok := pool.(types.PoolExitSwapExternAmountOutExtension) + 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.ExitSwapExternAmountOut(ctx, tokenOut, shareInMaxAmount) + shareInAmount, err = extendedPool.ExitSwapExactAmountOut(ctx, tokenOut, shareInMaxAmount) if err != nil { return sdk.Int{}, err } diff --git a/x/gamm/keeper/pool_service_test.go b/x/gamm/keeper/pool_service_test.go index e9dbdf7a446..63b49d28309 100644 --- a/x/gamm/keeper/pool_service_test.go +++ b/x/gamm/keeper/pool_service_test.go @@ -490,7 +490,7 @@ func (suite *KeeperTestSuite) TestActiveBalancerPool() { 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 f86e14d0430..756a744b814 100644 --- a/x/gamm/pool-models/balancer/amm.go +++ b/x/gamm/pool-models/balancer/amm.go @@ -412,7 +412,7 @@ func calcPoolSharesInGivenSingleAssetOut( return sharesInFeeIncluded } -func (p *Pool) ExitSwapExternAmountOut( +func (p *Pool) ExitSwapExactAmountOut( ctx sdk.Context, tokenOut sdk.Coin, shareInMaxAmount sdk.Int, diff --git a/x/gamm/pool-models/balancer/balancer_pool.go b/x/gamm/pool-models/balancer/balancer_pool.go index c822030c5b7..7f2e08e7253 100644 --- a/x/gamm/pool-models/balancer/balancer_pool.go +++ b/x/gamm/pool-models/balancer/balancer_pool.go @@ -15,7 +15,7 @@ import ( var ( _ types.PoolI = &Pool{} - _ types.PoolExitSwapExternAmountOutExtension = &Pool{} + _ types.PoolExitSwapExactAmountOutExtension = &Pool{} ) // NewPool returns a weighted CPMM pool with the provided parameters, and initial assets. diff --git a/x/gamm/types/pool.go b/x/gamm/types/pool.go index d2ff46bea08..2a7b8dc0c5b 100644 --- a/x/gamm/types/pool.go +++ b/x/gamm/types/pool.go @@ -75,16 +75,16 @@ type PoolI interface { PokePool(blockTime time.Time) } -// PoolExitSwapExternAmountOutExtension is an extension of the PoolI +// PoolExitSwapExactAmountOutExtension is an extension of the PoolI // interface definiting an abstraction for pools that hold tokens. -// In addition, it supports ExitSwapExternAmountOut method. +// In addition, it supports ExitSwapExactAmountOut method. // See definition below. -type PoolExitSwapExternAmountOutExtension interface { +type PoolExitSwapExactAmountOutExtension interface { PoolI - // ExitSwapExternAmountOut removes liquidity from a specified pool with a maximum amount of LP shares (shareInMaxAmount) + // 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) - ExitSwapExternAmountOut( + ExitSwapExactAmountOut( ctx sdk.Context, tokenOut sdk.Coin, shareInMaxAmount sdk.Int, From 57d57ed1f27597dfbec6f7f24808693eed0d702d Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 15 Apr 2022 12:41:20 -0400 Subject: [PATCH 19/27] comment out TestNewExitSwapShareAmountInCmd --- x/gamm/client/cli/cli_test.go | 80 +++++++++++++++++------------------ 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/x/gamm/client/cli/cli_test.go b/x/gamm/client/cli/cli_test.go index ed28b36706c..59a1e550d1c 100644 --- a/x/gamm/client/cli/cli_test.go +++ b/x/gamm/client/cli/cli_test.go @@ -717,51 +717,51 @@ func (s IntegrationTestSuite) TestNewExitSwapExternAmountOutCmd() { // } // } -func (s IntegrationTestSuite) TestNewExitSwapShareAmountInCmd() { - val := s.network.Validators[0] +// func (s IntegrationTestSuite) TestNewExitSwapShareAmountInCmd() { +// val := s.network.Validators[0] - testCases := []struct { - name string - args []string - expectErr bool - respType proto.Message - expectedCode uint32 - }{ - { - "exit swap share amount in", // osmosisd tx gamm exit-swap-share-amount-in --pool-id=1 stake 10 1 --from=validator --keyring-backend=test --chain-id=testing --yes - []string{ - "stake", "10000000000000000000", "1", - 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 share amount in", // osmosisd tx gamm exit-swap-share-amount-in --pool-id=1 stake 10 1 --from=validator --keyring-backend=test --chain-id=testing --yes +// []string{ +// "stake", "10000000000000000000", "1", +// 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.NewExitSwapShareAmountIn() - clientCtx := val.ClientCtx +// s.Run(tc.name, func() { +// cmd := cli.NewExitSwapShareAmountIn() +// 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()) +// } +// }) +// } +// } func (s *IntegrationTestSuite) TestGetCmdPools() { val := s.network.Validators[0] From a4ecefbe819a6551b02864a1338cd36a4d31bf1c Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 15 Apr 2022 17:49:33 -0400 Subject: [PATCH 20/27] fix ExitSwapExactAmountOut by truncating int on return from calcPoolSharesInGivenSingleAssetOut --- x/gamm/pool-models/balancer/amm.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/x/gamm/pool-models/balancer/amm.go b/x/gamm/pool-models/balancer/amm.go index 756a744b814..7715d673de2 100644 --- a/x/gamm/pool-models/balancer/amm.go +++ b/x/gamm/pool-models/balancer/amm.go @@ -429,19 +429,19 @@ func (p *Pool) ExitSwapExactAmountOut( tokenOut.Amount.ToDec(), p.GetSwapFee(ctx), p.GetExitFee(ctx), - ) + ).TruncateInt() - if sharesIn.LTE(sdk.ZeroDec()) { + if sharesIn.LTE(sdk.ZeroInt()) { return sdk.Int{}, sdkerrors.Wrapf(types.ErrInvalidMathApprox, "token amount is zero or negative") } - if sharesIn.GT(shareInMaxAmount.ToDec()) { + 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), sdk.Int(sharesIn)); err != nil { + if err := p.exitPool(ctx, sdk.NewCoins(tokenOut), sharesIn); err != nil { return sdk.Int{}, err } - return sharesIn.TruncateInt(), nil + return sharesIn, nil } From 84c257b7aeb15d30e27dea3faf63abe4eb7fbd26 Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 15 Apr 2022 18:14:57 -0400 Subject: [PATCH 21/27] revert x/gamm/cli_test.go to original state with TestNewExitSwapExternAmountOutCmd uncommented --- x/gamm/client/cli/cli_test.go | 188 +++++++++++++++++----------------- 1 file changed, 94 insertions(+), 94 deletions(-) diff --git a/x/gamm/client/cli/cli_test.go b/x/gamm/client/cli/cli_test.go index 59a1e550d1c..e001dbd5a35 100644 --- a/x/gamm/client/cli/cli_test.go +++ b/x/gamm/client/cli/cli_test.go @@ -717,51 +717,51 @@ func (s IntegrationTestSuite) TestNewExitSwapExternAmountOutCmd() { // } // } -// func (s IntegrationTestSuite) TestNewExitSwapShareAmountInCmd() { -// val := s.network.Validators[0] +func (s IntegrationTestSuite) TestNewExitSwapShareAmountInCmd() { + val := s.network.Validators[0] -// testCases := []struct { -// name string -// args []string -// expectErr bool -// respType proto.Message -// expectedCode uint32 -// }{ -// { -// "exit swap share amount in", // osmosisd tx gamm exit-swap-share-amount-in --pool-id=1 stake 10 1 --from=validator --keyring-backend=test --chain-id=testing --yes -// []string{ -// "stake", "10000000000000000000", "1", -// 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 share amount in", // osmosisd tx gamm exit-swap-share-amount-in --pool-id=1 stake 10 1 --from=validator --keyring-backend=test --chain-id=testing --yes + []string{ + "stake", "10000000000000000000", "1", + 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.NewExitSwapShareAmountIn() -// clientCtx := val.ClientCtx + s.Run(tc.name, func() { + cmd := cli.NewExitSwapShareAmountIn() + 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()) + } + }) + } +} func (s *IntegrationTestSuite) TestGetCmdPools() { val := s.network.Validators[0] @@ -1069,69 +1069,69 @@ func (s *IntegrationTestSuite) TestGetCmdSpotPrice() { // } // } -// func (s IntegrationTestSuite) TestNewSwapExactAmountInCmd() { -// val := s.network.Validators[0] +func (s IntegrationTestSuite) TestNewSwapExactAmountInCmd() { + val := s.network.Validators[0] -// info, _, err := val.ClientCtx.Keyring.NewMnemonic("NewSwapExactAmountIn", -// keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1) -// s.Require().NoError(err) + info, _, err := val.ClientCtx.Keyring.NewMnemonic("NewSwapExactAmountIn", + keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1) + s.Require().NoError(err) -// newAddr := sdk.AccAddress(info.GetPubKey().Address()) + newAddr := sdk.AccAddress(info.GetPubKey().Address()) -// _, err = banktestutil.MsgSendExec( -// val.ClientCtx, -// val.Address, -// newAddr, -// sdk.NewCoins(sdk.NewInt64Coin(s.cfg.BondDenom, 20000), sdk.NewInt64Coin("node0token", 20000)), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), -// fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), -// osmoutils.DefaultFeeString(s.cfg), -// ) -// s.Require().NoError(err) + _, err = banktestutil.MsgSendExec( + val.ClientCtx, + val.Address, + newAddr, + sdk.NewCoins(sdk.NewInt64Coin(s.cfg.BondDenom, 20000), sdk.NewInt64Coin("node0token", 20000)), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), + fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), + osmoutils.DefaultFeeString(s.cfg), + ) + s.Require().NoError(err) -// testCases := []struct { -// name string -// args []string + testCases := []struct { + name string + args []string -// expectErr bool -// respType proto.Message -// expectedCode uint32 -// }{ -// { -// "swap exact amount in", // osmosisd tx gamm swap-exact-amount-in 10stake 3 --swap-route-pool-ids=1 --swap-route-denoms=node0token --from=validator --keyring-backend=test --chain-id=testing --yes -// []string{ -// "10stake", "3", -// fmt.Sprintf("--%s=%d", cli.FlagSwapRoutePoolIds, 1), -// fmt.Sprintf("--%s=%s", cli.FlagSwapRouteDenoms, "node0token"), -// fmt.Sprintf("--%s=%s", flags.FlagFrom, newAddr), -// // 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, -// }, -// } + expectErr bool + respType proto.Message + expectedCode uint32 + }{ + { + "swap exact amount in", // osmosisd tx gamm swap-exact-amount-in 10stake 3 --swap-route-pool-ids=1 --swap-route-denoms=node0token --from=validator --keyring-backend=test --chain-id=testing --yes + []string{ + "10stake", "3", + fmt.Sprintf("--%s=%d", cli.FlagSwapRoutePoolIds, 1), + fmt.Sprintf("--%s=%s", cli.FlagSwapRouteDenoms, "node0token"), + fmt.Sprintf("--%s=%s", flags.FlagFrom, newAddr), + // 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.NewSwapExactAmountInCmd() -// clientCtx := val.ClientCtx + s.Run(tc.name, func() { + cmd := cli.NewSwapExactAmountInCmd() + 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()) + } + }) + } +} func TestIntegrationTestSuite(t *testing.T) { suite.Run(t, new(IntegrationTestSuite)) From 191f9be680dafaa3f69f0bd3ab1f101e74891579 Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 15 Apr 2022 18:17:08 -0400 Subject: [PATCH 22/27] uncomment ExitSwapExactAmountOut in TestActiveBalancerPool --- x/gamm/keeper/pool_service_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/gamm/keeper/pool_service_test.go b/x/gamm/keeper/pool_service_test.go index 63b49d28309..4b5af5b799e 100644 --- a/x/gamm/keeper/pool_service_test.go +++ b/x/gamm/keeper/pool_service_test.go @@ -482,8 +482,8 @@ 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)) From 8a46f30ba68d8649975c86fc5374458bace6f65f Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 15 Apr 2022 18:21:13 -0400 Subject: [PATCH 23/27] change variable name to exitingCoins --- x/gamm/pool-models/balancer/amm.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/x/gamm/pool-models/balancer/amm.go b/x/gamm/pool-models/balancer/amm.go index 7715d673de2..cd1b4c3ff06 100644 --- a/x/gamm/pool-models/balancer/amm.go +++ b/x/gamm/pool-models/balancer/amm.go @@ -330,23 +330,23 @@ 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 } - if err := p.exitPool(ctx, exitedCoins, exitingShares); err != nil { + if err := p.exitPool(ctx, exitingCoins, exitingShares); err != nil { return sdk.Coins{}, err } - return exitedCoins, nil + return exitingCoins, nil } -// exitPool exits the pool given exitedCoins and exitingShares. +// exitPool exits the pool given exitingCoins and exitingShares. // updates the pool's liquidity and totalShares -func (p *Pool) exitPool(ctx sdk.Context, exitedCoins sdk.Coins, exitingShares sdk.Int) error { - balances := p.GetTotalPoolLiquidity(ctx).Sub(exitedCoins) +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 } From 37c95cc98597d6e655bf61d36c13a6a5ab7b71d1 Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 15 Apr 2022 18:32:17 -0400 Subject: [PATCH 24/27] fmt --- x/gamm/pool-models/balancer/balancer_pool.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/gamm/pool-models/balancer/balancer_pool.go b/x/gamm/pool-models/balancer/balancer_pool.go index 7f2e08e7253..82932209840 100644 --- a/x/gamm/pool-models/balancer/balancer_pool.go +++ b/x/gamm/pool-models/balancer/balancer_pool.go @@ -14,8 +14,8 @@ import ( ) var ( - _ types.PoolI = &Pool{} - _ types.PoolExitSwapExactAmountOutExtension = &Pool{} + _ types.PoolI = &Pool{} + _ types.PoolExitSwapExactAmountOutExtension = &Pool{} ) // NewPool returns a weighted CPMM pool with the provided parameters, and initial assets. From 795d50cd6f9526c919a1d25ef3eca57ac434c207 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Sat, 16 Apr 2022 07:19:21 -0400 Subject: [PATCH 25/27] Update x/gamm/keeper/pool_service.go --- x/gamm/keeper/pool_service.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/gamm/keeper/pool_service.go b/x/gamm/keeper/pool_service.go index cb6e4e20458..d75c43664c5 100644 --- a/x/gamm/keeper/pool_service.go +++ b/x/gamm/keeper/pool_service.go @@ -421,5 +421,6 @@ func (k Keeper) ExitSwapExactAmountOut( return sdk.Int{}, err } + return shareInAmount, nil } From 798c33d845f64ad4a0cf160ff0d465fe281e9863 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Sat, 16 Apr 2022 07:20:05 -0400 Subject: [PATCH 26/27] Update x/gamm/pool-models/balancer/amm.go --- x/gamm/pool-models/balancer/amm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/gamm/pool-models/balancer/amm.go b/x/gamm/pool-models/balancer/amm.go index cd1b4c3ff06..9e3c33de7ea 100644 --- a/x/gamm/pool-models/balancer/amm.go +++ b/x/gamm/pool-models/balancer/amm.go @@ -344,7 +344,7 @@ func (p *Pool) ExitPool(ctx sdk.Context, exitingShares sdk.Int, exitFee sdk.Dec) } // exitPool exits the pool given exitingCoins and exitingShares. -// updates the pool's liquidity and totalShares +// 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 { From 685f439eb819cea6aa1ef46f439c9e74a5aca783 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Sat, 16 Apr 2022 07:20:11 -0400 Subject: [PATCH 27/27] Update x/gamm/keeper/pool_service.go --- x/gamm/keeper/pool_service.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/gamm/keeper/pool_service.go b/x/gamm/keeper/pool_service.go index d75c43664c5..cb6e4e20458 100644 --- a/x/gamm/keeper/pool_service.go +++ b/x/gamm/keeper/pool_service.go @@ -421,6 +421,5 @@ func (k Keeper) ExitSwapExactAmountOut( return sdk.Int{}, err } - return shareInAmount, nil }