[Wallet] Move 7702 batch transaction test to integration tests #7166
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes involve reorganizing and consolidating tests for in-app wallet functionality. A test file for EIP-7702 minimal account batch transactions was deleted, and its core batch transaction test was integrated into the main in-app wallet integration test suite. All integration tests now run conditionally based on an environment variable. Changes
Sequence Diagram(s)sequenceDiagram
participant TestSuite
participant InAppWallet
participant BlockchainClient
participant Chain
participant Account
TestSuite->>InAppWallet: Instantiate (EIP-7702, gas sponsorship)
InAppWallet->>BlockchainClient: Connect (guest strategy, baseSepolia)
TestSuite->>Account: Generate addresses (2x)
TestSuite->>InAppWallet: Prepare batch transactions (to Account1, Account2)
InAppWallet->>BlockchainClient: Send batch transaction
BlockchainClient-->>InAppWallet: Return transaction hash
InAppWallet-->>TestSuite: Assert transaction hash is defined
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
![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 (1)
packages/thirdweb/src/wallets/in-app/web/lib/in-app-integration.test.ts (1)
119-149: Consider adding event log validation to the batch transaction test.The batch transaction test is functionally correct but lacks the comprehensive validation present in other tests. Consider adding event log parsing to verify that both transactions in the batch were executed successfully.
Add event log validation similar to the other tests:
const result = await sendBatchTransaction({ account, transactions: [tx1, tx2], }); expect(result.transactionHash).toBeDefined(); + const logs = parseEventLogs({ + logs: result.logs, + events: [executedEvent()], + }); + expect(logs).toHaveLength(2); // Verify both transactions were executed + logs.forEach((log) => { + expect(log.args.value).toBe(0n); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/thirdweb/src/wallets/in-app/core/eip7702/minimal-account.test.ts(0 hunks)packages/thirdweb/src/wallets/in-app/web/lib/in-app-integration.test.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/thirdweb/src/wallets/in-app/core/eip7702/minimal-account.test.ts
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Unit Tests
- GitHub Check: Build Packages
- GitHub Check: Size
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (5)
packages/thirdweb/src/wallets/in-app/web/lib/in-app-integration.test.ts (5)
3-3: LGTM! Appropriate imports for new functionality.The new imports (
baseSepolia,sendBatchTransaction,generateAccount) are correctly added to support the new batch transaction test functionality.Also applies to: 9-9, 11-11
14-16: Excellent use of conditional test execution.Using
describe.runIf(process.env.TW_SECRET_KEY)is a best practice for integration tests that require credentials, preventing test failures in environments where the secret key is not available.
17-29: Message signing tests look good.Both backend and guest strategy tests are properly structured and verify the expected functionality. The tests correctly assert that both the account address and signed message are defined.
Also applies to: 31-42
44-78: Well-implemented EIP-7702 gas sponsorship test.The test properly verifies gas sponsorship functionality and includes comprehensive event log validation to ensure the transaction was executed correctly with the expected parameters.
80-117: Enhanced EIP-4337 test with proper event validation.The test correctly validates
userOperationEventEventlogs and checks both sender address and success status, providing good coverage for the EIP-4337 execution mode.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7166 +/- ##
=======================================
Coverage 55.66% 55.66%
=======================================
Files 904 904
Lines 58391 58391
Branches 4111 4112 +1
=======================================
Hits 32505 32505
+ Misses 25781 25780 -1
- Partials 105 106 +1
🚀 New features to boost your workflow:
|
size-limit report 📦
|
joaquim-verges
deleted the
_Wallet_Move_7702_batch_transaction_test_to_integration_tests
branch
PR-Codex overview
This PR refactors the integration tests for the
inAppWallet, improving structure and adding new test cases for signing messages and sponsoring gas for smart accounts.Detailed summary
describewithdescribe.runIf(process.env.TW_SECRET_KEY)for conditional test execution.7702smart account.7702account.Summary by CodeRabbit