From 4997976e49cedd7d0a5a75cf1fc7fa7e0554ffb7 Mon Sep 17 00:00:00 2001
From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com>
Date: Thu, 8 Dec 2022 11:25:34 +0100
Subject: [PATCH] perf: Refactor Coins/Validate to avoid unnecessary map
 (backport #14163) (#14182)

Co-authored-by: Simon Warta <2603011+webmaster128@users.noreply.github.com>
Co-authored-by: Julien Robert <julien@rbrt.fr>
---
 CHANGELOG.md       |  4 ++++
 types/coin.go      | 11 ++++-------
 types/coin_test.go | 29 ++++++++++++++++++++++++++++-
 3 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 58322e2e51c9..bf01ee32c598 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -46,6 +46,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 * (genesis) [#14149](https://github.com/cosmos/cosmos-sdk/pull/14149) Add `simd genesis` command, which contains all genesis-related sub-commands.
 
+### Improvements
+
+* (types) [#14163](https://github.com/cosmos/cosmos-sdk/pull/14163) Refactor `(coins Coins) Validate()` to avoid unnecessary map.
+
 ## [v0.47.0-alpha2](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.47.0-alpha2) - 2022-12-06
 
 ### Improvements
diff --git a/types/coin.go b/types/coin.go
index 8e0ee7f86b75..e124abf1bad8 100644
--- a/types/coin.go
+++ b/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
diff --git a/types/coin_test.go b/types/coin_test.go
index 7172c771989b..8ac8e5faae1a 100644
--- a/types/coin_test.go
+++ b/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,7 +806,7 @@ func (s *coinTestSuite) TestCoins_Validate() {
 			false,
 		},
 		{
-			"duplicate denomination",
+			"duplicate denomination (1)",
 			sdk.Coins{
 				{"gas", math.OneInt()},
 				{"gas", math.OneInt()},
@@ -805,6 +814,24 @@ func (s *coinTestSuite) TestCoins_Validate() {
 			},
 			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 {