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

CHANGELOG.md

@@ -77,6 +77,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
 * [#2418](https://github.com/osmosis-labs/osmosis/pull/2418) x/mint remove SetInitialSupplyOffsetDuringMigration from keeper

x/gamm/keeper/pool_service.go

@@ -236,7 +236,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

@@ -661,6 +661,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, tokensJoined, 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, tokensJoined)
+	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
@@ -687,12 +700,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()
@@ -713,12 +724,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, 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 tokensJoined.IsEqual(tokensIn) {
 		return numShares, tokensJoined, nil
 	}
 
@@ -727,13 +741,13 @@ 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
 	}
 	newTotalShares := totalShares.Add(numShares)
 
 	// 5) Now single asset join each remaining coin.
+	remainingTokensIn := tokensIn.Sub(tokensJoined)
 	newNumSharesFromRemaining, newLiquidityFromRemaining, err := p.calcJoinSingleAssetTokensIn(remainingTokensIn, newTotalShares, poolAssetsByDenom, swapFee)
 	if err != nil {
 		return sdk.ZeroInt(), sdk.NewCoins(), err
@@ -749,6 +763,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, 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(), err
+	}
+
+	err = ensureDenomInPool(poolAssetsByDenom, tokensIn)
+	if err != nil {
+		return sdk.ZeroInt(), sdk.NewCoins(), err
+	}
+
+	// ensure that there aren't too many or too few assets in `tokensIn`
+	if tokensIn.Len() != p.NumAssets() {
+		return sdk.ZeroInt(), sdk.NewCoins(), 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(), 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(), errors.New("an error has occurred, more coins joined than token In")
+	}
+
+	return numShares, tokensJoined, 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_suite_test.go

@@ -790,6 +790,28 @@ func (suite *KeeperTestSuite) TestCalcJoinPoolShares() {
 			),
 			expectShares: sdk.NewInt(100_000_000),
 		},
+		{
+			// Input assets do not exist in pool
+			// P_issued = 1e20 * 1e-12 = 1e8
+			name:    "attempt join with denoms not in pool",
+			swapFee: sdk.MustNewDecFromStr("0"),
+			poolAssets: []balancer.PoolAsset{
+				{
+					Token:  sdk.NewInt64Coin("uosmo", 1_000_000_000_000),
+					Weight: sdk.NewInt(100),
+				},
+				{
+					Token:  sdk.NewInt64Coin("uatom", 1_000_000_000_000),
+					Weight: sdk.NewInt(100),
+				},
+			},
+			tokensIn: sdk.NewCoins(
+				sdk.NewInt64Coin("foo", 1),
+				sdk.NewInt64Coin("baz", 1),
+			),
+			expectShares: sdk.NewInt(0),
+			expErr:       types.ErrDenomNotFoundInPool,
+		},
 	}
 	testCases = append(testCases, calcSingleAssetJoinTestCases...)