🐛 app: misc fixes#1028
Conversation
🦋 Changeset detectedLatest commit: 3a93b05 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughAdds three patch changesets and implements: LI.FI-backed token filtering for wallet balances, batching of paymaster ERC-20 approvals into the bridge sendCallsTx, and Alchemy-based fee estimation for cross-chain SmartAccount clients. ChangesBridge and portfolio refinements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces several improvements to the mobile application, including batching fee token approvals into bridge transactions, enhancing bridge fee estimation with a dedicated estimator and multiplier, and filtering unverified tokens from the portfolio using Li.Fi data. Feedback identifies a critical issue where batching the paymaster approval into the same UserOperation as the bridge call will likely cause validation failures, as paymasters typically verify allowances before the execution phase. Additionally, a performance optimization is suggested to avoid recreating a large Set of tokens during frequent balance refreshes.
| ...(paymasterFee && paymasterApproval | ||
| ? [{ to: getAddress(paymasterFee.token.address), data: paymasterApproval }] | ||
| : []), |
There was a problem hiding this comment.
Batching the ERC-20 paymaster approval into the same UserOperation as the bridge call will likely cause the transaction to fail during the validation phase. Most ERC-4337 paymasters (including Alchemy's) check the allowance in validatePaymasterUserOp. Since the execution of the batched approve call only happens after successful validation, the paymaster will see a zero allowance and reject the UserOperation. This will trigger the fallback logic at line 584, causing the app to always use sponsored gas instead of the intended fee token when an approval is required.
| }) | ||
| : undefined, | ||
| ]); | ||
| const known = new Set(lifiTokens.map((token) => `${token.chainId}:${token.address.toLowerCase()}`)); |
There was a problem hiding this comment.
Creating a new Set by mapping over the entire lifiTokens array (which can contain thousands of items) on every balance refresh (every 30 seconds) may impact performance on mobile devices. Consider memoizing this Set or moving the transformation logic into the lifiTokensOptions query using the select option to avoid redundant processing.
References
- Prefer inline mappings for simple, self-contained data transformations instead of extracting them into constants.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1028 +/- ##
==========================================
- Coverage 73.84% 72.89% -0.95%
==========================================
Files 243 243
Lines 10787 10343 -444
Branches 3653 3407 -246
==========================================
- Hits 7966 7540 -426
+ Misses 2516 2499 -17
+ Partials 305 304 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary by CodeRabbit
Bug Fixes
Improvements
Chores