Merge PR #5449: Add New constructor for the DecCoin

CHANGELOG.md

@@ -50,6 +50,8 @@ logic has been implemented for v0.38 target version. Applications can migrate vi
 
 ### API Breaking Changes
 
+* (types) [\#5430](https://github.com/cosmos/cosmos-sdk/pull/5430) `DecCoins#Add` parameter changed from `DecCoins` 
+  to `...DecCoin`, `Coins#Add` parameter changed from `Coins` to `...Coin`
 * (baseapp/types) [\#5421](https://github.com/cosmos/cosmos-sdk/pull/5421) The `Error` interface (`types/errors.go`)
   has been removed in favor of the concrete type defined in `types/errors/` which implements the standard `error`
   interface. As a result, the `Handler` and `Querier` implementations now return a standard `error`.

simapp/export.go

@@ -91,7 +91,7 @@ func (app *SimApp) prepForZeroHeightGenesis(ctx sdk.Context, jailWhiteList []str
 		// donate any unwithdrawn outstanding reward fraction tokens to the community pool
 		scraps := app.DistrKeeper.GetValidatorOutstandingRewards(ctx, val.GetOperator())
 		feePool := app.DistrKeeper.GetFeePool(ctx)
-		feePool.CommunityPool = feePool.CommunityPool.Add(scraps)
+		feePool.CommunityPool = feePool.CommunityPool.Add(scraps...)
 		app.DistrKeeper.SetFeePool(ctx, feePool)
 
 		app.DistrKeeper.Hooks().AfterValidatorCreated(ctx, val.GetOperator())

simapp/genesis_account_test.go

@@ -82,7 +82,7 @@ func TestSimGenesisAccountValidate(t *testing.T) {
 			"valid basic account with invalid original vesting coins",
 			simapp.SimGenesisAccount{
 				BaseAccount:     baseAcc,
-				OriginalVesting: coins.Add(coins),
+				OriginalVesting: coins.Add(coins...),
 				StartTime:       vestingStart.Unix(),
 				EndTime:         vestingStart.Add(1 * time.Hour).Unix(),
 			},

simapp/test_helpers.go

@@ -88,7 +88,7 @@ func AddTestAddrs(app *SimApp, ctx sdk.Context, accNum int, accAmt sdk.Int) []sd
 	initCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), accAmt))
 	totalSupply := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), accAmt.MulRaw(int64(len(testAddrs)))))
 	prevSupply := app.SupplyKeeper.GetSupply(ctx)
-	app.SupplyKeeper.SetSupply(ctx, supply.NewSupply(prevSupply.GetTotal().Add(totalSupply)))
+	app.SupplyKeeper.SetSupply(ctx, supply.NewSupply(prevSupply.GetTotal().Add(totalSupply...)))
 
 	// fill all the addresses with some coins, set the loose pool tokens simultaneously
 	for _, addr := range testAddrs {

types/coin.go

@@ -244,7 +244,7 @@ func (coins Coins) IsValid() bool {
 //
 // CONTRACT: Add will never return Coins where one Coin has a non-positive
 // amount. In otherwords, IsValid will always return true.
-func (coins Coins) Add(coinsB Coins) Coins {
+func (coins Coins) Add(coinsB ...Coin) Coins {
 	return coins.safeAdd(coinsB)
 }
 
@@ -506,6 +506,11 @@ func (coins Coins) AmountOf(denom string) Int {
 	}
 }
 
+// GetDenomByIndex returns the Denom of the certain coin to make the findDup generic
+func (coins Coins) GetDenomByIndex(i int) string {
+	return coins[i].Denom
+}
+
 // IsAllPositive returns true if there is at least one coin and all currencies
 // have a positive value.
 func (coins Coins) IsAllPositive() bool {
@@ -669,18 +674,23 @@ func ParseCoins(coinsStr string) (Coins, error) {
 	return coins, nil
 }
 
+type findDupDescriptor interface {
+	GetDenomByIndex(int) string
+	Len() int
+}
+
 // findDup works on the assumption that coins is sorted
-func findDup(coins Coins) int {
-	if len(coins) <= 1 {
+func findDup(coins findDupDescriptor) int {
+	if coins.Len() <= 1 {
 		return -1
 	}
 
-	prevDenom := coins[0].Denom
-	for i := 1; i < len(coins); i++ {
-		if coins[i].Denom == prevDenom {
+	prevDenom := coins.GetDenomByIndex(0)
+	for i := 1; i < coins.Len(); i++ {
+		if coins.GetDenomByIndex(i) == prevDenom {
 			return i
 		}
-		prevDenom = coins[i].Denom
+		prevDenom = coins.GetDenomByIndex(i)
 	}
 
 	return -1

types/coin_benchmark_test.go

@@ -21,7 +21,7 @@ func BenchmarkCoinsAdditionIntersect(b *testing.B) {
 			b.ResetTimer()
 
 			for i := 0; i < b.N; i++ {
-				coinsA.Add(coinsB)
+				coinsA.Add(coinsB...)
 			}
 		}
 	}
@@ -50,7 +50,7 @@ func BenchmarkCoinsAdditionNoIntersect(b *testing.B) {
 			b.ResetTimer()
 
 			for i := 0; i < b.N; i++ {
-				coinsA.Add(coinsB)
+				coinsA.Add(coinsB...)
 			}
 		}
 	}

types/coin_test.go

@@ -251,7 +251,7 @@ func TestAddCoins(t *testing.T) {
 	}
 
 	for tcIndex, tc := range cases {
-		res := tc.inputOne.Add(tc.inputTwo)
+		res := tc.inputOne.Add(tc.inputTwo...)
 		assert.True(t, res.IsValid())
 		require.Equal(t, tc.expected, res, "sum of coins is incorrect, tc #%d", tcIndex)
 	}

types/dec_coin.go

@@ -157,9 +157,32 @@ func (coin DecCoin) IsValid() bool {
 // DecCoins defines a slice of coins with decimal values
 type DecCoins []DecCoin
 
-// NewDecCoins constructs a new coin set with decimal values
+// NewDecCoins constructs a new coin set with with decimal values
+// from DecCoins.
+func NewDecCoins(decCoins ...DecCoin) DecCoins {
+	// remove zeroes
+	newDecCoins := removeZeroDecCoins(DecCoins(decCoins))
+	if len(newDecCoins) == 0 {
+		return DecCoins{}
+	}
+
+	newDecCoins.Sort()
+
+	// detect duplicate Denoms
+	if dupIndex := findDup(newDecCoins); dupIndex != -1 {
+		panic(fmt.Errorf("find duplicate denom: %s", newDecCoins[dupIndex]))
+	}
+
+	if !newDecCoins.IsValid() {
+		panic(fmt.Errorf("invalid coin set: %s", newDecCoins))
+	}
+
+	return newDecCoins
+}
+
+// NewDecCoinsFromCoin constructs a new coin set with decimal values
 // from regular Coins.
-func NewDecCoins(coins Coins) DecCoins {
+func NewDecCoinsFromCoins(coins ...Coin) DecCoins {
 	decCoins := make(DecCoins, len(coins))
 	newCoins := NewCoins(coins...)
 	for i, coin := range newCoins {
@@ -191,10 +214,10 @@ func (coins DecCoins) TruncateDecimal() (truncatedCoins Coins, changeCoins DecCo
 	for _, coin := range coins {
 		truncated, change := coin.TruncateDecimal()
 		if !truncated.IsZero() {
-			truncatedCoins = truncatedCoins.Add(NewCoins(truncated))
+			truncatedCoins = truncatedCoins.Add(truncated)
 		}
 		if !change.IsZero() {
-			changeCoins = changeCoins.Add(DecCoins{change})
+			changeCoins = changeCoins.Add(change)
 		}
 	}
 
@@ -208,7 +231,7 @@ func (coins DecCoins) TruncateDecimal() (truncatedCoins Coins, changeCoins DecCo
 //
 // CONTRACT: Add will never return Coins where one Coin has a non-positive
 // amount. In otherwords, IsValid will always return true.
-func (coins DecCoins) Add(coinsB DecCoins) DecCoins {
+func (coins DecCoins) Add(coinsB ...DecCoin) DecCoins {
 	return coins.safeAdd(coinsB)
 }
 
@@ -311,6 +334,11 @@ func (coins DecCoins) Intersect(coinsB DecCoins) DecCoins {
 	return removeZeroDecCoins(res)
 }
 
+// GetDenomByIndex returns the Denom to make the findDup generic
+func (coins DecCoins) GetDenomByIndex(i int) string {
+	return coins[i].Denom
+}
+
 // IsAnyNegative returns true if there is at least one coin whose amount
 // is negative; returns false otherwise. It returns false if the DecCoins set
 // is empty too.
@@ -338,7 +366,7 @@ func (coins DecCoins) MulDec(d Dec) DecCoins {
 		}
 
 		if !product.IsZero() {
-			res = res.Add(DecCoins{product})
+			res = res.Add(product)
 		}
 	}
 
@@ -359,7 +387,7 @@ func (coins DecCoins) MulDecTruncate(d Dec) DecCoins {
 		}
 
 		if !product.IsZero() {
-			res = res.Add(DecCoins{product})
+			res = res.Add(product)
 		}
 	}
 
@@ -382,7 +410,7 @@ func (coins DecCoins) QuoDec(d Dec) DecCoins {
 		}
 
 		if !quotient.IsZero() {
-			res = res.Add(DecCoins{quotient})
+			res = res.Add(quotient)
 		}
 	}
 
@@ -406,7 +434,7 @@ func (coins DecCoins) QuoDecTruncate(d Dec) DecCoins {
 		}
 
 		if !quotient.IsZero() {
-			res = res.Add(DecCoins{quotient})
+			res = res.Add(quotient)
 		}
 	}
 

types/dec_coin_test.go

@@ -91,7 +91,7 @@ func TestAddDecCoins(t *testing.T) {
 	}
 
 	for tcIndex, tc := range cases {
-		res := tc.inputOne.Add(tc.inputTwo)
+		res := tc.inputOne.Add(tc.inputTwo...)
 		require.Equal(t, tc.expected, res, "sum of coins is incorrect, tc #%d", tcIndex)
 	}
 }
@@ -172,7 +172,7 @@ func TestSubDecCoins(t *testing.T) {
 		msg        string
 	}{
 		{
-			NewDecCoins(Coins{NewCoin("mytoken", NewInt(10)), NewCoin("btc", NewInt(20)), NewCoin("eth", NewInt(30))}),
+			NewDecCoinsFromCoins(NewCoin("mytoken", NewInt(10)), NewCoin("btc", NewInt(20)), NewCoin("eth", NewInt(30))),
 			true,
 			"sorted coins should have passed",
 		},
@@ -188,7 +188,7 @@ func TestSubDecCoins(t *testing.T) {
 		},
 	}
 
-	decCoins := NewDecCoins(Coins{NewCoin("btc", NewInt(10)), NewCoin("eth", NewInt(15)), NewCoin("mytoken", NewInt(5))})
+	decCoins := NewDecCoinsFromCoins(NewCoin("btc", NewInt(10)), NewCoin("eth", NewInt(15)), NewCoin("mytoken", NewInt(5)))
 
 	for _, tc := range tests {
 		tc := tc
@@ -421,3 +421,75 @@ func TestDecCoinsQuoDecTruncate(t *testing.T) {
 		}
 	}
 }
+
+func TestNewDecCoinsWithIsValid(t *testing.T) {
+	fake1 := append(NewDecCoins(NewDecCoin("mytoken", NewInt(10))), DecCoin{Denom: "BTC", Amount: NewDec(10)})
+	fake2 := append(NewDecCoins(NewDecCoin("mytoken", NewInt(10))), DecCoin{Denom: "BTC", Amount: NewDec(-10)})
+
+	tests := []struct {
+		coin       DecCoins
+		expectPass bool
+		msg        string
+	}{
+		{
+			NewDecCoins(NewDecCoin("mytoken", NewInt(10))),
+			true,
+			"valid coins should have passed",
+		},
+		{
+			fake1,
+			false,
+			"invalid denoms",
+		},
+		{
+			fake2,
+			false,
+			"negative amount",
+		},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		if tc.expectPass {
+			require.True(t, tc.coin.IsValid(), tc.msg)
+		} else {
+			require.False(t, tc.coin.IsValid(), tc.msg)
+		}
+	}
+}
+
+func TestDecCoins_AddDecCoinWithIsValid(t *testing.T) {
+	lengthTestDecCoins := NewDecCoins().Add(NewDecCoin("mytoken", NewInt(10))).Add(DecCoin{Denom: "BTC", Amount: NewDec(10)})
+	require.Equal(t, 2, len(lengthTestDecCoins), "should be 2")
+
+	tests := []struct {
+		coin       DecCoins
+		expectPass bool
+		msg        string
+	}{
+		{
+			NewDecCoins().Add(NewDecCoin("mytoken", NewInt(10))),
+			true,
+			"valid coins should have passed",
+		},
+		{
+			NewDecCoins().Add(NewDecCoin("mytoken", NewInt(10))).Add(DecCoin{Denom: "BTC", Amount: NewDec(10)}),
+			false,
+			"invalid denoms",
+		},
+		{
+			NewDecCoins().Add(NewDecCoin("mytoken", NewInt(10))).Add(DecCoin{Denom: "BTC", Amount: NewDec(-10)}),
+			false,
+			"negative amount",
+		},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		if tc.expectPass {
+			require.True(t, tc.coin.IsValid(), tc.msg)
+		} else {
+			require.False(t, tc.coin.IsValid(), tc.msg)
+		}
+	}
+}

x/auth/types/stdtx.go

@@ -203,7 +203,7 @@ func (fee StdFee) Bytes() []byte {
 // originally part of the submitted transaction because the fee is computed
 // as fee = ceil(gasWanted * gasPrices).
 func (fee StdFee) GasPrices() sdk.DecCoins {
-	return sdk.NewDecCoins(fee.Amount).QuoDec(sdk.NewDec(int64(fee.Gas)))
+	return sdk.NewDecCoinsFromCoins(fee.Amount...).QuoDec(sdk.NewDec(int64(fee.Gas)))
 }
 
 //__________________________________________________________

x/auth/vesting/types/genesis_test.go

@@ -25,7 +25,7 @@ func TestValidateGenesisInvalidAccounts(t *testing.T) {
 	baseVestingAcc, err := NewBaseVestingAccount(&acc1, acc1.Coins, 1548775410)
 	require.NoError(t, err)
 	// invalid delegated vesting
-	baseVestingAcc.DelegatedVesting = acc1.Coins.Add(acc1.Coins)
+	baseVestingAcc.DelegatedVesting = acc1.Coins.Add(acc1.Coins...)
 
 	acc2 := authtypes.NewBaseAccountWithAddress(sdk.AccAddress(addr2))
 	acc2.Coins = sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, 150))

x/auth/vesting/types/vesting_account.go

@@ -74,7 +74,7 @@ func (bva BaseVestingAccount) SpendableCoinsVestingAccount(vestingCoins sdk.Coin
 		spendableCoin := sdk.NewCoin(coin.Denom, min)
 
 		if !spendableCoin.IsZero() {
-			spendableCoins = spendableCoins.Add(sdk.Coins{spendableCoin})
+			spendableCoins = spendableCoins.Add(spendableCoin)
 		}
 	}
 
@@ -108,12 +108,12 @@ func (bva *BaseVestingAccount) TrackDelegation(vestingCoins, amount sdk.Coins) {
 
 		if !x.IsZero() {
 			xCoin := sdk.NewCoin(coin.Denom, x)
-			bva.DelegatedVesting = bva.DelegatedVesting.Add(sdk.Coins{xCoin})
+			bva.DelegatedVesting = bva.DelegatedVesting.Add(xCoin)
 		}
 
 		if !y.IsZero() {
 			yCoin := sdk.NewCoin(coin.Denom, y)
-			bva.DelegatedFree = bva.DelegatedFree.Add(sdk.Coins{yCoin})
+			bva.DelegatedFree = bva.DelegatedFree.Add(yCoin)
 		}
 	}
 }
@@ -543,7 +543,7 @@ func (pva PeriodicVestingAccount) GetVestedCoins(blockTime time.Time) sdk.Coins
 		if x < period.Length {
 			break
 		}
-		vestedCoins = vestedCoins.Add(period.Amount)
+		vestedCoins = vestedCoins.Add(period.Amount...)
 		// Update the start time of the next period
 		currentPeriodStartTime += period.Length
 	}
@@ -589,7 +589,7 @@ func (pva PeriodicVestingAccount) Validate() error {
 	originalVesting := sdk.NewCoins()
 	for _, p := range pva.VestingPeriods {
 		endTime += p.Length
-		originalVesting = originalVesting.Add(p.Amount)
+		originalVesting = originalVesting.Add(p.Amount...)
 	}
 	if endTime != pva.EndTime {
 		return errors.New("vesting end time does not match length of all vesting periods")