Backport: Removal of regex usage on denom validation (#570)

* feat(x/bank): Replace regex parsing of denom validation to generated parsing (cosmos#19511)

* fix: fix ragel denom validation introduced in cosmos#19511 (cosmos#19701)

* fix: use loop instead of ragel (cosmos#19710)

* Fix test

* Fix integration test as well

* Try linting

* Fix lint

* Fix lint

* add changelog

---------

Co-authored-by: Marko <[email protected]>
Co-authored-by: Adam Tucker <[email protected]>
Co-authored-by: Adam Tucker <[email protected]>

simapp/go.mod

@@ -207,6 +207,8 @@ require (
 
 // Below are the long-lived replace of the SimApp
 replace (
+	// Needs to be replaced due to iavlFastNodeModuleWhitelist feature
+	cosmossdk.io/store => github.com/osmosis-labs/cosmos-sdk/store v0.1.0-alpha.1.0.20240509221435-b8feb2ffb728
 	// use cosmos fork of keyring
 	github.com/99designs/keyring => github.com/cosmos/keyring v1.2.0
 	// Simapp always use the latest version of the cosmos-sdk

simapp/go.sum

@@ -200,8 +200,6 @@ cosmossdk.io/log v1.3.1 h1:UZx8nWIkfbbNEWusZqzAx3ZGvu54TZacWib3EzUYmGI=
 cosmossdk.io/log v1.3.1/go.mod h1:2/dIomt8mKdk6vl3OWJcPk2be3pGOS8OQaLUM/3/tCM=
 cosmossdk.io/math v1.3.0 h1:RC+jryuKeytIiictDslBP9i1fhkVm6ZDmZEoNP316zE=
 cosmossdk.io/math v1.3.0/go.mod h1:vnRTxewy+M7BtXBNFybkuhSH4WfedVAAnERHgVFhp3k=
-cosmossdk.io/store v1.1.0 h1:LnKwgYMc9BInn9PhpTFEQVbL9UK475G2H911CGGnWHk=
-cosmossdk.io/store v1.1.0/go.mod h1:oZfW/4Fc/zYqu3JmQcQdUJ3fqu5vnYTn3LZFFy8P8ng=
 cosmossdk.io/tools/confix v0.1.1 h1:aexyRv9+y15veH3Qw16lxQwo+ki7r2I+g0yNTEFEQM8=
 cosmossdk.io/tools/confix v0.1.1/go.mod h1:nQVvP1tHsGXS83PonPVWJtSbddIqyjEw99L4M3rPJyQ=
 cosmossdk.io/x/circuit v0.1.1 h1:KPJCnLChWrxD4jLwUiuQaf5mFD/1m7Omyo7oooefBVQ=
@@ -865,6 +863,8 @@ github.com/openzipkin/zipkin-go v0.2.1/go.mod h1:NaW6tEwdmWMaCDZzg8sh+IBNOxHMPnh
 github.com/openzipkin/zipkin-go v0.2.2/go.mod h1:NaW6tEwdmWMaCDZzg8sh+IBNOxHMPnhQw8ySjnjRyN4=
 github.com/ory/dockertest v3.3.5+incompatible h1:iLLK6SQwIhcbrG783Dghaaa3WPzGc+4Emza6EbVUUGA=
 github.com/ory/dockertest v3.3.5+incompatible/go.mod h1:1vX4m9wsvi00u5bseYwXaSnhNrne+V0E6LAcBILJdPs=
+github.com/osmosis-labs/cosmos-sdk/store v0.1.0-alpha.1.0.20240509221435-b8feb2ffb728 h1:AMz4HWC+WA/MwBQdsb11yIF9ForIvSLYYVy/jyhJ3/I=
+github.com/osmosis-labs/cosmos-sdk/store v0.1.0-alpha.1.0.20240509221435-b8feb2ffb728/go.mod h1:gjE3DZe4t/+VeIk6CmrouyqiuDbZ7QOVDDq3nLqBTpg=
 github.com/pact-foundation/pact-go v1.0.4/go.mod h1:uExwJY4kCzNPcHRj+hCR/HBbOOIwwtUjcrb0b5/5kLM=
 github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
 github.com/pascaldekloe/goe v0.1.0 h1:cBOtyMzM9HTpWjXfbbunk26uA6nG3a8n06Wieeh0MwY=

tests/go.mod

@@ -207,6 +207,8 @@ require (
 replace (
 	// We always want to test against the latest version of the simapp.
 	cosmossdk.io/simapp => ../simapp
+	// Needs to be replaced due to iavlFastNodeModuleWhitelist feature
+	cosmossdk.io/store => github.com/osmosis-labs/cosmos-sdk/store v0.1.0-alpha.1.0.20240509221435-b8feb2ffb728
 	github.com/99designs/keyring => github.com/cosmos/keyring v1.2.0
 	// We always want to test against the latest version of the SDK.
 	github.com/cosmos/cosmos-sdk => ../.

tests/go.sum

@@ -200,8 +200,6 @@ cosmossdk.io/log v1.3.1 h1:UZx8nWIkfbbNEWusZqzAx3ZGvu54TZacWib3EzUYmGI=
 cosmossdk.io/log v1.3.1/go.mod h1:2/dIomt8mKdk6vl3OWJcPk2be3pGOS8OQaLUM/3/tCM=
 cosmossdk.io/math v1.3.0 h1:RC+jryuKeytIiictDslBP9i1fhkVm6ZDmZEoNP316zE=
 cosmossdk.io/math v1.3.0/go.mod h1:vnRTxewy+M7BtXBNFybkuhSH4WfedVAAnERHgVFhp3k=
-cosmossdk.io/store v1.1.0 h1:LnKwgYMc9BInn9PhpTFEQVbL9UK475G2H911CGGnWHk=
-cosmossdk.io/store v1.1.0/go.mod h1:oZfW/4Fc/zYqu3JmQcQdUJ3fqu5vnYTn3LZFFy8P8ng=
 cosmossdk.io/x/circuit v0.1.1 h1:KPJCnLChWrxD4jLwUiuQaf5mFD/1m7Omyo7oooefBVQ=
 cosmossdk.io/x/circuit v0.1.1/go.mod h1:B6f/urRuQH8gjt4eLIXfZJucrbreuYrKh5CSjaOxr+Q=
 cosmossdk.io/x/evidence v0.1.1 h1:Ks+BLTa3uftFpElLTDp9L76t2b58htjVbSZ86aoK/E4=
@@ -866,6 +864,8 @@ github.com/openzipkin/zipkin-go v0.2.1/go.mod h1:NaW6tEwdmWMaCDZzg8sh+IBNOxHMPnh
 github.com/openzipkin/zipkin-go v0.2.2/go.mod h1:NaW6tEwdmWMaCDZzg8sh+IBNOxHMPnhQw8ySjnjRyN4=
 github.com/ory/dockertest v3.3.5+incompatible h1:iLLK6SQwIhcbrG783Dghaaa3WPzGc+4Emza6EbVUUGA=
 github.com/ory/dockertest v3.3.5+incompatible/go.mod h1:1vX4m9wsvi00u5bseYwXaSnhNrne+V0E6LAcBILJdPs=
+github.com/osmosis-labs/cosmos-sdk/store v0.1.0-alpha.1.0.20240509221435-b8feb2ffb728 h1:AMz4HWC+WA/MwBQdsb11yIF9ForIvSLYYVy/jyhJ3/I=
+github.com/osmosis-labs/cosmos-sdk/store v0.1.0-alpha.1.0.20240509221435-b8feb2ffb728/go.mod h1:gjE3DZe4t/+VeIk6CmrouyqiuDbZ7QOVDDq3nLqBTpg=
 github.com/pact-foundation/pact-go v1.0.4/go.mod h1:uExwJY4kCzNPcHRj+hCR/HBbOOIwwtUjcrb0b5/5kLM=
 github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
 github.com/pascaldekloe/goe v0.1.0 h1:cBOtyMzM9HTpWjXfbbunk26uA6nG3a8n06Wieeh0MwY=

tests/integration/bank/keeper/deterministic_test.go

@@ -34,7 +34,7 @@ import (
 )
 
 var (
-	denomRegex   = sdk.DefaultCoinDenomRegex()
+	denomRegex   = `[a-zA-Z][a-zA-Z0-9/:._-]{2,127}`
 	addr1        = sdk.MustAccAddressFromBech32("cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5")
 	coin1        = sdk.NewCoin("denom", math.NewInt(10))
 	metadataAtom = banktypes.Metadata{

tests/integration/evidence/keeper/infraction_test.go

@@ -193,7 +193,7 @@ func TestHandleDoubleSign(t *testing.T) {
 
 	assert.NilError(t, f.slashingKeeper.AddPubkey(f.sdkCtx, valpubkey))
 
-	info := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(valpubkey.Address()), f.sdkCtx.BlockHeight(), int64(0), time.Unix(0, 0), false, int64(0))
+	info := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(valpubkey.Address()), f.sdkCtx.BlockHeight(), time.Unix(0, 0), false, int64(0))
 	f.slashingKeeper.SetValidatorSigningInfo(f.sdkCtx, sdk.ConsAddress(valpubkey.Address()), info)
 
 	// handle a signature to set signing info

tests/integration/staking/keeper/determinstic_test.go

@@ -818,12 +818,13 @@ func TestGRPCRedelegations(t *testing.T) {
 }
 
 func TestGRPCParams(t *testing.T) {
+	coinDenomRegex := `[a-zA-Z][a-zA-Z0-9/:._-]{2,127}`
 	t.Parallel()
 	f := initDeterministicFixture(t)
 
 	rapid.Check(t, func(rt *rapid.T) {
 		params := stakingtypes.Params{
-			BondDenom:         rapid.StringMatching(sdk.DefaultCoinDenomRegex()).Draw(rt, "bond-denom"),
+			BondDenom:         rapid.StringMatching(coinDenomRegex).Draw(rt, "bond-denom"),
 			UnbondingTime:     durationGenerator().Draw(rt, "duration"),
 			MaxValidators:     rapid.Uint32Min(1).Draw(rt, "max-validators"),
 			MaxEntries:        rapid.Uint32Min(1).Draw(rt, "max-entries"),

types/coin.go

@@ -8,6 +8,8 @@ import (
 	"sort"
 	"strings"
 
+	"unicode"
+
 	"cosmossdk.io/math"
 )
 
@@ -828,30 +830,15 @@ func (coins Coins) Sort() Coins {
 	return coins
 }
 
-//-----------------------------------------------------------------------------
-// Parsing
-
 var (
-	// Denominations can be 3 ~ 128 characters long and support letters, followed by either
-	// a letter, a number or a separator ('/', ':', '.', '_' or '-').
-	reDnmString = `[a-zA-Z][a-zA-Z0-9/:._-]{2,127}`
-	reDecAmt    = `[[:digit:]]+(?:\.[[:digit:]]+)?|\.[[:digit:]]+`
-	reSpc       = `[[:space:]]*`
-	reDnm       *regexp.Regexp
-	reDecCoin   *regexp.Regexp
-)
+	reDecAmt = `[[:digit:]]+(?:\.[[:digit:]]+)?|\.[[:digit:]]+`
+	reSpc    = `[[:space:]]*`
 
-func init() {
-	SetCoinDenomRegex(DefaultCoinDenomRegex)
-}
-
-// DefaultCoinDenomRegex returns the default regex string
-func DefaultCoinDenomRegex() string {
-	return reDnmString
-}
+	coinDenomRegex func() string
 
-// coinDenomRegex returns the current regex string and can be overwritten for custom validation
-var coinDenomRegex = DefaultCoinDenomRegex
+	reDnm     *regexp.Regexp
+	reDecCoin *regexp.Regexp
+)
 
 // SetCoinDenomRegex allows for coin's custom validation by overriding the regular
 // expression string used for denom validation.
@@ -864,12 +851,49 @@ func SetCoinDenomRegex(reFn func() string) {
 
 // ValidateDenom is the default validation function for Coin.Denom.
 func ValidateDenom(denom string) error {
-	if !reDnm.MatchString(denom) {
+	if reDnm == nil || reDecCoin == nil { //nolint:gocritic
+		// Convert the string to a byte slice as required by the Ragel-generated function.
+
+		// Call the Ragel-generated function.
+		if !MatchDenom(denom) { //nolint:gocritic
+			return fmt.Errorf("invalid denom: %s", denom)
+		}
+	} else if !reDnm.MatchString(denom) { // If reDnm has been initialized, use it for matching.
 		return fmt.Errorf("invalid denom: %s", denom)
 	}
+
 	return nil
 }
 
+// isValidRune checks if a given rune is a valid character for a rune.
+// It returns true if the rune is a letter, digit, '/', ':', '.', '_', or '-'.
+func isValidRune(r rune) bool {
+	return unicode.IsLetter(r) || unicode.IsDigit(r) || r == '/' || r == ':' || r == '.' || r == '_' || r == '-'
+}
+
+// MatchDenom checks if the given string is a valid denomination.
+// A valid denomination must have a length between 3 and 128 characters,
+// start with a letter, and only contain valid runes.
+func MatchDenom(s string) bool {
+	length := len(s)
+	if length < 3 || length > 128 {
+		return false
+	}
+
+	firstRune := rune(s[0])
+	if !unicode.IsLetter(firstRune) {
+		return false
+	}
+
+	for _, r := range s[1:] {
+		if !isValidRune(r) {
+			return false
+		}
+	}
+
+	return true
+}
+
 func mustValidateDenom(denom string) {
 	if err := ValidateDenom(denom); err != nil {
 		panic(err)

types/coin_test.go

@@ -107,6 +107,8 @@ func (s *coinTestSuite) TestCoinIsValid() {
 
 func (s *coinTestSuite) TestCustomValidation() {
 	newDnmRegex := `[\x{1F600}-\x{1F6FF}]`
+	reDnmString := `[a-zA-Z][a-zA-Z0-9/:._-]{2,127}`
+
 	sdk.SetCoinDenomRegex(func() string {
 		return newDnmRegex
 	})
@@ -125,7 +127,7 @@ func (s *coinTestSuite) TestCustomValidation() {
 	for i, tc := range cases {
 		s.Require().Equal(tc.expectPass, tc.coin.IsValid(), "unexpected result for IsValid, tc #%d", i)
 	}
-	sdk.SetCoinDenomRegex(sdk.DefaultCoinDenomRegex)
+	sdk.SetCoinDenomRegex(func() string { return reDnmString })
 }
 
 func (s *coinTestSuite) TestCoinsDenoms() {
@@ -987,6 +989,33 @@ func (s *coinTestSuite) TestParseCoins() {
 	}
 }
 
+func (s *coinTestSuite) TestValidateDenom() {
+	cases := []struct {
+		input string
+		valid bool
+	}{
+		{"", false},
+		{"stake", true},
+		{"stake,", false},
+		{"me coin", false},
+		{"me coin much", false},
+		{"not a coin", false},
+		{"foo:bar", true}, // special characters '/' | ':' | '.' | '_' | '-' are allowed
+		{"atom10", true},  // number in denom is allowed
+		{"transfer/channelToA/uatom", true},
+		{"ibc/7F1D3FCF4AE79E1554D670D1AD949A9BA4E4A3C76C63093E17E446A46061A7A2", true},
+	}
+
+	for _, tc := range cases {
+		err := sdk.ValidateDenom(tc.input)
+		if !tc.valid {
+			s.Require().Error(err)
+		} else {
+			s.Require().NoError(err)
+		}
+	}
+}
+
 func (s *coinTestSuite) TestSortCoins() {
 	good := sdk.Coins{
 		sdk.NewInt64Coin("gas", 1),

types/dec_coin.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"sort"
 	"strings"
+	"unicode"
 
 	"cosmossdk.io/errors"
 	"cosmossdk.io/math"
@@ -624,14 +625,23 @@ func (coins DecCoins) Sort() DecCoins {
 // ParseDecCoin parses a decimal coin from a string, returning an error if
 // invalid. An empty string is considered invalid.
 func ParseDecCoin(coinStr string) (coin DecCoin, err error) {
-	coinStr = strings.TrimSpace(coinStr)
+	var amountStr, denomStr string
+	// if custom parsing has not been set, use default coin regex
+	if reDecCoin == nil { //nolint:gocritic
+		amountStr, denomStr, err = ParseDecAmount(coinStr)
+		if err != nil {
+			return DecCoin{}, err
+		}
+	} else {
+		coinStr = strings.TrimSpace(coinStr)
 
-	matches := reDecCoin.FindStringSubmatch(coinStr)
-	if matches == nil {
-		return DecCoin{}, fmt.Errorf("invalid decimal coin expression: %s", coinStr)
-	}
+		matches := reDecCoin.FindStringSubmatch(coinStr)
+		if matches == nil {
+			return DecCoin{}, fmt.Errorf("invalid decimal coin expression: %s", coinStr)
+		}
 
-	amountStr, denomStr := matches[1], matches[2]
+		amountStr, denomStr = matches[1], matches[2]
+	}
 
 	amount, err := math.LegacyNewDecFromStr(amountStr)
 	if err != nil {
@@ -645,6 +655,50 @@ func ParseDecCoin(coinStr string) (coin DecCoin, err error) {
 	return NewDecCoinFromDec(denomStr, amount), nil
 }
 
+// ParseDecAmount parses the given string into amount, denomination.
+func ParseDecAmount(coinStr string) (string, string, error) {
+	var amountRune, denomRune []rune
+
+	// Indicates the start of denom parsing
+	seenLetter := false
+	// Indicates we're currently parsing the amount
+	parsingAmount := true
+
+	for _, r := range strings.TrimSpace(coinStr) {
+		if parsingAmount { //nolint:gocritic
+			if unicode.IsDigit(r) || r == '.' { //nolint:gocritic
+				amountRune = append(amountRune, r)
+			} else if unicode.IsSpace(r) { // if space is seen, indicates that we have finished parsing amount
+				parsingAmount = false
+			} else if unicode.IsLetter(r) { // if letter is seen, indicates that it is the start of denom
+				parsingAmount = false
+				seenLetter = true
+				denomRune = append(denomRune, r)
+			} else { // Invalid character encountered in amount part
+				return "", "", fmt.Errorf("invalid character in coin string: %s", string(r))
+			}
+		} else if !seenLetter { // This logic flow is for skipping spaces between amount and denomination
+			if unicode.IsLetter(r) {
+				seenLetter = true
+				denomRune = append(denomRune, r)
+			} else if !unicode.IsSpace(r) {
+				// Invalid character before denomination starts
+				return "", "", fmt.Errorf("invalid start of denomination: %s", string(r))
+			}
+		} else {
+			// Parsing the denomination
+			if unicode.IsLetter(r) || unicode.IsDigit(r) || r == '/' || r == ':' || r == '.' || r == '_' || r == '-' {
+				denomRune = append(denomRune, r)
+			} else {
+				// Invalid character encountered in denomination part
+				return "", "", fmt.Errorf("invalid character in denomination: %s", string(r))
+			}
+		}
+	}
+
+	return string(amountRune), string(denomRune), nil
+}
+
 // ParseDecCoins will parse out a list of decimal coins separated by commas. If the parsing is successuful,
 // the provided coins will be sanitized by removing zero coins and sorting the coin set. Lastly
 // a validation of the coin set is executed. If the check passes, ParseDecCoins will return the sanitized coins.

x/authz/client/cli/tx_test.go

@@ -348,7 +348,7 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()),
 			},
 			true,
-			"invalid decimal coin expression",
+			"nvalid character in denomination: ",
 		},
 		{
 			"invalid authorization type",

x/gov/client/cli/tx_test.go

@@ -139,7 +139,7 @@ func (s *CLITestSuite) TestNewCmdSubmitProposal() {
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()),
 			},
-			"invalid decimal coin expression",
+			"invalid character in coin string",
 		},
 		{
 			"valid proposal",
@@ -297,7 +297,7 @@ func (s *CLITestSuite) TestNewCmdDeposit() {
 				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()),
 			},
-			"invalid decimal coin expression: invalidCoin",
+			"failed to parse decimal coin amount",
 		},
 		{
 			"deposit on a proposal",

x/gov/client/cli/util_test.go

@@ -349,7 +349,7 @@ func TestReadGovPropFlags(t *testing.T) {
 			name:     "only deposit invalid coins",
 			fromAddr: nil,
 			args:     []string{argDeposit, "not really coins"},
-			expErr:   []string{"invalid deposit", "invalid decimal coin expression", "not really coins"},
+			expErr:   []string{"invalid deposit", "invalid character in denomination"},
 		},
 		{
 			name:     "only deposit two coins",
@@ -379,19 +379,19 @@ func TestReadGovPropFlags(t *testing.T) {
 			name:     "only deposit coin 1 of 3 bad",
 			fromAddr: nil,
 			args:     []string{argDeposit, "1bad^coin,2bcoin,3ccoin"},
-			expErr:   []string{"invalid deposit", "invalid decimal coin expression", "1bad^coin"},
+			expErr:   []string{"invalid deposit", "invalid character in denomination"},
 		},
 		{
 			name:     "only deposit coin 2 of 3 bad",
 			fromAddr: nil,
 			args:     []string{argDeposit, "1acoin,2bad^coin,3ccoin"},
-			expErr:   []string{"invalid deposit", "invalid decimal coin expression", "2bad^coin"},
+			expErr:   []string{"invalid deposit", "invalid character in denomination"},
 		},
 		{
 			name:     "only deposit coin 3 of 3 bad",
 			fromAddr: nil,
 			args:     []string{argDeposit, "1acoin,2bcoin,3bad^coin"},
-			expErr:   []string{"invalid deposit", "invalid decimal coin expression", "3bad^coin"},
+			expErr:   []string{"invalid deposit", "invalid character in denomination"},
 		},
 		// As far as I can tell, there's no way to make flagSet.GetString return an error for a defined string flag.
 		// So I don't have a test for the "could not read deposit" error case.