-
Notifications
You must be signed in to change notification settings - Fork 116
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
feat(fungible
): add Gateway address in protocol contract list
#2578
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughWalkthroughThe changes introduce significant enhancements to the ZetaChain protocol, focusing on the management of gateway contracts. A new command and message type allow users to update the gateway contract address through the CLI, while updates to various protocols and specifications ensure these changes are well-integrated. This functionality supports dynamic contract management and increases the operational flexibility of the ZetaChain ecosystem. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant MsgServer
participant Keeper
participant EventSystem
User->>CLI: CmdUpdateGatewayContract(newAddress)
CLI->>MsgServer: UpdateGatewayContract(msg)
MsgServer->>Keeper: Update gateway contract state
Keeper->>EventSystem: Emit EventGatewayContractUpdated
EventSystem->>MsgServer: Confirmation of event
MsgServer-->>CLI: Success response
CLI-->>User: Update successful
Assessment against linked issues
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 @@
## develop #2578 +/- ##
===========================================
+ Coverage 71.57% 71.62% +0.05%
===========================================
Files 327 329 +2
Lines 17798 17849 +51
===========================================
+ Hits 12739 12785 +46
- Misses 4490 4493 +3
- Partials 569 571 +2
|
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
Outside diff range, codebase verification and nitpick comments (8)
x/fungible/client/cli/tx_update_gateway_contract.go (1)
12-34
: Remove the extra space in the command usage string.The usage string has an extra space at the end.
- Use: "update-gateway-contract [contract-address] ", + Use: "update-gateway-contract [contract-address]",docs/cli/zetacored/zetacored_tx_fungible.md (1)
35-35
: Fix the hard tabs issue.Replace hard tabs with spaces to adhere to Markdownlint rules.
- * [zetacored tx fungible update-gateway-contract](zetacored_tx_fungible_update-gateway-contract.md) - Broadcast message UpdateGatewayContract to update the gateway contract address + * [zetacored tx fungible update-gateway-contract](zetacored_tx_fungible_update-gateway-contract.md) - Broadcast message UpdateGatewayContract to update the gateway contract addressTools
Markdownlint
35-35: Column: 100
Hard tabs(MD010, no-hard-tabs)
docs/spec/fungible/messages.md (1)
143-152
: Clear and accurate documentation forMsgUpdateGatewayContract
.The documentation for the new message type
MsgUpdateGatewayContract
is clear and accurate, describing its purpose and fields. However, there are hard tab issues that need to be fixed.message MsgUpdateGatewayContract { - string creator = 1; - string new_gateway_contract_address = 2; + string creator = 1; + string new_gateway_contract_address = 2; }Tools
Markdownlint
149-149: Column: 1
Hard tabs(MD010, no-hard-tabs)
150-150: Column: 1
Hard tabs(MD010, no-hard-tabs)
docs/cli/zetacored/zetacored_tx_fungible_update-gateway-contract.md (5)
1-1
: Fix the heading level.The heading level should start from
##
instead of#
to maintain consistency with the documentation hierarchy.-# tx fungible update-gateway-contract +## tx fungible update-gateway-contract
5-5
: Specify the language for the fenced code block.Fenced code blocks should have a language specified for proper syntax highlighting.
-``` +```shellTools
Markdownlint
5-5: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
11-11
: Specify the language for the fenced code block.Fenced code blocks should have a language specified for proper syntax highlighting.
-``` +```shellTools
Markdownlint
11-11: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
42-42
: Specify the language for the fenced code block.Fenced code blocks should have a language specified for proper syntax highlighting.
-``` +```shellTools
Markdownlint
42-42: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
52-52
: Avoid using hard tabs.Replace hard tabs with spaces for consistency and to avoid formatting issues.
-* [zetacored tx fungible](zetacored_tx_fungible.md) - fungible transactions subcommands +* [zetacored tx fungible](zetacored_tx_fungible.md) - fungible transactions subcommandsTools
Markdownlint
52-52: Column: 52
Hard tabs(MD010, no-hard-tabs)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (6)
typescript/zetachain/zetacore/fungible/events_pb.d.ts
is excluded by!**/*_pb.d.ts
typescript/zetachain/zetacore/fungible/system_contract_pb.d.ts
is excluded by!**/*_pb.d.ts
typescript/zetachain/zetacore/fungible/tx_pb.d.ts
is excluded by!**/*_pb.d.ts
x/fungible/types/events.pb.go
is excluded by!**/*.pb.go
,!**/*.pb.go
x/fungible/types/system_contract.pb.go
is excluded by!**/*.pb.go
,!**/*.pb.go
x/fungible/types/tx.pb.go
is excluded by!**/*.pb.go
,!**/*.pb.go
Files selected for processing (16)
- docs/cli/zetacored/zetacored_tx_fungible.md (1 hunks)
- docs/cli/zetacored/zetacored_tx_fungible_update-gateway-contract.md (1 hunks)
- docs/openapi/openapi.swagger.yaml (2 hunks)
- docs/spec/fungible/messages.md (1 hunks)
- proto/zetachain/zetacore/fungible/events.proto (1 hunks)
- proto/zetachain/zetacore/fungible/system_contract.proto (1 hunks)
- proto/zetachain/zetacore/fungible/tx.proto (2 hunks)
- x/authority/types/authorization_list.go (1 hunks)
- x/authority/types/authorization_list_test.go (1 hunks)
- x/fungible/client/cli/tx.go (1 hunks)
- x/fungible/client/cli/tx_update_gateway_contract.go (1 hunks)
- x/fungible/keeper/msg_server_update_gateway_contract.go (1 hunks)
- x/fungible/keeper/msg_server_update_gateway_contract_test.go (1 hunks)
- x/fungible/types/codec.go (2 hunks)
- x/fungible/types/message_update_gateway_contract.go (1 hunks)
- x/fungible/types/message_update_gateway_contract_test.go (1 hunks)
Additional context used
Path-based instructions (12)
proto/zetachain/zetacore/fungible/system_contract.proto (1)
Pattern
**/*.proto
: Review the Protobuf definitions, point out issues relative to compatibility, and expressiveness.x/fungible/client/cli/tx.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/fungible/client/cli/tx_update_gateway_contract.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/fungible/types/codec.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.proto/zetachain/zetacore/fungible/events.proto (1)
Pattern
**/*.proto
: Review the Protobuf definitions, point out issues relative to compatibility, and expressiveness.x/fungible/types/message_update_gateway_contract.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/fungible/keeper/msg_server_update_gateway_contract.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/fungible/types/message_update_gateway_contract_test.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.proto/zetachain/zetacore/fungible/tx.proto (1)
Pattern
**/*.proto
: Review the Protobuf definitions, point out issues relative to compatibility, and expressiveness.x/fungible/keeper/msg_server_update_gateway_contract_test.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/authority/types/authorization_list.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/authority/types/authorization_list_test.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
Markdownlint
docs/cli/zetacored/zetacored_tx_fungible.md
35-35: Column: 100
Hard tabs(MD010, no-hard-tabs)
docs/spec/fungible/messages.md
149-149: Column: 1
Hard tabs(MD010, no-hard-tabs)
150-150: Column: 1
Hard tabs(MD010, no-hard-tabs)
docs/cli/zetacored/zetacored_tx_fungible_update-gateway-contract.md
9-9: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
52-52: Column: 52
Hard tabs(MD010, no-hard-tabs)
5-5: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
11-11: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
42-42: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
GitHub Check: codecov/patch
x/fungible/keeper/msg_server_update_gateway_contract.go
[warning] 21-21: x/fungible/keeper/msg_server_update_gateway_contract.go#L21
Added line #L21 was not covered by tests
[warning] 48-49: x/fungible/keeper/msg_server_update_gateway_contract.go#L48-L49
Added lines #L48 - L49 were not covered by tests
Additional comments not posted (30)
docs/openapi/openapi.swagger.yaml (2)
Line range hint
1-1
:
Addition offungibleMsgUpdateGatewayContractResponse
looks good.The new response type is well-defined and enhances the API's ability to handle gateway contract updates.
Line range hint
1-1
:
Addition ofgateway
property looks good.The new property enhances the API's ability to convey information related to the gateway.
proto/zetachain/zetacore/fungible/system_contract.proto (1)
11-11
: Addition ofgateway
field looks good.The new field enhances the
SystemContract
message by allowing it to store information related to the gateway.x/fungible/client/cli/tx.go (1)
32-32
: Addition ofCmdUpdateGatewayContract
command looks good.The new command enhances the CLI by allowing users to update the gateway contract.
x/fungible/client/cli/tx_update_gateway_contract.go (3)
1-10
: Imports and package declaration are appropriate.The package and imports are correctly defined and necessary for the functionality.
12-34
: Function implementation is correct.The function correctly creates the transaction context, constructs the message, validates it, and broadcasts it.
32-34
: Transaction flags are correctly added.Adding transaction flags to the command is necessary for its correct functionality.
x/fungible/types/codec.go (2)
20-20
: New message type is correctly registered.The new message type
MsgUpdateGatewayContract
is correctly registered with the codec.
34-34
: New message type is correctly registered with the interface registry.The new message type
MsgUpdateGatewayContract
is correctly registered with the interface registry.proto/zetachain/zetacore/fungible/events.proto (1)
71-76
: New message definition is appropriate.The new message
EventGatewayContractUpdated
is correctly defined to capture the necessary information for gateway contract updates.x/fungible/types/message_update_gateway_contract.go (6)
14-19
: LGTM!The method correctly initializes the
MsgUpdateGatewayContract
message.
21-23
: LGTM!The method correctly returns the router key.
25-27
: LGTM!The method correctly returns the message type.
29-35
: LGTM!The method correctly retrieves the signers and handles the error appropriately.
37-40
: LGTM!The method correctly marshals the message to JSON and sorts it.
42-56
: LGTM!The method correctly validates the creator address and the gateway contract address. Proper error handling is in place.
x/fungible/keeper/msg_server_update_gateway_contract.go (1)
13-53
: LGTM!The method correctly handles authorization, updates the state, and emits events.
Tools
GitHub Check: codecov/patch
[warning] 21-21: x/fungible/keeper/msg_server_update_gateway_contract.go#L21
Added line #L21 was not covered by tests
[warning] 48-49: x/fungible/keeper/msg_server_update_gateway_contract.go#L48-L49
Added lines #L48 - L49 were not covered by testsx/fungible/types/message_update_gateway_contract_test.go (5)
14-44
: Comprehensive test coverage forValidateBasic
.The test cases effectively cover invalid and valid addresses for the
ValidateBasic
method. Consider adding edge cases, such as empty strings or excessively long addresses, to ensure robustness.
47-81
: Effective test coverage forGetSigners
.The test cases appropriately handle scenarios with valid and invalid signers, ensuring the method's robustness.
84-89
: Correct verification of message type.The test case accurately verifies the
Type
method's return value.
91-96
: Correct verification of router key.The test case accurately verifies the
Route
method's return value.
98-105
: EnsuringGetSignBytes
does not panic.The test case effectively ensures that the
GetSignBytes
method does not cause a panic.proto/zetachain/zetacore/fungible/tx.proto (3)
27-28
: Correct addition ofUpdateGatewayContract
RPC method.The new RPC method
UpdateGatewayContract
is correctly defined and integrated, allowing for updates to the gateway contract address.
116-119
: Correct definition ofMsgUpdateGatewayContract
message.The new message type
MsgUpdateGatewayContract
is correctly defined with fields for the creator and the new gateway contract address.
121-121
: Correct definition ofMsgUpdateGatewayContractResponse
message.The new message type
MsgUpdateGatewayContractResponse
is correctly defined as an empty message.x/fungible/keeper/msg_server_update_gateway_contract_test.go (3)
14-54
: Good test case structure.The test case
can update the gateway contract address stored in the module
is well-structured with clear arrangement, act, and assert phases.
56-93
: Comprehensive test coverage.The test case
can update and overwrite the gateway contract if system contract state variable not found
covers an important edge case where the system contract state variable is not found.
95-116
: Correctly handles unauthorized access.The test case
should prevent update the gateway contract if not admin
correctly handles the scenario where an unauthorized user attempts to update the gateway contract.x/authority/types/authorization_list.go (1)
31-31
: New message type added correctly.The new message type
"/zetachain.zetacore.fungible.MsgUpdateGatewayContract"
is correctly added to theAdminPolicyMessages
list.x/authority/types/authorization_list_test.go (1)
423-423
: Confirm the correct integration of the new message type.The addition of
sdk.MsgTypeURL(&fungibletypes.MsgUpdateGatewayContract{})
to theAdminPolicyMessageList
appears correct. Ensure that this message type is properly defined and integrated within the system.Verification successful
The new message type
MsgUpdateGatewayContract
is correctly integrated.The addition of
sdk.MsgTypeURL(&fungibletypes.MsgUpdateGatewayContract{})
to theAdminPolicyMessageList
is appropriate and well-supported by the existing codebase. The message type is properly defined and integrated within the system.
- Definition found in
x/fungible/types/tx.pb.go
.- Usage confirmed across multiple files, including
x/fungible/keeper/msg_server_update_gateway_contract.go
and various test files.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the definition and integration of MsgUpdateGatewayContract. # Test: Search for the definition of MsgUpdateGatewayContract. Expect: Definition should exist. rg --type go 'type MsgUpdateGatewayContract' -A 10 # Test: Search for the usage of MsgUpdateGatewayContract. Expect: Proper integration within the system. rg --type go 'MsgUpdateGatewayContract'Length of output: 11643
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.
Left some non-blocking comments
Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com>
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.
We should add gateway contract address to this test
https://github.com/zeta-chain/zeta-node/blob/d1c9c32398f8371388bcdaaaa367cf0ef1b399a9/x/fungible/keeper/grpc_query_system_contract_test.go#L28-L40
Description
Closes: #2577
Add the gateway contract address in the SystemContract structure and add a message to update it.
Give admin permission for the message (shall be called almost never)
Unit tested the message
For the E2E tests, we'll have to make more progress in the new architecture environmment before testing all components
Summary by CodeRabbit
New Features
zetacored
CLI.MsgUpdateGatewayContract
to facilitate gateway contract updates.Bug Fixes
Tests
MsgUpdateGatewayContract
functionality and the associated CLI command.Documentation