Audit Fix: Don't use coin.Add in deposit

every sdk.Coin.Add happening in DepositCore results in multiple traversals over all coins
in sharesIssued , and also involves allocation of additional data structures, thus resulting in quadratic
scaling of the DepositCore runtime (because it invokes sdk.Coins Add() for each issued share).

utils/bank.go

@@ -2,6 +2,7 @@ package utils
 
 import (
 	"fmt"
+	"sort"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/neutron-org/neutron/x/dex/types"
@@ -105,3 +106,24 @@ func FilteredPaginateAccountBalances(
 
 	return res, nil
 }
+
+// SanitizeCoins takes an unsorted list of coins and sorts them, removes coins with amount zero and combines duplicate coins
+func SanitizeCoins(coins []sdk.Coin) sdk.Coins {
+	sort.SliceStable(coins, func(i, j int) bool {
+		return coins[i].Denom < coins[j].Denom
+	})
+	cleanCoins := sdk.Coins{}
+	lastDenom := ""
+	for _, coin := range coins {
+		if coin.IsZero() {
+			continue
+		}
+		if lastDenom != coin.Denom {
+			cleanCoins = append(cleanCoins, coin)
+		} else {
+			cleanCoins[len(cleanCoins)-1].Add(coin)
+		}
+		lastDenom = coin.Denom
+	}
+	return cleanCoins
+}

x/dex/keeper/core.go

@@ -7,9 +7,10 @@ import (
 	sdkerrors "cosmossdk.io/errors"
 	"cosmossdk.io/math"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/neutron-org/neutron/utils"
 	math_utils "github.com/neutron-org/neutron/utils/math"
 	"github.com/neutron-org/neutron/x/dex/types"
-	"github.com/neutron-org/neutron/x/dex/utils"
+	dexutils "github.com/neutron-org/neutron/x/dex/utils"
 )
 
 // NOTE: Currently we are using TruncateInt in multiple places for converting Decs back into math.Ints.
@@ -79,7 +80,7 @@ func (k Keeper) DepositCore(
 		if err := k.MintShares(ctx, receiverAddr, outShares); err != nil {
 			return nil, nil, nil, err
 		}
-		sharesIssued = sharesIssued.Add(outShares)
+		sharesIssued = append(sharesIssued, outShares)
 
 		amounts0Deposited[i] = inAmount0
 		amounts1Deposited[i] = inAmount1
@@ -99,6 +100,9 @@ func (k Keeper) DepositCore(
 		))
 	}
 
+	// At this point shares issued is not sorted and may have duplicates
+	// we must sanitize to convert it to a valid set of coins
+	sharesIssued = utils.SanitizeCoins(sharesIssued)
 	if totalAmountReserve0.IsPositive() {
 		coin0 := sdk.NewCoin(pairID.Token0, totalAmountReserve0)
 		if err := k.bankKeeper.SendCoinsFromAccountToModule(ctx, callerAddr, types.ModuleName, sdk.Coins{coin0}); err != nil {
@@ -257,7 +261,7 @@ func (k Keeper) MultiHopSwapCore(
 	if len(routeErrors) == len(routes) {
 		// All routes have failed
 
-		allErr := utils.JoinErrors(types.ErrAllMultiHopRoutesFailed, routeErrors...)
+		allErr := dexutils.JoinErrors(types.ErrAllMultiHopRoutesFailed, routeErrors...)
 
 		return sdk.Coin{}, allErr
 	}