fix: fix ragel denom validation introduced in #19511

[mattverse]

Description

Closes: #XXXX

Fixes a bug introduced in #19511.
Spaces are not supposed to be allowed in denominations, whilst existing ragel code allowed did. This PR fixes it.

This bug was previously not found since we do not have any direct test testing ValidateDenom. I've also added the test to ensure that this is fully covered.

---

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...

• included the correct type prefix in the PR title
• confirmed ! in the type prefix if API or client breaking change
• targeted the correct branch (see PR Targeting)
• provided a link to the relevant issue or specification
• reviewed "Files changed" and left comments if necessary
• included the necessary unit and integration tests
• added a changelog entry to CHANGELOG.md
• updated the relevant documentation or specification, including comments for documenting Go code
• confirmed all CI checks have passed

Reviewers 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...

• confirmed the correct type prefix in the PR title
• confirmed all author checklist items have been addressed
• reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

Summary by CodeRabbit

• New Features
  • Enhanced denomination matching to include alphabets, numbers, and special characters within a broader range.
• Tests
  • Introduced new tests for validating denominations with diverse character sets.

[coderabbitai]

Walkthrough

Walkthrough

The recent updates focus on enhancing the regex parsing capabilities for cryptocurrency denominations. These modifications span across various files, introducing refined parsing logic in the regex generator, expanding the denomination pattern to include alphanumeric and special characters, and incorporating comprehensive tests to validate these changes. The core aim remains to improve the accuracy and range of denomination matching within the system.

Changes

File(s)	Summary
types/coin_regex.go	Adjusted regex parsing logic including variable declarations, transitions, actions, and indices.
types/coin_regex.rl	Expanded MatchDenom function's pattern to include alphabetNumber and special characters, with 2-127 range.
types/coin_test.go	Added TestValidateDenom in coinTestSuite to validate denominations with various characters and numbers.

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?

Share

• X
• Mastodon
• Reddit
• LinkedIn

---

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit-tests for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit tests for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit tests.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: .coderabbit.yml

Commits

Files that changed from the base of the PR and between 28801fc and d5690a4.

Files selected for processing (3)

• types/coin_regex.go (2 hunks)
• types/coin_regex.rl (1 hunks)
• types/coin_test.go (1 hunks)

Additional comments: 3

types/coin_regex.rl (1)

• 29-31: The adjustment to denom_pattern in coin_regex.rl, including both alphabetNumber and special characters within a repetition range of 2 to 127, is a crucial enhancement for the denomination validation logic. This change effectively tightens the validation criteria, ensuring that denominations adhere to the intended format without spaces. The syntax and logic of the modification are correct and align with the PR objectives.

types/coin_regex.go (1)

• 17-467: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-467]

The modifications in coin_regex.go are consistent with the expected output from the adjustments made in the Ragel source file (coin_regex.rl). These changes to the regex parsing logic, including adjustments to variable declarations, transitions, actions, and indices, are in line with the PR objectives to enhance denomination validation. No new issues are introduced, and the core functionality remains focused on matching denominations within the specified range.

types/coin_test.go (1)

• 1024-1046: The new test function TestValidateDenom provides comprehensive coverage for the denomination validation logic, including edge cases and special characters. It's a valuable addition to ensure the robustness of the validation logic.

[julienrbrt]

--- FAIL: TestCLITestSuite (0.03s)
    --- FAIL: TestCLITestSuite/TestNewCmdSubmitProposal (0.00s)
        --- FAIL: TestCLITestSuite/TestNewCmdSubmitProposal/invalid_proposal (0.00s)
            tx_test.go:171: 
                        Error Trace:    /home/julien/projects/cosmos/cosmos-sdk/x/gov/client/cli/tx_test.go:171
                                                                /home/julien/tools/go/pkg/mod/github.com/stretchr/[email protected]/suite/suite.go:115
                        Error:          "invalid character in coin string: -" does not contain "invalid decimal coin expression"
                        Test:           TestCLITestSuite/TestNewCmdSubmitProposal/invalid_proposal
FAIL
FAIL    cosmossdk.io/x/gov/client/cli   0.120s

--- FAIL: TestKeeperTestSuite (0.04s)
    --- FAIL: TestKeeperTestSuite/TestMsgSetSendEnabled (0.00s)
        --- FAIL: TestKeeperTestSuite/TestMsgSetSendEnabled/bad_first_denom_name,_(invalid_send_enabled_denom_present_in_list) (0.00s)
            msg_server_test.go:357: 
                        Error Trace:    /home/julien/projects/cosmos/cosmos-sdk/x/bank/keeper/msg_server_test.go:357
                                                                /home/julien/tools/go/pkg/mod/github.com/stretchr/[email protected]/suite/suite.go:115
                        Error:          An error is expected but got nil.
                        Test:           TestKeeperTestSuite/TestMsgSetSendEnabled/bad_first_denom_name,_(invalid_send_enabled_denom_present_in_list)
        --- FAIL: TestKeeperTestSuite/TestMsgSetSendEnabled/bad_second_denom_name,_(invalid_send_enabled_denom_present_in_list) (0.00s)
            msg_server_test.go:357: 
                        Error Trace:    /home/julien/projects/cosmos/cosmos-sdk/x/bank/keeper/msg_server_test.go:357
                                                                /home/julien/tools/go/pkg/mod/github.com/stretchr/[email protected]/suite/suite.go:115
                        Error:          An error is expected but got nil.
                        Test:           TestKeeperTestSuite/TestMsgSetSendEnabled/bad_second_denom_name,_(invalid_send_enabled_denom_present_in_list)
        --- FAIL: TestKeeperTestSuite/TestMsgSetSendEnabled/invalid_UseDefaultFor_denom (0.00s)
            msg_server_test.go:357: 
                        Error Trace:    /home/julien/projects/cosmos/cosmos-sdk/x/bank/keeper/msg_server_test.go:357
                                                                /home/julien/tools/go/pkg/mod/github.com/stretchr/[email protected]/suite/suite.go:115
                        Error:          An error is expected but got nil.
                        Test:           TestKeeperTestSuite/TestMsgSetSendEnabled/invalid_UseDefaultFor_denom
FAIL
FAIL    cosmossdk.io/x/bank/keeper      0.061s
ok      cosmossdk.io/x/bank/simulation  0.024s
ok      cosmossdk.io/x/bank/types       0.021s
FAIL
Running unit tests for module cosmossdk.io/x/circuit
?       cosmossdk.io/x/circuit  [no test files]
ok      cosmossdk.io/x/circuit/ante     0.014s
?       cosmossdk.io/x/circuit/types    [no test files]
ok      cosmossdk.io/x/circuit/keeper   0.018s
Running unit tests for module cosmossdk.io/x/distribution
?       cosmossdk.io/x/distribution     [no test files]
?       cosmossdk.io/x/distribution/client/cli  [no test files]
ok      cosmossdk.io/x/distribution/client/common       0.023s
ok      cosmossdk.io/x/distribution/keeper      0.026s
?       cosmossdk.io/x/distribution/testutil    [no test files]
ok      cosmossdk.io/x/distribution/migrations/v4       0.020s
ok      cosmossdk.io/x/distribution/simulation  0.028s
ok      cosmossdk.io/x/distribution/types       0.019s
Running unit tests for module cosmossdk.io/x/evidence
?       cosmossdk.io/x/evidence [no test files]
?       cosmossdk.io/x/evidence/client  [no test files]
?       cosmossdk.io/x/evidence/client/cli      [no test files]
?       cosmossdk.io/x/evidence/exported        [no test files]
?       cosmossdk.io/x/evidence/testutil        [no test files]
ok      cosmossdk.io/x/evidence/keeper  0.042s
ok      cosmossdk.io/x/evidence/simulation      0.017s
ok      cosmossdk.io/x/evidence/types   0.019s
Running unit tests for module cosmossdk.io/x/feegrant
ok      cosmossdk.io/x/feegrant 0.043s
?       cosmossdk.io/x/feegrant/testutil        [no test files]
ok      cosmossdk.io/x/feegrant/client/cli      0.069s
ok      cosmossdk.io/x/feegrant/keeper  0.033s
ok      cosmossdk.io/x/feegrant/migrations/v2   0.028s
ok      cosmossdk.io/x/feegrant/module  0.026s
ok      cosmossdk.io/x/feegrant/simulation      0.041s
Running unit tests for module cosmossdk.io/x/gov
?       cosmossdk.io/x/gov/client       [no test files]
ok      cosmossdk.io/x/gov      0.029s [no tests to run]
?       cosmossdk.io/x/gov/client/testutil      [no test files]
--- FAIL: TestReadGovPropFlags (0.02s)
    --- FAIL: TestReadGovPropFlags/only_deposit_invalid_coins (0.00s)
        util_test.go:701: 
                Error Trace:    /home/julien/projects/cosmos/cosmos-sdk/x/gov/client/cli/util_test.go:701
                Error:          Error "invalid deposit: invalid character in denomination:  " does not contain "invalid decimal coin expression"
                Test:           TestReadGovPropFlags/only_deposit_invalid_coins
                Messages:       ReadGovPropFlags error
        util_test.go:701: 
                Error Trace:    /home/julien/projects/cosmos/cosmos-sdk/x/gov/client/cli/util_test.go:701
                Error:          Error "invalid deposit: invalid character in denomination:  " does not contain "not really coins"
                Test:           TestReadGovPropFlags/only_deposit_invalid_coins
                Messages:       ReadGovPropFlags error
    --- FAIL: TestReadGovPropFlags/only_deposit_coin_1_of_3_bad (0.00s)
        util_test.go:701: 
                Error Trace:    /home/julien/projects/cosmos/cosmos-sdk/x/gov/client/cli/util_test.go:701
                Error:          Error "invalid deposit: invalid character in denomination: ^" does not contain "invalid decimal coin expression"
                Test:           TestReadGovPropFlags/only_deposit_coin_1_of_3_bad
                Messages:       ReadGovPropFlags error
        util_test.go:701: 
                Error Trace:    /home/julien/projects/cosmos/cosmos-sdk/x/gov/client/cli/util_test.go:701
                Error:          Error "invalid deposit: invalid character in denomination: ^" does not contain "1bad^coin"
                Test:           TestReadGovPropFlags/only_deposit_coin_1_of_3_bad
                Messages:       ReadGovPropFlags error
    --- FAIL: TestReadGovPropFlags/only_deposit_coin_2_of_3_bad (0.00s)
        util_test.go:701: 
                Error Trace:    /home/julien/projects/cosmos/cosmos-sdk/x/gov/client/cli/util_test.go:701
                Error:          Error "invalid deposit: invalid character in denomination: ^" does not contain "invalid decimal coin expression"
                Test:           TestReadGovPropFlags/only_deposit_coin_2_of_3_bad
                Messages:       ReadGovPropFlags error
        util_test.go:701: 
                Error Trace:    /home/julien/projects/cosmos/cosmos-sdk/x/gov/client/cli/util_test.go:701
                Error:          Error "invalid deposit: invalid character in denomination: ^" does not contain "2bad^coin"
                Test:           TestReadGovPropFlags/only_deposit_coin_2_of_3_bad
                Messages:       ReadGovPropFlags error
    --- FAIL: TestReadGovPropFlags/only_deposit_coin_3_of_3_bad (0.00s)
        util_test.go:701: 
                Error Trace:    /home/julien/projects/cosmos/cosmos-sdk/x/gov/client/cli/util_test.go:701
                Error:          Error "invalid deposit: invalid character in denomination: ^" does not contain "invalid decimal coin expression"
                Test:           TestReadGovPropFlags/only_deposit_coin_3_of_3_bad
                Messages:       ReadGovPropFlags error
        util_test.go:701: 
                Error Trace:    /home/julien/projects/cosmos/cosmos-sdk/x/gov/client/cli/util_test.go:701
                Error:          Error "invalid deposit: invalid character in denomination: ^" does not contain "3bad^coin"
                Test:           TestReadGovPropFlags/only_deposit_coin_3_of_3_bad
                Messages:       ReadGovPropFlags error

canceled transaction
--- FAIL: TestCLITestSuite (0.03s)
    --- FAIL: TestCLITestSuite/TestCLITxGrantAuthorization (0.00s)
        --- FAIL: TestCLITestSuite/TestCLITxGrantAuthorization/invalid_decimal_coin_expression_with_more_than_single_coin (0.00s)
            tx_test.go:455: 
                        Error Trace:    /home/julien/projects/cosmos/cosmos-sdk/x/authz/client/cli/tx_test.go:455
                                                                /home/julien/tools/go/pkg/mod/github.com/stretchr/[email protected]/suite/suite.go:115
                        Error:          "invalid character in denomination: ," does not contain "invalid decimal coin expression"
                        Test:           TestCLITestSuite/TestCLITxGrantAuthorization/invalid_decimal_coin_expression_with_more_than_single_coin
FAIL
FAIL    cosmossdk.io/x/authz/client/cli 0.074

Those tests were failing on main since the previous Rachel PR.
Does this fix it too?

AFK right now, but I'll run make test once I'm back, as CI runs on the diff only.

[julienrbrt]

So it seems like , should be rejected too, while it passes after this still.