From 282afcdb123b1c5744d412d4f8ff51d2a0a6f05e Mon Sep 17 00:00:00 2001
From: Ira Miller <72319+iramiller@users.noreply.github.com>
Date: Tue, 28 Jul 2020 04:17:58 -0600
Subject: [PATCH] Fix for 6786 removeZeroCoins mutates original slice (#6811)

* added test cases to show errors identified in 6786

* mods to fix remove zero coin mutation in slice

Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 types/coin.go          | 15 +++++-------
 types/coin_test.go     | 55 ++++++++++++++++++++++++++++++++++++++++++
 types/dec_coin.go      | 15 +++++-------
 types/dec_coin_test.go | 55 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 122 insertions(+), 18 deletions(-)

diff --git a/types/coin.go b/types/coin.go
index 96b9c71b2d..ad287a0a78 100644
--- a/types/coin.go
+++ b/types/coin.go
@@ -546,18 +546,15 @@ func (coins Coins) negative() Coins {
 
 // removeZeroCoins removes all zero coins from the given coin set in-place.
 func removeZeroCoins(coins Coins) Coins {
-	i, l := 0, len(coins)
-	for i < l {
-		if coins[i].IsZero() {
-			// remove coin
-			coins = append(coins[:i], coins[i+1:]...)
-			l--
-		} else {
-			i++
+	result := make([]Coin, 0, len(coins))
+
+	for _, coin := range coins {
+		if !coin.IsZero() {
+			result = append(result, coin)
 		}
 	}
 
-	return coins[:i]
+	return result
 }
 
 //-----------------------------------------------------------------------------
diff --git a/types/coin_test.go b/types/coin_test.go
index 7177a180b0..9d896861ab 100644
--- a/types/coin_test.go
+++ b/types/coin_test.go
@@ -191,6 +191,61 @@ func TestCoinIsZero(t *testing.T) {
 	require.False(t, res)
 }
 
+func TestFilteredZeroCoins(t *testing.T) {
+	cases := []struct {
+		name     string
+		input    Coins
+		original string
+		expected string
+	}{
+		{
+			name: "all greater than zero",
+			input: Coins{
+				{"testa", NewInt(1)},
+				{"testb", NewInt(2)},
+				{"testc", NewInt(3)},
+				{"testd", NewInt(4)},
+				{"teste", NewInt(5)},
+			},
+			original: "1testa,2testb,3testc,4testd,5teste",
+			expected: "1testa,2testb,3testc,4testd,5teste",
+		},
+		{
+			name: "zero coin in middle",
+			input: Coins{
+				{"testa", NewInt(1)},
+				{"testb", NewInt(2)},
+				{"testc", NewInt(0)},
+				{"testd", NewInt(4)},
+				{"teste", NewInt(5)},
+			},
+			original: "1testa,2testb,0testc,4testd,5teste",
+			expected: "1testa,2testb,4testd,5teste",
+		},
+		{
+			name: "zero coin end (unordered)",
+			input: Coins{
+				{"teste", NewInt(5)},
+				{"testc", NewInt(3)},
+				{"testa", NewInt(1)},
+				{"testd", NewInt(4)},
+				{"testb", NewInt(0)},
+			},
+			original: "5teste,3testc,1testa,4testd,0testb",
+			expected: "1testa,3testc,4testd,5teste",
+		},
+	}
+
+	for _, tt := range cases {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			undertest := NewCoins(tt.input...)
+			require.Equal(t, tt.expected, undertest.String(), "NewCoins must return expected results")
+			require.Equal(t, tt.original, tt.input.String(), "input must be unmodified and match original")
+		})
+	}
+}
+
 // ----------------------------------------------------------------------------
 // Coins tests
 
diff --git a/types/dec_coin.go b/types/dec_coin.go
index 4b3396ca87..128db1cae3 100644
--- a/types/dec_coin.go
+++ b/types/dec_coin.go
@@ -556,18 +556,15 @@ func (coins DecCoins) IsAllPositive() bool {
 }
 
 func removeZeroDecCoins(coins DecCoins) DecCoins {
-	i, l := 0, len(coins)
-	for i < l {
-		if coins[i].IsZero() {
-			// remove coin
-			coins = append(coins[:i], coins[i+1:]...)
-			l--
-		} else {
-			i++
+	result := make([]DecCoin, 0, len(coins))
+
+	for _, coin := range coins {
+		if !coin.IsZero() {
+			result = append(result, coin)
 		}
 	}
 
-	return coins[:i]
+	return result
 }
 
 //-----------------------------------------------------------------------------
diff --git a/types/dec_coin_test.go b/types/dec_coin_test.go
index 2a20bd6469..598ea2f486 100644
--- a/types/dec_coin_test.go
+++ b/types/dec_coin_test.go
@@ -96,6 +96,61 @@ func TestAddDecCoins(t *testing.T) {
 	}
 }
 
+func TestFilteredZeroDecCoins(t *testing.T) {
+	cases := []struct {
+		name     string
+		input    DecCoins
+		original string
+		expected string
+	}{
+		{
+			name: "all greater than zero",
+			input: DecCoins{
+				{"testa", NewDec(1)},
+				{"testb", NewDec(2)},
+				{"testc", NewDec(3)},
+				{"testd", NewDec(4)},
+				{"teste", NewDec(5)},
+			},
+			original: "1.000000000000000000testa,2.000000000000000000testb,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste",
+			expected: "1.000000000000000000testa,2.000000000000000000testb,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste",
+		},
+		{
+			name: "zero coin in middle",
+			input: DecCoins{
+				{"testa", NewDec(1)},
+				{"testb", NewDec(2)},
+				{"testc", NewDec(0)},
+				{"testd", NewDec(4)},
+				{"teste", NewDec(5)},
+			},
+			original: "1.000000000000000000testa,2.000000000000000000testb,0.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste",
+			expected: "1.000000000000000000testa,2.000000000000000000testb,4.000000000000000000testd,5.000000000000000000teste",
+		},
+		{
+			name: "zero coin end (unordered)",
+			input: DecCoins{
+				{"teste", NewDec(5)},
+				{"testc", NewDec(3)},
+				{"testa", NewDec(1)},
+				{"testd", NewDec(4)},
+				{"testb", NewDec(0)},
+			},
+			original: "5.000000000000000000teste,3.000000000000000000testc,1.000000000000000000testa,4.000000000000000000testd,0.000000000000000000testb",
+			expected: "1.000000000000000000testa,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste",
+		},
+	}
+
+	for _, tt := range cases {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			undertest := NewDecCoins(tt.input...)
+			require.Equal(t, tt.expected, undertest.String(), "NewDecCoins must return expected results")
+			require.Equal(t, tt.original, tt.input.String(), "input must be unmodified and match original")
+		})
+	}
+}
+
 func TestIsValid(t *testing.T) {
 	tests := []struct {
 		coin       DecCoin