Feat/onramp implementation #333
Conversation
- Simplified the stateProps object in MainPageContent for better readability. - Introduced isSwapped state in TransactionForm to manage swap mode between token and currency. - Updated calculations for amount sent and received based on swap mode. - Enhanced dropdowns and button for swapping fields, improving user experience.
- Added support for wallet recipients in the transaction process, allowing users to send tokens directly to wallet addresses. - Introduced new state management for onramp mode, differentiating between wallet and bank/mobile money transactions. - Updated API interactions to handle wallet recipient data, including saving and migrating wallet recipients. - Enhanced UI components to accommodate wallet-specific fields and improve user experience during recipient selection. - Implemented ENS name resolution for wallet addresses to provide better visibility for users. - Updated database schema to include a new table for saved wallet recipients, ensuring proper data handling and security.
- Added a `name` field to wallet recipient details for better identification. - Implemented `getCurrencySymbol` utility to standardize currency symbol retrieval across components. - Updated transaction handling to validate and include recipient names in API interactions. - Enhanced UI components to display recipient names and improve user experience during transactions. - Modified database schema to ensure recipient names are stored and managed correctly.
📝 WalkthroughPre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (10)
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:
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%]"Input text color (line 83):
text-whiteis hardcoded, but the input is on a transparent background. Verify this works correctly in light theme, or add adark:variant if needed.app/components/recipient/RecipientListItem.tsx (1)
16-107: Wallet recipient rendering and ENS resolution look solid; consider minor defensive resetThe 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/isResolvingEnswhenwalletAddressbecomes 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 considerImplementation of
getCurrencySymbol,resolveEnsNameOrShorten,getAvatarImage, andcopyToClipboardlooks functionally sound and aligns with how they’re used in the UI.Two optional improvements:
- Avoid recreating static maps on every call: lift
currencySymbolsout ofgetCurrencySymbolinto a module‑level constant to reduce allocations on hot paths.- Trim wallet inputs before validation/shortening: in
resolveEnsNameOrShorten, considerconst trimmed = address.trim();and validate/operate ontrimmedso 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 helpersThe new onramp path here is well put together:
isSwappedcleanly switches between walletAddress input and the existing bank/mobile‑money fields.- Saved recipients are correctly split into
walletRecipientsvsbankRecipients, and bothselectSavedRecipientanddeleteRecipienthandle 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:
Trim before validating walletAddress
In the wallet register block you validate withisValidEvmAddressCaseInsensitive(value). If users paste addresses with leading/trailing spaces, they’ll get a spurious “Invalid wallet address format”. Doingconst trimmed = value.trim();insidevalidateand checkingtrimmed(while still storing the trimmed value) would make this more forgiving.Avoid hard‑coded network map when you already have display names
formatNetworkNamemaintains a small map of slugs to display strings, butnetworkNamehere is already a human‑readable name (e.g., “Base”, “Arbitrum One”). You could simplify to justnetworkNameor, if you need slug→name behavior elsewhere, reuse the existingnormalizeNetworkNameutility 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 considerThe modal’s new behavior aligns with the onramp flow:
isSwappedcleanly 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
walletAddressfor wallets vsaccountIdentifierfor banks, andindexis passed through toRecipientListItemas required.Two optional UX improvements:
- Search by name for wallets as well as address: in the swapped branch you currently match only on
walletAddress. Includingrecipient.namein 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 withisSwapped && networkNameso 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
saveRecipientfails or returnsfalse, 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_addressandnormalized_recipient_wallet_address, but theBEFORE INSERTtrigger 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 toupdate_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();
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
public/images/onramp-avatar/avatar1.pngis excluded by!**/*.pngpublic/images/onramp-avatar/avatar2.pngis excluded by!**/*.pngpublic/images/onramp-avatar/avatar3.pngis excluded by!**/*.pngpublic/images/onramp-avatar/avatar4.pngis excluded by!**/*.png
📒 Files selected for processing (18)
app/api/aggregator.tsapp/api/v1/recipients/route.tsapp/components/MainPageContent.tsxapp/components/index.tsapp/components/recipient/AddBeneficiaryModal.tsxapp/components/recipient/RecipientDetailsForm.tsxapp/components/recipient/RecipientListItem.tsxapp/components/recipient/SavedBeneficiariesModal.tsxapp/components/recipient/types.tsapp/hooks/useSwapButton.tsapp/mocks.tsapp/pages/MakePayment.tsxapp/pages/TransactionForm.tsxapp/pages/TransactionPreview.tsxapp/pages/TransactionStatus.tsxapp/types.tsapp/utils.tssupabase/migrations/create_saved_recipients.sql
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
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.
📚 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/api/v1/recipients/route.tsapp/components/recipient/types.tsapp/components/recipient/AddBeneficiaryModal.tsxapp/hooks/useSwapButton.tsapp/pages/MakePayment.tsxapp/pages/TransactionForm.tsxapp/api/aggregator.tsapp/components/recipient/RecipientDetailsForm.tsxapp/components/recipient/RecipientListItem.tsxapp/components/recipient/SavedBeneficiariesModal.tsxapp/types.tsapp/pages/TransactionPreview.tsxapp/pages/TransactionStatus.tsx
📚 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/recipient/types.tsapp/components/recipient/AddBeneficiaryModal.tsxapp/hooks/useSwapButton.tsapp/pages/TransactionForm.tsxapp/components/recipient/RecipientDetailsForm.tsxapp/components/recipient/RecipientListItem.tsxapp/components/recipient/SavedBeneficiariesModal.tsxapp/types.tsapp/pages/TransactionPreview.tsxapp/pages/TransactionStatus.tsx
📚 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/recipient/types.tsapp/components/recipient/AddBeneficiaryModal.tsxapp/hooks/useSwapButton.tsapp/pages/TransactionForm.tsxapp/components/recipient/RecipientDetailsForm.tsxapp/components/recipient/RecipientListItem.tsxapp/components/recipient/SavedBeneficiariesModal.tsxapp/types.tsapp/pages/TransactionPreview.tsxapp/pages/TransactionStatus.tsx
📚 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/recipient/RecipientDetailsForm.tsxapp/components/recipient/SavedBeneficiariesModal.tsx
📚 Learning: 2025-07-23T07:30:23.720Z
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".
Applied to files:
app/components/recipient/RecipientDetailsForm.tsxapp/utils.tsapp/pages/TransactionPreview.tsx
📚 Learning: 2025-07-23T07:28:30.491Z
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.
Applied to files:
app/utils.ts
📚 Learning: 2025-10-13T14:59:46.723Z
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.
Applied to files:
app/pages/TransactionStatus.tsx
🧬 Code graph analysis (10)
app/api/v1/recipients/route.ts (3)
app/lib/supabase.ts (1)
supabaseAdmin(11-20)app/types.ts (2)
RecipientDetailsWithId(401-403)SaveRecipientResponse(411-414)app/lib/server-analytics.ts (1)
trackBusinessEvent(362-375)
app/components/recipient/AddBeneficiaryModal.tsx (1)
app/utils.ts (2)
getAvatarImage(1308-1311)shortenAddress(591-600)
app/hooks/useSwapButton.ts (2)
app/context/index.ts (1)
useInjectedWallet(6-6)app/context/InjectedWalletContext.tsx (1)
useInjectedWallet(114-114)
app/pages/MakePayment.tsx (4)
app/components/index.ts (3)
MakePayment(33-33)secondaryBtnClasses(47-47)primaryBtnClasses(46-46)app/types.ts (1)
TransactionPreviewProps(45-49)app/api/aggregator.ts (1)
fetchOrderDetails(233-245)app/utils.ts (4)
getCurrencySymbol(93-111)formatNumberWithCommas(59-63)copyToClipboard(1319-1329)classNames(26-28)
app/components/recipient/RecipientDetailsForm.tsx (7)
app/context/NetworksContext.tsx (1)
useNetwork(96-102)app/hooks/useActualTheme.ts (1)
useActualTheme(4-25)app/api/aggregator.ts (1)
fetchSavedRecipients(618-635)app/lib/validation.ts (1)
isValidEvmAddressCaseInsensitive(19-21)app/utils.ts (2)
classNames(26-28)getNetworkImageUrl(902-907)app/components/InputError.tsx (1)
InputError(5-20)app/components/AnimatedComponents.tsx (1)
AnimatedFeedbackItem(275-288)
app/utils.ts (1)
app/lib/validation.ts (1)
isValidEvmAddressCaseInsensitive(19-21)
app/components/recipient/RecipientListItem.tsx (3)
app/components/index.ts (1)
RecipientListItem(41-41)app/components/recipient/types.ts (1)
RecipientListItemProps(14-20)app/utils.ts (5)
getAvatarImage(1308-1311)resolveEnsNameOrShorten(609-650)classNames(26-28)getRandomColor(756-761)shortenAddress(591-600)
app/components/MainPageContent.tsx (3)
app/types.ts (1)
STEPS(30-35)app/pages/MakePayment.tsx (1)
MakePayment(26-323)app/components/index.ts (1)
MakePayment(33-33)
app/components/recipient/SavedBeneficiariesModal.tsx (2)
app/components/recipient/types.ts (1)
SavedBeneficiariesModalProps(22-35)app/components/AnimatedComponents.tsx (1)
AnimatedModal(331-421)
app/pages/TransactionPreview.tsx (1)
app/utils.ts (6)
getCurrencySymbol(93-111)formatNumberWithCommas(59-63)shortenAddress(591-600)formatCurrency(73-86)getInstitutionNameByCode(45-51)getNetworkImageUrl(902-907)
🔇 Additional comments (17)
app/components/recipient/types.ts (1)
19-19: LGTM! Well-designed type extensions.The optional props added (
index,isSwapped,networkName) maintain backward compatibility while enabling onramp/offramp mode handling. The inline comments clearly document their purpose.Also applies to: 33-34
app/api/aggregator.ts (1)
755-758: Improved error logging for wallet recipients.Using
walletAddressas the identifier for wallet-type recipients provides better debugging information. However, ensurewalletAddressis defined before accessing it to avoid logging "undefined" in error messages.Consider adding a fallback:
const recipientName = recipient.type === "wallet" ? (recipient.walletAddress || "unknown-wallet") : recipient.name;app/components/index.ts (1)
33-33: LGTM!The MakePayment component is properly exported from the barrel file, maintaining consistency with other page component exports.
app/components/MainPageContent.tsx (1)
395-401: LGTM! Properly integrated MakePayment step.The MAKE_PAYMENT case is correctly added to the rendering switch with appropriate props (
handleBackButtonClickandstateProps), consistent with the PREVIEW step implementation.app/types.ts (3)
24-24: LGTM! Well-designed additive changes.Adding
walletAddressas an optional field toFormDataand theMAKE_PAYMENTstep toSTEPSproperly supports the onramp workflow while maintaining backward compatibility with existing offramp flows.Also applies to: 33-33
54-56: LGTM! Props properly support onramp mode.The optional props (
isSwapped,token,networkName) maintain backward compatibility while enabling mode-aware rendering and validation in the recipient form.
276-280: LGTM! Supporting type changes are well-designed.
Network.imageUrltype widening supports theme-aware logos (light/dark variants)- Adding
"onramp"toTransactionHistoryTypeproperly categorizes onramp transactions- Converting
RecipientDetailsWithIdto an intersection type (RecipientDetails & { id: string }) works better with discriminated unions while maintaining the same shapeAlso applies to: 305-305, 401-403
app/hooks/useSwapButton.ts (1)
14-14: LGTM! Excellent onramp mode implementation.The changes properly adapt the swap button logic for onramp mode:
- Balance check is correctly skipped in onramp (users pay fiat, not crypto)
- Recipient validation switches between
walletAddress(onramp) andrecipientName(offramp)- The
hasRecipientabstraction improves code clarity and maintainability- Backward compatible with
isSwappeddefaulting tofalseAlso applies to: 25-25, 29-29, 42-46, 73-73, 84-84
app/components/recipient/AddBeneficiaryModal.tsx (1)
11-45: LGTM! Well-structured component logic.The props interface, state management, and event handlers are properly designed:
- Input validation with trim
- Clean state reset on close
- Controlled component pattern
- Good separation of concerns
app/pages/TransactionPreview.tsx (1)
93-176: Onramp/offramp branching in preview is coherent; relying onwalletAddressas the discriminator is acceptable but be mindful of future reuseThe introduction of
isOnramp = !!walletAddressdrives:
- Correct TSX rendering (fiat “You send” vs token, “Receive amount” labelling, network row, wallet recipient shortened with
shortenAddress).- Proper behavior in
handlePaymentConfirmation(onramp goes tomake_payment; offramp keeps the existing createOrder flow and balance checks).- Appropriate transaction metadata in
saveTransactionData(transactionType,fromCurrency,toCurrency, and the wallet‑based recipient object).This all lines up well with the new form and recipient types. The only thing to keep in mind is that
walletAddressis now a semantic flag for “this is onramp” throughout the flow; if you ever introduce other uses ofwalletAddressin the form (e.g., for a pure crypto→crypto swap), you’ll want to revisitisOnramp = !!walletAddressto avoid misclassifying those transactions.For the current onramp implementation, this looks good.
Also applies to: 181-199, 393-405, 420-459, 579-635, 642-645
app/pages/TransactionForm.tsx (1)
84-141: Swap (onramp) vs standard (offramp) math and UI behavior look correctThe new
isSwappedflow is wired cleanly:
- Rate semantics are clearly documented and applied correctly:
- Offramp: Send = token, Receive = fiat →
receive = send * rate,send = receive / rate.- Onramp: Send = fiat, Receive = token →
receive = send / rate,send = receive * rate.handleSentAmountChange/handleReceivedAmountChangedriveisReceiveInputActive, and thecalculateReceiveAmounteffect respects that flag to avoid fighting the user’s active field.- Balance and “Insufficient balance” messaging are correctly scoped to
!isSwapped, so onramp doesn’t incorrectly block on wallet token balance.- The token/currency dropdowns, memo visibility, and rate text all switch cleanly between modes.
Given how central this is to the onramp behavior, this looks solid.
Also applies to: 183-210, 318-365, 495-575, 712-839, 840-855, 897-929, 1038-1055
app/pages/TransactionStatus.tsx (4)
159-183: LGTM on the computed values for onramp/offramp distinction.The logic correctly uses
walletAddresspresence to determine the transaction type, and the derived display values (recipientDisplayName,primaryAmountDisplay,fundStatusLabel,shareMessage) are well-structured with appropriate fallbacks.
549-615: Helper functions are well-structured.The
buildRecipientandfindRecipientInSavedfunctions cleanly separate wallet vs bank/mobile_money logic. The null/undefined returns for invalid states are appropriate for defensive coding.
633-644: MissingawaitforremoveRecipientcall.The
removeRecipientfunction is async but is called withoutawaiton line 642. While the checkbox state is set synchronously, any errors from the removal won't be caught, and the UI won't reflect failure states.Proposed fix
const handleAddToBeneficiariesChange = async (checked: boolean) => { setAddToBeneficiaries(checked); if (checked) { if (isOnramp) { setIsOnrampModalOpen(true); return; } await addBeneficiary(recipientName); } else { - await removeRecipient(); + try { + await removeRecipient(); + } catch (error) { + // Revert checkbox state on failure + setAddToBeneficiaries(true); + } } };⛔ Skipped due to learnings
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.
1206-1212: ThewalletAddressfallback is not an issue. TheisOnrampModalOpenstate is only set to true whenisOnrampis true, which requireswalletAddressto be truthy. Therefore, the|| recipientDisplayNamefallback will never execute—AddBeneficiaryModalalways receives a valid wallet address when the modal is open.Likely an incorrect or invalid review comment.
supabase/migrations/create_saved_recipients.sql (2)
93-102: Schema design looks correct.The
saved_wallet_recipientstable appropriately separates wallet-type recipients from bank/mobile_money recipients. The normalized address fields with regex constraints ensure data integrity for Ethereum addresses.
145-174: RLS policies correctly restrict access to service role.The policies ensure only the backend API (using service role) can access this table, which is appropriate for this architecture. Direct client access is properly blocked.
| existingRecipients.map((r) => { | ||
| if (r.type === "wallet") { | ||
| return `wallet-${r.walletAddress}`; | ||
| } else { | ||
| return `${r.institutionCode}-${r.accountIdentifier}`; | ||
| } | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Verify recipient data shape before key generation.
The key generation logic assumes that walletAddress exists when type === "wallet" and that institutionCode/accountIdentifier exist for bank/mobile_money types. If the runtime data doesn't match the type guards, this could produce malformed keys like "wallet-undefined".
Consider adding defensive checks or validation before key generation to handle potentially malformed data from localStorage.
🔎 Suggested defensive handling
const existingKeys = new Set(
existingRecipients.map((r) => {
if (r.type === "wallet") {
+ if (!r.walletAddress) {
+ console.warn("Wallet recipient missing walletAddress", r);
+ return `wallet-invalid-${r.id}`;
+ }
return `wallet-${r.walletAddress}`;
} else {
+ if (!r.institutionCode || !r.accountIdentifier) {
+ console.warn("Bank/mobile_money recipient missing required fields", r);
+ return `${r.type}-invalid-${r.id}`;
+ }
return `${r.institutionCode}-${r.accountIdentifier}`;
}
}),
);🤖 Prompt for AI Agents
In app/api/aggregator.ts around lines 725 to 732, the code generates recipient
keys assuming walletAddress exists for type === "wallet" and
institutionCode/accountIdentifier exist for other types; add defensive
validation before key generation to avoid producing strings like
"wallet-undefined". Fix by checking the required fields per type (e.g., if
r.type === "wallet" ensure r.walletAddress is a non-empty string; otherwise
ensure r.institutionCode and r.accountIdentifier are present), filter out or
skip invalid recipients (or provide a safe default and log a warning), and only
then map to the key format; keep behavior consistent (returning an array of only
valid keys) and include a brief debug/warn log when malformed entries are
dropped.
| // Mock payment account details - will be replaced with actual API call | ||
| const [paymentDetails, setPaymentDetails] = useState<PaymentAccountDetails>({ | ||
| provider: "(4Bay7 Enterprise)", | ||
| accountNumber: "952157815", | ||
| amount: amountSent || 29000, | ||
| currency: currency || "KES", | ||
| expiresAt: new Date(Date.now() + 30 * 60 * 1000), // 30 minutes from now | ||
| }); |
There was a problem hiding this comment.
Replace hard‑coded payment details with real provider data before shipping onramp
The MakePayment flow and polling logic look reasonable, but paymentDetails is currently initialized with hard‑coded provider/account values and a generic 30‑minute expiry:
const [paymentDetails, setPaymentDetails] = useState<PaymentAccountDetails>({
provider: "(4Bay7 Enterprise)",
accountNumber: "952157815",
amount: amountSent || 29000,
currency: currency || "KES",
expiresAt: new Date(Date.now() + 30 * 60 * 1000),
});If this goes to production as‑is, every onramp user will see the same static bank details, which almost certainly won’t match what the aggregator/PSP expects for a given order. That will prevent correct reconciliation and can send funds to the wrong account.
Before enabling this step in the real flow, you should:
- Populate
provider,accountNumber,amount,currency, andexpiresAtfrom the backend (e.g., order creation or a dedicated “payment instructions” endpoint), and - Treat absence of those fields as a blocking error for entering this screen.
The rest of the polling and cleanup around fetchOrderDetails looks fine once it’s wired to real order data.
Also applies to: 79-160, 190-287
🤖 Prompt for AI Agents
In app/pages/MakePayment.tsx around lines 38-45 (and also review ranges 79-160,
190-287), the paymentDetails state is initialized with hard-coded
provider/account/amount/currency/expiresAt values; replace this with values
fetched from the backend (order creation or a payment-instructions endpoint) and
setPaymentDetails with the response, validate that provider, accountNumber,
amount, currency and expiresAt are present and valid before allowing the screen
to render (show an error state or redirect if missing), and derive expiresAt
from the backend timestamp rather than computing a local 30-minute window;
ensure fetch errors set a blocking error and stop the flow until real payment
instructions are returned.
| useEffect(() => { | ||
| // Only run once to restore state | ||
| if (hasRestoredStateRef.current) { | ||
| return; | ||
| } | ||
|
|
||
| const shouldBeSwapped = Boolean(walletAddress); | ||
| if (shouldBeSwapped !== isSwapped) { | ||
| setIsSwapped(shouldBeSwapped); | ||
| hasRestoredStateRef.current = true; | ||
| } | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, []); | ||
|
|
||
| // Handle swap button click to switch between token/currency dropdowns | ||
| const handleSwapFields = async () => { | ||
| const currentAmountSent = amountSent; | ||
| const currentAmountReceived = amountReceived; | ||
| const willBeSwapped = !isSwapped; | ||
|
|
||
| // Toggle swap mode FIRST | ||
| setIsSwapped(willBeSwapped); | ||
|
|
||
| // Swap amounts | ||
| setValue("amountSent", currentAmountReceived || 0, { shouldDirty: true }); | ||
| setValue("amountReceived", currentAmountSent || 0, { shouldDirty: true }); | ||
|
|
||
| // Swap formatted amounts | ||
| setFormattedSentAmount(formattedReceivedAmount); | ||
| setFormattedReceivedAmount(formattedSentAmount); | ||
|
|
||
| // Set defaults only if not already selected | ||
| if (willBeSwapped) { | ||
| // Switching to onramp mode: Set currency based on user location | ||
| if (!currency) { | ||
| try { | ||
| const countryCode = await fetchUserCountryCode(); | ||
| const locationCurrency = countryCode | ||
| ? mapCountryToCurrency(countryCode) | ||
| : null; | ||
|
|
||
| // Use location-based currency if available and supported, otherwise default to NGN | ||
| const defaultCurrency = locationCurrency && | ||
| currencies.find(c => c.name === locationCurrency && !c.disabled) | ||
| ? locationCurrency | ||
| : "NGN"; | ||
|
|
||
| setValue("currency", defaultCurrency, { shouldDirty: true }); | ||
| } catch { | ||
| // Fallback to NGN if location detection fails | ||
| setValue("currency", "NGN", { shouldDirty: true }); | ||
| } | ||
| } | ||
| // Only set default token if not already selected | ||
| if (!token && fetchedTokens.length > 0) { | ||
| const usdcToken = fetchedTokens.find((t) => t.symbol === "USDC"); | ||
| const defaultToken = usdcToken?.symbol || fetchedTokens[0]?.symbol; | ||
| if (defaultToken) { | ||
| setValue("token", defaultToken, { shouldDirty: true }); | ||
| } | ||
| } | ||
| // Clear any existing walletAddress to start fresh | ||
| if (!walletAddress) { | ||
| setValue("walletAddress", "", { shouldDirty: true }); | ||
| } | ||
| } else { | ||
| // Switching back to offramp mode | ||
| if (!token && fetchedTokens.length > 0) { | ||
| const usdcToken = fetchedTokens.find((t) => t.symbol === "USDC"); | ||
| const defaultToken = usdcToken?.symbol || fetchedTokens[0]?.symbol; | ||
| if (defaultToken) { | ||
| setValue("token", defaultToken, { shouldDirty: true }); | ||
| } | ||
| } | ||
| // Clear walletAddress when switching to offramp mode | ||
| setValue("walletAddress", "", { shouldDirty: true }); | ||
| } | ||
|
|
||
| // Reset rate to trigger recalculation | ||
| stateProps.setRate(0); | ||
| }; |
There was a problem hiding this comment.
Clarify walletAddress reset behavior when toggling swap mode
In handleSwapFields, the comment says “Clear any existing walletAddress to start fresh”, but the condition is:
// Clear any existing walletAddress to start fresh
if (!walletAddress) {
setValue("walletAddress", "", { shouldDirty: true });
}This only executes when walletAddress is already falsy, so it never actually clears a non‑empty address. It’s effectively a no‑op right now.
Depending on intent, either:
- Drop the block entirely (the value is already empty when entering onramp from offramp), or
- Flip the condition to
if (walletAddress)so toggling into onramp mode always clears any stale address carried over from previous runs.
Functionally this isn’t breaking anything today, but aligning the code with the comment will avoid confusion later.
🤖 Prompt for AI Agents
app/pages/TransactionForm.tsx lines 495-575: The block meant to "Clear any
existing walletAddress to start fresh" only runs when walletAddress is falsy
(no-op); change it to run when walletAddress is truthy so toggling into onramp
clears a previously set address — replace the condition with one that checks if
(walletAddress) and then call setValue("walletAddress", "", { shouldDirty: true
}); (alternatively, remove the block entirely if clearing is unnecessary).
| const shareMessage = isOnramp | ||
| ? `Yay! I just sent crypto to a bank account in ${shareDuration} on noblocks.xyz` | ||
| : `Yay! I just swapped ${token} for ${currency} in ${shareDuration} on noblocks.xyz`; |
There was a problem hiding this comment.
Share message appears swapped for onramp.
The shareMessage for isOnramp says "sent crypto to a bank account" but onramp is fiat→crypto (receiving crypto into a wallet). The messages appear to be inverted.
Proposed fix
const shareMessage = isOnramp
- ? `Yay! I just sent crypto to a bank account in ${shareDuration} on noblocks.xyz`
- : `Yay! I just swapped ${token} for ${currency} in ${shareDuration} on noblocks.xyz`;
+ ? `Yay! I just swapped ${currency} for ${token} in ${shareDuration} on noblocks.xyz`
+ : `Yay! I just swapped ${token} for ${currency} in ${shareDuration} on noblocks.xyz`;📝 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 shareMessage = isOnramp | |
| ? `Yay! I just sent crypto to a bank account in ${shareDuration} on noblocks.xyz` | |
| : `Yay! I just swapped ${token} for ${currency} in ${shareDuration} on noblocks.xyz`; | |
| const shareMessage = isOnramp | |
| ? `Yay! I just swapped ${currency} for ${token} in ${shareDuration} on noblocks.xyz` | |
| : `Yay! I just swapped ${token} for ${currency} in ${shareDuration} on noblocks.xyz`; |
🤖 Prompt for AI Agents
In app/pages/TransactionStatus.tsx around lines 181 to 183, the shareMessage
branch for isOnramp is inverted (it says "sent crypto to a bank account" which
describes an off-ramp); swap the two messages so isOnramp produces a message
like "Yay! I just bought/received crypto using a bank account in
${shareDuration} on noblocks.xyz" and the else branch (offramp) produces "Yay! I
just sent crypto to a bank account in ${shareDuration} on noblocks.xyz" while
preserving token/currency interpolation where relevant.
| export type RecipientDetails = | ||
| | { | ||
| type: "wallet"; | ||
| walletAddress: string; | ||
| name: string; | ||
| } | ||
| | { | ||
| type: "bank" | "mobile_money"; | ||
| name: string; | ||
| institution: string; | ||
| institutionCode: string; | ||
| accountIdentifier: string; | ||
| walletAddress?: never; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for all RecipientDetails usages and their context
rg -n 'RecipientDetails' --type=ts --type=tsx -g '!node_modules' -B2 -A2Repository: paycrest/noblocks
Length of output: 88
🏁 Script executed:
# Search for direct property accesses that might not be guarded (wallet/institution properties)
rg -n '\.(walletAddress|institution|accountIdentifier|institutionCode)' --type=ts --type=tsx -B3 -A1 | head -100Repository: paycrest/noblocks
Length of output: 88
🏁 Script executed:
# Search for type guards on RecipientDetails to see how variants are being handled
rg -n 'type\s*===|\.type\s*===' --type=ts --type=tsx -B2 -A2 | head -100Repository: paycrest/noblocks
Length of output: 88
🏁 Script executed:
# Find all TypeScript/TSX files and search for RecipientDetails usage
fd -e ts -e tsx | xargs rg -n 'RecipientDetails' -B2 -A2 2>/dev/null | head -150Repository: paycrest/noblocks
Length of output: 8604
🏁 Script executed:
# Search for property accesses that might be unguarded
fd -e ts -e tsx | xargs rg -n '\.(walletAddress|institution|accountIdentifier|institutionCode)' -B2 -A1 2>/dev/null | head -150Repository: paycrest/noblocks
Length of output: 12009
🏁 Script executed:
# Search for type guards on recipient.type
fd -e ts -e tsx | xargs rg -n 'recipient\.type|\.type\s*===' -B2 -A2 2>/dev/null | head -100Repository: paycrest/noblocks
Length of output: 6983
Discriminated union properly designed, but critical type safety violations found during usage.
Converting RecipientDetails to a discriminated union with walletAddress?: never is the correct approach for type safety. However, not all consuming code properly handles both variants:
Critical issues found:
-
RecipientListItem.tsx: AccesseswalletAddress,accountIdentifier, andinstitutionwithout type guards. For example, line 23 readsconst walletAddress = recipient.walletAddress || ""and lines 76-78 renderaccountIdentifierandinstitutionwithout checkingrecipient.typefirst. -
SavedBeneficiariesModal.tsx: Lines 79 and 82 accessaccountIdentifierandinstitutionCodewithout type guards in the filter logic.
These components must guard property access with if (recipient.type === "wallet") or if (recipient.type !== "wallet") before accessing type-specific fields, matching the pattern already correctly implemented in RecipientDetailsForm.tsx and TransactionStatus.tsx.
🤖 Prompt for AI Agents
In app/types.ts around lines 59 to 72 the RecipientDetails discriminated union
is correct, but consuming code must be updated: in RecipientListItem.tsx (around
line 23 and lines ~76-78) guard accesses to walletAddress, accountIdentifier,
and institution with type checks (e.g., if (recipient.type === "wallet") { use
walletAddress } else { use accountIdentifier/institution }) or conditional
rendering so the compiler narrows the union; similarly update
SavedBeneficiariesModal.tsx (around lines 79 and 82) to only access
accountIdentifier and institutionCode after confirming recipient.type !==
"wallet" (or the specific non-wallet types) in filter logic; ensure no
cross-variant properties are accessed without these type guards so TypeScript
type narrowing prevents runtime errors.
- Introduced type predicates for recipient details to improve type safety in filtering bank/mobile money and wallet recipients. - Added validation for wallet address format in the API, ensuring correct input before processing. - Updated UI components to utilize new type predicates for better handling of recipient data. - Enhanced error handling for missing required fields in recipient data during local storage migration. - Improved styling in the AddBeneficiaryModal for better user experience.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
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 whenwalletAddressis already empty. This makes thesetValue("walletAddress", "", ...)call a no-op—it's setting an empty value to empty.Options:
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 }); }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
existingKeysSet (lines 725-740), but it doesn't guard against missing required fields. Ifrecipient.walletAddress,recipient.institutionCode, orrecipient.accountIdentifierareundefined, the filter will produce malformed keys like"wallet-undefined"or"undefined-undefined", which won't match entries inexistingKeys. 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); });
🧹 Nitpick comments (3)
app/components/recipient/RecipientListItem.tsx (1)
29-43: Optimize useEffect dependency array to prevent unnecessary re-renders.The effect depends on both
recipientandwalletAddress, butwalletAddressis derived fromrecipient(line 26). Includingrecipientin the dependency array means the effect will re-run whenever any property of the recipient object changes (e.g.,id,name), even ifwalletAddresshasn'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
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";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 sequenceThese 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 requestThis 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."
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/api/aggregator.tsapp/api/v1/recipients/route.tsapp/components/recipient/AddBeneficiaryModal.tsxapp/components/recipient/RecipientListItem.tsxapp/components/recipient/SavedBeneficiariesModal.tsxapp/pages/TransactionForm.tsxapp/pages/TransactionStatus.tsxapp/utils.tssupabase/migrations/create_saved_recipients.sql
🚧 Files skipped from review as they are similar to previous changes (2)
- app/components/recipient/AddBeneficiaryModal.tsx
- app/components/recipient/SavedBeneficiariesModal.tsx
🧰 Additional context used
🧠 Learnings (6)
📚 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/api/v1/recipients/route.tsapp/api/aggregator.tsapp/components/recipient/RecipientListItem.tsxapp/pages/TransactionStatus.tsxapp/pages/TransactionForm.tsx
📚 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/api/v1/recipients/route.tsapp/api/aggregator.tsapp/components/recipient/RecipientListItem.tsxapp/pages/TransactionStatus.tsxapp/pages/TransactionForm.tsx
📚 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/api/v1/recipients/route.tsapp/api/aggregator.tsapp/components/recipient/RecipientListItem.tsxapp/pages/TransactionStatus.tsxapp/utils.tsapp/pages/TransactionForm.tsx
📚 Learning: 2025-10-13T14:59:46.723Z
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.
Applied to files:
app/pages/TransactionStatus.tsx
📚 Learning: 2025-07-23T07:30:23.720Z
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".
Applied to files:
app/utils.ts
📚 Learning: 2025-07-23T07:28:30.491Z
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.
Applied to files:
app/utils.ts
🧬 Code graph analysis (3)
app/api/v1/recipients/route.ts (4)
app/lib/supabase.ts (1)
supabaseAdmin(11-20)app/types.ts (2)
RecipientDetailsWithId(401-403)SaveRecipientResponse(411-414)app/lib/validation.ts (1)
isValidEvmAddressCaseInsensitive(19-21)app/lib/server-analytics.ts (1)
trackBusinessEvent(362-375)
app/components/recipient/RecipientListItem.tsx (3)
app/components/index.ts (1)
RecipientListItem(41-41)app/components/recipient/types.ts (1)
RecipientListItemProps(14-20)app/utils.ts (6)
isWalletRecipient(41-45)getAvatarImage(1335-1338)resolveEnsNameOrShorten(636-677)classNames(53-55)getRandomColor(783-788)shortenAddress(618-627)
app/pages/TransactionForm.tsx (4)
app/utils.ts (3)
fetchUserCountryCode(1042-1051)mapCountryToCurrency(1060-1075)formatNumberWithCommas(86-90)app/components/index.ts (3)
FormDropdown(37-37)AnimatedComponent(15-15)slideInOut(19-19)app/components/FormDropdown.tsx (1)
FormDropdown(18-96)app/components/AnimatedComponents.tsx (2)
AnimatedComponent(172-188)slideInOut(28-32)
🔇 Additional comments (6)
app/api/v1/recipients/route.ts (1)
484-522: LGTM! DELETE route correctly handles multi-table lookup.The implementation properly uses
maybeSingle()to query the wallet recipients table and correctly distinguishes between "no rows found" (error code PGRST116, which is expected and safe to ignore) and genuine query errors (which should be thrown). The conditional deletion logic based on which table contains the recipient is sound.app/pages/TransactionStatus.tsx (2)
181-183: Share messages are correctly aligned with transaction direction.The share messages are now correct:
- Onramp (line 182): "swapped {currency} for {token}" → fiat to crypto ✓
- Offramp (line 183): "swapped {token} for {currency}" → crypto to fiat ✓
The past review comment flagging these as inverted appears to have been addressed or was incorrect.
550-615: LGTM! Helper functions properly abstract recipient management.The
buildRecipientandfindRecipientInSavedhelper functions cleanly separate the logic for onramp (wallet-based) vs offramp (bank/mobile_money) recipients. Both functions return early withnull/undefinedwhen required data is missing, preventing invalid operations downstream.supabase/migrations/create_saved_recipients.sql (3)
117-143: LGTM! Normalization and timestamp triggers are well-implemented.The normalization function correctly converts both wallet addresses to lowercase before storage, ensuring case-insensitive matching for the unique constraint. The separate function names (
_wallet_recipientssuffix) properly avoid collisions with the existingsaved_recipientstable functions. Trigger timing (BEFORE insert/update) is correct.
145-174: LGTM! RLS policies are consistent with the existing architecture.The service role-only policies correctly mirror the pattern used by the existing
saved_recipientstable. The idempotentdrop policy if existsstatements ensure safe re-execution of the migration. The architecture delegates user-level authorization to the API layer, which is appropriate given the comment indicating service role key usage.
93-114: Address format constraint is appropriate for system scope.The CHECK constraint
^0x[0-9a-f]{40}$is correct. The system explicitly supports only EVM-compatible networks (Polygon, Base, Arbitrum, BNB Chain, Ethereum, Celo, Lisk, Optimism), and all EVM chains use the same address format: 0x prefix followed by 40 hexadecimal characters. The supported stablecoins (USDC, USDT, DAI, cNGN) are all deployed on these EVM chains only.Likely an incorrect or invalid review comment.
Description
This PR implements the Onramp transaction flow for Noblocks, enabling users to swap local fiat currencies (NGN, KES, TZS, UGX) for stablecoins (USDC, USDT, etc.) to fund their wallets within the Paycrest ecosystem.
User Story:
As a Noblocks user, I want to easily and securely swap my local fiat currency (NGN, KES, TZS, UGX) for stablecoins (USDC, USDT, etc.) on the app, so that I can conveniently acquire crypto assets and fund my wallet within the Paycrest ecosystem.
Key Features Implemented:
References
Checklist
mainFiles Changed:
noblocks/app/pages/TransactionStatus.tsx- Onramp support, beneficiary modal, status messagesnoblocks/app/pages/TransactionPreview.tsx- Currency symbols, onramp detectionnoblocks/app/pages/MakePayment.tsx- Payment verification, currency symbols, pending statenoblocks/app/components/recipient/addBeneficiaryModal.tsx- New component for wallet beneficiary name inputnoblocks/app/components/recipient/RecipientListItem.tsx- Display name for wallet recipientsnoblocks/app/api/v1/recipients/route.ts- Name validation for wallet recipientsnoblocks/app/utils.ts- AddedgetCurrencySymbolfunctionnoblocks/app/types.ts- UpdatedRecipientDetailstypenoblocks/supabase/migrations/create_saved_recipients.sql- Added name field to saved_wallet_recipientsBy submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.