update price-feeder to v0.1.12 #260
Conversation
WalkthroughThe pull request updates the Changes
Possibly related PRs
Suggested reviewers
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: 0
🧹 Outside diff range and nitpick comments (8)
proto/exocore/dogfood/v1/params.proto (1)
Line range hint
30-34: LGTM! Consider enhancing the field documentation.The type annotations for
min_self_delegationare correct and follow Cosmos SDK best practices. Usingcosmos.Intwith non-nullable constraint is appropriate for monetary values.Consider expanding the field documentation to clarify the expected format and units:
- // MinSelfDelegation is the minimum self delegation in USD required to be a validator. + // MinSelfDelegation is the minimum self delegation in USD (as a Cosmos SDK Int) required to be a validator. + // The value is stored as an integer with precision of 6 decimal places (e.g., 1000000 = 1 USD).proto/exocore/operator/v1/tx.proto (1)
Line range hint
191-201: LGTM! Well-structured new message typeThe
SlashFromAssetsPoolmessage is well-defined with appropriate field types and annotations. Consider adding more detailed documentation about when and how this message is used in the slashing process.Add more detailed documentation:
// SlashFromAssetsPool records the slash detail from the operator assets pool +// This message is used when executing a slash event to track the amount of assets +// that are slashed from an operator's assets pool. It works in conjunction with +// SlashExecutionInfo to provide a complete record of the slashing event. message SlashFromAssetsPool {proto/exocore/operator/v1/query.proto (1)
Line range hint
369-408: LGTM! Well-structured validator messagesThe new validator-related messages are well-defined with appropriate pagination support and field annotations. Consider adding examples in the documentation to illustrate typical usage scenarios.
Add usage examples in documentation:
// QueryValidatorsRequest is request type for Query/Validators RPC method. +// Example usage: +// { +// "chain": "exocore-1", +// "pagination": { +// "key": "base64_encoded_key", +// "offset": 0, +// "limit": 100, +// "count_total": true +// } +// } message QueryValidatorsRequest {proto/exocore/avs/v1/genesis.proto (1)
55-62: Consider adding validation constraints for chain_id fieldThe ChainIDInfo message structure is well-defined, but given its importance in configuration, consider adding field constraints or validation rules for the chain_id field.
// chain_id is an optional parameter to specify the chain_id of the AVS, if any - string chain_id = 2; + string chain_id = 2 [(gogoproto.moretags) = "validate:\"required,chainID\""];proto/exocore/avs/v1/query.proto (1)
Line range hint
74-97: Consider standardizing HTTP endpoint pathsThe Query service is well-defined, but the HTTP endpoint paths show some inconsistency:
/exocore/avs/QueryAVSInfo/exocore/avstask/v1/GetAVSTaskInfoReq/exocore/avs/QueryAVSAddrByChainIDConsider standardizing the paths to follow a consistent pattern.
rpc QueryAVSTaskInfo(QueryAVSTaskInfoReq) returns (TaskInfo) { - option (google.api.http).get = "/exocore/avstask/v1/GetAVSTaskInfoReq"; + option (google.api.http).get = "/exocore/avs/v1/task_info"; }proto/exocore/avs/v1/tx.proto (2)
46-54: Consider adding validation for reward percentageThe avs_reward field should have validation to ensure it's a valid percentage value.
string avs_reward = 16 [ (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec", (gogoproto.nullable) = false + (gogoproto.moretags) = "validate:\"gte=0,lte=1\"" ];
Line range hint
127-148: Consider adding validation for power valuesThe OperatorActivePowerList and OperatorActivePowerInfo messages handle power values, which are crucial for the system. Consider adding validation to ensure these values are non-negative.
string active_power = 2 [ (cosmos_proto.scalar) = "cosmos.Dec", (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec", (gogoproto.nullable) = false, (gogoproto.customname) = "SelfActivePower" + (gogoproto.moretags) = "validate:\"gte=0\"" ];proto/exocore/feedistribution/v1/genesis.proto (1)
Line range hint
47-78: Consider adding address format validationThe address fields (
val_addrandstaker_addr) in the new message types could benefit from additional validation constraints. Consider adding pattern validation or length constraints using proto3 field options.Example enhancement:
message ValidatorCurrentRewardsList { - string val_addr = 1; + string val_addr = 1 [(gogoproto.moretags) = "validate:\"address\""]; ValidatorCurrentRewards current_rewards = 2; }This pattern should be applied to all address fields in the new message types.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (33)
proto/evmos/erc20/v1/erc20.proto(1 hunks)proto/evmos/incentives/v1/incentives.proto(4 hunks)proto/exocore/appchain/common/v1/common.proto(4 hunks)proto/exocore/appchain/coordinator/v1/coordinator.proto(1 hunks)proto/exocore/appchain/coordinator/v1/params.proto(1 hunks)proto/exocore/appchain/coordinator/v1/query.proto(2 hunks)proto/exocore/appchain/coordinator/v1/tx.proto(2 hunks)proto/exocore/appchain/subscriber/v1/query.proto(2 hunks)proto/exocore/appchain/subscriber/v1/tx.proto(1 hunks)proto/exocore/assets/v1/genesis.proto(2 hunks)proto/exocore/assets/v1/tx.proto(9 hunks)proto/exocore/avs/v1/genesis.proto(2 hunks)proto/exocore/avs/v1/query.proto(2 hunks)proto/exocore/avs/v1/tx.proto(8 hunks)proto/exocore/delegation/v1/genesis.proto(3 hunks)proto/exocore/delegation/v1/query.proto(1 hunks)proto/exocore/dogfood/v1/params.proto(1 hunks)proto/exocore/dogfood/v1/query.proto(2 hunks)proto/exocore/dogfood/v1/tx.proto(1 hunks)proto/exocore/epochs/v1/epochs.proto(1 hunks)proto/exocore/feedistribution/v1/distribution.proto(1 hunks)proto/exocore/feedistribution/v1/genesis.proto(4 hunks)proto/exocore/feedistribution/v1/tx.proto(1 hunks)proto/exocore/operator/v1/genesis.proto(4 hunks)proto/exocore/operator/v1/query.proto(7 hunks)proto/exocore/operator/v1/tx.proto(6 hunks)proto/exocore/operator/v1/validator.proto(4 hunks)proto/exocore/oracle/v1/genesis.proto(1 hunks)proto/exocore/oracle/v1/native_token.proto(1 hunks)proto/exocore/oracle/v1/params.proto(2 hunks)proto/exocore/oracle/v1/query.proto(4 hunks)x/avs/types/genesis_test.go(6 hunks)x/oracle/keeper/msg_server_update_params_test.go(1 hunks)
✅ Files skipped from review due to trivial changes (24)
- proto/exocore/feedistribution/v1/tx.proto
- proto/exocore/epochs/v1/epochs.proto
- proto/exocore/feedistribution/v1/distribution.proto
- proto/exocore/oracle/v1/genesis.proto
- proto/exocore/delegation/v1/genesis.proto
- proto/exocore/oracle/v1/params.proto
- x/avs/types/genesis_test.go
- proto/exocore/appchain/subscriber/v1/tx.proto
- proto/exocore/dogfood/v1/tx.proto
- proto/exocore/appchain/coordinator/v1/coordinator.proto
- proto/exocore/oracle/v1/native_token.proto
- proto/exocore/delegation/v1/query.proto
- proto/evmos/erc20/v1/erc20.proto
- proto/exocore/appchain/subscriber/v1/query.proto
- proto/exocore/assets/v1/genesis.proto
- proto/exocore/appchain/coordinator/v1/tx.proto
- proto/exocore/dogfood/v1/query.proto
- proto/exocore/appchain/common/v1/common.proto
- proto/exocore/operator/v1/validator.proto
- proto/exocore/appchain/coordinator/v1/query.proto
- proto/evmos/incentives/v1/incentives.proto
- proto/exocore/assets/v1/tx.proto
- proto/exocore/operator/v1/genesis.proto
- proto/exocore/oracle/v1/query.proto
🔇 Additional comments (14)
proto/exocore/operator/v1/tx.proto (2)
53-55: LGTM! Field formatting improvements
The formatting changes enhance readability while maintaining consistency with proto3 style guidelines.
Also applies to: 82-82, 132-135
327-327: LGTM! Service method formatting improvements
The formatting changes to the RPC method definitions enhance readability while maintaining proto3 service definition standards.
Also applies to: 333-333, 338-338, 342-343
proto/exocore/operator/v1/query.proto (2)
19-19: LGTM! Field formatting improvements
The formatting changes enhance readability while maintaining proto3 style guidelines.
Also applies to: 53-53, 59-59, 246-246
281-337: LGTM! RPC method formatting improvements
The formatting changes to the RPC method definitions enhance readability while maintaining proto3 service definition standards.
proto/exocore/avs/v1/genesis.proto (1)
Line range hint 46-54: LGTM: ChallengeInfo message structure is well-defined
The message structure properly captures challenge-related information with clear field definitions and appropriate types.
proto/exocore/avs/v1/tx.proto (2)
Line range hint 213-224: LGTM: Phase enum is well-defined
The Phase enum properly handles the two-phase submission process with clear documentation and appropriate custom naming.
261-279: LGTM: Msg service is well-structured
The Msg service properly defines all necessary RPCs with appropriate HTTP annotations and consistent naming.
proto/exocore/feedistribution/v1/genesis.proto (2)
Line range hint 4-6: LGTM: Proper import declarations
The imports are correctly specified and include all necessary proto definitions for the new reward tracking functionality.
Line range hint 1-78: Verify scope alignment with PR objectives
The PR description focuses on updating the price-feeder to v0.1.12 and handling slotsPerEpoch, but these proto changes introduce new reward tracking functionality. Could you please clarify if these changes are intentionally included in this PR's scope?
Let's check if these changes are related to the price-feeder update:
x/oracle/keeper/msg_server_update_params_test.go (2)
19-19: LGTM: Variable scope change is safe.
The change from global to local variable declaration using := is safe as the variable is only used within this test file's scope.
Line range hint 1-200: Verify test coverage for price-feeder v0.1.12 changes.
While the existing tests are comprehensive for parameter updates, we should verify if additional test cases are needed to cover the new price-feeder v0.1.12 functionality, specifically around the handling of slotsPerEpoch when the endpoint doesn't support querying config spec.
proto/exocore/appchain/coordinator/v1/params.proto (3)
23-27: LGTM! Clean formatting of ibc_timeout_period options
The multi-line formatting of protobuf options improves readability while maintaining the same functionality.
30-30: LGTM! Consistent formatting of timeout period fields
The formatting changes for both init_timeout_period and vsc_timeout_period maintain consistency with the codebase's style while preserving the same functionality.
Also applies to: 33-36
23-36: Verify if proto regeneration is needed
Since there are changes to the proto definitions, even though they're formatting-only, we should verify if the Go code needs to be regenerated.
Description
update price-feeder dependency to v0.1.12 which set a fallback
slotsPerEpochvalue to 32ETH instead of 0 when the endpoint does not support query the config speceth/v1/config/specCloses #XXX
Summary by CodeRabbit
genesisandqueryproto files to expand functionality related to rewards and AVS queries.