[x/gamm][Misc Improvements] Fix API inconsistency between JoinPool and CalcJoinPoolShares

[ValarDragon]

The API is currently:

	// JoinPool joins the pool using all of the tokensIn provided.
	// The AMM swaps to the correct internal ratio should be and returns the number of shares created.
	// This function is mutative and updates the pool's internal state if there is no error.
	// It is up to pool implementation if they support LP'ing at arbitrary ratios, or a subset of ratios.
	// 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)
	// 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)

CalcJoinPoolShares should either not return newLiquidity or JoinPool should return it. The current method of one method not being guaranteed to use all liquidity, while the other is required to use all liquidity is ... odd.

[mattverse]

@ValarDragon wdym by

The current method of one method not being guaranteed to use all liquidity, while the other is required to use all liquidity

imo, the API inconsistency makes sense: since JoinPool deosn't need to return newLiquidity because the return value would not be used. We also have the option to make both not return newLiquidity as you have mentioned, but then we would have to move the layer of calculating the updated liquidity to JoinPool which would result in extra overhead

[p0mvn]

I agree with @mattverse here. I don't see a reason for returning newLiquidity from JoinPool. At the same, if we don't return it from CalcJoinPoolShares, then we must essentially merge the two methods.

I will close this issue for now. However, if anyone disagrees / would like to discuss further, please let me know