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

types/Coin: compile and reuse Regexps to reduce massive RAM+CPU burn #8001

Merged
merged 3 commits into from
Nov 24, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Features

* (types/coin.go) [\#6755](https://github.com/cosmos/cosmos-sdk/pull/6755) Add custom regex validation for `Coin` denom by overwriting `CoinDenomRegex` when using `/types/coin.go`.
* (types/coin.go) [\#6755](https://github.com/cosmos/cosmos-sdk/pull/6755) [\#8001](https://github.com/cosmos/cosmos-sdk/pull/8001) Allow custom regex validation for `Coin` denom through `SetCoinDenomRegex()`.
* (version) [\#7835](https://github.com/cosmos/cosmos-sdk/issues/7835) [\#7940](https://github.com/cosmos/cosmos-sdk/issues/7940) The version --long command now shows the list of build dependencies and their versioning information.

### Improvements
Expand Down
4 changes: 2 additions & 2 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ See the [Cosmos SDK 0.39.2 milestone](https://github.com/cosmos/cosmos-sdk/miles

## Allow ValidateDenom() to be customised per application

Applications can now customise `types.Coin` denomination validation by
replacing `types.CoinDenomRegex` with their application-specific validation function.
Applications can now customise `types.Coin` denomination validation by passing
their application-specific validation function to `types.SetCoinDenomRegex()`.

## Upgrade queries don't work after upgrade

Expand Down
30 changes: 30 additions & 0 deletions types/bench_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package types_test

import (
"testing"

"github.com/cosmos/cosmos-sdk/types"
)

var coinStrs = []string{
"2000ATM",
"5000AMX",
"192XXX",
"1e9BTC",
}

func BenchmarkParseCoin(b *testing.B) {
var blankCoin types.Coin
b.ReportAllocs()
for i := 0; i < b.N; i++ {
for _, coinStr := range coinStrs {
coin, err := types.ParseCoin(coinStr)
if err != nil {
b.Fatal(err)
}
if coin == blankCoin {
b.Fatal("Unexpectedly returned a blank coin")
}
}
}
}
34 changes: 19 additions & 15 deletions types/coin.go
Original file line number Diff line number Diff line change
Expand Up @@ -599,33 +599,37 @@ var (
reAmt = `[[:digit:]]+`
reDecAmt = `[[:digit:]]*\.[[:digit:]]+`
reSpc = `[[:space:]]*`
reDnm = returnReDnm
reCoin = returnReCoin
reDecCoin = returnDecCoin
reDnm *regexp.Regexp
reCoin *regexp.Regexp
reDecCoin *regexp.Regexp
)

func returnDecCoin() *regexp.Regexp {
return regexp.MustCompile(fmt.Sprintf(`^(%s)%s(%s)$`, reDecAmt, reSpc, CoinDenomRegex()))
}
func returnReCoin() *regexp.Regexp {
return regexp.MustCompile(fmt.Sprintf(`^(%s)%s(%s)$`, reAmt, reSpc, CoinDenomRegex()))
}
func returnReDnm() *regexp.Regexp {
return regexp.MustCompile(fmt.Sprintf(`^%s$`, CoinDenomRegex()))
func init() {
SetCoinDenomRegex(DefaultCoinDenomRegex)
}

// DefaultCoinDenomRegex returns the default regex string
func DefaultCoinDenomRegex() string {
return reDnmString
}

// CoinDenomRegex returns the current regex string and can be overwritten for custom validation
var CoinDenomRegex = DefaultCoinDenomRegex
// coinDenomRegex returns the current regex string and can be overwritten for custom validation
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
var coinDenomRegex = DefaultCoinDenomRegex
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved

// SetCoinDenomRegex allows for coin's custom validation by overriding the regular
// expression string used for denom validation.
func SetCoinDenomRegex(reFn func() string) {
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
coinDenomRegex = reFn

reDnm = regexp.MustCompile(fmt.Sprintf(`^%s$`, coinDenomRegex()))
reCoin = regexp.MustCompile(fmt.Sprintf(`^(%s)%s(%s)$`, reAmt, reSpc, coinDenomRegex()))
reDecCoin = regexp.MustCompile(fmt.Sprintf(`^(%s)%s(%s)$`, reDecAmt, reSpc, coinDenomRegex()))
}

// ValidateDenom validates a denomination string returning an error if it is
// invalid.
func ValidateDenom(denom string) error {
if !reDnm().MatchString(denom) {
if !reDnm.MatchString(denom) {
return fmt.Errorf("invalid denom: %s", denom)
}
return nil
Expand All @@ -642,7 +646,7 @@ func mustValidateDenom(denom string) {
func ParseCoin(coinStr string) (coin Coin, err error) {
coinStr = strings.TrimSpace(coinStr)

matches := reCoin().FindStringSubmatch(coinStr)
matches := reCoin.FindStringSubmatch(coinStr)
if matches == nil {
return Coin{}, fmt.Errorf("invalid coin expression: %s", coinStr)
}
Expand Down
6 changes: 3 additions & 3 deletions types/coin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ func TestCoinIsValid(t *testing.T) {
func TestCustomValidation(t *testing.T) {

newDnmRegex := `[\x{1F600}-\x{1F6FF}]`
CoinDenomRegex = func() string {
SetCoinDenomRegex(func() string {
return newDnmRegex
}
})

cases := []struct {
coin Coin
Expand All @@ -92,7 +92,7 @@ func TestCustomValidation(t *testing.T) {
for i, tc := range cases {
require.Equal(t, tc.expectPass, tc.coin.IsValid(), "unexpected result for IsValid, tc #%d", i)
}
CoinDenomRegex = DefaultCoinDenomRegex
SetCoinDenomRegex(DefaultCoinDenomRegex)
}

func TestAddCoin(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion types/dec_coin.go
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ func (coins DecCoins) Sort() DecCoins {
func ParseDecCoin(coinStr string) (coin DecCoin, err error) {
coinStr = strings.TrimSpace(coinStr)

matches := reDecCoin().FindStringSubmatch(coinStr)
matches := reDecCoin.FindStringSubmatch(coinStr)
if matches == nil {
return DecCoin{}, fmt.Errorf("invalid decimal coin expression: %s", coinStr)
}
Expand Down