fix(affiliates): Check address is valid in ReferredBy query [OTE-896]
#2567
Conversation
WalkthroughThe changes in this pull request enhance the error handling in the Changes
Possibly related PRs
Suggested reviewers
Poem
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
teddyding
changed the title
ReferredBy queryReferredBy query
|
@Mergifyio backport release/protocol/v7.x |
✅ Backports have been created
|
teddyding
changed the title
ReferredBy queryReferredBy query [OTE-896]
![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
🧹 Outside diff range and nitpick comments (2)
protocol/x/affiliates/keeper/grpc_query.go (1)
62-68: Consider extracting common address validation logic.This address validation pattern appears in multiple methods (e.g., AffiliateInfo and ReferredBy). Consider extracting it into a helper method to reduce code duplication and maintain consistency.
Example implementation:
+ func (k Keeper) validateBech32Address(address string) error { + _, err := sdk.AccAddressFromBech32(address) + if err != nil { + return errorsmod.Wrapf(types.ErrInvalidAddress, "address: %s, error: %s", + address, err.Error()) + } + return nil + }Then use it in both methods:
- // Check req.Address is a valid bech32 address - _, err := sdk.AccAddressFromBech32(req.GetAddress()) - if err != nil { - return nil, errorsmod.Wrapf(types.ErrInvalidAddress, "address: %s, error: %s", - req.GetAddress(), err.Error()) - } + if err := k.validateBech32Address(req.GetAddress()); err != nil { + return nil, err + }protocol/x/affiliates/keeper/grpc_query_test.go (1)
193-200: LGTM! Consider adding more invalid address test cases.The new test case correctly verifies that an invalid bech32 address returns
ErrInvalidAddress. The implementation follows the established test pattern and aligns with the PR objective.Consider adding more test cases to cover different invalid address scenarios:
- Empty string
- Valid string but not a bech32 address (e.g., "cosmos1...")
- Malformed bech32 string with valid prefix
- Address with incorrect length
Example addition:
"Invalid bech32 address": { req: &types.ReferredByRequest{ Address: "Foo", }, setup: func(ctx sdk.Context, k keeper.Keeper) {}, expected: nil, expectError: types.ErrInvalidAddress, }, +"Empty address": { + req: &types.ReferredByRequest{ + Address: "", + }, + setup: func(ctx sdk.Context, k keeper.Keeper) {}, + expected: nil, + expectError: types.ErrInvalidAddress, +}, +"Invalid bech32 format": { + req: &types.ReferredByRequest{ + Address: "cosmos1invalid", + }, + setup: func(ctx sdk.Context, k keeper.Keeper) {}, + expected: nil, + expectError: types.ErrInvalidAddress, +},
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
protocol/x/affiliates/keeper/grpc_query.go(1 hunks)protocol/x/affiliates/keeper/grpc_query_test.go(1 hunks)
🔇 Additional comments (2)
protocol/x/affiliates/keeper/grpc_query.go (1)
62-68: LGTM! Address validation implementation looks good.
The validation is implemented correctly and follows the same pattern as other methods in the keeper.
protocol/x/affiliates/keeper/grpc_query_test.go (1)
193-200: Implementation aligns with PR objectives
The new test case successfully validates the enhancement to the ReferredBy query by ensuring it returns an error for invalid bech32 addresses. This implementation directly addresses the PR's goal of preventing confusion from user input errors.
Changelist
ReferredByquery returns error if input address is not valid bech32 address. Prevents confusion when user/client typos the addressTest Plan
Test on localnet
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit
New Features
Bug Fixes
Tests