From 9fd1825a90568d2de5b7e2808eade30debc02c50 Mon Sep 17 00:00:00 2001
From: Simon Warta <2603011+webmaster128@users.noreply.github.com>
Date: Wed, 7 Dec 2022 00:12:25 +0100
Subject: [PATCH] perf: Refactor Coins/Validate to avoid unnecessary map
 (#14163)

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

and add a few tests

* Add CHANGELOG entry

Co-authored-by: Marko <marbar3778@yahoo.com>
Co-authored-by: Julien Robert <julien@rbrt.fr>
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
---
 CHANGELOG.md       |  1 +
 types/coin.go      | 11 ++++-------
 types/coin_test.go | 29 ++++++++++++++++++++++++++++-
 3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index cb9ea9cb8a8b..4c1798abc3f1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -58,6 +58,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 
+* (types) [#14163](https://github.com/cosmos/cosmos-sdk/pull/14163) Refactor `(coins Coins) Validate()` to avoid unnecessary map.
 * (signing) [#14087](https://github.com/cosmos/cosmos-sdk/pull/14087) Add SignModeHandlerWithContext interface with a new `GetSignBytesWithContext` to get the sign bytes using `context.Context` as an argument to access state.
 * (server) [#14062](https://github.com/cosmos/cosmos-sdk/pull/14062) Remove rosetta from server start.
 * [13882] (https://github.com/cosmos/cosmos-sdk/pull/13882) Add tx `encode` and `decode` endpoints to amino tx service.
diff --git a/types/coin.go b/types/coin.go
index ff72453a3fb2..5a2e2970f895 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 {