Merge PR cosmos#3666: improve denom validation

karzak · Mar 1, 2019 · b47032d · b47032d
1 parent bdf55d9
commit b47032d
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 23 deletions.
diff --git a/PENDING.md b/PENDING.md
@@ -23,6 +23,7 @@
 
 * [\#3669] Ensure consistency in message naming, codec registration, and JSON
 tags.
+* [\#3666] Improve coins denom validation.
 * [\#3751] Disable (temporarily) support for ED25519 account key pairs.
 
 ### Tendermint

diff --git a/types/coin.go b/types/coin.go
@@ -1,6 +1,7 @@
 package types
 
 import (
+	"errors"
 	"fmt"
 	"regexp"
 	"sort"
@@ -27,10 +28,10 @@ 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)
+	mustValidateDenom(denom)
 
 	if amount.LT(ZeroInt()) {
-		panic(fmt.Sprintf("negative coin amount: %v\n", amount))
+		panic(fmt.Errorf("negative coin amount: %v", amount))
 	}
 
 	return Coin{
@@ -148,7 +149,7 @@ func (coins Coins) IsValid() bool {
 	case 0:
 		return true
 	case 1:
-		if strings.ToLower(coins[0].Denom) != coins[0].Denom {
+		if err := validateDenom(coins[0].Denom); err != nil {
 			return false
 		}
 		return coins[0].IsPositive()
@@ -360,7 +361,7 @@ func (coins Coins) Empty() bool {
 
 // Returns the amount of a denom from coins
 func (coins Coins) AmountOf(denom string) Int {
-	validateDenom(denom)
+	mustValidateDenom(denom)
 
 	switch len(coins) {
 	case 0:
@@ -477,20 +478,25 @@ func (coins Coins) Sort() Coins {
 
 var (
 	// Denominations can be 3 ~ 16 characters long.
-	reDnm     = `[[:alpha:]][[:alnum:]]{2,15}`
-	reAmt     = `[[:digit:]]+`
-	reDecAmt  = `[[:digit:]]*\.[[:digit:]]+`
-	reSpc     = `[[:space:]]*`
-	reCoin    = regexp.MustCompile(fmt.Sprintf(`^(%s)%s(%s)$`, reAmt, reSpc, reDnm))
-	reDecCoin = regexp.MustCompile(fmt.Sprintf(`^(%s)%s(%s)$`, reDecAmt, reSpc, reDnm))
+	reDnmString = `[a-z][a-z0-9]{2,15}`
+	reAmt       = `[[:digit:]]+`
+	reDecAmt    = `[[:digit:]]*\.[[:digit:]]+`
+	reSpc       = `[[:space:]]*`
+	reDnm       = regexp.MustCompile(fmt.Sprintf(`^%s$`, reDnmString))
+	reCoin      = regexp.MustCompile(fmt.Sprintf(`^(%s)%s(%s)$`, reAmt, reSpc, reDnmString))
+	reDecCoin   = regexp.MustCompile(fmt.Sprintf(`^(%s)%s(%s)$`, reDecAmt, reSpc, reDnmString))
 )
 
-func validateDenom(denom string) {
-	if len(denom) < 3 || len(denom) > 16 {
-		panic(fmt.Sprintf("invalid denom length: %s", denom))
+func validateDenom(denom string) error {
+	if !reDnm.MatchString(denom) {
+		return errors.New("illegal characters")
 	}
-	if strings.ToLower(denom) != denom {
-		panic(fmt.Sprintf("denom cannot contain upper case characters: %s", denom))
+	return nil
+}
+
+func mustValidateDenom(denom string) {
+	if err := validateDenom(denom); err != nil {
+		panic(err)
 	}
 }
 
@@ -511,8 +517,8 @@ func ParseCoin(coinStr string) (coin Coin, err error) {
 		return Coin{}, fmt.Errorf("failed to parse coin amount: %s", amountStr)
 	}
 
-	if denomStr != strings.ToLower(denomStr) {
-		return Coin{}, fmt.Errorf("denom cannot contain upper case characters: %s", denomStr)
+	if err := validateDenom(denomStr); err != nil {
+		return Coin{}, fmt.Errorf("invalid denom cannot contain upper case characters or spaces: %s", err)
 	}
 
 	return NewCoin(denomStr, amount), nil

diff --git a/types/dec_coin.go b/types/dec_coin.go
@@ -19,7 +19,7 @@ type DecCoin struct {
 }
 
 func NewDecCoin(denom string, amount Int) DecCoin {
-	validateDenom(denom)
+	mustValidateDenom(denom)
 
 	if amount.LT(ZeroInt()) {
 		panic(fmt.Sprintf("negative coin amount: %v\n", amount))
@@ -32,7 +32,7 @@ func NewDecCoin(denom string, amount Int) DecCoin {
 }
 
 func NewDecCoinFromDec(denom string, amount Dec) DecCoin {
-	validateDenom(denom)
+	mustValidateDenom(denom)
 
 	if amount.LT(ZeroDec()) {
 		panic(fmt.Sprintf("negative decimal coin amount: %v\n", amount))
@@ -366,7 +366,7 @@ func (coins DecCoins) Empty() bool {
 
 // returns the amount of a denom from deccoins
 func (coins DecCoins) AmountOf(denom string) Dec {
-	validateDenom(denom)
+	mustValidateDenom(denom)
 
 	switch len(coins) {
 	case 0:
@@ -429,7 +429,7 @@ func (coins DecCoins) IsValid() bool {
 		return true
 
 	case 1:
-		if strings.ToLower(coins[0].Denom) != coins[0].Denom {
+		if err := validateDenom(coins[0].Denom); err != nil {
 			return false
 		}
 		return coins[0].IsPositive()
@@ -529,8 +529,8 @@ func ParseDecCoin(coinStr string) (coin DecCoin, err error) {
 		return DecCoin{}, errors.Wrap(err, fmt.Sprintf("failed to parse decimal coin amount: %s", amountStr))
 	}
 
-	if denomStr != strings.ToLower(denomStr) {
-		return DecCoin{}, fmt.Errorf("denom cannot contain upper case characters: %s", denomStr)
+	if err := validateDenom(denomStr); err != nil {
+		return DecCoin{}, fmt.Errorf("invalid denom cannot contain upper case characters or spaces: %s", err)
 	}
 
 	return NewDecCoinFromDec(denomStr, amount), nil