Conversation
WalkthroughAdds EIP‑7702 support: wallet‑mode verification, delegated‑EOA authorization checks, a new upgrade modal (UI, polling, gas estimation), provider/context and hooks, propagates authorization into estimate/send flows, updates WalletPortfolio slice, many component/test additions, and bumps Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Kit as TransactionKit
participant Chain
participant Modal
User->>App: login / app load
App->>Kit: create temp kit for verification
App->>Chain: query EOA code & balances per chain
App->>App: derive walletMode & eip7702Info
App->>App: useEIP7702Upgrade.checkOnLogin -> calculate gas & eligibility
alt eligible and hasEnoughGas
App->>Modal: open EIP7702UpgradeModal
end
User->>Modal: "Upgrade now"
Modal->>Kit: delegateSmartAccountToEoa
Kit->>Chain: submit delegation (userOp)
Chain-->>Kit: userOpHash
loop every 2s
Modal->>Kit: getUserOperationStatus(userOpHash)
Kit->>Chain: fetch status
Kit-->>Modal: status update
end
Kit-->>Modal: status = completed
Modal->>App: dispatch completion actions
sequenceDiagram
participant User
participant SendUI
participant EIPAuth as EIP7702Auth
participant Kit as TransactionKit
participant Chain
User->>SendUI: initiate send
SendUI->>SendUI: determine wallet mode (provider/kit)
alt delegatedEoa
SendUI->>SendUI: force Native fee, disable gasless
else modular
SendUI->>SendUI: allow Gasless options
end
SendUI->>EIPAuth: getEIP7702AuthorizationIfNeeded(kit, chainId)
EIPAuth->>Chain: read code at wallet address
alt our implementation present
EIPAuth-->>SendUI: null (no auth needed)
else other/absent
EIPAuth->>Kit: delegateSmartAccountToEoa(delegateImmediately=false)
Kit-->>EIPAuth: authorization
EIPAuth-->>SendUI: authorization
end
SendUI->>Kit: estimate/send (authorization?)
Kit->>Chain: estimate/send
Chain-->>Kit: userOpHash (chainGroups[0])
Kit-->>SendUI: result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–75 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
Deploying x with
|
| Latest commit: |
3f8aa18
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://66f2bd15.x-e62.pages.dev |
| Branch Preview URL: | https://feat-pro-3797-7702-onboardin.x-e62.pages.dev |
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3797/7702-onboarding)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
src/apps/the-exchange/index.tsx (1)
32-36: Consider adding defensive handling for unexpected wallet modes.The conditional provider selection assumes
walletModeis either'modular'or falls through to thegetWalletClient()path. IfwalletModecould be undefined or have unexpected values, consider adding validation.If additional safety is desired, you could add:
const { walletMode } = kit.getEtherspotProvider().getConfig(); + if (!walletMode) { + console.warn('Wallet mode not configured, defaulting to wallet client provider'); + } const provider = walletMode === 'modular' ? kit.getProvider() : kit.getEtherspotProvider().getWalletClient();src/providers/EIP7702Provider.tsx (1)
1-1: Consider memoizing the context value instead of disabling the linting rule.The inline object
{ eip7702Info }is recreated on every render. While this may not cause issues if the provider doesn't re-render frequently, memoizing the value follows React best practices and avoids the eslint-disable.Apply this refactor:
-/* eslint-disable react/jsx-no-constructed-context-values */ -import React, { createContext } from 'react'; +import React, { createContext, useMemo } from 'react'; // types import { EIP7702Info } from '../hooks/useWalletModeVerification';export const EIP7702Provider: React.FC<EIP7702ProviderProps> = ({ eip7702Info, children, }) => { + const value = useMemo(() => ({ eip7702Info }), [eip7702Info]); + return ( - <EIP7702Context.Provider value={{ eip7702Info }}> + <EIP7702Context.Provider value={value}> {children} </EIP7702Context.Provider> ); };Also applies to: 22-25
src/components/EIP7702UpgradeModal/EIP7702UpgradeProgressStep.tsx (1)
39-41: Consider simplifying the line status conditional.The ternary chain on line 40 is quite long and could be more readable.
const getLineClasses = () => { - return `w-0.5 h-4 ml-2 ${lineStatus === 'completed' || lineStatus === 'upgrading' || lineStatus === 'failed' ? 'bg-[#8A77FF]' : 'bg-[#121116]'}`; + const isActive = ['completed', 'upgrading', 'failed'].includes(lineStatus); + return `w-0.5 h-4 ml-2 ${isActive ? 'bg-[#8A77FF]' : 'bg-[#121116]'}`; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/assets/images/confirmed-icon.svgis excluded by!**/*.svgsrc/assets/images/failed-icon.svgis excluded by!**/*.svgsrc/assets/images/pending.svgis excluded by!**/*.svgsrc/assets/images/upgrade-diagonal-arrows.svgis excluded by!**/*.svgsrc/assets/images/upgrade-green-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (27)
package.json(1 hunks)src/apps/pillarx-app/components/WalletPortfolioButtons/WalletPortfolioButtons.tsx(3 hunks)src/apps/pillarx-app/index.tsx(4 hunks)src/apps/pillarx-app/reducer/WalletPortfolioSlice.ts(4 hunks)src/apps/pulse/components/Sell/PreviewSell.tsx(2 hunks)src/apps/pulse/components/Sell/Sell.tsx(1 hunks)src/apps/pulse/hooks/useGasEstimation.ts(2 hunks)src/apps/the-exchange/index.tsx(1 hunks)src/components/BottomMenuModal/SendModal/SendModalBatchesTabView.tsx(4 hunks)src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx(13 hunks)src/components/EIP7702UpgradeModal/EIP7702UpgradeAction.tsx(1 hunks)src/components/EIP7702UpgradeModal/EIP7702UpgradeDetails.tsx(1 hunks)src/components/EIP7702UpgradeModal/EIP7702UpgradeModal.tsx(1 hunks)src/components/EIP7702UpgradeModal/EIP7702UpgradeProgressStep.tsx(1 hunks)src/components/EIP7702UpgradeModal/EIP7702UpgradeStatus.tsx(1 hunks)src/components/EIP7702UpgradeModal/EIP7702UpgradeStatusButton.tsx(1 hunks)src/components/EIP7702UpgradeModal/EIP7702UpgradeStatusIcon.tsx(1 hunks)src/components/EIP7702UpgradeModal/types.ts(1 hunks)src/containers/Authorized.tsx(5 hunks)src/hooks/useEIP7702Info.tsx(1 hunks)src/hooks/useEIP7702Upgrade.tsx(1 hunks)src/hooks/useWalletModeVerification.tsx(1 hunks)src/providers/EIP7702Provider.tsx(1 hunks)src/services/walletConnect.ts(4 hunks)src/utils/common.ts(1 hunks)src/utils/eip7702Authorization.ts(1 hunks)src/utils/upgrade.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-20T09:14:16.888Z
Learnt from: RanaBug
PR: pillarwallet/x#374
File: src/apps/pillarx-app/index.tsx:12-12
Timestamp: 2025-08-20T09:14:16.888Z
Learning: In this codebase, Transaction Kit providers are set up at the container level (src/containers/Authorized.tsx), not at individual app component levels. App components like src/apps/pillarx-app/index.tsx are children that consume the context through the provider tree.
Applied to files:
src/providers/EIP7702Provider.tsxsrc/apps/the-exchange/index.tsxsrc/apps/pulse/hooks/useGasEstimation.tssrc/containers/Authorized.tsxsrc/apps/pillarx-app/index.tsx
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
PR: pillarwallet/x#391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Sell/Sell.tsx
🧬 Code graph analysis (20)
src/hooks/useEIP7702Info.tsx (1)
src/providers/EIP7702Provider.tsx (1)
EIP7702Context(11-11)
src/components/EIP7702UpgradeModal/EIP7702UpgradeStatus.tsx (2)
src/components/EIP7702UpgradeModal/types.ts (1)
UpgradeStatus(1-6)src/utils/upgrade.tsx (1)
getStatusConfig(103-105)
src/apps/pulse/components/Sell/PreviewSell.tsx (1)
src/utils/eip7702Authorization.ts (1)
getEIP7702AuthorizationIfNeeded(16-95)
src/components/EIP7702UpgradeModal/EIP7702UpgradeProgressStep.tsx (1)
src/components/EIP7702UpgradeModal/types.ts (1)
UpgradeStatus(1-6)
src/providers/EIP7702Provider.tsx (1)
src/hooks/useWalletModeVerification.tsx (1)
EIP7702Info(16-23)
src/components/EIP7702UpgradeModal/EIP7702UpgradeAction.tsx (1)
src/utils/number.tsx (1)
limitDigitsNumber(42-69)
src/components/EIP7702UpgradeModal/EIP7702UpgradeModal.tsx (5)
src/hooks/useTransactionDebugLogger.tsx (1)
useTransactionDebugLogger(1-15)src/components/EIP7702UpgradeModal/types.ts (2)
UpgradeStatus(1-6)UserOpUpgradeStatus(8-15)src/hooks/useEIP7702Upgrade.tsx (1)
useEIP7702Upgrade(27-181)src/services/userOpStatus.ts (1)
getUserOperationStatus(3-44)src/apps/pillarx-app/reducer/WalletPortfolioSlice.ts (2)
setHasCompletedEIP7702Upgrade(143-145)setIsEIP7702Eligible(140-142)
src/components/BottomMenuModal/SendModal/SendModalBatchesTabView.tsx (1)
src/utils/eip7702Authorization.ts (1)
getEIP7702AuthorizationIfNeeded(16-95)
src/components/EIP7702UpgradeModal/EIP7702UpgradeDetails.tsx (2)
src/components/EIP7702UpgradeModal/types.ts (1)
UpgradeStatus(1-6)src/utils/upgrade.tsx (1)
formatStepTimestamp(117-132)
src/apps/pillarx-app/components/WalletPortfolioButtons/WalletPortfolioButtons.tsx (1)
src/hooks/useEIP7702Upgrade.tsx (1)
useEIP7702Upgrade(27-181)
src/hooks/useEIP7702Upgrade.tsx (3)
src/hooks/useTransactionDebugLogger.tsx (1)
useTransactionDebugLogger(1-15)src/hooks/useEIP7702Info.tsx (1)
useEIP7702Info(6-12)src/apps/pillarx-app/reducer/WalletPortfolioSlice.ts (2)
setIsEIP7702Eligible(140-142)setIsUpgradeWalletModalOpen(137-139)
src/components/EIP7702UpgradeModal/EIP7702UpgradeStatusButton.tsx (2)
src/components/EIP7702UpgradeModal/types.ts (1)
UpgradeStatus(1-6)src/utils/upgrade.tsx (1)
getButtonConfig(110-112)
src/services/walletConnect.ts (1)
src/utils/walletConnect.ts (4)
PERSONAL_SIGN(4-4)ETH_SIGN_TYPED_DATA(8-8)ETH_SIGN_TYPED_DATA_V4(9-9)ETH_SEND_TX(6-6)
src/apps/pulse/hooks/useGasEstimation.ts (1)
src/utils/eip7702Authorization.ts (1)
getEIP7702AuthorizationIfNeeded(16-95)
src/containers/Authorized.tsx (3)
src/hooks/useWalletModeVerification.tsx (1)
useWalletModeVerification(37-191)src/providers/EtherspotTransactionKitProvider.tsx (1)
EtherspotTransactionKitProvider(30-73)src/providers/EIP7702Provider.tsx (1)
EIP7702Provider(18-27)
src/utils/upgrade.tsx (2)
src/apps/pulse/types/types.ts (1)
ButtonConfig(61-66)src/components/EIP7702UpgradeModal/types.ts (1)
UpgradeStatus(1-6)
src/hooks/useWalletModeVerification.tsx (2)
src/utils/blockchain.ts (1)
visibleChains(161-163)src/utils/common.ts (1)
sanitizeError(83-110)
src/components/EIP7702UpgradeModal/EIP7702UpgradeStatusIcon.tsx (2)
src/components/EIP7702UpgradeModal/types.ts (1)
UpgradeStatus(1-6)src/utils/upgrade.tsx (1)
getStatusConfig(103-105)
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (1)
src/utils/eip7702Authorization.ts (1)
getEIP7702AuthorizationIfNeeded(16-95)
src/apps/pillarx-app/index.tsx (3)
src/apps/pillarx-app/hooks/useReducerHooks.tsx (2)
useAppSelector(6-6)useAppDispatch(5-5)src/hooks/useEIP7702Upgrade.tsx (1)
useEIP7702Upgrade(27-181)src/apps/pillarx-app/reducer/WalletPortfolioSlice.ts (1)
setIsUpgradeWalletModalOpen(137-139)
🪛 Biome (2.1.2)
src/components/EIP7702UpgradeModal/EIP7702UpgradeAction.tsx
[error] 1-1: Do not shadow the global "isNaN" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (19)
src/utils/common.ts (1)
77-110: LGTM! Well-structured error sanitization utility.The
sanitizeErrorfunction properly handles multiple error types and safely redacts sensitive data using regex escaping. The try/catch aroundJSON.stringifyprevents potential circular reference errors.src/apps/pulse/components/Sell/Sell.tsx (1)
23-29: LGTM! Import reorganization only.The import statements have been reorganized to include
MobulaChainNamesandgetChainIdfrom the constants module. No logic changes.src/hooks/useEIP7702Info.tsx (1)
1-12: LGTM! Standard context hook pattern.The hook correctly throws an error when
EIP7702Contextis not provided, ensuring it's used within the proper provider tree.src/apps/pillarx-app/components/WalletPortfolioButtons/WalletPortfolioButtons.tsx (2)
5-6: LGTM! Clean integration of EIP-7702 upgrade hook.The
useEIP7702Upgradehook is properly imported and consumed to provide eligibility state and upgrade action.Also applies to: 21-21
49-60: LGTM! Upgrade button follows existing patterns.The Upgrade button is conditionally rendered based on eligibility and maintains consistent styling with the existing Receive button. The upward arrow (via
rotate-180) effectively communicates the "upgrade" action.src/apps/pillarx-app/reducer/WalletPortfolioSlice.ts (2)
35-37: LGTM! Clean addition of EIP-7702 upgrade state.The three new state fields are properly typed and initialized, following the existing pattern in the slice.
Also applies to: 62-64
137-145: LGTM! Standard Redux Toolkit reducer pattern.The new reducers and exported actions follow Redux Toolkit best practices with proper typing via
PayloadAction<boolean>.Also applies to: 168-170
package.json (1)
25-25: Package version verified as valid with no known security advisories.The @etherspot/transaction-kit@2.1.1 package version exists on the npm registry and no security vulnerabilities are currently recorded for this package. The upgrade is safe to proceed with.
src/components/EIP7702UpgradeModal/EIP7702UpgradeStatus.tsx (1)
1-35: LGTM!The component is well-structured with proper memoization and clean composition. The use of
getStatusConfigcentralizes status configuration logic effectively.src/apps/pulse/hooks/useGasEstimation.ts (1)
94-101: LGTM!The EIP-7702 authorization integration is properly implemented. Authorization is fetched after transaction building and correctly passed to the estimation call with appropriate fallback handling.
src/apps/pulse/components/Sell/PreviewSell.tsx (2)
373-380: LGTM!The authorization integration is consistent with the pattern used in
useGasEstimation.tsand properly handles the EIP-7702 flow.
384-386: Comment mentions PillarX but this is in Pulse app.The comment references PillarX batching behavior, but this file is in the Pulse app (
src/apps/pulse/...). While the batching logic may be shared, the comment could be misleading.Consider updating the comment:
- // In PillarX we only batch transactions per chainId, this is why sendBatch should only + // We only batch transactions per chainId, this is why sendBatch should only // have one chainGroup per batchAdditionally, the access to
chainGroups[0]assumes the array exists and has at least one element. Consider adding an optional chaining or validation:- const userOpHash = sentBatch?.chainGroups?.[0]?.userOpHash; + const userOpHash = sentBatch?.chainGroups?.[0]?.userOpHash;Actually, optional chaining is already present, so the access is safe. The comment fix is the main suggestion.
Likely an incorrect or invalid review comment.
src/components/EIP7702UpgradeModal/types.ts (1)
1-15: LGTM!The type definitions are clear and comprehensive, covering the complete lifecycle of the upgrade flow from ready to completed/failed states.
src/components/EIP7702UpgradeModal/EIP7702UpgradeDetails.tsx (2)
32-57: LGTM!The step status calculation logic correctly maps the overall upgrade status to individual step states, supporting a clear linear progression through the upgrade flow.
60-143: LGTM!The timestamp formatting logic and button behavior are well-designed. Timestamps are appropriately shown only for completed intermediate steps, and the "Done" button correctly handles both closing the details view and the entire modal based on the upgrade state.
src/components/EIP7702UpgradeModal/EIP7702UpgradeProgressStep.tsx (1)
54-127: LGTM!The timestamp display logic and responsive behavior are well-implemented. The component appropriately shows full timestamps on larger screens and time-only on smaller screens.
src/components/BottomMenuModal/SendModal/SendModalBatchesTabView.tsx (2)
169-254: LGTM!The EIP-7702 authorization integration is consistent with the patterns used in other components. Authorization is properly fetched once and reused for both estimation and sending operations.
294-296: LGTM!The comment accurately describes the batching behavior and the userOpHash extraction is safely implemented with optional chaining.
src/components/EIP7702UpgradeModal/EIP7702UpgradeAction.tsx (1)
38-136: LGTM!The component is well-structured with appropriate loading states, proper button disabled states, and clear visual feedback for gas checking and insufficient gas scenarios.
| // Check if user is eligible for EIP-7702 upgrade (when walletAddress loads on login) | ||
| const handleCheckEligibility = useCallback(() => { | ||
| if (walletAddress) { | ||
| checkOnLogin(); | ||
| } | ||
| }, [walletAddress, checkOnLogin]); | ||
|
|
||
| useEffect(() => { | ||
| handleCheckEligibility(); | ||
| }, [handleCheckEligibility]); | ||
|
|
||
| const handleCloseModal = () => { | ||
| dispatch(setIsUpgradeWalletModalOpen(false)); | ||
| }; |
There was a problem hiding this comment.
Prevent modal eligibility check from wiping upgrade gas data
Because the effect depends on handleCheckEligibility, it fires every time checkOnLogin changes identity (e.g., whenever isUpgradeWalletModalOpen flips inside useEIP7702Upgrade). That re-enters checkOnLogin(), which immediately runs checkEligibilityAndShowModal, hits setGasUpgradeInfo(undefined), and then returns early because the modal is already open. The modal stays visible but its gas info is cleared, so users lose the freshly fetched upgrade cost each time the modal opens. Apply this diff to guard the effect on wallet-address changes only:
- const handleCheckEligibility = useCallback(() => {
- if (walletAddress) {
- checkOnLogin();
- }
- }, [walletAddress, checkOnLogin]);
-
- useEffect(() => {
- handleCheckEligibility();
- }, [handleCheckEligibility]);
+ const lastCheckedWalletRef = useRef<string | null>(null);
+
+ useEffect(() => {
+ if (!walletAddress || lastCheckedWalletRef.current === walletAddress) {
+ return;
+ }
+
+ lastCheckedWalletRef.current = walletAddress;
+ checkOnLogin();
+ }, [walletAddress, checkOnLogin]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check if user is eligible for EIP-7702 upgrade (when walletAddress loads on login) | |
| const handleCheckEligibility = useCallback(() => { | |
| if (walletAddress) { | |
| checkOnLogin(); | |
| } | |
| }, [walletAddress, checkOnLogin]); | |
| useEffect(() => { | |
| handleCheckEligibility(); | |
| }, [handleCheckEligibility]); | |
| const handleCloseModal = () => { | |
| dispatch(setIsUpgradeWalletModalOpen(false)); | |
| }; | |
| // Check if user is eligible for EIP-7702 upgrade (when walletAddress loads on login) | |
| const lastCheckedWalletRef = useRef<string | null>(null); | |
| useEffect(() => { | |
| if (!walletAddress || lastCheckedWalletRef.current === walletAddress) { | |
| return; | |
| } | |
| lastCheckedWalletRef.current = walletAddress; | |
| checkOnLogin(); | |
| }, [walletAddress, checkOnLogin]); | |
| const handleCloseModal = () => { | |
| dispatch(setIsUpgradeWalletModalOpen(false)); | |
| }; |
🤖 Prompt for AI Agents
In src/apps/pillarx-app/index.tsx around lines 180–193, the useEffect currently
depends on handleCheckEligibility which causes checkOnLogin to rerun whenever
checkOnLogin identity changes (e.g. when isUpgradeWalletModalOpen flips),
clearing gas info; change the effect so it only watches walletAddress: replace
the effect dependency with [walletAddress] and inside the effect call
checkOnLogin() only when walletAddress is truthy, ensuring checkOnLogin is not
triggered by modal state changes and thus preventing
setGasUpgradeInfo(undefined) from wiping the fetched gas data.
There was a problem hiding this comment.
Tested behaviour and how it is works as well as suggestion
There was a problem hiding this comment.
@RanaBug, thank you for testing both the current behavior and the suggested fix! I'm glad to hear the solution works as expected. The ref-based approach will ensure the eligibility check only runs when the wallet address actually changes, preventing the gas data from being cleared unnecessarily when the modal state updates.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
| const authorization = await getEIP7702AuthorizationIfNeeded( | ||
| kit, | ||
| chainIdToUse | ||
| ); | ||
|
|
||
| // Estimate (no paymasterDetails) | ||
| transactionDebugLog('Estimating single payload transaction'); | ||
| const estimated = await kit.estimate(); | ||
| const estimated = await kit.estimate({ | ||
| authorization: authorization || undefined, | ||
| }); |
There was a problem hiding this comment.
Provide a fallback chainId for single-transaction payloads
Here chainIdToUse comes straight from payload.transaction. That field is optional in our SendModalData (the rest of this file already falls back to etherspotDefaultChainId when it’s missing), so passing undefined into getEIP7702AuthorizationIfNeeded makes delegateSmartAccountToEoa throw and the WalletConnect flow fails. Please default chainIdToUse to something like payloadTx?.chainId ?? etherspotDefaultChainId (and use the same value for the authorization call) before invoking the helper.
🤖 Prompt for AI Agents
In src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx around
lines 1387 to 1396, the code passes chainIdToUse directly from
payload.transaction (which can be undefined) into
getEIP7702AuthorizationIfNeeded causing delegateSmartAccountToEoa to throw;
ensure you compute a fallback first (e.g. const chainIdToUse =
payloadTx?.chainId ?? etherspotDefaultChainId) and then pass that resolved value
into getEIP7702AuthorizationIfNeeded (and use the same resolved chainId for any
subsequent calls like kit.estimate) so undefined is never forwarded.
There was a problem hiding this comment.
Fixed (see comment above)
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the current state of the chainIdToUse assignment in the PR branch
# Look for the line where chainIdToUse is defined in the payload transaction section
echo "=== Checking chainIdToUse assignment around line 1375 ==="
sed -n '1370,1395p' src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx | grep -A 5 -B 5 "chainIdToUse"
echo ""
echo "=== Checking if etherspotDefaultChainId is used as fallback ==="
sed -n '1370,1395p' src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx | grep "etherspotDefaultChainId"
Length of output: 908
🏁 Script executed:
#!/bin/bash
# Check recent commits to see if there's a fix that might not be in the current state
git log --oneline -10 --all -- src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx
echo ""
echo "=== Checking the most recent commit for this file ==="
git log -1 --stat -- src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx
Length of output: 937
🏁 Script executed:
#!/bin/bash
# Check the specific commit that mentions "resolution of single transaction chainId"
echo "=== Checking commit 2bb7eed for the chainId fix ==="
git show 2bb7eed:src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx | sed -n '1370,1395p' | grep -A 5 -B 5 "chainIdToUse"
echo ""
echo "=== Checking current HEAD for chainIdToUse assignment ==="
git show HEAD:src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx | sed -n '1370,1395p' | grep -A 3 -B 3 "chainIdToUse"
Length of output: 651
🏁 Script executed:
#!/bin/bash
# Check a broader range around the payload transaction section
echo "=== Looking at lines 1350-1410 to see full payload transaction section ==="
sed -n '1350,1410p' src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx
echo ""
echo "=== Searching for all uses of etherspotDefaultChainId in payload section ==="
sed -n '1300,1500p' src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx | grep -n "etherspotDefaultChainId"
Length of output: 2201
@RanaBug, thanks for the update! However, I've checked the current code on the PR branch and the fix doesn't appear to be applied yet.
Here's what I'm seeing at line 1375:
const chainIdToUse = payloadTx?.chainId;This chainIdToUse can still be undefined because payloadTx?.chainId is optional. This undefined value is then passed to:
kit.transaction({ chainId: chainIdToUse, ...})on line 1377getEIP7702AuthorizationIfNeeded(kit, chainIdToUse)on line 1387
The needed fix:
const chainIdToUse = payloadTx?.chainId ?? etherspotDefaultChainId;This ensures chainIdToUse always has a valid number value before being passed to the authorization helper and kit.transaction. The same pattern is already used elsewhere in this file (e.g., line 1198 for regular sends and line 1912 for batch adds).
Could you please apply this fallback? Let me know if you've already pushed this change and I'm looking at an outdated version! 🙏
| useEffect(() => { | ||
| if (provider && chainId) { | ||
| const tempKitConfig = { | ||
| provider, | ||
| chainId, | ||
| privateKey, | ||
| bundlerApiKey: import.meta.env.VITE_ETHERSPOT_BUNDLER_API_KEY, | ||
| walletMode: 'modular' as const, // Always start with modular for verification | ||
| } as unknown as EtherspotTransactionKitConfig; | ||
|
|
||
| const kit = new EtherspotTransactionKit(tempKitConfig); | ||
| setTempKit(kit); | ||
| } | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [provider, privateKey]); |
There was a problem hiding this comment.
Recreate the temp kit when chainId changes
useEffect depends only on provider and privateKey, yet its body also reads chainId. When the user switches networks the hook never reruns, so tempKit stays bound to the previous chain and useWalletModeVerification continues working with stale data. That leaves walletMode/eip7702Info incorrect and can block delegated-EOA flows on chain switches. Please add chainId to the dependency list (and clear the temp kit when either input becomes falsy) so the verification kit is rebuilt for the new chain.
🤖 Prompt for AI Agents
In src/containers/Authorized.tsx around lines 63 to 77, the useEffect currently
lists [provider, privateKey] but also reads chainId, so it fails to recreate the
tempKit when the user switches networks; update the effect to include chainId in
the dependency array and ensure that when any of provider, chainId, or
privateKey are falsy you clear tempKit (setTempKit(undefined)) instead of
creating it, otherwise build the EtherspotTransactionKit with the provider,
chainId, privateKey, bundlerApiKey and walletMode as before.
There was a problem hiding this comment.
This is on purpose that we do not include the chainId in the dependency list, this is because this effect should only run if the private key or provider changes. Indeed this useEffect triggers a tempkit that is used only to do some checks on all supported chains regardless of the chosen chainId.
There was a problem hiding this comment.
@RanaBug, thank you for the clarification! I understand now—since useWalletModeVerification performs checks across all supported chains internally, the chainId passed to tempKit is only used as an initial configuration and doesn't affect the verification results. There's no need to recreate the kit when the user switches networks. I've noted this design decision for future reviews.
🐰✨
✏️ Learnings added
Learnt from: RanaBug
Repo: pillarwallet/x PR: 445
File: src/containers/Authorized.tsx:63-77
Timestamp: 2025-11-04T14:34:00.341Z
Learning: In src/containers/Authorized.tsx, the tempKit useEffect intentionally excludes chainId from its dependency array because the kit is used by useWalletModeVerification to check wallet mode across all supported chains, regardless of the currently selected chainId. The chainId parameter is only used for initial kit configuration and does not affect the multi-chain verification results.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: RanaBug
Repo: pillarwallet/x PR: 374
File: src/apps/pillarx-app/index.tsx:12-12
Timestamp: 2025-08-20T09:14:16.888Z
Learning: In this codebase, Transaction Kit providers are set up at the container level (src/containers/Authorized.tsx), not at individual app component levels. App components like src/apps/pillarx-app/index.tsx are children that consume the context through the provider tree.
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Learnt from: RanaBug
Repo: pillarwallet/x PR: 320
File: src/components/BottomMenuModal/HistoryModal/TransactionInfo.tsx:177-182
Timestamp: 2025-05-28T14:30:02.702Z
Learning: In the transaction history tracking feature, chain ID validation is performed earlier in the flow before the values reach the TransactionInfo component, so additional validation at the display level is not needed.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (11)
src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeAction.test.tsx (4)
40-45: Consider the value of snapshot testing.Snapshot tests can become brittle and may not catch meaningful regressions if not carefully reviewed during updates. Consider whether integration or specific assertion-based tests provide better value for this component.
84-85: Fragile CSS selector for checkmark verification.The selector
[class*="bg-"]is overly broad and matches any element with "bg-" in its class name, which could include backgrounds, borders, or unrelated styling. Consider usingdata-testidattributes on checkmark icons or more specific ARIA roles for reliable verification.Apply this approach:
- const checkIcons = container.querySelectorAll('[class*="bg-"]'); - expect(checkIcons.length).toBeGreaterThanOrEqual(3); + const checkIcons = container.querySelectorAll('[data-testid="benefit-checkmark"]'); + expect(checkIcons).toHaveLength(3);
121-126: Potential duplicate test.This test appears to duplicate the assertion at lines 106-112, which already verifies that the button is disabled when
hasEnoughGas={false}.
204-220: Tests tightly coupled to specific color values.Using CSS selectors like
.bg-\\[\\#8A77FF\\]tightly couples tests to exact hex colors. Theme or design updates will break these tests even if the component functions correctly. Consider testing the enabled/disabled state through accessibility attributes or behavior instead of visual styling.it('applies active button styles when enabled', () => { - const { container } = render(<EIP7702UpgradeAction {...baseProps} />); + render(<EIP7702UpgradeAction {...baseProps} />); - const button = container.querySelector('.bg-\\[\\#8A77FF\\]'); - expect(button).toBeInTheDocument(); + const button = screen.getByText('Upgrade now').closest('button'); + expect(button).not.toBeDisabled(); + expect(button).toHaveAttribute('aria-disabled', 'false'); });src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeStatusButton.test.tsx (2)
88-102: Consider consolidating duplicate tests for upgrading and submitted statuses.Both tests check identical behavior since the mocked
getButtonConfigreturns the same default configuration forupgradingandsubmittedstatuses. Unless these statuses have distinct UI behaviors in the actual component, consider using a parameterized test or removing one of them to reduce maintenance overhead.Example using
it.each:- it('renders with upgrading status', () => { - render(<EIP7702UpgradeStatusButton {...baseProps} status="upgrading" />); - - expect(screen.getByTestId('animated-button')).toBeInTheDocument(); - expect(screen.getByText('View Details')).toBeInTheDocument(); - expect(screen.getByTestId('tail-spin')).toBeInTheDocument(); - }); - - it('renders with submitted status', () => { - render(<EIP7702UpgradeStatusButton {...baseProps} status="submitted" />); - - expect(screen.getByTestId('animated-button')).toBeInTheDocument(); - expect(screen.getByText('View Details')).toBeInTheDocument(); - expect(screen.getByTestId('tail-spin')).toBeInTheDocument(); - }); + it.each(['upgrading', 'submitted'] as const)( + 'renders with %s status showing loading state', + (status) => { + render(<EIP7702UpgradeStatusButton {...baseProps} status={status} />); + + expect(screen.getByTestId('animated-button')).toBeInTheDocument(); + expect(screen.getByText('View Details')).toBeInTheDocument(); + expect(screen.getByTestId('tail-spin')).toBeInTheDocument(); + } + );
161-165: Remove unnecessary negative test.This test verifies that
onClickis not called when the button is not clicked, which is self-evident and adds no coverage value. The test on lines 152-159 already confirms the onClick behavior works correctly.Apply this diff to remove the unnecessary test:
- it('does not call onClick when button is not clicked', () => { - render(<EIP7702UpgradeStatusButton {...baseProps} />); - - expect(baseProps.onClick).not.toHaveBeenCalled(); - });src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeStatusIcon.test.tsx (3)
70-96: Consider usingtest.eachto reduce duplication.The rendering tests follow an identical pattern across four statuses. While the current approach is clear, you could make it more maintainable with parameterized tests.
Example refactor:
- it('renders with upgrading status', () => { - render(<EIP7702UpgradeStatusIcon status="upgrading" />); - - expect(screen.getByTestId('animated-div')).toBeInTheDocument(); - expect(screen.getByAltText('upgrading')).toBeInTheDocument(); - }); - - it('renders with submitted status', () => { - render(<EIP7702UpgradeStatusIcon status="submitted" />); - - expect(screen.getByTestId('animated-div')).toBeInTheDocument(); - expect(screen.getByAltText('submitted')).toBeInTheDocument(); - }); - - it('renders with completed status', () => { - render(<EIP7702UpgradeStatusIcon status="completed" />); - - expect(screen.getByTestId('animated-div')).toBeInTheDocument(); - expect(screen.getByAltText('completed')).toBeInTheDocument(); - }); - - it('renders with failed status', () => { - render(<EIP7702UpgradeStatusIcon status="failed" />); - - expect(screen.getByTestId('animated-div')).toBeInTheDocument(); - expect(screen.getByAltText('failed')).toBeInTheDocument(); - }); + it.each(['upgrading', 'submitted', 'completed', 'failed'] as const)( + 'renders with %s status', + (status) => { + render(<EIP7702UpgradeStatusIcon status={status} />); + + expect(screen.getByTestId('animated-div')).toBeInTheDocument(); + expect(screen.getByAltText(status)).toBeInTheDocument(); + } + );
100-134: Consider using Testing Library queries instead ofquerySelector.While
container.querySelector('img')works, Testing Library'sscreen.getByRole('img')is more idiomatic and provides better error messages.Example for one test:
it('shows confirmed icon for completed status', () => { - const { container } = render( + render( <EIP7702UpgradeStatusIcon status="completed" /> ); - const img = container.querySelector('img'); - expect(img?.getAttribute('alt')).toBe('completed'); + expect(screen.getByRole('img')).toHaveAttribute('alt', 'completed'); });
160-162: Theact()wrapper is unnecessary here.React Testing Library automatically wraps state updates in
act(), so explicitly wrapping a synchronousrerendercall is redundant. While not harmful, it adds visual noise.- act(() => { - rerender(<EIP7702UpgradeStatusIcon status="completed" />); - }); + rerender(<EIP7702UpgradeStatusIcon status="completed" />);src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeModal.test.tsx (2)
175-184: Replace tautological assertion with a real upgrade verificationThis test currently ends with
expect(true).toBe(true), so it never fails even if the upgrade button stops wiring throughhandleUpgradeClick. Please stub the hook with a dedicatedhandleUpgradeClickmock (e.g. viavi.mocked(useEIP7702Upgrade).mockReturnValueOnce({ …handleUpgradeClick })), click the button, and assert that the mock was invoked. That keeps the test meaningful and surfaces regressions in the upgrade trigger path.
191-209: Keep hook stubs isolated per testUsing
.mockReturnValuehere mutates the globaluseEIP7702Upgrademock, andvi.clearAllMocks()doesn’t restore implementations. Subsequent tests therefore inherit this altered return shape, which hides ordering bugs. Prefer.mockReturnValueOnce(...)(or re-seeding the default implementation inbeforeEach/ callingvi.resetAllMocks()) so every test runs with the expected hook state in isolation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/components/EIP7702UpgradeModal/tests/__snapshots__/EIP7702UpgradeAction.test.tsx.snapis excluded by!**/*.snapsrc/components/EIP7702UpgradeModal/tests/__snapshots__/EIP7702UpgradeDetails.test.tsx.snapis excluded by!**/*.snapsrc/components/EIP7702UpgradeModal/tests/__snapshots__/EIP7702UpgradeModal.test.tsx.snapis excluded by!**/*.snapsrc/components/EIP7702UpgradeModal/tests/__snapshots__/EIP7702UpgradeProgressStep.test.tsx.snapis excluded by!**/*.snapsrc/components/EIP7702UpgradeModal/tests/__snapshots__/EIP7702UpgradeStatus.test.tsx.snapis excluded by!**/*.snapsrc/components/EIP7702UpgradeModal/tests/__snapshots__/EIP7702UpgradeStatusButton.test.tsx.snapis excluded by!**/*.snapsrc/components/EIP7702UpgradeModal/tests/__snapshots__/EIP7702UpgradeStatusIcon.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (10)
package.json(1 hunks)src/apps/pulse/components/Sell/tests/PreviewSell.test.tsx(2 hunks)src/apps/the-exchange/index.tsx(1 hunks)src/components/EIP7702UpgradeModal/EIP7702UpgradeDetails.tsx(1 hunks)src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeAction.test.tsx(1 hunks)src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeDetails.test.tsx(1 hunks)src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeModal.test.tsx(1 hunks)src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeProgressStep.test.tsx(1 hunks)src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeStatusButton.test.tsx(1 hunks)src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeStatusIcon.test.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/apps/the-exchange/index.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeModal.test.tsx (1)
src/hooks/useEIP7702Upgrade.tsx (1)
useEIP7702Upgrade(27-181)
src/components/EIP7702UpgradeModal/EIP7702UpgradeDetails.tsx (2)
src/components/EIP7702UpgradeModal/types.ts (1)
UpgradeStatus(1-6)src/utils/upgrade.tsx (1)
formatStepTimestamp(117-132)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: unit-tests
🔇 Additional comments (10)
src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeAction.test.tsx (4)
1-32: Test setup looks good.The mocks and base props are well-structured and provide a solid foundation for the test suite.
148-155: Verify disabled button click behavior.Browsers typically don't fire click events on disabled buttons, but React Testing Library's
fireEventmay artificially trigger events on disabled elements. This test might pass regardless of whether proper event handling is implemented. Consider usinguserEvent.click()from@testing-library/user-event, which more accurately simulates real user behavior and respects disabled states.+import userEvent from '@testing-library/user-event'; it('does not call handleUpgrade when button is disabled', () => { render(<EIP7702UpgradeAction {...baseProps} hasEnoughGas={false} />); const button = screen.getByText('Not enough gas'); - fireEvent.click(button); + userEvent.click(button); expect(baseProps.handleUpgrade).not.toHaveBeenCalled(); });
158-187: Edge cases are well covered.The tests appropriately verify handling of missing, invalid, and empty gas fee estimates, ensuring the UI gracefully degrades.
189-202: Good defensive testing.Testing with minimal required props ensures the component handles missing optional props gracefully.
package.json (1)
25-25: Verify that @etherspot/transaction-kit 2.1.1 is installable and does not conflict with known vulnerabilities.The package-lock.json references v2.1.1 with a valid npm registry URL and integrity hash, but web search found only v2.0.3 publicly available. The codebase's audit-report.json also flags @etherspot/transaction-kit with a moderate severity vulnerability requiring a semver-major fix. Confirm this version bump is intentional, that v2.1.1 is genuinely available for installation, and that it addresses (or is unaffected by) the documented vulnerabilities.
src/apps/pulse/components/Sell/tests/PreviewSell.test.tsx (2)
66-70: No action required — PreviewSell already correctly handles the nested chainGroups structure.The component correctly extracts
userOpHashfrom the nested structure using optional chaining (chainGroups?.[0]?.userOpHash) at line 369 of PreviewSell.tsx, which matches the test data format and includes proper guards before usage.
47-49: No changes needed – mock is correct and necessary.The
getWalletModemock is essential because thegetEIP7702AuthorizationIfNeededutility checks wallet mode early and returnsundefinedif it's not'delegatedEoa'. PreviewSell invokes this utility inexecuteSellDirectly(), so the mock must provide this method.The hardcoded
'modular'value is appropriate here: the PreviewSell component itself contains no wallet-mode-conditional logic. It passes the authorization result tokit.sendBatches()regardless of mode. If testing different wallet mode behaviors for EIP-7702 authorization is desired, that belongs in dedicated tests forgetEIP7702AuthorizationIfNeeded, not in the PreviewSell component test.src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeStatusButton.test.tsx (1)
64-67: The useSpring mock is correct and matches actual component usage.The component destructures
useSpringinto[springs, api], usesspringsas a style object at line 99, and callsapi.set()andapi.start()at lines 45-50. The mock returns exactly this structure with appropriate implementations for the actual methods used in the component.src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeStatusIcon.test.tsx (2)
1-7: LGTM!Standard test setup with appropriate imports and component under test.
34-59: LGTM!SVG asset mocks and @react-spring/web mock are properly structured for testing. The
useSpringtuple return matches the API form when used with a function argument.
src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeStatusButton.test.tsx
Outdated
Show resolved
Hide resolved
| vi.mock('../../assets/images/confirmed-icon.svg', () => ({ | ||
| default: 'confirmed-icon.svg', | ||
| })); | ||
|
|
||
| vi.mock('../../assets/images/failed-icon.svg', () => ({ | ||
| default: 'failed-icon.svg', | ||
| })); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the SVG asset mock paths.
The mock paths for SVG assets use '../../assets/images/', which would resolve to src/components/assets/images/. Typically, assets are located at src/assets/. If so, the correct paths should be '../../../assets/images/confirmed-icon.svg' and '../../../assets/images/failed-icon.svg'.
Run this script to verify the asset locations:
🏁 Script executed:
#!/bin/bash
# Find the actual location of the icon files
fd -t f "confirmed-icon.svg" -e svg
fd -t f "failed-icon.svg" -e svgLength of output: 149
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the component file and check its imports
fd -t f "EIP7702UpgradeStatusButton" --type tsx --type ts
# Also look for what files import from assets/images
rg -n "confirmed-icon|failed-icon" --type tsx --type ts -C2
Length of output: 198
---
🏁 Script executed:
```shell
#!/bin/bash
# Find EIP7702UpgradeStatusButton component file
fd "EIP7702UpgradeStatusButton" -e tsx -e ts
# Search for imports of these icons across the codebase
rg "confirmed-icon\.svg|failed-icon\.svg" -n -C1
Length of output: 4295
Update mock paths and component imports to correct asset location.
The SVG imports in both the component (src/components/EIP7702UpgradeModal/EIP7702UpgradeStatusButton.tsx, lines 6-7) and the test file use ../../assets/images/, which resolves to the non-existent src/components/assets/images/. The actual SVGs are at src/assets/images/.
Update both files to use ../../../assets/images/:
- Component: lines 6-7
- Test: lines 41 and 45
🤖 Prompt for AI Agents
In src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeStatusButton.test.tsx
around lines 41 to 47 and in
src/components/EIP7702UpgradeModal/EIP7702UpgradeStatusButton.tsx around lines 6
to 7, the SVG import paths point to ../../assets/images/ which resolves
incorrectly; update both import/mock paths to ../../../assets/images/ so they
reference src/assets/images/ (change the two mock paths in the test at lines 41
and 45 and the two SVG imports in the component at lines 6-7).
src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeStatusButton.test.tsx
Show resolved
Hide resolved
src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeStatusButton.test.tsx
Show resolved
Hide resolved
src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeStatusIcon.test.tsx
Outdated
Show resolved
Hide resolved
src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeStatusIcon.test.tsx
Show resolved
Hide resolved
github-actions
bot
requested a deployment
to
Preview (feat/PRO-3797/7702-onboarding)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/services/walletConnect.ts(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/services/walletConnect.ts (1)
src/utils/walletConnect.ts (4)
PERSONAL_SIGN(4-4)ETH_SIGN_TYPED_DATA(8-8)ETH_SIGN_TYPED_DATA_V4(9-9)ETH_SEND_TX(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: unit-tests
github-actions
bot
requested a deployment
to
Preview (feat/PRO-3797/7702-onboarding)
github-actions
bot
requested a deployment
to
Preview (feat/PRO-3797/7702-onboarding)
github-actions
bot
requested a deployment
to
Preview (feat/PRO-3797/7702-onboarding)
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3797/7702-onboarding)
IAmKio
left a comment
IAmKio
left a comment
There was a problem hiding this comment.
Just two small changes and a few non-blocking questions but other than that - looking good!
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3797/7702-onboarding)
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (1)
1383-1399: Restore the chainId fallback for payload transactions.
Line [1388] now fails whenpayloadTx.chainIdis missing, but that field is optional inSendModalData. WalletConnect payloads routinely omit it, so instead of falling back toetherspotDefaultChainIdwe now abort the flow.- const chainIdToUse = payloadTx?.chainId; + const chainIdToUse = + payloadTx?.chainId ?? etherspotDefaultChainId;
♻️ Duplicate comments (3)
src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeStatusIcon.test.tsx (1)
138-151: Memoization test doesn't verify actual memoization behavior.This test checks that the element remains in the document after rerender but doesn't prove React.memo prevents re-renders. The test would pass even without memoization. Consider renaming to "renders consistently with identical props" or adding a comment noting the limitation.
src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeStatusButton.test.tsx (2)
168-195: Button styling tests only verify existence, not correctness.The styling tests use
container.querySelector('[class*="bg-"]')which only confirms some element has a background class, not that the button itself has the correct status-specific classes. Consider asserting specific classes on the button element.
198-211: Memoization test doesn't verify actual memoization.This test rerenders with identical props and checks the element is still present, but doesn't prove memoization prevents re-renders. Consider renaming to accurately reflect what's tested or add a comment noting the limitation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/apps/pillarx-app/components/TokenMarketDataRow/tests/__snapshots__/LeftColumnTokenMarketDataRow.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/TokensWithMarketDataTile/test/__snapshots__/TokensWithMarketDataTile.test.tsx.snapis excluded by!**/*.snapsrc/components/EIP7702UpgradeModal/tests/__snapshots__/EIP7702UpgradeAction.test.tsx.snapis excluded by!**/*.snapsrc/components/EIP7702UpgradeModal/tests/__snapshots__/EIP7702UpgradeStatusIcon.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (16)
__mocks__/useTransactionKit.ts(1 hunks)src/apps/pillarx-app/components/PortfolioOverview/PortfolioOverview.tsx(2 hunks)src/apps/pillarx-app/components/WalletPortfolioButtons/WalletPortfolioButtons.tsx(4 hunks)src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx(15 hunks)src/components/EIP7702UpgradeModal/EIP7702UpgradeAction.tsx(1 hunks)src/components/EIP7702UpgradeModal/EIP7702UpgradeDetails.tsx(1 hunks)src/components/EIP7702UpgradeModal/EIP7702UpgradeStatus.tsx(1 hunks)src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeAction.test.tsx(1 hunks)src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeDetails.test.tsx(1 hunks)src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeStatusButton.test.tsx(1 hunks)src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeStatusIcon.test.tsx(1 hunks)src/hooks/useEIP7702Upgrade.tsx(1 hunks)src/hooks/useWalletModeVerification.tsx(1 hunks)src/services/walletConnect.ts(9 hunks)src/utils/eip7702Authorization.ts(1 hunks)src/utils/upgrade.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/EIP7702UpgradeModal/EIP7702UpgradeDetails.tsx
- src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeAction.test.tsx
- src/components/EIP7702UpgradeModal/EIP7702UpgradeAction.tsx
- src/apps/pillarx-app/components/WalletPortfolioButtons/WalletPortfolioButtons.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-04T14:34:00.341Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 445
File: src/containers/Authorized.tsx:63-77
Timestamp: 2025-11-04T14:34:00.341Z
Learning: In src/containers/Authorized.tsx, the tempKit useEffect intentionally excludes chainId from its dependency array because the kit is used by useWalletModeVerification to check wallet mode across all supported chains, regardless of the currently selected chainId. The chainId parameter is only used for initial kit configuration and does not affect the multi-chain verification results.
Applied to files:
src/apps/pillarx-app/components/PortfolioOverview/PortfolioOverview.tsxsrc/services/walletConnect.tssrc/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsxsrc/hooks/useWalletModeVerification.tsxsrc/hooks/useEIP7702Upgrade.tsxsrc/utils/eip7702Authorization.ts
📚 Learning: 2025-08-20T09:14:16.888Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 374
File: src/apps/pillarx-app/index.tsx:12-12
Timestamp: 2025-08-20T09:14:16.888Z
Learning: In this codebase, Transaction Kit providers are set up at the container level (src/containers/Authorized.tsx), not at individual app component levels. App components like src/apps/pillarx-app/index.tsx are children that consume the context through the provider tree.
Applied to files:
src/apps/pillarx-app/components/PortfolioOverview/PortfolioOverview.tsx
📚 Learning: 2025-05-28T14:30:02.702Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 320
File: src/components/BottomMenuModal/HistoryModal/TransactionInfo.tsx:177-182
Timestamp: 2025-05-28T14:30:02.702Z
Learning: In the transaction history tracking feature, chain ID validation is performed earlier in the flow before the values reach the TransactionInfo component, so additional validation at the display level is not needed.
Applied to files:
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsxsrc/hooks/useEIP7702Upgrade.tsx
🧬 Code graph analysis (6)
src/utils/upgrade.tsx (2)
src/apps/pulse/types/types.ts (1)
ButtonConfig(61-66)src/components/EIP7702UpgradeModal/types.ts (1)
UpgradeStatus(1-6)
src/services/walletConnect.ts (2)
src/utils/walletConnect.ts (4)
PERSONAL_SIGN(4-4)ETH_SIGN_TYPED_DATA(8-8)ETH_SIGN_TYPED_DATA_V4(9-9)ETH_SEND_TX(6-6)src/apps/deposit/utils/blockchain.tsx (1)
getNetworkViem(71-94)
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (2)
src/utils/blockchain.ts (1)
supportedChains(157-159)src/utils/eip7702Authorization.ts (1)
getEIP7702AuthorizationIfNeeded(16-95)
src/components/EIP7702UpgradeModal/EIP7702UpgradeStatus.tsx (2)
src/components/EIP7702UpgradeModal/types.ts (1)
UpgradeStatus(1-6)src/utils/upgrade.tsx (1)
getStatusConfig(101-103)
src/hooks/useWalletModeVerification.tsx (3)
src/utils/blockchain.ts (1)
visibleChains(161-163)src/utils/eip7702Authorization.ts (1)
OUR_EIP7702_IMPLEMENTATION_ADDRESS(5-6)src/utils/common.ts (1)
sanitizeError(83-110)
src/hooks/useEIP7702Upgrade.tsx (4)
src/hooks/useTransactionDebugLogger.tsx (1)
useTransactionDebugLogger(1-15)src/hooks/useEIP7702Info.tsx (1)
useEIP7702Info(6-12)src/apps/pillarx-app/reducer/WalletPortfolioSlice.ts (2)
setIsEIP7702Eligible(140-142)setIsUpgradeWalletModalOpen(137-139)src/apps/deposit/utils/blockchain.tsx (1)
getNetworkViem(71-94)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (12)
src/services/walletConnect.ts (4)
1178-1205: LGTM! Consistent wallet mode handling for typed data signing.The branching logic for delegatedEoa vs modular mode is clean and consistent with the PERSONAL_SIGN flow. Account validation is properly in place.
1208-1250: LGTM! Transaction handling preserves existing flow.The ETH_SEND_TX handling maintains the established pattern for both approval and regular transactions. Approval detection via function selector
0x095ea7b3is standard.
1261-1271: Good defensive error handling.The nested try-catch ensures cleanup happens even when the error response itself fails, preventing cascading relay failures from leaving the app in an inconsistent state.
1333-1339: LGTM! Graceful handling of relayer errors.The new
handleUnknownEventhandler appropriately logs relayer errors without disrupting the session lifecycle. These are typically benign events from expired sessions or incompatible dApps.Also applies to: 1350-1353, 1368-1373
__mocks__/useTransactionKit.ts (1)
15-15: LGTM! Mock correctly extends provider API.The
getWalletModemock returning'modular'properly supports the new wallet mode detection used throughout the EIP-7702 flows.src/apps/pillarx-app/components/PortfolioOverview/PortfolioOverview.tsx (1)
36-54: LGTM! Correct wallet mode check for dropdown visibility.The logic properly hides the WalletConnect dropdown in
delegatedEoamode, aligning with the two-mode architecture. The explanatory comment is helpful.src/components/EIP7702UpgradeModal/EIP7702UpgradeStatus.tsx (1)
1-35: LGTM! Clean composition pattern.The component cleanly composes the icon, label, and button elements using the status config. The structure is straightforward and maintainable.
src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeStatusIcon.test.tsx (1)
1-167: Comprehensive test coverage for icon component.The test suite thoroughly covers rendering across all statuses, icon selection, and prop changes. The mocks are properly configured and tests are well-structured.
src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeStatusButton.test.tsx (1)
1-227: Thorough test coverage for button component.The test suite comprehensively covers rendering, icon states, and user interactions across all statuses. The mocks properly simulate dependencies.
src/utils/upgrade.tsx (2)
20-96: LGTM! Well-structured config mapping.The STATUS_CONFIG and BUTTON_CONFIG provide clean, centralized configuration for all upgrade statuses. The color scheme is consistent and the Tailwind classes are properly formatted.
98-110: LGTM! Simple and effective getter functions.The getter functions provide type-safe access to the configuration mappings.
src/components/EIP7702UpgradeModal/tests/EIP7702UpgradeDetails.test.tsx (1)
1-258: Excellent comprehensive test coverage.This test suite thoroughly covers:
- All status states and their visual representation
- Step status propagation logic
- Timestamp display behavior
- User interaction flows (onClose vs onCloseModal)
- Edge cases with missing data
The mock structure enables deterministic testing of complex status flows.
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3797/7702-onboarding)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/hooks/useEIP7702Upgrade.tsx (2)
98-98: Consider dynamic gas estimation for better accuracy.The hardcoded estimate of 50,000 gas units may not reflect actual transaction costs. For production use, consider estimating gas dynamically using the actual transaction data (e.g., via
publicClient.estimateGaswith the EIP-7702 authorization transaction parameters) to provide users with more accurate cost information.
149-149: Optional: Add defensive error handling for localStorage.While localStorage access is generally reliable, wrapping it in a try/catch block would make the code more defensive against edge cases (e.g., storage quotas, browser restrictions).
Example:
let wasDismissed = false; try { wasDismissed = localStorage.getItem('eip7702_upgrade_dismissed') === 'true'; } catch (error) { // Silently handle localStorage errors transactionDebugLog('Failed to check localStorage:', error); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/hooks/useEIP7702Upgrade.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-05-28T14:30:02.702Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 320
File: src/components/BottomMenuModal/HistoryModal/TransactionInfo.tsx:177-182
Timestamp: 2025-05-28T14:30:02.702Z
Learning: In the transaction history tracking feature, chain ID validation is performed earlier in the flow before the values reach the TransactionInfo component, so additional validation at the display level is not needed.
Applied to files:
src/hooks/useEIP7702Upgrade.tsx
📚 Learning: 2025-11-04T14:34:00.341Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 445
File: src/containers/Authorized.tsx:63-77
Timestamp: 2025-11-04T14:34:00.341Z
Learning: In src/containers/Authorized.tsx, the tempKit useEffect intentionally excludes chainId from its dependency array because the kit is used by useWalletModeVerification to check wallet mode across all supported chains, regardless of the currently selected chainId. The chainId parameter is only used for initial kit configuration and does not affect the multi-chain verification results.
Applied to files:
src/hooks/useEIP7702Upgrade.tsx
🧬 Code graph analysis (1)
src/hooks/useEIP7702Upgrade.tsx (4)
src/hooks/useTransactionDebugLogger.tsx (1)
useTransactionDebugLogger(1-15)src/hooks/useEIP7702Info.tsx (1)
useEIP7702Info(6-12)src/apps/pillarx-app/reducer/WalletPortfolioSlice.ts (2)
setIsEIP7702Eligible(140-142)setIsUpgradeWalletModalOpen(137-139)src/apps/deposit/utils/blockchain.tsx (1)
getNetworkViem(71-94)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (1)
src/hooks/useEIP7702Upgrade.tsx (1)
170-187: LGTM!The wrapper functions and return statement are well-structured. The public API provides clear separation between manual upgrade actions (
handleUpgradeClick) and auto-check flows (checkOnLogin), and exposes all necessary state and functions for consumers.
| const checkEligibility = useCallback(() => { | ||
| // If user has already completed the upgrade, do not allow upgrading again | ||
| if (hasCompletedUpgrade) { | ||
| dispatch(setIsEIP7702Eligible(false)); | ||
| return false; | ||
| } | ||
|
|
||
| const selectedChainEIP7702Info = eip7702Info[chainId]; // Default mainnet chainId is 1 | ||
| const hasSelectedChainEIP7702 = | ||
| selectedChainEIP7702Info?.hasImplementation || false; | ||
|
|
||
| const eligible = | ||
| !hasSelectedChainEIP7702 && | ||
| Object.keys(eip7702Info).length > 0 && | ||
| !!walletAddress; | ||
|
|
||
| dispatch(setIsEIP7702Eligible(eligible)); | ||
|
|
||
| return eligible; | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [eip7702Info, walletAddress, isEligible, hasCompletedUpgrade, chainId]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused dependency from checkEligibility.
The dependency array includes isEligible (Line 73), but this value is never referenced inside the callback. The function only writes to isEligible via dispatch but never reads it. Including unused dependencies causes unnecessary re-computation and can lead to subtle bugs.
Apply this diff to remove the unused dependency:
return eligible;
- // eslint-disable-next-line react-hooks/exhaustive-deps
- }, [eip7702Info, walletAddress, isEligible, hasCompletedUpgrade, chainId]);
+ }, [eip7702Info, walletAddress, hasCompletedUpgrade, chainId]);🤖 Prompt for AI Agents
In src/hooks/useEIP7702Upgrade.tsx around lines 53 to 73, the checkEligibility
callback includes an unused dependency isEligible in its dependency array;
remove isEligible from the array so the callback depends only on eip7702Info,
walletAddress, hasCompletedUpgrade, and chainId (update the dependency list
accordingly) to prevent unnecessary re-renders and false positives from ESLint.
- Fixed 3 no-lonely-if errors by converting else-if pattern - Fixed 2 no-continue errors by refactoring to use nested conditionals - All changes in parseSearchData.ts - Tests still passing: Search (19/20, 1 skipped), AppWrapper (10/10) - Left Prettier formatting errors to avoid Tailwind corruption risk Following PR #445 methodology: manual fixes, verified after each change
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Tests
Chore