feat: add handling for empty string in address validation #187
Conversation
- Updated ValidateAddressesOrHexAddresses to return nil for empty input. - Added a test case for empty string input in TestValidateAddressesOrHexAddresses.
Benzbeeb
requested review from
WasinWatt,
songwongtp and
traviolus
as code owners
WalkthroughAn early return was added to ValidateAddressesOrHexAddresses to accept an empty string as valid input. Corresponding unit test coverage was updated by adding a test case asserting no error for empty input. No public APIs or signatures were changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant V as ValidateAddressesOrHexAddresses
Caller->>V: ValidateAddressesOrHexAddresses(s)
alt s == ""
note right of V: New early guard
V-->>Caller: nil (no error)
else s != ""
V->>V: Iterate addresses / hex addresses
V-->>Caller: nil or error
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
common/validate.go (1)
121-129: Potential panic and incorrect error message in ValidateDecFromStr.Indexing str[0] before checking len(str) can panic on empty input, and the error says “cannot be positive” when the string is actually negative.
-func ValidateDecFromStr(str string) error { - if str[0] == '-' { - return fmt.Errorf("decimal string cannot be positive") - } - if len(str) == 0 { - return ErrLegacyEmptyDecimalStr - } +func ValidateDecFromStr(str string) error { + if len(str) == 0 { + return ErrLegacyEmptyDecimalStr + } + if str[0] == '-' { + return fmt.Errorf("decimal string cannot be negative") + }
🧹 Nitpick comments (1)
common/validate.go (1)
382-386: Trim tokens and skip empties to handle whitespace and trailing commas gracefully.Prevents false negatives for inputs like "addr1, addr2" and treats whitespace-only as empty. If acceptable, apply:
- for _, address := range strings.Split(s, ",") { - if err := ValidateAnyHexAddressOrAddress(address); err != nil { - return err - } - } + for _, address := range strings.Split(s, ",") { + address = strings.TrimSpace(address) + if address == "" { + continue + } + if err := ValidateAnyHexAddressOrAddress(address); err != nil { + return err + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
common/validate.go(1 hunks)common/validate_test.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Integration Tests on Ubuntu
🔇 Additional comments (2)
common/validate_test.go (1)
280-284: LGTM: empty-input test covers the new behavior.Nice parity with ValidateAddresses treating empty as valid.
Consider adding follow-ups if you adopt whitespace handling:
- "whitespace only" input: " " → no error
- spaces around tokens: "addr1, addr2" → no error
common/validate.go (1)
379-381: Approve: empty-input early return is correct.Matches ValidateAddresses; tests (common/validate_test.go — TestValidateAddressesOrHexAddresses includes an empty-string case expecting no error) and the only non-test caller found is models/minitia/launch.go (registers this validator), so this is backward‑compatible.
2 participants
Description
Closes: #XXXX
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...
!in the type prefix if API or client breaking changeReviewers 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...
Summary by CodeRabbit