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.

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

refactor: ExitSwapExternAmountOut #1244

Merged
merged 28 commits into from
Apr 18, 2022
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
3a505af
refactor: ExitSwapExternAmountOut
p0mvn Apr 13, 2022
118bd85
better name for interface extension
p0mvn Apr 13, 2022
cedc684
implement types.PoolExitSwapExternAmountOutExtension
p0mvn Apr 13, 2022
b9af906
update changelog
p0mvn Apr 13, 2022
3cee732
interface assertion for PoolI in balancer pool
p0mvn Apr 14, 2022
0426eec
err check on extendedPool.ExitSwapExternAmountOut
p0mvn Apr 14, 2022
5f3e4b9
ZeroDec()
p0mvn Apr 14, 2022
a8fd3eb
comma in calcPoolInGivenSingleOut call
p0mvn Apr 14, 2022
c7f3728
redundnet type in calcPoolInGivenSingleOut
p0mvn Apr 14, 2022
a3c2e58
comment for feeRatio
p0mvn Apr 14, 2022
10d2e1f
fix comment for calcPoolInGivenSingleOut and the converse
p0mvn Apr 14, 2022
b566003
whitespace in ExitSwapExternAmountOut
p0mvn Apr 14, 2022
955831d
fix changelog entry
p0mvn Apr 14, 2022
f2c428a
godoc for PoolExitSwapExternAmountOutExtension
p0mvn Apr 14, 2022
ebb8b96
fmt
p0mvn Apr 14, 2022
135fc28
gofumpt
p0mvn Apr 14, 2022
0239280
deduct shares on exit, make exit pool logic shared, improve names
p0mvn Apr 15, 2022
e8966c2
rename to ExitSwapExactAmountOutExtension
p0mvn Apr 15, 2022
57d57ed
comment out TestNewExitSwapShareAmountInCmd
p0mvn Apr 15, 2022
a4ecefb
fix ExitSwapExactAmountOut by truncating int on return from calcPoolS…
p0mvn Apr 15, 2022
0ea012a
Merge branch 'main' into roman/1130
p0mvn Apr 15, 2022
84c257b
revert x/gamm/cli_test.go to original state with TestNewExitSwapExter…
p0mvn Apr 15, 2022
191f9be
uncomment ExitSwapExactAmountOut in TestActiveBalancerPool
p0mvn Apr 15, 2022
8a46f30
change variable name to exitingCoins
p0mvn Apr 15, 2022
37c95cc
fmt
p0mvn Apr 15, 2022
795d50c
Update x/gamm/keeper/pool_service.go
alexanderbez Apr 16, 2022
798c33d
Update x/gamm/pool-models/balancer/amm.go
alexanderbez Apr 16, 2022
685f439
Update x/gamm/keeper/pool_service.go
alexanderbez Apr 16, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
81 changes: 40 additions & 41 deletions x/gamm/client/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion x/gamm/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
32 changes: 22 additions & 10 deletions x/gamm/keeper/pool_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,20 +395,32 @@ 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
}


alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
return shareInAmount, nil
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
}
6 changes: 3 additions & 3 deletions x/gamm/keeper/pool_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
88 changes: 79 additions & 9 deletions x/gamm/pool-models/balancer/amm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
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) {
Expand Down Expand Up @@ -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))
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved

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
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to update the total shares as well, ala

https://github.com/osmosis-labs/osmosis/pull/1244/files#diff-897e45c36167c79b080379402216acbe7c118711dcaa23ca7852cd2a70aca3c1R346

Maybe we refactor share updating + token balance updating for exits into a single re-used function between the two?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, I extracted this functionality from ExitPool into exitPool

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! That change LGTM

return sharesIn, nil
}
5 changes: 4 additions & 1 deletion x/gamm/pool-models/balancer/balancer_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
16 changes: 16 additions & 0 deletions x/gamm/types/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down