test: add E2E tests for Sui fungible token withdrawAndCall#3831
test: add E2E tests for Sui fungible token withdrawAndCall#3831ws4charlie merged 7 commits intodevelopfrom
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 update introduces comprehensive end-to-end (E2E) tests for the Sui blockchain's withdraw-and-call functionality, covering both native SUI and fungible token scenarios, including revert handling. The changes add new E2E test cases, supporting test logic, and utility methods for payload construction and transaction execution. Adjustments are made to the Move contract to work around type mismatches, and related configuration and deployment scripts are updated to reflect the removal of the pool object. The test runner is enhanced to support the new test flows, and documentation is updated to reflect the additions. Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner as E2ERunner
participant SuiGateway as GatewayZEVM
participant SuiContract as ExampleContract
participant ZEVM as ZEVM
TestRunner->>SuiGateway: SuiWithdrawAndCallFungibleToken(receiver, amount, payload, revertOptions)
SuiGateway->>SuiContract: withdraw_and_call(payload)
alt Success
SuiContract-->>SuiGateway: on_call executed
SuiGateway-->>TestRunner: Transaction receipt (mined)
TestRunner->>ZEVM: Verify balance and call count
else Revert
SuiGateway-->>ZEVM: onRevert callback (with revert payload)
ZEVM-->>TestRunner: Verify revert, refund, and callback
end
Assessment against linked issues
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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
|
|
!!!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 |
ws4charlie
added
test
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
e2e/contracts/sui/example/sources/example.move (1)
46-57: Appropriate function signature simplificationThe removal of the
TARGET_COINgeneric parameter and the commented pool parameter is a suitable approach to resolve type mismatches during testing. The added comments provide good context for future maintainers.However, consider adding a TODO to restore this functionality in production code if this is a temporary test-only workaround.
// Note: this pool type is hardcoded as <SUI, TOKEN> and therefore causes type mismatch error in the // fungible token withdrawAndCall test, where the SOURCE_COIN type is FAKE_USDC instead of TOKEN. // Disabling the pool object for now is the easiest solution to allow the E2E tests to go through. +// TODO: Consider a more robust solution for production code that supports different coin types. // _pool: &mut Pool<SOURCE_COIN, TARGET_COIN>,e2e/runner/sui.go (1)
241-259: Well-designed payload creation helperThe implementation centralizes payload creation logic, which improves maintainability. The method handles the hex decoding of addresses properly and follows a clear error handling pattern.
Consider adding a validation check for the suiAddress parameter to ensure it starts with "0x" before attempting to remove the prefix.
// SuiCreateExampleWACPayload creates a payload for on_call function in Sui the example package // The example on_call function will just forward the withdrawn token to given 'suiAddress' func (r *E2ERunner) SuiCreateExampleWACPayload(suiAddress string) (sui.CallPayload, error) { + // Validate the address format + if !strings.HasPrefix(suiAddress, "0x") { + return sui.CallPayload{}, fmt.Errorf("invalid Sui address format: missing 0x prefix") + } // only the CCTX's coinType is needed, no additional arguments argumentTypes := []string{} objects := []string{ r.SuiExample.GlobalConfigID.String(), r.SuiExample.PartnerID.String(), r.SuiExample.ClockID.String(), } // create the payload message from the sui address message, err := hex.DecodeString(suiAddress[2:]) // remove 0x prefix if err != nil { return sui.CallPayload{}, err } return sui.NewCallPayload(argumentTypes, objects, message), nil }e2e/e2etests/test_sui_token_withdraw_and_call_revert_with_call.go (1)
27-29: Consider making the invalid address generation more explicit.The hardcoded invalid address works for the test, but it might be clearer to use a function or constant with a more descriptive name to indicate why this address is invalid (e.g., too short, invalid format).
- invalidAddress := "8f569597ebca884b784d32678a6f" + // Use an address that's too short to be valid in Sui + invalidAddress := "8f569597ebca884b784d32678a6f" // Invalid: insufficient length
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
e2e/contracts/sui/example/Move.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
changelog.md(1 hunks)cmd/zetae2e/local/local.go(1 hunks)e2e/config/config.go(0 hunks)e2e/contracts/sui/example/Makefile(1 hunks)e2e/contracts/sui/example/sources/example.move(1 hunks)e2e/e2etests/e2etests.go(2 hunks)e2e/e2etests/test_sui_token_withdraw_and_call.go(1 hunks)e2e/e2etests/test_sui_token_withdraw_and_call_revert_with_call.go(1 hunks)e2e/e2etests/test_sui_withdraw_and_call.go(2 hunks)e2e/e2etests/test_sui_withdraw_and_call_revert_with_call.go(2 hunks)e2e/runner/setup_sui.go(1 hunks)e2e/runner/sui.go(2 hunks)
💤 Files with no reviewable changes (1)
- e2e/config/config.go
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/runner/setup_sui.gocmd/zetae2e/local/local.goe2e/e2etests/test_sui_withdraw_and_call.goe2e/e2etests/test_sui_withdraw_and_call_revert_with_call.goe2e/e2etests/test_sui_token_withdraw_and_call_revert_with_call.goe2e/e2etests/test_sui_token_withdraw_and_call.goe2e/runner/sui.goe2e/e2etests/e2etests.go
🧬 Code Graph Analysis (2)
cmd/zetae2e/local/local.go (1)
e2e/e2etests/e2etests.go (5)
TestSuiWithdrawRevertWithCallName(109-109)TestSuiWithdrawAndCallRevertWithCallName(110-110)TestSuiTokenWithdrawName(105-105)TestSuiTokenWithdrawAndCallName(106-106)TestSuiTokenWithdrawAndCallRevertWithCallName(107-107)
e2e/e2etests/e2etests.go (3)
e2e/runner/e2etest.go (3)
NewE2ETest(30-47)ArgDefinition(50-53)WithMinimumVersion(13-17)e2e/e2etests/test_sui_token_withdraw_and_call.go (1)
TestSuiTokenWithdrawAndCall(14-62)e2e/e2etests/test_sui_token_withdraw_and_call_revert_with_call.go (1)
TestSuiTokenWithdrawAndCallRevertWithCall(18-80)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: start-e2e-test / e2e
🔇 Additional comments (15)
changelog.md (1)
43-44: Documentation update for PR #3831 looks good.The addition to the changelog properly documents the new E2E tests for Sui fungible token withdraw and call functionality, providing clear information about what was added.
e2e/contracts/sui/example/Makefile (1)
13-14: Build output destination path modified appropriately.The destination paths for bytecode modules have been changed to copy files one directory up (
../) instead of the current directory. This change aligns with the new test structure and ensures the compiled modules are accessible where needed by the Sui fungible token tests.e2e/runner/setup_sui.go (2)
152-152: Removed pool parameter to simplify test structure.The removal of the pool filter from
objectTypeFiltersaligns with the modifications to the Move contract where the pool parameter was commented out in theon_callfunction. This simplification helps avoid type mismatches in tests and streamlines the test setup.
168-174: Properly updated SuiExample struct initialization.The
PoolIDfield has been removed from theSuiExamplestruct initialization, maintaining consistency with the changes to the Move contract and the corresponding removal ine2e/config/config.go. This ensures a cohesive implementation across the codebase.cmd/zetae2e/local/local.go (1)
525-529: Test order rearranged and new tests added for Sui fungible token.The changes accomplish two things:
- Reorder existing Sui tests for better logical flow
- Add the new tests for Sui fungible token withdraw-and-call functionality:
TestSuiTokenWithdrawAndCallNameTestSuiTokenWithdrawAndCallRevertWithCallNameThese additions complete the test coverage for Sui fungible token operations, ensuring both success and revert cases are properly tested.
e2e/e2etests/test_sui_withdraw_and_call_revert_with_call.go (2)
15-15: LGTM: Clear test documentation addedThe added comment clearly explains the purpose of this test function, which enhances code readability and maintenance.
26-28: Improved code maintainabilityGood refactoring to use the centralized helper method for payload creation rather than manual construction. This promotes code reuse and consistency across tests.
e2e/e2etests/test_sui_withdraw_and_call.go (2)
22-26: Improved test robustness with dynamic address retrievalExcellent change to use a dynamically retrieved signer address instead of hardcoded values. This approach is more flexible and prevents potential issues if the test environment changes.
31-33: Enhanced maintainability with shared payload creation logicGood use of the helper method for payload creation. This promotes consistency and reduces code duplication across tests.
e2e/runner/sui.go (1)
120-148: Well-implemented fungible token withdraw-and-call operationThis implementation properly parallels the existing SUI token functionality but for fungible tokens. The method follows the established pattern with consistent error handling and validation.
e2e/e2etests/test_sui_token_withdraw_and_call.go (1)
1-62: Test logic is well-structured and follows best practices.The end-to-end test for Sui fungible token withdraw-and-call functionality follows a clear arrange-act-assert pattern, with appropriate error handling and comprehensive assertions. The test properly verifies both the transaction status and side effects (balance change and call count increment).
e2e/e2etests/test_sui_token_withdraw_and_call_revert_with_call.go (2)
15-18: Good documentation of test purpose and behavior.The comment clearly explains the test's purpose and expected behavior, which enhances maintainability.
18-80: Test comprehensively verifies revert behavior and token refund.The test thoroughly checks all aspects of the revert flow: transaction status, callback execution, sender verification, and token refund to the dApp address.
e2e/e2etests/e2etests.go (2)
106-107: New test constants follow consistent naming convention.The test constants for Sui fungible token withdraw-and-call functionality follow the established naming pattern in the codebase.
907-924: New test registrations are properly configured.The test registrations include appropriate descriptions, argument definitions, and version requirements (v30.0.0) consistent with other tests in the suite.
3 participants
Description
Added E2E tests:
sui_token_withdraw_and_callsui_token_withdraw_and_call_revert_with_callCloses: #3742
How Has This Been Tested?
Summary by CodeRabbit