Add Safety Measures to Coin/Coins (#2797)

PENDING.md

@@ -62,8 +62,10 @@ IMPROVEMENTS
 
 * SDK
  - [x/mock/simulation] [\#2720] major cleanup, introduction of helper objects, reorganization
+ - \#2821 Codespaces are now strings
+ - [types] #2776 Improve safety of `Coin` and `Coins` types. Various functions
+ and methods will panic when a negative amount is discovered.
  - #2815 Gas unit fields changed from `int64` to `uint64`.
- - #2821 Codespaces are now strings
 
 * Tendermint
  - #2796 Update to go-amino 0.14.1

cmd/gaia/app/genesis.go

@@ -279,10 +279,12 @@ func CollectStdTxs(cdc *codec.Codec, moniker string, genTxsDir string, genDoc tm
 func NewDefaultGenesisAccount(addr sdk.AccAddress) GenesisAccount {
 	accAuth := auth.NewBaseAccountWithAddress(addr)
 	coins := sdk.Coins{
-		{"fooToken", sdk.NewInt(1000)},
+		sdk.NewCoin("fooToken", sdk.NewInt(1000)),
-		{bondDenom, freeFermionsAcc},
+		sdk.NewCoin(bondDenom, freeFermionsAcc),
 	}
+
 	coins.Sort()
+
 	accAuth.Coins = coins
 	return NewGenesisAccount(&accAuth)
 }

cmd/gaia/app/sim_test.go

@@ -63,7 +63,7 @@ func appStateFn(r *rand.Rand, accs []simulation.Account) json.RawMessage {
 
 	// Randomly generate some genesis accounts
 	for _, acc := range accs {
-		coins := sdk.Coins{sdk.Coin{stakeTypes.DefaultBondDenom, sdk.NewInt(amount)}}
+		coins := sdk.Coins{sdk.NewCoin(stakeTypes.DefaultBondDenom, sdk.NewInt(amount))}
 		genesisAccounts = append(genesisAccounts, GenesisAccount{
 			Address: acc.Address,
 			Coins:   coins,

docs/_attic/sdk/core/examples/app2_test.go

@@ -20,7 +20,7 @@ func TestEncoding(t *testing.T) {
 	sendMsg := MsgSend{
 		From:   addr1,
 		To:     addr2,
-		Amount: sdk.Coins{{"testCoins", sdk.NewInt(100)}},
+		Amount: sdk.Coins{sdk.NewCoin("testCoins", sdk.NewInt(100))},
 	}
 
 	// Construct transaction

docs/_attic/sdk/core/examples/app4_test.go

@@ -30,7 +30,7 @@ func InitTestChain(bc *bapp.BaseApp, chainID string, addrs ...sdk.AccAddress) {
 	for _, addr := range addrs {
 		acc := GenesisAccount{
 			Address: addr,
-			Coins:   sdk.Coins{{"testCoin", sdk.NewInt(100)}},
+			Coins:   sdk.Coins{sdk.NewCoin("testCoin", sdk.NewInt(100))},
 		}
 		accounts = append(accounts, &acc)
 	}
@@ -61,12 +61,12 @@ func TestBadMsg(t *testing.T) {
 	addr2 := priv2.PubKey().Address().Bytes()
 
 	// Attempt to spend non-existent funds
-	msg := GenerateSpendMsg(addr1, addr2, sdk.Coins{{"testCoin", sdk.NewInt(100)}})
+	msg := GenerateSpendMsg(addr1, addr2, sdk.Coins{sdk.NewCoin("testCoin", sdk.NewInt(100))})
 
 	// Construct transaction
 	fee := auth.StdFee{
 		Gas:    1000000000000000,
-		Amount: sdk.Coins{{"testCoin", sdk.NewInt(0)}},
+		Amount: sdk.Coins{sdk.NewCoin("testCoin", sdk.NewInt(0))},
 	}
 	signBytes := auth.StdSignBytes("test-chain", 0, 0, fee, []sdk.Msg{msg}, "")
 	sig, err := priv1.Sign(signBytes)
@@ -108,11 +108,11 @@ func TestMsgSend(t *testing.T) {
 	InitTestChain(bc, "test-chain", addr1)
 
 	// Send funds to addr2
-	msg := GenerateSpendMsg(addr1, addr2, sdk.Coins{{"testCoin", sdk.NewInt(100)}})
+	msg := GenerateSpendMsg(addr1, addr2, sdk.Coins{sdk.NewCoin("testCoin", sdk.NewInt(100))})
 
 	fee := auth.StdFee{
 		Gas:    1000000000000000,
-		Amount: sdk.Coins{{"testCoin", sdk.NewInt(0)}},
+		Amount: sdk.Coins{sdk.NewCoin("testCoin", sdk.NewInt(0))},
 	}
 	signBytes := auth.StdSignBytes("test-chain", 0, 0, fee, []sdk.Msg{msg}, "")
 	sig, err := priv1.Sign(signBytes)

docs/examples/democoin/x/cool/app_test.go

@@ -90,15 +90,15 @@ func TestMsgQuiz(t *testing.T) {
 	// Set the trend, submit a really cool quiz and check for reward
 	mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{setTrendMsg1}, []int64{0}, []int64{0}, true, true, priv1)
 	mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{quizMsg1}, []int64{0}, []int64{1}, true, true, priv1)
-	mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"icecold", sdk.NewInt(69)}})
+	mock.CheckBalance(t, mapp, addr1, sdk.Coins{sdk.NewCoin("icecold", sdk.NewInt(69))})
 	mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{quizMsg2}, []int64{0}, []int64{2}, false, false, priv1) // result without reward
-	mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"icecold", sdk.NewInt(69)}})
+	mock.CheckBalance(t, mapp, addr1, sdk.Coins{sdk.NewCoin("icecold", sdk.NewInt(69))})
 	mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{quizMsg1}, []int64{0}, []int64{3}, true, true, priv1)
-	mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"icecold", sdk.NewInt(138)}})
+	mock.CheckBalance(t, mapp, addr1, sdk.Coins{sdk.NewCoin("icecold", sdk.NewInt(138))})
 	mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{setTrendMsg2}, []int64{0}, []int64{4}, true, true, priv1) // reset the trend
 	mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{quizMsg1}, []int64{0}, []int64{5}, false, false, priv1)   // the same answer will nolonger do!
-	mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"icecold", sdk.NewInt(138)}})
+	mock.CheckBalance(t, mapp, addr1, sdk.Coins{sdk.NewCoin("icecold", sdk.NewInt(138))})
 	mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{quizMsg2}, []int64{0}, []int64{6}, true, true, priv1) // earlier answer now relevant again
-	mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"badvibesonly", sdk.NewInt(69)}, {"icecold", sdk.NewInt(138)}})
+	mock.CheckBalance(t, mapp, addr1, sdk.Coins{sdk.NewCoin("badvibesonly", sdk.NewInt(69)), sdk.NewCoin("icecold", sdk.NewInt(138))})
 	mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{setTrendMsg3}, []int64{0}, []int64{7}, false, false, priv1) // expect to fail to set the trend to something which is not cool
 }

docs/examples/democoin/x/pow/app_test.go

@@ -75,12 +75,12 @@ func TestMsgMine(t *testing.T) {
 	// Mine and check for reward
 	mineMsg1 := GenerateMsgMine(addr1, 1, 2)
 	mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{mineMsg1}, []int64{0}, []int64{0}, true, true, priv1)
-	mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"pow", sdk.NewInt(1)}})
+	mock.CheckBalance(t, mapp, addr1, sdk.Coins{sdk.NewCoin("pow", sdk.NewInt(1))})
 	// Mine again and check for reward
 	mineMsg2 := GenerateMsgMine(addr1, 2, 3)
 	mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{mineMsg2}, []int64{0}, []int64{1}, true, true, priv1)
-	mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"pow", sdk.NewInt(2)}})
+	mock.CheckBalance(t, mapp, addr1, sdk.Coins{sdk.NewCoin("pow", sdk.NewInt(2))})
 	// Mine again - should be invalid
 	mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{mineMsg2}, []int64{0}, []int64{1}, false, false, priv1)
-	mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"pow", sdk.NewInt(2)}})
+	mock.CheckBalance(t, mapp, addr1, sdk.Coins{sdk.NewCoin("pow", sdk.NewInt(2))})
 }

docs/intro/ocap.md

@@ -63,7 +63,7 @@ principle:
 type AppAccount struct {...}
 var account := &AppAccount{
     Address: pub.Address(),
-    Coins: sdk.Coins{{"ATM", 100}},
+    Coins: sdk.Coins{sdk.NewInt64Coin("ATM", 100)},
 }
 var sumValue := externalModule.ComputeSumValue(account)
 ```

types/coin.go

@@ -4,23 +4,41 @@ import (
 	"fmt"
 	"regexp"
 	"sort"
-	"strconv"
 	"strings"
 )
 
-// Coin hold some amount of one currency
+//-----------------------------------------------------------------------------
+// Coin
+
+// Coin hold some amount of one currency.
+//
+// CONTRACT: A coin will never hold a negative amount of any denomination.
+//
+// TODO: Make field members private for further safety.
 type Coin struct {
-	Denom  string `json:"denom"`
+	Denom string `json:"denom"`
-	Amount Int    `json:"amount"`
+
+	// To allow the use of unsigned integers (see: #1273) a larger refactor will
+	// need to be made. So we use signed integers for now with safety measures in
+	// place preventing negative values being used.
+	Amount Int `json:"amount"`
 }
 
+// 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 {
+	if amount.LT(ZeroInt()) {
+		panic("negative coin amount")
+	}
+
 	return Coin{
 		Denom:  denom,
 		Amount: amount,
 	}
 }
 
+// NewInt64Coin returns a new coin with a denomination and amount. It will panic
+// if the amount is negative.
 func NewInt64Coin(denom string, amount int64) Coin {
 	return NewCoin(denom, NewInt(amount))
 }
@@ -57,33 +75,46 @@ func (coin Coin) IsEqual(other Coin) bool {
 	return coin.SameDenomAs(other) && (coin.Amount.Equal(other.Amount))
 }
 
-// IsPositive returns true if coin amount is positive
+// 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) {
+		panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, coinB.Denom))
+	}
+
+	return Coin{coin.Denom, coin.Amount.Add(coinB.Amount)}
+}
+
+// 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) {
+		panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, coinB.Denom))
+	}
+
+	res := Coin{coin.Denom, coin.Amount.Sub(coinB.Amount)}
+	if !res.IsNotNegative() {
+		panic("negative count amount")
+	}
+
+	return res
+}
+
+// IsPositive returns true if coin amount is positive.
+//
+// TODO: Remove once unsigned integers are used.
 func (coin Coin) IsPositive() bool {
 	return (coin.Amount.Sign() == 1)
 }
 
-// IsNotNegative returns true if coin amount is not negative
+// IsNotNegative returns true if coin amount is not negative and false otherwise.
+//
+// TODO: Remove once unsigned integers are used.
 func (coin Coin) IsNotNegative() bool {
 	return (coin.Amount.Sign() != -1)
 }
 
-// Adds amounts of two coins with same denom
+//-----------------------------------------------------------------------------
-func (coin Coin) Plus(coinB Coin) Coin {
-	if !coin.SameDenomAs(coinB) {
-		return coin
-	}
-	return Coin{coin.Denom, coin.Amount.Add(coinB.Amount)}
-}
-
-// Subtracts amounts of two coins with same denom
-func (coin Coin) Minus(coinB Coin) Coin {
-	if !coin.SameDenomAs(coinB) {
-		return coin
-	}
-	return Coin{coin.Denom, coin.Amount.Sub(coinB.Amount)}
-}
-
-//----------------------------------------
 // Coins
 
 // Coins is a set of Coin, one per currency
@@ -101,127 +132,157 @@ func (coins Coins) String() string {
 	return out[:len(out)-1]
 }
 
-// IsValid asserts the Coins are sorted, and don't have 0 amounts
+// IsValid asserts the Coins are sorted and have positive amounts.
 func (coins Coins) IsValid() bool {
 	switch len(coins) {
 	case 0:
 		return true
 	case 1:
-		return !coins[0].IsZero()
+		return coins[0].IsPositive()
 	default:
 		lowDenom := coins[0].Denom
+
 		for _, coin := range coins[1:] {
 			if coin.Denom <= lowDenom {
 				return false
 			}
-			if coin.IsZero() {
+			if !coin.IsPositive() {
 				return false
 			}
+
 			// we compare each coin against the last denom
 			lowDenom = coin.Denom
 		}
+
 		return true
 	}
 }
 
-// Plus combines two sets of coins
+// Plus adds two sets of coins.
-// CONTRACT: Plus will never return Coins where one Coin has a 0 amount.
+//
+// e.g.
+// {2A} + {A, 2B} = {3A, 2B}
+// {2A} + {0B} = {2A}
+//
+// 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 Coins) Plus(coinsB Coins) Coins {
+	return coins.safePlus(coinsB)
+}
+
+// safePlus will perform addition of two coins 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 Coins) safePlus(coinsB Coins) Coins {
 	sum := ([]Coin)(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, removeZeroCoins(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, removeZeroCoins(coins[indexA:])...)
 		}
+
 		coinA, coinB := coins[indexA], coinsB[indexB]
+
 		switch strings.Compare(coinA.Denom, coinB.Denom) {
-		case -1:
+		case -1: // coin A denom < coin B denom
-			if coinA.IsZero() {
+			if !coinA.IsZero() {
-				// ignore 0 sum coin type
-			} else {
 				sum = append(sum, coinA)
 			}
+
 			indexA++
-		case 0:
+
-			if coinA.Amount.Add(coinB.Amount).IsZero() {
+		case 0: // coin A denom == coin B denom
-				// ignore 0 sum coin type
+			res := coinA.Plus(coinB)
-			} else {
+			if !res.IsZero() {
-				sum = append(sum, coinA.Plus(coinB))
+				sum = append(sum, res)
 			}
+
 			indexA++
 			indexB++
-		case 1:
+
-			if coinB.IsZero() {
+		case 1: // coin A denom > coin B denom
-				// ignore 0 sum coin type
+			if !coinB.IsZero() {
-			} else {
 				sum = append(sum, coinB)
 			}
+
 			indexB++
 		}
 	}
 }
 
-// Negative returns a set of coins with all amount negative
+// Minus subtracts a set of coins from another.
-func (coins Coins) Negative() Coins {
+//
-	res := make([]Coin, 0, len(coins))
+// e.g.
-	for _, coin := range coins {
+// {2A, 3B} - {A} = {A, 3B}
-		res = append(res, Coin{
+// {2A} - {0B} = {2A}
-			Denom:  coin.Denom,
+// {A, B} - {A} = {B}
-			Amount: coin.Amount.Neg(),
+//
-		})
+// CONTRACT: Minus will never return Coins where one Coin has a non-positive
-	}
+// amount. In otherwords, IsValid will always return true.
-	return res
-}
-
-// Minus subtracts a set of coins from another (adds the inverse)
 func (coins Coins) Minus(coinsB Coins) Coins {
-	return coins.Plus(coinsB.Negative())
+	diff, hasNeg := coins.SafeMinus(coinsB)
+	if hasNeg {
+		panic("negative coin amount")
+	}
+
+	return diff
 }
 
-// IsAllGT returns True iff for every denom in coins, the denom is present at a
+// SafeMinus performs the same arithmetic as Minus but returns a boolean if any
+// negative coin amount was returned.
+func (coins Coins) SafeMinus(coinsB Coins) (Coins, bool) {
+	diff := coins.safePlus(coinsB.negative())
+	return diff, !diff.IsNotNegative()
+}
+
+// IsAllGT returns true iff for every denom in coins, the denom is present at a
 // greater amount in coinsB.
 func (coins Coins) IsAllGT(coinsB Coins) bool {
-	diff := coins.Minus(coinsB)
+	diff, _ := coins.SafeMinus(coinsB)
 	if len(diff) == 0 {
 		return false
 	}
+
 	return diff.IsPositive()
 }
 
-// IsAllGTE returns True iff for every denom in coins, the denom is present at an
+// IsAllGTE returns true iff for every denom in coins, the denom is present at
-// equal or greater amount in coinsB.
+// an equal or greater amount in coinsB.
 func (coins Coins) IsAllGTE(coinsB Coins) bool {
-	diff := coins.Minus(coinsB)
+	diff, _ := coins.SafeMinus(coinsB)
 	if len(diff) == 0 {
 		return true
 	}
+
 	return diff.IsNotNegative()
 }
 
 // IsAllLT returns True iff for every denom in coins, the denom is present at
 // a smaller amount in coinsB.
 func (coins Coins) IsAllLT(coinsB Coins) bool {
-	diff := coinsB.Minus(coins)
+	return coinsB.IsAllGT(coins)
-	if len(diff) == 0 {
-		return false
-	}
-	return diff.IsPositive()
 }
 
-// IsAllLTE returns True iff for every denom in coins, the denom is present at
+// IsAllLTE returns true iff for every denom in coins, the denom is present at
 // a smaller or equal amount in coinsB.
 func (coins Coins) IsAllLTE(coinsB Coins) bool {
-	diff := coinsB.Minus(coins)
+	return coinsB.IsAllGTE(coins)
-	if len(diff) == 0 {
-		return true
-	}
-	return diff.IsNotNegative()
 }
 
 // IsZero returns true if there are no coins or all coins are zero.
@@ -239,40 +300,22 @@ func (coins Coins) IsEqual(coinsB Coins) bool {
 	if len(coins) != len(coinsB) {
 		return false
 	}
+
+	coins = coins.Sort()
+	coinsB = coinsB.Sort()
+
 	for i := 0; i < len(coins); i++ {
 		if coins[i].Denom != coinsB[i].Denom || !coins[i].Amount.Equal(coinsB[i].Amount) {
 			return false
 		}
 	}
+
 	return true
 }
 
-// IsPositive returns true if there is at least one coin, and all
+// Empty returns true if there are no coins and false otherwise.
-// currencies have a positive value
+func (coins Coins) Empty() bool {
-func (coins Coins) IsPositive() bool {
+	return len(coins) == 0
-	if len(coins) == 0 {
-		return false
-	}
-	for _, coin := range coins {
-		if !coin.IsPositive() {
-			return false
-		}
-	}
-	return true
-}
-
-// IsNotNegative returns true if there is no currency with a negative value
-// (even no coins is true here)
-func (coins Coins) IsNotNegative() bool {
-	if len(coins) == 0 {
-		return true
-	}
-	for _, coin := range coins {
-		if !coin.IsNotNegative() {
-			return false
-		}
-	}
-	return true
 }
 
 // Returns the amount of a denom from coins
@@ -280,15 +323,18 @@ func (coins Coins) AmountOf(denom string) Int {
 	switch len(coins) {
 	case 0:
 		return ZeroInt()
+
 	case 1:
 		coin := coins[0]
 		if coin.Denom == denom {
 			return coin.Amount
 		}
 		return ZeroInt()
+
 	default:
 		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 {
@@ -299,7 +345,75 @@ func (coins Coins) AmountOf(denom string) Int {
 	}
 }
 
-//----------------------------------------
+// IsPositive 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 {
+	if len(coins) == 0 {
+		return false
+	}
+
+	for _, coin := range coins {
+		if !coin.IsPositive() {
+			return false
+		}
+	}
+
+	return true
+}
+
+// IsNotNegative returns true if there is no coin amount with a negative value
+// (even no coins is true here).
+//
+// TODO: Remove once unsigned integers are used.
+func (coins Coins) IsNotNegative() bool {
+	if len(coins) == 0 {
+		return true
+	}
+
+	for _, coin := range coins {
+		if !coin.IsNotNegative() {
+			return false
+		}
+	}
+
+	return true
+}
+
+// negative returns a set of coins with all amount negative.
+//
+// TODO: Remove once unsigned integers are used.
+func (coins Coins) negative() Coins {
+	res := make([]Coin, 0, len(coins))
+
+	for _, coin := range coins {
+		res = append(res, Coin{
+			Denom:  coin.Denom,
+			Amount: coin.Amount.Neg(),
+		})
+	}
+
+	return res
+}
+
+// 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++
+		}
+	}
+
+	return coins[:i]
+}
+
+//-----------------------------------------------------------------------------
 // Sort interface
 
 //nolint
@@ -315,7 +429,7 @@ func (coins Coins) Sort() Coins {
 	return coins
 }
 
-//----------------------------------------
+//-----------------------------------------------------------------------------
 // Parsing
 
 var (
@@ -333,17 +447,17 @@ func ParseCoin(coinStr string) (coin Coin, err error) {
 
 	matches := reCoin.FindStringSubmatch(coinStr)
 	if matches == nil {
-		err = fmt.Errorf("invalid coin expression: %s", coinStr)
+		return Coin{}, fmt.Errorf("invalid coin expression: %s", coinStr)
-		return
 	}
+
 	denomStr, amountStr := matches[2], matches[1]
 
-	amount, err := strconv.Atoi(amountStr)
+	amount, ok := NewIntFromString(amountStr)
-	if err != nil {
+	if !ok {
-		return
+		return Coin{}, fmt.Errorf("failed to parse coin amount: %s", amountStr)
 	}
 
-	return Coin{denomStr, NewInt(int64(amount))}, nil
+	return Coin{denomStr, amount}, nil
 }
 
 // ParseCoins will parse out a list of coins separated by commas.

types/coin_benchmark_test.go

@@ -0,0 +1,64 @@
+package types
+
+import (
+	"fmt"
+	"testing"
+)
+
+func BenchmarkCoinsAdditionIntersect(b *testing.B) {
+	benchmarkingFunc := func(numCoinsA int, numCoinsB int) func(b *testing.B) {
+		return func(b *testing.B) {
+			coinsA := Coins(make([]Coin, numCoinsA))
+			coinsB := Coins(make([]Coin, numCoinsB))
+
+			for i := 0; i < numCoinsA; i++ {
+				coinsA[i] = NewCoin("COINZ_"+string(i), NewInt(int64(i)))
+			}
+			for i := 0; i < numCoinsB; i++ {
+				coinsB[i] = NewCoin("COINZ_"+string(i), NewInt(int64(i)))
+			}
+
+			b.ResetTimer()
+
+			for i := 0; i < b.N; i++ {
+				coinsA.Plus(coinsB)
+			}
+		}
+	}
+
+	benchmarkSizes := [][]int{{1, 1}, {5, 5}, {5, 20}, {1, 1000}, {2, 1000}}
+	for i := 0; i < len(benchmarkSizes); i++ {
+		sizeA := benchmarkSizes[i][0]
+		sizeB := benchmarkSizes[i][1]
+		b.Run(fmt.Sprintf("sizes: A_%d, B_%d", sizeA, sizeB), benchmarkingFunc(sizeA, sizeB))
+	}
+}
+
+func BenchmarkCoinsAdditionNoIntersect(b *testing.B) {
+	benchmarkingFunc := func(numCoinsA int, numCoinsB int) func(b *testing.B) {
+		return func(b *testing.B) {
+			coinsA := Coins(make([]Coin, numCoinsA))
+			coinsB := Coins(make([]Coin, numCoinsB))
+
+			for i := 0; i < numCoinsA; i++ {
+				coinsA[i] = NewCoin("COINZ_"+string(numCoinsB+i), NewInt(int64(i)))
+			}
+			for i := 0; i < numCoinsB; i++ {
+				coinsB[i] = NewCoin("COINZ_"+string(i), NewInt(int64(i)))
+			}
+
+			b.ResetTimer()
+
+			for i := 0; i < b.N; i++ {
+				coinsA.Plus(coinsB)
+			}
+		}
+	}
+
+	benchmarkSizes := [][]int{{1, 1}, {5, 5}, {5, 20}, {1, 1000}, {2, 1000}, {1000, 2}}
+	for i := 0; i < len(benchmarkSizes); i++ {
+		sizeA := benchmarkSizes[i][0]
+		sizeB := benchmarkSizes[i][1]
+		b.Run(fmt.Sprintf("sizes: A_%d, B_%d", sizeA, sizeB), benchmarkingFunc(sizeA, sizeB))
+	}
+}

types/coin_test.go

@@ -1,43 +1,20 @@
 package types
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-func TestIsPositiveCoin(t *testing.T) {
+// ----------------------------------------------------------------------------
-	cases := []struct {
+// Coin tests
-		inputOne Coin
-		expected bool
-	}{
-		{NewInt64Coin("A", 1), true},
-		{NewInt64Coin("A", 0), false},
-		{NewInt64Coin("a", -1), false},
-	}
 
-	for tcIndex, tc := range cases {
+func TestCoin(t *testing.T) {
-		res := tc.inputOne.IsPositive()
+	require.Panics(t, func() { NewInt64Coin("A", -1) })
-		require.Equal(t, tc.expected, res, "%s positivity is incorrect, tc #%d", tc.inputOne.String(), tcIndex)
+	require.Panics(t, func() { NewCoin("A", NewInt(-1)) })
-	}
+	require.Equal(t, NewInt(5), NewInt64Coin("A", 5).Amount)
-}
+	require.Equal(t, NewInt(5), NewCoin("A", NewInt(5)).Amount)
-
-func TestIsNotNegativeCoin(t *testing.T) {
-	cases := []struct {
-		inputOne Coin
-		expected bool
-	}{
-		{NewInt64Coin("A", 1), true},
-		{NewInt64Coin("A", 0), true},
-		{NewInt64Coin("a", -1), false},
-	}
-
-	for tcIndex, tc := range cases {
-		res := tc.inputOne.IsNotNegative()
-		require.Equal(t, tc.expected, res, "%s not-negativity is incorrect, tc #%d", tc.inputOne.String(), tcIndex)
-	}
 }
 
 func TestSameDenomAsCoin(t *testing.T) {
@@ -49,8 +26,7 @@ func TestSameDenomAsCoin(t *testing.T) {
 		{NewInt64Coin("A", 1), NewInt64Coin("A", 1), true},
 		{NewInt64Coin("A", 1), NewInt64Coin("a", 1), false},
 		{NewInt64Coin("a", 1), NewInt64Coin("b", 1), false},
-		{NewInt64Coin("stake", 1), NewInt64Coin("stake", 10), true},
+		{NewInt64Coin("steak", 1), NewInt64Coin("steak", 10), true},
-		{NewInt64Coin("stake", -11), NewInt64Coin("stake", 10), true},
 	}
 
 	for tcIndex, tc := range cases {
@@ -59,6 +35,78 @@ func TestSameDenomAsCoin(t *testing.T) {
 	}
 }
 
+func TestIsEqualCoin(t *testing.T) {
+	cases := []struct {
+		inputOne Coin
+		inputTwo Coin
+		expected bool
+	}{
+		{NewInt64Coin("A", 1), NewInt64Coin("A", 1), true},
+		{NewInt64Coin("A", 1), NewInt64Coin("a", 1), false},
+		{NewInt64Coin("a", 1), NewInt64Coin("b", 1), false},
+		{NewInt64Coin("steak", 1), NewInt64Coin("steak", 10), 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)
+	}
+}
+
+func TestPlusCoin(t *testing.T) {
+	cases := []struct {
+		inputOne    Coin
+		inputTwo    Coin
+		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},
+	}
+
+	for tcIndex, tc := range cases {
+		if tc.shouldPanic {
+			require.Panics(t, func() { tc.inputOne.Plus(tc.inputTwo) })
+		} else {
+			res := tc.inputOne.Plus(tc.inputTwo)
+			require.Equal(t, tc.expected, res, "sum of coins is incorrect, tc #%d", tcIndex)
+		}
+	}
+}
+
+func TestMinusCoin(t *testing.T) {
+	cases := []struct {
+		inputOne    Coin
+		inputTwo    Coin
+		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},
+	}
+
+	for tcIndex, tc := range cases {
+		if tc.shouldPanic {
+			require.Panics(t, func() { tc.inputOne.Minus(tc.inputTwo) })
+		} else {
+			res := tc.inputOne.Minus(tc.inputTwo)
+			require.Equal(t, tc.expected, res, "difference of coins is incorrect, tc #%d", tcIndex)
+		}
+	}
+
+	tc := struct {
+		inputOne Coin
+		inputTwo Coin
+		expected int64
+	}{NewInt64Coin("A", 1), NewInt64Coin("A", 1), 0}
+	res := tc.inputOne.Minus(tc.inputTwo)
+	require.Equal(t, tc.expected, res.Amount.Int64())
+}
+
 func TestIsGTECoin(t *testing.T) {
 	cases := []struct {
 		inputOne Coin
@@ -67,8 +115,7 @@ func TestIsGTECoin(t *testing.T) {
 	}{
 		{NewInt64Coin("A", 1), NewInt64Coin("A", 1), true},
 		{NewInt64Coin("A", 2), NewInt64Coin("A", 1), true},
-		{NewInt64Coin("A", -1), NewInt64Coin("A", 5), false},
+		{NewInt64Coin("A", 1), NewInt64Coin("B", 1), false},
-		{NewInt64Coin("a", 1), NewInt64Coin("b", 1), false},
 	}
 
 	for tcIndex, tc := range cases {
@@ -85,7 +132,6 @@ func TestIsLTCoin(t *testing.T) {
 	}{
 		{NewInt64Coin("A", 1), NewInt64Coin("A", 1), false},
 		{NewInt64Coin("A", 2), NewInt64Coin("A", 1), false},
-		{NewInt64Coin("A", -1), NewInt64Coin("A", 5), true},
 		{NewInt64Coin("a", 0), NewInt64Coin("b", 1), false},
 		{NewInt64Coin("a", 1), NewInt64Coin("b", 1), false},
 		{NewInt64Coin("a", 1), NewInt64Coin("a", 1), false},
@@ -98,76 +144,18 @@ func TestIsLTCoin(t *testing.T) {
 	}
 }
 
-func TestIsEqualCoin(t *testing.T) {
+func TestCoinIsZero(t *testing.T) {
-	cases := []struct {
+	coin := NewInt64Coin("A", 0)
-		inputOne Coin
+	res := coin.IsZero()
-		inputTwo Coin
+	require.True(t, res)
-		expected bool
-	}{
-		{NewInt64Coin("A", 1), NewInt64Coin("A", 1), true},
-		{NewInt64Coin("A", 1), NewInt64Coin("a", 1), false},
-		{NewInt64Coin("a", 1), NewInt64Coin("b", 1), false},
-		{NewInt64Coin("stake", 1), NewInt64Coin("stake", 10), false},
-		{NewInt64Coin("stake", -11), NewInt64Coin("stake", 10), false},
-	}
 
-	for tcIndex, tc := range cases {
+	coin = NewInt64Coin("A", 1)
-		res := tc.inputOne.IsEqual(tc.inputTwo)
+	res = coin.IsZero()
-		require.Equal(t, tc.expected, res, "coin equality relation is incorrect, tc #%d", tcIndex)
+	require.False(t, res)
-	}
 }
 
-func TestPlusCoin(t *testing.T) {
+// ----------------------------------------------------------------------------
-	cases := []struct {
+// Coins tests
-		inputOne Coin
-		inputTwo Coin
-		expected Coin
-	}{
-		{NewInt64Coin("A", 1), NewInt64Coin("A", 1), NewInt64Coin("A", 2)},
-		{NewInt64Coin("A", 1), NewInt64Coin("B", 1), NewInt64Coin("A", 1)},
-		{NewInt64Coin("asdf", -4), NewInt64Coin("asdf", 5), NewInt64Coin("asdf", 1)},
-	}
-
-	for tcIndex, tc := range cases {
-		res := tc.inputOne.Plus(tc.inputTwo)
-		require.Equal(t, tc.expected, res, "sum of coins is incorrect, tc #%d", tcIndex)
-	}
-
-	tc := struct {
-		inputOne Coin
-		inputTwo Coin
-		expected int64
-	}{NewInt64Coin("asdf", -1), NewInt64Coin("asdf", 1), 0}
-	res := tc.inputOne.Plus(tc.inputTwo)
-	require.Equal(t, tc.expected, res.Amount.Int64())
-}
-
-func TestMinusCoin(t *testing.T) {
-	cases := []struct {
-		inputOne Coin
-		inputTwo Coin
-		expected Coin
-	}{
-
-		{NewInt64Coin("A", 1), NewInt64Coin("B", 1), NewInt64Coin("A", 1)},
-		{NewInt64Coin("asdf", -4), NewInt64Coin("asdf", 5), NewInt64Coin("asdf", -9)},
-		{NewInt64Coin("asdf", 10), NewInt64Coin("asdf", 1), NewInt64Coin("asdf", 9)},
-	}
-
-	for tcIndex, tc := range cases {
-		res := tc.inputOne.Minus(tc.inputTwo)
-		require.Equal(t, tc.expected, res, "difference of coins is incorrect, tc #%d", tcIndex)
-	}
-
-	tc := struct {
-		inputOne Coin
-		inputTwo Coin
-		expected int64
-	}{NewInt64Coin("A", 1), NewInt64Coin("A", 1), 0}
-	res := tc.inputOne.Minus(tc.inputTwo)
-	require.Equal(t, tc.expected, res.Amount.Int64())
-
-}
 
 func TestIsZeroCoins(t *testing.T) {
 	cases := []struct {
@@ -199,8 +187,7 @@ func TestEqualCoins(t *testing.T) {
 		{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},
-		// TODO: is it expected behaviour? shouldn't we sort the coins before comparing them?
+		{Coins{NewInt64Coin("A", 0), NewInt64Coin("B", 1)}, Coins{NewInt64Coin("B", 1), NewInt64Coin("A", 0)}, true},
-		{Coins{NewInt64Coin("A", 0), NewInt64Coin("B", 1)}, Coins{NewInt64Coin("B", 1), NewInt64Coin("A", 0)}, false},
 	}
 
 	for tcnum, tc := range cases {
@@ -209,16 +196,65 @@ func TestEqualCoins(t *testing.T) {
 	}
 }
 
-func TestCoins(t *testing.T) {
+func TestPlusCoins(t *testing.T) {
+	zero := NewInt(0)
+	one := NewInt(1)
+	two := NewInt(2)
 
-	//Define the coins to be used in tests
+	cases := []struct {
+		inputOne Coins
+		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)},
+	}
+
+	for tcIndex, tc := range cases {
+		res := tc.inputOne.Plus(tc.inputTwo)
+		assert.True(t, res.IsValid())
+		require.Equal(t, tc.expected, res, "sum of coins is incorrect, tc #%d", tcIndex)
+	}
+}
+
+func TestMinusCoins(t *testing.T) {
+	zero := NewInt(0)
+	one := NewInt(1)
+	two := NewInt(2)
+
+	testCases := []struct {
+		inputOne    Coins
+		inputTwo    Coins
+		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},
+	}
+
+	for i, tc := range testCases {
+		if tc.shouldPanic {
+			require.Panics(t, func() { tc.inputOne.Minus(tc.inputTwo) })
+		} else {
+			res := tc.inputOne.Minus(tc.inputTwo)
+			assert.True(t, res.IsValid())
+			require.Equal(t, tc.expected, res, "sum of coins is incorrect, tc #%d", i)
+		}
+	}
+}
+
+func TestCoins(t *testing.T) {
 	good := Coins{
 		{"GAS", NewInt(1)},
 		{"MINERAL", NewInt(1)},
 		{"TREE", NewInt(1)},
 	}
-	neg := good.Negative()
-	sum := good.Plus(neg)
 	empty := Coins{
 		{"GOLD", NewInt(0)},
 	}
@@ -228,6 +264,7 @@ func TestCoins(t *testing.T) {
 		{"GAS", NewInt(1)},
 		{"MINERAL", NewInt(1)},
 	}
+
 	// both are after the first one, but the second and third are in the wrong order
 	badSort2 := Coins{
 		{"GAS", NewInt(1)},
@@ -251,13 +288,10 @@ func TestCoins(t *testing.T) {
 	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)
-	assert.False(t, neg.IsPositive(), "Expected neg coins to not be positive: %v", neg)
-	assert.Zero(t, len(sum), "Expected 0 coins")
 	assert.False(t, badSort1.IsValid(), "Coins are not sorted")
 	assert.False(t, badSort2.IsValid(), "Coins are not sorted")
 	assert.False(t, badAmt.IsValid(), "Coins cannot include 0 amounts")
 	assert.False(t, dup.IsValid(), "Duplicate coin")
-
 }
 
 func TestCoinsGT(t *testing.T) {
@@ -314,32 +348,6 @@ func TestCoinsLTE(t *testing.T) {
 	assert.True(t, Coins{}.IsAllLTE(Coins{{"A", one}}))
 }
 
-func TestPlusCoins(t *testing.T) {
-	one := NewInt(1)
-	zero := NewInt(0)
-	negone := NewInt(-1)
-	two := NewInt(2)
-
-	cases := []struct {
-		inputOne Coins
-		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", zero}, {"B", zero}}, Coins{{"A", zero}, {"B", zero}}, Coins(nil)},
-		{Coins{{"A", one}, {"B", zero}}, Coins{{"A", negone}, {"B", zero}}, Coins(nil)},
-		{Coins{{"A", negone}, {"B", zero}}, Coins{{"A", zero}, {"B", zero}}, Coins{{"A", negone}}},
-	}
-
-	for tcIndex, tc := range cases {
-		res := tc.inputOne.Plus(tc.inputTwo)
-		assert.True(t, res.IsValid())
-		require.Equal(t, tc.expected, res, "sum of coins is incorrect, tc #%d", tcIndex)
-	}
-}
-
-//Test the parsing of Coin and Coins
 func TestParse(t *testing.T) {
 	one := NewInt(1)
 
@@ -370,11 +378,9 @@ func TestParse(t *testing.T) {
 			require.Equal(t, tc.expected, res, "coin parsing was incorrect, tc #%d", tcIndex)
 		}
 	}
-
 }
 
 func TestSortCoins(t *testing.T) {
-
 	good := Coins{
 		NewInt64Coin("GAS", 1),
 		NewInt64Coin("MINERAL", 1),
@@ -424,7 +430,6 @@ func TestSortCoins(t *testing.T) {
 }
 
 func TestAmountOf(t *testing.T) {
-
 	case0 := Coins{}
 	case1 := Coins{
 		NewInt64Coin("", 0),
@@ -481,55 +486,3 @@ func TestAmountOf(t *testing.T) {
 		assert.Equal(t, NewInt(tc.amountOfTREE), tc.coins.AmountOf("TREE"))
 	}
 }
-
-func BenchmarkCoinsAdditionIntersect(b *testing.B) {
-	benchmarkingFunc := func(numCoinsA int, numCoinsB int) func(b *testing.B) {
-		return func(b *testing.B) {
-			coinsA := Coins(make([]Coin, numCoinsA))
-			coinsB := Coins(make([]Coin, numCoinsB))
-			for i := 0; i < numCoinsA; i++ {
-				coinsA[i] = NewCoin("COINZ_"+string(i), NewInt(int64(i)))
-			}
-			for i := 0; i < numCoinsB; i++ {
-				coinsB[i] = NewCoin("COINZ_"+string(i), NewInt(int64(i)))
-			}
-			b.ResetTimer()
-			for i := 0; i < b.N; i++ {
-				coinsA.Plus(coinsB)
-			}
-		}
-	}
-
-	benchmarkSizes := [][]int{{1, 1}, {5, 5}, {5, 20}, {1, 1000}, {2, 1000}}
-	for i := 0; i < len(benchmarkSizes); i++ {
-		sizeA := benchmarkSizes[i][0]
-		sizeB := benchmarkSizes[i][1]
-		b.Run(fmt.Sprintf("sizes: A_%d, B_%d", sizeA, sizeB), benchmarkingFunc(sizeA, sizeB))
-	}
-}
-
-func BenchmarkCoinsAdditionNoIntersect(b *testing.B) {
-	benchmarkingFunc := func(numCoinsA int, numCoinsB int) func(b *testing.B) {
-		return func(b *testing.B) {
-			coinsA := Coins(make([]Coin, numCoinsA))
-			coinsB := Coins(make([]Coin, numCoinsB))
-			for i := 0; i < numCoinsA; i++ {
-				coinsA[i] = NewCoin("COINZ_"+string(numCoinsB+i), NewInt(int64(i)))
-			}
-			for i := 0; i < numCoinsB; i++ {
-				coinsB[i] = NewCoin("COINZ_"+string(i), NewInt(int64(i)))
-			}
-			b.ResetTimer()
-			for i := 0; i < b.N; i++ {
-				coinsA.Plus(coinsB)
-			}
-		}
-	}
-
-	benchmarkSizes := [][]int{{1, 1}, {5, 5}, {5, 20}, {1, 1000}, {2, 1000}, {1000, 2}}
-	for i := 0; i < len(benchmarkSizes); i++ {
-		sizeA := benchmarkSizes[i][0]
-		sizeB := benchmarkSizes[i][1]
-		b.Run(fmt.Sprintf("sizes: A_%d, B_%d", sizeA, sizeB), benchmarkingFunc(sizeA, sizeB))
-	}
-}

types/int.go

@@ -322,11 +322,11 @@ func NewUint(n uint64) Uint {
 
 // NewUintFromBigUint constructs Uint from big.Uint
 func NewUintFromBigInt(i *big.Int) Uint {
-	// Check overflow
+	res := Uint{i}
-	if i.Sign() == -1 || i.Sign() == 1 && i.BitLen() > 256 {
+	if UintOverflow(res) {
 		panic("Uint overflow")
 	}
-	return Uint{i}
+	return res
 }
 
 // NewUintFromString constructs Uint from string
@@ -353,11 +353,12 @@ func NewUintWithDecimal(n uint64, dec int) Uint {
 	i := new(big.Int)
 	i.Mul(new(big.Int).SetUint64(n), exp)
 
-	// Check overflow
+	res := Uint{i}
-	if i.Sign() == -1 || i.Sign() == 1 && i.BitLen() > 256 {
+	if UintOverflow(res) {
 		panic("NewUintWithDecimal() out of bound")
 	}
-	return Uint{i}
+
+	return res
 }
 
 // ZeroUint returns Uint value with zero
@@ -408,8 +409,7 @@ func (i Uint) LT(i2 Uint) bool {
 // Add adds Uint from another
 func (i Uint) Add(i2 Uint) (res Uint) {
 	res = Uint{add(i.i, i2.i)}
-	// Check overflow
+	if UintOverflow(res) {
-	if res.Sign() == -1 || res.Sign() == 1 && res.i.BitLen() > 256 {
 		panic("Uint overflow")
 	}
 	return
@@ -423,13 +423,23 @@ func (i Uint) AddRaw(i2 uint64) Uint {
 // Sub subtracts Uint from another
 func (i Uint) Sub(i2 Uint) (res Uint) {
 	res = Uint{sub(i.i, i2.i)}
-	// Check overflow
+	if UintOverflow(res) {
-	if res.Sign() == -1 || res.Sign() == 1 && res.i.BitLen() > 256 {
 		panic("Uint overflow")
 	}
 	return
 }
 
+// SafeSub attempts to subtract one Uint from another. A boolean is also returned
+// indicating if the result contains integer overflow.
+func (i Uint) SafeSub(i2 Uint) (Uint, bool) {
+	res := Uint{sub(i.i, i2.i)}
+	if UintOverflow(res) {
+		return res, true
+	}
+
+	return res, false
+}
+
 // SubRaw subtracts uint64 from Uint
 func (i Uint) SubRaw(i2 uint64) Uint {
 	return i.Sub(NewUint(i2))
@@ -437,15 +447,15 @@ func (i Uint) SubRaw(i2 uint64) Uint {
 
 // Mul multiples two Uints
 func (i Uint) Mul(i2 Uint) (res Uint) {
-	// Check overflow
 	if i.i.BitLen()+i2.i.BitLen()-1 > 256 {
 		panic("Uint overflow")
 	}
+
 	res = Uint{mul(i.i, i2.i)}
-	// Check overflow
+	if UintOverflow(res) {
-	if res.Sign() == -1 || res.Sign() == 1 && res.i.BitLen() > 256 {
 		panic("Uint overflow")
 	}
+
 	return
 }
 
@@ -530,6 +540,12 @@ func (i *Uint) UnmarshalJSON(bz []byte) error {
 
 //__________________________________________________________________________
 
+// UintOverflow returns true if a given unsigned integer overflows and false
+// otherwise.
+func UintOverflow(x Uint) bool {
+	return x.i.Sign() == -1 || x.i.Sign() == 1 && x.i.BitLen() > 256
+}
+
 // AddUint64Overflow performs the addition operation on two uint64 integers and
 // returns a boolean on whether or not the result overflows.
 func AddUint64Overflow(a, b uint64) (uint64, bool) {

types/int_test.go

@@ -591,6 +591,31 @@ func TestEncodingTableUint(t *testing.T) {
 	}
 }
 
+func TestSafeSub(t *testing.T) {
+	testCases := []struct {
+		x, y     Uint
+		expected uint64
+		overflow bool
+	}{
+		{NewUint(0), NewUint(0), 0, false},
+		{NewUint(10), NewUint(5), 5, false},
+		{NewUint(5), NewUint(10), 5, true},
+		{NewUint(math.MaxUint64), NewUint(0), math.MaxUint64, false},
+	}
+
+	for i, tc := range testCases {
+		res, overflow := tc.x.SafeSub(tc.y)
+		require.Equal(
+			t, tc.overflow, overflow,
+			"invalid overflow result; x: %s, y: %s, tc: #%d", tc.x, tc.y, i,
+		)
+		require.Equal(
+			t, tc.expected, res.BigInt().Uint64(),
+			"invalid subtraction result; x: %s, y: %s, tc: #%d", tc.x, tc.y, i,
+		)
+	}
+}
+
 func TestAddUint64Overflow(t *testing.T) {
 	testCases := []struct {
 		a, b     uint64

x/auth/ante.go

@@ -281,24 +281,36 @@ func deductFees(acc Account, fee StdFee) (Account, sdk.Result) {
 	coins := acc.GetCoins()
 	feeAmount := fee.Amount
 
-	newCoins := coins.Minus(feeAmount)
+	if !feeAmount.IsValid() {
-	if !newCoins.IsNotNegative() {
+		return nil, sdk.ErrInsufficientFee(fmt.Sprintf("invalid fee amount: %s", feeAmount)).Result()
+	}
+
+	newCoins, ok := coins.SafeMinus(feeAmount)
+	if ok {
 		errMsg := fmt.Sprintf("%s < %s", coins, feeAmount)
 		return nil, sdk.ErrInsufficientFunds(errMsg).Result()
 	}
+
 	err := acc.SetCoins(newCoins)
 	if err != nil {
 		// Handle w/ #870
 		panic(err)
 	}
+
 	return acc, sdk.Result{}
 }
 
 func ensureSufficientMempoolFees(ctx sdk.Context, stdTx StdTx) sdk.Result {
 	// currently we use a very primitive gas pricing model with a constant gasPrice.
 	// adjustFeesByGas handles calculating the amount of fees required based on the provided gas.
-	// TODO: Make the gasPrice not a constant, and account for tx size.
+	//
-	requiredFees := adjustFeesByGas(ctx.MinimumFees(), stdTx.Fee.Gas)
+	// TODO:
+	// - Make the gasPrice not a constant, and account for tx size.
+	// - Make Gas an unsigned integer and use tx basic validation
+	if stdTx.Fee.Gas <= 0 {
+		return sdk.ErrInternal(fmt.Sprintf("invalid gas supplied: %d", stdTx.Fee.Gas)).Result()
+	}
+	requiredFees := adjustFeesByGas(ctx.MinimumFees(), uint64(stdTx.Fee.Gas))
 
 	// NOTE: !A.IsAllGTE(B) is not the same as A.IsAllLT(B).
 	if !ctx.MinimumFees().IsZero() && !stdTx.Fee.Amount.IsAllGTE(requiredFees) {

x/auth/ante_test.go

@@ -708,7 +708,6 @@ func TestAdjustFeesByGas(t *testing.T) {
 	}{
 		{"nil coins", args{sdk.Coins{}, 10000}, sdk.Coins{}},
 		{"nil coins", args{sdk.Coins{sdk.NewInt64Coin("A", 10), sdk.NewInt64Coin("B", 0)}, 10000}, sdk.Coins{sdk.NewInt64Coin("A", 20), sdk.NewInt64Coin("B", 10)}},
-		{"negative coins", args{sdk.Coins{sdk.NewInt64Coin("A", -10), sdk.NewInt64Coin("B", 10)}, 10000}, sdk.Coins{sdk.NewInt64Coin("B", 20)}},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

x/bank/keeper.go

@@ -182,11 +182,13 @@ func hasCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt s
 // SubtractCoins subtracts amt from the coins at the addr.
 func subtractCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error) {
 	ctx.GasMeter().ConsumeGas(costSubtractCoins, "subtractCoins")
+
 	oldCoins := getCoins(ctx, am, addr)
-	newCoins := oldCoins.Minus(amt)
+	newCoins, hasNeg := oldCoins.SafeMinus(amt)
-	if !newCoins.IsNotNegative() {
+	if hasNeg {
 		return amt, nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", oldCoins, amt))
 	}
+
 	err := setCoins(ctx, am, addr, newCoins)
 	tags := sdk.NewTags("sender", []byte(addr.String()))
 	return newCoins, tags, err

x/bank/msgs_test.go

@@ -35,8 +35,6 @@ func TestInputValidation(t *testing.T) {
 	emptyCoins := sdk.Coins{}
 	emptyCoins2 := sdk.Coins{sdk.NewInt64Coin("eth", 0)}
 	someEmptyCoins := sdk.Coins{sdk.NewInt64Coin("eth", 10), sdk.NewInt64Coin("atom", 0)}
-	minusCoins := sdk.Coins{sdk.NewInt64Coin("eth", -34)}
-	someMinusCoins := sdk.Coins{sdk.NewInt64Coin("atom", 20), sdk.NewInt64Coin("eth", -34)}
 	unsortedCoins := sdk.Coins{sdk.NewInt64Coin("eth", 1), sdk.NewInt64Coin("atom", 1)}
 
 	cases := []struct {
@@ -52,8 +50,6 @@ func TestInputValidation(t *testing.T) {
 		{false, NewInput(addr1, emptyCoins)},     // invalid coins
 		{false, NewInput(addr1, emptyCoins2)},    // invalid coins
 		{false, NewInput(addr1, someEmptyCoins)}, // invalid coins
-		{false, NewInput(addr1, minusCoins)},     // negative coins
-		{false, NewInput(addr1, someMinusCoins)}, // negative coins
 		{false, NewInput(addr1, unsortedCoins)},  // unsorted coins
 	}
 
@@ -77,8 +73,6 @@ func TestOutputValidation(t *testing.T) {
 	emptyCoins := sdk.Coins{}
 	emptyCoins2 := sdk.Coins{sdk.NewInt64Coin("eth", 0)}
 	someEmptyCoins := sdk.Coins{sdk.NewInt64Coin("eth", 10), sdk.NewInt64Coin("atom", 0)}
-	minusCoins := sdk.Coins{sdk.NewInt64Coin("eth", -34)}
-	someMinusCoins := sdk.Coins{sdk.NewInt64Coin("atom", 20), sdk.NewInt64Coin("eth", -34)}
 	unsortedCoins := sdk.Coins{sdk.NewInt64Coin("eth", 1), sdk.NewInt64Coin("atom", 1)}
 
 	cases := []struct {
@@ -94,8 +88,6 @@ func TestOutputValidation(t *testing.T) {
 		{false, NewOutput(addr1, emptyCoins)},     // invalid coins
 		{false, NewOutput(addr1, emptyCoins2)},    // invalid coins
 		{false, NewOutput(addr1, someEmptyCoins)}, // invalid coins
-		{false, NewOutput(addr1, minusCoins)},     // negative coins
-		{false, NewOutput(addr1, someMinusCoins)}, // negative coins
 		{false, NewOutput(addr1, unsortedCoins)},  // unsorted coins
 	}
 

x/bank/simulation/msgs.go

@@ -82,7 +82,7 @@ func createSingleInputSendMsg(r *rand.Rand, ctx sdk.Context, accs []simulation.A
 		toAddr.String(),
 	)
 
-	coins := sdk.Coins{{initFromCoins[denomIndex].Denom, amt}}
+	coins := sdk.Coins{sdk.NewCoin(initFromCoins[denomIndex].Denom, amt)}
 	msg = bank.MsgSend{
 		Inputs:  []bank.Input{bank.NewInput(fromAcc.Address, coins)},
 		Outputs: []bank.Output{bank.NewOutput(toAddr, coins)},

x/gov/msgs_test.go

@@ -13,7 +13,6 @@ import (
 var (
 	coinsPos         = sdk.Coins{sdk.NewInt64Coin(stakeTypes.DefaultBondDenom, 1000)}
 	coinsZero        = sdk.Coins{}
-	coinsNeg         = sdk.Coins{sdk.NewInt64Coin(stakeTypes.DefaultBondDenom, -10000)}
 	coinsPosNotAtoms = sdk.Coins{sdk.NewInt64Coin("foo", 10000)}
 	coinsMulti       = sdk.Coins{sdk.NewInt64Coin(stakeTypes.DefaultBondDenom, 1000), sdk.NewInt64Coin("foo", 10000)}
 )
@@ -40,7 +39,6 @@ func TestMsgSubmitProposal(t *testing.T) {
 		{"Test Proposal", "the purpose of this proposal is to test", 0x05, addrs[0], coinsPos, false},
 		{"Test Proposal", "the purpose of this proposal is to test", ProposalTypeText, sdk.AccAddress{}, coinsPos, false},
 		{"Test Proposal", "the purpose of this proposal is to test", ProposalTypeText, addrs[0], coinsZero, true},
-		{"Test Proposal", "the purpose of this proposal is to test", ProposalTypeText, addrs[0], coinsNeg, false},
 		{"Test Proposal", "the purpose of this proposal is to test", ProposalTypeText, addrs[0], coinsMulti, true},
 	}
 
@@ -66,7 +64,6 @@ func TestMsgDeposit(t *testing.T) {
 		{0, addrs[0], coinsPos, true},
 		{1, sdk.AccAddress{}, coinsPos, false},
 		{1, addrs[0], coinsZero, true},
-		{1, addrs[0], coinsNeg, false},
 		{1, addrs[0], coinsMulti, true},
 	}
 

x/slashing/handler_test.go

@@ -20,7 +20,11 @@ func TestCannotUnjailUnlessJailed(t *testing.T) {
 	got := stake.NewHandler(sk)(ctx, msg)
 	require.True(t, got.IsOK())
 	stake.EndBlocker(ctx, sk)
-	require.Equal(t, ck.GetCoins(ctx, sdk.AccAddress(addr)), sdk.Coins{{sk.GetParams(ctx).BondDenom, initCoins.Sub(amt)}})
+
+	require.Equal(
+		t, ck.GetCoins(ctx, sdk.AccAddress(addr)),
+		sdk.Coins{sdk.NewCoin(sk.GetParams(ctx).BondDenom, initCoins.Sub(amt))},
+	)
 	require.True(t, sdk.NewDecFromInt(amt).Equal(sk.Validator(ctx, addr).GetPower()))
 
 	// assert non-jailed validator can't be unjailed

x/slashing/keeper_test.go

@@ -35,7 +35,10 @@ func TestHandleDoubleSign(t *testing.T) {
 	got := stake.NewHandler(sk)(ctx, NewTestMsgCreateValidator(operatorAddr, val, amt))
 	require.True(t, got.IsOK())
 	stake.EndBlocker(ctx, sk)
-	require.Equal(t, ck.GetCoins(ctx, sdk.AccAddress(operatorAddr)), sdk.Coins{{sk.GetParams(ctx).BondDenom, initCoins.Sub(amt)}})
+	require.Equal(
+		t, ck.GetCoins(ctx, sdk.AccAddress(operatorAddr)),
+		sdk.Coins{sdk.NewCoin(sk.GetParams(ctx).BondDenom, initCoins.Sub(amt))},
+	)
 	require.True(t, sdk.NewDecFromInt(amt).Equal(sk.Validator(ctx, operatorAddr).GetPower()))
 
 	// handle a signature to set signing info
@@ -76,7 +79,10 @@ func TestSlashingPeriodCap(t *testing.T) {
 	require.True(t, got.IsOK())
 	stake.EndBlocker(ctx, sk)
 	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1)
-	require.Equal(t, ck.GetCoins(ctx, sdk.AccAddress(operatorAddr)), sdk.Coins{{sk.GetParams(ctx).BondDenom, initCoins.Sub(amt)}})
+	require.Equal(
+		t, ck.GetCoins(ctx, sdk.AccAddress(operatorAddr)),
+		sdk.Coins{sdk.NewCoin(sk.GetParams(ctx).BondDenom, initCoins.Sub(amt))},
+	)
 	require.True(t, sdk.NewDecFromInt(amt).Equal(sk.Validator(ctx, operatorAddr).GetPower()))
 
 	// handle a signature to set signing info
@@ -140,8 +146,13 @@ func TestHandleAbsentValidator(t *testing.T) {
 	got := sh(ctx, NewTestMsgCreateValidator(addr, val, amt))
 	require.True(t, got.IsOK())
 	stake.EndBlocker(ctx, sk)
-	require.Equal(t, ck.GetCoins(ctx, sdk.AccAddress(addr)), sdk.Coins{{sk.GetParams(ctx).BondDenom, initCoins.Sub(amt)}})
+
+	require.Equal(
+		t, ck.GetCoins(ctx, sdk.AccAddress(addr)),
+		sdk.Coins{sdk.NewCoin(sk.GetParams(ctx).BondDenom, initCoins.Sub(amt))},
+	)
 	require.True(t, sdk.NewDecFromInt(amt).Equal(sk.Validator(ctx, addr).GetPower()))
+
 	// will exist since the validator has been bonded
 	info, found := keeper.getValidatorSigningInfo(ctx, sdk.ConsAddress(val.Address()))
 	require.True(t, found)
@@ -296,7 +307,11 @@ func TestHandleNewValidator(t *testing.T) {
 	got := sh(ctx, NewTestMsgCreateValidator(addr, val, sdk.NewInt(amt)))
 	require.True(t, got.IsOK())
 	stake.EndBlocker(ctx, sk)
-	require.Equal(t, ck.GetCoins(ctx, sdk.AccAddress(addr)), sdk.Coins{{sk.GetParams(ctx).BondDenom, initCoins.SubRaw(amt)}})
+
+	require.Equal(
+		t, ck.GetCoins(ctx, sdk.AccAddress(addr)),
+		sdk.Coins{sdk.NewCoin(sk.GetParams(ctx).BondDenom, initCoins.SubRaw(amt))},
+	)
 	require.Equal(t, sdk.NewDec(amt), sk.Validator(ctx, addr).GetPower())
 
 	// Now a validator, for two blocks

x/slashing/tick_test.go

@@ -20,7 +20,10 @@ func TestBeginBlocker(t *testing.T) {
 	got := stake.NewHandler(sk)(ctx, NewTestMsgCreateValidator(addr, pk, amt))
 	require.True(t, got.IsOK())
 	stake.EndBlocker(ctx, sk)
-	require.Equal(t, ck.GetCoins(ctx, sdk.AccAddress(addr)), sdk.Coins{{sk.GetParams(ctx).BondDenom, initCoins.Sub(amt)}})
+	require.Equal(
+		t, ck.GetCoins(ctx, sdk.AccAddress(addr)),
+		sdk.Coins{sdk.NewCoin(sk.GetParams(ctx).BondDenom, initCoins.Sub(amt))},
+	)
 	require.True(t, sdk.NewDecFromInt(amt).Equal(sk.Validator(ctx, addr).GetPower()))
 
 	val := abci.Validator{

x/stake/types/msg_test.go

@@ -12,7 +12,6 @@ import (
 var (
 	coinPos  = sdk.NewInt64Coin(DefaultBondDenom, 1000)
 	coinZero = sdk.NewInt64Coin(DefaultBondDenom, 0)
-	coinNeg  = sdk.NewInt64Coin(DefaultBondDenom, -10000)
 )
 
 // test ValidateBasic for MsgCreateValidator
@@ -34,8 +33,6 @@ func TestMsgCreateValidator(t *testing.T) {
 		{"empty address", "a", "b", "c", "d", commission2, emptyAddr, pk1, coinPos, false},
 		{"empty pubkey", "a", "b", "c", "d", commission1, addr1, emptyPubkey, coinPos, true},
 		{"empty bond", "a", "b", "c", "d", commission2, addr1, pk1, coinZero, false},
-		{"negative bond", "a", "b", "c", "d", commission2, addr1, pk1, coinNeg, false},
-		{"negative bond", "a", "b", "c", "d", commission1, addr1, pk1, coinNeg, false},
 	}
 
 	for _, tc := range tests {
@@ -96,8 +93,6 @@ func TestMsgCreateValidatorOnBehalfOf(t *testing.T) {
 		{"empty validator address", "a", "b", "c", "d", commission2, sdk.AccAddress(addr1), emptyAddr, pk2, coinPos, false},
 		{"empty pubkey", "a", "b", "c", "d", commission1, sdk.AccAddress(addr1), addr2, emptyPubkey, coinPos, true},
 		{"empty bond", "a", "b", "c", "d", commission2, sdk.AccAddress(addr1), addr2, pk2, coinZero, false},
-		{"negative bond", "a", "b", "c", "d", commission1, sdk.AccAddress(addr1), addr2, pk2, coinNeg, false},
-		{"negative bond", "a", "b", "c", "d", commission2, sdk.AccAddress(addr1), addr2, pk2, coinNeg, false},
 	}
 
 	for _, tc := range tests {
@@ -136,7 +131,6 @@ func TestMsgDelegate(t *testing.T) {
 		{"empty delegator", sdk.AccAddress(emptyAddr), addr1, coinPos, false},
 		{"empty validator", sdk.AccAddress(addr1), emptyAddr, coinPos, false},
 		{"empty bond", sdk.AccAddress(addr1), addr2, coinZero, false},
-		{"negative bond", sdk.AccAddress(addr1), addr2, coinNeg, false},
 	}
 
 	for _, tc := range tests {