Merge PR cosmos#4058: Fix DecCoins Bugs

b00ris · Apr 5, 2019 · 576eb51 · 576eb51
1 parent 1a8ab1c
commit 576eb51
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,31 @@
 # Changelog
 
+* [0.34.0](#0340)
+  * [Breaking Changes](#breaking-changes)
+    * [Gaia](#gaia)
+    * [Gaia CLI](#gaia-cli)
+    * [SDK](#sdk)
+    * [Tendermint](#tendermint)
+  * [New features](#new-features)
+    * [SDK](#sdk-1)
+    * [Gaia](#gaia-1)
+    * [Gaia CLI](#gaia-cli-1)
+    * [Gaia REST API](#gaia-rest-api)
+  * [Improvements](#improvements)
+    * [Gaia](#gaia-2)
+    * [Gaia CLI](#gaia-cli-2)
+    * [SDK](#sdk-2)
+  * [Bug Fixes](#bug-fixes)
+    * [Gaia](#gaia-3)
+    * [Gaia CLI](#gaia-cli-3)
+    * [SDK](#sdk-3)
+* [0.33.2](#0332)
+  * [Improvements](#improvements-1)
+    * [Tendermint](#tendermint-1)
+* [0.33.1](#0331)
+  * [Bug Fixes](#bug-fixes-1)
+    * [Gaia](#gaia-4)
+
 ## 0.34.0
 
 ### Breaking Changes
@@ -116,6 +142,8 @@
 * [\#3977](https://github.com/cosmos/cosmos-sdk/issues/3977) Fix docker image build
 * [\#4020](https://github.com/cosmos/cosmos-sdk/issues/4020) Fix queryDelegationRewards by returning an error
 when the validator or delegation do not exist.
+* [\#4050](https://github.com/cosmos/cosmos-sdk/issues/4050) Fix DecCoins APIs
+where rounding or truncation could result in zero decimal coins.
 
 ## 0.33.2
 

diff --git a/types/dec_coin.go b/types/dec_coin.go
@@ -312,55 +312,93 @@ func (coins DecCoins) IsAnyNegative() bool {
 	return false
 }
 
-// multiply all the coins by a decimal
+// MulDec multiplies all the coins by a decimal.
+//
+// CONTRACT: No zero coins will be returned.
 func (coins DecCoins) MulDec(d Dec) DecCoins {
-	res := make([]DecCoin, len(coins))
-	for i, coin := range coins {
+	var res DecCoins
+	for _, coin := range coins {
 		product := DecCoin{
 			Denom:  coin.Denom,
 			Amount: coin.Amount.Mul(d),
 		}
-		res[i] = product
+
+		if !product.IsZero() {
+			res = res.Add(DecCoins{product})
+		}
 	}
+
 	return res
 }
 
-// multiply all the coins by a decimal, truncating
+// MulDecTruncate multiplies all the decimal coins by a decimal, truncating. It
+// panics if d is zero.
+//
+// CONTRACT: No zero coins will be returned.
 func (coins DecCoins) MulDecTruncate(d Dec) DecCoins {
-	res := make([]DecCoin, len(coins))
-	for i, coin := range coins {
+	if d.IsZero() {
+		panic("invalid zero decimal")
+	}
+
+	var res DecCoins
+	for _, coin := range coins {
 		product := DecCoin{
 			Denom:  coin.Denom,
 			Amount: coin.Amount.MulTruncate(d),
 		}
-		res[i] = product
+
+		if !product.IsZero() {
+			res = res.Add(DecCoins{product})
+		}
 	}
+
 	return res
 }
 
-// divide all the coins by a decimal
+// QuoDec divides all the decimal coins by a decimal. It panics if d is zero.
+//
+// CONTRACT: No zero coins will be returned.
 func (coins DecCoins) QuoDec(d Dec) DecCoins {
-	res := make([]DecCoin, len(coins))
-	for i, coin := range coins {
+	if d.IsZero() {
+		panic("invalid zero decimal")
+	}
+
+	var res DecCoins
+	for _, coin := range coins {
 		quotient := DecCoin{
 			Denom:  coin.Denom,
 			Amount: coin.Amount.Quo(d),
 		}
-		res[i] = quotient
+
+		if !quotient.IsZero() {
+			res = res.Add(DecCoins{quotient})
+		}
 	}
+
 	return res
 }
 
-// divide all the coins by a decimal, truncating
+// QuoDecTruncate divides all the decimal coins by a decimal, truncating. It
+// panics if d is zero.
+//
+// CONTRACT: No zero coins will be returned.
 func (coins DecCoins) QuoDecTruncate(d Dec) DecCoins {
-	res := make([]DecCoin, len(coins))
-	for i, coin := range coins {
+	if d.IsZero() {
+		panic("invalid zero decimal")
+	}
+
+	var res DecCoins
+	for _, coin := range coins {
 		quotient := DecCoin{
 			Denom:  coin.Denom,
 			Amount: coin.Amount.QuoTruncate(d),
 		}
-		res[i] = quotient
+
+		if !quotient.IsZero() {
+			res = res.Add(DecCoins{quotient})
+		}
 	}
+
 	return res
 }
 

diff --git a/types/dec_coin_test.go b/types/dec_coin_test.go
@@ -290,3 +290,28 @@ func TestDecCoinsTruncateDecimal(t *testing.T) {
 		)
 	}
 }
+
+func TestDecCoinsQuoDecTruncate(t *testing.T) {
+	x := MustNewDecFromStr("1.00")
+	y := MustNewDecFromStr("10000000000000000000.00")
+
+	testCases := []struct {
+		coins  DecCoins
+		input  Dec
+		result DecCoins
+		panics bool
+	}{
+		{DecCoins{}, ZeroDec(), DecCoins(nil), true},
+		{DecCoins{NewDecCoinFromDec("foo", x)}, y, DecCoins(nil), false},
+		{DecCoins{NewInt64DecCoin("foo", 5)}, NewDec(2), DecCoins{NewDecCoinFromDec("foo", MustNewDecFromStr("2.5"))}, false},
+	}
+
+	for i, tc := range testCases {
+		if tc.panics {
+			require.Panics(t, func() { tc.coins.QuoDecTruncate(tc.input) })
+		} else {
+			res := tc.coins.QuoDecTruncate(tc.input)
+			require.Equal(t, tc.result, res, "unexpected result; tc #%d, coins: %s, input: %s", i, tc.coins, tc.input)
+		}
+	}
+}