diff --git a/PENDING.md b/PENDING.md index f023471dec..e82bc08818 100644 --- a/PENDING.md +++ b/PENDING.md @@ -40,6 +40,8 @@ IMPROVEMENTS * Gaia * SDK + * [\#3311] Reconcile the `DecCoin/s` API with the `Coin/s` API. + * [\#3614] Add coin denom length checks to the coins constructors. * \#3621 remove many inter-module dependancies * [\#3601] JSON-stringify the ABCI log response which includes the log and message index. diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 7e64c114e6..c1fca6f919 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -256,7 +256,7 @@ func TestBaseAppOptionSeal(t *testing.T) { } func TestSetMinGasPrices(t *testing.T) { - minGasPrices := sdk.DecCoins{sdk.NewDecCoin("stake", 5000)} + minGasPrices := sdk.DecCoins{sdk.NewInt64DecCoin("stake", 5000)} app := newBaseApp(t.Name(), SetMinGasPrices(minGasPrices.String())) require.Equal(t, minGasPrices, app.minGasPrices) } diff --git a/server/config/config_test.go b/server/config/config_test.go index c2f8832756..e828a0716d 100644 --- a/server/config/config_test.go +++ b/server/config/config_test.go @@ -15,6 +15,6 @@ func TestDefaultConfig(t *testing.T) { func TestSetMinimumFees(t *testing.T) { cfg := DefaultConfig() - cfg.SetMinGasPrices(sdk.DecCoins{sdk.NewDecCoin("foo", 5)}) + cfg.SetMinGasPrices(sdk.DecCoins{sdk.NewInt64DecCoin("foo", 5)}) require.Equal(t, "5.000000000000000000foo", cfg.MinGasPrices) } diff --git a/types/coin.go b/types/coin.go index 675ddbb290..f969d2c434 100644 --- a/types/coin.go +++ b/types/coin.go @@ -27,12 +27,11 @@ type Coin struct { // NewCoin returns a new coin with a denomination and amount. It will panic if // the amount is negative. func NewCoin(denom string, amount Int) Coin { + validateDenom(denom) + if amount.LT(ZeroInt()) { panic(fmt.Sprintf("negative coin amount: %v\n", amount)) } - if strings.ToLower(denom) != denom { - panic(fmt.Sprintf("denom cannot contain upper case characters: %s\n", denom)) - } return Coin{ Denom: denom, @@ -51,11 +50,6 @@ func (coin Coin) String() string { return fmt.Sprintf("%v%v", coin.Amount, coin.Denom) } -// SameDenomAs returns true if the two coins are the same denom -func (coin Coin) SameDenomAs(other Coin) bool { - return (coin.Denom == other.Denom) -} - // IsZero returns if this represents no money func (coin Coin) IsZero() bool { return coin.Amount.IsZero() @@ -64,24 +58,36 @@ func (coin Coin) IsZero() bool { // IsGTE returns true if they are the same type and the receiver is // an equal or greater value func (coin Coin) IsGTE(other Coin) bool { - return coin.SameDenomAs(other) && (!coin.Amount.LT(other.Amount)) + if coin.Denom != other.Denom { + panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, other.Denom)) + } + + return !coin.Amount.LT(other.Amount) } // IsLT returns true if they are the same type and the receiver is // a smaller value func (coin Coin) IsLT(other Coin) bool { - return coin.SameDenomAs(other) && coin.Amount.LT(other.Amount) + if coin.Denom != other.Denom { + panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, other.Denom)) + } + + return coin.Amount.LT(other.Amount) } // IsEqual returns true if the two sets of Coins have the same value func (coin Coin) IsEqual(other Coin) bool { - return coin.SameDenomAs(other) && (coin.Amount.Equal(other.Amount)) + if coin.Denom != other.Denom { + panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, other.Denom)) + } + + return coin.Amount.Equal(other.Amount) } // Adds amounts of two coins with same denom. If the coins differ in denom then // it panics. func (coin Coin) Plus(coinB Coin) Coin { - if !coin.SameDenomAs(coinB) { + if coin.Denom != coinB.Denom { panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, coinB.Denom)) } @@ -91,7 +97,7 @@ func (coin Coin) Plus(coinB Coin) Coin { // Subtracts amounts of two coins with same denom. If the coins differ in denom // then it panics. func (coin Coin) Minus(coinB Coin) Coin { - if !coin.SameDenomAs(coinB) { + if coin.Denom != coinB.Denom { panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, coinB.Denom)) } @@ -273,7 +279,7 @@ func (coins Coins) IsAllGT(coinsB Coins) bool { return false } - return diff.IsPositive() + return diff.IsAllPositive() } // IsAllGTE returns true iff for every denom in coins, the denom is present at @@ -339,7 +345,7 @@ func (coins Coins) IsEqual(coinsB Coins) bool { coinsB = coinsB.Sort() for i := 0; i < len(coins); i++ { - if coins[i].Denom != coinsB[i].Denom || !coins[i].Amount.Equal(coinsB[i].Amount) { + if !coins[i].IsEqual(coinsB[i]) { return false } } @@ -354,9 +360,8 @@ func (coins Coins) Empty() bool { // Returns the amount of a denom from coins func (coins Coins) AmountOf(denom string) Int { - if strings.ToLower(denom) != denom { - panic(fmt.Sprintf("denom cannot contain upper case characters: %s\n", denom)) - } + validateDenom(denom) + switch len(coins) { case 0: return ZeroInt() @@ -382,11 +387,11 @@ func (coins Coins) AmountOf(denom string) Int { } } -// IsPositive returns true if there is at least one coin and all currencies +// IsAllPositive returns true if there is at least one coin and all currencies // have a positive value. // // TODO: Remove once unsigned integers are used. -func (coins Coins) IsPositive() bool { +func (coins Coins) IsAllPositive() bool { if len(coins) == 0 { return false } @@ -403,12 +408,9 @@ func (coins Coins) IsPositive() bool { // IsAnyNegative returns true if there is at least one coin whose amount // is negative; returns false otherwise. It returns false if the coin set // is empty too. +// // TODO: Remove once unsigned integers are used. func (coins Coins) IsAnyNegative() bool { - if len(coins) == 0 { - return false - } - for _, coin := range coins { if coin.IsNegative() { return true @@ -485,6 +487,15 @@ var ( reDecCoin = regexp.MustCompile(fmt.Sprintf(`^(%s)%s(%s)$`, reDecAmt, reSpc, reDnm)) ) +func validateDenom(denom string) { + if len(denom) < 3 || len(denom) > 16 { + panic(fmt.Sprintf("invalid denom length: %s", denom)) + } + if strings.ToLower(denom) != denom { + panic(fmt.Sprintf("denom cannot contain upper case characters: %s", denom)) + } +} + // ParseCoin parses a cli input for one coin type, returning errors if invalid. // This returns an error on an empty string as well. func ParseCoin(coinStr string) (coin Coin, err error) { diff --git a/types/coin_test.go b/types/coin_test.go index 277c29c46b..3bbc497f68 100644 --- a/types/coin_test.go +++ b/types/coin_test.go @@ -1,39 +1,28 @@ package types import ( + "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +var ( + testDenom1 = "atom" + testDenom2 = "muon" +) + // ---------------------------------------------------------------------------- // Coin tests func TestCoin(t *testing.T) { - require.Panics(t, func() { NewInt64Coin("a", -1) }) - require.Panics(t, func() { NewCoin("a", NewInt(-1)) }) - require.Panics(t, func() { NewInt64Coin("Atom", 10) }) - require.Panics(t, func() { NewCoin("Atom", NewInt(10)) }) - require.Equal(t, NewInt(5), NewInt64Coin("a", 5).Amount) - require.Equal(t, NewInt(5), NewCoin("a", NewInt(5)).Amount) -} - -func TestSameDenomAsCoin(t *testing.T) { - cases := []struct { - inputOne Coin - inputTwo Coin - expected bool - }{ - {NewInt64Coin("a", 1), NewInt64Coin("a", 1), true}, - {NewInt64Coin("a", 1), NewInt64Coin("b", 1), false}, - {NewInt64Coin("steak", 1), NewInt64Coin("steak", 10), true}, - } - - for tcIndex, tc := range cases { - res := tc.inputOne.SameDenomAs(tc.inputTwo) - require.Equal(t, tc.expected, res, "coin denominations didn't match, tc #%d", tcIndex) - } + require.Panics(t, func() { NewInt64Coin(testDenom1, -1) }) + require.Panics(t, func() { NewCoin(testDenom1, NewInt(-1)) }) + require.Panics(t, func() { NewInt64Coin(strings.ToUpper(testDenom1), 10) }) + require.Panics(t, func() { NewCoin(strings.ToUpper(testDenom1), NewInt(10)) }) + require.Equal(t, NewInt(5), NewInt64Coin(testDenom1, 5).Amount) + require.Equal(t, NewInt(5), NewCoin(testDenom1, NewInt(5)).Amount) } func TestIsEqualCoin(t *testing.T) { @@ -41,15 +30,20 @@ func TestIsEqualCoin(t *testing.T) { inputOne Coin inputTwo Coin expected bool + panics bool }{ - {NewInt64Coin("a", 1), NewInt64Coin("a", 1), true}, - {NewInt64Coin("a", 1), NewInt64Coin("b", 1), false}, - {NewInt64Coin("steak", 1), NewInt64Coin("steak", 10), false}, + {NewInt64Coin(testDenom1, 1), NewInt64Coin(testDenom1, 1), true, false}, + {NewInt64Coin(testDenom1, 1), NewInt64Coin(testDenom2, 1), false, true}, + {NewInt64Coin("steak", 1), NewInt64Coin("steak", 10), false, false}, } for tcIndex, tc := range cases { - res := tc.inputOne.IsEqual(tc.inputTwo) - require.Equal(t, tc.expected, res, "coin equality relation is incorrect, tc #%d", tcIndex) + if tc.panics { + require.Panics(t, func() { tc.inputOne.IsEqual(tc.inputTwo) }) + } else { + res := tc.inputOne.IsEqual(tc.inputTwo) + require.Equal(t, tc.expected, res, "coin equality relation is incorrect, tc #%d", tcIndex) + } } } @@ -60,9 +54,9 @@ func TestPlusCoin(t *testing.T) { expected Coin shouldPanic bool }{ - {NewInt64Coin("a", 1), NewInt64Coin("a", 1), NewInt64Coin("a", 2), false}, - {NewInt64Coin("a", 1), NewInt64Coin("a", 0), NewInt64Coin("a", 1), false}, - {NewInt64Coin("a", 1), NewInt64Coin("b", 1), NewInt64Coin("a", 1), true}, + {NewInt64Coin(testDenom1, 1), NewInt64Coin(testDenom1, 1), NewInt64Coin(testDenom1, 2), false}, + {NewInt64Coin(testDenom1, 1), NewInt64Coin(testDenom1, 0), NewInt64Coin(testDenom1, 1), false}, + {NewInt64Coin(testDenom1, 1), NewInt64Coin(testDenom2, 1), NewInt64Coin(testDenom1, 1), true}, } for tcIndex, tc := range cases { @@ -82,11 +76,11 @@ func TestMinusCoin(t *testing.T) { expected Coin shouldPanic bool }{ - {NewInt64Coin("a", 1), NewInt64Coin("b", 1), NewInt64Coin("a", 1), true}, - {NewInt64Coin("a", 10), NewInt64Coin("a", 1), NewInt64Coin("a", 9), false}, - {NewInt64Coin("a", 5), NewInt64Coin("a", 3), NewInt64Coin("a", 2), false}, - {NewInt64Coin("a", 5), NewInt64Coin("a", 0), NewInt64Coin("a", 5), false}, - {NewInt64Coin("a", 1), NewInt64Coin("a", 5), Coin{}, true}, + {NewInt64Coin(testDenom1, 1), NewInt64Coin(testDenom2, 1), NewInt64Coin(testDenom1, 1), true}, + {NewInt64Coin(testDenom1, 10), NewInt64Coin(testDenom1, 1), NewInt64Coin(testDenom1, 9), false}, + {NewInt64Coin(testDenom1, 5), NewInt64Coin(testDenom1, 3), NewInt64Coin(testDenom1, 2), false}, + {NewInt64Coin(testDenom1, 5), NewInt64Coin(testDenom1, 0), NewInt64Coin(testDenom1, 5), false}, + {NewInt64Coin(testDenom1, 1), NewInt64Coin(testDenom1, 5), Coin{}, true}, } for tcIndex, tc := range cases { @@ -102,7 +96,7 @@ func TestMinusCoin(t *testing.T) { inputOne Coin inputTwo Coin expected int64 - }{NewInt64Coin("a", 1), NewInt64Coin("a", 1), 0} + }{NewInt64Coin(testDenom1, 1), NewInt64Coin(testDenom1, 1), 0} res := tc.inputOne.Minus(tc.inputTwo) require.Equal(t, tc.expected, res.Amount.Int64()) } @@ -112,15 +106,20 @@ func TestIsGTECoin(t *testing.T) { inputOne Coin inputTwo Coin expected bool + panics bool }{ - {NewInt64Coin("a", 1), NewInt64Coin("a", 1), true}, - {NewInt64Coin("a", 2), NewInt64Coin("a", 1), true}, - {NewInt64Coin("a", 1), NewInt64Coin("b", 1), false}, + {NewInt64Coin(testDenom1, 1), NewInt64Coin(testDenom1, 1), true, false}, + {NewInt64Coin(testDenom1, 2), NewInt64Coin(testDenom1, 1), true, false}, + {NewInt64Coin(testDenom1, 1), NewInt64Coin(testDenom2, 1), false, true}, } for tcIndex, tc := range cases { - res := tc.inputOne.IsGTE(tc.inputTwo) - require.Equal(t, tc.expected, res, "coin GTE relation is incorrect, tc #%d", tcIndex) + if tc.panics { + require.Panics(t, func() { tc.inputOne.IsGTE(tc.inputTwo) }) + } else { + res := tc.inputOne.IsGTE(tc.inputTwo) + require.Equal(t, tc.expected, res, "coin GTE relation is incorrect, tc #%d", tcIndex) + } } } @@ -129,27 +128,32 @@ func TestIsLTCoin(t *testing.T) { inputOne Coin inputTwo Coin expected bool + panics bool }{ - {NewInt64Coin("a", 1), NewInt64Coin("a", 1), false}, - {NewInt64Coin("a", 2), NewInt64Coin("a", 1), false}, - {NewInt64Coin("a", 0), NewInt64Coin("b", 1), false}, - {NewInt64Coin("a", 1), NewInt64Coin("b", 1), false}, - {NewInt64Coin("a", 1), NewInt64Coin("a", 1), false}, - {NewInt64Coin("a", 1), NewInt64Coin("a", 2), true}, + {NewInt64Coin(testDenom1, 1), NewInt64Coin(testDenom1, 1), false, false}, + {NewInt64Coin(testDenom1, 2), NewInt64Coin(testDenom1, 1), false, false}, + {NewInt64Coin(testDenom1, 0), NewInt64Coin(testDenom2, 1), false, true}, + {NewInt64Coin(testDenom1, 1), NewInt64Coin(testDenom2, 1), false, true}, + {NewInt64Coin(testDenom1, 1), NewInt64Coin(testDenom1, 1), false, false}, + {NewInt64Coin(testDenom1, 1), NewInt64Coin(testDenom1, 2), true, false}, } for tcIndex, tc := range cases { - res := tc.inputOne.IsLT(tc.inputTwo) - require.Equal(t, tc.expected, res, "coin LT relation is incorrect, tc #%d", tcIndex) + if tc.panics { + require.Panics(t, func() { tc.inputOne.IsLT(tc.inputTwo) }) + } else { + res := tc.inputOne.IsLT(tc.inputTwo) + require.Equal(t, tc.expected, res, "coin LT relation is incorrect, tc #%d", tcIndex) + } } } func TestCoinIsZero(t *testing.T) { - coin := NewInt64Coin("a", 0) + coin := NewInt64Coin(testDenom1, 0) res := coin.IsZero() require.True(t, res) - coin = NewInt64Coin("a", 1) + coin = NewInt64Coin(testDenom1, 1) res = coin.IsZero() require.False(t, res) } @@ -163,10 +167,10 @@ func TestIsZeroCoins(t *testing.T) { expected bool }{ {Coins{}, true}, - {Coins{NewInt64Coin("a", 0)}, true}, - {Coins{NewInt64Coin("a", 0), NewInt64Coin("b", 0)}, true}, - {Coins{NewInt64Coin("a", 1)}, false}, - {Coins{NewInt64Coin("a", 0), NewInt64Coin("b", 1)}, false}, + {Coins{NewInt64Coin(testDenom1, 0)}, true}, + {Coins{NewInt64Coin(testDenom1, 0), NewInt64Coin(testDenom2, 0)}, true}, + {Coins{NewInt64Coin(testDenom1, 1)}, false}, + {Coins{NewInt64Coin(testDenom1, 0), NewInt64Coin(testDenom2, 1)}, false}, } for _, tc := range cases { @@ -180,19 +184,24 @@ func TestEqualCoins(t *testing.T) { inputOne Coins inputTwo Coins expected bool + panics bool }{ - {Coins{}, Coins{}, true}, - {Coins{NewInt64Coin("a", 0)}, Coins{NewInt64Coin("a", 0)}, true}, - {Coins{NewInt64Coin("a", 0), NewInt64Coin("b", 1)}, Coins{NewInt64Coin("a", 0), NewInt64Coin("b", 1)}, true}, - {Coins{NewInt64Coin("a", 0)}, Coins{NewInt64Coin("b", 0)}, false}, - {Coins{NewInt64Coin("a", 0)}, Coins{NewInt64Coin("a", 1)}, false}, - {Coins{NewInt64Coin("a", 0)}, Coins{NewInt64Coin("a", 0), NewInt64Coin("b", 1)}, false}, - {Coins{NewInt64Coin("a", 0), NewInt64Coin("b", 1)}, Coins{NewInt64Coin("b", 1), NewInt64Coin("a", 0)}, true}, + {Coins{}, Coins{}, true, false}, + {Coins{NewInt64Coin(testDenom1, 0)}, Coins{NewInt64Coin(testDenom1, 0)}, true, false}, + {Coins{NewInt64Coin(testDenom1, 0), NewInt64Coin(testDenom2, 1)}, Coins{NewInt64Coin(testDenom1, 0), NewInt64Coin(testDenom2, 1)}, true, false}, + {Coins{NewInt64Coin(testDenom1, 0)}, Coins{NewInt64Coin(testDenom2, 0)}, false, true}, + {Coins{NewInt64Coin(testDenom1, 0)}, Coins{NewInt64Coin(testDenom1, 1)}, false, false}, + {Coins{NewInt64Coin(testDenom1, 0)}, Coins{NewInt64Coin(testDenom1, 0), NewInt64Coin(testDenom2, 1)}, false, false}, + {Coins{NewInt64Coin(testDenom1, 0), NewInt64Coin(testDenom2, 1)}, Coins{NewInt64Coin(testDenom1, 0), NewInt64Coin(testDenom2, 1)}, true, false}, } for tcnum, tc := range cases { - res := tc.inputOne.IsEqual(tc.inputTwo) - require.Equal(t, tc.expected, res, "Equality is differed from expected. tc #%d, expected %b, actual %b.", tcnum, tc.expected, res) + if tc.panics { + require.Panics(t, func() { tc.inputOne.IsEqual(tc.inputTwo) }) + } else { + res := tc.inputOne.IsEqual(tc.inputTwo) + require.Equal(t, tc.expected, res, "Equality is differed from expected. tc #%d, expected %b, actual %b.", tcnum, tc.expected, res) + } } } @@ -206,11 +215,11 @@ func TestPlusCoins(t *testing.T) { inputTwo Coins expected Coins }{ - {Coins{{"a", one}, {"b", one}}, Coins{{"a", one}, {"b", one}}, Coins{{"a", two}, {"b", two}}}, - {Coins{{"a", zero}, {"b", one}}, Coins{{"a", zero}, {"b", zero}}, Coins{{"b", one}}}, - {Coins{{"a", two}}, Coins{{"b", zero}}, Coins{{"a", two}}}, - {Coins{{"a", one}}, Coins{{"a", one}, {"b", two}}, Coins{{"a", two}, {"b", two}}}, - {Coins{{"a", zero}, {"b", zero}}, Coins{{"a", zero}, {"b", zero}}, Coins(nil)}, + {Coins{{testDenom1, one}, {testDenom2, one}}, Coins{{testDenom1, one}, {testDenom2, one}}, Coins{{testDenom1, two}, {testDenom2, two}}}, + {Coins{{testDenom1, zero}, {testDenom2, one}}, Coins{{testDenom1, zero}, {testDenom2, zero}}, Coins{{testDenom2, one}}}, + {Coins{{testDenom1, two}}, Coins{{testDenom2, zero}}, Coins{{testDenom1, two}}}, + {Coins{{testDenom1, one}}, Coins{{testDenom1, one}, {testDenom2, two}}, Coins{{testDenom1, two}, {testDenom2, two}}}, + {Coins{{testDenom1, zero}, {testDenom2, zero}}, Coins{{testDenom1, zero}, {testDenom2, zero}}, Coins(nil)}, } for tcIndex, tc := range cases { @@ -231,11 +240,11 @@ func TestMinusCoins(t *testing.T) { expected Coins shouldPanic bool }{ - {Coins{{"a", two}}, Coins{{"a", one}, {"b", two}}, Coins{{"a", one}, {"b", two}}, true}, - {Coins{{"a", two}}, Coins{{"b", zero}}, Coins{{"a", two}}, false}, - {Coins{{"a", one}}, Coins{{"b", zero}}, Coins{{"a", one}}, false}, - {Coins{{"a", one}, {"b", one}}, Coins{{"a", one}}, Coins{{"b", one}}, false}, - {Coins{{"a", one}, {"b", one}}, Coins{{"a", two}}, Coins{}, true}, + {Coins{{testDenom1, two}}, Coins{{testDenom1, one}, {testDenom2, two}}, Coins{{testDenom1, one}, {testDenom2, two}}, true}, + {Coins{{testDenom1, two}}, Coins{{testDenom2, zero}}, Coins{{testDenom1, two}}, false}, + {Coins{{testDenom1, one}}, Coins{{testDenom2, zero}}, Coins{{testDenom1, one}}, false}, + {Coins{{testDenom1, one}, {testDenom2, one}}, Coins{{testDenom1, one}}, Coins{{testDenom2, one}}, false}, + {Coins{{testDenom1, one}, {testDenom2, one}}, Coins{{testDenom1, two}}, Coins{}, true}, } for i, tc := range testCases { @@ -302,8 +311,8 @@ func TestCoins(t *testing.T) { assert.False(t, mixedCase1.IsValid(), "Coins denoms contain upper case characters") assert.False(t, mixedCase2.IsValid(), "First Coins denoms contain upper case characters") assert.False(t, mixedCase3.IsValid(), "Single denom in Coins contains upper case characters") - assert.True(t, good.IsPositive(), "Expected coins to be positive: %v", good) - assert.False(t, null.IsPositive(), "Expected coins to not be positive: %v", null) + assert.True(t, good.IsAllPositive(), "Expected coins to be positive: %v", good) + assert.False(t, null.IsAllPositive(), "Expected coins to not be positive: %v", null) assert.True(t, good.IsAllGTE(empty), "Expected %v to be >= %v", good, empty) assert.False(t, good.IsAllLT(empty), "Expected %v to be < %v", good, empty) assert.True(t, empty.IsAllLT(good), "Expected %v to be < %v", empty, good) @@ -319,11 +328,11 @@ func TestCoinsGT(t *testing.T) { two := NewInt(2) assert.False(t, Coins{}.IsAllGT(Coins{})) - assert.True(t, Coins{{"a", one}}.IsAllGT(Coins{})) - assert.False(t, Coins{{"a", one}}.IsAllGT(Coins{{"a", one}})) - assert.False(t, Coins{{"a", one}}.IsAllGT(Coins{{"b", one}})) - assert.True(t, Coins{{"a", one}, {"b", one}}.IsAllGT(Coins{{"b", one}})) - assert.False(t, Coins{{"a", one}, {"b", one}}.IsAllGT(Coins{{"b", two}})) + assert.True(t, Coins{{testDenom1, one}}.IsAllGT(Coins{})) + assert.False(t, Coins{{testDenom1, one}}.IsAllGT(Coins{{testDenom1, one}})) + assert.False(t, Coins{{testDenom1, one}}.IsAllGT(Coins{{testDenom2, one}})) + assert.True(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllGT(Coins{{testDenom2, one}})) + assert.False(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllGT(Coins{{testDenom2, two}})) } func TestCoinsGTE(t *testing.T) { @@ -331,11 +340,11 @@ func TestCoinsGTE(t *testing.T) { two := NewInt(2) assert.True(t, Coins{}.IsAllGTE(Coins{})) - assert.True(t, Coins{{"a", one}}.IsAllGTE(Coins{})) - assert.True(t, Coins{{"a", one}}.IsAllGTE(Coins{{"a", one}})) - assert.False(t, Coins{{"a", one}}.IsAllGTE(Coins{{"b", one}})) - assert.True(t, Coins{{"a", one}, {"b", one}}.IsAllGTE(Coins{{"b", one}})) - assert.False(t, Coins{{"a", one}, {"b", one}}.IsAllGTE(Coins{{"b", two}})) + assert.True(t, Coins{{testDenom1, one}}.IsAllGTE(Coins{})) + assert.True(t, Coins{{testDenom1, one}}.IsAllGTE(Coins{{testDenom1, one}})) + assert.False(t, Coins{{testDenom1, one}}.IsAllGTE(Coins{{testDenom2, one}})) + assert.True(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllGTE(Coins{{testDenom2, one}})) + assert.False(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllGTE(Coins{{testDenom2, two}})) } func TestCoinsLT(t *testing.T) { @@ -343,14 +352,14 @@ func TestCoinsLT(t *testing.T) { two := NewInt(2) assert.False(t, Coins{}.IsAllLT(Coins{})) - assert.False(t, Coins{{"a", one}}.IsAllLT(Coins{})) - assert.False(t, Coins{{"a", one}}.IsAllLT(Coins{{"a", one}})) - assert.False(t, Coins{{"a", one}}.IsAllLT(Coins{{"b", one}})) - assert.False(t, Coins{{"a", one}, {"b", one}}.IsAllLT(Coins{{"b", one}})) - assert.False(t, Coins{{"a", one}, {"b", one}}.IsAllLT(Coins{{"b", two}})) - assert.False(t, Coins{{"a", one}, {"b", one}}.IsAllLT(Coins{{"a", one}, {"b", one}})) - assert.True(t, Coins{{"a", one}, {"b", one}}.IsAllLT(Coins{{"a", one}, {"b", two}})) - assert.True(t, Coins{}.IsAllLT(Coins{{"a", one}})) + assert.False(t, Coins{{testDenom1, one}}.IsAllLT(Coins{})) + assert.False(t, Coins{{testDenom1, one}}.IsAllLT(Coins{{testDenom1, one}})) + assert.False(t, Coins{{testDenom1, one}}.IsAllLT(Coins{{testDenom2, one}})) + assert.False(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllLT(Coins{{testDenom2, one}})) + assert.False(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllLT(Coins{{testDenom2, two}})) + assert.False(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllLT(Coins{{testDenom1, one}, {testDenom2, one}})) + assert.True(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllLT(Coins{{testDenom1, one}, {testDenom2, two}})) + assert.True(t, Coins{}.IsAllLT(Coins{{testDenom1, one}})) } func TestCoinsLTE(t *testing.T) { @@ -358,14 +367,14 @@ func TestCoinsLTE(t *testing.T) { two := NewInt(2) assert.True(t, Coins{}.IsAllLTE(Coins{})) - assert.False(t, Coins{{"a", one}}.IsAllLTE(Coins{})) - assert.True(t, Coins{{"a", one}}.IsAllLTE(Coins{{"a", one}})) - assert.False(t, Coins{{"a", one}}.IsAllLTE(Coins{{"b", one}})) - assert.False(t, Coins{{"a", one}, {"b", one}}.IsAllLTE(Coins{{"b", one}})) - assert.False(t, Coins{{"a", one}, {"b", one}}.IsAllLTE(Coins{{"b", two}})) - assert.True(t, Coins{{"a", one}, {"b", one}}.IsAllLTE(Coins{{"a", one}, {"b", one}})) - assert.True(t, Coins{{"a", one}, {"b", one}}.IsAllLTE(Coins{{"a", one}, {"b", two}})) - assert.True(t, Coins{}.IsAllLTE(Coins{{"a", one}})) + assert.False(t, Coins{{testDenom1, one}}.IsAllLTE(Coins{})) + assert.True(t, Coins{{testDenom1, one}}.IsAllLTE(Coins{{testDenom1, one}})) + assert.False(t, Coins{{testDenom1, one}}.IsAllLTE(Coins{{testDenom2, one}})) + assert.False(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllLTE(Coins{{testDenom2, one}})) + assert.False(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllLTE(Coins{{testDenom2, two}})) + assert.True(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllLTE(Coins{{testDenom1, one}, {testDenom2, one}})) + assert.True(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllLTE(Coins{{testDenom1, one}, {testDenom2, two}})) + assert.True(t, Coins{}.IsAllLTE(Coins{{testDenom1, one}})) } func TestParse(t *testing.T) { @@ -452,30 +461,18 @@ func TestSortCoins(t *testing.T) { func TestAmountOf(t *testing.T) { case0 := Coins{} case1 := Coins{ - NewInt64Coin("", 0), - } - case2 := Coins{ - NewInt64Coin(" ", 0), - } - case3 := Coins{ NewInt64Coin("gold", 0), } - case4 := Coins{ + case2 := Coins{ NewInt64Coin("gas", 1), NewInt64Coin("mineral", 1), NewInt64Coin("tree", 1), } - case5 := Coins{ + case3 := Coins{ NewInt64Coin("mineral", 1), NewInt64Coin("tree", 1), } - case6 := Coins{ - NewInt64Coin("", 6), - } - case7 := Coins{ - NewInt64Coin(" ", 7), - } - case8 := Coins{ + case4 := Coins{ NewInt64Coin("gas", 8), } @@ -489,18 +486,12 @@ func TestAmountOf(t *testing.T) { }{ {case0, 0, 0, 0, 0, 0}, {case1, 0, 0, 0, 0, 0}, - {case2, 0, 0, 0, 0, 0}, - {case3, 0, 0, 0, 0, 0}, - {case4, 0, 0, 1, 1, 1}, - {case5, 0, 0, 0, 1, 1}, - {case6, 6, 0, 0, 0, 0}, - {case7, 0, 7, 0, 0, 0}, - {case8, 0, 0, 8, 0, 0}, + {case2, 0, 0, 1, 1, 1}, + {case3, 0, 0, 0, 1, 1}, + {case4, 0, 0, 8, 0, 0}, } for _, tc := range cases { - assert.Equal(t, NewInt(tc.amountOf), tc.coins.AmountOf("")) - assert.Equal(t, NewInt(tc.amountOfSpace), tc.coins.AmountOf(" ")) assert.Equal(t, NewInt(tc.amountOfGAS), tc.coins.AmountOf("gas")) assert.Equal(t, NewInt(tc.amountOfMINERAL), tc.coins.AmountOf("mineral")) assert.Equal(t, NewInt(tc.amountOfTREE), tc.coins.AmountOf("tree")) @@ -514,17 +505,17 @@ func TestCoinsIsAnyGTE(t *testing.T) { two := NewInt(2) assert.False(t, Coins{}.IsAnyGTE(Coins{})) - assert.False(t, Coins{{"a", one}}.IsAnyGTE(Coins{})) - assert.False(t, Coins{}.IsAnyGTE(Coins{{"a", one}})) - assert.False(t, Coins{{"a", one}}.IsAnyGTE(Coins{{"a", two}})) - assert.False(t, Coins{{"a", one}}.IsAnyGTE(Coins{{"b", one}})) - assert.True(t, Coins{{"a", one}, {"b", two}}.IsAnyGTE(Coins{{"a", two}, {"b", one}})) - assert.True(t, Coins{{"a", one}}.IsAnyGTE(Coins{{"a", one}})) - assert.True(t, Coins{{"a", two}}.IsAnyGTE(Coins{{"a", one}})) - assert.True(t, Coins{{"a", one}}.IsAnyGTE(Coins{{"a", one}, {"b", two}})) - assert.True(t, Coins{{"b", two}}.IsAnyGTE(Coins{{"a", one}, {"b", two}})) - assert.False(t, Coins{{"b", one}}.IsAnyGTE(Coins{{"a", one}, {"b", two}})) - assert.True(t, Coins{{"a", one}, {"b", two}}.IsAnyGTE(Coins{{"a", one}, {"b", one}})) - assert.True(t, Coins{{"a", one}, {"b", one}}.IsAnyGTE(Coins{{"a", one}, {"b", two}})) - assert.True(t, Coins{{"x", one}, {"y", one}}.IsAnyGTE(Coins{{"b", one}, {"c", one}, {"y", one}, {"z", one}})) + assert.False(t, Coins{{testDenom1, one}}.IsAnyGTE(Coins{})) + assert.False(t, Coins{}.IsAnyGTE(Coins{{testDenom1, one}})) + assert.False(t, Coins{{testDenom1, one}}.IsAnyGTE(Coins{{testDenom1, two}})) + assert.False(t, Coins{{testDenom1, one}}.IsAnyGTE(Coins{{testDenom2, one}})) + assert.True(t, Coins{{testDenom1, one}, {testDenom2, two}}.IsAnyGTE(Coins{{testDenom1, two}, {testDenom2, one}})) + assert.True(t, Coins{{testDenom1, one}}.IsAnyGTE(Coins{{testDenom1, one}})) + assert.True(t, Coins{{testDenom1, two}}.IsAnyGTE(Coins{{testDenom1, one}})) + assert.True(t, Coins{{testDenom1, one}}.IsAnyGTE(Coins{{testDenom1, one}, {testDenom2, two}})) + assert.True(t, Coins{{testDenom2, two}}.IsAnyGTE(Coins{{testDenom1, one}, {testDenom2, two}})) + assert.False(t, Coins{{testDenom2, one}}.IsAnyGTE(Coins{{testDenom1, one}, {testDenom2, two}})) + assert.True(t, Coins{{testDenom1, one}, {testDenom2, two}}.IsAnyGTE(Coins{{testDenom1, one}, {testDenom2, one}})) + assert.True(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAnyGTE(Coins{{testDenom1, one}, {testDenom2, two}})) + assert.True(t, Coins{{"xxx", one}, {"yyy", one}}.IsAnyGTE(Coins{{testDenom2, one}, {"ccc", one}, {"yyy", one}, {"zzz", one}})) } diff --git a/types/context_test.go b/types/context_test.go index 7edaa6e202..56f951584e 100644 --- a/types/context_test.go +++ b/types/context_test.go @@ -163,7 +163,7 @@ func TestContextWithCustom(t *testing.T) { logger := NewMockLogger() voteinfos := []abci.VoteInfo{{}} meter := types.NewGasMeter(10000) - minGasPrices := types.DecCoins{types.NewDecCoin("feetoken", 1)} + minGasPrices := types.DecCoins{types.NewInt64DecCoin("feetoken", 1)} ctx = types.NewContext(nil, header, ischeck, logger) require.Equal(t, header, ctx.BlockHeader()) diff --git a/types/dec_coin.go b/types/dec_coin.go index e3222ae320..9634b85360 100644 --- a/types/dec_coin.go +++ b/types/dec_coin.go @@ -18,27 +18,25 @@ type DecCoin struct { Amount Dec `json:"amount"` } -func NewDecCoin(denom string, amount int64) DecCoin { - if amount < 0 { - panic(fmt.Sprintf("negative decimal coin amount: %v\n", amount)) - } - if strings.ToLower(denom) != denom { - panic(fmt.Sprintf("denom cannot contain upper case characters: %s\n", denom)) +func NewDecCoin(denom string, amount Int) DecCoin { + validateDenom(denom) + + if amount.LT(ZeroInt()) { + panic(fmt.Sprintf("negative coin amount: %v\n", amount)) } return DecCoin{ Denom: denom, - Amount: NewDec(amount), + Amount: NewDecFromInt(amount), } } func NewDecCoinFromDec(denom string, amount Dec) DecCoin { + validateDenom(denom) + if amount.LT(ZeroDec()) { panic(fmt.Sprintf("negative decimal coin amount: %v\n", amount)) } - if strings.ToLower(denom) != denom { - panic(fmt.Sprintf("denom cannot contain upper case characters: %s\n", denom)) - } return DecCoin{ Denom: denom, @@ -60,6 +58,46 @@ func NewDecCoinFromCoin(coin Coin) DecCoin { } } +// NewInt64DecCoin returns a new DecCoin with a denomination and amount. It will +// panic if the amount is negative or denom is invalid. +func NewInt64DecCoin(denom string, amount int64) DecCoin { + return NewDecCoin(denom, NewInt(amount)) +} + +// IsZero returns if the DecCoin amount is zero. +func (coin DecCoin) IsZero() bool { + return coin.Amount.IsZero() +} + +// IsGTE returns true if they are the same type and the receiver is +// an equal or greater value. +func (coin DecCoin) IsGTE(other DecCoin) bool { + if coin.Denom != other.Denom { + panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, other.Denom)) + } + + return !coin.Amount.LT(other.Amount) +} + +// IsLT returns true if they are the same type and the receiver is +// a smaller value. +func (coin DecCoin) IsLT(other DecCoin) bool { + if coin.Denom != other.Denom { + panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, other.Denom)) + } + + return coin.Amount.LT(other.Amount) +} + +// IsEqual returns true if the two sets of Coins have the same value. +func (coin DecCoin) IsEqual(other DecCoin) bool { + if coin.Denom != other.Denom { + panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, other.Denom)) + } + + return coin.Amount.Equal(other.Amount) +} + // Adds amounts of two coins with same denom func (coin DecCoin) Plus(coinB DecCoin) DecCoin { if coin.Denom != coinB.Denom { @@ -90,6 +128,13 @@ func (coin DecCoin) IsPositive() bool { return coin.Amount.IsPositive() } +// IsNegative returns true if the coin amount is negative and false otherwise. +// +// TODO: Remove once unsigned integers are used. +func (coin DecCoin) IsNegative() bool { + return coin.Amount.Sign() == -1 +} + // String implements the Stringer interface for DecCoin. It returns a // human-readable representation of a decimal coin. func (coin DecCoin) String() string { @@ -125,55 +170,87 @@ func (coins DecCoins) String() string { return out[:len(out)-1] } -// return the coins with trunctated decimals, and return the change +// TruncateDecimal returns the coins with truncated decimals and returns the +// change. func (coins DecCoins) TruncateDecimal() (Coins, DecCoins) { changeSum := DecCoins{} out := make(Coins, len(coins)) + for i, coin := range coins { truncated, change := coin.TruncateDecimal() out[i] = truncated changeSum = changeSum.Plus(DecCoins{change}) } + return out, changeSum } -// Plus combines two sets of coins -// CONTRACT: Plus will never return Coins where one Coin has a 0 amount. +// Plus adds two sets of DecCoins. +// +// NOTE: Plus operates under the invariant that coins are sorted by +// denominations. +// +// CONTRACT: Plus will never return Coins where one Coin has a non-positive +// amount. In otherwords, IsValid will always return true. func (coins DecCoins) Plus(coinsB DecCoins) DecCoins { + return coins.safePlus(coinsB) +} + +// safePlus will perform addition of two DecCoins sets. If both coin sets are +// empty, then an empty set is returned. If only a single set is empty, the +// other set is returned. Otherwise, the coins are compared in order of their +// denomination and addition only occurs when the denominations match, otherwise +// the coin is simply added to the sum assuming it's not zero. +func (coins DecCoins) safePlus(coinsB DecCoins) DecCoins { sum := ([]DecCoin)(nil) indexA, indexB := 0, 0 lenA, lenB := len(coins), len(coinsB) + for { if indexA == lenA { if indexB == lenB { + // return nil coins if both sets are empty return sum } - return append(sum, coinsB[indexB:]...) + + // return set B (excluding zero coins) if set A is empty + return append(sum, removeZeroDecCoins(coinsB[indexB:])...) } else if indexB == lenB { - return append(sum, coins[indexA:]...) + // return set A (excluding zero coins) if set B is empty + return append(sum, removeZeroDecCoins(coins[indexA:])...) } + coinA, coinB := coins[indexA], coinsB[indexB] + switch strings.Compare(coinA.Denom, coinB.Denom) { - case -1: - sum = append(sum, coinA) - indexA++ - case 0: - if coinA.Amount.Add(coinB.Amount).IsZero() { - // ignore 0 sum coin type - } else { - sum = append(sum, coinA.Plus(coinB)) + case -1: // coin A denom < coin B denom + if !coinA.IsZero() { + sum = append(sum, coinA) } + + indexA++ + + case 0: // coin A denom == coin B denom + res := coinA.Plus(coinB) + if !res.IsZero() { + sum = append(sum, res) + } + indexA++ indexB++ - case 1: - sum = append(sum, coinB) + + case 1: // coin A denom > coin B denom + if !coinB.IsZero() { + sum = append(sum, coinB) + } + indexB++ } } } -// Negative returns a set of coins with all amount negative -func (coins DecCoins) Negative() DecCoins { +// negative returns a set of coins with all amount negative. +func (coins DecCoins) negative() DecCoins { res := make([]DecCoin, 0, len(coins)) for _, coin := range coins { res = append(res, DecCoin{ @@ -184,9 +261,36 @@ func (coins DecCoins) Negative() DecCoins { return res } -// Minus subtracts a set of coins from another (adds the inverse) +// Minus subtracts a set of DecCoins from another (adds the inverse). func (coins DecCoins) Minus(coinsB DecCoins) DecCoins { - return coins.Plus(coinsB.Negative()) + diff, hasNeg := coins.SafeMinus(coinsB) + if hasNeg { + panic("negative coin amount") + } + + return diff +} + +// SafeMinus performs the same arithmetic as Minus but returns a boolean if any +// negative coin amount was returned. +func (coins DecCoins) SafeMinus(coinsB DecCoins) (DecCoins, bool) { + diff := coins.safePlus(coinsB.negative()) + return diff, diff.IsAnyNegative() +} + +// 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. +// +// TODO: Remove once unsigned integers are used. +func (coins DecCoins) IsAnyNegative() bool { + for _, coin := range coins { + if coin.IsNegative() { + return true + } + } + + return false } // multiply all the coins by a decimal @@ -241,20 +345,30 @@ func (coins DecCoins) QuoDecTruncate(d Dec) DecCoins { return res } +// Empty returns true if there are no coins and false otherwise. +func (coins DecCoins) Empty() bool { + return len(coins) == 0 +} + // returns the amount of a denom from deccoins func (coins DecCoins) AmountOf(denom string) Dec { + validateDenom(denom) + switch len(coins) { case 0: return ZeroDec() + case 1: coin := coins[0] if coin.Denom == denom { return coin.Amount } return ZeroDec() + default: - midIdx := len(coins) / 2 // binary search + midIdx := len(coins) / 2 // 2:1, 3:1, 4:2 coin := coins[midIdx] + if denom < coin.Denom { return coins[:midIdx].AmountOf(denom) } else if denom == coin.Denom { @@ -265,14 +379,22 @@ func (coins DecCoins) AmountOf(denom string) Dec { } } -// has a negative DecCoin amount -func (coins DecCoins) HasNegative() bool { - for _, coin := range coins { - if coin.Amount.IsNegative() { - return true +// IsEqual returns true if the two sets of DecCoins have the same value. +func (coins DecCoins) IsEqual(coinsB DecCoins) bool { + if len(coins) != len(coinsB) { + return false + } + + coins = coins.Sort() + coinsB = coinsB.Sort() + + for i := 0; i < len(coins); i++ { + if !coins[i].IsEqual(coinsB[i]) { + return false } } - return false + + return true } // return whether all coins are zero @@ -324,6 +446,39 @@ func (coins DecCoins) IsValid() bool { } } +// IsAllPositive returns true if there is at least one coin and all currencies +// have a positive value. +// +// TODO: Remove once unsigned integers are used. +func (coins DecCoins) IsAllPositive() bool { + if len(coins) == 0 { + return false + } + + for _, coin := range coins { + if !coin.IsPositive() { + return false + } + } + + return true +} + +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++ + } + } + + return coins[:i] +} + //----------------------------------------------------------------------------- // Sorting diff --git a/types/dec_coin_test.go b/types/dec_coin_test.go index b2502ced1d..ba7f49add1 100644 --- a/types/dec_coin_test.go +++ b/types/dec_coin_test.go @@ -1,6 +1,7 @@ package types import ( + "strings" "testing" "github.com/stretchr/testify/require" @@ -8,61 +9,61 @@ import ( func TestNewDecCoin(t *testing.T) { require.NotPanics(t, func() { - NewDecCoin("a", 5) + NewInt64DecCoin(testDenom1, 5) }) require.NotPanics(t, func() { - NewDecCoin("a", 0) + NewInt64DecCoin(testDenom1, 0) }) require.Panics(t, func() { - NewDecCoin("A", 5) + NewInt64DecCoin(strings.ToUpper(testDenom1), 5) }) require.Panics(t, func() { - NewDecCoin("a", -5) + NewInt64DecCoin(testDenom1, -5) }) } func TestNewDecCoinFromDec(t *testing.T) { require.NotPanics(t, func() { - NewDecCoinFromDec("a", NewDec(5)) + NewDecCoinFromDec(testDenom1, NewDec(5)) }) require.NotPanics(t, func() { - NewDecCoinFromDec("a", ZeroDec()) + NewDecCoinFromDec(testDenom1, ZeroDec()) }) require.Panics(t, func() { - NewDecCoinFromDec("A", NewDec(5)) + NewDecCoinFromDec(strings.ToUpper(testDenom1), NewDec(5)) }) require.Panics(t, func() { - NewDecCoinFromDec("a", NewDec(-5)) + NewDecCoinFromDec(testDenom1, NewDec(-5)) }) } func TestNewDecCoinFromCoin(t *testing.T) { require.NotPanics(t, func() { - NewDecCoinFromCoin(Coin{"a", NewInt(5)}) + NewDecCoinFromCoin(Coin{testDenom1, NewInt(5)}) }) require.NotPanics(t, func() { - NewDecCoinFromCoin(Coin{"a", NewInt(0)}) + NewDecCoinFromCoin(Coin{testDenom1, NewInt(0)}) }) require.Panics(t, func() { - NewDecCoinFromCoin(Coin{"A", NewInt(5)}) + NewDecCoinFromCoin(Coin{strings.ToUpper(testDenom1), NewInt(5)}) }) require.Panics(t, func() { - NewDecCoinFromCoin(Coin{"a", NewInt(-5)}) + NewDecCoinFromCoin(Coin{testDenom1, NewInt(-5)}) }) } func TestDecCoinIsPositive(t *testing.T) { - dc := NewDecCoin("a", 5) + dc := NewInt64DecCoin(testDenom1, 5) require.True(t, dc.IsPositive()) - dc = NewDecCoin("a", 0) + dc = NewInt64DecCoin(testDenom1, 0) require.False(t, dc.IsPositive()) } func TestPlusDecCoin(t *testing.T) { - decCoinA1 := NewDecCoinFromDec("a", NewDecWithPrec(11, 1)) - decCoinA2 := NewDecCoinFromDec("a", NewDecWithPrec(22, 1)) - decCoinB1 := NewDecCoinFromDec("b", NewDecWithPrec(11, 1)) + decCoinA1 := NewDecCoinFromDec(testDenom1, NewDecWithPrec(11, 1)) + decCoinA2 := NewDecCoinFromDec(testDenom1, NewDecWithPrec(22, 1)) + decCoinB1 := NewDecCoinFromDec(testDenom2, NewDecWithPrec(11, 1)) // regular add res := decCoinA1.Plus(decCoinA1) @@ -84,9 +85,9 @@ func TestPlusDecCoins(t *testing.T) { inputTwo DecCoins expected DecCoins }{ - {DecCoins{{"a", one}, {"b", one}}, DecCoins{{"a", one}, {"b", one}}, DecCoins{{"a", two}, {"b", two}}}, - {DecCoins{{"a", zero}, {"b", one}}, DecCoins{{"a", zero}, {"b", zero}}, DecCoins{{"b", one}}}, - {DecCoins{{"a", zero}, {"b", zero}}, DecCoins{{"a", zero}, {"b", zero}}, DecCoins(nil)}, + {DecCoins{{testDenom1, one}, {testDenom2, one}}, DecCoins{{testDenom1, one}, {testDenom2, one}}, DecCoins{{testDenom1, two}, {testDenom2, two}}}, + {DecCoins{{testDenom1, zero}, {testDenom2, one}}, DecCoins{{testDenom1, zero}, {testDenom2, zero}}, DecCoins{{testDenom2, one}}}, + {DecCoins{{testDenom1, zero}, {testDenom2, zero}}, DecCoins{{testDenom1, zero}, {testDenom2, zero}}, DecCoins(nil)}, } for tcIndex, tc := range cases { @@ -97,32 +98,32 @@ func TestPlusDecCoins(t *testing.T) { func TestSortDecCoins(t *testing.T) { good := DecCoins{ - NewDecCoin("gas", 1), - NewDecCoin("mineral", 1), - NewDecCoin("tree", 1), + NewInt64DecCoin("gas", 1), + NewInt64DecCoin("mineral", 1), + NewInt64DecCoin("tree", 1), } empty := DecCoins{ - NewDecCoin("gold", 0), + NewInt64DecCoin("gold", 0), } badSort1 := DecCoins{ - NewDecCoin("tree", 1), - NewDecCoin("gas", 1), - NewDecCoin("mineral", 1), + NewInt64DecCoin("tree", 1), + NewInt64DecCoin("gas", 1), + NewInt64DecCoin("mineral", 1), } badSort2 := DecCoins{ // both are after the first one, but the second and third are in the wrong order - NewDecCoin("gas", 1), - NewDecCoin("tree", 1), - NewDecCoin("mineral", 1), + NewInt64DecCoin("gas", 1), + NewInt64DecCoin("tree", 1), + NewInt64DecCoin("mineral", 1), } badAmt := DecCoins{ - NewDecCoin("gas", 1), - NewDecCoin("tree", 0), - NewDecCoin("mineral", 1), + NewInt64DecCoin("gas", 1), + NewInt64DecCoin("tree", 0), + NewInt64DecCoin("mineral", 1), } dup := DecCoins{ - NewDecCoin("gas", 1), - NewDecCoin("gas", 1), - NewDecCoin("mineral", 1), + NewInt64DecCoin("gas", 1), + NewInt64DecCoin("gas", 1), + NewInt64DecCoin("mineral", 1), } cases := []struct { @@ -150,14 +151,14 @@ func TestDecCoinsIsValid(t *testing.T) { expected bool }{ {DecCoins{}, true}, - {DecCoins{DecCoin{"a", NewDec(5)}}, true}, - {DecCoins{DecCoin{"a", NewDec(5)}, DecCoin{"b", NewDec(100000)}}, true}, - {DecCoins{DecCoin{"a", NewDec(-5)}}, false}, - {DecCoins{DecCoin{"A", NewDec(5)}}, false}, - {DecCoins{DecCoin{"a", NewDec(5)}, DecCoin{"B", NewDec(100000)}}, false}, - {DecCoins{DecCoin{"a", NewDec(5)}, DecCoin{"b", NewDec(-100000)}}, false}, - {DecCoins{DecCoin{"a", NewDec(-5)}, DecCoin{"b", NewDec(100000)}}, false}, - {DecCoins{DecCoin{"A", NewDec(5)}, DecCoin{"b", NewDec(100000)}}, false}, + {DecCoins{DecCoin{testDenom1, NewDec(5)}}, true}, + {DecCoins{DecCoin{testDenom1, NewDec(5)}, DecCoin{testDenom2, NewDec(100000)}}, true}, + {DecCoins{DecCoin{testDenom1, NewDec(-5)}}, false}, + {DecCoins{DecCoin{"AAA", NewDec(5)}}, false}, + {DecCoins{DecCoin{testDenom1, NewDec(5)}, DecCoin{"B", NewDec(100000)}}, false}, + {DecCoins{DecCoin{testDenom1, NewDec(5)}, DecCoin{testDenom2, NewDec(-100000)}}, false}, + {DecCoins{DecCoin{testDenom1, NewDec(-5)}, DecCoin{testDenom2, NewDec(100000)}}, false}, + {DecCoins{DecCoin{"AAA", NewDec(5)}, DecCoin{testDenom2, NewDec(100000)}}, false}, } for i, tc := range testCases { diff --git a/x/bank/msgs.go b/x/bank/msgs.go index 8b3ee2cb44..11479ff733 100644 --- a/x/bank/msgs.go +++ b/x/bank/msgs.go @@ -35,7 +35,7 @@ func (msg MsgSend) ValidateBasic() sdk.Error { if msg.ToAddress.Empty() { return sdk.ErrInvalidAddress("missing recipient address") } - if !msg.Amount.IsPositive() { + if !msg.Amount.IsAllPositive() { return sdk.ErrInsufficientCoins("send amount must be positive") } return nil @@ -112,7 +112,7 @@ func (in Input) ValidateBasic() sdk.Error { if !in.Coins.IsValid() { return sdk.ErrInvalidCoins(in.Coins.String()) } - if !in.Coins.IsPositive() { + if !in.Coins.IsAllPositive() { return sdk.ErrInvalidCoins(in.Coins.String()) } return nil @@ -140,7 +140,7 @@ func (out Output) ValidateBasic() sdk.Error { if !out.Coins.IsValid() { return sdk.ErrInvalidCoins(out.Coins.String()) } - if !out.Coins.IsPositive() { + if !out.Coins.IsAllPositive() { return sdk.ErrInvalidCoins(out.Coins.String()) } return nil diff --git a/x/bank/msgs_test.go b/x/bank/msgs_test.go index fbc42ddc2a..b065dc9cc0 100644 --- a/x/bank/msgs_test.go +++ b/x/bank/msgs_test.go @@ -172,7 +172,6 @@ func TestMsgMultiSendValidation(t *testing.T) { input2 := NewInput(addr1, eth123) output1 := NewOutput(addr2, atom123) output2 := NewOutput(addr2, atom124) - output3 := NewOutput(addr2, eth123) outputMulti := NewOutput(addr2, atom123eth123) var emptyAddr sdk.AccAddress @@ -195,19 +194,6 @@ func TestMsgMultiSendValidation(t *testing.T) { Inputs: []Input{input1}, Outputs: []Output{output2}}, // amounts dont match }, - {false, MsgMultiSend{ - Inputs: []Input{input1}, - Outputs: []Output{output3}}, // amounts dont match - }, - {false, MsgMultiSend{ - Inputs: []Input{input1}, - Outputs: []Output{outputMulti}}, // amounts dont match - }, - {false, MsgMultiSend{ - Inputs: []Input{input2}, - Outputs: []Output{output1}}, // amounts dont match - }, - {true, MsgMultiSend{ Inputs: []Input{input1}, Outputs: []Output{output1}}, diff --git a/x/distribution/keeper/delegation.go b/x/distribution/keeper/delegation.go index d75691bdf5..6738deb924 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -41,7 +41,7 @@ func (k Keeper) calculateDelegationRewardsBetween(ctx sdk.Context, val sdk.Valid starting := k.GetValidatorHistoricalRewards(ctx, val.GetOperator(), startingPeriod) ending := k.GetValidatorHistoricalRewards(ctx, val.GetOperator(), endingPeriod) difference := ending.CumulativeRewardRatio.Minus(starting.CumulativeRewardRatio) - if difference.HasNegative() { + if difference.IsAnyNegative() { panic("negative rewards should not be possible") } // note: necessary to truncate so we don't allow withdrawing more rewards than owed @@ -100,6 +100,7 @@ func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val sdk.Validator, de // truncate coins, return remainder to community pool coins, remainder := rewards.TruncateDecimal() outstanding := k.GetOutstandingRewards(ctx) + k.SetOutstandingRewards(ctx, outstanding.Minus(rewards)) feePool := k.GetFeePool(ctx) feePool.CommunityPool = feePool.CommunityPool.Plus(remainder) diff --git a/x/distribution/keeper/delegation_test.go b/x/distribution/keeper/delegation_test.go index a34960b1c5..5286a141f1 100644 --- a/x/distribution/keeper/delegation_test.go +++ b/x/distribution/keeper/delegation_test.go @@ -270,28 +270,31 @@ func TestWithdrawDelegationRewardsBasic(t *testing.T) { power := int64(100) valTokens := sdk.TokensFromTendermintPower(power) commission := staking.NewCommissionMsg(sdk.NewDecWithPrec(5, 1), sdk.NewDecWithPrec(5, 1), sdk.NewDec(0)) - msg := staking.NewMsgCreateValidator(valOpAddr1, valConsPk1, - sdk.NewCoin(sdk.DefaultBondDenom, valTokens), staking.Description{}, commission, sdk.OneInt()) + msg := staking.NewMsgCreateValidator( + valOpAddr1, valConsPk1, + sdk.NewCoin(sdk.DefaultBondDenom, valTokens), + staking.Description{}, commission, sdk.OneInt(), + ) require.True(t, sh(ctx, msg).IsOK()) // assert correct initial balance expTokens := balanceTokens.Sub(valTokens) require.Equal(t, - sdk.Coins{{sdk.DefaultBondDenom, expTokens}}, - ak.GetAccount(ctx, sdk.AccAddress(valOpAddr1)).GetCoins()) + sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, expTokens)}, + ak.GetAccount(ctx, sdk.AccAddress(valOpAddr1)).GetCoins(), + ) // end block to bond validator staking.EndBlocker(ctx, sk) - // set zero outstanding rewards - k.SetOutstandingRewards(ctx, sdk.DecCoins{}) - // fetch validator and delegation val := sk.Validator(ctx, valOpAddr1) // allocate some rewards initial := sdk.TokensFromTendermintPower(10) - tokens := sdk.DecCoins{{sdk.DefaultBondDenom, sdk.NewDecFromInt(initial)}} + tokens := sdk.DecCoins{sdk.NewDecCoin(sdk.DefaultBondDenom, initial)} + + k.SetOutstandingRewards(ctx, tokens) k.AllocateTokensToValidator(ctx, val, tokens) // historical count should be 2 (initial + latest for delegation) @@ -305,14 +308,20 @@ func TestWithdrawDelegationRewardsBasic(t *testing.T) { // assert correct balance exp := balanceTokens.Sub(valTokens).Add(initial.DivRaw(2)) - require.Equal(t, sdk.Coins{{sdk.DefaultBondDenom, exp}}, ak.GetAccount(ctx, sdk.AccAddress(valOpAddr1)).GetCoins()) + require.Equal(t, + sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, exp)}, + ak.GetAccount(ctx, sdk.AccAddress(valOpAddr1)).GetCoins(), + ) // withdraw commission require.Nil(t, k.WithdrawValidatorCommission(ctx, valOpAddr1)) // assert correct balance exp = balanceTokens.Sub(valTokens).Add(initial) - require.Equal(t, sdk.Coins{{sdk.DefaultBondDenom, exp}}, ak.GetAccount(ctx, sdk.AccAddress(valOpAddr1)).GetCoins()) + require.Equal(t, + sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, exp)}, + ak.GetAccount(ctx, sdk.AccAddress(valOpAddr1)).GetCoins(), + ) } func TestCalculateRewardsAfterManySlashesInSameBlock(t *testing.T) { @@ -458,8 +467,12 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { ctx, _, k, sk, _ := CreateTestInputDefault(t, false, 1000) sh := staking.NewHandler(sk) + initial := int64(20) + totalRewards := sdk.DecCoins{sdk.NewDecCoinFromDec(sdk.DefaultBondDenom, sdk.NewDec(initial*2))} + tokens := sdk.DecCoins{sdk.NewDecCoinFromDec(sdk.DefaultBondDenom, sdk.NewDec(initial))} + // initialize state - k.SetOutstandingRewards(ctx, sdk.DecCoins{}) + k.SetOutstandingRewards(ctx, totalRewards) // create validator with 50% commission commission := staking.NewCommissionMsg(sdk.NewDecWithPrec(5, 1), sdk.NewDecWithPrec(5, 1), sdk.NewDec(0)) @@ -475,8 +488,6 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { del1 := sk.Delegation(ctx, sdk.AccAddress(valOpAddr1), valOpAddr1) // allocate some rewards - initial := int64(20) - tokens := sdk.DecCoins{{sdk.DefaultBondDenom, sdk.NewDec(initial)}} k.AllocateTokensToValidator(ctx, val, tokens) // historical count should be 2 (validator init, delegation init) @@ -529,6 +540,9 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { // commission should be zero require.True(t, k.GetValidatorAccumulatedCommission(ctx, valOpAddr1).IsZero()) + totalRewards = k.GetOutstandingRewards(ctx).Plus(tokens) + k.SetOutstandingRewards(ctx, totalRewards) + // allocate some more rewards k.AllocateTokensToValidator(ctx, val, tokens) @@ -553,6 +567,9 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { // commission should be half initial require.Equal(t, sdk.DecCoins{{sdk.DefaultBondDenom, sdk.NewDec(initial / 2)}}, k.GetValidatorAccumulatedCommission(ctx, valOpAddr1)) + totalRewards = k.GetOutstandingRewards(ctx).Plus(tokens) + k.SetOutstandingRewards(ctx, totalRewards) + // allocate some more rewards k.AllocateTokensToValidator(ctx, val, tokens) diff --git a/x/distribution/keeper/keeper_test.go b/x/distribution/keeper/keeper_test.go index 56ca707034..fc6f7cc0a1 100644 --- a/x/distribution/keeper/keeper_test.go +++ b/x/distribution/keeper/keeper_test.go @@ -6,7 +6,6 @@ import ( "github.com/stretchr/testify/require" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/distribution/types" ) func TestSetWithdrawAddr(t *testing.T) { @@ -26,21 +25,23 @@ func TestSetWithdrawAddr(t *testing.T) { func TestWithdrawValidatorCommission(t *testing.T) { ctx, ak, keeper, _, _ := CreateTestInputDefault(t, false, 1000) + valCommission := sdk.DecCoins{ + sdk.NewDecCoinFromDec("mytoken", sdk.NewDec(5).Quo(sdk.NewDec(4))), + sdk.NewDecCoinFromDec("stake", sdk.NewDec(3).Quo(sdk.NewDec(2))), + } + // set zero outstanding rewards - keeper.SetOutstandingRewards(ctx, types.OutstandingRewards{}) + keeper.SetOutstandingRewards(ctx, valCommission) // check initial balance balance := ak.GetAccount(ctx, sdk.AccAddress(valOpAddr3)).GetCoins() expTokens := sdk.TokensFromTendermintPower(1000) require.Equal(t, sdk.Coins{ - {"stake", sdk.TokensFromTendermintPower(1000)}, + sdk.NewCoin("stake", sdk.TokensFromTendermintPower(1000)), }, balance) // set commission - keeper.SetValidatorAccumulatedCommission(ctx, valOpAddr3, sdk.DecCoins{ - {"mytoken", sdk.NewDec(5).Quo(sdk.NewDec(4))}, - {"stake", sdk.NewDec(3).Quo(sdk.NewDec(2))}, - }) + keeper.SetValidatorAccumulatedCommission(ctx, valOpAddr3, valCommission) // withdraw commission keeper.WithdrawValidatorCommission(ctx, valOpAddr3) @@ -48,15 +49,15 @@ func TestWithdrawValidatorCommission(t *testing.T) { // check balance increase balance = ak.GetAccount(ctx, sdk.AccAddress(valOpAddr3)).GetCoins() require.Equal(t, sdk.Coins{ - {"mytoken", sdk.NewInt(1)}, - {"stake", expTokens.AddRaw(1)}, + sdk.NewCoin("mytoken", sdk.NewInt(1)), + sdk.NewCoin("stake", expTokens.AddRaw(1)), }, balance) // check remainder remainder := keeper.GetValidatorAccumulatedCommission(ctx, valOpAddr3) require.Equal(t, sdk.DecCoins{ - {"mytoken", sdk.NewDec(1).Quo(sdk.NewDec(4))}, - {"stake", sdk.NewDec(1).Quo(sdk.NewDec(2))}, + sdk.NewDecCoinFromDec("mytoken", sdk.NewDec(1).Quo(sdk.NewDec(4))), + sdk.NewDecCoinFromDec("stake", sdk.NewDec(1).Quo(sdk.NewDec(2))), }, remainder) require.True(t, true) diff --git a/x/distribution/simulation/invariants.go b/x/distribution/simulation/invariants.go index ac4fcdffe5..f722410651 100644 --- a/x/distribution/simulation/invariants.go +++ b/x/distribution/simulation/invariants.go @@ -31,7 +31,7 @@ func AllInvariants(d distr.Keeper, stk types.StakingKeeper) sdk.Invariant { func NonNegativeOutstandingInvariant(k distr.Keeper) sdk.Invariant { return func(ctx sdk.Context) error { outstanding := k.GetOutstandingRewards(ctx) - if outstanding.HasNegative() { + if outstanding.IsAnyNegative() { return fmt.Errorf("negative outstanding coins: %v", outstanding) } return nil diff --git a/x/distribution/types/fee_pool.go b/x/distribution/types/fee_pool.go index 1bf2083c93..bb34ab47cd 100644 --- a/x/distribution/types/fee_pool.go +++ b/x/distribution/types/fee_pool.go @@ -20,7 +20,7 @@ func InitialFeePool() FeePool { // ValidateGenesis validates the fee pool for a genesis state func (f FeePool) ValidateGenesis() error { - if f.CommunityPool.HasNegative() { + if f.CommunityPool.IsAnyNegative() { return fmt.Errorf("negative CommunityPool in distribution fee pool, is %v", f.CommunityPool) }