diff --git a/tm2/pkg/std/coin.go b/tm2/pkg/std/coin.go index 16c8f8fe7ff..bbeeb5e6128 100644 --- a/tm2/pkg/std/coin.go +++ b/tm2/pkg/std/coin.go @@ -121,7 +121,7 @@ func (coin Coin) IsEqual(other Coin) bool { func (coin Coin) Add(coinB Coin) Coin { res := coin.AddUnsafe(coinB) if !res.IsValid() { - panic(fmt.Sprintf("invalid result: %v + %v = %v", coin, coinB, res)) + panic(fmt.Sprintf("invalid result: %v + %v = %v (lowercase-only denoms, and positive amounts)", coin, coinB, res)) } return res } @@ -144,7 +144,7 @@ func (coin Coin) AddUnsafe(coinB Coin) Coin { func (coin Coin) Sub(coinB Coin) Coin { res := coin.SubUnsafe(coinB) if !res.IsValid() { - panic(fmt.Sprintf("invalid result: %v - %v = %v", coin, coinB, res)) + panic(fmt.Sprintf("invalid result: %v - %v = %v (lowercase-only denoms, and positive amounts)", coin, coinB, res)) } return res } @@ -192,7 +192,10 @@ func NewCoins(coins ...Coin) Coins { } if !newCoins.IsValid() { - panic(fmt.Errorf("invalid coin set: %s", newCoins)) + panic(fmt.Errorf( + "invalid coin set: %s (must be sorted, lowercase-only denoms, and positive amounts)", + newCoins, + )) } return newCoins @@ -271,7 +274,10 @@ func (coins Coins) IsValid() bool { func (coins Coins) Add(coinsB Coins) Coins { res := coins.AddUnsafe(coinsB) if !res.IsValid() { - panic(fmt.Sprintf("invalid result: %v + %v = %v", coins, coinsB, res)) + panic(fmt.Sprintf( + "invalid result: %v + %v = %v (coins must be sorted, lowercase-only denoms, and positive amounts)", + coins, coinsB, res, + )) } return res } @@ -372,7 +378,10 @@ func (coins Coins) DenomsSubsetOf(coinsB Coins) bool { func (coins Coins) Sub(coinsB Coins) Coins { res := coins.SubUnsafe(coinsB) if !res.IsValid() { - panic(fmt.Sprintf("invalid result: %v - %v = %v", coins, coinsB, res)) + panic(fmt.Sprintf( + "invalid result: %v - %v = %v (must be sorted, lowercase-only denoms, and positive amounts)", + coins, coinsB, res, + )) } return res } @@ -717,7 +726,10 @@ func ParseCoins(coinsStr string) (Coins, error) { // validate coins before returning if !coins.IsValid() { - return nil, fmt.Errorf("parseCoins invalid: %#v", coins) + return nil, fmt.Errorf( + "parseCoins: invalid coins %v (must be sorted, lowercase-only denoms, and positive amounts)", + coins, + ) } return coins, nil diff --git a/tm2/pkg/std/coin_test.go b/tm2/pkg/std/coin_test.go index ba3c8f49c54..781211d139c 100644 --- a/tm2/pkg/std/coin_test.go +++ b/tm2/pkg/std/coin_test.go @@ -10,8 +10,9 @@ import ( ) var ( - testDenom1 = "atom" - testDenom2 = "muon" + testDenom1 = "atom" + testDenom2 = "muon" + testDenomInvalid = "Atom" ) // ---------------------------------------------------------------------------- @@ -83,6 +84,13 @@ func TestAddCoin(t *testing.T) { {NewCoin(testDenom1, 1), NewCoin(testDenom1, 1), NewCoin(testDenom1, 2), false}, {NewCoin(testDenom1, 1), NewCoin(testDenom1, 0), NewCoin(testDenom1, 1), false}, {NewCoin(testDenom1, 1), NewCoin(testDenom2, 1), NewCoin(testDenom1, 1), true}, + {Coin{ + Denom: testDenomInvalid, + Amount: 1, + }, Coin{ + Denom: testDenomInvalid, + Amount: 1, + }, NewCoin(testDenom1, 0), true}, } for tcIndex, tc := range cases { @@ -109,6 +117,10 @@ func TestSubCoin(t *testing.T) { {NewCoin(testDenom1, 5), NewCoin(testDenom1, 3), NewCoin(testDenom1, 2), false}, {NewCoin(testDenom1, 5), NewCoin(testDenom1, 0), NewCoin(testDenom1, 5), false}, {NewCoin(testDenom1, 1), NewCoin(testDenom1, 5), Coin{}, true}, + {NewCoin(testDenom1, 1), Coin{ + Denom: testDenomInvalid, + Amount: 1, + }, Coin{}, true}, } for tcIndex, tc := range cases { @@ -254,18 +266,24 @@ func TestAddCoins(t *testing.T) { inputOne Coins inputTwo Coins expected Coins + panics bool }{ - {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)}, + {Coins{{testDenom1, one}, {testDenom2, one}}, Coins{{testDenom1, one}, {testDenom2, one}}, Coins{{testDenom1, two}, {testDenom2, two}}, false}, + {Coins{{testDenom1, zero}, {testDenom2, one}}, Coins{{testDenom1, zero}, {testDenom2, zero}}, Coins{{testDenom2, one}}, false}, + {Coins{{testDenom1, two}}, Coins{{testDenom2, zero}}, Coins{{testDenom1, two}}, false}, + {Coins{{testDenom1, one}}, Coins{{testDenom1, one}, {testDenom2, two}}, Coins{{testDenom1, two}, {testDenom2, two}}, false}, + {Coins{{testDenom1, zero}, {testDenom2, zero}}, Coins{{testDenom1, zero}, {testDenom2, zero}}, Coins(nil), false}, + {Coins{{testDenom1, zero}}, Coins{{testDenomInvalid, one}}, Coins{}, true}, } for tcIndex, tc := range cases { - 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) + if tc.panics { + require.Panics(t, func() { tc.inputOne.Add(tc.inputTwo) }) + } else { + 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) + } } } @@ -434,6 +452,8 @@ func TestParse(t *testing.T) { {"2 3foo, 97 bar", false, nil}, // 3foo is invalid coin name {"11me coin, 12you coin", false, nil}, // no spaces in coin names {"1.2btc", false, nil}, // amount must be integer + {"-5foo", false, nil}, // amount must be positive + {"5Foo", false, nil}, // denom must be lowercase } for tcIndex, tc := range cases { @@ -615,6 +635,17 @@ func TestNewCoins(t *testing.T) { tenatom := NewCoin("atom", 10) tenbtc := NewCoin("btc", 10) zeroeth := NewCoin("eth", 0) + + // don't use NewCoin(...) to avoid early panic + uppercase := Coin{ + Denom: "UPC", + Amount: 10, + } + negative := Coin{ + Denom: "neg", + Amount: -5, + } + tests := []struct { name string coins Coins @@ -625,6 +656,8 @@ func TestNewCoins(t *testing.T) { {"one coin", []Coin{tenatom}, Coins{tenatom}, false}, {"sort after create", []Coin{tenbtc, tenatom}, Coins{tenatom, tenbtc}, false}, {"sort and remove zeroes", []Coin{zeroeth, tenbtc, tenatom}, Coins{tenatom, tenbtc}, false}, + {"panic on uppercase denom", []Coin{uppercase}, Coins{}, true}, + {"panic on negative amount", []Coin{negative}, Coins{}, true}, {"panic on dups", []Coin{tenatom, tenatom}, Coins{}, true}, } for _, tt := range tests {