add oracle flag & flag checker to msgupdateoracle of opchild #105
Conversation
|
Warning Rate limit exceeded@beer-1 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 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. WalkthroughThe changes introduce new message types and configurations for oracle management within the codebase. Key modifications include the addition of Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MsgServer
participant Keeper
participant BridgeConfig
User->>MsgServer: UpdateOracleConfig(request)
MsgServer->>Keeper: GetBridgeInfo()
Keeper-->>MsgServer: BridgeInfo
MsgServer->>BridgeConfig: Check OracleEnabled
alt OracleEnabled is true
MsgServer->>Keeper: ApplyOracleUpdate()
Keeper-->>MsgServer: Update Success
else OracleEnabled is false
MsgServer-->>User: ErrOracleDisabled
end
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 Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #105 +/- ##
==========================================
- Coverage 47.66% 47.61% -0.06%
==========================================
Files 57 57
Lines 4204 4249 +45
==========================================
+ Hits 2004 2023 +19
- Misses 1782 1804 +22
- Partials 418 422 +4
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
x/ophost/types/tx.pb.gois excluded by!**/*.pb.gox/ophost/types/types.pb.gois excluded by!**/*.pb.go
Files selected for processing (11)
- api/opinit/ophost/v1/tx.pulsar.go (25 hunks)
- api/opinit/ophost/v1/types.pulsar.go (16 hunks)
- proto/opinit/ophost/v1/tx.proto (2 hunks)
- proto/opinit/ophost/v1/types.proto (1 hunks)
- x/opchild/keeper/msg_server.go (1 hunks)
- x/opchild/keeper/msg_server_test.go (2 hunks)
- x/opchild/types/errors.go (1 hunks)
- x/ophost/keeper/msg_server.go (2 hunks)
- x/ophost/keeper/msg_server_test.go (2 hunks)
- x/ophost/types/event.go (1 hunks)
- x/ophost/types/tx.go (1 hunks)
Additional context used
GitHub Check: codecov/patch
x/ophost/types/tx.go
[warning] 390-395: x/ophost/types/tx.go#L390-L395
Added lines #L390 - L395 were not covered by tests
[warning] 399-402: x/ophost/types/tx.go#L399-L402
Added lines #L399 - L402 were not covered by tests
[warning] 404-407: x/ophost/types/tx.go#L404-L407
Added lines #L404 - L407 were not covered by testsx/opchild/keeper/msg_server.go
[warning] 595-596: x/opchild/keeper/msg_server.go#L595-L596
Added lines #L595 - L596 were not covered by testsx/ophost/keeper/msg_server.go
[warning] 503-504: x/ophost/keeper/msg_server.go#L503-L504
Added lines #L503 - L504 were not covered by tests
[warning] 509-510: x/ophost/keeper/msg_server.go#L509-L510
Added lines #L509 - L510 were not covered by tests
[warning] 520-521: x/ophost/keeper/msg_server.go#L520-L521
Added lines #L520 - L521 were not covered by tests
Additional comments not posted (45)
x/ophost/types/event.go (1)
34-34: LGTM!The constant
AttributeKeyOracleEnabledis correctly defined and follows the naming conventions used in the file.The code changes are approved.
x/opchild/types/errors.go (2)
35-35: LGTM!The error variable
ErrOracleDisabledis correctly defined and registered with the error module. The error message is clear and descriptive.The code changes are approved.
38-38: LGTM!The error variable
ErrRedundantTxis correctly updated to reflect the new registration code. The error message remains clear and descriptive.The code changes are approved.
proto/opinit/ophost/v1/types.proto (2)
55-56: LGTM!The field
oracle_enabledis correctly defined and follows the naming conventions used in the file. The addition enhances the configuration capabilities of theBridgeConfigmessage.The code changes are approved.
59-59: LGTM!The field
metadatais correctly updated to reflect the new position. The change ensures that the neworacle_enabledfield is correctly positioned in the updated schema.The code changes are approved.
x/ophost/types/tx.go (1)
385-396: LGTM!The function is correctly initializing the
MsgUpdateOracleConfigstruct.The code changes are approved.
Tools
GitHub Check: codecov/patch
[warning] 390-395: x/ophost/types/tx.go#L390-L395
Added lines #L390 - L395 were not covered by testsproto/opinit/ophost/v1/tx.proto (3)
251-251: LGTM!The updated comment enhances the clarity of the documentation.
The code changes are approved.
266-276: LGTM!The message type is correctly defined and enhances the functionality of the protocol.
The code changes are approved.
278-279: LGTM!The message type is correctly defined.
The code changes are approved.
x/opchild/keeper/msg_server_test.go (1)
628-647: LGTM!The test case correctly validates the behavior of
UpdateOraclewhen the Oracle is disabled.The code changes are approved.
x/ophost/keeper/msg_server_test.go (1)
444-488: LGTM!The test case correctly validates the functionality of updating the Oracle configuration under various conditions.
The code changes are approved.
api/opinit/ophost/v1/types.pulsar.go (9)
569-569: LGTM!The addition of the field descriptor
fd_BridgeConfig_oracle_enabledis appropriate.The code changes are approved.
582-582: LGTM!The initialization of the field descriptor
fd_BridgeConfig_oracle_enabledis appropriate.The code changes are approved.
687-692: LGTM!The update to include the
OracleEnabledfield in theRangemethod is appropriate.The code changes are approved.
726-727: LGTM!The update to include the
OracleEnabledfield in theHasmethod is appropriate.The code changes are approved.
758-759: LGTM!The update to reset the
OracleEnabledfield to false in theClearmethod is appropriate.The code changes are approved.
799-801: LGTM!The update to retrieve the value of the
OracleEnabledfield in theGetmethod is appropriate.The code changes are approved.
839-840: LGTM!The update to allow setting the value of the
OracleEnabledfield in theSetmethod is appropriate.The code changes are approved.
888-889: LGTM!The update to include a panic condition for the
OracleEnabledfield in theMutablemethod is appropriate.The code changes are approved.
Line range hint
1019-1070: LGTM!The update to account for the
OracleEnabledfield during serialization and deserialization processes in theProtoMethodsfunction is appropriate.The code changes are approved.
api/opinit/ophost/v1/tx.pulsar.go (25)
9164-9169: LGTM!The initialization of new message and field descriptors for
MsgUpdateOracleConfigis correctly implemented.The code changes are approved.
Also applies to: 9171-9177
9179-9185: LGTM!The
fastReflection_MsgUpdateOracleConfigtype and its associated methods are correctly implemented.The code changes are approved.
Also applies to: 9187-9197, 9199-9212
9213-9218: LGTM!The additional methods for
fastReflection_MsgUpdateOracleConfigprovide comprehensive reflection capabilities and are correctly implemented.The code changes are approved.
Also applies to: 9220-9236, 9238-9262, 9264-9289, 9291-9311, 9313-9335, 9338-9362, 9364-9387, 9390-9407, 9409-9417, 9420-9436, 9438-9448, 9450-9684
9686-9688: LGTM!The
MsgUpdateOracleConfigResponsetype and its associated methods are correctly implemented, providing necessary reflection capabilities.The code changes are approved.
Also applies to: 9690-9693, 9695-9701, 9703-9713, 9715-9728, 9730-9734, 9736-9752, 9754-9760, 9762-9781, 9783-9797, 9799-9812, 9815-9832, 9835-9852, 9855-9866, 9868-9877, 9879-9884, 9886-9895, 9897-9907, 9909-10040
Line range hint
10066-10070: LGTM!The updates to the
MsgUpdateMetadatatype and its associated methods are correctly implemented.The code changes are approved.
Line range hint
10598-10602: LGTM!The updates to the
MsgUpdateMetadataResponsetype and its associated methods are correctly implemented.The code changes are approved.
Line range hint
11050-11054: LGTM!The updates to the
MsgUpdateParamstype and its associated methods are correctly implemented.The code changes are approved.
Line range hint
11545-11549: LGTM!The updates to the
MsgUpdateParamsResponsetype and its associated methods are correctly implemented.The code changes are approved.
Line range hint
12681-12685: LGTM!The new fields
Authority,BridgeId, andOracleEnabledare correctly added to theMsgUpdateOracleConfigtype.The code changes are approved.
12774-12853: LGTM!The
MsgUpdateOracleConfigtype and its associated methods are correctly implemented, providing necessary reflection capabilities.The code changes are approved.
Line range hint
12874-12878: LGTM!The updates to the
MsgUpdateMetadatatype and its associated methods are correctly implemented.The code changes are approved.
Line range hint
12927-12931: LGTM!The updates to the
MsgUpdateMetadataResponsetype and its associated methods are correctly implemented.The code changes are approved.
Line range hint
12974-12978: LGTM!The updates to the
MsgUpdateParamstype and its associated methods are correctly implemented.The code changes are approved.
Line range hint
13015-13019: LGTM!The updates to the
MsgUpdateParamsResponsetype and its associated methods are correctly implemented.The code changes are approved.
13272-13300: LGTM!The new fields
Authority,BridgeId, andOracleEnabledare correctly added to theMsgUpdateOracleConfigtype.The code changes are approved.
13301-13327: LGTM!The updates to the
MsgUpdateOracleConfigResponsetype and its associated methods are correctly implemented.The code changes are approved.
Line range hint
13426-13433: LGTM!The updates to the
MsgUpdateMetadatatype and its associated methods are correctly implemented.The code changes are approved.
13446-13455: LGTM!The updates to the
MsgUpdateOracleConfigtype and its associated methods are correctly implemented.The code changes are approved.
Line range hint
13472-13484: LGTM!The updates to the
MsgUpdateMetadatatype and its associated methods are correctly implemented.The code changes are approved.
13483-13484: LGTM!The updates to the
MsgUpdateParamstype and its associated methods are correctly implemented.The code changes are approved.
Line range hint
13716-13721: LGTM!The updates to the
MsgUpdateOracleConfigtype and its associated methods are correctly implemented.The code changes are approved.
Line range hint
13728-13733: LGTM!The updates to the
MsgUpdateOracleConfigResponsetype and its associated methods are correctly implemented.The code changes are approved.
Line range hint
13740-13745: LGTM!The updates to the
MsgUpdateMetadatatype and its associated methods are correctly implemented.The code changes are approved.
13752-13761: LGTM!The updates to the
MsgUpdateMetadataResponsetype and its associated methods are correctly implemented.The code changes are approved.
13764-13773: LGTM!The updates to the
MsgUpdateParamstype and its associated methods are correctly implemented.The code changes are approved.
| // Validate performs basic MsgUpdateOracleConfig message validation. | ||
| func (msg MsgUpdateOracleConfig) Validate(accAddressCodec address.Codec) error { | ||
| if _, err := accAddressCodec.StringToBytes(msg.Authority); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if msg.BridgeId == 0 { | ||
| return ErrInvalidBridgeId | ||
| } | ||
| return nil |
There was a problem hiding this comment.
LGTM! But add tests for validation.
The validation logic is correct. However, the added lines are not covered by tests.
The code changes are approved.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 399-402: x/ophost/types/tx.go#L399-L402
Added lines #L399 - L402 were not covered by tests
[warning] 404-407: x/ophost/types/tx.go#L404-L407
Added lines #L404 - L407 were not covered by tests
| // config check | ||
| info, err := ms.Keeper.BridgeInfo.Get(ctx) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if !info.BridgeConfig.OracleEnabled { | ||
| return nil, types.ErrOracleDisabled | ||
| } |
There was a problem hiding this comment.
LGTM! But add tests for the configuration check.
The added configuration check enhances the robustness of the function. However, the added lines are not covered by tests.
The code changes are approved.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 595-596: x/opchild/keeper/msg_server.go#L595-L596
Added lines #L595 - L596 were not covered by tests
| func (ms MsgServer) UpdateOracleConfig(ctx context.Context, req *types.MsgUpdateOracleConfig) (*types.MsgUpdateOracleConfigResponse, error) { | ||
| if err := req.Validate(ms.authKeeper.AddressCodec()); err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
Ensure validation errors are covered by tests.
The validation check for the request is not covered by tests.
Add a test case to cover validation errors for the request.
Tools
GitHub Check: codecov/patch
[warning] 503-504: x/ophost/keeper/msg_server.go#L503-L504
Added lines #L503 - L504 were not covered by tests
| bridgeId := req.BridgeId | ||
| config, err := ms.GetBridgeConfig(ctx, bridgeId) | ||
| if err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
Ensure error handling for bridge configuration retrieval is covered by tests.
The error handling for retrieving the bridge configuration is not covered by tests.
Add a test case to cover errors when retrieving the bridge configuration.
Tools
GitHub Check: codecov/patch
[warning] 509-510: x/ophost/keeper/msg_server.go#L509-L510
Added lines #L509 - L510 were not covered by tests
| // gov or current proposer can update metadata. | ||
| if ms.authority != req.Authority && config.Proposer != req.Authority { | ||
| return nil, govtypes.ErrInvalidSigner.Wrapf("invalid authority; expected %s or %s, got %s", ms.authority, config.Proposer, req.Authority) | ||
| } |
There was a problem hiding this comment.
Ensure permission checks are covered by tests.
The permission check for updating the Oracle configuration is correctly implemented but not covered by tests.
Add a test case to cover permission checks for updating the Oracle configuration.
| config.OracleEnabled = req.OracleEnabled | ||
|
|
||
| if err := ms.SetBridgeConfig(ctx, bridgeId, config); err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
Ensure setting the bridge configuration is covered by tests.
The process of setting the bridge configuration is not covered by tests.
Add a test case to cover setting the bridge configuration.
Tools
GitHub Check: codecov/patch
[warning] 520-521: x/ophost/keeper/msg_server.go#L520-L521
Added lines #L520 - L521 were not covered by tests
| sdk.UnwrapSDKContext(ctx).EventManager().EmitEvent(sdk.NewEvent( | ||
| types.EventTypeUpdateBatchInfo, | ||
| sdk.NewAttribute(types.AttributeKeyBridgeId, strconv.FormatUint(bridgeId, 10)), | ||
| sdk.NewAttribute(types.AttributeKeyOracleEnabled, strconv.FormatBool(config.OracleEnabled)), | ||
| )) |
There was a problem hiding this comment.
Ensure event emission is covered by tests.
The event emission for updating the Oracle configuration is correctly implemented but not covered by tests.
Add a test case to cover event emission for updating the Oracle configuration.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation