Backport: Removal of regex usage on denom validation #570
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThe update primarily focuses on enhancing the handling and validation of coin denominations within the system. It introduces a specialized function for character validation, revamps the regular expression handling, and implements a new function for checking the validity of denomination strings. Additionally, the test suite has been expanded to include custom validation and specific denomination validation, ensuring a robust and error-free denomination handling mechanism. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (4)
- tests/integration/bank/keeper/deterministic_test.go (1 hunks)
- types/coin.go (3 hunks)
- types/coin_test.go (3 hunks)
- types/dec_coin.go (3 hunks)
Check Runs (14)
tests (03) completed (1)
tests (02) completed (2)
tests (01) completed (2)
test-sim-nondeterminism completed (1)
test-simapp completed (1)
build (arm64) completed (1)
test-integration completed (3)
build (arm) completed (1)
build (amd64) completed (1)
split-test-files completed (1)
run_staticmajor completed (1)
golangci-lint completed (4)
- types/dec_coin.go: 665-665: ifElseChain: rewrite if-else to switch statement (gocritic)
- types/dec_coin.go: 666-666: ifElseChain: rewrite if-else to switch statement (gocritic)
Gosec completed (2)
labeler completed (1)
Additional comments: 8
tests/integration/bank/keeper/deterministic_test.go (1)
- 39-39: The change to use a custom regex pattern
[a-zA-Z][a-zA-Z0-9/:._-]{2,127}fordenomRegexaligns with the PR's objective to move away from regex-based validation in the main codebase. Ensure this pattern accurately reflects the new validation logic for denominations.types/dec_coin.go (1)
- 7-7: Importing the
unicodepackage supports the new custom parsing logic for coin denominations, aligning with the PR's objective to move away from regex-based validation.types/coin.go (3)
- 874-878: The implementation of
isValidRunecorrectly checks for valid characters in a denomination. This approach is more efficient than using regular expressions and allows for clearer rules around what constitutes a valid character.- 880-901: The
MatchDenomfunction introduces a new validation logic for denominations, ensuring they start with a letter and only contain valid characters as defined byisValidRune. This method improves the clarity and efficiency of denomination validation. However, it's important to ensure that all existing denominations in the system conform to these new rules to avoid breaking changes.- 860-867: The conditional logic in
ValidateDenomto use either the newMatchDenomfunction or the regular expression-based validation (if initialized) is a good approach for backward compatibility. It allows for a transition period where both methods can be used, depending on the system's configuration. However, it's crucial to document this behavior clearly to avoid confusion among developers.Consider adding detailed documentation about the conditional validation logic in
ValidateDenomto clarify under what circumstances each validation method is used.types/coin_test.go (3)
- 115-116: The new regular expression
reDnmStringis introduced for custom validation. Ensure that this regex pattern accurately captures the intended validation rules for denominations. It's crucial to verify that the pattern covers all valid cases and correctly rejects invalid ones to prevent any potential issues with denomination validation.- 135-135: After updating the denomination validation regex with
sdk.SetCoinDenomRegex, it's reset back toreDnmStringat the end of theTestCustomValidationfunction. This approach ensures that the custom validation logic is only applied within the scope of this test. However, consider isolating test-specific configurations more robustly to prevent unintended side effects in parallel or future tests.- 1003-1028: The
TestValidateDenomfunction tests various inputs against thesdk.ValidateDenomfunction to ensure that only valid denominations are accepted. This is a crucial test to ensure the integrity of coin denominations within the system. It's well-structured and covers a range of test cases, including those with special characters and IBC denominations. Ensure that all relevant cases are covered, including edge cases that might not be immediately obvious, to fully validate the denomination validation logic.
ValarDragon
reviewed
Mar 13, 2024
Comment on lines
+876
to
+877
| func isValidRune(r rune) bool { | ||
| return unicode.IsLetter(r) || unicode.IsDigit(r) || r == '/' || r == ':' || r == '.' || r == '_' || r == '-' |
There was a problem hiding this comment.
btw we should change this upstream to just be bytes, and a couple of if statements. Rune's / these unicode calls are even more complex than what we need.
e.g. imagine checking [0-9/_] as byte >= 0x2F && byte <= 0x3A.
There was a problem hiding this comment.
Can make an issue for trying this upstream
|
Setting as draft until tests are fixed |
|
linting is failing but tbh idk why + im not sure if we want to create divergance with upstream just for this lint 🤔 |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (4)
- tests/e2e/authz/tx.go (1 hunks)
- tests/integration/staking/keeper/determinstic_test.go (1 hunks)
- x/authz/client/cli/tx_test.go (1 hunks)
- x/gov/client/cli/util_test.go (2 hunks)
Additional comments: 6
x/gov/client/cli/util_test.go (4)
- 350-350: The error message update to "invalid character in denomination" improves clarity. However, ensure that this new message accurately reflects all possible causes of the error, not just invalid characters in the denomination. If there are other reasons for an "invalid deposit" error, consider adding more specific error handling or a more general error message.
- 380-380: Similar to the previous comment, the updated error message is more descriptive but ensure it covers all potential error scenarios related to the deposit's denomination. If there are multiple reasons for the deposit to be considered invalid, the error message should either be generalized or handle specific cases distinctly.
- 386-386: Again, the updated error message is clearer but verify that it encompasses all reasons why a deposit might be invalid. If the error could be caused by factors other than invalid characters in the denomination, consider refining the error handling to address these cases specifically or use a more general error message.
- 392-392: As with the previous comments, the updated error message enhances clarity but ensure it accurately reflects all possible causes of the error. If there are other reasons for an "invalid deposit" error besides invalid characters in the denomination, additional error handling or a more general error message might be necessary.
tests/integration/staking/keeper/determinstic_test.go (1)
- 690-693: The custom regex pattern for coin denomination validation appears to align with the PR's objectives of enhancing validation logic. However, it's crucial to ensure that this pattern comprehensively covers all valid coin denominations and does not inadvertently exclude any. Consider reviewing the pattern against the system's requirements for coin denominations to confirm its adequacy.
tests/e2e/authz/tx.go (1)
- 342-342: The error message in the test case "invalid decimal coin expression with more than single coin" has been updated from "invalid decimal coin expression" to "invalid character in denomination". This change aligns with the PR objectives to enhance validation of coin denominations by eliminating the use of regular expressions. Ensure that the updated error message accurately reflects the validation logic implemented in the codebase.
| @@ -326,7 +326,7 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() { | |||
| fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(10))).String()), | |||
| }, | |||
| true, | |||
| "invalid decimal coin expression", | |||
| "nvalid character in denomination: ", | |||
There was a problem hiding this comment.
The error message has been modified from "invalid decimal coin expression" to "nvalid character in denomination: ". This change seems to have a typo, missing the initial "I" in "Invalid". Correcting this typo will ensure the error message is clear and accurately reflects the validation error.
- "nvalid character in denomination: "
+ "Invalid character in denomination: "Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| "nvalid character in denomination: ", | |
| "Invalid character in denomination: ", |
|
I agree, idc about linting |
|
I mean, can we add an ignore line so every future PR doesnt fail |
|
Its just saying to use switch instead of if else, should be able to add a gocritic ignore |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Commits
Files that changed from the base of the PR and between 00b88c7 and 95f01859710148ec7fad4f304f79651b364b2d3d.Files selected for processing (1)
- types/coin.go (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- types/coin.go
|
@czarcas7ic ok, I actually can't fix the lint ( |
|
Yeah I am fine with any solution that keeps a lint CI check and passed |
mattverse
force-pushed
the
mattverse/fix-rag
branch
from
275db28 to
00b88c7
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Commits
Files that changed from the base of the PR and between 95f01859710148ec7fad4f304f79651b364b2d3d and 275db287ea3416a15f859a3c63bed62b4de2b928.Files selected for processing (4)
- .github/workflows/lint.yml (1 hunks)
- Makefile (1 hunks)
- scripts/go-lint-all.bash (1 hunks)
- types/coin.go (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- types/coin.go
Additional comments: 12
.github/workflows/lint.yml (6)
- 5-5: Triggering the workflow on pushes to the
mainbranch is a standard practice for continuous integration. This ensures that the codebase remains clean and adheres to linting standards.- 16-19: The update to
actions/setup-go@v5and specifying Go version1.20withcheck-latest: trueensures that the workflow uses the latest patch version of Go 1.20. This is a good practice for staying up-to-date with Go releases.- 23-30: The use of
technote-space/[email protected]to set up conditional linting based on file patterns is a smart approach to optimize CI runtime. It avoids unnecessary linting steps when unrelated files are changed.- 31-36: Conditional execution of linting steps based on the presence of changes detected by
get-diff-actionis a good practice. It ensures that linting is only performed when necessary, saving CI resources.- 37-43: Introducing a second
get-diff-actionstep to handle different file patterns (Go files in this case) after the first linting step is skipped is a clever way to ensure all relevant changes are linted. However, ensure that the environment variableGIT_DIFFis correctly set and used in subsequent steps.Please verify that the
GIT_DIFFenvironment variable is correctly propagated and utilized in the linting steps. It might be beneficial to explicitly set this variable in the workflow file for clarity and maintainability.
- 44-50: The final linting step, conditioned on the outcome of the previous steps and the presence of changes in Go files, demonstrates a well-thought-out workflow optimization. This ensures that linting is performed efficiently and only when necessary.
scripts/go-lint-all.bash (4)
- 1-1: Starting the script with
#!/usr/bin/env bashis a good practice for portability across different Unix-like systems.- 8-11: Adding conditional tags based on the presence of the
NIXenvironment variable allows for flexible linting configurations. This is particularly useful when certain dependencies or configurations are only available in a Nix environment.- 14-28: The
lint_modulefunction provides a structured way to lint individual modules within the repository. It handles both cases where the module is specified by a path or directly by ago.modfile. This flexibility is beneficial for repositories with multiple Go modules.- 32-61: The conditional logic for linting only the files changed in the current commit (
LINT_DIFFenvironment variable) is a smart optimization for CI/CD pipelines. It reduces the linting time by focusing on the most relevant changes. However, ensure that theGIT_DIFFvariable is correctly populated and used throughout the script.Please verify the correct population and usage of the
GIT_DIFFvariable, especially in the context of CI/CD pipelines where the environment might differ from local development setups.Makefile (2)
- 352-355: Introducing a
lint-installtarget to ensuregolangci-lintis installed before linting is a good practice. It guarantees that the linter is available and up-to-date, which is crucial for consistent linting results.- 360-361: Modifying the
linttarget to include thelint-installstep and specifying a timeout of 15 minutes for the linting script improves the reliability of the linting process. It ensures that the linter is installed and allows for a generous timeout to accommodate larger codebases or slower CI environments.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (2)
- types/coin.go (3 hunks)
- types/dec_coin.go (3 hunks)
Check Runs (1)
golangci-lint failure (4)
- types/dec_coin.go: 665-665: ifElseChain: rewrite if-else to switch statement (gocritic)
- types/dec_coin.go: 666-666: ifElseChain: rewrite if-else to switch statement (gocritic)
Files skipped from review as they are similar to previous changes (1)
- types/coin.go
Additional comments: 2
types/dec_coin.go (2)
- 7-7: The addition of the
unicodepackage is appropriate for the new parsing logic introduced inParseDecAmount, which relies on character classification functions likeunicode.IsDigitandunicode.IsLetter. This change supports the PR's objective of removing regex usage for denom validation.- 625-641: The modification to
ParseDecCointo conditionally use custom parsing logic (ParseDecAmount) instead of regex (whenreDecCoinisnil) is a significant change aligning with the PR's objective. This approach enhances maintainability and potentially improves performance by avoiding regex for common cases. However, ensure that the fallback to regex (lines 633-641) is still necessary and correctly implemented, as the PR's goal is to move away from regex usage.
types/dec_coin.go
Outdated
Comment on lines
655
to
697
| // 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 { | ||
| if unicode.IsDigit(r) || r == '.' { | ||
| 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 | ||
| } |
There was a problem hiding this comment.
The introduction of ParseDecAmount for parsing the amount and denomination from a coin string directly, without relying on regular expressions, is a significant improvement. This change aligns with the PR's objective and enhances the code's readability and maintainability. However, consider refactoring the if-else chain in lines 665-665 and 666-666 into a switch statement for improved clarity and maintainability.
for _, r := range strings.TrimSpace(coinStr) {
- if parsingAmount {
+ switch {
+ case parsingAmount:
if unicode.IsDigit(r) || r == '.' {
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
+ case !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 {
+ default:
// 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))
}
}
}Additionally, ensure comprehensive testing of ParseDecAmount to cover various edge cases and input scenarios, as this function is critical for parsing coin strings accurately.
Would you like me to help generate test cases for ParseDecAmount to ensure it handles edge cases effectively?
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| // 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 { | |
| if unicode.IsDigit(r) || r == '.' { | |
| 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 | |
| } | |
| for _, r := range strings.TrimSpace(coinStr) { | |
| switch { | |
| case parsingAmount: | |
| if unicode.IsDigit(r) || r == '.' { | |
| 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)) | |
| } | |
| case !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)) | |
| } | |
| default: | |
| // 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)) | |
| } | |
| } | |
| } |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- types/coin.go (3 hunks)
- types/dec_coin.go (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- types/coin.go
- types/dec_coin.go
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- CHANGELOG.md (1 hunks)
Additional comments: 3
CHANGELOG.md (3)
- 40-51: The entry for the removal of regex usage on denom validation is correctly placed under the "State Compatible" section, aligning with the nature of the change as it doesn't break existing state but improves upon it.
- 40-51: There are several instances of possible spelling mistakes or typographical errors throughout the changelog. While these are minor and do not impact the functionality or clarity significantly, it's good practice to maintain a high standard of professionalism in documentation. Consider reviewing and correcting these for a polished presentation.
Examples include "CacheKV speedups" which might be better as "Cache KV speedups", and "authz" which could be clarified as "authorization (authz)" on its first occurrence to aid readability.
- 40-51: The inclusion of PR links for each change is excellent for traceability and further investigation. However, ensure all links are valid and lead to the correct PRs. This practice enhances the changelog's utility as a historical document and reference.
mergify bot
pushed a commit
that referenced
this pull request
Mar 16, 2024
* 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]> (cherry picked from commit ab4bc05)
19 tasks
mergify bot
pushed a commit
that referenced
this pull request
Mar 16, 2024
* 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]> (cherry picked from commit ab4bc05)
19 tasks
czarcas7ic
pushed a commit
that referenced
this pull request
Mar 16, 2024
* 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]> (cherry picked from commit ab4bc05) Co-authored-by: Matt, Park <[email protected]>
czarcas7ic
pushed a commit
that referenced
this pull request
Mar 16, 2024
* 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]> (cherry picked from commit ab4bc05) Co-authored-by: Matt, Park <[email protected]>
czarcas7ic
added a commit
that referenced
this pull request
Mar 25, 2024
This reverts commit ab4bc05.
czarcas7ic
added a commit
that referenced
this pull request
Mar 25, 2024
This reverts commit ab4bc05.
czarcas7ic
added a commit
that referenced
this pull request
Mar 25, 2024
This reverts commit ab4bc05.
czarcas7ic
added a commit
that referenced
this pull request
Mar 25, 2024
This reverts commit a07f5de.
czarcas7ic
added a commit
that referenced
this pull request
Mar 25, 2024
czarcas7ic
added a commit
that referenced
this pull request
Mar 25, 2024
* Reapply "Backport: Removal of regex usage on denom validation (#570)" This reverts commit a07f5de. * changelog (cherry picked from commit 0732c8a) Co-authored-by: Adam Tucker <[email protected]>
czarcas7ic
added a commit
that referenced
this pull request
May 9, 2024
* 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]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Closes: #XXXX
Backports removal of regex usage on denom validation upstream
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking changeSummary by CodeRabbit