test(e2e): sui deposits #3560
test(e2e): sui deposits #3560
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 comprehensive support for the Sui blockchain. It adds configuration options, account types, and contract parameters for Sui in multiple configuration files and extends the E2E test suite with new Sui deposit and call test functions. In addition, the E2ERunner is enhanced with Sui-specific fields and methods for contract deployment, token requests, and transaction handling. Several scripts and utility files are updated to fund Sui tester accounts and improve error handling and logging consistency across modules. Changes
Sequence Diagram(s)sequenceDiagram
participant TR as Test Routine
participant R as E2ERunner
participant F as Sui Faucet/Blockchain
TR->>R: Invoke SetupSui & suiTestRoutine
R->>F: Request SUI tokens (RequestSuiFaucetToken)
F-->>R: Return faucet token
R->>R: Deploy Fake USDC & set chain parameters (deployFakeUSDC, whitelistFakeUSDC)
TR->>R: Execute SUI deposit/deposit-and-call tests
R->>F: Submit deposit transaction (SUIDeposit/SuiFungibleTokenDeposit)
F-->>R: Confirm transaction mined & return cctx
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 ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3560 +/- ##
========================================
Coverage 64.69% 64.70%
========================================
Files 466 466
Lines 32477 32462 -15
========================================
- Hits 21010 21003 -7
+ Misses 10512 10504 -8
Partials 955 955
|
|
!!!WARNING!!! Be very careful about using Only suppress a single rule (or a specific set of rules) within a section of code, while continuing to scan for other problems. To do this, you can list the rule(s) to be suppressed within the #nosec annotation, e.g: /* #nosec G401 */ or //#nosec G201 G202 G203 Pay extra attention to the way |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (27)
x/crosschain/types/inbound_parsing_test.go (1)
7-8: Consider implementing the empty test functions.The file contains empty test functions with a TODO comment referencing issue #2669.
Would you like me to help implement these test functions or create a new issue to track this task?
contrib/localnet/scripts/start-zetacored.sh (1)
321-323: Fix formatting: Add newline before comment.The "migration tester" comment is appended to the same line as the Sui tester account addition. This affects readability.
Apply this diff to fix the formatting:
- zetacored add-genesis-account "$address" 100000000000000000000000000azeta# migration tester + zetacored add-genesis-account "$address" 100000000000000000000000000azeta +# migration testerzetaclient/chains/evm/observer/outbound_test.go (1)
569-569: LGTM! Consider addressing the TODO comment.The change to use
FungibleTokenCoinTypealigns with the broader codebase transition. However, this test is currently commented out and marked with a TODO. Consider implementing the test to ensure proper coverage of fungible token handling.Would you like me to help implement the test or create an issue to track this task?
e2e/runner/setup_zevm.go (1)
174-193: LGTM! Consider adding error documentation.The
SetupSUIZRC20implementation follows the same pattern as other chain-specific setup methods, with proper error handling and logging.Consider adding a function comment to document the error conditions:
// SetupSUIZRC20 sets up the SUI ZRC20 in the runner from the values queried from the chain. // Returns early if chain operations are skipped. // Requires SystemContract to be initialized. // Panics if GasCoinZRC20ByChainId call fails or if ZRC20 contract creation fails. func (r *E2ERunner) SetupSUIZRC20() {cmd/zetae2e/local/local.go (1)
425-433: LGTM! Consider adding more test cases.The Sui test suite is well-structured and includes basic operations. However, consider expanding the test coverage.
Consider adding these test cases to match the coverage of other chains:
- Withdrawal tests
- Concurrent operation tests
- Error case tests (e.g., restricted addresses, invalid parameters)
- Revert scenarios
e2e/e2etests/test_sui_token_deposit_and_call.go (2)
18-18: Consider using deterministic test data.Using
randomPayloadcould make tests non-deterministic. Consider using a fixed test payload or parameterizing it for better test reproducibility.
13-30: Add test cases for edge cases.The test only covers the happy path. Consider adding test cases for:
- Invalid amounts
- Failed transactions
- Error scenarios
e2e/e2etests/test_sui_token_deposit.go (2)
13-13: Add function documentation.Consider adding a function comment explaining the test's purpose, parameters, and expected behavior.
+// TestSuiTokenDeposit validates the Sui fungible token deposit functionality by: +// - Depositing a specified amount of tokens +// - Verifying the transaction is mined +// - Checking the CCTX status and coin type func TestSuiTokenDeposit(r *runner.E2ERunner, args []string) {
13-28: Add error handling test cases.The test only covers successful deposits. Consider adding test cases for:
- Invalid amounts
- Transaction failures
- Timeout scenarios
e2e/e2etests/test_sui_deposit.go (1)
13-28: Enhance test coverage with additional test cases.The test function should include:
- Validation that the deposited amount matches the expected value in the cross-chain context.
- Error scenarios such as invalid amounts or failed transactions.
Add the following test cases:
func TestSuiDeposit(r *runner.E2ERunner, args []string) { require.Len(r, args, 1) amount := utils.ParseBigInt(r, args[0]) // make the deposit transaction resp := r.SUIDeposit(r.EVMAddress(), math.NewUintFromBigInt(amount)) r.Logger.Info("Sui deposit tx: %s", resp.Digest) // wait for the cctx to be mined cctx := utils.WaitCctxMinedByInboundHash(r.Ctx, resp.Digest, r.CctxClient, r.Logger, r.CctxTimeout) r.Logger.CCTX(*cctx, "deposit") require.EqualValues(r, crosschaintypes.CctxStatus_OutboundMined, cctx.CctxStatus.Status) require.EqualValues(r, coin.CoinType_Gas, cctx.InboundParams.CoinType) + // Validate deposited amount + require.EqualValues(r, amount.String(), cctx.InboundParams.Amount.String(), "Deposited amount mismatch") } + +func TestSuiDepositInvalidAmount(r *runner.E2ERunner, args []string) { + require.Len(r, args, 1) + + // Test with zero amount + amount := big.NewInt(0) + resp := r.SUIDeposit(r.EVMAddress(), math.NewUintFromBigInt(amount)) + + // Expect transaction to fail + require.Error(r, resp.Error, "Expected error for zero amount deposit") +}zetaclient/chains/sui/observer/inbound.go (2)
26-26: Structure log messages for better observability.The log messages should follow a consistent structure with relevant fields.
Enhance logging with structured fields:
-ob.Logger().Inbound.Info().Msg("Cursor: " + ob.getCursor()) +ob.Logger().Inbound.Info(). + Str("cursor", ob.getCursor()). + Str("chain", "sui"). + Msg("Processing inbound transactions") -ob.Logger().Inbound.Info().Int("events", len(events)).Msg("Processing sui inbound events") +ob.Logger().Inbound.Info(). + Int("events_count", len(events)). + Str("chain", "sui"). + Str("cursor", ob.getCursor()). + Msg("Processing inbound events")Also applies to: 41-41
46-47: Consider concurrent event processing for better performance.The comment suggests potential for concurrent processing, which could improve throughput.
Consider implementing a worker pool pattern for concurrent event processing with proper error handling and rate limiting.
e2e/e2etests/test_sui_deposit_and_call.go (1)
13-30: Enhance payload validation in test cases.The test should validate the payload handling in the cross-chain context.
Add payload validation and error scenarios:
func TestSuiDepositAndCall(r *runner.E2ERunner, args []string) { require.Len(r, args, 1) amount := utils.ParseBigInt(r, args[0]) payload := randomPayload(r) + // Validate payload is not empty + require.NotEmpty(r, payload, "Payload should not be empty") // make the deposit transaction resp := r.SUIDepositAndCall(r.TestDAppV2ZEVMAddr, math.NewUintFromBigInt(amount), []byte(payload)) r.Logger.Info("Sui deposit and call tx: %s", resp.Digest) // wait for the cctx to be mined cctx := utils.WaitCctxMinedByInboundHash(r.Ctx, resp.Digest, r.CctxClient, r.Logger, r.CctxTimeout) r.Logger.CCTX(*cctx, "deposit") require.EqualValues(r, crosschaintypes.CctxStatus_OutboundMined, cctx.CctxStatus.Status) require.EqualValues(r, coin.CoinType_Gas, cctx.InboundParams.CoinType) + // Validate payload in cross-chain context + require.EqualValues(r, payload, cctx.GetInboundParams().Message, "Payload mismatch in cross-chain context") } + +func TestSuiDepositAndCallInvalidPayload(r *runner.E2ERunner, args []string) { + require.Len(r, args, 1) + amount := utils.ParseBigInt(r, args[0]) + + // Test with empty payload + resp := r.SUIDepositAndCall(r.TestDAppV2ZEVMAddr, math.NewUintFromBigInt(amount), []byte{}) + require.Error(r, resp.Error, "Expected error for empty payload") +}e2e/txserver/zeta_tx_server.go (1)
499-509: Extract Sui deployment parameters to configuration.The Sui fungible coin deployment parameters are hardcoded, making it less maintainable.
Extract parameters to configuration:
+type FungibleCoinConfig struct { + ChainID int64 + Decimals uint32 + Symbol string + Name string + CoinType coin.CoinType + GasLimit uint64 +} + +var SuiFungibleCoinConfig = FungibleCoinConfig{ + ChainID: chains.SuiLocalnet.ChainId, + Decimals: 9, + Symbol: "SUI", + Name: "SUI", + CoinType: coin.CoinType_Gas, + GasLimit: 100000, +} -fungibletypes.NewMsgDeployFungibleCoinZRC20( - deployerAddr, - "", - chains.SuiLocalnet.ChainId, - 9, - "SUI", - "SUI", - coin.CoinType_Gas, - 100000, - nil, -), +fungibletypes.NewMsgDeployFungibleCoinZRC20( + deployerAddr, + "", + SuiFungibleCoinConfig.ChainID, + SuiFungibleCoinConfig.Decimals, + SuiFungibleCoinConfig.Symbol, + SuiFungibleCoinConfig.Name, + SuiFungibleCoinConfig.CoinType, + SuiFungibleCoinConfig.GasLimit, + nil, +),cmd/zetae2e/local/sui.go (2)
66-67: Remove redundant error return.The error is already handled in the test execution block. The final return can be simplified.
- return err + return nil
47-49: Enhance logging for better observability.Add more context to the log message to improve debugging capabilities.
- deployerRunner.Logger.Info("Sui USDC mint tx: %s", txRes.Digest) + deployerRunner.Logger.Info("Sui USDC mint tx (amount: %s, address: %s): %s", "10000000", suiRunnerSigner.Address(), txRes.Digest)e2e/runner/runner.go (2)
119-128: Improve field documentation for clarity.The documentation for Sui-related fields could be more descriptive.
-// contract Sui +// Sui contract identifiers GatewayPackageID string GatewayObjectID string -// SuiTokenCoinType is the coin type identifying the fungible token for SUI +// SuiTokenCoinType is the fully qualified type identifier for the fungible token on Sui network SuiTokenCoinType string -// SuiTokenTreasuryCap is the treasury cap for the SUI token that allows minting, only using in local tests +// SuiTokenTreasuryCap is the object ID of the treasury cap that grants minting capability for the Sui token. +// Note: This is only used in local testing environments. SuiTokenTreasuryCap string
145-160: Improve organization of ZRC20 contracts.Consider grouping ZRC20 contracts by chain type for better readability.
-// zrc20 contracts +// Layer 1 chain ZRC20 contracts ERC20ZRC20Addr ethcommon.Address ERC20ZRC20 *zrc20.ZRC20 SPLZRC20Addr ethcommon.Address SPLZRC20 *zrc20.ZRC20 ETHZRC20Addr ethcommon.Address ETHZRC20 *zrc20.ZRC20 BTCZRC20Addr ethcommon.Address BTCZRC20 *zrc20.ZRC20 SOLZRC20Addr ethcommon.Address SOLZRC20 *zrc20.ZRC20 + +// Layer 2 chain ZRC20 contracts TONZRC20Addr ethcommon.Address TONZRC20 *zrc20.ZRC20 SUIZRC20Addr ethcommon.Address SUIZRC20 *zrc20.ZRC20pkg/contracts/sui/crypto.go (2)
48-61: AddressFromPubKeyECDSA might benefit from explicit curve checks.
You may wish to confirm the key’s curve matches secp256k1 before computing the Blake2b hash to avoid misuse. Otherwise, function logic seems compliant with Sui’s address generation.func AddressFromPubKeyECDSA(pk *ecdsa.PublicKey) string { + // For safety, confirm pk is secp256k1 before continuing: + // if pk.Curve != secp256k1.S256() { + // return "", errors.New("unsupported curve, expected secp256k1") + // } pubBytes := elliptic.MarshalCompressed(pk.Curve, pk.X, pk.Y) ... }
63-81: SerializeSignatureECDSA properly handles signature length but lacks checks for invalid signatures.
While skipping the recovery byte is acceptable per Sui’s convention, consider verifying the 65-byte signature array to ensure none of the R/S bytes are out of range. This is minor since test code likely ensures well-formed data already.e2e/runner/setup_sui.go (1)
78-118: SetupSui successfully orchestrates Sui gateway setup but might prefer explicit comments on fallback logic.
The approach to publishing, extracting packageID, gatewayID, and whitelistID is sensible. Consider logging or handling partial successes (e.g., missing gatewayID or whitelistID) with a more descriptive error to simplify troubleshooting.e2e/config/config.go (1)
421-429: Enhance error handling in SuiSigner method.Consider adding validation for empty or invalid private keys before decoding.
func (a Account) SuiSigner() (*sui_utils.SignerSecp256k1, error) { + if a.RawPrivateKey == "" { + return nil, fmt.Errorf("empty private key") + } privateKeyBytes, err := hex.DecodeString(a.RawPrivateKey.String()) if err != nil { return nil, fmt.Errorf("decode private key: %w", err) } signer := sui_utils.NewSignerSecp256k1FromSecretKey(privateKeyBytes) return signer, nil }e2e/runner/sui.go (5)
16-45: Parameterize gas budget for flexibility.The gas budget is hardcoded in the deposit methods. Consider making it configurable to handle network changes or different transaction requirements.
+// DefaultGasBudget is the default gas budget for Sui transactions +const DefaultGasBudget = "5000000000" + // SUIDeposit calls Deposit on Sui func (r *E2ERunner) SUIDeposit( receiver ethcommon.Address, amount math.Uint, + gasBudget string, ) models.SuiTransactionBlockResponse { + if gasBudget == "" { + gasBudget = DefaultGasBudget + } // ... rest of the implementation
47-76: Add documentation for coin type format.The methods use a specific format for coin type (
0xprefix) but this requirement isn't documented.+// SuiFungibleTokenDeposit calls Deposit with fungible token on Sui +// coinType format: "<packageID>::<module>::<name>" +// Example: "0x2::sui::SUI" func (r *E2ERunner) SuiFungibleTokenDeposit( receiver ethcommon.Address, amount math.Uint, ) models.SuiTransactionBlockResponse {
78-105: Fix typo and add amount validation.The method has a typo in the comment ("trasuryCap") and should validate the amount parameter.
-// this function requires the signer to be the owner of the trasuryCap +// this function requires the signer to be the owner of the treasuryCap func (r *E2ERunner) MintSuiUSDC( amount, receiver string, ) models.SuiTransactionBlockResponse { + if amount == "" || receiver == "" { + r.FailNow("amount and receiver cannot be empty") + }
192-237: Add retry mechanism for coin finding.Network latency might cause delays in coin availability after splitting. Consider adding retries with backoff.
+const ( + maxRetries = 3 + retryDelay = 1 * time.Second +) + func (r *E2ERunner) findSuiCoin( address string, balance math.Uint, coinType string, comp compFunc, ) (coinID string) { + for i := 0; i < maxRetries; i++ { res, err := r.Clients.Sui.SuiXGetCoins(r.Ctx, models.SuiXGetCoinsRequest{ Owner: address, CoinType: coinType, }) require.NoError(r, err) for _, data := range res.Data { coinBalance, err := math.ParseUint(data.Balance) require.NoError(r, err) if comp(coinBalance, balance) { return data.CoinObjectId } } + if i < maxRetries-1 { + time.Sleep(retryDelay) + } + }
239-255: Add transaction status validation.Consider validating the transaction status in the response to ensure successful execution.
func (r *E2ERunner) executeSuiTx( signer *sui.SignerSecp256k1, tx models.TxnMetaData, ) models.SuiTransactionBlockResponse { // ... existing implementation ... require.NoError(r, err) + + // Validate transaction status + if resp.Effects != nil && resp.Effects.Status != nil { + status := resp.Effects.Status.Status + require.Equal(r, "success", status, "transaction failed with status: %s", status) + } return resp }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (38)
changelog.md(2 hunks)cmd/zetae2e/config/config.go(1 hunks)cmd/zetae2e/config/contracts.go(2 hunks)cmd/zetae2e/config/local.yml(1 hunks)cmd/zetae2e/config/localnet.yml(1 hunks)cmd/zetae2e/local/local.go(2 hunks)cmd/zetae2e/local/sui.go(1 hunks)contrib/localnet/orchestrator/start-zetae2e.sh(1 hunks)contrib/localnet/scripts/start-zetacored.sh(1 hunks)e2e/config/config.go(7 hunks)e2e/contracts/sui/bin.go(1 hunks)e2e/e2etests/e2etests.go(2 hunks)e2e/e2etests/test_sui_deposit.go(1 hunks)e2e/e2etests/test_sui_deposit_and_call.go(1 hunks)e2e/e2etests/test_sui_token_deposit.go(1 hunks)e2e/e2etests/test_sui_token_deposit_and_call.go(1 hunks)e2e/runner/logger.go(2 hunks)e2e/runner/runner.go(7 hunks)e2e/runner/setup_sui.go(2 hunks)e2e/runner/setup_zevm.go(2 hunks)e2e/runner/solana.go(1 hunks)e2e/runner/sui.go(1 hunks)e2e/txserver/zeta_tx_server.go(1 hunks)pkg/chains/chain.go(1 hunks)pkg/coin/coin_test.go(1 hunks)pkg/contracts/sui/crypto.go(1 hunks)testutil/keeper/crosschain.go(1 hunks)x/crosschain/keeper/cctx_orchestrator_validate_inbound.go(4 hunks)x/crosschain/keeper/evm_deposit_test.go(7 hunks)x/crosschain/keeper/msg_server_vote_inbound_tx.go(2 hunks)x/crosschain/types/cctx.go(1 hunks)x/crosschain/types/inbound_parsing_test.go(2 hunks)x/fungible/keeper/deposits.go(1 hunks)zetaclient/chains/evm/observer/outbound_test.go(1 hunks)zetaclient/chains/sui/client/client.go(1 hunks)zetaclient/chains/sui/observer/inbound.go(2 hunks)zetaclient/chains/sui/observer/observer.go(2 hunks)zetaclient/chains/sui/observer/observer_test.go(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- x/crosschain/keeper/evm_deposit_test.go
- e2e/runner/solana.go
- changelog.md
- e2e/runner/logger.go
- x/crosschain/types/cctx.go
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.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/coin/coin_test.gopkg/chains/chain.gox/crosschain/keeper/cctx_orchestrator_validate_inbound.gotestutil/keeper/crosschain.gocmd/zetae2e/local/local.gox/fungible/keeper/deposits.gox/crosschain/types/inbound_parsing_test.goe2e/e2etests/test_sui_deposit.gozetaclient/chains/evm/observer/outbound_test.goe2e/e2etests/test_sui_deposit_and_call.gocmd/zetae2e/config/config.goe2e/e2etests/test_sui_token_deposit_and_call.goe2e/runner/setup_zevm.gozetaclient/chains/sui/observer/observer.goe2e/e2etests/test_sui_token_deposit.gox/crosschain/keeper/msg_server_vote_inbound_tx.goe2e/contracts/sui/bin.gocmd/zetae2e/config/contracts.gozetaclient/chains/sui/client/client.goe2e/txserver/zeta_tx_server.gozetaclient/chains/sui/observer/inbound.goe2e/e2etests/e2etests.goe2e/config/config.gocmd/zetae2e/local/sui.gozetaclient/chains/sui/observer/observer_test.goe2e/runner/setup_sui.goe2e/runner/runner.gopkg/contracts/sui/crypto.goe2e/runner/sui.go
`**/*.sh`: Review the shell scripts, point out issues relati...
**/*.sh: Review the shell scripts, point out issues relative to security, performance, and maintainability.
contrib/localnet/orchestrator/start-zetae2e.shcontrib/localnet/scripts/start-zetacored.sh
🪛 GitHub Check: codecov/patch
pkg/chains/chain.go
[warning] 138-139: pkg/chains/chain.go#L138-L139
Added lines #L138 - L139 were not covered by tests
x/crosschain/keeper/cctx_orchestrator_validate_inbound.go
[warning] 36-36: x/crosschain/keeper/cctx_orchestrator_validate_inbound.go#L36
Added line #L36 was not covered by tests
🪛 Gitleaks (8.21.2)
cmd/zetae2e/config/local.yml
42-42: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
cmd/zetae2e/config/localnet.yml
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (30)
contrib/localnet/orchestrator/start-zetae2e.sh (1)
116-117: LGTM!The addition of Sui tester account funding follows the established pattern and maintains consistency with other test account configurations.
pkg/coin/coin_test.go (1)
147-147: LGTM!The error message update provides more precise type information, improving test clarity.
x/crosschain/types/inbound_parsing_test.go (1)
36-36: LGTM!The type references have been updated to use the more specific
FungibleTokenCoinType, maintaining consistency with other changes.Also applies to: 76-76
zetaclient/chains/sui/client/client.go (1)
20-20:❓ Verification inconclusive
Verify the impact of reducing DefaultEventsLimit.
The reduction from 100 to 50 could affect:
- Memory usage and network traffic (positive impact)
- Number of API calls needed to fetch all events (potential negative impact)
- Response times for event queries
Please ensure this change doesn't adversely affect performance in high-load scenarios.
Run the following script to analyze the impact:
🏁 Script executed:
#!/bin/bash # Description: Search for QueryModuleEvents usage to assess the impact # Find all files using QueryModuleEvents rg -A 5 "QueryModuleEvents" # Find any performance-related tests rg -l "TestQueryModuleEvents|BenchmarkQueryModuleEvents"Length of output: 4231
Action Required: Verify Performance Impact of DefaultEventsLimit Change
Based on our investigation:
- The
QueryModuleEventsfunction is used across several modules (e.g., inclient_live_test.go,observer/inbound.go, and in test mocks) without dedicated performance benchmarks.- The search for keywords such as
TestQueryModuleEventsorBenchmarkQueryModuleEventsdid not return any explicit test cases, which leaves the true performance impact under high-load scenarios unverified.Next Steps:
- Manually test event queries under load to ensure that reducing the default limit from 100 to 50 does not lead to an increase in the number of API calls or degrade response times.
- Consider implementing performance benchmarks to monitor memory usage, network traffic, and overall responsiveness if high-load conditions are expected.
Please verify these aspects in your staging or testing environment before finalizing the change.
cmd/zetae2e/config/config.go (1)
61-64: LGTM! The Sui contract configuration follows the established pattern.The implementation correctly follows the existing pattern of using
DoubleQuotedStringfor contract identifiers, maintaining consistency with other blockchain configurations.testutil/keeper/crosschain.go (1)
316-316: LGTM! The parameter type update in the comment reflects the correct type.The comment update from
CoinTypetoFungibleTokenCoinTypealigns with the broader changes in the codebase for more specific type usage.e2e/contracts/sui/bin.go (2)
11-12: LGTM! The fake USDC contract embedding follows the established pattern.The implementation correctly uses the
go:embeddirective to include the contract bytecode.
19-22: LGTM! The base64 encoding function follows the gateway function pattern.The implementation maintains consistency with the existing
GatewayBytecodeBase64function.e2e/runner/setup_zevm.go (1)
101-101: LGTM! The addition of Sui setup follows existing patterns.The call to
SetupSUIZRC20inSetupZEVMZRC20sis correctly placed alongside other chain-specific setups.x/crosschain/keeper/msg_server_vote_inbound_tx.go (1)
78-78: LGTM! Error messages are more descriptive.The updated error messages provide better context about the failure points:
- "failed to vote on inbound ballot" instead of using
voteInboundID- "failed to validate inbound" instead of using
voteInboundIDThis improves error reporting and debugging capabilities.
Also applies to: 105-105
cmd/zetae2e/local/local.go (1)
260-262: LGTM! Sui setup is properly conditioned.The Sui setup is correctly guarded by the
testSuiflag and follows the same pattern as other chain-specific setups.x/crosschain/keeper/cctx_orchestrator_validate_inbound.go (2)
24-25: LGTM! Enhanced error handling with descriptive context.The error wrapping provides better context for debugging and error tracing.
46-46: LGTM! Improved error context.The error wrapping enhances error tracing and debugging capabilities.
cmd/zetae2e/config/local.yml (1)
39-42: LGTM! Consistent account configuration for Sui.The Sui account configuration follows the established pattern and includes all required fields.
🧰 Tools
🪛 Gitleaks (8.21.2)
42-42: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
cmd/zetae2e/config/localnet.yml (1)
37-40: LGTM! Consistent Sui account configuration across environments.The Sui account configuration maintains consistency with local.yml, ensuring uniform behavior across environments.
🧰 Tools
🪛 Gitleaks (8.21.2)
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
x/fungible/keeper/deposits.go (1)
263-267: LGTM! Enhanced error context.The improved error message now includes the asset and chainID, making it easier to diagnose issues when a foreign coin is not found.
zetaclient/chains/sui/observer/observer_test.go (1)
70-70: LGTM! Simplified test setup.The removal of mock gateway setup and initialization of cursor with empty string simplifies the test without compromising its effectiveness.
cmd/zetae2e/config/contracts.go (2)
120-132: LGTM! The Sui contract configuration follows established patterns.The implementation is clean and consistent with the existing configuration patterns for other chains.
241-250: LGTM! The SUIZRC20 configuration is well-implemented.The implementation follows the established pattern for ZRC20 tokens and includes proper error handling.
zetaclient/chains/sui/observer/observer.go (1)
102-114: LGTM! Simplified cursor initialization logic.The changes address the issue mentioned in the previous review where the initial transaction cursor was causing observation failures. The simplified logic is more robust.
e2e/e2etests/e2etests.go (2)
85-91: Well-structured addition of new constants for Sui tests.
No issues detected; the naming convention aligns well with existing pattern, and the added constants accurately reflect the introduced Sui deposit & call functionalities.
673-707: New Sui test definitions look consistent with existing patterns.
Each test correctly sets distinct arguments for deposit and deposit+call scenarios, matching typical usage in the E2E framework. No logical or correctness issues noted. Good job ensuring each test is clearly named and well-defined.pkg/contracts/sui/crypto.go (2)
16-30: Digest function is correctly implementing Blake2b hashing.
Implementation follows Sui’s recommended hashing for transaction data; base64 decoding and error wrapping are handled cleanly. The function logic is straightforward and consistent with standard Go error-handling best practices.
38-46: messageWithIntentPrefix is concise and correct.
Prepending the intent bytes is in line with Sui’s specification, and no potential memory or performance issues are apparent in the slice copy operations.e2e/runner/setup_sui.go (4)
26-35: RequestSuiFaucetToken function is straightforward.
Fetching tokens from faucet is essential in test environments; ensure that the faucetURL is pre-validated or logged for easier debugging in case of failures. Logic itself is sound and minimal.
120-175: deployFakeUSDC method is clear and robust.
Deployment steps and error checks are well-defined; searching for the “TreasuryCap” via substring match is workable but might be more explicit if you verify the packageID. Nevertheless, it aligns with the current test logic.
177-211: whitelistFakeUSDC includes essential administration steps.
You have enumerated the whitelisting steps and placed a clear TODO comment to replicate this logic with a standard mechanism once available. This is organized and functional, but keep an eye on possible concurrency or repeated calls in parallel tests.
213-270: setSuiChainParams uses a polling approach to confirm chain parameters.
The loop design with time.Sleep in test code is fine; consider logging the retry attempts for clarity. Everything else, including chain param defaults and resetting nonces, looks well-structured and consistent.e2e/config/config.go (2)
75-75: LGTM! Well-structured Sui blockchain integration.The new Sui-related structs and fields follow consistent naming patterns and maintain good organization of configuration data.
Also applies to: 126-126, 135-141
353-356: LGTM! Consistent account generation implementation.The UserSui account generation follows the established pattern and maintains consistency with other account generations.
# Conflicts: # changelog.md # e2e/config/config.go # pkg/contracts/sui/crypto.go # pkg/contracts/sui/gateway.go # zetaclient/chains/sui/observer/observer_test.go
gartnera
added
the
CONSENSUS_BREAKING_ACK
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Description
Closes #3557
Closes #3474
Summary by CodeRabbit
New Features
Tests
Chores