Refactor (coins Coins) Validate() to avoid unnecessary map

and add a few tests

types/coin.go

@@ -247,26 +247,23 @@ func (coins Coins) Validate() error {
 		}
 
 		lowDenom := coins[0].Denom
-		seenDenoms := make(map[string]bool)
-		seenDenoms[lowDenom] = true
 
 		for _, coin := range coins[1:] {
-			if seenDenoms[coin.Denom] {
-				return fmt.Errorf("duplicate denomination %s", coin.Denom)
-			}
 			if err := ValidateDenom(coin.Denom); err != nil {
 				return err
 			}
-			if coin.Denom <= lowDenom {
+			if coin.Denom < lowDenom {
 				return fmt.Errorf("denomination %s is not sorted", coin.Denom)
 			}
+			if coin.Denom == lowDenom {
+				return fmt.Errorf("duplicate denomination %s", coin.Denom)
+			}
 			if !coin.IsPositive() {
 				return fmt.Errorf("coin %s amount is not positive", coin.Denom)
 			}
 
 			// we compare each coin against the last denom
 			lowDenom = coin.Denom
-			seenDenoms[coin.Denom] = true
 		}
 
 		return nil

types/coin_test.go

@@ -778,6 +778,15 @@ func (s *coinTestSuite) TestCoins_Validate() {
 			},
 			false,
 		},
+		{
+			"bad sort (3)",
+			sdk.Coins{
+				{"gas", math.OneInt()},
+				{"tree", math.OneInt()},
+				{"gas", math.OneInt()},
+			},
+			false,
+		},
 		{
 			"non-positive amount (1)",
 			sdk.Coins{
@@ -797,14 +806,32 @@ func (s *coinTestSuite) TestCoins_Validate() {
 			false,
 		},
 		{
-			"duplicate denomination",
+			"duplicate denomination (1)",
 			sdk.Coins{
 				{"gas", math.OneInt()},
 				{"gas", math.OneInt()},
 				{"mineral", math.OneInt()},
 			},
 			false,
 		},
+		{
+			"duplicate denomination (2)",
+			sdk.Coins{
+				{"gold", math.OneInt()},
+				{"gold", math.OneInt()},
+			},
+			false,
+		},
+		{
+			"duplicate denomination (3)",
+			sdk.Coins{
+				{"gas", math.OneInt()},
+				{"mineral", math.OneInt()},
+				{"silver", math.OneInt()},
+				{"silver", math.OneInt()},
+			},
+			false,
+		},
 	}
 
 	for _, tc := range testCases {