x/gamm: Add check to ensure JoinPoolNoSwap cannot be called with arbi…

…trary denoms (#1930)

* add denom equivalence check for input vs. expected

* add test for new logic

* refactor new logic and add more relevant error messages

* update changelog

* Revert "update changelog"

This reverts commit fa33bf7.

* Update changelog

CHANGELOG.md

@@ -64,6 +64,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * [#1539] Superfluid: Combine superfluid and staking query on querying delegation by delegator
 
 ### Bug Fixes
+
+* [1930](https://github.com/osmosis-labs/osmosis/pull/1930) Ensure you can't `JoinPoolNoSwap` tokens that are not in the pool
 * [1700](https://github.com/osmosis-labs/osmosis/pull/1700) Upgrade sdk fork with missing snapshot manager fix.
 * [1716](https://github.com/osmosis-labs/osmosis/pull/1716) Fix secondary over-LP shares bug with uneven swap amounts in `CalcJoinPoolShares`.
 * [1759](https://github.com/osmosis-labs/osmosis/pull/1759) Fix pagination filter in incentives query.

x/gamm/keeper/pool_service.go

@@ -203,6 +203,9 @@ func (k Keeper) JoinPoolNoSwap(
 		if !(neededLpLiquidity.DenomsSubsetOf(tokenInMaxs) && tokenInMaxs.IsAllGTE(neededLpLiquidity)) {
 			return sdkerrors.Wrapf(types.ErrLimitMaxAmount, "TokenInMaxs is less than the needed LP liquidity to this JoinPoolNoSwap,"+
 				" upperbound: %v, needed %v", tokenInMaxs, neededLpLiquidity)
+		} else if !(tokenInMaxs.DenomsSubsetOf(neededLpLiquidity)) {
+			return sdkerrors.Wrapf(types.ErrDenomNotFoundInPool, "TokenInMaxs includes tokens that are not part of the target pool,"+
+				" input tokens: %v, pool tokens %v", tokenInMaxs, neededLpLiquidity)
 		}
 	}
 

x/gamm/keeper/pool_service_test.go

@@ -316,6 +316,18 @@ func (suite *KeeperTestSuite) TestJoinPoolNoSwap() {
 				suite.Require().Equal("15000bar,15000foo", liquidity.String())
 			},
 		},
+		{
+			fn: func(poolId uint64) {
+				keeper := suite.App.GAMMKeeper
+				// Test the "tokenInMaxs" with an additional invalid denom
+				// In this case, to get the 50 * OneShare amount of share token, the foo, bar token are expected to be provided as 5000 amounts.
+				// The test input has the correct amount for each, but also includes an incorrect denom that should cause an error
+				err := keeper.JoinPoolNoSwap(suite.Ctx, suite.TestAccs[1], poolId, types.OneShare.MulRaw(50), sdk.Coins{
+					sdk.NewCoin("bar", sdk.NewInt(5000)), sdk.NewCoin("foo", sdk.NewInt(5000)), sdk.NewCoin("baz", sdk.NewInt(5000)),
+				})
+				suite.Require().Error(err)
+			},
+		},
 	}
 
 	for _, test := range tests {