Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add boilerplate logic for stableswap swaps and spot price #1287

Merged
merged 6 commits into from
Apr 25, 2022
+98 −13
Merged

Add boilerplate logic for stableswap swaps and spot price #1287

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dd05943
Add boilerplate logic for stableswap swaps and spot price
ValarDragon Apr 18, 2022
60f4c35
applySwap -> updatePoolLiquidityForSwap
ValarDragon Apr 19, 2022
0b2da2f
l -> numTokens
ValarDragon Apr 20, 2022
c613b44
Fix bug matt pointed out
ValarDragon Apr 25, 2022
3456a19
Revert rounding update
ValarDragon Apr 25, 2022
56b87cc
Merge branch 'main' into dev/stableswap_interface
ValarDragon Apr 25, 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
6 changes: 5 additions & 1 deletion x/gamm/pool-models/balancer/amm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,11 @@ func (p *Pool) SwapInAmtGivenOut(
if err != nil {
return sdk.Coin{}, sdkerrors.Wrapf(types.ErrInvalidMathApprox, "token amount is zero or negative")
}
tokenInCoin, _ := tokenInDecCoin.TruncateDecimal()
tokenInCoin, tokenInDecimal := tokenInDecCoin.TruncateDecimal()
// if tokenInDecimal is non-zero, we add 1 to the tokenInCoin
if tokenInDecimal.Amount.IsPositive() {
tokenInCoin.Amount.AddRaw(1)
Copy link
Member

Choose a reason for hiding this comment

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

don't we have to do tokenInCoin = tokenInCoin.Amount.AddRaw(1) ?

Copy link
Member

Choose a reason for hiding this comment

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

Also, I'm curious why we add 1 in the case of non-zeros!

Copy link
Member Author

@ValarDragon ValarDragon Apr 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

ah yeah your right, good catch.

The idea here is that I want to get exactly 100 tokens out after the trade. But to do that, I have to provide say 50.82 tokens as input. Tokens are stored as integers, so before we were only requiring 50 tokens to be sent. But actually it should have been 51 when its converted to integers.

We should always round-up here for correctness!

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm going to revert this change for Balancer, and fix it in a second PR that fixes CalcSpotPrice to be an integer API

}
if !tokenInCoin.Amount.IsPositive() {
return sdk.Coin{}, sdkerrors.Wrapf(types.ErrInvalidMathApprox, "token amount must be positive")
}
Expand Down
28 changes: 28 additions & 0 deletions x/gamm/pool-models/stableswap/amm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,3 +188,31 @@ func spotPrice(baseReserve, quoteReserve sdk.Dec) sdk.Dec {
// no need to divide by a, since a = 1.
return solveCfmm(baseReserve, quoteReserve, a)
}

// returns outAmt as a decimal
func (pa *Pool) calcOutAmtGivenIn(tokenIn sdk.Coin, tokenOutDenom string, swapFee sdk.Dec) (sdk.Dec, error) {
reserves, err := pa.getPoolAmts(tokenIn.Denom, tokenOutDenom)
if err != nil {
return sdk.Dec{}, err
}
tokenInSupply := reserves[0].ToDec()
tokenOutSupply := reserves[1].ToDec()
// We are solving for the amount of token out, hence x = tokenOutSupply, y = tokenInSupply
outAmt := solveCfmm(tokenOutSupply, tokenInSupply, tokenIn.Amount.ToDec())
return outAmt, nil
}

// returns inAmt as a decimal
func (pa *Pool) calcInAmtGivenOut(tokenOut sdk.Coin, tokenInDenom string, swapFee sdk.Dec) (sdk.Dec, error) {
reserves, err := pa.getPoolAmts(tokenInDenom, tokenOut.Denom)
if err != nil {
return sdk.Dec{}, err
}
tokenInSupply := reserves[0].ToDec()
tokenOutSupply := reserves[1].ToDec()
// We are solving for the amount of token in, cfmm(x,y) = cfmm(x + x_in, y - y_out)
// x = tokenInSupply, y = tokenOutSupply, yIn = -tokenOutAmount
inAmtRaw := solveCfmm(tokenInSupply, tokenOutSupply, tokenOut.Amount.ToDec().Neg())
Comment on lines +213 to +215
Copy link
Member

Choose a reason for hiding this comment

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

Could you explain why we use a negative value for y in?

inAmt := inAmtRaw.NegMut()
return inAmt, nil
}
2 changes: 0 additions & 2 deletions x/gamm/pool-models/stableswap/amm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ func TestCFMMInvariant(t *testing.T) {
sdk.NewDec(100),
sdk.NewDec(100),
sdk.NewDec(1000),
// returns 87.445364416281417284
// should return 99.84973704262359
},
// {
// sdk.NewDec(100000),
Expand Down
72 changes: 62 additions & 10 deletions x/gamm/pool-models/stableswap/pool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

"github.com/osmosis-labs/osmosis/v7/x/gamm/types"
)
Expand Down Expand Up @@ -68,34 +69,85 @@ func (pa Pool) getPoolAmts(denoms ...string) ([]sdk.Int, error) {
return result, nil
}

// These should all get moved to amm.go
// applySwap updates the pool liquidity.
// It requires caller to validate that tokensIn and tokensOut only consist of
// denominations in the pool.
// The function sanity checks this, and panics if not the case.
func (p *Pool) applySwap(tokensIn sdk.Coins, tokensOut sdk.Coins) {
l := p.PoolLiquidity.Len()
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved
// update liquidity
p.PoolLiquidity = p.PoolLiquidity.Add(tokensIn...).Sub(tokensOut)
// sanity check that no new denoms were added
if len(p.PoolLiquidity) != l {
panic("applySwap changed number of tokens in pool")
Copy link
Member

Choose a reason for hiding this comment

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

q: At first, I thought that we can propagate the error up top. However, it seems that the panic here is for a reason. We don't want to let the caller handle the error if the number of tokens happens to change. Is my reasoning correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, this should never happen. (It should have been caught by the methods that call it in this package, and not be able to be triggered from functions outside the package)

}
}

// TODO: These should all get moved to amm.go
func (pa Pool) CalcOutAmtGivenIn(ctx sdk.Context, tokenIn sdk.Coins, tokenOutDenom string, swapFee sdk.Dec) (tokenOut sdk.DecCoin, err error) {
if tokenIn.Len() != 1 {
return sdk.DecCoin{}, errors.New("asdf")
return sdk.DecCoin{}, errors.New("stableswap CalcOutAmtGivenIn: tokenIn is of wrong length")
Copy link
Member

Choose a reason for hiding this comment

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

does it make sense to format the length value in the message?

}
reserves, err := pa.getPoolAmts(tokenIn[0].Denom, tokenOutDenom)
amt, err := pa.calcOutAmtGivenIn(tokenIn[0], tokenOutDenom, swapFee)
if err != nil {
return sdk.DecCoin{}, err
}
// document which is x vs y
outAmt := solveCfmm(reserves[1].ToDec(), reserves[0].ToDec(), tokenIn[0].Amount.ToDec())
return sdk.DecCoin{Denom: tokenOutDenom, Amount: outAmt}, nil
return sdk.DecCoin{Denom: tokenOutDenom, Amount: amt}, nil
}

func (pa *Pool) SwapOutAmtGivenIn(ctx sdk.Context, tokenIn sdk.Coins, tokenOutDenom string, swapFee sdk.Dec) (tokenOut sdk.Coin, err error) {
return sdk.Coin{}, types.ErrNotImplemented
tokenOutDec, err := pa.CalcOutAmtGivenIn(ctx, tokenIn, tokenOutDenom, swapFee)
if err != nil {
return sdk.Coin{}, err
}
// we ignore the decimal component, as token out amount must round down
tokenOut, _ = tokenOutDec.TruncateDecimal()
if !tokenOut.Amount.IsPositive() {
return sdk.Coin{}, sdkerrors.Wrapf(types.ErrInvalidMathApprox, "token amount must be positive")
Copy link
Member

Choose a reason for hiding this comment

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

does it make sense to format what it actually was in the message?

}
pa.applySwap(tokenIn, sdk.NewCoins(tokenOut))

return tokenOut, nil
}

func (pa Pool) CalcInAmtGivenOut(ctx sdk.Context, tokenOut sdk.Coins, tokenInDenom string, swapFee sdk.Dec) (tokenIn sdk.DecCoin, err error) {
return sdk.DecCoin{}, types.ErrNotImplemented
if tokenOut.Len() != 1 {
return sdk.DecCoin{}, errors.New("stableswap CalcInAmtGivenOut: tokenOut is of wrong length")
Copy link
Member

Choose a reason for hiding this comment

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

nit: format the length if makes sense

}
// TODO: Refactor this later to handle scaling factors
amt, err := pa.calcInAmtGivenOut(tokenOut[0], tokenInDenom, swapFee)
if err != nil {
return sdk.DecCoin{}, err
}
return sdk.DecCoin{Denom: tokenInDenom, Amount: amt}, nil
}

func (pa *Pool) SwapInAmtGivenOut(ctx sdk.Context, tokenOut sdk.Coins, tokenInDenom string, swapFee sdk.Dec) (tokenIn sdk.Coin, err error) {
return sdk.Coin{}, types.ErrNotImplemented
tokenInDec, err := pa.CalcInAmtGivenOut(ctx, tokenOut, tokenInDenom, swapFee)
if err != nil {
return sdk.Coin{}, err
}
tokenIn, tokenInDecimal := tokenInDec.TruncateDecimal()
// if tokenInDecimal is non-zero, we add 1 to the tokenInCoin
// this is because tokenIn must round up
if tokenInDecimal.Amount.IsPositive() {
tokenIn.Amount.AddRaw(1)
}
if !tokenIn.Amount.IsPositive() {
return sdk.Coin{}, sdkerrors.Wrapf(types.ErrInvalidMathApprox, "token amount must be positive")
}
pa.applySwap(sdk.NewCoins(tokenIn), tokenOut)

return tokenIn, nil
}

func (pa Pool) SpotPrice(ctx sdk.Context, baseAssetDenom string, quoteAssetDenom string) (sdk.Dec, error) {
return sdk.Dec{}, types.ErrNotImplemented
reserves, err := pa.getPoolAmts(baseAssetDenom, quoteAssetDenom)
if err != nil {
return sdk.Dec{}, err
}
// TODO: apply scaling factors here
return spotPrice(reserves[0].ToDec(), reserves[1].ToDec()), nil
}

func (pa Pool) CalcJoinPoolShares(ctx sdk.Context, tokensIn sdk.Coins, swapFee sdk.Dec) (numShares sdk.Int, newLiquidity sdk.Coins, err error) {
Expand Down