-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
types: update coin regex #7027
types: update coin regex #7027
Changes from 16 commits
fbec784
497e011
2b22419
9df2c67
431ede8
afe2627
9d4c70e
216ebc8
595a9d2
cbb88a0
88a6874
0bd648f
e7c09f7
94fb4a9
549e121
12eece3
9945305
e12e38d
928099c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,16 +12,18 @@ import ( | |
// Coin | ||
|
||
// NewCoin returns a new coin with a denomination and amount. It will panic if | ||
// the amount is negative. | ||
// the amount is negative or if the denomination is invalid. | ||
func NewCoin(denom string, amount Int) Coin { | ||
if err := validate(denom, amount); err != nil { | ||
panic(err) | ||
} | ||
|
||
return Coin{ | ||
coin := Coin{ | ||
Denom: denom, | ||
Amount: amount, | ||
} | ||
|
||
if err := coin.Validate(); err != nil { | ||
panic(err) | ||
} | ||
|
||
return coin | ||
} | ||
|
||
// NewInt64Coin returns a new coin with a denomination and amount. It will panic | ||
|
@@ -35,26 +37,23 @@ func (coin Coin) String() string { | |
return fmt.Sprintf("%v%v", coin.Amount, coin.Denom) | ||
} | ||
|
||
// validate returns an error if the Coin has a negative amount or if | ||
// Validate returns an error if the Coin has a negative amount or if | ||
// the denom is invalid. | ||
func validate(denom string, amount Int) error { | ||
if err := ValidateDenom(denom); err != nil { | ||
func (coin Coin) Validate() error { | ||
if err := ValidateDenom(coin.Denom); err != nil { | ||
return err | ||
} | ||
|
||
if amount.IsNegative() { | ||
return fmt.Errorf("negative coin amount: %v", amount) | ||
if coin.Amount.IsNegative() { | ||
return fmt.Errorf("negative coin amount: %v", coin.Amount) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
// IsValid returns true if the Coin has a non-negative amount and the denom is vaild. | ||
// IsValid returns true if the Coin has a non-negative amount and the denom is valid. | ||
func (coin Coin) IsValid() bool { | ||
if err := validate(coin.Denom, coin.Amount); err != nil { | ||
return false | ||
} | ||
return true | ||
return coin.Validate() == nil | ||
} | ||
|
||
// IsZero returns if this represents no money | ||
|
@@ -91,7 +90,7 @@ func (coin Coin) IsEqual(other Coin) bool { | |
return coin.Amount.Equal(other.Amount) | ||
} | ||
|
||
// Adds amounts of two coins with same denom. If the coins differ in denom then | ||
// Add adds amounts of two coins with same denom. If the coins differ in denom then | ||
// it panics. | ||
func (coin Coin) Add(coinB Coin) Coin { | ||
if coin.Denom != coinB.Denom { | ||
|
@@ -101,7 +100,7 @@ func (coin Coin) Add(coinB Coin) Coin { | |
return Coin{coin.Denom, coin.Amount.Add(coinB.Amount)} | ||
} | ||
|
||
// Subtracts amounts of two coins with same denom. If the coins differ in denom | ||
// Sub subtracts amounts of two coins with same denom. If the coins differ in denom | ||
// then it panics. | ||
func (coin Coin) Sub(coinB Coin) Coin { | ||
if coin.Denom != coinB.Denom { | ||
|
@@ -146,13 +145,8 @@ func NewCoins(coins ...Coin) Coins { | |
|
||
newCoins.Sort() | ||
|
||
// detect duplicate Denoms | ||
if dupIndex := findDup(newCoins); dupIndex != -1 { | ||
panic(fmt.Errorf("find duplicate denom: %s", newCoins[dupIndex])) | ||
} | ||
|
||
if !newCoins.IsValid() { | ||
panic(fmt.Errorf("invalid coin set: %s", newCoins)) | ||
if err := newCoins.Validate(); err != nil { | ||
panic(fmt.Errorf("invalid coin set %s: %w", newCoins, err)) | ||
} | ||
|
||
return newCoins | ||
|
@@ -182,43 +176,61 @@ func (coins Coins) String() string { | |
return out[:len(out)-1] | ||
} | ||
|
||
// IsValid asserts the Coins are sorted, have positive amount, | ||
// and Denom does not contain upper case characters. | ||
func (coins Coins) IsValid() bool { | ||
// Validate checks that the Coins are sorted, have positive amount, with a valid and unique | ||
// denomination (i.e no duplicates). Otherwise, it returns an error. | ||
func (coins Coins) Validate() error { | ||
switch len(coins) { | ||
case 0: | ||
return true | ||
return nil | ||
|
||
case 1: | ||
if err := ValidateDenom(coins[0].Denom); err != nil { | ||
return false | ||
return err | ||
} | ||
return coins[0].IsPositive() | ||
if !coins[0].IsPositive() { | ||
return fmt.Errorf("coin %s amount is not positive", coins[0]) | ||
} | ||
return nil | ||
|
||
default: | ||
// check single coin case | ||
if !(Coins{coins[0]}).IsValid() { | ||
return false | ||
if err := (Coins{coins[0]}).Validate(); err != nil { | ||
return err | ||
} | ||
|
||
lowDenom := coins[0].Denom | ||
seenDenoms := make(map[string]bool) | ||
seenDenoms[lowDenom] = true | ||
|
||
for _, coin := range coins[1:] { | ||
if strings.ToLower(coin.Denom) != coin.Denom { | ||
fedekunze marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return false | ||
if seenDenoms[coin.Denom] { | ||
return fmt.Errorf("duplicate denomination %s", coin.Denom) | ||
colin-axner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
if err := ValidateDenom(coin.Denom); err != nil { | ||
return err | ||
} | ||
if coin.Denom <= lowDenom { | ||
return false | ||
return fmt.Errorf("denomination %s is not sorted", coin.Denom) | ||
} | ||
if !coin.IsPositive() { | ||
return false | ||
return fmt.Errorf("coin %s amount is not positive", coin.Denom) | ||
} | ||
|
||
// we compare each coin against the last denom | ||
lowDenom = coin.Denom | ||
seenDenoms[coin.Denom] = true | ||
} | ||
|
||
return true | ||
return nil | ||
} | ||
} | ||
|
||
// IsValid calls Validate and returns true when the Coins are sorted, have positive amount, with a | ||
// valid and unique denomination (i.e no duplicates). | ||
func (coins Coins) IsValid() bool { | ||
return coins.Validate() == nil | ||
} | ||
|
||
// Add adds two sets of coins. | ||
// | ||
// e.g. | ||
|
@@ -463,7 +475,7 @@ func (coins Coins) Empty() bool { | |
return len(coins) == 0 | ||
} | ||
|
||
// Returns the amount of a denom from coins | ||
// AmountOf returns the amount of a denom from coins | ||
func (coins Coins) AmountOf(denom string) Int { | ||
mustValidateDenom(denom) | ||
|
||
|
@@ -560,13 +572,18 @@ func removeZeroCoins(coins Coins) Coins { | |
//----------------------------------------------------------------------------- | ||
// Sort interface | ||
|
||
func (coins Coins) Len() int { return len(coins) } | ||
// Len implements sort.Interface for Coins | ||
func (coins Coins) Len() int { return len(coins) } | ||
|
||
// Less implements sort.Interface for Coins | ||
func (coins Coins) Less(i, j int) bool { return coins[i].Denom < coins[j].Denom } | ||
func (coins Coins) Swap(i, j int) { coins[i], coins[j] = coins[j], coins[i] } | ||
|
||
// Swap implements sort.Interface for Coins | ||
func (coins Coins) Swap(i, j int) { coins[i], coins[j] = coins[j], coins[i] } | ||
|
||
var _ sort.Interface = Coins{} | ||
|
||
// Sort is a helper function to sort the set of coins inplace | ||
// Sort is a helper function to sort the set of coins in-place | ||
func (coins Coins) Sort() Coins { | ||
sort.Sort(coins) | ||
return coins | ||
|
@@ -576,8 +593,9 @@ func (coins Coins) Sort() Coins { | |
// Parsing | ||
|
||
var ( | ||
// Denominations can be 3 ~ 64 characters long. | ||
reDnmString = `[a-z][a-z0-9/]{2,63}` | ||
// Denominations can be 3 ~ 128 characters long and support letters, followed by either | ||
// a letter, a number or a separator ('/'). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: There's the potential for confusing UI here. Suppose I create a denom 5atom10,300btc There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unfortunately, there's no workaround for this. I added a test case and documented the expected format on the |
||
reDnmString = `[a-zA-Z][a-zA-Z0-9/]{2,127}` | ||
fedekunze marked this conversation as resolved.
Show resolved
Hide resolved
|
||
reAmt = `[[:digit:]]+` | ||
reDecAmt = `[[:digit:]]*\.[[:digit:]]+` | ||
reSpc = `[[:space:]]*` | ||
|
@@ -619,15 +637,14 @@ func ParseCoin(coinStr string) (coin Coin, err error) { | |
} | ||
|
||
if err := ValidateDenom(denomStr); err != nil { | ||
return Coin{}, fmt.Errorf("invalid denom cannot contain upper case characters or spaces: %s", err) | ||
return Coin{}, err | ||
} | ||
|
||
return NewCoin(denomStr, amount), nil | ||
} | ||
|
||
// ParseCoins will parse out a list of coins separated by commas. | ||
// If nothing is provided, it returns nil Coins. | ||
// Returned coins are sorted. | ||
// ParseCoins will parse out a list of coins separated by commas. If nothing is provided, it returns | ||
// nil Coins. If the coins aren't valid they return an error. Returned coins are sorted. | ||
func ParseCoins(coinsStr string) (Coins, error) { | ||
coinsStr = strings.TrimSpace(coinsStr) | ||
if len(coinsStr) == 0 { | ||
|
@@ -649,31 +666,9 @@ func ParseCoins(coinsStr string) (Coins, error) { | |
coins.Sort() | ||
|
||
// validate coins before returning | ||
if !coins.IsValid() { | ||
return nil, fmt.Errorf("parseCoins invalid: %#v", coins) | ||
if err := coins.Validate(); err != nil { | ||
return nil, err | ||
} | ||
|
||
return coins, nil | ||
} | ||
|
||
type findDupDescriptor interface { | ||
GetDenomByIndex(int) string | ||
Len() int | ||
} | ||
|
||
// findDup works on the assumption that coins is sorted | ||
func findDup(coins findDupDescriptor) int { | ||
if coins.Len() <= 1 { | ||
return -1 | ||
} | ||
|
||
prevDenom := coins.GetDenomByIndex(0) | ||
for i := 1; i < coins.Len(); i++ { | ||
if coins.GetDenomByIndex(i) == prevDenom { | ||
return i | ||
} | ||
prevDenom = coins.GetDenomByIndex(i) | ||
} | ||
|
||
return -1 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we document the rational to why a single coin of ZeroInt is valid but a set of coins with a coin of ZeroInt is invalid? Seems counter-intuitive to me and if we support this it, the reason should be well documented. I'd be in favor of only considering positive amount
Coin
s valid