Refactor: Change Pool API to have separate methods for JoinPool and JoinPoolNoSwap

CHANGELOG.md

@@ -72,6 +72,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * [#1667](https://github.com/osmosis-labs/osmosis/pull/1673) Move wasm-bindings code out of app package into its own root level package.
 * [#2013](https://github.com/osmosis-labs/osmosis/pull/2013) Make `SetParams`, `SetPool`, `SetTotalLiquidity`, and `SetDenomLiquidity` GAMM APIs private
 * [#1857](https://github.com/osmosis-labs/osmosis/pull/1857) x/mint rename GetLastHalvenEpochNum to GetLastReductionEpochNum
+* [#2133](https://github.com/osmosis-labs/osmosis/pull/2133) Add `JoinPoolNoSwap` and `CalcJoinPoolNoSwapShares` to GAMM pool interface and route `JoinPoolNoSwap` in pool_service.go to new method in pool interface
 * [#2394](https://github.com/osmosis-labs/osmosis/pull/2394) Remove unused interface methods from expected keepers of each module
 * [#2390](https://github.com/osmosis-labs/osmosis/pull/2390) x/mint remove unused mintCoins parameter from AfterDistributeMintedCoin
 

x/gamm/keeper/pool_service.go

@@ -209,7 +209,7 @@ func (k Keeper) JoinPoolNoSwap(
 		}
 	}
 
-	sharesOut, err = pool.JoinPool(ctx, neededLpLiquidity, pool.GetSwapFee(ctx))
+	sharesOut, err = pool.JoinPoolNoSwap(ctx, neededLpLiquidity, pool.GetSwapFee(ctx))
 	if err != nil {
 		return nil, sdk.ZeroInt(), err
 	}

x/gamm/pool-models/balancer/amm.go

@@ -228,3 +228,15 @@ func calcPoolSharesInGivenSingleAssetOut(
 	sharesInFeeIncluded := sharesIn.Quo(sdk.OneDec().Sub(exitFee))
 	return sharesInFeeIncluded
 }
+
+// check to make sure the input denoms exist in the provided pool asset map
+func ensureDenomInPool(poolAssetsByDenom map[string]PoolAsset, tokensIn sdk.Coins) error {
+	for _, coin := range tokensIn {
+		_, ok := poolAssetsByDenom[coin.Denom]
+		if !ok {
+			return sdkerrors.Wrapf(types.ErrDenomNotFoundInPool, invalidInputDenomsErrFormat, coin.Denom)
+		}
+	}
+
+	return nil
+}

x/gamm/pool-models/balancer/export_test.go

@@ -15,6 +15,7 @@ var (
 	UpdateIntermediaryPoolAssetsLiquidity = updateIntermediaryPoolAssetsLiquidity
 
 	GetPoolAssetsByDenom = getPoolAssetsByDenom
+	EnsureDenomInPool    = ensureDenomInPool
 )
 
 func (p *Pool) CalcSingleAssetJoin(tokenIn sdk.Coin, swapFee sdk.Dec, tokenInPoolAsset PoolAsset, totalShares sdk.Int) (numShares sdk.Int, err error) {

x/gamm/pool-models/balancer/pool.go

@@ -663,6 +663,19 @@ func (p *Pool) JoinPool(ctx sdk.Context, tokensIn sdk.Coins, swapFee sdk.Dec) (n
 	return numShares, nil
 }
 
+// JoinPoolNoSwap calculates the number of shares needed for an all-asset join given tokensIn with swapFee applied.
+// It updates the liquidity if the pool is joined successfully. If not, returns error.
+func (p *Pool) JoinPoolNoSwap(ctx sdk.Context, tokensIn sdk.Coins, swapFee sdk.Dec) (numShares sdk.Int, err error) {
+	numShares, newLiquidity, _, err := p.CalcJoinPoolNoSwapShares(ctx, tokensIn, swapFee)
+	if err != nil {
+		return sdk.Int{}, err
+	}
+
+	// update pool with the calculated share and liquidity needed to join pool
+	p.IncreaseLiquidity(numShares, newLiquidity)
+	return numShares, nil
+}
+
 // CalcJoinPoolShares calculates the number of shares created to join pool with the provided amount of `tokenIn`.
 // The input tokens must either be:
 // - a single token
@@ -689,12 +702,10 @@ func (p *Pool) CalcJoinPoolShares(ctx sdk.Context, tokensIn sdk.Coins, swapFee s
 		return sdk.ZeroInt(), sdk.NewCoins(), err
 	}
 
-	// check to make sure the input denoms exist in the pool
-	for _, coin := range tokensIn {
-		_, ok := poolAssetsByDenom[coin.Denom]
-		if !ok {
-			return sdk.ZeroInt(), sdk.NewCoins(), sdkerrors.Wrapf(types.ErrDenomAlreadyInPool, invalidInputDenomsErrFormat, coin.Denom)
-		}
+	// check to make sure the input denom exists in the pool
+	err = ensureDenomInPool(poolAssetsByDenom, tokensIn)
+	if err != nil {
+		return sdk.ZeroInt(), sdk.NewCoins(), err
 	}
 
 	totalShares := p.GetTotalShares()
@@ -715,12 +726,15 @@ func (p *Pool) CalcJoinPoolShares(ctx sdk.Context, tokensIn sdk.Coins, swapFee s
 	// * numShares is how many shares are perfectly matched.
 	// * remainingTokensIn is how many coins we have left to join, that have not already been used.
 	// if remaining coins is empty, logic is done (we joined all tokensIn)
-	numShares, remainingTokensIn, err := cfmm_common.MaximalExactRatioJoin(p, sdk.Context{}, tokensIn)
+	numShares, tokensJoined, remainingTokensIn, err := p.CalcJoinPoolNoSwapShares(ctx, tokensIn, swapFee)
 	if err != nil {
 		return sdk.ZeroInt(), sdk.NewCoins(), err
 	}
-	if remainingTokensIn.Empty() {
-		tokensJoined = tokensIn
+
+	// safely ends the calculation if all input tokens are successfully LP'd
+	if tokensJoined.IsAnyGT(tokensIn) {
+		return sdk.ZeroInt(), sdk.NewCoins(), errors.New("an error has occurred, more coins joined than tokens passed in")
+	} else if remainingTokensIn.Empty() {
 		return numShares, tokensJoined, nil
 	}
 
@@ -729,7 +743,6 @@ func (p *Pool) CalcJoinPoolShares(ctx sdk.Context, tokensIn sdk.Coins, swapFee s
 	// Instead, it mutates the pool assets argument to be further used by the caller.
 	// * We add the joined coins to our "current pool liquidity" object (poolAssetsByDenom)
 	// * We increment a variable for our "newTotalShares" to add in the shares that've been added.
-	tokensJoined = tokensIn.Sub(remainingTokensIn)
 	if err := updateIntermediaryPoolAssetsLiquidity(tokensJoined, poolAssetsByDenom); err != nil {
 		return sdk.ZeroInt(), sdk.NewCoins(), err
 	}
@@ -751,6 +764,49 @@ func (p *Pool) CalcJoinPoolShares(ctx sdk.Context, tokensIn sdk.Coins, swapFee s
 	return numShares, tokensJoined, nil
 }
 
+// CalcJoinPoolNoSwapShares calculates the number of shares created to execute an all-asset pool join with the provided amount of `tokensIn`.
+// The input tokens must contain exactly the same tokens as the pool contains
+//
+// Returns the number of shares created, the amount of coins actually joined into the pool, (in case of not being able to fully join),
+// and the remaining tokens in `tokensIn` after joining. If an all-asset join is not possible, returns an error.
+//
+// Since CalcJoinPoolNoSwapShares is non-mutative, the steps for updating pool shares / liquidity are
+// more complex / don't just alter the state.
+// We should simplify this logic further in the future using multi-join equations.
+func (p *Pool) CalcJoinPoolNoSwapShares(ctx sdk.Context, tokensIn sdk.Coins, swapFee sdk.Dec) (numShares sdk.Int, tokensJoined sdk.Coins, remainingTokens sdk.Coins, err error) {
+	// get all 'pool assets' (aka current pool liquidity + balancer weight)
+	poolAssetsByDenom, err := getPoolAssetsByDenom(p.GetAllPoolAssets())
+	if err != nil {
+		return sdk.ZeroInt(), sdk.NewCoins(), tokensIn, err
+	}
+
+	err = ensureDenomInPool(poolAssetsByDenom, tokensIn)
+	if err != nil {
+		return sdk.ZeroInt(), sdk.NewCoins(), tokensIn, err
+	}
+
+	// ensure that there aren't too many or too few assets in `tokensIn`
+	if tokensIn.Len() != p.NumAssets() {
+		return sdk.ZeroInt(), sdk.NewCoins(), tokensIn, errors.New("no-swap joins require LP'ing with all assets in pool")
+	}
+
+	// execute a no-swap join with as many tokens as possible given a perfect ratio:
+	// * numShares is how many shares are perfectly matched.
+	// * remainingTokensIn is how many coins we have left to join that have not already been used.
+	numShares, remainingTokensIn, err := cfmm_common.MaximalExactRatioJoin(p, sdk.Context{}, tokensIn)
+	if err != nil {
+		return sdk.ZeroInt(), sdk.NewCoins(), tokensIn, err
+	}
+
+	// ensure that no more tokens have been joined than is possible with the given `tokensIn`
+	tokensJoined = tokensIn.Sub(remainingTokensIn)
+	if tokensJoined.IsAnyGT(tokensIn) {
+		return sdk.ZeroInt(), sdk.NewCoins(), tokensIn, errors.New("an error has occurred, more coins joined than token In")
+	}
+
+	return numShares, tokensJoined, remainingTokensIn, nil
+}
+
 // calcJoinSingleAssetTokensIn attempts to calculate single
 // asset join for all tokensIn given totalShares in pool,
 // poolAssetsByDenom and swapFee. totalShares is the number

x/gamm/pool-models/balancer/pool_test.go

@@ -469,6 +469,59 @@ func TestGetPoolAssetsByDenom(t *testing.T) {
 	}
 }
 
+func (suite *KeeperTestSuite) TestEnsureDenomInPool() {
+	tests := map[string]struct {
+		poolAssets  []balancer.PoolAsset
+		tokensIn    sdk.Coins
+		expectPass  bool
+		expectedErr error
+	}{
+		"all of tokensIn is in pool asset map": {
+			poolAssets:  []balancer.PoolAsset{defaultOsmoPoolAsset, defaultAtomPoolAsset},
+			tokensIn:    sdk.NewCoins(sdk.NewCoin("uatom", sdk.OneInt())),
+			expectPass:  true,
+			expectedErr: nil,
+		},
+		"one of tokensIn is in pool asset map": {
+			poolAssets:  []balancer.PoolAsset{defaultOsmoPoolAsset, defaultAtomPoolAsset},
+			tokensIn:    sdk.NewCoins(sdk.NewCoin("uatom", sdk.OneInt()), sdk.NewCoin("foo", sdk.OneInt())),
+			expectPass:  false,
+			expectedErr: types.ErrDenomNotFoundInPool,
+		},
+		"none of tokensIn is in pool asset map": {
+			poolAssets:  []balancer.PoolAsset{defaultOsmoPoolAsset, defaultAtomPoolAsset},
+			tokensIn:    sdk.NewCoins(sdk.NewCoin("foo", sdk.OneInt())),
+			expectPass:  false,
+			expectedErr: types.ErrDenomNotFoundInPool,
+		},
+	}
+
+	for name, tc := range tests {
+		suite.Run(name, func() {
+			suite.SetupTest()
+
+			k := suite.App.GAMMKeeper
+
+			poolId := suite.PrepareBalancerPoolWithPoolAsset(tc.poolAssets)
+			poolI, err := k.GetPoolAndPoke(suite.Ctx, poolId)
+			suite.Require().NoError(err, "test: %s", name)
+			balancerPool, isPool := poolI.(*balancer.Pool)
+			suite.Require().True(isPool, "test: %s", name)
+
+			poolAssetsByDenom, err := balancer.GetPoolAssetsByDenom(balancerPool.GetAllPoolAssets())
+			suite.Require().NoError(err, "test: %s", name)
+
+			err = balancer.EnsureDenomInPool(poolAssetsByDenom, tc.tokensIn)
+
+			if tc.expectPass {
+				suite.Require().NoError(err, "test: %s", name)
+			} else {
+				suite.Require().ErrorIs(err, tc.expectedErr, "test: %s", name)
+			}
+		})
+	}
+}
+
 // TestCalculateAmountOutAndIn_InverseRelationship tests that the same amount of token is guaranteed upon
 // sequential operation of CalcInAmtGivenOut and CalcOutAmtGivenIn.
 func (suite *BalancerTestSuite) TestBalancerCalculateAmountOutAndIn_InverseRelationship(t *testing.T) {
@@ -1218,3 +1271,91 @@ func TestIsActive(t *testing.T) {
 		})
 	}
 }
+
+func TestCalcJoinPoolNoSwapShares(t *testing.T) {
+	balancerPoolAsset := []balancer.PoolAsset{
+		{Token: sdk.NewInt64Coin("foo", 100), Weight: sdk.NewIntFromUint64(5)},
+		{Token: sdk.NewInt64Coin("bar", 100), Weight: sdk.NewIntFromUint64(5)},
+	}
+
+	tests := map[string]struct {
+		tokensIn        sdk.Coins
+		expNumShare     sdk.Int
+		expTokensJoined sdk.Coins
+		expPoolAssets   sdk.Coins
+		expRemainingCoins      sdk.Coins
+		expectPass      bool
+	}{
+		"two asset pool, same tokenIn ratio": {
+			tokensIn:        sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(10)), sdk.NewCoin("bar", sdk.NewInt(10))),
+			expNumShare:     sdk.NewIntFromUint64(10000000000000000000),
+			expTokensJoined: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(10)), sdk.NewCoin("bar", sdk.NewInt(10))),
+			expPoolAssets:   sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(100)), sdk.NewCoin("bar", sdk.NewInt(100))),
+			expRemainingCoins:      sdk.Coins{},
+			expectPass:      true,
+		},
+		"two asset pool, different tokenIn ratio with pool": {
+			tokensIn:        sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(10)), sdk.NewCoin("bar", sdk.NewInt(11))),
+			expNumShare:     sdk.NewIntFromUint64(10000000000000000000),
+			expTokensJoined: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(10)), sdk.NewCoin("bar", sdk.NewInt(10))),
+			expPoolAssets:   sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(100)), sdk.NewCoin("bar", sdk.NewInt(100))),
+			expRemainingCoins:      sdk.NewCoins(sdk.NewCoin("bar", sdk.NewIntFromUint64(1))),
+			expectPass:      true,
+		},
+		"two asset pool, no-swap join attempt with one asset": {
+			tokensIn:        sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(10))),
+			expNumShare:     sdk.NewIntFromUint64(0),
+			expTokensJoined: sdk.Coins{},
+			expPoolAssets:   sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(100)), sdk.NewCoin("bar", sdk.NewInt(100))),
+			expRemainingCoins:      sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(10))),
+			expectPass:      false,
+		},
+		"two asset pool, no-swap join attempt with one valid and one invalid asset": {
+			tokensIn:        sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(10)), sdk.NewCoin("baz", sdk.NewInt(10))),
+			expNumShare:     sdk.NewIntFromUint64(0),
+			expTokensJoined: sdk.Coins{},
+			expPoolAssets:   sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(100)), sdk.NewCoin("bar", sdk.NewInt(100))),
+			expRemainingCoins:      sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(10)), sdk.NewCoin("baz", sdk.NewInt(10))),
+			expectPass:      false,
+		},
+		"two asset pool, no-swap join attempt with two invalid assets": {
+			tokensIn:        sdk.NewCoins(sdk.NewCoin("baz", sdk.NewInt(10)), sdk.NewCoin("qux", sdk.NewInt(10))),
+			expNumShare:     sdk.NewIntFromUint64(0),
+			expTokensJoined: sdk.Coins{},
+			expPoolAssets:   sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(100)), sdk.NewCoin("bar", sdk.NewInt(100))),
+			expRemainingCoins:      sdk.NewCoins(sdk.NewCoin("baz", sdk.NewInt(10)), sdk.NewCoin("qux", sdk.NewInt(10))),
+			expectPass:      false,
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			ctx := sdk.Context{}
+			balancerPool, err := balancer.NewBalancerPool(
+				defaultPoolId,
+				balancer.PoolParams{SwapFee: defaultSwapFee, ExitFee: defaultExitFee},
+				balancerPoolAsset,
+				defaultFutureGovernor,
+				defaultCurBlockTime,
+			)
+			require.NoError(t, err)
+
+			numShare, tokensJoined, remainingCoins, err := balancerPool.CalcJoinPoolNoSwapShares(ctx, test.tokensIn, balancerPool.GetSwapFee(ctx))
+			poolAssets := sdk.NewCoins(balancerPool.PoolAssets[0].Token, balancerPool.PoolAssets[1].Token)
+
+			if test.expectPass {
+				require.NoError(t, err)
+				require.Equal(t, test.expPoolAssets, poolAssets)
+				require.Equal(t, test.expNumShare, numShare)
+				require.Equal(t, test.expTokensJoined, tokensJoined)
+				require.Equal(t, test.expRemainingCoins, remainingCoins)
+			} else {
+				require.Error(t, err)
+				require.Equal(t, test.expPoolAssets, poolAssets)
+				require.Equal(t, test.expNumShare, numShare)
+				require.Equal(t, test.expTokensJoined, tokensJoined)
+				require.Equal(t, test.expRemainingCoins, remainingCoins)
+			}
+		})
+	}
+}

x/gamm/pool-models/stableswap/pool.go

@@ -258,11 +258,21 @@ func (p *Pool) CalcJoinPoolShares(ctx sdk.Context, tokensIn sdk.Coins, swapFee s
 	return pCopy.joinPoolSharesInternal(ctx, tokensIn, swapFee)
 }
 
+// TODO: implement this
+func (p *Pool) CalcJoinPoolNoSwapShares(ctx sdk.Context, tokensIn sdk.Coins, swapFee sdk.Dec) (numShares sdk.Int, newLiquidity sdk.Coins, remainingCoins sdk.Coins, err error) {
+	return sdk.ZeroInt(), nil, nil, err
+}
+
 func (p *Pool) JoinPool(ctx sdk.Context, tokensIn sdk.Coins, swapFee sdk.Dec) (numShares sdk.Int, err error) {
 	numShares, _, err = p.joinPoolSharesInternal(ctx, tokensIn, swapFee)
 	return numShares, err
 }
 
+// TODO: implement this
+func (p *Pool) JoinPoolNoSwap(ctx sdk.Context, tokensIn sdk.Coins, swapFee sdk.Dec) (numShares sdk.Int, err error) {
+	return sdk.ZeroInt(), err
+}
+
 func (p *Pool) ExitPool(ctx sdk.Context, exitingShares sdk.Int, exitFee sdk.Dec) (exitingCoins sdk.Coins, err error) {
 	exitingCoins, err = p.CalcExitPoolCoinsFromShares(ctx, exitingShares, exitFee)
 	if err != nil {

x/gamm/types/pool.go

@@ -59,10 +59,19 @@ type PoolI interface {
 	// Pools are expected to guarantee LP'ing at the exact ratio, and single sided LP'ing.
 	JoinPool(ctx sdk.Context, tokensIn sdk.Coins, swapFee sdk.Dec) (numShares sdk.Int, err error)
 
+	// JoinPoolNoSwap joins the pool with an all-asset join using the maximum amount possible given the tokensIn provided.
+	// This function is mutative and updates the pool's internal state if there is no error.
+	// Pools are expected to guarantee LP'ing at the exact ratio.
+	JoinPoolNoSwap(ctx sdk.Context, tokensIn sdk.Coins, swapFee sdk.Dec) (numShares sdk.Int, err error)
+
 	// CalcJoinPoolShares returns how many LP shares JoinPool would return on these arguments.
 	// This does not mutate the pool, or state.
 	CalcJoinPoolShares(ctx sdk.Context, tokensIn sdk.Coins, swapFee sdk.Dec) (numShares sdk.Int, newLiquidity sdk.Coins, err error)
 
+	// CalcJoinPoolNoSwapShares returns how many LP shares JoinPoolNoSwap would return on these arguments.
+	// This does not mutate the pool, or state.
+	CalcJoinPoolNoSwapShares(ctx sdk.Context, tokensIn sdk.Coins, swapFee sdk.Dec) (numShares sdk.Int, newLiquidity sdk.Coins, remainingTokens sdk.Coins, err error)
+
 	// ExitPool exits #numShares LP shares from the pool, decreases its internal liquidity & LP share totals,
 	// and returns the number of coins that are being returned.
 	// This mutates the pool and state.