Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

R4R: improve denom validation #3666

Merged
merged 6 commits into from
Mar 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
40 changes: 23 additions & 17 deletions types/coin.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package types

import (
"errors"
"fmt"
"regexp"
"sort"
Expand All @@ -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{
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)
}
}

Expand All @@ -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
Expand Down
12 changes: 6 additions & 6 deletions types/dec_coin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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))
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down