feat(e2e): setup sui #3485
feat(e2e): setup sui #3485
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 📝 WalkthroughWalkthroughThis pull request integrates SUI blockchain testing into the end-to-end test framework. It updates the GitHub workflow by adding a new output variable ( Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant MF as Makefile (start-sui-test)
participant Runner as E2E Runner
participant Setup as SetupSui
participant SuiNet as Sui Network
GH->>GH: Evaluate PR labels (SUI_TESTS)
GH->>MF: Trigger start-sui-test target
MF->>Runner: Execute SUI test command
Runner->>Setup: Call SetupSui(faucetURL)
Setup->>Setup: Retrieve deployer signer & prepare transaction
Setup->>SuiNet: Request SUI tokens and publish gateway module
SuiNet-->>Setup: Return transaction response
Setup-->>Runner: Complete SUI gateway setup
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
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3485 +/- ##
===========================================
+ Coverage 65.29% 65.42% +0.13%
===========================================
Files 441 442 +1
Lines 30425 30525 +100
===========================================
+ Hits 19865 19970 +105
+ Misses 9704 9700 -4
+ Partials 856 855 -1 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
e2e/utils/sui/signer.go (1)
61-79: Simplify public key retrieval using built-in serialization methodsThe
GetPublicKeymethod manually constructs the compressed public key. Utilize theSerializeCompressedmethod provided by thesecp256k1library for cleaner and more reliable code.Refactor the method as follows:
func (s *SignerSecp256k1) GetPublicKey() []byte { - pub := s.privkey.Public().(*ecdsa.PublicKey) - - // Create compressed public key format: - // 0x02/0x03 | x-coordinate (32 bytes) - x := pub.X.Bytes() - - // Ensure x coordinate is 32 bytes with leading zeros if needed - paddedX := make([]byte, 32) - copy(paddedX[32-len(x):], x) - - // Prefix with 0x02 for even Y or 0x03 for odd Y - prefix := byte(0x02) - if pub.Y.Bit(0) == 1 { - prefix = 0x03 - } - - return append([]byte{prefix}, paddedX...) + secpPubKey := secp256k1.PublicKey{ + Curve: secp256k1.S256(), + X: s.privkey.PublicKey.X, + Y: s.privkey.PublicKey.Y, + } + return secpPubKey.SerializeCompressed() }e2e/utils/sui/signer_test.go (1)
51-54: Consider extracting test vector to a constant.The base64-encoded public key string should be declared as a constant with the other test vectors at the top of the file.
e2e/config/config.go (1)
405-413: Improve method documentation and error handling.The
SuiSignermethod could benefit from better documentation and error handling:Apply this diff to enhance the implementation:
-// SuiAddress derives the blake2b hash from the private key +// SuiSigner creates a new Sui signer from the account's private key. +// Returns a SignerSecp256k1 instance that can be used for Sui blockchain operations. func (a Account) SuiSigner() (*sui_utils.SignerSecp256k1, error) { privateKeyBytes, err := hex.DecodeString(a.RawPrivateKey.String()) if err != nil { - return nil, fmt.Errorf("decode private key: %w", err) + return nil, fmt.Errorf("failed to decode Sui private key from hex: %w", err) } signer := sui_utils.NewSignerSecp256k1FromSecretKey(privateKeyBytes) return signer, nil }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (12)
.github/workflows/e2e.yml(6 hunks)Makefile(1 hunks)cmd/zetae2e/config/clients.go(3 hunks)cmd/zetae2e/config/localnet.yml(1 hunks)cmd/zetae2e/local/local.go(4 hunks)e2e/config/config.go(4 hunks)e2e/runner/clients.go(2 hunks)e2e/runner/setup_sui.go(1 hunks)e2e/utils/sui/gateway.go(1 hunks)e2e/utils/sui/signer.go(1 hunks)e2e/utils/sui/signer_test.go(1 hunks)go.mod(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.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.
e2e/runner/clients.goe2e/utils/sui/gateway.gocmd/zetae2e/config/clients.gocmd/zetae2e/local/local.goe2e/runner/setup_sui.goe2e/config/config.goe2e/utils/sui/signer.goe2e/utils/sui/signer_test.go
🔇 Additional comments (17)
e2e/utils/sui/gateway.go (1)
1-15: Confirm appropriate use of embedded binary for contract deploymentEmbedding the
gateway.mvbinary directly in the code is acceptable for small binaries, but it can increase the binary size and complicate version management. Consider whether this approach aligns with your deployment and maintenance strategies.Ensure that embedding the binary is the optimal solution. Alternatively, you might load the binary from an external file or use a build process to manage it.
e2e/runner/clients.go (1)
7-7: LGTM! Clean integration of Sui client.The addition of the Sui client interface is well-structured and follows the existing pattern in the Clients struct.
Also applies to: 29-29
cmd/zetae2e/config/clients.go (1)
55-58: LGTM! Well-structured client initialization.The Sui client initialization follows the established pattern and properly handles the optional nature of the Sui RPC configuration.
Also applies to: 75-75
e2e/config/config.go (1)
105-106: LGTM! Well-structured RPC configuration.The addition of Sui RPC fields follows the existing pattern and maintains consistency with other chain configurations.
cmd/zetae2e/local/local.go (3)
39-39: LGTM!The flag declaration follows the established pattern and naming convention.
109-109: LGTM!The flag retrieval is consistent with other test flags and uses the
musthelper correctly.
200-202: LGTM!The Sui setup logic is correctly guarded by the test flag and skip condition, following the established pattern for blockchain setups.
cmd/zetae2e/config/localnet.yml (1)
108-109: LGTM!The Sui RPC endpoints are correctly configured using Docker service names and standard ports.
.github/workflows/e2e.yml (4)
124-124: LGTM!The
SUI_TESTSoutput variable follows the established pattern for test outputs.
158-158: LGTM!The Sui test label handling is consistent with other test labels.
190-190: LGTM!The Sui test handling for release branches follows the established pattern.
273-275: LGTM!The matrix entry for Sui tests is correctly configured and follows the established pattern.
Makefile (1)
315-318: LGTM!The Sui test target is correctly configured:
- Follows the established pattern of other test targets
- Sets appropriate E2E_ARGS
- Uses the correct Docker Compose profile
go.mod (4)
122-122: Direct Dependency Activation for secp256k1/v4
The update on this line convertsgithub.com/decred/dcrd/dcrec/secp256k1/v4 v4.3.0from an indirect to a direct dependency. This change makes the version explicit, which aids in troubleshooting and ensures version stability for cryptographic operations.
307-307: New Sui SDK Dependency Addition
The inclusion ofgithub.com/block-vision/sui-go-sdk v1.0.6supports the PR’s objective to integrate Sui blockchain testing into the e2e framework. Please ensure that its initialization and associated configurations are properly integrated in the relevant client and configuration modules.
337-339: Introduction of Go-Playground Libraries for Validation and Translation
The addition ofgithub.com/go-playground/locales v0.14.1,github.com/go-playground/universal-translator v0.18.1, andgithub.com/go-playground/validator/v10 v10.12.0expands the module's capability for advanced data validation and localization. Verify that these dependencies are consistently used throughout the configuration or input handling logic to meet the PR’s end-to-end testing requirements.
351-351: Addition of go-urn Library
The new dependencygithub.com/leodido/go-urn v1.2.2is introduced to support URN parsing or manipulation functionalities. Please confirm that this package is utilized where required in the Sui integration or other related areas.
TODO:
store contract address in configThis can be merged if/when someone else want's to take over this process. Just let me know and I'll hand it over.
Signer and publish logic derived from protocol-contracts-sui with some refactoring and improved test coverage.
Update: I think this is a good start and first PR, I think we should do the rest in separate PRs
Notes:
Related to #3318 and #3474 and #3465
Summary by CodeRabbit
New Features
Tests
Chores