-
Notifications
You must be signed in to change notification settings - Fork 198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: validators query to use power index at bonded validator query #194
Conversation
WalkthroughWalkthroughThe Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant Client
participant Querier
participant ValidatorStore
Client->>Querier: QueryValidatorsRequest(status)
Querier->>ValidatorStore: Fetch validators based on status
alt Status is Bonded
ValidatorStore-->>Querier: Bonded validators
else Other statuses
ValidatorStore-->>Querier: Validators based on status
end
Querier-->>Client: QueryValidatorsResponse(validators)
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- x/mstaking/keeper/grpc_query.go (1 hunks)
Additional comments not posted (1)
x/mstaking/keeper/grpc_query.go (1)
51-51
: Ensure comprehensive error handling for new query paths.The addition of separate handling for the 'Bonded' status introduces new paths in the code. It's important to ensure that all error scenarios are covered adequately. Consider adding more specific error messages or handling for cases where the new query logic might fail.
var validators []types.Validator | ||
var pageRes *query.PageResponse | ||
var err error | ||
|
||
if req.Status == types.Bonded.String() { | ||
validators, pageRes, err = query.CollectionPaginate(ctx, q.Keeper.ValidatorsByConsPowerIndex, req.Pagination, func(key collections.Pair[int64, []byte], _ bool) (types.Validator, error) { | ||
valAddr := key.K2() | ||
return q.Keeper.Validators.Get(ctx, valAddr) | ||
}) | ||
} else { | ||
validators, pageRes, err = query.CollectionFilteredPaginate(ctx, q.Keeper.Validators, req.Pagination, func(valAddr []byte, val types.Validator) (include bool, err error) { | ||
return (req.Status == "" || strings.EqualFold(val.GetStatus().String(), req.Status)), nil | ||
}, func(valAddr []byte, val types.Validator) (types.Validator, error) { | ||
return val, nil | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactor the handling of bonded validators to optimize query performance.
The separation of the 'Bonded' status handling is a good approach to optimize the performance for specific queries. However, consider abstracting the querying logic into separate methods to improve readability and maintainability. Additionally, ensure that the new querying logic is covered by unit tests to prevent regressions.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #194 +/- ##
=======================================
Coverage 37.81% 37.81%
=======================================
Files 251 251
Lines 24115 24125 +10
=======================================
+ Hits 9119 9123 +4
- Misses 13523 13529 +6
Partials 1473 1473
|
No description provided.