feat: add Sui chain info #3489
feat: add Sui chain info #3489
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 pull request updates the OpenAPI specification by adding new options for consensus, network, and virtual machine types ( Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pkg/chains/chains.go (1)
332-341: Consider using sequential chain IDs for better maintainabilityThe current chain IDs for Sui networks (101, 103, 104) use non-sequential numbering. Consider using a more systematic approach:
// SuiTestnet - ChainId: 103, + ChainId: 1002, // SuiLocalnet - ChainId: 104, + ChainId: 1003,Also applies to: 458-467
proto/zetachain/zetacore/pkg/chains/chains.proto (1)
1-154: Overall Protobuf File Enhancements: Maintain ConsistencyThe updates across the enums in this file effectively integrate new SUI chain information into the codebase. Ensure that all documentation, client specifications, and test cases are updated accordingly. This holistic improvement aids in maintaining consistency and future compatibility.
🧰 Tools
🪛 Buf (1.47.2)
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pkg/chains/chains.pb.gois excluded by!**/*.pb.go,!**/*.pb.gotypescript/zetachain/zetacore/pkg/chains/chains_pb.d.tsis excluded by!**/*_pb.d.ts
📒 Files selected for processing (5)
docs/openapi/openapi.swagger.yaml(3 hunks)pkg/chains/chain_test.go(3 hunks)pkg/chains/chains.go(4 hunks)pkg/chains/chains_test.go(7 hunks)proto/zetachain/zetacore/pkg/chains/chains.proto(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.proto`: Review the Protobuf definitions, point out iss...
**/*.proto: Review the Protobuf definitions, point out issues relative to compatibility, and expressiveness.
proto/zetachain/zetacore/pkg/chains/chains.proto
`**/*.go`: Review the Go code, point out issues relative to ...
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
pkg/chains/chain_test.gopkg/chains/chains_test.gopkg/chains/chains.go
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: start-e2e-test / e2e
- GitHub Check: build-and-test
🔇 Additional comments (8)
pkg/chains/chain_test.go (1)
76-76: LGTM!The test cases have been correctly updated to validate against the new Sui-related enum values.
Also applies to: 104-104, 118-118
pkg/chains/chains_test.go (1)
43-43: LGTM!The test cases have been comprehensively updated to include the new Sui chains across all relevant test functions.
Also applies to: 66-66, 78-78, 152-156, 202-204, 248-250
docs/openapi/openapi.swagger.yaml (3)
58823-58829: New Consensus Option Added
The addition ofsui_consensusin the consensus mechanisms list aligns well with the corresponding proto definitions. Please ensure that any backend or client components consuming this OpenAPI specification are updated to recognize and handle the new consensus mechanism correctly.
58847-58853: New Network Option Introduced
The inclusion ofsuias a valid option in theNetworkdefinition is consistent with the intended updates from the PR. Verify that this new network mapping is properly integrated across all services and that documentation reflects this addition uniformly.
58883-58889: New Virtual Machine Type Added
The new entrymvm_suiunder theVmdefinition is correctly added to support the updated SUI chain configurations. Confirm that all corresponding service logic and proto definitions are synchronized with this change to maintain consistency throughout the system.proto/zetachain/zetacore/pkg/chains/chains.proto (3)
70-70: New Enum Value in Network: Ensure Seamless Backwards CompatibilityThe new enum value
sui = 12;within theNetworkenum extends the supported networks by appending to the previous value (worldchain = 11). Please verify that all systems consuming this enum correctly handle the new SUI network type and that there is no conflict with existing values.
91-91: New Enum Value in Vm: Validate SUI Virtual Machine IntegrationThe addition of
mvm_sui = 4;to theVmenum introduces support for the SUI virtual machine. Ensure that any components or services relying on VM type information are updated to account for this new value and that corresponding tests validate its proper usage.
107-107: New Enum Value in Consensus: Confirm Consensus Mechanism SupportThe inclusion of
sui_consensus = 8;in theConsensusenum extends the consensus algorithm options to support SUI. Confirm that the OpenAPI specifications, business logic, and tests in the chains package reflect this new consensus type to avoid integration issues.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3489 +/- ##
========================================
Coverage 65.41% 65.42%
========================================
Files 442 442
Lines 30522 30525 +3
========================================
+ Hits 19967 19970 +3
Misses 9700 9700
Partials 855 855
|
Description
Closes #3472
Don't set the real mainnet chain id for now, will not be necessary until mainnet integration
Summary by CodeRabbit
New Features
Documentation