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

x/gamm: Internal audit and documentation #1858

Merged
merged 9 commits into from
Jun 30, 2022
6 changes: 4 additions & 2 deletions x/gamm/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ import (
"github.com/osmosis-labs/osmosis/v7/x/gamm/types"
)

// InitGenesis initializes the capability module's state from a provided genesis
// state.
// InitGenesis initializes the x/gamm module's state from a provided genesis
// state, which includes the current live pools, global pool parameters (e.g. pool creation fee), next pool number etc.
func (k Keeper) InitGenesis(ctx sdk.Context, genState types.GenesisState, unpacker codectypes.AnyUnpacker) {
k.SetParams(ctx, genState.Params)
k.SetNextPoolNumber(ctx, genState.NextPoolNumber)

// Sums up the liquidity in all genesis state pools to find the total liquidity across all pools.
// Also adds each genesis state pool to the x/gamm module's state
liquidity := sdk.Coins{}
for _, any := range genState.Pools {
var pool types.PoolI
Expand Down
22 changes: 21 additions & 1 deletion x/gamm/keeper/multihop.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,20 @@ func (k Keeper) MultihopSwapExactAmountIn(
tokenOutMinAmount sdk.Int,
) (tokenOutAmount sdk.Int, err error) {
for i, route := range routes {
// To prevent the multihop swap from being interrupted prematurely, we keep
// the minimum expected output at a very low number until the last pool
_outMinAmount := sdk.NewInt(1)
if len(routes)-1 == i {
_outMinAmount = tokenOutMinAmount
}

// Execute the expected swap on the current routed pool
tokenOutAmount, err = k.SwapExactAmountIn(ctx, sender, route.PoolId, tokenIn, route.TokenOutDenom, _outMinAmount)
if err != nil {
return sdk.Int{}, err
}

// Chain output of current pool as the input for the next routed pool
tokenIn = sdk.NewCoin(route.TokenOutDenom, tokenOutAmount)
}
return
Expand All @@ -42,24 +47,35 @@ func (k Keeper) MultihopSwapExactAmountOut(
tokenInMaxAmount sdk.Int,
tokenOut sdk.Coin,
) (tokenInAmount sdk.Int, err error) {
// Determine what the estimated input would be for each pool along the multihop route
insExpected, err := k.createMultihopExpectedSwapOuts(ctx, routes, tokenOut)
if err != nil {
return sdk.Int{}, err
}

insExpected[0] = tokenInMaxAmount

// Iterates through each routed pool and executes their respective swaps. Note that all of the work to get the return
// value of this method is done when we calculate insExpected – this for loop primarily serves to execute the actual
// swaps on each pool.
for i, route := range routes {
_tokenOut := tokenOut

// If there is one pool left in the route, set the expected output of the current swap
// to the estimated input of the final pool.
if i != len(routes)-1 {
_tokenOut = sdk.NewCoin(routes[i+1].TokenInDenom, insExpected[i+1])
}

// Execute the expected swap on the current routed pool
_tokenInAmount, err := k.SwapExactAmountOut(ctx, sender, route.PoolId, route.TokenInDenom, insExpected[i], _tokenOut)
if err != nil {
return sdk.Int{}, err
}

// Sets the final amount of tokens that need to be input into the first pool. Even though this is the final return value for the
// whole method and will not change after the first iteration, we still iterate through the rest of the pools to execute their respective
// swaps.
if i == 0 {
tokenInAmount = _tokenInAmount
}
Expand All @@ -68,7 +84,11 @@ func (k Keeper) MultihopSwapExactAmountOut(
return tokenInAmount, nil
}

// TODO: Document this function.
// createMultihopExpectedSwapOuts defines the output denom and output amount for the last pool in
// the route of pools the caller is intending to hop through in a fixed-output multihop tx. It estimates the input
// amount for this last pool and then chains that input as the output of the previous pool in the route, repeating
// until the first pool is reached. It returns an array of inputs, each of which correspond to a pool ID in the
// route of pools for the original multihop transaction.
func (k Keeper) createMultihopExpectedSwapOuts(ctx sdk.Context, routes []types.SwapAmountOutRoute, tokenOut sdk.Coin) ([]sdk.Int, error) {
insExpected := make([]sdk.Int, len(routes))
for i := len(routes) - 1; i >= 0; i-- {
Expand Down
2 changes: 1 addition & 1 deletion x/gamm/keeper/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (k Keeper) GetPoolAndPoke(ctx sdk.Context, poolId uint64) (types.PoolI, err
return pool, nil
}

// Get pool, and check if the pool is active / allowed to be swapped against
// Get pool and check if the pool is active, i.e. allowed to be swapped against.
func (k Keeper) getPoolForSwap(ctx sdk.Context, poolId uint64) (types.PoolI, error) {
pool, err := k.GetPoolAndPoke(ctx, poolId)
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion x/gamm/keeper/pool_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func (k Keeper) JoinPoolNoSwap(
return err
}

// getMaximalNoSwapLPAmount returns the coins(lp liquidity) needed to get the specified amount of share of the pool.
// getMaximalNoSwapLPAmount returns the coins(lp liquidity) needed to get the specified amount of shares in the pool.
// Steps to getting the needed lp liquidity coins needed for the share of the pools are
// 1. calculate how much percent of the pool does given share account for(# of input shares / # of current total shares)
// 2. since we know how much % of the pool we want, iterate through all pool liquidity to calculate how much coins we need for
Expand Down Expand Up @@ -317,6 +317,7 @@ func (k Keeper) JoinSwapShareAmountOut(
}

tokenIn := sdk.NewCoins(sdk.NewCoin(tokenInDenom, tokenInAmount))
// Not using generic JoinPool because we want to guarantee exact shares out
extendedPool.IncreaseLiquidity(shareOutAmount, tokenIn)

err = k.applyJoinPoolStateChange(ctx, pool, sender, shareOutAmount, tokenIn)
Expand Down
4 changes: 4 additions & 0 deletions x/gamm/keeper/swap.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ func (k Keeper) swapExactAmountIn(
}
tokensIn := sdk.Coins{tokenIn}

// Executes the swap in the pool and stores the output. Updates pool assets but
// does not actually transfer any tokens to or from the pool.
tokenOutCoin, err := pool.SwapOutAmtGivenIn(ctx, tokensIn, tokenOutDenom, swapFee)
if err != nil {
return sdk.Int{}, err
Expand All @@ -64,6 +66,8 @@ func (k Keeper) swapExactAmountIn(
return sdk.Int{}, sdkerrors.Wrapf(types.ErrLimitMinAmount, "%s token is lesser than min amount", tokenOutDenom)
}

// Settles balances between the tx sender and the pool to match the swap that was executed earlier.
// Also emits swap event and updates related liquidity metrics
if err := k.updatePoolForSwap(ctx, pool, sender, tokenIn, tokenOutCoin); err != nil {
return sdk.Int{}, err
}
Expand Down
2 changes: 1 addition & 1 deletion x/gamm/pool-models/balancer/balancer_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ func (pa *Pool) updateAllWeights(newWeights []PoolAsset) {
}

// PokePool checks to see if the pool's token weights need to be updated, and
// if so, does so.
// if so, does so. Currently doesn't do anything outside out LBPs.
func (pa *Pool) PokePool(blockTime time.Time) {
// check if pool weights didn't change
poolWeightsChanging := pa.PoolParams.SmoothWeightChangeParams != nil
Expand Down
8 changes: 2 additions & 6 deletions x/tokenfactory/keeper/admins_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,7 @@ func (suite *KeeperTestSuite) TestAdminMsgs() {
// * Only the admin of a denom can mint tokens for it
// * The admin of a denom can mint tokens for it
func (suite *KeeperTestSuite) TestMintDenom() {
var (
addr0bal = int64(0)
)
var addr0bal int64

// Create a denom
suite.CreateDefaultDenom()
Expand Down Expand Up @@ -122,9 +120,7 @@ func (suite *KeeperTestSuite) TestMintDenom() {
}

func (suite *KeeperTestSuite) TestBurnDenom() {
var (
addr0bal = int64(0)
)
var addr0bal int64

// Create a denom.
suite.CreateDefaultDenom()
Expand Down
1 change: 0 additions & 1 deletion x/tokenfactory/keeper/createdenom_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ func (suite *KeeperTestSuite) TestCreateDenom() {
setup: func() {
_, err := suite.msgServer.CreateDenom(sdk.WrapSDKContext(suite.Ctx), types.NewMsgCreateDenom(suite.TestAccs[0].String(), "bitcoin"))
suite.Require().NoError(err)

},
subdenom: "bitcoin",
valid: false,
Expand Down
4 changes: 2 additions & 2 deletions x/tokenfactory/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import (
type KeeperTestSuite struct {
apptesting.KeeperTestHelper

queryClient types.QueryClient
msgServer types.MsgServer
queryClient types.QueryClient
msgServer types.MsgServer
// defaultDenom is on the suite, as it depends on the creator test address.
defaultDenom string
}
Expand Down