feat: implement wallet migration feature with modals and context support#334
feat: implement wallet migration feature with modals and context support#334sundayonah wants to merge 5 commits intomainfrom
Conversation
- Added WalletMigrationBanner and WalletMigrationModal components for user migration prompts. - Introduced WalletTransferApprovalModal for handling wallet transfer approvals. - Created hooks for managing EIP-7702 account setup and migration status checks. - Implemented backend API routes for deprecating old wallets and checking migration status. - Updated context to include migration-related components and logic. - Enhanced UI with new images and improved user feedback during migration processes.
📝 WalkthroughWalkthroughAdds end-to-end wallet migration: backend routes for deprecating wallets and querying migration status, a new wallet_migrations DB table with RLS and triggers, client hooks/context to detect migration needs, UI components and modals to run multi-chain fund transfers via EIP-7702/Biconomy paths, and integration into app layout and transfer flows. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI_Banner as WalletMigrationBanner
participant UI_Modal as WalletMigrationModal
participant UI_Transfer as WalletTransferApprovalModal
participant Privy as Privy Client
participant MEE as Biconomy/MEE
participant Backend as Backend API
participant DB as wallet_migrations (Supabase)
User->>UI_Banner: clicks "Start migration"
UI_Banner->>UI_Modal: open overview modal
User->>UI_Modal: clicks "Approve migration"
UI_Modal->>UI_Transfer: open transfer approval modal
UI_Transfer->>Privy: fetch per-chain balances for old SCW
Privy-->>UI_Transfer: tokens & balances
alt tokens exist
UI_Transfer->>Privy: switch to chain N
Privy-->>UI_Transfer: chain switched
UI_Transfer->>MEE: build & sign MEE authorization (if EOA path)
MEE-->>UI_Transfer: execute gasless batched transfer
MEE->>Privy: relay tx / receipt
else SCW-only path
UI_Transfer->>Privy: submit batched SCW transfer tx
Privy-->>UI_Transfer: tx receipt
end
UI_Transfer->>Backend: POST /api/v1/wallets/deprecate (old,new,txHash,userId)
Backend->>DB: upsert wallet_migrations, set deprecated, updated_at trigger fires
Backend-->>UI_Transfer: success / partial-success (KYC fallback)
UI_Transfer->>User: show success modal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (4)
supabase/migrations/create_wallet_migrations.sql (1)
16-23: Redundant index onprivy_user_id.The unique constraint
unique_user_migrationonprivy_user_id(lines 17-19) already creates an implicit index. The explicit indexidx_wallet_migrations_user_id(lines 22-23) is redundant.🔎 Proposed fix
-- Unique constraint to prevent duplicate migrations per user alter table public.wallet_migrations add constraint unique_user_migration unique (privy_user_id); --- Indexes for efficient lookups -create index if not exists idx_wallet_migrations_user_id - on public.wallet_migrations(privy_user_id); - +-- Indexes for efficient lookups (privy_user_id index covered by unique constraint) create index if not exists idx_wallet_migrations_old_scw on public.wallet_migrations(old_scw_address);app/components/WalletMigrationBanner.tsx (1)
39-46: Consider extracting the migration deadline to a shared constant.The deadline "6th June, 2025" is hardcoded in two places (desktop line 43 and mobile line 83). Extracting this to a constant or configuration would make updates easier and ensure consistency.
🔎 Proposed fix
+"use client"; +import Image from "next/image"; +import React, { useState } from "react"; +import { motion } from "framer-motion"; +import WalletMigrationModal from "./WalletMigrationModal"; + +const MIGRATION_DEADLINE = "6th June, 2025"; + export const WalletMigrationBanner = () => {Then use
{MIGRATION_DEADLINE}in both banner variants.Also applies to: 80-85
app/components/WalletTransferApprovalModal.tsx (1)
61-96: Missing cleanup may cause state updates on unmounted component.If the modal is closed while
fetchAllChainBalancesis running, the async function continues and callssetAllChainBalances/setIsFetchingBalanceson an unmounted component, potentially causing React warnings or memory leaks.🔎 Proposed fix
useEffect(() => { if (!isOpen || !oldAddress) return; + let cancelled = false; const fetchAllChainBalances = async () => { setIsFetchingBalances(true); const balancesByChain: Record<string, Record<string, number>> = {}; for (const network of networks) { try { + if (cancelled) return; const publicClient = createPublicClient({ chain: network.chain, transport: http(), }); const result = await fetchWalletBalance( publicClient, oldAddress ); const hasBalance = Object.values(result.balances).some(b => b > 0); if (hasBalance) { balancesByChain[network.chain.name] = result.balances; } } catch (error) { console.error(`Error fetching balances for ${network.chain.name}:`, error); } } + if (cancelled) return; setAllChainBalances(balancesByChain); setIsFetchingBalances(false); }; fetchAllChainBalances(); + return () => { cancelled = true; }; }, [isOpen, oldAddress]);app/hooks/useEIP7702Account.ts (1)
76-76: Effect dependencywallets.lengthmay miss wallet updates.Using
wallets.lengthas a dependency means the effect won't re-run if wallet properties change (e.g., address updates) while the array length remains the same. This could cause stale wallet data.🔎 Proposed fix
- }, [authenticated, user?.id, wallets.length]); + }, [authenticated, user?.id, wallets]);Note: If
walletsarray reference changes too frequently causing performance issues, consider using a more specific dependency likewallets.map(w => w.address).join(',')or memoizing the embedded wallet lookup.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
public/images/checkmark-circle.svgis excluded by!**/*.svgpublic/images/desktop-eip-migration.pngis excluded by!**/*.pngpublic/images/mobile-eip-migration.pngis excluded by!**/*.pngpublic/images/wallet.pngis excluded by!**/*.png
📒 Files selected for processing (13)
app/api/v1/wallets/deprecate/route.tsapp/api/v1/wallets/migration-status/route.tsapp/components/AppLayout.tsxapp/components/WalletMigrationBanner.tsxapp/components/WalletMigrationModal.tsxapp/components/WalletMigrationSuccessModal.tsxapp/components/WalletTransferApprovalModal.tsxapp/components/index.tsapp/context/MigrationContext.tsxapp/context/index.tsapp/hooks/useEIP7702Account.tsapp/lib/privy-config.tssupabase/migrations/create_wallet_migrations.sql
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-10T16:44:32.125Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 244
File: app/components/CopyAddressWarningModal.tsx:48-52
Timestamp: 2025-10-10T16:44:32.125Z
Learning: In the CopyAddressWarningModal component (app/components/CopyAddressWarningModal.tsx), selectedNetwork from useNetwork() is always defined and does not require null safety checks when accessing its properties like selectedNetwork.chain.name.
Applied to files:
app/components/WalletTransferApprovalModal.tsx
📚 Learning: 2025-12-05T02:22:28.063Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 312
File: app/page.tsx:7-11
Timestamp: 2025-12-05T02:22:28.063Z
Learning: In Next.js 15 with React 19, components using useSearchParams() cannot be statically prerendered. When app/layout.tsx has `export const dynamic = "force-static"`, MainPageContent must be dynamically imported with ssr: false to avoid "Element type is invalid" errors during build. The dynamic import pattern `dynamic(() => import("./components/MainPageContent").then(mod => ({ default: mod.MainPageContent })), { ssr: false })` is the correct solution for this scenario in the noblocks repository.
Applied to files:
app/components/AppLayout.tsx
🧬 Code graph analysis (4)
app/components/WalletMigrationBanner.tsx (1)
app/components/index.ts (1)
WalletMigrationBanner(57-57)
app/api/v1/wallets/migration-status/route.ts (1)
app/lib/supabase.ts (1)
supabaseAdmin(11-20)
app/api/v1/wallets/deprecate/route.ts (2)
app/lib/rate-limit.ts (1)
withRateLimit(32-59)app/lib/supabase.ts (1)
supabaseAdmin(11-20)
app/components/AppLayout.tsx (2)
app/context/MigrationContext.tsx (1)
MigrationBannerWrapper(5-19)app/context/index.ts (1)
MigrationBannerWrapper(15-15)
🔇 Additional comments (11)
app/lib/privy-config.ts (2)
1-1: EIP-7702 configuration correctly enables new user provisioning.Line 16 appropriately sets
createOnLogin: "all-users"to provision new users as EIP-7702-enabled EOAs per the PR objectives. All imported chains are utilized in thesupportedChainsarray (line 28), and the required networks for migration (Base, Polygon, Arbitrum, Lisk, and BNB Chain) are included.
1-1: Verify that baseSepolia removal was intentional.The PR commit message mentions "removes an unused chain reference (baseSepolia)", but the AI summary describes this as a "whitespace-only modification." Please confirm that baseSepolia was removed intentionally and is no longer needed for this configuration.
app/context/index.ts (1)
15-15: LGTM!The export follows the established pattern in this file.
app/components/index.ts (1)
57-57: LGTM!The export follows the established pattern in this file.
app/components/AppLayout.tsx (1)
15-27: LGTM!The
MigrationBannerWrapperis properly imported and positioned in the header area after the existingNoticeBanner, following the established layout pattern.app/context/MigrationContext.tsx (1)
5-19: LGTM!Clean conditional rendering with clear early-return logic. The component appropriately hides itself during the checking phase and when migration isn't needed.
app/components/WalletMigrationBanner.tsx (1)
7-104: LGTM!The component properly handles responsive design with separate desktop/mobile layouts, and the modal integration is clean. The fixed positioning with
top-16correctly places the banner below the navbar.app/components/WalletMigrationSuccessModal.tsx (1)
12-98: LGTM!The modal implementation follows good practices with proper accessibility attributes, smooth animations via Framer Motion, and correct Headless UI Dialog usage.
app/components/WalletMigrationModal.tsx (2)
14-29: LGTM!The modal state management and transition logic between the two modals is clean. The 300ms delay for the animation transition is a reasonable approach to ensure smooth UX.
31-211: LGTM!The modal UI implementation is well-structured with proper accessibility attributes (
aria-label), responsive design patterns, and consistent dark mode support. The AnimatePresence wrapper correctly handles exit animations.app/components/WalletTransferApprovalModal.tsx (1)
276-282: Partial migration continues silently on chain errors.When a chain transfer fails, the error is caught and a toast is shown, but the loop continues to process remaining chains. This can leave users in a partial migration state where some chains succeeded and others failed, yet the deprecation API is still called.
Consider tracking failed chains and either retrying, stopping the migration, or informing the user more explicitly about partial failures before proceeding with deprecation.
| } catch (error) { | ||
| console.error("Error checking migration status:", error); | ||
| return NextResponse.json({ | ||
| error: "Internal server error", | ||
| migrationCompleted: false | ||
| }, { status: 500 }); | ||
| } |
There was a problem hiding this comment.
Inconsistent response shape on error.
The success response includes migrationCompleted, status, and hasSmartWallet, but the error response only includes error and migrationCompleted. Consider returning a consistent shape for easier client-side handling.
🔎 Proposed fix
} catch (error) {
console.error("Error checking migration status:", error);
return NextResponse.json({
error: "Internal server error",
- migrationCompleted: false
+ migrationCompleted: false,
+ status: "unknown",
+ hasSmartWallet: false
}, { status: 500 });
}🤖 Prompt for AI Agents
In app/api/v1/wallets/migration-status/route.ts around lines 30 to 36, the error
branch returns a different JSON shape than the success response; update the
catch block to return the same keys as the success response (migrationCompleted,
status, hasSmartWallet) plus an error message so the client always receives a
consistent shape. Populate migrationCompleted:false, status:"error" (or the same
enum/string used on success), hasSmartWallet:false (or null if that better
matches types), and include error: error?.message || String(error) in the JSON,
and keep the 500 status code.
| <p className="mb-8 text-center text-sm font-normal leading-relaxed text-text-body/90 dark:text-white/90"> | ||
| You can now continue converting your crypto to <br /> fiats at zero fees on noblocks | ||
| </p> |
There was a problem hiding this comment.
Minor grammatical fix: "fiats" should be "fiat".
"Fiat" is typically used as an uncountable noun in financial contexts.
🔎 Proposed fix
<p className="mb-8 text-center text-sm font-normal leading-relaxed text-text-body/90 dark:text-white/90">
- You can now continue converting your crypto to <br /> fiats at zero fees on noblocks
+ You can now continue converting your crypto to <br /> fiat at zero fees on Noblocks
</p>📝 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.
| <p className="mb-8 text-center text-sm font-normal leading-relaxed text-text-body/90 dark:text-white/90"> | |
| You can now continue converting your crypto to <br /> fiats at zero fees on noblocks | |
| </p> | |
| <p className="mb-8 text-center text-sm font-normal leading-relaxed text-text-body/90 dark:text-white/90"> | |
| You can now continue converting your crypto to <br /> fiat at zero fees on Noblocks | |
| </p> |
🤖 Prompt for AI Agents
In app/components/WalletMigrationSuccessModal.tsx around lines 80 to 82, the
copy uses the plural "fiats" which is grammatically incorrect in this context;
change the string to use the uncountable noun "fiat" (e.g., "convert your crypto
to fiat at zero fees on noblocks") and keep the existing markup and line break
behavior intact.
…y config - Corrected the return type documentation in the TokensContext function. - Removed baseSepolia from the supportedChains list in PrivyClientConfig as it is no longer needed.
sundayonah
force-pushed
the
feat/eip-7702-wallet-migration-clean
branch
from
a2c30cd to
8971509
Compare
… validation - Implemented JWT verification for secure access to the wallet deprecation endpoint. - Added checks to ensure the authenticated user matches the provided userId and wallet address. - Improved error handling for unauthorized access and token validation. - Introduced rollback mechanisms for database operations to maintain data integrity during migration. - Updated KYC data migration logic to allow for partial success reporting.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
app/components/WalletTransferApprovalModal.tsx (1)
173-179: Contradictory zero-balance migration logic remains unresolved.The code at lines 173-179 explicitly handles zero-balance migration ("No tokens to migrate, but deprecating old wallet..."), but line 479 disables the button when
tokens.length === 0. Users with zero balance cannot initiate the wallet deprecation they need.🔎 Proposed fix
<button onClick={handleApproveTransfer} - disabled={isProcessing || isLoading || isFetchingBalances || tokens.length === 0} + disabled={isProcessing || isLoading || isFetchingBalances} className="w-full rounded-xl bg-lavender-500 px-6 py-3.5 text-base font-semibold text-white hover:opacity-90 disabled:opacity-50 disabled:cursor-not-allowed transition-opacity" > - {isProcessing ? progress || "Processing migration..." : isFetchingBalances ? "Loading balances..." : "Approve transfer"} + {isProcessing ? progress || "Processing migration..." : isFetchingBalances ? "Loading balances..." : tokens.length === 0 ? "Deprecate wallet" : "Approve transfer"} </button>Also applies to: 479-479
🧹 Nitpick comments (3)
app/hooks/useEIP7702Account.ts (1)
107-167: Potential stale closure issue withsmartWalletBalance.The
smartWalletBalancevalue is captured at line 132 before the asyncfetchcall. IfallBalances.smartWallet?.totalchanges during the API call, the fallback logic (lines 154-155 and 159-160) will use the stale value. While this is unlikely to cause significant issues in practice, it's worth noting.app/components/WalletTransferApprovalModal.tsx (1)
68-89: Consider parallel balance fetching for better UX.The current implementation fetches balances sequentially for each network. For 5+ networks, this can result in noticeable delays. Consider using
Promise.allSettledfor parallel fetching.🔎 Proposed optimization
- // Fetch balances for each supported network - for (const network of networks) { - try { - const publicClient = createPublicClient({ - chain: network.chain, - transport: http(), - }); - - const result = await fetchWalletBalance( - publicClient, - oldAddress - ); - - // Only include chains with non-zero balances - const hasBalance = Object.values(result.balances).some(b => b > 0); - if (hasBalance) { - balancesByChain[network.chain.name] = result.balances; - } - } catch (error) { - console.error(`Error fetching balances for ${network.chain.name}:`, error); - } - } + // Fetch balances for all networks in parallel + const results = await Promise.allSettled( + networks.map(async (network) => { + const publicClient = createPublicClient({ + chain: network.chain, + transport: http(), + }); + const result = await fetchWalletBalance(publicClient, oldAddress); + return { chainName: network.chain.name, result }; + }) + ); + + for (const settled of results) { + if (settled.status === 'fulfilled') { + const { chainName, result } = settled.value; + const hasBalance = Object.values(result.balances).some(b => b > 0); + if (hasBalance) { + balancesByChain[chainName] = result.balances; + } + } + }supabase/migrations/create_wallet_migrations.sql (1)
22-23: Redundant index onprivy_user_id.The
UNIQUEconstraint onprivy_user_id(line 17-19) already creates an implicit B-tree index. The explicitidx_wallet_migrations_user_idindex is redundant.🔎 Proposed fix
-- Indexes for efficient lookups -create index if not exists idx_wallet_migrations_user_id - on public.wallet_migrations(privy_user_id); - create index if not exists idx_wallet_migrations_old_scw on public.wallet_migrations(old_scw_address);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/api/v1/wallets/deprecate/route.tsapp/api/v1/wallets/migration-status/route.tsapp/components/WalletTransferApprovalModal.tsxapp/hooks/useEIP7702Account.tssupabase/migrations/create_wallet_migrations.sql
🚧 Files skipped from review as they are similar to previous changes (2)
- app/api/v1/wallets/deprecate/route.ts
- app/api/v1/wallets/migration-status/route.ts
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-10T16:44:32.125Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 244
File: app/components/CopyAddressWarningModal.tsx:48-52
Timestamp: 2025-10-10T16:44:32.125Z
Learning: In the CopyAddressWarningModal component (app/components/CopyAddressWarningModal.tsx), selectedNetwork from useNetwork() is always defined and does not require null safety checks when accessing its properties like selectedNetwork.chain.name.
Applied to files:
app/components/WalletTransferApprovalModal.tsxapp/hooks/useEIP7702Account.ts
📚 Learning: 2025-11-06T07:37:39.036Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is true (verification failed/returned "Ok"), the recipient safety alert should display when: isRecipientNameEditable && recipientName && !errors.recipientName && !recipientNameError. The !isFetchingRecipientName check is redundant because recipientName is cleared at fetch start and only populated after fetching completes or when the user manually enters it.
Applied to files:
app/components/WalletTransferApprovalModal.tsxapp/hooks/useEIP7702Account.ts
📚 Learning: 2025-11-06T07:37:39.036Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is false (verification succeeded), the recipient safety alert should display when: !isRecipientNameEditable && recipientName && !recipientNameError. The !errors.recipientName check is unnecessary because in non-editable mode the recipient name is displayed as read-only text (not an input field), so form validation errors cannot occur.
Applied to files:
app/components/WalletTransferApprovalModal.tsxapp/hooks/useEIP7702Account.ts
📚 Learning: 2025-11-06T07:08:54.593Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:271-280
Timestamp: 2025-11-06T07:08:54.593Z
Learning: In the RecipientDetailsForm component (app/components/recipient/RecipientDetailsForm.tsx), Mobile Money institutions always return "Ok" from the fetchAccountName verification endpoint. Therefore, checking for accountName.toLowerCase() === "ok" is sufficient to handle both verification failures and Mobile Money institutions without needing explicit institution type checks in the frontend.
Applied to files:
app/hooks/useEIP7702Account.ts
🧬 Code graph analysis (1)
app/components/WalletTransferApprovalModal.tsx (4)
app/mocks.ts (1)
networks(44-88)app/context/index.ts (3)
useBalance(3-3)useTokens(10-10)useNetwork(2-2)app/hooks/useActualTheme.ts (1)
useActualTheme(4-25)app/utils.ts (3)
fetchWalletBalance(460-506)formatCurrency(70-83)shortenAddress(577-586)
🔇 Additional comments (6)
app/hooks/useEIP7702Account.ts (2)
62-72: Previous issue addressed - LGTM.The early return case when
embeddedWalletis missing now properly callssetAccountwithisReady: true, preventing the perpetual loading state identified in the previous review.
22-86: LGTM - Well-structured hook with proper state management.The
useEIP7702Accounthook correctly handles all edge cases (unauthenticated, no smart wallet, no embedded wallet) and setsisReady: trueappropriately in each branch.app/components/WalletTransferApprovalModal.tsx (2)
225-230: Precision fix applied correctly, but inherent limitation remains.The
parseUnitsusage is an improvement overMath.floor, buttoken.amountis still aNumberwhich has ~15-17 significant digits. For tokens with 18 decimals and values with many significant figures, minor precision loss is possible. This would require refactoringfetchWalletBalanceto return raw BigInt balances for a complete fix.
30-52: Well-structured component with comprehensive wallet migration handling.The component properly integrates with Privy smart wallets, handles multi-chain transfers, and provides good user feedback through progress indicators and toast notifications.
supabase/migrations/create_wallet_migrations.sql (2)
2-14: Solid table design with proper constraints.Good use of CHECK constraints for address validation and status enumeration. The nullable
new_eoa_addressallows for pending migration states.
64-69: RLS policy correctly validates JWT sub claim mapping.The policy properly restricts reads to rows where
privy_user_idmatches the JWTsubclaim from Supabase'scurrent_setting('request.jwt.claims'). The Privy JWT verification in middleware (line 86 of middleware.ts) confirms thatpayload.subcontains the privy_user_id. In Supabase's authenticated context, JWT claims are always available, so theNULLhandling is safe—a missing claim would simply return no rows, which is the correct security default.Note: The
wallet_migrationstable is defined with RLS but is not currently populated by any application code. If this is intentional for future use, the table is correctly configured. If it should be actively used, integration with the wallet deprecation workflow may be needed.
| -- Trigger to automatically update updated_at | ||
| create trigger wallet_migrations_update_updated_at | ||
| before update on public.wallet_migrations | ||
| for each row execute function public.update_wallet_migrations_updated_at(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add DROP TRIGGER IF EXISTS for idempotent migrations.
The trigger creation will fail if the migration runs twice (e.g., in development or rollback scenarios). Add a drop statement for idempotency.
🔎 Proposed fix
-- Trigger to automatically update updated_at
+drop trigger if exists wallet_migrations_update_updated_at on public.wallet_migrations;
create trigger wallet_migrations_update_updated_at
before update on public.wallet_migrations
for each row execute function public.update_wallet_migrations_updated_at();📝 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.
| -- Trigger to automatically update updated_at | |
| create trigger wallet_migrations_update_updated_at | |
| before update on public.wallet_migrations | |
| for each row execute function public.update_wallet_migrations_updated_at(); | |
| -- Trigger to automatically update updated_at | |
| drop trigger if exists wallet_migrations_update_updated_at on public.wallet_migrations; | |
| create trigger wallet_migrations_update_updated_at | |
| before update on public.wallet_migrations | |
| for each row execute function public.update_wallet_migrations_updated_at(); |
🤖 Prompt for AI Agents
In supabase/migrations/create_wallet_migrations.sql around lines 43 to 46, the
CREATE TRIGGER will fail if the migration is run more than once; make the
migration idempotent by adding a DROP TRIGGER IF EXISTS statement immediately
before the CREATE TRIGGER (drop the trigger on public.wallet_migrations with the
same name), then keep the existing CREATE TRIGGER line; ensure the DROP
statement targets the correct schema/table and ends with a semicolon so the
migration can be safely re-run.
…lection logic - Introduced useMigrationStatus hook to check and manage wallet migration status. - Updated components (MobileDropdown, Navbar, SettingsDropdown, WalletDetails, TransactionForm, TransactionPreview) to utilize migration status for determining active wallet and balance. - Enhanced error handling in migration status API response to improve user experience during wallet transitions. - Adjusted balance fetching logic to prioritize embedded wallets post-migration. - Refactored wallet selection logic to ensure correct wallet type is displayed based on migration state.
- Refactored wallet migration handling across components to utilize the new useShouldUseEOA hook for determining active wallet type. - Updated MobileDropdown, Navbar, SettingsDropdown, WalletDetails, and TransferForm components to reflect changes in wallet selection based on migration status. - Improved balance fetching logic to prioritize embedded wallets post-migration and ensure accurate display of wallet balances. - Enhanced UI elements in WalletMigrationBanner and WalletMigrationModal for better user experience during migration processes. - Added new dependencies and updated configuration files to support recent changes.
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
app/components/SettingsDropdown.tsx (1)
278-287:⚠️ Potential issue | 🟡 Minor"Export wallet" menu item lacks a
<button>wrapper — accessibility gap.Other menu items in this dropdown wrap their interactive content in a
<button>, providing keyboard focus and activation. This<li>with a bareonClickis not focusable or activatable via keyboard.Proposed fix
{!isInjectedWallet && ( <li role="menuitem" className="flex cursor-pointer items-center gap-2.5 rounded-lg transition-all duration-300 hover:bg-accent-gray dark:hover:bg-neutral-700" - onClick={exportWallet} > - <AccessIcon className="size-5 text-icon-outline-secondary dark:text-white/50" /> - <p>Export wallet</p> + <button + type="button" + className="flex w-full items-center gap-2.5" + onClick={exportWallet} + > + <AccessIcon className="size-5 text-icon-outline-secondary dark:text-white/50" /> + <p>Export wallet</p> + </button> </li> )}app/components/MobileDropdown.tsx (1)
150-183:⚠️ Potential issue | 🟠 Major
trackLogoutWithFetchis defined but never called — dead code.In
SettingsDropdown,handleLogoutcallstrackLogoutWithFetch(orsendBeacon) before logging out. Here inMobileDropdown,handleLogout(lines 171-183) skips tracking entirely, leavingtrackLogoutWithFetchunused. Either remove it or wire it intohandleLogoutto matchSettingsDropdown's behavior.app/hooks/useSmartWalletTransfer.ts (1)
81-92:⚠️ Potential issue | 🟠 Major
getEmbeddedWalletAddressalways returns the EOA address, even for Smart Wallet transfers.
saveTransferTransactionuses this helper as thefromfield (Line 286). For the Smart Wallet path (Lines 190-206), the actual sender is the smart wallet, not the embedded wallet. Transaction history will record the wrong sender address for pre-migration users.Proposed fix
- const getEmbeddedWalletAddress = (): `0x${string}` | undefined => { - if (!user?.linkedAccounts) return undefined; - const wallet = user.linkedAccounts.find( - (account) => - account.type === "wallet" && - (account as any).connectorType === "embedded" && - typeof (account as any).address === "string", - ) as { address?: string } | undefined; - return wallet?.address as `0x${string}` | undefined; - }; + const getSenderAddress = (): `0x${string}` | undefined => { + if (!user?.linkedAccounts) return undefined; + if (shouldUseEOA && embeddedWallet) { + return embeddedWallet.address as `0x${string}`; + } + const smartWallet = user.linkedAccounts.find( + (account) => account.type === "smart_wallet", + ) as { address?: string } | undefined; + if (smartWallet?.address) return smartWallet.address as `0x${string}`; + // Fallback to embedded wallet + const embedded = user.linkedAccounts.find( + (account) => + account.type === "wallet" && + (account as any).connectorType === "embedded" && + typeof (account as any).address === "string", + ) as { address?: string } | undefined; + return embedded?.address as `0x${string}` | undefined; + };And update the call site on Line 286:
- const from = getEmbeddedWalletAddress(); + const from = getSenderAddress();app/pages/TransactionPreview.tsx (1)
439-448:⚠️ Potential issue | 🟠 MajorDuplicate and mislabeled analytics events for non-smart-wallet paths.
After
createOrder()completes, lines 445-448 unconditionally fire atrackEvent("Swap started", { "Wallet type": "Smart wallet" }). However, the injected-wallet path (line 297) and MEE path (line 380) already fire their owntrackEventwith the correct wallet type. This results in:
- Injected wallet → two "Swap started" events (one correct, one saying "Smart wallet")
- MEE (EIP-7702) → two "Swap started" events (one correct, one saying "Smart wallet")
Move the shared
trackEventinto the smart-walletelsebranch, or addreturn/ early-exit after path-specific tracking.🐛 Proposed fix — move trackEvent into the smart wallet branch
} else { - // Smart wallet (pre-migration) + // Smart wallet (pre-migration) if (!client) { throw new Error("Smart wallet not found"); } // ... existing smart wallet logic ... + + trackEvent("Swap started", { + "Entry point": "Transaction preview", + "Wallet type": "Smart wallet", + }); } await getOrderId(); toast.success("Order created successfully"); refreshBalance(); - - trackEvent("Swap started", { - "Entry point": "Transaction preview", - "Wallet type": "Smart wallet", - });
🤖 Fix all issues with AI agents
In `@app/api/v1/wallets/deprecate/route.ts`:
- Around line 115-130: The rollback update against the "wallets" table can
itself fail and is currently ignored; wrap the
supabaseAdmin.from("wallets").update(...) call in a try/catch, and in the catch
use trackApiError(request, "/api/v1/wallets/deprecate", "POST", rollbackError,
500) (or similar) to log the rollback failure along with context (include
oldAddress and userId), then either attach the rollback error to the thrown
upsertError (e.g., include both errors in a combined error or add a property) or
throw a new Error that preserves both upsertError and rollbackError; ensure you
still throw after logging so the original failure is reported to the caller.
- Around line 44-54: The request validation in this route is missing txHash:
ensure txHash is treated as a required field along with walletAddress,
oldAddress, newAddress, and userId by updating the conditional that checks
presence (where walletAddress, oldAddress, newAddress, userId are validated) to
also validate txHash, and update the error path (trackApiError and
NextResponse.json) to reflect the missing txHash; this ensures migration_tx_hash
stored later is never null and points to the same identifiers (walletAddress,
oldAddress, newAddress, txHash, userId) used in the handler.
- Around line 104-113: The second upsert call using supabaseAdmin (the block
creating/updating the new EOA wallet with address/newAddress and user_id/userId)
is missing the onConflict option and may fail or create duplicates on retries;
update that upsert to include the same conflict target used earlier (onConflict:
"address,user_id") so retries will perform an update instead of causing unique
constraint errors or duplicate rows.
In `@app/api/v1/wallets/migration-status/route.ts`:
- Around line 4-11: The GET handler exposes any user's migration status because
it trusts the query param userId directly; update the GET function (export async
function GET) to validate the caller's authentication token (e.g., verify Privy
token from the request headers) and extract the authenticated user id, then
compare that id to the requested searchParams.get("userId"); if the token is
missing/invalid return a 401 and if the ids do not match return a 403, otherwise
proceed with the existing logic. Ensure you call the existing token verification
utility or implement a small verifyPrivyToken helper, use NextRequest headers to
read the auth token and NextResponse to return the proper status codes, and fail
closed on any verification error.
- Around line 32-50: The current handler returns HTTP 200 for DB errors which
masks failures; update the PGRST205 branch and the generic error branch to
return appropriate 5xx status codes (e.g., 503 Service Unavailable for transient
DB/schema issues and 500 for unexpected DB errors) instead of always 200 by
changing the NextResponse.json status values, and add structured logging/metrics
calls that include error.code and error.message (e.g., log with a
logger/error-metrics method) so monitoring can alert on occurrences; locate and
modify the error handling around the error.code === "PGRST205" check and the
subsequent generic error path where NextResponse.json is returned.
- Around line 13-19: The route currently queries a non-existent table via
supabaseAdmin.from("wallets") and requests columns that don't match your
migrations; update app/api/v1/wallets/migration-status/route.ts to query
"wallet_migrations" instead of "wallets", select the correct columns (e.g.,
.select("privy_user_id, status") or whatever exists in the migration schema),
replace .eq("user_id", userId) with .eq("privy_user_id", userId), and treat
migration completion by checking status === "completed" (or map status values
'pending'|'in_progress'|'completed'|'failed' appropriately) rather than relying
on a non-existent migration_completed field; also apply the same fix to other
routes referencing supabaseAdmin.from("wallets") such as
app/api/v1/wallets/deprecate/route.ts or create a new SQL migration to add a
wallets table with the expected columns if you prefer that approach.
In `@app/components/SettingsDropdown.tsx`:
- Around line 22-23: Remove the unused Download01Icon import from the
SettingsDropdown component import list; only keep AccessIcon (and any other
icons actually referenced in the JSX). Locate the import statement that includes
Download01Icon and AccessIcon in SettingsDropdown.tsx, delete the Download01Icon
token so the file no longer imports an unused symbol and run a quick lint/check
to ensure no other references remain.
In `@app/components/WalletMigrationModal.tsx`:
- Around line 186-188: In WalletMigrationModal (the JSX <p> with className
"text-sm font-normal leading-5 tracking-normal text-text-body align-middle
dark:text-[`#FFFFFFCC`]") fix the typo in the displayed copy by replacing "heavy
liftings" with "heavy lifting" so the sentence reads "...and we will do all the
heavy lifting for you"; update only the string inside that <p> element.
In `@app/components/WalletTransferApprovalModal.tsx`:
- Around line 345-346: The success modal may be lost because onClose() can
unmount this component before setShowSuccessModal(true) runs; change the order
or lift the state: call setShowSuccessModal(true) before invoking onClose() (or
delay calling onClose() until after setShowSuccessModal has run), ensuring
WalletMigrationSuccessModal (and the showSuccessModal state) is set while this
component is still mounted, or alternatively lift showSuccessModal state to the
parent so the modal is rendered outside this component's lifecycle.
- Around line 58-93: The current useEffect's fetchAllChainBalances runs network
requests serially in fetchAllChainBalances; change it to fire parallel requests
via Promise.allSettled over networks and then aggregate successful results,
keeping the same filtering logic and state updates in setAllChainBalances and
setIsFetchingBalances; also replace the createPublicClient transport http() call
with the thirdweb-backed RPC from getRpcUrl(...) (import getRpcUrl from
app/utils) when building each public client so each parallel request uses the
explicit RPC endpoint used elsewhere in the codebase; keep error handling by
ignoring failed settlements and logging them, and ensure fetchWalletBalance is
still called for each network result before storing balances.
- Around line 245-249: The publicClient is created with createPublicClient using
http() which defaults to the chain's public RPC; replace that with the project
RPC by calling http(getRpcUrl(chainName)) so it uses our configured RPC
endpoint. Locate the createPublicClient call that assigns publicClient in
WalletTransferApprovalModal and pass the result of getRpcUrl for the current
chain (use the same chainName/chain reference used elsewhere in this component)
to http(), keeping the rest of the createPublicClient options (chain) unchanged.
- Around line 312-336: The deprecation call (fetch("/api/v1/wallets/deprecate")
in WalletTransferApprovalModal) runs unconditionally even if some chain
transfers failed, risking stranded funds; modify the transfer loop to record
per-chain outcomes (e.g., failedChains or a boolean allTransfersSucceeded
derived from allTxHashes and caught errors), and gate the deprecation step (the
setProgress("Finalizing migration...") + fetch call) so it only runs when
allTransfersSucceeded; if any failures exist, surface a UI decision path (show a
warning modal or retry option) that lets the user retry failed chains or confirm
deprecation, or pass the failedChains info to the backend (include failedChains
in the deprecation request) instead of blindly deprecating
oldAddress/newAddress.
- Around line 326-331: The frontend currently sends only allTxHashes[0] as
txHash; change the request body to include the full array (allTxHashes) — e.g.,
send txHashes: allTxHashes along with oldAddress, newAddress, userId — and
update any uses of txHash to txHashes. On the backend route
/api/v1/wallets/deprecate, update the request schema/handler to accept txHashes:
string[] (instead of a single txHash), persist all hashes (either by changing
the DB column to an array type or storing them in a related table), and update
any functions that previously referenced txHash to handle the array. Ensure
user.id, oldAddress, newAddress handling remains unchanged and add validation
for an empty or null array.
In `@app/hooks/useEIP7702Account.ts`:
- Around line 25-33: The function parseEip7702AuthorizedAddress can return a
shortened address if the input is truncated; update it to validate that there
are at least 40 hex characters (20 bytes) after the EIP7702_MAGIC_PREFIX before
slicing. Specifically, in parseEip7702AuthorizedAddress compute start = idx +
EIP7702_MAGIC_PREFIX.length and check that normalized.length >= start + 40 (or
otherwise return null); then slice exactly 40 characters and return `0x${...}`
as Address.
- Around line 86-92: The migration-status endpoint is being called without
authentication from useEIP7702Account (the fetch call to
`/api/v1/wallets/migration-status?userId=${user.id}`), so either protect the
route server-side or send and validate a JWT: on the server add the route to
your middleware matcher (e.g., `/api/v1/wallets/:path*`) or implement JWT
validation in the migration-status handler to ensure the token's subject matches
the requested userId; on the client (useEIP7702Account) include the
Authorization header with the user's JWT when calling the endpoint (fetch(..., {
headers: { Authorization: `Bearer ${token}` } })) so the handler can verify
ownership before returning migrationCompleted.
In `@app/hooks/useSmartWalletTransfer.ts`:
- Around line 157-160: In useSmartWalletTransfer, avoid passing an empty string
to createMeeClient when the Biconomy API key is missing; instead validate
process.env.NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY before calling createMeeClient
and throw or return a clear error (e.g., throw new Error indicating the missing
NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY) so callers fail fast with an actionable
message; update the code around the createMeeClient(...) call to use the
validated key variable (or bail out) rather than using the ?? "" fallback.
In `@app/pages/TransactionPreview.tsx`:
- Around line 369-383: The code sets UI success flags too early: move the calls
to setIsGatewayApproved(true) and setIsOrderCreated(true) so they occur only
after meeClient.waitForSupertransactionReceipt({ hash }) confirms the
transaction receipt (and ideally after validating the receipt indicates
success). Update the TransactionPreview flow around meeClient.execute and
meeClient.waitForSupertransactionReceipt to perform the state updates
(setIsGatewayApproved, setIsOrderCreated) after the wait/receipt check (and
optionally move trackEvent("Swap started", ...) to run post-confirmation to
match the injected-wallet path).
- Around line 301-331: The code passes NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY
(possibly empty) into createMeeClient which will fail opaquely; before calling
createMeeClient (in the block around shouldUseEOA && embeddedWallet) validate
process.env.NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY is present and non-empty and
throw a clear Error (e.g., "Missing Biconomy paymaster API key: set
NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY") if not, so createMeeClient is only invoked
with a real apiKey; update any log/exception handling around meeClient creation
to surface this validation.
🧹 Nitpick comments (11)
app/components/WalletMigrationSuccessModal.tsx (1)
17-19:handleContinueis a trivial wrapper overonClose.This indirection adds no value. Consider passing
onClosedirectly to the button'sonClick.Proposed simplification
- const handleContinue = () => { - onClose(); - }; - ... <button - onClick={handleContinue} + onClick={onClose}app/components/SettingsDropdown.tsx (1)
50-65: Wallet selection logic is duplicated across four components.The
embeddedWallet/smartWalletderivation and theactiveWallet/walletAddressternary appear identically inSettingsDropdown,MobileDropdown,Navbar, andWalletDetails. Consider extracting a shareduseActiveWallet()hook to DRY this up and ensure future changes are applied consistently.app/components/MobileDropdown.tsx (1)
241-241: Prop namesmartWalletis semantically misleading.
walletForCopymay be an EOA or injected wallet, not necessarily a smart wallet. If theWalletViewcomponent's prop can't be renamed right now, at minimum add a comment clarifying the mismatch.app/components/WalletMigrationModal.tsx (2)
99-120: Remove commented-out code.This block contains hardcoded developer-specific content ("Chibie" avatar/name). Dead code like this adds noise; remove it and track any planned feature in a separate issue if needed.
20-25: Hardcoded 300ms delay couples to animation duration.The
setTimeouton Line 22 must stay in sync with the exit animation duration on Line 45. If either changes independently, the user may see a visual glitch (flash or gap between modals). Consider deriving both from a shared constant.Proposed fix
+const MODAL_TRANSITION_MS = 300; + const WalletMigrationModal: React.FC<WalletMigrationModalProps> = ({ isOpen, onClose, }) => { const [showTransferModal, setShowTransferModal] = useState(false); const handleApproveMigration = () => { onClose(); setTimeout(() => { setShowTransferModal(true); - }, 300); + }, MODAL_TRANSITION_MS); };app/context/BalanceContext.tsx (1)
223-236:walletsin the dependency array may cause excessive re-fetching.
useWallets()can return a new array reference on every render even when the wallet list hasn't changed. Adding it to theuseEffectdeps (Line 229) risks triggeringfetchBalancesrepeatedly, hammering RPC endpoints.Consider memoizing a stable derived value (e.g., the embedded wallet address string) instead of passing the whole
walletsarray:Proposed fix
Extract the address outside the effect and depend on that:
+ const embeddedWalletAddress = wallets.find( + (wallet) => wallet.walletClientType === "privy" + )?.address; + const fetchBalances = async () => { // ... use embeddedWalletAddress instead of re-finding from wallets }; useEffect(() => { fetchBalances(); }, [ ready, user, - wallets, + embeddedWalletAddress, selectedNetwork, isInjectedWallet, injectedReady, injectedAddress, cngnRate, isMigrationComplete, ]);app/pages/TransactionPreview.tsx (2)
134-157:activeWallettypes are inconsistent across branches.The three branches produce objects with different shapes:
injectedWallet:{ address: string, type: "injected_wallet" }- EOA branch:
{ address: string, type: "eoa" }smartWallet: a PrivyLinkedAccountobject (withaddress,type: "smart_wallet", plus many other fields)While the code only accesses
.addressand occasionally.type, the lack of a common type makes this fragile. Consider normalizing into a shared shape.♻️ Suggested normalization
- const activeWallet = injectedWallet || - (shouldUseEOA && embeddedWallet - ? { address: embeddedWallet.address, type: "eoa" } - : smartWallet); + const activeWallet: { address: string; type: string } | null | undefined = + injectedWallet || + (shouldUseEOA && embeddedWallet + ? { address: embeddedWallet.address, type: "eoa" } + : smartWallet + ? { address: smartWallet.address, type: "smart_wallet" } + : null);
723-724: Minor: template literal className could useclassNamesfor consistency.The rest of the file uses the
classNameshelper (imported at line 10), but this spot switched to a raw template literal. Not a functional issue, just a consistency nit.app/components/WalletTransferApprovalModal.tsx (1)
96-119: CNGN rate fetching is also sequential and could be parallelized.Similar to the balance fetching, CNGN rates are fetched one chain at a time in a
for...ofloop. UsePromise.allSettledfor parallel requests.app/hooks/useEIP7702Account.ts (2)
75-104: Duplicate migration-status API calls across hooks.
useMigrationStatus(line 75) fetches/api/v1/wallets/migration-status, anduseWalletMigrationStatus(line 186) independently fetches the same endpoint. SinceuseShouldUseEOAcallsuseMigrationStatus, and various components use both hooks, the same API call can fire 3+ times on page load.Consider consolidating into a single source of truth — either a shared context/provider, or have
useWalletMigrationStatusconsumeuseMigrationStatusinternally instead of making its own fetch.♻️ Sketch: reuse useMigrationStatus inside useWalletMigrationStatus
export function useWalletMigrationStatus(): WalletMigrationStatus { const { user, authenticated } = usePrivy(); const { allBalances, isLoading: isBalanceLoading } = useBalance(); + const { isMigrationComplete, isLoading: isMigrationLoading } = useMigrationStatus(); const [needsMigration, setNeedsMigration] = useState(false); const [showZeroBalanceMessage, setShowZeroBalanceMessage] = useState(false); const [isChecking, setIsChecking] = useState(true); useEffect(() => { - async function checkMigrationStatus() { + function checkMigrationStatus() { if (!authenticated || !user) { /* ... */ return; } const smartWallet = /* ... */; if (!smartWallet) { /* ... */ return; } - if (isBalanceLoading) return; + if (isBalanceLoading || isMigrationLoading) return; const smartWalletBalance = allBalances.smartWallet?.total ?? 0; - try { - const response = await fetch(`/api/v1/wallets/migration-status?userId=${user.id}`); - // ... parse response ... - } catch { /* ... */ } + if (isMigrationComplete) { + setNeedsMigration(false); + setShowZeroBalanceMessage(false); + setIsChecking(false); + return; + } + const hasBalance = smartWalletBalance > 0; + setNeedsMigration(hasBalance); + setShowZeroBalanceMessage(!hasBalance); + setIsChecking(false); } checkMigrationStatus(); - }, [authenticated, user?.id, allBalances.smartWallet?.total, isBalanceLoading]); + }, [authenticated, user?.id, allBalances.smartWallet?.total, isBalanceLoading, isMigrationComplete, isMigrationLoading]);Also applies to: 154-223
180-181: Early return during balance loading leavesisCheckingstuck astrue.Line 181 returns early without calling
setIsChecking(false). While the effect will re-run onceisBalanceLoadingflips tofalse(due to the dependency array), any consumer relying onisCheckingto hide a loading spinner will show the spinner for the entire balance-loading duration rather than just the migration-check duration. This is likely acceptable behavior but worth documenting the intent —isCheckingeffectively means "migration status not yet determined" which includes waiting for balances.📝 Suggested comment clarification
- if (isBalanceLoading) return; + // Wait for balance to settle before deciding migration state. + // isChecking intentionally remains true until balance + API check are both done. + if (isBalanceLoading) return;
| const walletAddress = request.headers.get("x-wallet-address")?.toLowerCase(); | ||
| const body = await request.json(); | ||
| const { oldAddress, newAddress, txHash, userId } = body; | ||
|
|
||
| if (!walletAddress || !oldAddress || !newAddress || !userId) { | ||
| trackApiError(request, "/api/v1/wallets/deprecate", "POST", new Error("Missing required fields"), 400); | ||
| return NextResponse.json( | ||
| { success: false, error: "Missing required fields" }, | ||
| { status: 400 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
txHash is not validated but used as migration proof.
txHash is destructured from the body but omitted from the required-fields check on Line 48. It's then stored as migration_tx_hash on Line 93. If a caller omits txHash, the migration record will be created with a null transaction hash, undermining auditability.
Proposed fix
- if (!walletAddress || !oldAddress || !newAddress || !userId) {
+ if (!walletAddress || !oldAddress || !newAddress || !userId || !txHash) {🤖 Prompt for AI Agents
In `@app/api/v1/wallets/deprecate/route.ts` around lines 44 - 54, The request
validation in this route is missing txHash: ensure txHash is treated as a
required field along with walletAddress, oldAddress, newAddress, and userId by
updating the conditional that checks presence (where walletAddress, oldAddress,
newAddress, userId are validated) to also validate txHash, and update the error
path (trackApiError and NextResponse.json) to reflect the missing txHash; this
ensures migration_tx_hash stored later is never null and points to the same
identifiers (walletAddress, oldAddress, newAddress, txHash, userId) used in the
handler.
| // Create or update new EOA wallet record | ||
| const { error: upsertError } = await supabaseAdmin | ||
| .from("wallets") | ||
| .upsert({ | ||
| address: newAddress.toLowerCase(), | ||
| user_id: userId, | ||
| wallet_type: "eoa", | ||
| status: "active", | ||
| created_at: new Date().toISOString(), | ||
| }); |
There was a problem hiding this comment.
Second upsert is missing onConflict, risking failure on retries.
The first upsert (Line 83-97) correctly specifies { onConflict: "address,user_id" }, but this upsert for the new EOA wallet does not. If the endpoint is called again (e.g., client retry after a timeout), this upsert will either fail on a unique constraint violation or insert a duplicate row depending on table constraints, instead of gracefully updating the existing record.
Proposed fix
const { error: upsertError } = await supabaseAdmin
.from("wallets")
.upsert({
address: newAddress.toLowerCase(),
user_id: userId,
wallet_type: "eoa",
status: "active",
- created_at: new Date().toISOString(),
- });
+ updated_at: new Date().toISOString(),
+ }, { onConflict: "address,user_id" });🤖 Prompt for AI Agents
In `@app/api/v1/wallets/deprecate/route.ts` around lines 104 - 113, The second
upsert call using supabaseAdmin (the block creating/updating the new EOA wallet
with address/newAddress and user_id/userId) is missing the onConflict option and
may fail or create duplicates on retries; update that upsert to include the same
conflict target used earlier (onConflict: "address,user_id") so retries will
perform an update instead of causing unique constraint errors or duplicate rows.
| if (upsertError) { | ||
| // Rollback: Restore old wallet status | ||
| await supabaseAdmin | ||
| .from("wallets") | ||
| .update({ | ||
| status: "active", | ||
| deprecated_at: null, | ||
| migration_completed: false, | ||
| migration_tx_hash: null, | ||
| }) | ||
| .eq("address", oldAddress.toLowerCase()) | ||
| .eq("user_id", userId); | ||
|
|
||
| trackApiError(request, "/api/v1/wallets/deprecate", "POST", upsertError, 500); | ||
| throw upsertError; | ||
| } |
There was a problem hiding this comment.
Rollback failure is silently ignored.
If the rollback update on Lines 117-126 fails (e.g., network issue, DB outage), the error is never logged or surfaced. The code proceeds to throw upsertError, but the old wallet remains in a deprecated state with no record of the rollback failure.
Proposed fix
if (upsertError) {
// Rollback: Restore old wallet status
- await supabaseAdmin
+ const { error: rollbackError } = await supabaseAdmin
.from("wallets")
.update({
status: "active",
deprecated_at: null,
migration_completed: false,
migration_tx_hash: null,
})
.eq("address", oldAddress.toLowerCase())
.eq("user_id", userId);
+ if (rollbackError) {
+ console.error("Rollback failed after upsert error:", rollbackError);
+ }
+
trackApiError(request, "/api/v1/wallets/deprecate", "POST", upsertError, 500);
throw upsertError;
}📝 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.
| if (upsertError) { | |
| // Rollback: Restore old wallet status | |
| await supabaseAdmin | |
| .from("wallets") | |
| .update({ | |
| status: "active", | |
| deprecated_at: null, | |
| migration_completed: false, | |
| migration_tx_hash: null, | |
| }) | |
| .eq("address", oldAddress.toLowerCase()) | |
| .eq("user_id", userId); | |
| trackApiError(request, "/api/v1/wallets/deprecate", "POST", upsertError, 500); | |
| throw upsertError; | |
| } | |
| if (upsertError) { | |
| // Rollback: Restore old wallet status | |
| const { error: rollbackError } = await supabaseAdmin | |
| .from("wallets") | |
| .update({ | |
| status: "active", | |
| deprecated_at: null, | |
| migration_completed: false, | |
| migration_tx_hash: null, | |
| }) | |
| .eq("address", oldAddress.toLowerCase()) | |
| .eq("user_id", userId); | |
| if (rollbackError) { | |
| console.error("Rollback failed after upsert error:", rollbackError); | |
| } | |
| trackApiError(request, "/api/v1/wallets/deprecate", "POST", upsertError, 500); | |
| throw upsertError; | |
| } |
🤖 Prompt for AI Agents
In `@app/api/v1/wallets/deprecate/route.ts` around lines 115 - 130, The rollback
update against the "wallets" table can itself fail and is currently ignored;
wrap the supabaseAdmin.from("wallets").update(...) call in a try/catch, and in
the catch use trackApiError(request, "/api/v1/wallets/deprecate", "POST",
rollbackError, 500) (or similar) to log the rollback failure along with context
(include oldAddress and userId), then either attach the rollback error to the
thrown upsertError (e.g., include both errors in a combined error or add a
property) or throw a new Error that preserves both upsertError and
rollbackError; ensure you still throw after logging so the original failure is
reported to the caller.
| export async function GET(request: NextRequest) { | ||
| try { | ||
| const { searchParams } = new URL(request.url); | ||
| const userId = searchParams.get("userId"); | ||
|
|
||
| if (!userId) { | ||
| return NextResponse.json({ error: "User ID required" }, { status: 400 }); | ||
| } |
There was a problem hiding this comment.
No authentication or authorization — any caller can query any user's migration status.
The userId is taken directly from the query string with no verification that the caller is the corresponding authenticated user. This exposes migration status to unauthenticated probing. Add Privy token validation (or similar auth middleware) and verify the authenticated user matches the requested userId.
🤖 Prompt for AI Agents
In `@app/api/v1/wallets/migration-status/route.ts` around lines 4 - 11, The GET
handler exposes any user's migration status because it trusts the query param
userId directly; update the GET function (export async function GET) to validate
the caller's authentication token (e.g., verify Privy token from the request
headers) and extract the authenticated user id, then compare that id to the
requested searchParams.get("userId"); if the token is missing/invalid return a
401 and if the ids do not match return a 403, otherwise proceed with the
existing logic. Ensure you call the existing token verification utility or
implement a small verifyPrivyToken helper, use NextRequest headers to read the
auth token and NextResponse to return the proper status codes, and fail closed
on any verification error.
| // Check if user has completed migration | ||
| const { data, error } = await supabaseAdmin | ||
| .from("wallets") | ||
| .select("migration_completed, status, wallet_type") | ||
| .eq("user_id", userId) | ||
| .eq("wallet_type", "smart_contract") | ||
| .single(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Finding SQL migration files ==="
fd -e sql
echo ""
echo "=== Checking for CREATE TABLE statements in migrations ==="
rg -n "CREATE TABLE" --type sql -A 10
echo ""
echo "=== Checking for wallets table definition ==="
rg -n "wallets" --type sql -B 2 -A 5Repository: paycrest/noblocks
Length of output: 2959
🏁 Script executed:
#!/bin/bash
echo "=== Content of wallet_migrations.sql ==="
cat supabase/migrations/create_wallet_migrations.sql
echo ""
echo "=== Full content of migration-status route ==="
cat app/api/v1/wallets/migration-status/route.ts
echo ""
echo "=== Searching for 'wallets' table reference elsewhere ==="
rg -n "\.from\(['\"]wallets['\"]" --type tsRepository: paycrest/noblocks
Length of output: 6642
Create the wallets table or update the route to query wallet_migrations with correct column mappings.
The code queries a non-existent .from("wallets") table. SQL migrations only define wallet_migrations, not wallets. Additionally, the queried columns (migration_completed, status, wallet_type) and filter (user_id) do not match the wallet_migrations schema, which uses privy_user_id and has status values of 'pending', 'in_progress', 'completed', or 'failed'. Either:
- Create a new SQL migration for the
walletstable with the required columns, or - Update the route to query
wallet_migrationsinstead and map column names correctly (e.g.,privy_user_idinstead ofuser_id, adjust status field logic)
The PGRST205 error handling suggests this was known, but the route will fail unless the missing table is created or the query is corrected. Multiple other routes (app/api/v1/wallets/deprecate/route.ts) also reference the non-existent wallets table.
🤖 Prompt for AI Agents
In `@app/api/v1/wallets/migration-status/route.ts` around lines 13 - 19, The route
currently queries a non-existent table via supabaseAdmin.from("wallets") and
requests columns that don't match your migrations; update
app/api/v1/wallets/migration-status/route.ts to query "wallet_migrations"
instead of "wallets", select the correct columns (e.g., .select("privy_user_id,
status") or whatever exists in the migration schema), replace .eq("user_id",
userId) with .eq("privy_user_id", userId), and treat migration completion by
checking status === "completed" (or map status values
'pending'|'in_progress'|'completed'|'failed' appropriately) rather than relying
on a non-existent migration_completed field; also apply the same fix to other
routes referencing supabaseAdmin.from("wallets") such as
app/api/v1/wallets/deprecate/route.ts or create a new SQL migration to add a
wallets table with the expected columns if you prefer that approach.
| export function parseEip7702AuthorizedAddress( | ||
| code: string | null | undefined | ||
| ): Address | null { | ||
| if (!code || code === "0x" || code === "0x0") return null; | ||
| const normalized = code.toLowerCase(); | ||
| const idx = normalized.indexOf(EIP7702_MAGIC_PREFIX); | ||
| if (idx === -1) return null; | ||
| return `0x${normalized.slice(idx + EIP7702_MAGIC_PREFIX.length, idx + EIP7702_MAGIC_PREFIX.length + 40)}` as Address; | ||
| } |
There was a problem hiding this comment.
parseEip7702AuthorizedAddress does not validate that enough bytes follow the prefix.
If the code string is truncated (e.g., only a few characters after the magic prefix), the slice on line 32 will silently return a malformed, shorter-than-20-byte address. Add a length check.
🛡️ Proposed fix
export function parseEip7702AuthorizedAddress(
code: string | null | undefined
): Address | null {
if (!code || code === "0x" || code === "0x0") return null;
const normalized = code.toLowerCase();
const idx = normalized.indexOf(EIP7702_MAGIC_PREFIX);
if (idx === -1) return null;
- return `0x${normalized.slice(idx + EIP7702_MAGIC_PREFIX.length, idx + EIP7702_MAGIC_PREFIX.length + 40)}` as Address;
+ const addrStart = idx + EIP7702_MAGIC_PREFIX.length;
+ if (normalized.length < addrStart + 40) return null;
+ return `0x${normalized.slice(addrStart, addrStart + 40)}` as Address;
}📝 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.
| export function parseEip7702AuthorizedAddress( | |
| code: string | null | undefined | |
| ): Address | null { | |
| if (!code || code === "0x" || code === "0x0") return null; | |
| const normalized = code.toLowerCase(); | |
| const idx = normalized.indexOf(EIP7702_MAGIC_PREFIX); | |
| if (idx === -1) return null; | |
| return `0x${normalized.slice(idx + EIP7702_MAGIC_PREFIX.length, idx + EIP7702_MAGIC_PREFIX.length + 40)}` as Address; | |
| } | |
| export function parseEip7702AuthorizedAddress( | |
| code: string | null | undefined | |
| ): Address | null { | |
| if (!code || code === "0x" || code === "0x0") return null; | |
| const normalized = code.toLowerCase(); | |
| const idx = normalized.indexOf(EIP7702_MAGIC_PREFIX); | |
| if (idx === -1) return null; | |
| const addrStart = idx + EIP7702_MAGIC_PREFIX.length; | |
| if (normalized.length < addrStart + 40) return null; | |
| return `0x${normalized.slice(addrStart, addrStart + 40)}` as Address; | |
| } |
🤖 Prompt for AI Agents
In `@app/hooks/useEIP7702Account.ts` around lines 25 - 33, The function
parseEip7702AuthorizedAddress can return a shortened address if the input is
truncated; update it to validate that there are at least 40 hex characters (20
bytes) after the EIP7702_MAGIC_PREFIX before slicing. Specifically, in
parseEip7702AuthorizedAddress compute start = idx + EIP7702_MAGIC_PREFIX.length
and check that normalized.length >= start + 40 (or otherwise return null); then
slice exactly 40 characters and return `0x${...}` as Address.
|
|
||
| try { | ||
| const response = await fetch(`/api/v1/wallets/migration-status?userId=${user.id}`); | ||
| if (response.ok) { | ||
| const data = await response.json(); | ||
| setIsMigrationComplete(data.migrationCompleted ?? false); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the migration-status route
find . -type f -name "*.ts" -o -name "*.js" | xargs grep -l "migration-status" | head -20Repository: paycrest/noblocks
Length of output: 1316
🏁 Script executed:
#!/bin/bash
# Search for the API route in typical Next.js locations
find ./app -type f \( -name "*.ts" -o -name "*.js" \) -path "*api*" 2>/dev/null | grep -i wallet | head -20Repository: paycrest/noblocks
Length of output: 194
🏁 Script executed:
#!/bin/bash
# Read the migration-status route implementation
cat -n ./app/api/v1/wallets/migration-status/route.tsRepository: paycrest/noblocks
Length of output: 4228
🏁 Script executed:
#!/bin/bash
# Check for middleware that might protect this route
find ./app -type f -name "middleware.ts" -o -name "middleware.js" 2>/dev/null | xargs cat -n 2>/dev/null | head -50Repository: paycrest/noblocks
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check for route groups or auth protection in the wallets API directory
ls -la ./app/api/v1/wallets/ 2>/dev/null
cat -n ./app/api/v1/wallets/layout.ts 2>/dev/null || cat -n ./app/api/v1/middleware.ts 2>/dev/null || echo "No layout/middleware found"Repository: paycrest/noblocks
Length of output: 319
🏁 Script executed:
#!/bin/bash
# Check for middleware at root and app level
ls -la middleware.ts 2>/dev/null || echo "No root middleware"
find ./app -maxdepth 2 -name "middleware.ts" 2>/dev/nullRepository: paycrest/noblocks
Length of output: 123
🏁 Script executed:
#!/bin/bash
# Check if there's any auth library being used and any auth context
grep -r "getCurrentUser\|getSession\|auth()" ./app/api/v1/wallets/migration-status/ 2>/dev/null || echo "No auth checks found"Repository: paycrest/noblocks
Length of output: 81
🏁 Script executed:
#!/bin/bash
# Read the middleware.ts to check for auth protection
cat -n middleware.ts | head -100Repository: paycrest/noblocks
Length of output: 3675
🏁 Script executed:
#!/bin/bash
# Read the rest of middleware.ts to see routing logic
cat -n middleware.ts | tail -80Repository: paycrest/noblocks
Length of output: 2964
Add JWT authentication to the migration-status endpoint or enforce middleware protection.
The /api/v1/wallets/migration-status route accepts userId as an unprotected query parameter with no authentication required. The route is not included in the middleware.ts matcher (which protects /api/v1/account, /api/v1/transactions, etc.), allowing any unauthenticated request to query any user's migration status. Add the route to the middleware matcher (e.g., /api/v1/wallets/:path*) or implement explicit JWT validation in the route handler to verify the authenticated user owns the requested userId.
🤖 Prompt for AI Agents
In `@app/hooks/useEIP7702Account.ts` around lines 86 - 92, The migration-status
endpoint is being called without authentication from useEIP7702Account (the
fetch call to `/api/v1/wallets/migration-status?userId=${user.id}`), so either
protect the route server-side or send and validate a JWT: on the server add the
route to your middleware matcher (e.g., `/api/v1/wallets/:path*`) or implement
JWT validation in the migration-status handler to ensure the token's subject
matches the requested userId; on the client (useEIP7702Account) include the
Authorization header with the user's JWT when calling the endpoint (fetch(..., {
headers: { Authorization: `Bearer ${token}` } })) so the handler can verify
ownership before returning migrationCompleted.
| const meeClient = await createMeeClient({ | ||
| account: nexusAccount, | ||
| apiKey: process.env.NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY ?? "", | ||
| }); |
There was a problem hiding this comment.
Empty-string fallback for Biconomy API key will produce a cryptic error.
If NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY is unset, ?? "" passes an empty string to the SDK, which will likely fail deep inside Biconomy's internals with an unhelpful message. Fail fast with a clear error instead.
Proposed fix
+ const biconomyApiKey = process.env.NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY;
+ if (!biconomyApiKey) throw new Error("Biconomy paymaster API key is not configured");
const meeClient = await createMeeClient({
account: nexusAccount,
- apiKey: process.env.NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY ?? "",
+ apiKey: biconomyApiKey,
});📝 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.
| const meeClient = await createMeeClient({ | |
| account: nexusAccount, | |
| apiKey: process.env.NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY ?? "", | |
| }); | |
| const biconomyApiKey = process.env.NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY; | |
| if (!biconomyApiKey) throw new Error("Biconomy paymaster API key is not configured"); | |
| const meeClient = await createMeeClient({ | |
| account: nexusAccount, | |
| apiKey: biconomyApiKey, | |
| }); |
🤖 Prompt for AI Agents
In `@app/hooks/useSmartWalletTransfer.ts` around lines 157 - 160, In
useSmartWalletTransfer, avoid passing an empty string to createMeeClient when
the Biconomy API key is missing; instead validate
process.env.NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY before calling createMeeClient
and throw or return a clear error (e.g., throw new Error indicating the missing
NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY) so callers fail fast with an actionable
message; update the code around the createMeeClient(...) call to use the
validated key variable (or bail out) rather than using the ?? "" fallback.
| } else if (shouldUseEOA && embeddedWallet) { | ||
| // EIP-7702 + Biconomy MEE path (migrated EOA or 0-balance SCW) | ||
| const chain = selectedNetwork?.chain; | ||
| if (!chain) throw new Error("Network not ready"); | ||
| const chainId = chain.id; | ||
| const gatewayAddress = getGatewayContractAddress( | ||
| selectedNetwork.chain.name, | ||
| ) as `0x${string}`; | ||
|
|
||
| await embeddedWallet.switchChain(chainId); | ||
|
|
||
| const provider = await embeddedWallet.getEthereumProvider(); | ||
| const authorization = await signBiconomyAuthorization(chainId); | ||
|
|
||
| // Match Biconomy example: signer is EIP-1193 provider so SDK uses eth_requestAccounts (no getAddresses) | ||
| const nexusAccount = await toMultichainNexusAccount({ | ||
| chainConfigurations: [ | ||
| { | ||
| chain, | ||
| transport: http(getRpcUrl(selectedNetwork.chain.name)), | ||
| version: getMEEVersion(MEEVersion.V2_1_0), | ||
| accountAddress: embeddedWallet.address as `0x${string}`, | ||
| }, | ||
| ], | ||
| signer: provider, | ||
| }); | ||
|
|
||
| const meeClient = await createMeeClient({ | ||
| account: nexusAccount, | ||
| apiKey: process.env.NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY ?? "", | ||
| }); |
There was a problem hiding this comment.
NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY falls back to empty string — MEE client will fail opaquely.
Line 330 defaults to "" if the env var is missing. An empty API key will cause the Biconomy MEE client to reject requests at runtime with an unclear error. Consider validating the key before proceeding and throwing a clear error.
🛡️ Proposed guard
+ const biconomyApiKey = process.env.NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY;
+ if (!biconomyApiKey) {
+ throw new Error("Biconomy paymaster API key is not configured");
+ }
+
const meeClient = await createMeeClient({
account: nexusAccount,
- apiKey: process.env.NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY ?? "",
+ apiKey: biconomyApiKey,
});🤖 Prompt for AI Agents
In `@app/pages/TransactionPreview.tsx` around lines 301 - 331, The code passes
NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY (possibly empty) into createMeeClient which
will fail opaquely; before calling createMeeClient (in the block around
shouldUseEOA && embeddedWallet) validate
process.env.NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY is present and non-empty and
throw a clear Error (e.g., "Missing Biconomy paymaster API key: set
NEXT_PUBLIC_BICONOMY_PAYMASTER_KEY") if not, so createMeeClient is only invoked
with a real apiKey; update any log/exception handling around meeClient creation
to surface this validation.
| const { hash } = await meeClient.execute({ | ||
| authorizations: [authorization], | ||
| delegate: true, | ||
| sponsorship: true, | ||
| instructions: [approveInstruction, createOrderInstruction], | ||
| }); | ||
| setIsGatewayApproved(true); | ||
| setIsOrderCreated(true); | ||
|
|
||
| await meeClient.waitForSupertransactionReceipt({ hash }); | ||
|
|
||
| trackEvent("Swap started", { | ||
| "Entry point": "Transaction preview", | ||
| "Wallet type": "EIP-7702 (MEE)", | ||
| }); |
There was a problem hiding this comment.
Success state set before transaction receipt is confirmed; risk of false-positive UI.
Lines 375-376 set isGatewayApproved and isOrderCreated to true immediately after meeClient.execute(), but before waitForSupertransactionReceipt (line 378) confirms on-chain success. If the super-transaction fails or reverts, the UI will incorrectly display the order as created. Move the state updates after receipt confirmation, similar to the injected-wallet path which waits for receipts.
🐛 Proposed fix
const { hash } = await meeClient.execute({
authorizations: [authorization],
delegate: true,
sponsorship: true,
instructions: [approveInstruction, createOrderInstruction],
});
- setIsGatewayApproved(true);
- setIsOrderCreated(true);
await meeClient.waitForSupertransactionReceipt({ hash });
+ setIsGatewayApproved(true);
+ setIsOrderCreated(true);
trackEvent("Swap started", {
"Entry point": "Transaction preview",
"Wallet type": "EIP-7702 (MEE)",
});📝 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.
| const { hash } = await meeClient.execute({ | |
| authorizations: [authorization], | |
| delegate: true, | |
| sponsorship: true, | |
| instructions: [approveInstruction, createOrderInstruction], | |
| }); | |
| setIsGatewayApproved(true); | |
| setIsOrderCreated(true); | |
| await meeClient.waitForSupertransactionReceipt({ hash }); | |
| trackEvent("Swap started", { | |
| "Entry point": "Transaction preview", | |
| "Wallet type": "EIP-7702 (MEE)", | |
| }); | |
| const { hash } = await meeClient.execute({ | |
| authorizations: [authorization], | |
| delegate: true, | |
| sponsorship: true, | |
| instructions: [approveInstruction, createOrderInstruction], | |
| }); | |
| await meeClient.waitForSupertransactionReceipt({ hash }); | |
| setIsGatewayApproved(true); | |
| setIsOrderCreated(true); | |
| trackEvent("Swap started", { | |
| "Entry point": "Transaction preview", | |
| "Wallet type": "EIP-7702 (MEE)", | |
| }); |
🤖 Prompt for AI Agents
In `@app/pages/TransactionPreview.tsx` around lines 369 - 383, The code sets UI
success flags too early: move the calls to setIsGatewayApproved(true) and
setIsOrderCreated(true) so they occur only after
meeClient.waitForSupertransactionReceipt({ hash }) confirms the transaction
receipt (and ideally after validating the receipt indicates success). Update the
TransactionPreview flow around meeClient.execute and
meeClient.waitForSupertransactionReceipt to perform the state updates
(setIsGatewayApproved, setIsOrderCreated) after the wait/receipt check (and
optionally move trackEvent("Swap started", ...) to run post-confirmation to
match the injected-wallet path).
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 7 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on February 26
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| }); | ||
|
|
||
| onClose(); | ||
| setTimeout(() => setShowSuccessModal(true), 300); |
There was a problem hiding this comment.
Migration deprecates wallet even when all transfers fail
High Severity
The per-chain try/catch inside the transfer loop (line 305) silently swallows chain transfer failures and continues. After the loop, the deprecate API call at line 319 always executes regardless of how many chains failed. The success modal and toast also always appear. This means if some or all chain transfers fail, the old SCW is still marked as deprecated in the database and the user sees "Migration Complete!" — but their tokens remain stranded in the now-deprecated wallet. The totalTokensMigrated counter is only used in the toast description text, not in any control flow to prevent deprecation.
Additional Locations (1)
| const value = smartWalletBalance === 0; | ||
| lastValueRef.current = value; | ||
| return value; | ||
| } |
There was a problem hiding this comment.
Wallet address flickers based on selected network
High Severity
useShouldUseEOA decides whether to show the EOA or SCW address based on allBalances.smartWallet?.total, which reflects the balance for the currently selected network only. For a non-migrated user with SCW funds on Base but zero on Polygon, switching to Polygon causes the hook to return true (show EOA), and switching to Base returns false (show SCW). This makes the displayed wallet address in the Navbar, SettingsDropdown, MobileDropdown, and other components change depending on which network is selected — a confusing and potentially dangerous behavior since users may copy and share a different address than where their funds reside.
| const amountInWei = parseUnits( | ||
| token.amount.toString(), | ||
| token.decimals | ||
| ); |
There was a problem hiding this comment.
Float-to-string may produce scientific notation breaking parseUnits
Medium Severity
token.amount is a JavaScript number from floating-point division (Number(balanceInWei) / Math.pow(10, decimals) in fetchWalletBalance). Calling .toString() on very small values produces scientific notation (e.g., "1e-18" for dust in 18-decimal tokens), which viem's parseUnits cannot parse and will throw. Since this happens inside chainTokens.map(), one dust-balance token causes the entire chain's migration batch to fail — and due to the silent catch above, the wallet still gets deprecated.
| .select("migration_completed, status, wallet_type") | ||
| .eq("user_id", userId) | ||
| .eq("wallet_type", "smart_contract") | ||
| .single(); |
There was a problem hiding this comment.
Migration status endpoint lacks authentication check
Medium Severity
The /api/v1/wallets/migration-status GET endpoint accepts a userId query parameter and queries the database without any authentication or authorization checks. Any unauthenticated caller can look up any user's migration status, wallet type, and whether they have a smart wallet. This is inconsistent with the /api/v1/wallets/deprecate endpoint, which properly verifies JWT authentication and enforces user ID matching.
| transport: http(rpcUrl), | ||
| }); | ||
| return get7702ImplementationAddress(publicClient, address); | ||
| } |
There was a problem hiding this comment.
Unused exported EIP-7702 helper functions
Low Severity
Three exported functions — parseEip7702AuthorizedAddress, get7702ImplementationAddress, and get7702AuthorizedImplementationForAddress — are defined and exported but never imported or called anywhere in the codebase. These are dead code that adds maintenance burden without providing any current functionality.
| const handlePaymentConfirmation = async () => { | ||
| // Check balance including sender fee | ||
| const totalRequired = amountSent + senderFeeAmount; | ||
|
|
There was a problem hiding this comment.
MEE path fires duplicate analytics event with wrong type
Medium Severity
The new MEE code path (lines 380-383) correctly fires a trackEvent("Swap started") with "Wallet type": "EIP-7702 (MEE)". But after the if/else-if/else block closes at line 437, lines 445-448 run unconditionally for all paths, firing a second trackEvent("Swap started") with "Wallet type": "Smart wallet". For MEE transactions, this produces a duplicate analytics event attributed to the wrong wallet type, corrupting swap analytics data.
Additional Locations (1)
| wallet_type: "eoa", | ||
| status: "active", | ||
| created_at: new Date().toISOString(), | ||
| }); |
There was a problem hiding this comment.
EOA wallet upsert missing onConflict causes duplicate rows
Medium Severity
The second upsert call (for the new EOA wallet) omits the onConflict: "address,user_id" option that the first upsert (line 96) explicitly includes. Without onConflict, Supabase defaults to conflict detection on the primary key. If the primary key is an auto-generated id not present in the payload, the upsert always inserts a new row instead of updating, creating duplicate wallet records on retries or re-migrations.
1 participant
Description
This PR implements a comprehensive wallet migration system to transition existing Noblocks users from Smart Contract Wallets (SCW) to EIP-7702-enabled Externally Owned Accounts (EOA). This migration ensures wallet portability and aligns with the latest Ethereum standards while maintaining a seamless user experience.
Key Features Implemented:
EIP-7702 Integration for New Users
privy-config.tswithcreateOnLogin: "all-users"to ensure all new accounts are EIP-7702-readyMigration Banner & Modal System
WalletTransferApprovalModal) displays all tokens across all supported networksMulti-Chain Balance Aggregation
Gasless Migration Transactions
References
Checklist
mainAdded Files:
app/api/v1/wallets/deprecate/route.tsapp/api/v1/wallets/migration-status/route.tsapp/components/WalletMigrationBanner.tsxapp/components/WalletMigrationModal.tsxapp/components/WalletMigrationSuccessModal.tsxapp/components/WalletTransferApprovalModal.tsxapp/context/MigrationContext.tsxapp/hooks/useEIP7702Account.tssupabase/migrations/create_wallet_migrations.sqlpublic/images/By submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.
Summary by CodeRabbit
New Features
Chores
Bug Fixes / Reliability