app/components/recipient/AddBeneficiaryModal.tsx (2)

64-75: Consider varying the avatar based on wallet address.

Currently, getAvatarImage(0) always displays the same avatar for all wallet beneficiaries. For better visual differentiation, consider deriving the avatar index from the wallet address (e.g., hash the address to select one of the 4 available avatars).

🔎 Suggested enhancement

+// Simple hash function to derive avatar index from wallet address
+const getAvatarIndex = (address: string): number => {
+  const hash = address.split('').reduce((acc, char) => acc + char.charCodeAt(0), 0);
+  return hash % 4;
+};
+
 <div className="flex justify-center">
   <div className="flex size-16 flex-shrink-0 items-center justify-center overflow-hidden rounded-full">
     <Image
-      src={getAvatarImage(0)}
+      src={getAvatarImage(getAvatarIndex(walletAddress))}
       alt="Wallet avatar"
       width={64}
       height={64}
       className="size-full object-cover"
     />
   </div>
 </div>

---

78-117: Input and buttons work well with minor style considerations.

The input field and button layout function correctly with good UX (autoFocus, Enter key handling, disabled states). A few optional improvements:

1. Inline styles (lines 100, 113): Consider using Tailwind width classes instead of inline styles for consistency:

   style={{ width: "35%" }}  // Could be: className="w-[35%]"
   style={{ width: "65%" }}  // Could be: className="w-[65%]"

2. Input text color (line 83): text-white is hardcoded, but the input is on a transparent background. Verify this works correctly in light theme, or add a dark: variant if needed.

app/components/recipient/RecipientListItem.tsx (1)

16-107: Wallet recipient rendering and ENS resolution look solid; consider minor defensive reset

The wallet vs bank/mobile-money branching, ENS resolution, and delete interactions all look correct and match the new RecipientDetails union. One small hardening you could add is clearing displayName / isResolvingEns when walletAddress becomes falsy so a reused list item can’t briefly show a stale ENS/short address while props change.

Example tweak inside the effect:

useEffect(() => {
  if (!isWalletRecipient || !walletAddress) {
    setDisplayName("");
    setIsResolvingEns(false);
    return;
  }
  // existing ENS resolution...
}, [isWalletRecipient, walletAddress]);

app/utils.ts (1)

88-111: New utility helpers are correct; a couple of small robustness/perf tweaks to consider

Implementation of getCurrencySymbol, resolveEnsNameOrShorten, getAvatarImage, and copyToClipboard looks functionally sound and aligns with how they’re used in the UI.

Two optional improvements:

• Avoid recreating static maps on every call: lift currencySymbols out of getCurrencySymbol into a module‑level constant to reduce allocations on hot paths.
• Trim wallet inputs before validation/shortening: in resolveEnsNameOrShorten, consider const trimmed = address.trim(); and validate/operate on trimmed so pasted addresses with stray spaces don’t get rejected or oddly sliced.

Otherwise these helpers are good to go and integrate cleanly with the new onramp UI.

Also applies to: 602-650, 1305-1329

app/components/recipient/RecipientDetailsForm.tsx (1)

31-42: Wallet recipient mode and saved‑recipient handling are correct; consider trimming input and reusing network‑name helpers

The new onramp path here is well put together:

• isSwapped cleanly switches between walletAddress input and the existing bank/mobile‑money fields.
• Saved recipients are correctly split into walletRecipients vs bankRecipients, and both selectSavedRecipient and deleteRecipient handle the union type without breaking existing behavior.
• The “Select beneficiary” button and modal wiring align with those filtered lists, so the user only sees relevant recipients in each mode.
• Network context next to the wallet field (icon + “You are on … network”) is a nice safety affordance.

Two minor refinements:

1. Trim before validating walletAddress
   In the wallet register block you validate with isValidEvmAddressCaseInsensitive(value). If users paste addresses with leading/trailing spaces, they’ll get a spurious “Invalid wallet address format”. Doing const trimmed = value.trim(); inside validate and checking trimmed (while still storing the trimmed value) would make this more forgiving.

2. Avoid hard‑coded network map when you already have display names
   formatNetworkName maintains a small map of slugs to display strings, but networkName here is already a human‑readable name (e.g., “Base”, “Arbitrum One”). You could simplify to just networkName or, if you need slug→name behavior elsewhere, reuse the existing normalizeNetworkName utility instead of another partial map.

Overall, the onramp recipient UX and data wiring look good.

Also applies to: 50-58, 90-92, 121-148, 152-212, 379-401, 403-597

app/components/recipient/SavedBeneficiariesModal.tsx (1)

13-26: Wallet-aware filtering and rendering are correct; a couple of UX enhancements to consider

The modal’s new behavior aligns with the onramp flow:

• isSwapped cleanly switches between wallet-only and bank-only filtering.
• Wallet recipients are deduplicated by walletAddress, which avoids showing the same address multiple times.
• List keys now use walletAddress for wallets vs accountIdentifier for banks, and index is passed through to RecipientListItem as required.

Two optional UX improvements:

• Search by name for wallets as well as address: in the swapped branch you currently match only on walletAddress. Including recipient.name in the filter (when present) would make it easier to find saved wallets by label.
• Scope networkName in the title to wallet mode: Saved beneficiaries (${networkName}) is great context for wallets, but slightly confusing for bank/mobile-money mode where the network is irrelevant. You might wrap the interpolation with isSwapped && networkName so it only appears when viewing wallet recipients.

Functionally everything looks good; these would just smooth the experience.

Also applies to: 29-85, 90-91, 102-108, 111-117, 145-162

app/pages/TransactionStatus.tsx (2)

656-683: Consider adding user feedback on save failure.

When saveRecipient fails or returns false, the spinner stops but the user receives no feedback. Consider adding a toast notification for failures.

Suggested improvement

    try {
      const success = await saveRecipient(newRecipient, accessToken);
      if (success) {
        handleSaveSuccess();
      } else {
        setIsSavingRecipient(false);
+       toast.error("Failed to save recipient. Please try again.");
      }
    } catch (error) {
      console.error("Error saving recipient:", error);
      setIsSavingRecipient(false);
+     toast.error("Failed to save recipient. Please try again.");
    }

---

1063-1073: Complex inline string manipulation could be extracted.

The first-name capitalization logic (lines 1066-1072) is verbose inline. Consider extracting to a small helper or using a simpler approach for readability.

supabase/migrations/create_saved_recipients.sql (2)

177-212: Redundant normalized address values in INSERT.

The INSERT statements explicitly provide normalized_wallet_address and normalized_recipient_wallet_address, but the BEFORE INSERT trigger on line 127-129 will auto-populate these fields. The explicit values are unnecessary (though not harmful since the trigger overwrites them).

---

131-143: Duplicate function could be consolidated.

update_updated_at_column_wallet_recipients() is identical to update_updated_at_column() defined earlier (lines 45-51). You could reuse the existing function for the new trigger.

Proposed consolidation

- -- Function to update updated_at timestamp for wallet recipients
- create or replace function public.update_updated_at_column_wallet_recipients()
- returns trigger language plpgsql as $$
- begin
-   new.updated_at = now();
-   return new;
- end;
- $$;

  -- Trigger to automatically update updated_at
  create trigger saved_wallet_recipients_update_updated_at
  before update on public.saved_wallet_recipients
- for each row execute function public.update_updated_at_column_wallet_recipients();
+ for each row execute function public.update_updated_at_column();

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Learnt from: jeremy0x
Repo: paycrest/noblocks PR: 243
File: app/api/blockfest/cashback/route.ts:118-216
Timestamp: 2025-10-09T21:38:03.227Z
Learning: The Noblocks aggregator API's OrderDetailsData type does not include wallet address or sender information. When ownership validation is required, the on-chain transaction data must be queried using the txHash field via viem's publicClient.getTransaction() to extract the 'from' address.

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.

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.

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.

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.

Learnt from: jeremy0x
Repo: paycrest/noblocks PR: 177
File: app/utils.ts:179-196
Timestamp: 2025-07-23T07:30:23.720Z
Learning: The `normalizeNetworkName` function in app/utils.ts has been updated to be dynamic and scalable, converting any network identifier to sentence case with hyphens replaced by spaces and proper handling of acronyms like "BNB".

Learnt from: jeremy0x
Repo: paycrest/noblocks PR: 177
File: app/utils.ts:179-196
Timestamp: 2025-07-23T07:28:30.491Z
Learning: The `normalizeNetworkName` function should be dynamic and scalable, converting any network identifier to sentence case with hyphens replaced by spaces, rather than using a hardcoded mapping.

Learnt from: jeremy0x
Repo: paycrest/noblocks PR: 251
File: app/components/blockfest/BlockFestCashbackComponent.tsx:191-191
Timestamp: 2025-10-13T14:59:46.723Z
Learning: In app/components/blockfest/BlockFestCashbackComponent.tsx, the social share URLs intentionally mention "2% cashback" even though the component's cashbackPercentage prop defaults to "1%". The 2% represents the total potential cashback (1% instant + 1% bonus from showing the post at the booth), while the prop represents only the automatic portion.

app/pages/TransactionForm.tsx (1)

556-559: Clarify or remove confusing walletAddress clearing logic.

The comment at line 556 says "Clear walletAddress when switching to onramp mode", but the condition if (!walletAddress) only executes when walletAddress is already empty. This makes the setValue("walletAddress", "", ...) call a no-op—it's setting an empty value to empty.

Options:

1. If the intent is to clear stale addresses when entering onramp mode, change the condition to:

   - // Clear walletAddress when switching to onramp mode
   - if (!walletAddress) {
   + // Clear any existing walletAddress to start fresh in onramp mode
   + if (walletAddress) {
       setValue("walletAddress", "", { shouldDirty: true });
     }

2. If clearing is unnecessary (because the user will enter a new address anyway), remove the block entirely:

   - // Clear walletAddress when switching to onramp mode
   - if (!walletAddress) {
   -   setValue("walletAddress", "", { shouldDirty: true });
   - }

The current implementation is harmless but confusing.

app/api/aggregator.ts (1)

744-747: Apply the same defensive validation in the filter block.

The filter at lines 744-747 constructs keys using the same pattern as the existingKeys Set (lines 725-740), but it doesn't guard against missing required fields. If recipient.walletAddress, recipient.institutionCode, or recipient.accountIdentifier are undefined, the filter will produce malformed keys like "wallet-undefined" or "undefined-undefined", which won't match entries in existingKeys. This means potentially invalid recipients won't be filtered out correctly, leading to migration failures later.

🔎 Recommended defensive handling

 const recipientsToMigrate = recipients.filter((recipient) => {
+  if (recipient.type === "wallet") {
+    if (!recipient.walletAddress) {
+      console.warn("Skipping wallet recipient with missing walletAddress during migration", recipient);
+      return false;
+    }
+    const key = `wallet-${recipient.walletAddress}`;
+    return !existingKeys.has(key);
+  } else {
+    if (!recipient.institutionCode || !recipient.accountIdentifier) {
+      console.warn("Skipping bank/mobile_money recipient with missing required fields during migration", recipient);
+      return false;
+    }
+    const key = `${recipient.institutionCode}-${recipient.accountIdentifier}`;
+    return !existingKeys.has(key);
+  }
-  const key = recipient.type === "wallet"
-    ? `wallet-${recipient.walletAddress}`
-    : `${recipient.institutionCode}-${recipient.accountIdentifier}`;
-  return !existingKeys.has(key);
 });

app/components/recipient/RecipientListItem.tsx (1)

29-43: Optimize useEffect dependency array to prevent unnecessary re-renders.

The effect depends on both recipient and walletAddress, but walletAddress is derived from recipient (line 26). Including recipient in the dependency array means the effect will re-run whenever any property of the recipient object changes (e.g., id, name), even if walletAddress hasn't changed. This causes unnecessary ENS resolution calls.

🔎 Proposed optimization

   useEffect(() => {
     if (walletRecipient && walletAddress) {
       setIsResolvingEns(true);
       resolveEnsNameOrShorten(walletAddress)
         .then((name) => {
           setDisplayName(name);
           setIsResolvingEns(false);
         })
         .catch(() => {
           // Fallback to first 5 chars if resolution fails
           setDisplayName(walletAddress.slice(2, 7));
           setIsResolvingEns(false);
         });
     }
-  }, [recipient, walletAddress]);
+  }, [walletRecipient, walletAddress]);

This ensures the effect only re-runs when the wallet type or address changes, not on every recipient object mutation.

app/utils.ts (1)

629-677: Consider fallback RPC endpoints for ENS resolution reliability.

The function uses a hard-coded public RPC endpoint (https://eth.llamarpc.com) for ENS resolution on mainnet (line 659). Public RPCs can be rate-limited or temporarily unavailable, which could cause ENS resolution to fail even when the service is working.

💡 Potential improvements

1. Use environment variable for RPC URL to allow easy configuration and fallback endpoints:

   const rpcUrl = process.env.NEXT_PUBLIC_MAINNET_RPC_URL || "https://eth.llamarpc.com";

2. Add retry logic or fallback RPCs in the catch block before falling back to shortened address:

   const RPC_ENDPOINTS = [
     "https://eth.llamarpc.com",
     "https://rpc.ankr.com/eth",
     "https://ethereum.publicnode.com"
   ];
   // Try each endpoint in sequence

These are optional improvements since the current implementation has proper error handling with fallback to shortened address.

app/pages/TransactionStatus.tsx (1)

717-750: Consider clarifying onramp refund/processing messages.

The onramp messages at lines 721 and 732 say "payment to {recipientDisplayName}", which could be slightly ambiguous. In an onramp flow, the user is paying from their bank account to receive crypto, so "payment to" might sound like they're paying the recipient directly.

💡 Alternative phrasing for clarity

Refunded (lines 721-722):

- Your payment to {recipientDisplayName} was unsuccessful.
+ Your deposit request was unsuccessful.

Processing (lines 732-734):

- Processing payment to{" "}
- <span className="text-text-body dark:text-white">
-   {recipientDisplayName}
- </span>
+ Processing your deposit request

This makes it clearer that the user initiated the deposit, not a payment to someone else. The current wording is acceptable, though, as the success message (lines 743-747) is clear about "deposited into recipient wallet address."

Configuration used: defaults

Review profile: CHILL

Plan: Pro

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.

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.

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.

Learnt from: jeremy0x
Repo: paycrest/noblocks PR: 251
File: app/components/blockfest/BlockFestCashbackComponent.tsx:191-191
Timestamp: 2025-10-13T14:59:46.723Z
Learning: In app/components/blockfest/BlockFestCashbackComponent.tsx, the social share URLs intentionally mention "2% cashback" even though the component's cashbackPercentage prop defaults to "1%". The 2% represents the total potential cashback (1% instant + 1% bonus from showing the post at the booth), while the prop represents only the automatic portion.

Learnt from: jeremy0x
Repo: paycrest/noblocks PR: 177
File: app/utils.ts:179-196
Timestamp: 2025-07-23T07:30:23.720Z
Learning: The `normalizeNetworkName` function in app/utils.ts has been updated to be dynamic and scalable, converting any network identifier to sentence case with hyphens replaced by spaces and proper handling of acronyms like "BNB".

Learnt from: jeremy0x
Repo: paycrest/noblocks PR: 177
File: app/utils.ts:179-196
Timestamp: 2025-07-23T07:28:30.491Z
Learning: The `normalizeNetworkName` function should be dynamic and scalable, converting any network identifier to sentence case with hyphens replaced by spaces, rather than using a hardcoded mapping.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes