feat: implement wallet migration feature with modals and context support #334
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: new backend routes to deprecate wallets and query migration status, a DB ledger for migrations with RLS, client hooks/context to detect migration need, UI components to guide and run multi-chain fund transfers, and integration with Privy for batched gasless transfers and post-migration upsert/rollback logic. 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 Backend as Backend API
participant SC as Old Smart Contract Wallet
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 SC
Privy-->>UI_Transfer: return tokens & balances
User->>UI_Transfer: confirm transfer
par Per-chain batch transfer
UI_Transfer->>Privy: switch to chain N
Privy-->>UI_Transfer: chain switched
UI_Transfer->>Privy: submit batched gasless transfer tx
Privy->>SC: execute transfer from old SCW -> new EOA
SC-->>Privy: tx receipt
end
UI_Transfer->>Backend: POST /api/v1/wallets/deprecate (old, new, txHashes)
Backend->>Backend: deprecate old, upsert new, attempt KYC migration (log non-fatal)
Backend-->>UI_Transfer: success / partial-success
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 and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
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.
…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.
| if (tokenMeta?.address) { | ||
| // Get CNGN rate for this chain if needed | ||
| const chainRate = symbol.toUpperCase() === "CNGN" ? rate : undefined; | ||
| let usdValue = balanceNum; | ||
| if (chainRate) { | ||
| usdValue = balanceNum / chainRate; | ||
| } |
There was a problem hiding this comment.
CNGN rate applied incorrectly across chains.
The rate from useCNGNRate is tied to selectedNetwork, but this logic applies it to CNGN tokens on all chains. If the user has CNGN on multiple networks with different rates, the USD values will be incorrect for chains other than the selected network.
🔎 Suggested approach
Consider fetching CNGN rates per chain, or filtering CNGN conversion to only apply when chainName === selectedNetwork.chain.name:
// Get CNGN rate for this chain if needed
- const chainRate = symbol.toUpperCase() === "CNGN" ? rate : undefined;
+ // Only apply rate if this chain matches selectedNetwork (rate is network-specific)
+ const chainRate = symbol.toUpperCase() === "CNGN" && chainName === selectedNetwork.chain.name ? rate : undefined;
let usdValue = balanceNum;
if (chainRate) {
usdValue = balanceNum / chainRate;
+ } else if (symbol.toUpperCase() === "CNGN") {
+ // For CNGN on other chains, we don't have the rate - show raw balance or fetch rate
+ usdValue = balanceNum; // Consider fetching per-chain rate
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/components/WalletTransferApprovalModal.tsx around lines 114 to 120, the
code applies the single `rate` from useCNGNRate to any token with symbol "CNGN"
regardless of chain, causing incorrect USD values for CNGN on chains other than
the currently selected network; update the logic to only apply the CNGN
conversion when the token's chain matches the selectedNetwork (e.g., chainName
=== selectedNetwork.chain.name) or fetch/lookup a per-chain CNGN rate keyed by
chain and use that rate for the conversion, otherwise leave usdValue as
balanceNum.
| toast.success("🎉 Migration Complete!", { | ||
| description: hasTokens | ||
| ? `${allTxHashes.length} token(s) successfully migrated to your EOA` | ||
| : "Old wallet deprecated successfully", | ||
| duration: 5000, | ||
| }); |
There was a problem hiding this comment.
Misleading success message count.
allTxHashes.length represents the number of successful chain transactions, not the total number of tokens migrated. This could confuse users.
🔎 Proposed fix
toast.success("🎉 Migration Complete!", {
description: hasTokens
- ? `${allTxHashes.length} token(s) successfully migrated to your EOA`
+ ? `Assets from ${allTxHashes.length} chain(s) successfully migrated to your EOA`
: "Old wallet deprecated successfully",
duration: 5000,
});🤖 Prompt for AI Agents
In app/components/WalletTransferApprovalModal.tsx around lines 309 to 314, the
toast success message uses allTxHashes.length (number of successful chain
transactions) which is misleading as it does not reflect the actual number of
tokens migrated; change the message to use the true token count (e.g., a
migratedTokens.length or computed totalTokensMigrated variable) or otherwise
compute the number of tokens migrated from the migration result, and update the
description string (with proper pluralization) so it reports the token count
(and optionally also show transaction count like "X tokens migrated (Y txs)").
| <> | ||
| <AnimatePresence> | ||
| {isOpen && ( | ||
| <Dialog open={isOpen} onClose={onClose} className="relative z-50"> |
There was a problem hiding this comment.
Dialog can be closed during processing via backdrop click.
The close button is properly disabled during isProcessing, but clicking the backdrop still calls onClose. This could confuse users if they accidentally dismiss the modal mid-migration.
🔎 Proposed fix
- <Dialog open={isOpen} onClose={onClose} className="relative z-50">
+ <Dialog open={isOpen} onClose={isProcessing ? () => {} : onClose} className="relative z-50">📝 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.
| <Dialog open={isOpen} onClose={onClose} className="relative z-50"> | |
| <Dialog open={isOpen} onClose={isProcessing ? () => {} : onClose} className="relative z-50"> |
🤖 Prompt for AI Agents
In app/components/WalletTransferApprovalModal.tsx around line 339, the Dialog's
onClose currently allows backdrop/escape clicks to close the modal during
processing; change the onClose handler so it ignores close requests while
isProcessing is true (i.e., replace onClose directly with a wrapper that returns
early when isProcessing) so backdrop/escape clicks become no-ops during
processing; keep the close button disabled as-is and ensure any other internal
close triggers also call the same guarded onClose wrapper.
| -- 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.
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
✏️ Tip: You can customize this high-level summary in your review settings.