chore: stakers queries #232
Conversation
|
Warning Rate limit exceeded@troykessler has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 30 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughThis pull request introduces comprehensive changes to the KYVE protocol's configuration and query systems, focusing on enhancing delegation, validator metrics, and reward tracking. The modifications span multiple files, including configuration, protocol buffers, and query-related components. The changes primarily revolve around introducing more detailed tracking of validator and delegation information, adding new fields for delegation rewards, commission rewards, and providing more granular insights into validator stakes and delegations. Changes
Sequence DiagramsequenceDiagram
participant Client
participant QueryService
participant StakingKeeper
participant Validator
Client->>QueryService: Request Staker/Account Details
QueryService->>StakingKeeper: Get Full Staker/Account Info
StakingKeeper->>Validator: Retrieve Validator Details
Validator-->>StakingKeeper: Return Validator Data
StakingKeeper->>StakingKeeper: Calculate Rewards & Delegations
StakingKeeper-->>QueryService: Return Detailed Staker Info
QueryService-->>Client: Respond with Comprehensive Staker Metrics
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
docs/static/openapi.yml (1)
Line range hint
194-242: Improve API documentation clarity and completeness.The documentation has several issues that should be addressed:
- Complete the truncated descriptions:
delegationdescription ends with "..."delegation_unbondingdescription ends with "..."- Remove implementation details about gogoproto from user-facing API documentation.
Apply these changes:
delegation: type: string format: uint64 description: >- - delegation is the amount in $KYVE this account has in total delegated ... + delegation is the amount in $KYVE this account has in total delegated to validators delegation_unbonding: type: string format: uint64 description: >- - delegation_unbonding is the amount in $KYVE this account has in total currently unbonding ... + delegation_unbonding is the amount in $KYVE this account has in total currently unbonding from validators delegation_rewards: type: array items: type: object properties: denom: type: string amount: type: string description: >- Coin defines a token with a denomination and an amount. - - - NOTE: The amount field is an Int which implements the custom method - - signatures required by gogoproto.Apply similar changes to remove the gogoproto implementation note from
commission_rewards.
🧹 Nitpick comments (2)
x/query/keeper/grpc_query_stakers_by_pool.go (1)
28-32: Consider Error Handling Strategy in LoopWhen an error occurs in
GetFullStaker, the current implementation stops processing and returns the error immediately. Assess whether it's preferable to continue processing the remaining stakers and collect successful results, possibly logging the errors without halting the entire operation.proto/kyve/query/v1beta1/query.proto (1)
78-102: Consider adding field deprecation notices.For better backward compatibility, consider adding
[deprecated=true]to the old fields and keeping them for one release cycle before removal. This would allow clients to adapt to the new field structure gradually.Example for the old field:
+ // Deprecated: Use validator_total_pool_stake instead + uint64 total_pool_stake = 3 [deprecated=true];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
x/query/types/account.pb.gois excluded by!**/*.pb.gox/query/types/query.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (11)
config.yml(2 hunks)docs/static/openapi.yml(6 hunks)proto/kyve/query/v1beta1/account.proto(1 hunks)proto/kyve/query/v1beta1/query.proto(1 hunks)testutil/integration/checks.go(1 hunks)util/expected_keepers.go(1 hunks)x/query/keeper/grpc_account_assets.go(1 hunks)x/query/keeper/grpc_query_staker.go(2 hunks)x/query/keeper/grpc_query_stakers_by_pool.go(1 hunks)x/query/keeper/grpc_query_stakers_by_pool_count.go(1 hunks)x/query/keeper/helper.go(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- config.yml
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (20)
x/query/keeper/helper.go (2)
34-36: Verify Error Handling forIterateDelegatorUnbondingDelegationsEnsure that the
IterateDelegatorUnbondingDelegationsfunction returns an error and that it is being handled correctly. If it does not return an error, the error handling may be unnecessary.
49-52: Verify Error Handling forGetValidatorAccumulatedCommissionConfirm whether the
GetValidatorAccumulatedCommissionmethod returns an error. If it does not, the error handling may be unnecessary.x/query/keeper/grpc_query_stakers_by_pool_count.go (1)
23-27: Ensure Proper Handling in Accumulator FunctionReturning
falsefrom the accumulator may affect the pagination or iteration process. Verify that this is the intended behavior whenGetFullStakerencounters an error.x/query/keeper/grpc_query_staker.go (2)
26-29: LGTM! Improved error handling in accumulator function.The change properly handles errors from
GetFullStakerby skipping invalid stakers, preventing nil values from being added to the response.
67-71: LGTM! Enhanced error propagation.The change correctly propagates errors from
GetFullStakerto the caller, improving error handling robustness.x/query/keeper/grpc_account_assets.go (2)
48-52: LGTM! Improved delegation rewards calculation.The change correctly aggregates delegation rewards for each validator associated with the delegator's address.
53-54: LGTM! Added commission rewards tracking.The change adds commission rewards calculation, properly separating it from delegation rewards.
util/expected_keepers.go (1)
50-52: LGTM! Enhanced StakingKeeper interface with delegation methods.The new methods provide comprehensive access to delegation data:
IterateDelegatorUnbondingDelegations: For processing unbonding delegationsGetValidatorDelegations: For accessing all delegations of a validatorGetDelegation: For retrieving specific delegator-validator pairstestutil/integration/checks.go (1)
126-129: LGTM! Improved error handling in test verification.The change enhances the robustness of the test verification by:
- Properly handling errors from
GetFullStaker- Only including successfully retrieved stakers in the verification
docs/static/openapi.yml (1)
Line range hint
194-6733: Verify consistency with proto files.The OpenAPI changes should align with the proto file changes mentioned in the AI summary, particularly:
proto/kyve/query/v1beta1/account.protoproto/kyve/query/v1beta1/query.protoLet's verify the field definitions match:
proto/kyve/query/v1beta1/account.proto (4)
41-43: LGTM! Clear and descriptive field renaming.The renaming from
protocol_stakingtodelegationandprotocol_staking_unbondingtodelegation_unbondingimproves clarity and aligns better with domain terminology.
45-50: LGTM! Well-structured delegation rewards field.The new
delegation_rewardsfield is properly configured with appropriate protobuf options:
- Non-nullable to ensure consistent data
- Don't omit empty to maintain field presence
- Correct casting to Cosmos SDK Coins type
51-56: LGTM! Well-structured commission rewards field.The new
commission_rewardsfield follows the same robust configuration pattern as delegation_rewards.
Line range hint
58-62: Verify field numbering change impact.The
protocol_fundingfield has been moved from index 7 to 8. While the configuration is correct, this change in field numbering could impact existing clients.Run this script to check for potential breaking changes in field usage:
✅ Verification successful
Field numbering change appears safe
The verification found no direct usage of field numbers in client code. References to
protocol_fundingexist only in OpenAPI documentation and generated protobuf code, which will be automatically updated. This change should not impact existing clients as long as they use proper protobuf practices.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for direct usage of field numbers in the codebase rg -g '!*.pb.go' -g '!*.proto' "protocol_funding.*7"Length of output: 54
Script:
#!/bin/bash # Check git history for this specific proto file git log -p -- proto/kyve/query/v1beta1/account.proto | grep -A 5 -B 5 "protocol_funding" # Search for any usage of protocol_funding rg "protocol_funding" -g '!*.pb.go' -g '!*.proto' # Look for potential client implementations fd -e ts -e js -e py -e java -e go -e rs -e cpp -e cs --exec grep -l "protocol_funding" {} \;Length of output: 306
proto/kyve/query/v1beta1/query.proto (6)
78-80: LGTM! Clear validator metrics field.The
validator_delegatorsfield provides valuable insight into validator popularity and delegation distribution.
82-84: LGTM! Important self-delegation tracking.The
validator_self_delegationfield is crucial for transparency, allowing users to assess validator skin in the game.
86-88: LGTM! Essential unbonding tracking.The
validator_unbondingfield provides important visibility into validator's unbonding activities.
90-92: LGTM! Comprehensive stake tracking.The
validator_total_pool_stakefield effectively replaces and enhances the oldtotal_pool_stakefield with validator-specific context.
94-98: LGTM! Well-structured commission rewards field.The
validator_commission_rewardsfield follows the standard Cosmos SDK Coins configuration pattern.
102-102: Verify field reordering impact.The
poolsfield has been moved to index 8. This reordering could affect existing clients.Run this script to check for potential breaking changes in field usage:
✅ Verification successful
Field reordering appears safe to proceed
The codebase scan shows no direct usage of field numbers in application code or tests. All serialization is handled through generated protobuf code, which properly manages field numbers internally.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for direct usage of field numbers in the codebase rg -g '!*.pb.go' -g '!*.proto' "pools.*4"Length of output: 43
Script:
#!/bin/bash # Search for the pools field in proto files to understand the change rg -g '*.proto' "pools.*=.*[0-9]+" proto/ # Search for any potential serialization code or tests that might use field numbers rg -g '!*.pb.go' -g '!*.proto' -i "pools.*serializ|pools.*marshal|pools.*unmarshal|pools.*encode|pools.*decode" # Search for any test files that might reference the pools field rg -g '*_test.*' "pools"Length of output: 4089
Summary by CodeRabbit
New Features
Improvements
Documentation