feat: add Smile ID KYC submission functionality and integrate smart camera component in KycModal #286
Conversation
…amera component in KycModal
WalkthroughAdds tiered KYC and phone verification: new Supabase KYC table and migration, server APIs for OTP and SmileID KYC, provider routing (Termii/Twilio), client hooks and modals (phone, limits, profile, KYC capture), country utilities, and related dependencies. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Client as Noblocks App (client)
participant API as Next.js API
participant DB as Supabase
participant SMS as Termii/Twilio
Note over Client,API: Phone verification flow
User->>Client: Enter name & phone, request OTP
Client->>API: POST /api/phone/send-otp {phone, wallet}
API->>DB: Upsert user_kyc_profiles (otp_code, expires_at, provider)
API->>SMS: Send OTP via chosen provider
SMS-->>API: Provider response
API-->>Client: {success, provider, intlPhone}
User->>Client: Submit OTP code
Client->>API: POST /api/phone/verify-otp {phone, otp, wallet}
API->>DB: Fetch profile by wallet+phone, validate otp/expiry/attempts
alt OTP valid
DB-->>API: Update verified=true, verified_at, tier=1 (if applicable)
API-->>Client: 200 {verified:true, phoneNumber}
else OTP invalid/expired/locked
API-->>Client: 4xx error (attempts/expired)
end
sequenceDiagram
actor User
participant Client as Noblocks App (client)
participant KycAPI as /api/kyc/smile-id
participant Smile as SmileID Server
participant DB as Supabase
Note over Client,KycAPI: In-app SmileID capture and submission
User->>Client: Start Full KYC -> capture images + id_info via smart-camera-web
Client->>KycAPI: POST /api/kyc/smile-id {images, id_info, wallet} (Bearer token)
KycAPI->>Smile: submitSmileIDJob(...) (server-side)
Smile-->>KycAPI: {job_id, status, details}
alt submission success
KycAPI->>DB: Update user_kyc_profiles (platform array, id info, job_id, verified/tier if applicable)
KycAPI-->>Client: {status: "success", jobId, smileIdResponse}
else error
KycAPI-->>Client: 4xx/5xx error with diagnostics
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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
🧪 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 |
- Implemented TransactionLimitModal to manage user transaction limits based on KYC status. - Integrated phone verification and KYC status hooks to enhance user verification process. - Updated SettingsDropdown to include a Profile option and handle profile drawer state. - Created useKYCStatus and usePhoneVerification hooks for managing KYC and phone verification states. - Added country fetching utility and phone number validation logic. - Introduced SmileID integration for selfie verification. - Enhanced Supabase schema to support user KYC profiles and verification processes. - Updated TransactionForm to check transaction limits before processing swaps. - Added necessary dependencies for phone verification and KYC functionalities.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/hooks/useSwapButton.ts (1)
81-118: Fix inverted Fund Wallet condition and align KYC behaviour with button text/stateThere are a couple of behavioural bugs in the updated logic:
- Fund wallet condition is inverted
if (!hasInsufficientBalance && !isInjectedWallet && authenticated) { return handleFundWallet; }This triggers
handleFundWalletwhen the user has sufficient balance (and the button text in that case is usually"Swap"), which is clearly not intended. It should gate funding on insufficient balance.
- KYC vs text mismatch for insufficient & unverified
if (hasInsufficientBalance && !isUserVerified && (authenticated || isInjectedWallet)) { return setIsLimitModalOpen; }For an authenticated, non‑injected user with insufficient balance and
!isUserVerified,buttonTextis"Fund wallet"(because of the earlier text branch), but the action now opens the limit/KYC modal. That’s confusing UX and contradicts the label.
- Injected wallet + insufficient balance never reaches KYC
isEnabledstill short‑circuits with:if (isInjectedWallet && hasInsufficientBalance) { return false; }So for injected wallets with insufficient balance, the button is disabled and the new KYC branch in
buttonActionis never called, even though the condition includesisInjectedWallet.A minimal fix for the worst regression (1) and to make the order of actions clearer could be:
const buttonAction = ( handleSwap: () => void, login: () => void, handleFundWallet: () => void, setIsLimitModalOpen: () => void, isUserVerified: boolean, ) => { if (!authenticated && !isInjectedWallet) { return login; } - if (!hasInsufficientBalance && !isInjectedWallet && authenticated) { - return handleFundWallet; - } - if (hasInsufficientBalance && !isUserVerified && (authenticated || isInjectedWallet)) { - return setIsLimitModalOpen; - } - return handleSwap; + if (hasInsufficientBalance) { + if (!isUserVerified && (authenticated || isInjectedWallet)) { + return setIsLimitModalOpen; + } + if (!isInjectedWallet && authenticated) { + return handleFundWallet; + } + } + return handleSwap; };Follow‑ups (recommended, not strictly required for correctness of this fix):
- Update
buttonTextso that the label matches the action (e.g. show"Get started"or similar whensetIsLimitModalOpenwill fire, and only show"Fund wallet"when the actual action ishandleFundWallet).- Revisit
isEnabledfor injected wallets with insufficient balance if you really want KYC to be reachable in that scenario.
🧹 Nitpick comments (17)
app/api/kyc/transaction-summary/route.ts (1)
22-60: Clarify period semantics and consider explicit UTC handlingThe aggregation logic is correct, but two small points:
- The comment says “last 30 days” while the query uses
monthStart(start of current month). Either update the comment or change the filter to a rolling 30‑day window to avoid confusion.todayandmonthStartare based on the server’s local timezone whilecreated_atis likely stored as UTC. That’s usually fine, but if you care about strict UTC month/day boundaries, consider constructing these dates in UTC or documenting the intended behaviour.app/api/kyc/status/route.ts (1)
22-45: Handle Supabase errors explicitly and tighten tier fallback logicTwo things to consider here:
Supabase error handling
supabaseAdmin.from(...).single()can return anerror(e.g., network issue, Postgres error). Right now you ignore it, which means a DB failure looks like “no profile found” and you still return200with tier0. That makes observability harder and can block verified users due to transient backend issues.Suggestion: capture
{ data: kycProfile, error }and, iferroris truthy and not a simple “no rows” case, log/track it and return a 500, similar to the transaction-summary route.Tier fallback check
The comment says “use database tier if available, otherwise calculate”, but:let tier: 0 | 1 | 2 = kycProfile?.tier || 0; if (!kycProfile?.tier) { if (fullKYCVerified) tier = 2; else if (phoneVerified) tier = 1; }Because
0is falsy, a storedtier = 0is treated as “not set”, and the fallback can overwrite it based on verification flags. If you intend to respect an explicit0, change the condition to a null/undefined check, e.g.if (kycProfile?.tier == null) { ... }.app/lib/countries.ts (1)
15-104: Country lookup and caching logic looks solidThe fetch → transform → cache flow, plus the hardcoded fallback list, gives you a nice balance of UX and resilience if the REST Countries API is down.
searchCountriesandgetPopularCountriesare straightforward and usable by UI components.If this becomes performance‑critical later, you might consider adding a simple TTL to
countriesCache(so long‑lived server processes can refresh occasionally), but what you have is perfectly acceptable for now.app/components/TransactionLimitModal.tsx (2)
210-215: WiresetIsUserVerifiedto refresh KYC state and close parent on successHere
setIsUserVerified={() => { }}is a no-op, so a successful Smile ID KYC flow inKycModalwon’t update this component’s KYC state or close the TransactionLimitModal. The user has to manually close and reopen to see updated limits.You can use the success callback to refresh status and close the parent when verification succeeds:
- <KycModal - setIsKycModalOpen={setIsKycModalOpen} - setIsUserVerified={() => { }} - /> + <KycModal + setIsKycModalOpen={setIsKycModalOpen} + setIsUserVerified={async (verified) => { + if (verified) { + await refreshStatus(); + onClose(); + } + }} + />This keeps the limit UI in sync with backend KYC status and matches the expectation that the enclosing flow completes when verification passes.
54-59: Simplify tier check in auto-open effect
tieris typed as0 | 1 | 2and initialized to0, sotier === undefinedcan never be true. The condition can be simplified for clarity:- useEffect(() => { - if (isOpen && !isLoading && (tier < 1 || tier === undefined)) { - setIsPhoneModalOpen(true); - } - }, [isOpen, isLoading, tier]); + useEffect(() => { + if (isOpen && !isLoading && tier < 1) { + setIsPhoneModalOpen(true); + } + }, [isOpen, isLoading, tier]);app/components/ProfileDrawer.tsx (1)
42-47: Future‑proofmonthlyProgressagainst zero monthly limitsRight now
monthlyProgressis computed as:const monthlyProgress = (transactionSummary.monthlySpent / currentLimits.monthly) * 100;and the “Current Tier Status” card only renders when
tier >= 1, wherecurrentLimits.monthlyis non‑zero, so this is safe today. If KYC tiers orgetCurrentLimitsare ever extended to allow a non‑zero tier with a0monthly limit, this could become a division‑by‑zero and produceNaN/Infinity.Optional: add a small guard for robustness, e.g.:
const monthlyProgress = currentLimits.monthly > 0 ? (transactionSummary.monthlySpent / currentLimits.monthly) * 100 : 0;Overall, the drawer’s integration with
useKYCStatusandTransactionLimitModallooks good.app/lib/supabase.ts (1)
3-22: Align env validation forNEXT_PUBLIC_SUPABASE_URLwith other Supabase envs
supabaseAdminthrows early ifSUPABASE_URL/SUPABASE_SERVICE_ROLE_KEYare missing, butsupabaseUrlcan beundefinedifNEXT_PUBLIC_SUPABASE_URLis misconfigured, pushing the failure to wherever it’s used.To keep failures explicit and consistent, consider adding a simple guard:
export const supabaseAdmin = createClient( process.env.SUPABASE_URL, process.env.SUPABASE_SERVICE_ROLE_KEY, { auth: { autoRefreshToken: false, persistSession: false, }, } ); -export const supabaseUrl = process.env.NEXT_PUBLIC_SUPABASE_URL; +export const supabaseUrl = process.env.NEXT_PUBLIC_SUPABASE_URL; +if (!supabaseUrl) { + throw new Error("Missing env.NEXT_PUBLIC_SUPABASE_URL"); +}app/api/phone/status/route.ts (1)
22-40: Consider logging SupabasefetchErrorseparately for better observabilityRight now, if the Supabase query returns a
fetchError, the route still returns200withverified: falseand no analytics entry for the DB error (only a generictrackApiResponseabove).If distinguishing “no record” from “DB failure” would help debugging, you could log/track the
fetchErrorexplicitly before falling back to the non-verified response, for example:if (fetchError) { console.error("Supabase phone status fetch error:", fetchError); // optionally trackApiError(...) here } if (!verification) { return NextResponse.json({ success: true, verified: false, phoneNumber: null, verifiedAt: null, provider: null, }); }This keeps the user-facing behavior the same while making operational issues easier to spot.
app/api/kyc/smile-id/route.ts (1)
71-79: Revisit storing rawimagespayload inimage_links
image_linksis being set toJSON.stringify(images). Given this is biometric KYC data, storing full image payloads directly inuser_kyc_profilescan:
- Make the table heavy and slower to query.
- Increase exposure of sensitive content in case of accidental wide queries or logs.
If the column is intended to store references (e.g. URLs or IDs managed by SmileID or object storage), consider persisting only those references instead of the raw images array. If regulatory requirements mandate storing images, it may be cleaner to keep them in a dedicated, access-controlled table or storage bucket and store minimal metadata here.
app/pages/TransactionForm.tsx (2)
107-108: Clarify units forusdAmountused incanTransact/ limit checksIn
handleSwap, you compute:const formData = getValues(); const usdAmount = formData.amountReceived || 0; // Check transaction limits based on KYC tier const limitCheck = canTransact(usdAmount);The comment says “Calculate the USD amount”, but
amountReceivedis the fiat amount in the selected payoutcurrency, not necessarily USD. SinceuseKYCStatus.canTransactand the KYC tiers appear to be expressed in a single currency (displayed with$inTransactionLimitModal), this is slightly misleading and could cause incorrect checks if limits are indeed USD-based.Consider either:
- Renaming the variable to something currency-agnostic (
transactionAmount) if limits are intended to be in the payout currency, or- Converting
amountReceivedto the canonical currency used by your KYC limits (likely USD) before callingcanTransact, to keep the comparison consistent.This will make the behavior clearer and reduce future confusion around what units the limit checker expects.
Also applies to: 460-477
853-857: TransactionLimitModal integration and button callback look correctRendering
TransactionLimitModalwithisOpen,onClose, andtransactionAmount={blockedTransactionAmount}, and passing() => setIsLimitModalOpen(true)intobuttonActionkeeps all “limit exceeded / need KYC upgrade” cases funneled through a single modal.One small improvement you might consider later: when opening the modal from
buttonAction(nothandleSwap), pass the current intended receive amount intoblockedTransactionAmountas well, so the modal can consistently reflect the attempted transaction size.Also applies to: 887-888
app/hooks/usePhoneVerification.ts (1)
40-84: TightenuseCallbackdependencies to avoid future stale closures
checkVerificationStatuscloses overfetchGuardsandguardKeybut only depends onwalletAddress. This works today because both derive fromwalletAddress/window, but it can become brittle if the guard key computation changes.To keep this resilient and satisfy
react-hooks/exhaustive-deps, consider:- const checkVerificationStatus = useCallback(async () => { + const checkVerificationStatus = useCallback(async () => { // ... - }, [walletAddress]); + }, [walletAddress, guardKey, fetchGuards]);This ensures the guard logic always reflects the latest key/map while preserving behavior.
app/api/phone/verify-otp/route.ts (1)
51-65: Consider tracking analytics for expiry and max-attempts branchesThe OTP expiry and “maximum attempts exceeded” paths return 400/429 without calling
trackApiErrorortrackApiResponse. If you rely on analytics for monitoring abuse or UX friction, instrumenting these branches similarly to other error paths would make the metrics more complete.This is non-blocking but worth aligning for observability.
app/api/phone/send-otp/route.ts (1)
28-35: Optional: add analytics for invalid phone number formatWhen
!validation.isValid, the route returns a 400 but doesn’t log viatrackApiErrorortrackApiResponse. If you care about measuring validation failures (e.g. country code/format mistakes), consider adding analytics here similar to the “missing required fields” case.Non-blocking, just for observability consistency.
app/components/PhoneVerificationModal.tsx (1)
166-201: Clean uphandleOtpSubmitdependencies / robustness (optional)
handleOtpSubmit’s dependency list includesonVerifiedandonClose, but they aren’t referenced in the function body. This is harmless but can confuse readers and hook linters.Optionally either:
- Remove them from the deps array, or
- Use them (e.g. pass through success state here instead of in
renderVerified).Also, for extra safety you could guard against an unexpected empty
formattedPhonebefore POSTing (defensive check), though the current flow already sets it on successful OTP send.app/lib/phone-verification.ts (1)
146-150: Use a cryptographically stronger source for OTP generation
generateOTPcurrently usesMath.random(), which isn’t suitable for security‑sensitive codes:export function generateOTP(): string { return Math.floor(100000 + Math.random() * 900000).toString(); }For verification codes that gate KYC/limits, it’s safer to use a crypto-strength RNG, e.g.:
+import { randomInt } from 'crypto'; + export function generateOTP(): string { - return Math.floor(100000 + Math.random() * 900000).toString(); + return randomInt(100000, 1000000).toString(); // 6 digits }This keeps the UX the same while hardening against prediction.
app/components/KycModal.tsx (1)
621-625: Avoid logging potentially sensitive error details to console.Error messages and stack traces may contain sensitive information (wallet addresses, user data) that should not be logged client-side.
} catch (error) { - if (error instanceof Error) { - console.error("Error message:", error.message); - console.error("Error stack:", error.stack); - } + console.error("Failed to submit verification data"); toast.error("Failed to submit verification data"); setStep(STEPS.STATUS.FAILED); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/icons/placeholder.pngis excluded by!**/*.png
📒 Files selected for processing (24)
.env.example(1 hunks)app/api/aggregator.ts(1 hunks)app/api/kyc/smile-id/route.ts(1 hunks)app/api/kyc/status/route.ts(1 hunks)app/api/kyc/transaction-summary/route.ts(1 hunks)app/api/phone/send-otp/route.ts(1 hunks)app/api/phone/status/route.ts(1 hunks)app/api/phone/verify-otp/route.ts(1 hunks)app/components/KycModal.tsx(14 hunks)app/components/PhoneVerificationModal.tsx(1 hunks)app/components/ProfileDrawer.tsx(1 hunks)app/components/SettingsDropdown.tsx(4 hunks)app/components/TransactionLimitModal.tsx(1 hunks)app/components/index.ts(1 hunks)app/hooks/useKYCStatus.ts(1 hunks)app/hooks/usePhoneVerification.ts(1 hunks)app/hooks/useSwapButton.ts(1 hunks)app/lib/countries.ts(1 hunks)app/lib/phone-verification.ts(1 hunks)app/lib/smileID.ts(1 hunks)app/lib/supabase.ts(2 hunks)app/pages/TransactionForm.tsx(9 hunks)package.json(3 hunks)supabase/migrations/create_user_kyc_profiles.sql(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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/PhoneVerificationModal.tsxapp/pages/TransactionForm.tsxapp/components/KycModal.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/components/PhoneVerificationModal.tsxapp/pages/TransactionForm.tsxapp/components/KycModal.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/pages/TransactionForm.tsx
🧬 Code graph analysis (14)
app/api/kyc/smile-id/route.ts (2)
app/lib/smileID.ts (1)
submitSmileIDJob(6-65)app/lib/supabase.ts (1)
supabaseAdmin(11-20)
app/api/phone/status/route.ts (2)
app/api/kyc/status/route.ts (1)
GET(5-67)app/lib/supabase.ts (1)
supabaseAdmin(11-20)
app/api/kyc/transaction-summary/route.ts (3)
app/api/kyc/status/route.ts (1)
GET(5-67)app/api/phone/status/route.ts (1)
GET(5-59)app/lib/supabase.ts (1)
supabaseAdmin(11-20)
app/hooks/useKYCStatus.ts (1)
app/api/aggregator.ts (1)
fetchKYCStatus(306-352)
app/components/SettingsDropdown.tsx (1)
app/components/ProfileDrawer.tsx (1)
ProfileDrawer(28-375)
app/components/PhoneVerificationModal.tsx (1)
app/lib/countries.ts (4)
Country(1-6)fetchCountries(15-60)searchCountries(95-104)getPopularCountries(88-90)
app/api/aggregator.ts (2)
app/types.ts (1)
InitiateKYCResponse(231-238)app/lib/server-analytics.ts (3)
trackServerEvent(130-175)trackApiResponse(241-258)trackBusinessEvent(362-375)
app/api/kyc/status/route.ts (3)
app/api/kyc/transaction-summary/route.ts (1)
GET(5-81)app/api/phone/status/route.ts (1)
GET(5-59)app/lib/supabase.ts (1)
supabaseAdmin(11-20)
app/components/ProfileDrawer.tsx (4)
app/hooks/useKYCStatus.ts (2)
useKYCStatus(66-187)KYC_TIERS(21-46)app/utils.ts (3)
classNames(23-25)formatNumberWithCommas(56-60)shortenAddress(536-545)app/components/AnimatedComponents.tsx (1)
sidebarAnimation(117-127)app/components/TransactionLimitModal.tsx (1)
TransactionLimitModal(25-221)
app/components/TransactionLimitModal.tsx (5)
app/hooks/useKYCStatus.ts (2)
useKYCStatus(66-187)KYC_TIERS(21-46)app/hooks/usePhoneVerification.ts (1)
usePhoneVerification(19-106)app/utils.ts (1)
formatNumberWithCommas(56-60)app/components/PhoneVerificationModal.tsx (1)
PhoneVerificationModal(28-536)app/components/KycModal.tsx (1)
KycModal(57-670)
app/pages/TransactionForm.tsx (2)
app/hooks/useKYCStatus.ts (1)
useKYCStatus(66-187)app/components/TransactionLimitModal.tsx (1)
TransactionLimitModal(25-221)
app/api/phone/verify-otp/route.ts (3)
app/api/kyc/smile-id/route.ts (1)
POST(5-113)app/api/phone/send-otp/route.ts (1)
POST(11-100)app/lib/supabase.ts (1)
supabaseAdmin(11-20)
app/components/KycModal.tsx (4)
app/components/ImageAssets.tsx (3)
UserDetailsIcon(677-729)SadFaceIcon(771-807)VerificationPendingIcon(731-770)app/components/Styles.ts (1)
secondaryBtnClasses(6-6)app/api/aggregator.ts (1)
submitSmileIDData(680-724)app/hooks/analytics/useMixpanel.ts (1)
trackEvent(91-110)
app/api/phone/send-otp/route.ts (2)
app/lib/phone-verification.ts (4)
validatePhoneNumber(37-67)generateOTP(149-151)sendTermiiOTP(72-115)sendTwilioOTP(120-144)app/lib/supabase.ts (1)
supabaseAdmin(11-20)
🔇 Additional comments (10)
package.json (1)
36-73: New KYC/phone-related dependencies align with the rest of the PRThe additions of
@smileid/web-components,smile-identity-core,libphonenumber-js, andtwiliomatch the SmileID and phone verification flows wired up elsewhere in the PR. As long as Twilio and SmileID core are only imported from server routes (never client components), this setup looks consistent.app/components/index.ts (1)
30-31: Re-exports look consistent with new modal componentsExporting
PhoneVerificationModalandTransactionLimitModalfrom the central components index is consistent with how other modals are exposed and will simplify imports for callers.app/hooks/useKYCStatus.ts (1)
66-186: KYC status hook structure looks solidThe overall design of
useKYCStatus—per-wallet fetch guards, consolidated KYC + transaction summary state, and helper methods for limits andcanTransact—looks coherent and appropriate for the flows consuming it. I don’t see any blocking issues here.app/components/SettingsDropdown.tsx (1)
22-29: Profile drawer wiring from settings dropdown looks correctThe new “Profile” menu item and
isProfileDrawerOpenstate correctly open theProfileDrawerwhile closing the dropdown, and the drawer’sonClosecleanly resets its open state. No functional issues spotted with this integration.Also applies to: 39-40, 273-288, 314-317
app/pages/TransactionForm.tsx (1)
72-87: KYC status hook + limit modal state wiring looks consistentUsing
useKYCStatusforcanTransact,tier,isPhoneVerified, andrefreshStatus, plus the newisLimitModalOpen/blockedTransactionAmountstate, fits cleanly with the transaction-limit and KYC flows (includingTransactionLimitModal). No functional issues spotted here.app/components/KycModal.tsx (5)
8-16: LGTM: Standard web component type declaration.The global JSX intrinsic element declaration with
anytype is appropriate for integrating third-party web components that lack TypeScript definitions.
200-240: LGTM: Consistent icon sizing improvements.The icon size adjustments from
size-5tosize-7improve visual consistency, and the spacer div at line 240 maintains proper alignment for the policy link.
358-469: LGTM: Status rendering updates align with new capture flow.The status rendering functions have been appropriately updated with new iconography (Clock05Icon), revised copy, and consistent styling that aligns with the in-app SmileID capture flow.
656-666: LGTM: Clean render pattern using object literal.The refactored render logic using an object literal indexed by
stepis more concise and type-safe than a switch statement.
82-82: Remove unusedisCapturingstate.The
isCapturingstate is declared but never set totrueanywhere in the component, making the conditional rendering at lines 340-344 unreachable dead code.- const [isCapturing, setIsCapturing] = useState(false);And remove the dead code:
- {isCapturing && ( - <div className="text-center text-sm text-gray-500"> - Processing your verification... - </div> - )}Also applies to: 340-344
⛔ 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 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: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.
…er validation improvements
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
app/lib/phone-verification.ts (1)
4-8: Guard Twilio client initialization against missing env vars.The Twilio client is created at module load using non-null assertions. If
TWILIO_ACCOUNT_SIDorTWILIO_AUTH_TOKENare absent (e.g., local dev, tests, preview environments), this will throw during module import and break any route that depends on this file, even if Twilio isn't used.Consider lazily initializing the client inside
sendTwilioOTPwith explicit checks, as suggested in the previous review.app/api/kyc/smile-id/route.ts (1)
19-24: Add server-side wallet signature verification.The route requires
walletAddress,signature, andnoncebut never verifies that the signature corresponds to the wallet and message. Any caller who completes a SmileID job can associate that result with anywalletAddresswithout proving control of the wallet.Verify the signature server-side before trusting
walletAddressor updating the KYC profile, as detailed in the previous review.app/components/TransactionLimitModal.tsx (1)
113-124: Guard progress bar width calculation against zero monthly limit.For tier 0,
getCurrentLimits()returns{ monthly: 0 }, so line 123's division producesInfinity/NaN, resulting in an invalidwidth: "NaN%"style. This will display a broken progress bar for unverified users.Guard against zero monthly limit and clamp to 0-100%, as suggested in the previous review.
app/components/KycModal.tsx (1)
150-154: Add validation before proceeding to CAPTURE step.The code proceeds to
STEPS.CAPTUREwithout verifying that SmileID components have loaded (smileIdLoadedistrue). If loading failed due to network issues, the user will encounter a broken capture screen.Check
smileIdLoadedbefore advancing to CAPTURE, as detailed in the previous review.supabase/migrations/create_user_kyc_profiles.sql (1)
1-20:⚠️ Plaintext OTP and PII storage — security & compliance risk (duplicate of past feedback).This table stores sensitive fields in plaintext that create immediate DB-compromise and GDPR/compliance risk:
- Line 6:
phone_number TEXT NOT NULL— plaintext phone enables bulk harvesting on DB breach or backup theft.- Line 8:
otp_code TEXT— plaintext OTP enables immediate reuse; attacker with DB access can verify/exhaust 2FA.- Lines 13–14:
id_info JSONB, image_links JSONB— likely contain KYC documents (facial images, ID scans); plaintext JSONB = raw document exposure.This issue was already raised in detail in past review comments (lines 4–18). Recommended fixes: hash/HMAC OTP with server KEK (KMS-managed), use deterministic or randomized field-level encryption for phone/PII, avoid plaintext JSONB for docs.
🧹 Nitpick comments (5)
app/components/KycModal.tsx (1)
641-641: Remove debug console.log statement.Production code should not contain debug logging in event handlers.
Apply this diff:
const handleBack = (event: any) => { - console.log("Back detail:", event.detail); };supabase/migrations/create_user_kyc_profiles.sql (4)
28-34: Apply PL/pgSQL best practice: useCURRENT_TIMESTAMPinstead ofNOW().Lines 31 and 34 use
NOW(). PostgreSQL best practice is to preferCURRENT_TIMESTAMPfor consistency and to avoid function call overhead in triggers. Both are equivalent, butCURRENT_TIMESTAMPis standard SQL.CREATE OR REPLACE FUNCTION update_user_kyc_profiles_updated_at() RETURNS TRIGGER AS $$ BEGIN - NEW.updated_at = NOW(); + NEW.updated_at = CURRENT_TIMESTAMP; RETURN NEW; END; $$ language 'plpgsql';
43-52: Verify RLS context setup and document wallet_address injection point.Line 48 RLS policy uses
current_setting('app.wallet_address', true)to gate access. This assumes the application layer sets this context before issuing queries. If not set correctly:
- RLS policy silently permits no rows (fails open? or closed?).
- Test coverage may miss the misconfiguration.
Ensure the application layer (API routes, client library) explicitly calls:
SET app.wallet_address TO <wallet_address>; -- or via Supabase client library (if available)Add a comment or ADR documenting this dependency.
9-9: Add automatic cleanup trigger or scheduled job for expired OTPs.Line 9 (
expires_at) tracks OTP expiry, but there is no mechanism to delete expired or used OTP records. This accumulates stale data and increases DB size/query cost over time. Additionally, line 17 (attempts) tracks failed attempts but has no logic to reset or enforce limits.Add a database trigger (on successful OTP verification) to NULL out
otp_codeandattempts, or add a pg_cron job to delete records whereexpires_at < NOW()andverified = true.-- Example: auto-cleanup expired OTP records (run via pg_cron) DELETE FROM user_kyc_profiles WHERE expires_at < NOW() - INTERVAL '1 day' AND verified = true;Also applies to: 17-17
4-5: Consider adding foreign key and additional constraints for data integrity.
- Line 4 (
wallet_address PRIMARY KEY): Assumes wallet address uniquely identifies a user. If a user can have multiple wallets or if there's a separateuserstable, add a FK tousers(id)or clarify the identity model.- Line 12 (
smile_job_id): Likely references Smile ID external API; consider adding a NOT NULL constraint once Tier 2 users complete Smile ID verification (or ensure nullable is intentional for Tier 1–only users).- Line 15 (
tierCHECK): Good constraint; clarify in code comments that tier progression is: 0 (unverified) → 1 (phone verified) → 2 (full KYC via Smile ID).Also applies to: 12-12, 15-15
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/api/aggregator.ts(2 hunks)app/api/kyc/smile-id/route.ts(1 hunks)app/api/phone/send-otp/route.ts(1 hunks)app/api/phone/verify-otp/route.ts(1 hunks)app/components/KycModal.tsx(14 hunks)app/components/PhoneVerificationModal.tsx(1 hunks)app/components/TransactionLimitModal.tsx(1 hunks)app/lib/phone-verification.ts(1 hunks)app/types.ts(1 hunks)supabase/migrations/create_user_kyc_profiles.sql(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/api/phone/verify-otp/route.ts
🧰 Additional context used
🧠 Learnings (5)
📚 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/PhoneVerificationModal.tsxapp/components/TransactionLimitModal.tsxapp/lib/phone-verification.tsapp/components/KycModal.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/PhoneVerificationModal.tsxapp/components/TransactionLimitModal.tsxapp/components/KycModal.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/PhoneVerificationModal.tsxapp/components/KycModal.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/components/TransactionLimitModal.tsxapp/components/KycModal.tsx
📚 Learning: 2025-10-13T14:37:45.409Z
Learnt from: sundayonah
Repo: paycrest/noblocks PR: 250
File: app/components/AppLayout.tsx:33-46
Timestamp: 2025-10-13T14:37:45.409Z
Learning: In TypeScript/JavaScript/React codebases, JSX whitespace expressions like `{" "}` between elements should be classified as style/nitpick issues, not "Potential issues" or problems that affect functionality. These are purely cosmetic concerns about code cleanliness.
Applied to files:
app/components/KycModal.tsx
🧬 Code graph analysis (6)
app/api/aggregator.ts (2)
app/types.ts (1)
SmileIDSubmissionResponse(240-248)app/lib/server-analytics.ts (3)
trackServerEvent(130-175)trackApiResponse(241-258)trackBusinessEvent(362-375)
app/components/PhoneVerificationModal.tsx (1)
app/lib/countries.ts (4)
Country(1-6)fetchCountries(15-60)searchCountries(95-104)getPopularCountries(88-90)
app/components/TransactionLimitModal.tsx (5)
app/hooks/useKYCStatus.ts (2)
useKYCStatus(66-187)KYC_TIERS(21-46)app/hooks/usePhoneVerification.ts (1)
usePhoneVerification(19-106)app/utils.ts (1)
formatNumberWithCommas(56-60)app/components/PhoneVerificationModal.tsx (1)
PhoneVerificationModal(28-536)app/components/KycModal.tsx (1)
KycModal(57-673)
app/api/kyc/smile-id/route.ts (4)
app/api/phone/send-otp/route.ts (1)
POST(11-103)app/api/phone/verify-otp/route.ts (1)
POST(7-132)app/lib/smileID.ts (1)
submitSmileIDJob(6-65)app/lib/supabase.ts (1)
supabaseAdmin(11-20)
app/components/KycModal.tsx (2)
app/components/Styles.ts (1)
secondaryBtnClasses(6-6)app/api/aggregator.ts (1)
submitSmileIDData(681-724)
app/api/phone/send-otp/route.ts (2)
app/lib/phone-verification.ts (4)
validatePhoneNumber(43-75)generateOTP(157-159)sendTermiiOTP(80-123)sendTwilioOTP(128-152)app/lib/supabase.ts (1)
supabaseAdmin(11-20)
🔇 Additional comments (7)
app/api/aggregator.ts (1)
673-724: LGTM!The function correctly uses
SmileIDSubmissionResponseas return type and avoids logging sensitive KYC payload data. Analytics tracking includes only non-PII metadata (wallet_address, jobId).app/api/kyc/smile-id/route.ts (1)
92-99: Good addition: Row update verification.The check for
updatedProfile.length === 0ensures that a KYC profile row was actually updated and returns 404 if not. This prevents silent failures when the phone verification step hasn't created a profile yet.app/api/phone/send-otp/route.ts (1)
42-75: LGTM!The code now correctly uses provider-specific phone formats:
e164Format(no spaces) for Twilio and storage,digitsOnly(no + prefix) for Termii. This addresses the previous review concerns about provider compatibility.app/components/PhoneVerificationModal.tsx (1)
114-164: LGTM!The
handlePhoneSubmitdependency array now correctly includesselectedCountryandname, ensuring the OTP is sent with the current country code and name. This addresses the previous review concern.app/components/KycModal.tsx (1)
597-599: Good addition: Validation before submission.The validation checks ensure that
walletAddress,kycSignature, andkycNonceare present before constructing the payload. This prevents submitting invalid data to the backend.supabase/migrations/create_user_kyc_profiles.sql (2)
27-40: Verify thatupdate_user_kyc_profiles_updated_atfunction is not duplicated elsewhere in migration.Past review comments indicate lines 27–41 and 54–66 both define the same function and trigger. The provided code shows only lines 27–40. If the duplicate exists in the full file, remove lines 54–66 to keep one function and one trigger.
23-23: Remove this review comment—it references a non-existent security requirement.The migration file contains no security recommendation about phone_number encryption above line 23. Phone numbers are stored plaintext by design: the verification API requires equality queries on
phone_number(in send-otp, verify-otp, and status endpoints), which precludes encryption. The index onphone_numberis appropriate for these queryable fields. This reflects an architectural decision, not a security oversight.Likely an incorrect or invalid review comment.
…ion limit display logic
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/components/TransactionLimitModal.tsx (2)
55-59: Redundant undefined check for tier.The
tier === undefinedcheck on line 56 is unnecessary becausetieris typed as0 | 1 | 2inuseKYCStatusand initialized to0. TypeScript's type system ensurestiercan never beundefined.Apply this diff to simplify the condition:
- useEffect(() => { - if (isOpen && !isLoading && (tier < 1 || tier === undefined)) { - setIsPhoneModalOpen(true); - } - }, [isOpen, isLoading, tier]); + useEffect(() => { + if (isOpen && !isLoading && tier < 1) { + setIsPhoneModalOpen(true); + } + }, [isOpen, isLoading, tier]);
67-70: Revisit: parent onClose still called twice on phone verification.Following up on the previous discussion: when
PhoneVerificationModalsucceeds, it calls bothonVerified(formattedPhone)andonClose()(per the component's implementation). This means:
onVerifiedfires →handlePhoneVerifiedruns → callsonClose()(closes parent)onClosefires → the prop at lines 202-205 runs → callsonClose()(closes parent again)While calling
setIsPhoneModalOpen(false)twice is harmless, invoking the parentonClose()twice could cause unintended side effects depending on what that callback does elsewhere in the app.To eliminate the redundant call while preserving explicit control over the phone modal state, consider:
const handlePhoneVerified = async (phoneNumber: string) => { + await Promise.all([refreshStatus(), refreshPhoneStatus()]); setIsPhoneModalOpen(false); - onClose(); + // onClose will be called by PhoneVerificationModal's onClose prop };This way,
handlePhoneVerifiedrefreshes state and closes the phone modal, while thePhoneVerificationModal'sonCloseprop (lines 202–205) handles closing the parent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.env.example(1 hunks)app/components/TransactionLimitModal.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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/TransactionLimitModal.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/TransactionLimitModal.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/components/TransactionLimitModal.tsx
🧬 Code graph analysis (1)
app/components/TransactionLimitModal.tsx (5)
app/hooks/useKYCStatus.ts (2)
useKYCStatus(66-187)KYC_TIERS(21-46)app/hooks/usePhoneVerification.ts (1)
usePhoneVerification(19-106)app/utils.ts (1)
formatNumberWithCommas(56-60)app/components/PhoneVerificationModal.tsx (1)
PhoneVerificationModal(28-536)app/components/KycModal.tsx (1)
KycModal(57-673)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 113-113: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 114-114: [UnorderedKey] The SMILE_IDENTITY_API_KEY key should go before the SMILE_IDENTITY_BASE_URL key
(UnorderedKey)
[warning] 116-116: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 116-116: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 117-117: [UnorderedKey] The SMILE_IDENTITY_SERVER key should go before the SMILE_ID_CALLBACK_URL key
(UnorderedKey)
[warning] 117-117: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🔇 Additional comments (2)
app/components/TransactionLimitModal.tsx (2)
113-132: Progress bar guard implemented correctly.The division-by-zero guard for tier 0 users (when
currentLimits.monthlyis 0) has been properly implemented. The progress bar now safely displays "0%" instead of "NaN%" when the monthly limit is zero, and correctly clamps the percentage to a maximum of 100%.
209-221: Verify: Missing state refresh and user tracking after KYC completion.Two potential issues with the KYC modal integration:
Empty callback:
setIsUserVerified={() => { }}(line 217) passes an empty function toKycModal. If the parent component should track when KYC verification succeeds (to potentially refresh limits or update UI), this callback may need implementation.No status refresh after KYC: Unlike
handlePhoneVerified(which could callrefreshStatus()), there's no explicit refresh of KYC status after the KYC modal closes successfully. WhileKycModalmay handle internal state updates, theTransactionLimitModalwon't reflect the new tier/limits until the user reopens it or the page refreshes.Consider:
<AnimatedModal isOpen={isKycModalOpen} - onClose={() => { setIsKycModalOpen(false); onClose(); }} + onClose={async () => { + setIsKycModalOpen(false); + await refreshStatus(); // Refresh to reflect new tier + onClose(); + }} > <KycModal setIsKycModalOpen={setIsKycModalOpen} - setIsUserVerified={() => { }} + setIsUserVerified={async (verified) => { + if (verified) { + await refreshStatus(); + } + }} /> </AnimatedModal>Verify whether
KycModalalready triggersrefreshStatusvia a different mechanism, or if this refresh should be added here.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/api/phone/verify-otp/route.ts (3)
62-76: Add error tracking for observability.The expiration and max-attempts checks return error responses but don't call
trackApiError, unlike other error paths in this handler (lines 18, 28, 44, 88, 117, 136). This reduces observability.Apply this diff to add consistent error tracking:
// Check expiration if (new Date() > new Date(verification.expires_at)) { + trackApiError(request, '/api/phone/verify-otp', 'POST', new Error('OTP expired'), 400); return NextResponse.json( { success: false, error: 'OTP has expired. Please request a new one.' }, { status: 400 } ); } // Check attempts if (verification.attempts >= MAX_ATTEMPTS) { + trackApiError(request, '/api/phone/verify-otp', 'POST', new Error('Max attempts exceeded'), 429); return NextResponse.json( { success: false, error: 'Maximum verification attempts exceeded. Please request a new OTP.' }, { status: 429 } ); }
105-113: Minor: Fix whitespace formatting.Line 110 has inconsistent spacing (space before the comma).
Apply this diff:
.update({ verified: true, - verified_at: new Date().toISOString() , + verified_at: new Date().toISOString(), tier: 1 })
127-132: Consider returning the normalized phone number for consistency.The response returns the original
phoneNumberfrom the request body rather than the normalized E.164 format stored in the database. While this may be intentional for UX, it could lead to inconsistency if other endpoints return the normalized format.Optionally, return the normalized format for consistency:
return NextResponse.json({ success: true, message: 'Phone number verified successfully', verified: true, - phoneNumber: phoneNumber + phoneNumber: validation.e164Format });Or use property shorthand if keeping the original:
- phoneNumber: phoneNumber + phoneNumber
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/api/phone/verify-otp/route.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/api/phone/verify-otp/route.ts (3)
app/api/phone/send-otp/route.ts (1)
POST(11-103)app/lib/phone-verification.ts (1)
validatePhoneNumber(43-75)app/lib/supabase.ts (1)
supabaseAdmin(11-20)
🔇 Additional comments (5)
app/api/phone/verify-otp/route.ts (5)
1-6: LGTM!Imports and constants are appropriate for the OTP verification flow.
17-33: LGTM!Input validation is thorough and properly normalizes phone numbers to E.164 format for consistent querying.
35-49: LGTM!Verification record lookup correctly uses normalized E.164 format and lowercased wallet address for consistent querying.
51-60: LGTM!Idempotent check for already-verified records is appropriate and prevents unnecessary processing.
134-142: LGTM!Catch-all error handling appropriately logs, tracks, and returns a generic 500 response.
…and race condition handling
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
app/components/KycModal.tsx (3)
489-493: Remove commented-out code.The commented lines at 490-491 are from the old QR-based flow and are no longer needed.
Apply this diff:
if (newStatus === STEPS.STATUS.PENDING) { - // setKycUrl(response.data.url); - // setIsKycModalOpen(true); return; }
628-630: Remove debug console.log statement.The console.log at Line 629 should be removed for production code.
Apply this diff:
const handleBack = (event: any) => { - console.log("Back detail:", event.detail); };
145-149: Add validation before proceeding to CAPTURE step.The code proceeds to the CAPTURE step without verifying that SmileID components have successfully loaded. If
smileIdLoadedisfalse(e.g., due to network issues or loading failure), the user will encounter a broken capture screen.Apply this diff to add a guard:
setKycSignature(sigWithoutPrefix); setKycNonce(nonce); - // Skip old KYC initiation since we're using Smile ID - setStep(STEPS.CAPTURE); + // Check if SmileID components are loaded before proceeding + if (smileIdLoaded) { + setStep(STEPS.CAPTURE); + } else { + toast.error("Verification component not ready. Please try again."); + setIsKycModalOpen(false); + setStep(STEPS.TERMS); + }
🧹 Nitpick comments (5)
app/components/TransactionLimitModal.tsx (2)
59-62: Refresh KYC status after phone verification.After successful phone verification, the KYC status (tier, limits, etc.) should be refreshed so the user sees their updated tier when this modal reopens. Currently, the status remains stale until the next time the modal is opened.
Apply this diff to refresh status after verification:
const handlePhoneVerified = async (phoneNumber: string) => { + await refreshStatus(); setIsPhoneModalOpen(false); onClose(); };
207-210: Consider handling KYC completion state.The
setIsUserVerifiedprop is passed as an empty function, meaning successful Tier 2 KYC completion won't update any state in this component. If the parent component needs to know about KYC completion, consider passing a real handler or refreshing status whenKycModalcloses.If status tracking is needed:
<KycModal setIsKycModalOpen={setIsKycModalOpen} - setIsUserVerified={() => { }} + setIsUserVerified={async () => { + await refreshStatus(); + }} />app/components/KycModal.tsx (1)
83-95: Remove debug console.log statement.The console.log at Line 87 should be removed for production code.
Apply this diff:
import("@smileid/web-components/smart-camera-web") .then(() => { - console.log("SmileID web components loaded"); setSmileIdLoaded(true); })app/api/phone/verify-otp/route.ts (2)
62-76: Add analytics logging for OTP expiry/limit/invalid flowsThe business rules for Line 62–76 and Line 78–114 look right, but these key 4xx outcomes aren’t currently logged via
trackApiErrorortrackApiResponse, which makes it harder to monitor OTP failures vs system errors.Consider adding explicit error tracking in these branches, e.g.:
// Check expiration if (new Date() > new Date(verification.expires_at)) { + trackApiError(request, '/api/phone/verify-otp', 'POST', new Error('OTP expired'), 400); return NextResponse.json( { success: false, error: 'OTP has expired. Please request a new one.' }, { status: 400 } ); } // Check attempts if (verification.attempts >= MAX_ATTEMPTS) { + trackApiError(request, '/api/phone/verify-otp', 'POST', new Error('Maximum verification attempts exceeded'), 429); return NextResponse.json( { success: false, error: 'Maximum verification attempts exceeded. Please request a new OTP.' }, { status: 429 } ); } // Verify OTP if (verification.otp_code !== otpCode) { // Atomic increment with boundary check to prevent race conditions const { data: updated, error: attemptsError } = await supabaseAdmin .from('user_kyc_profiles') .update({ attempts: verification.attempts + 1 }) .eq('wallet_address', walletAddress.toLowerCase()) .lt('attempts', MAX_ATTEMPTS) .select('attempts') .single(); if (attemptsError) { console.error('Failed to increment OTP attempts:', attemptsError); trackApiError(request, '/api/phone/verify-otp', 'POST', attemptsError, 500); return NextResponse.json( { success: false, error: 'Failed to process verification attempt' }, { status: 500 } ); } // If no rows updated, attempts limit was hit mid-flight (race condition) if (!updated) { + trackApiError(request, '/api/phone/verify-otp', 'POST', new Error('Maximum verification attempts exceeded'), 429); return NextResponse.json( { success: false, error: 'Maximum verification attempts exceeded. Please request a new OTP.' }, { status: 429 } ); } + trackApiError(request, '/api/phone/verify-otp', 'POST', new Error('Invalid OTP code'), 400); return NextResponse.json( { success: false, error: 'Invalid OTP code', attemptsRemaining: MAX_ATTEMPTS - updated.attempts }, { status: 400 } ); }This keeps analytics for verification failures on par with the
/send-otproute.Also applies to: 78-114
116-143: Avoid unintentionally downgrading existing KYC tier when marking phone verifiedOn Line 119–123 you always set
tier: 1when the OTP is correct. If this route is ever hit for a profile that already has a higher tier (e.g., after SmileID full KYC), this would silently down-tier them.If higher tiers are possible on
user_kyc_profiles, consider preserving the maximum tier instead of forcing1, for example by only updating when the current tier is lower, or using a DB-sideGREATEST:- const { error: updateError } = await supabaseAdmin - .from('user_kyc_profiles') - .update({ - verified: true, - verified_at: new Date().toISOString() , - tier: 1 - }) - .eq('wallet_address', walletAddress.toLowerCase()); + const { error: updateError } = await supabaseAdmin + .from('user_kyc_profiles') + .update({ + verified: true, + verified_at: new Date().toISOString(), + // e.g. leave tier unchanged if already >= 1 + tier: 1, + }) + .eq('wallet_address', walletAddress.toLowerCase()); + // Optionally add a condition or DB-side expression to avoid lowering a higher tier + // .lte('tier', 1)Adjust the exact logic to match how tiers are modeled elsewhere in the KYC flow.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/api/phone/verify-otp/route.ts(1 hunks)app/components/KycModal.tsx(14 hunks)app/components/TransactionLimitModal.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 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/TransactionLimitModal.tsxapp/components/KycModal.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/TransactionLimitModal.tsxapp/components/KycModal.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/components/TransactionLimitModal.tsxapp/components/KycModal.tsx
📚 Learning: 2025-10-13T14:37:45.409Z
Learnt from: sundayonah
Repo: paycrest/noblocks PR: 250
File: app/components/AppLayout.tsx:33-46
Timestamp: 2025-10-13T14:37:45.409Z
Learning: In TypeScript/JavaScript/React codebases, JSX whitespace expressions like `{" "}` between elements should be classified as style/nitpick issues, not "Potential issues" or problems that affect functionality. These are purely cosmetic concerns about code cleanliness.
Applied to files:
app/components/KycModal.tsx
🧬 Code graph analysis (3)
app/components/TransactionLimitModal.tsx (4)
app/hooks/useKYCStatus.ts (2)
useKYCStatus(66-187)KYC_TIERS(21-46)app/utils.ts (1)
formatNumberWithCommas(56-60)app/components/PhoneVerificationModal.tsx (1)
PhoneVerificationModal(28-536)app/components/KycModal.tsx (1)
KycModal(55-661)
app/api/phone/verify-otp/route.ts (3)
app/api/phone/send-otp/route.ts (1)
POST(11-103)app/lib/phone-verification.ts (1)
validatePhoneNumber(43-75)app/lib/supabase.ts (1)
supabaseAdmin(11-20)
app/components/KycModal.tsx (4)
app/components/ImageAssets.tsx (3)
UserDetailsIcon(677-729)SadFaceIcon(771-807)VerificationPendingIcon(731-770)app/components/Styles.ts (1)
secondaryBtnClasses(6-6)app/api/aggregator.ts (1)
submitSmileIDData(681-724)app/hooks/analytics/useMixpanel.ts (1)
trackEvent(91-110)
🔇 Additional comments (1)
app/api/phone/verify-otp/route.ts (1)
25-41: Phone normalization and lookup logic looks solidUsing
validatePhoneNumberplus querying bywallet_address.toLowerCase()and normalizedvalidation.e164Formatkeeps/send-otpand/verify-otpin sync and avoids subtle mismatch bugs between display formats and stored values.
…enhancing security and access control
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
app/api/phone/verify-otp/route.ts (2)
60-69: Consider returning phoneNumber in already-verified response for consistency.The already-verified response doesn't include the
phoneNumberfield, while the successful verification response at line 151 does. For consistency and client convenience, consider adding it:return NextResponse.json({ success: true, message: 'Phone number already verified', - verified: true + verified: true, + phoneNumber: validation.internationalFormat });
147-152: Return normalized phone number format for consistency.Line 151 echoes back the raw
phoneNumberfrom the request body. For consistency with the send-otp endpoint (which returnsvalidation.internationalFormat) and to confirm the actual verified format, return the normalized value:return NextResponse.json({ success: true, message: 'Phone number verified successfully', verified: true, - phoneNumber: phoneNumber + phoneNumber: validation.internationalFormat });app/api/kyc/status/route.ts (1)
36-45: Consider more explicit nullish check for tier fallback.The condition
!kycProfile?.tierwill be true for bothundefinedand0, meaning an explicitly storedtier: 0would trigger recalculation. While tier 0 appears unused in the current system, a more explicit check would be clearer:- if (!kycProfile?.tier) { + if (kycProfile?.tier === undefined || kycProfile?.tier === null) { if (fullKYCVerified) {app/components/KycModal.tsx (2)
145-149: Add guard to ensure SmileID components loaded before advancing to CAPTURE.While the dynamic import typically completes quickly, proceeding to CAPTURE before
smileIdLoadedis true could result in a blank screen on slow connections. Consider:setKycSignature(sigWithoutPrefix); setKycNonce(nonce); - // Skip old KYC initiation since we're using Smile ID - setStep(STEPS.CAPTURE); + // Ensure SmileID components are loaded before proceeding + if (smileIdLoaded) { + setStep(STEPS.CAPTURE); + } else { + toast.error("Verification component not ready. Please try again."); + setIsKycModalOpen(false); + setStep(STEPS.TERMS); + }Alternatively, disable the "Accept and sign" button at line 300 until
smileIdLoadedis true:disabled={!termsAccepted || isSigning} + disabled={!termsAccepted || isSigning || !smileIdLoaded}
489-493: Consider removing commented-out code.The commented lines from the old QR-based flow are no longer needed and can be removed for cleaner code:
if (newStatus === STEPS.STATUS.PENDING) { - // setKycUrl(response.data.url); - // setIsKycModalOpen(true); return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/api/aggregator.ts(2 hunks)app/api/kyc/smile-id/route.ts(1 hunks)app/api/kyc/status/route.ts(1 hunks)app/api/phone/verify-otp/route.ts(1 hunks)app/components/KycModal.tsx(15 hunks)app/hooks/useKYCStatus.ts(1 hunks)middleware.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 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/KycModal.tsxapp/api/phone/verify-otp/route.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/KycModal.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/components/KycModal.tsx
📚 Learning: 2025-10-13T14:37:45.409Z
Learnt from: sundayonah
Repo: paycrest/noblocks PR: 250
File: app/components/AppLayout.tsx:33-46
Timestamp: 2025-10-13T14:37:45.409Z
Learning: In TypeScript/JavaScript/React codebases, JSX whitespace expressions like `{" "}` between elements should be classified as style/nitpick issues, not "Potential issues" or problems that affect functionality. These are purely cosmetic concerns about code cleanliness.
Applied to files:
app/components/KycModal.tsx
🧬 Code graph analysis (6)
app/api/kyc/status/route.ts (2)
app/api/phone/status/route.ts (1)
GET(5-59)app/lib/supabase.ts (1)
supabaseAdmin(11-20)
app/api/aggregator.ts (2)
app/types.ts (1)
SmileIDSubmissionResponse(240-248)app/lib/server-analytics.ts (3)
trackServerEvent(130-175)trackApiResponse(241-258)trackBusinessEvent(362-375)
app/api/kyc/smile-id/route.ts (4)
app/api/phone/verify-otp/route.ts (1)
POST(8-163)app/api/phone/send-otp/route.ts (1)
POST(11-103)app/lib/smileID.ts (1)
submitSmileIDJob(6-65)app/lib/supabase.ts (1)
supabaseAdmin(11-20)
app/hooks/useKYCStatus.ts (1)
app/api/aggregator.ts (1)
fetchKYCStatus(307-353)
app/components/KycModal.tsx (3)
app/components/Styles.ts (1)
secondaryBtnClasses(6-6)app/api/aggregator.ts (1)
submitSmileIDData(682-729)app/hooks/analytics/useMixpanel.ts (1)
trackEvent(91-110)
app/api/phone/verify-otp/route.ts (3)
app/api/phone/send-otp/route.ts (1)
POST(11-103)app/lib/phone-verification.ts (1)
validatePhoneNumber(43-75)app/lib/supabase.ts (1)
supabaseAdmin(11-20)
🔇 Additional comments (5)
middleware.ts (1)
267-268: LGTM! KYC routes are now properly protected.Adding these routes to the middleware matcher ensures JWT authentication and wallet address verification before accessing KYC endpoints, addressing the security concerns raised in previous review discussions.
app/api/aggregator.ts (1)
673-729: LGTM! Function follows established patterns.The implementation correctly:
- Uses JWT authentication via Bearer token
- Tracks analytics without exposing PII
- Returns the properly typed
SmileIDSubmissionResponse- Follows error handling patterns consistent with other functions in this file
app/api/kyc/smile-id/route.ts (1)
5-132: LGTM! Security and data integrity concerns properly addressed.The route implementation is solid:
- ✅ Protected by middleware JWT authentication (resolves previous security concerns)
- ✅ Validates all required fields before submission
- ✅ Explicitly checks that DB row was updated (lines 102-108), ensuring profile exists
- ✅ Comprehensive error handling for SmileID job states
The signature/nonce stored in the body are audit trail data; authentication is correctly handled via Privy JWT in middleware.
app/hooks/useKYCStatus.ts (1)
66-199: LGTM! Hook implementation is well-structured.The hook correctly:
- Implements per-wallet fetch guards to prevent duplicate concurrent requests
- Uses JWT authentication for secure API calls
- Derives tier and limits from KYC status and transaction data
- Properly chains useCallback dependencies to trigger refetch on wallet changes
app/components/KycModal.tsx (1)
563-645: LGTM! SmileID event handling is well-implemented.The event handling correctly:
- Guards execution to only run when in CAPTURE step with camera element ready
- Validates all required data before submission (addresses previous validation concerns)
- Uses JWT authentication for secure API calls
- Provides comprehensive error handling and user feedback
- Properly cleans up event listeners on unmount
…rate limiting - Added rate limiting to SmileID KYC submission endpoint. - Updated KYC submission to include id_info for biometric verification. - Enhanced error handling for SmileID job completion and success checks. - Modified KYC status retrieval to reflect changes in verification tiers. - Removed deprecated phone verification status endpoint. - Refactored KYC modal to support ID document selection and capture. - Updated database schema for user KYC profiles to include new fields and indexes. - Removed unused phone verification hook.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/components/KycModal.tsx (2)
166-182: Align KYC terms copy with actual data stored (PII/compliance risk).The bullets state:
- “We do not store any personal information. All personal data is handled exclusively by our third-party KYC provider.”
- “We only store the KYC reference code and signing wallet address for verification and audit purposes.”
However, the SmileID flow persists additional PII in
user_kyc_profiles(e.g.id_type,id_number,id_country,full_name,date_of_birth,email_address, platform entries). This makes the current copy misleading from a privacy/compliance standpoint.Either:
- update the text to accurately describe what is stored and for how long, or
- change the backend to only store what the text promises (which seems unlikely given KYC requirements).
Right now, the mismatch should be treated as a compliance‑level issue.
570-667: Fix pending-status polling effect to depend onstep(currently uses stale value).The polling
useEffectonly depends onwalletAddressbut readsstep. Sincestepis omitted from the dependency array (and lint is disabled), the effect captures whateverstepwas on first run (typicallyLOADING) and will never start polling whenstepis later set toPENDING.To make polling work as intended:
- useEffect(() => { + useEffect(() => { + if (!walletAddress) return; + let timeoutId: NodeJS.Timeout; const startTime = Date.now(); @@ - debouncedFetchStatus(); // Initial fetch + debouncedFetchStatus(); // Initial fetch @@ - if (step === STEPS.STATUS.PENDING) { + if (step === STEPS.STATUS.PENDING) { intervalId = setInterval(() => { @@ - return () => { + return () => { @@ - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [walletAddress]); + }, [walletAddress, step]);This way, when
steptransitions toPENDING, the effect re-runs, sets up the interval, and stops polling once the status changes or times out.
♻️ Duplicate comments (3)
app/components/KycModal.tsx (2)
594-598: Remove reintroduced commented-out QR flow remnants.The
newStatus === STEPS.STATUS.PENDINGbranch still contains commented-out code for the old QR-based flow:// setKycUrl(response.data.url); // setIsKycModalOpen(true);Since this flow has been superseded by the embedded SmileID flow, these comments are now dead code and should be removed to keep the status handler clean.
- if (newStatus === STEPS.STATUS.PENDING) { - // setKycUrl(response.data.url); - // setIsKycModalOpen(true); - return; - } + if (newStatus === STEPS.STATUS.PENDING) { + return; + }
386-401: Gate transition to CAPTURE on SmileID load state to avoid a broken capture screen.The “Continue” button in
renderIdInfomoves straight toSTEPS.CAPTUREwithout checkingsmileIdLoaded. If the dynamic import failed (or is still in flight), the user will see a non-functional<smart-camera-web>screen with no clear recovery.Reuse the existing
smileIdLoadedflag here, e.g.:- <button - type="button" - className={`${primaryBtnClasses} w-full`} - disabled={!selectedCountry || !selectedIdType || (!needsDocCapture && !idNumber)} - onClick={() => setStep(STEPS.CAPTURE)} - > + <button + type="button" + className={`${primaryBtnClasses} w-full`} + disabled={ + !selectedCountry || + !selectedIdType || + (!needsDocCapture && !idNumber) || + !smileIdLoaded + } + onClick={() => { + if (!smileIdLoaded) { + toast.error("Verification component not ready. Please try again."); + return; + } + setStep(STEPS.CAPTURE); + }} + >This prevents advancing into CAPTURE when the SmileID component failed to initialize.
Also applies to: 406-437
app/lib/smileID.ts (1)
76-84: Avoid logging full SmileID error payloads to prevent KYC/PII leakage.
console.errorcurrently logserror.response?.datawholesale, which can include sensitive KYC metadata (IDs, tokens, URLs). This is risky for logs that may be long‑lived or sent to third‑party sinks.Recommend logging only high‑level fields and optional sanitized codes, not the raw
dataobject, e.g.:- } catch (error: any) { - console.error('SmileID API Error:', { - status: error.response?.status, - statusText: error.response?.statusText, - data: error.response?.data, - message: error.message, - }); - throw error; - } + } catch (error: any) { + console.error("SmileID API Error:", { + status: error.response?.status, + statusText: error.response?.statusText, + message: error.message, + // Optionally surface a non-PII error code if present + code: error.response?.data?.code, + }); + throw error; + }If you need richer diagnostics, explicitly pick/whitelist non‑sensitive fields rather than dumping the full response.
🧹 Nitpick comments (3)
app/components/PhoneVerificationModal.tsx (1)
166-201: Remove unused dependencies fromhandleOtpSubmit.The
onVerifiedandonClosedependencies are listed in the dependency array but are never called within the function body. The function only sets the step toVERIFIED; those callbacks are invoked later inrenderVerified(lines 504-505).Including unused dependencies causes unnecessary re-creation of the callback and violates React useCallback optimization best practices.
Apply this diff:
- }, [otpCode, formattedPhone, walletAddress, onVerified, onClose]); + }, [otpCode, formattedPhone, walletAddress]);app/api/kyc/smile-id/route.ts (1)
38-45: Add server-side validation for ID number on Enhanced KYC ID types.The frontend enforces ID number input for certain ID types, but this route only checks
countryandid_type. For Enhanced KYC types (e.g. BVN/NIN variants that map to job_type 5 ingetJobTypeForIdType),id_info.id_numberis effectively required; without it,submitSmileIDJobwill likely fail and you’ll surface a 500 instead of a clear 4xx.Consider mirroring the job-type logic here and validating ID number for those types before calling SmileID, e.g.:
import { getJobTypeForIdType } from '@/app/lib/smileID'; // ... if (!id_info?.country || !id_info?.id_type) { return NextResponse.json( { status: 'error', message: 'Missing id_info: country and id_type are required' }, { status: 400 }, ); } const jobType = getJobTypeForIdType(id_info.id_type); if (jobType === 5 && !id_info.id_number) { return NextResponse.json( { status: 'error', message: 'ID number is required for this ID type' }, { status: 400 }, ); }This keeps validation consistent between FE and BE and avoids unnecessary SmileID job failures.
Also applies to: 70-81, 120-127
app/components/KycModal.tsx (1)
118-128: Remove debugconsole.logfrom SmileID component loader.The dynamic import logs
"SmileID web components loaded"on every successful load. This is useful during development but noisy in production.You can safely drop this log and keep the error path:
- import("@smileid/web-components/smart-camera-web") - .then(() => { - console.log("SmileID web components loaded"); - setSmileIdLoaded(true); - }) + import("@smileid/web-components/smart-camera-web") + .then(() => { + setSmileIdLoaded(true); + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/api/kyc/smile-id/route.ts(1 hunks)app/api/kyc/status/route.ts(1 hunks)app/api/phone/send-otp/route.ts(1 hunks)app/components/KycModal.tsx(14 hunks)app/components/PhoneVerificationModal.tsx(1 hunks)app/lib/smileID.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 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/KycModal.tsxapp/components/PhoneVerificationModal.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/KycModal.tsxapp/components/PhoneVerificationModal.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/components/KycModal.tsxapp/components/PhoneVerificationModal.tsx
📚 Learning: 2025-10-13T14:37:45.409Z
Learnt from: sundayonah
Repo: paycrest/noblocks PR: 250
File: app/components/AppLayout.tsx:33-46
Timestamp: 2025-10-13T14:37:45.409Z
Learning: In TypeScript/JavaScript/React codebases, JSX whitespace expressions like `{" "}` between elements should be classified as style/nitpick issues, not "Potential issues" or problems that affect functionality. These are purely cosmetic concerns about code cleanliness.
Applied to files:
app/components/KycModal.tsx
🧬 Code graph analysis (3)
app/api/phone/send-otp/route.ts (3)
app/lib/rate-limit.ts (1)
rateLimit(10-30)app/lib/phone-verification.ts (4)
validatePhoneNumber(43-75)generateOTP(157-159)sendTermiiOTP(80-123)sendTwilioOTP(128-152)app/lib/supabase.ts (1)
supabaseAdmin(11-20)
app/api/kyc/smile-id/route.ts (3)
app/lib/rate-limit.ts (1)
rateLimit(10-30)app/lib/smileID.ts (2)
submitSmileIDJob(22-85)SmileIDIdInfo(5-12)app/lib/supabase.ts (1)
supabaseAdmin(11-20)
app/api/kyc/status/route.ts (2)
app/api/kyc/transaction-summary/route.ts (1)
GET(5-81)app/lib/supabase.ts (1)
supabaseAdmin(11-20)
🔇 Additional comments (2)
app/api/phone/send-otp/route.ts (1)
1-111: LGTM! Clean implementation of OTP sending with proper phone normalization.The implementation correctly addresses the previous review feedback:
- Uses
e164Format(no spaces) for database storage and Twilio- Uses
digitsOnly(no+prefix) for Termii- Returns
internationalFormat(with spaces) for client displayError handling, rate limiting, and analytics tracking are all properly implemented.
app/components/PhoneVerificationModal.tsx (1)
1-538: Well-implemented phone verification flow with clean state management.The component properly handles the three-step verification process with:
- Comprehensive country selection with search and popular countries prioritization
- Robust OTP input with auto-focus and backspace navigation
- Proper state reset on close and after verification
- Error handling with user-friendly toast notifications
The previous dependency issue in
handlePhoneSubmithas been correctly addressed.
| // Check KYC profile for phone and SmileID verification status | ||
| const { data: kycProfile } = await supabaseAdmin | ||
| .from('user_kyc_profiles') | ||
| .select('verified, phone_number, tier') | ||
| .eq('wallet_address', walletAddress.toLowerCase()) | ||
| .single(); | ||
|
|
||
| const tier: 0 | 1 | 2 = (kycProfile?.tier as 0 | 1 | 2) || 0; | ||
| const phoneNumber = kycProfile?.phone_number || null; | ||
| const phoneVerified = kycProfile?.verified || false; | ||
|
|
||
| // Full KYC (SmileID) is verified if tier is 2 | ||
| const fullKYCVerified = tier >= 2; | ||
|
|
||
| const responseTime = Date.now() - startTime; | ||
| trackApiResponse( '/api/kyc/status', 'GET', 200, responseTime); | ||
|
|
||
| return NextResponse.json({ | ||
| success: true, | ||
| tier, | ||
| isPhoneVerified: phoneVerified, | ||
| phoneNumber, | ||
| isFullyVerified: fullKYCVerified, | ||
| }); |
There was a problem hiding this comment.
Handle Supabase errors instead of silently treating KYC as “unverified”.
The supabaseAdmin.from('user_kyc_profiles')...single() call ignores error, and the code always falls back to tier = 0 and success: true. A DB or query failure will be exposed to clients as “wallet not verified” rather than as a server error, and no analytics are recorded.
Consider explicitly handling errors and tracking them, while still treating the “no row” case as tier 0. For example:
- const { data: kycProfile } = await supabaseAdmin
- .from('user_kyc_profiles')
- .select('verified, phone_number, tier')
- .eq('wallet_address', walletAddress.toLowerCase())
- .single();
+ const { data: kycProfile, error } = await supabaseAdmin
+ .from('user_kyc_profiles')
+ .select('verified, phone_number, tier')
+ .eq('wallet_address', walletAddress)
+ .maybeSingle();
+
+ if (error) {
+ console.error('Error fetching KYC profile:', error);
+ trackApiError(request, '/api/kyc/status', 'GET', error, 500);
+ return NextResponse.json(
+ { success: false, error: 'Failed to fetch KYC status' },
+ { status: 500 },
+ );
+ }This keeps the “no profile yet” case as kycProfile === null (tier 0) but treats real DB errors as 500s and logs them properly.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/api/kyc/status/route.ts around lines 22 to 45, the Supabase query result
is used without checking for error so any DB/query failure is treated as “no
profile” and returns success: true; change the call to capture both data and
error, if error is present (and not just “no rows”) log/track the error and
return a 500 response; keep the existing behavior that data === null means tier
0 and proceed to return success, but ensure real DB errors are handled
explicitly (trackApiResponse with 500 and include error details in
logs/analytics before returning the error response).
…nagement and validation
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
supabase/migrations/create_user_kyc_profiles.sql (1)
38-45: Index strategy is well-aligned with expected query patterns.Indexes cover the primary lookups: tier (verification status), id_number (KYC lookups), user_id (account association), phone_number (phone verification), email_address, verified status, and the GIN index on platform (JSONB array of verification platforms) is appropriate.
Note: The plaintext phone_number index (line 43) was flagged in a prior review as a potential security/privacy concern. This was explicitly declined; no change needed unless risk appetite changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
supabase/migrations/create_user_kyc_profiles.sql(1 hunks)
🔇 Additional comments (1)
supabase/migrations/create_user_kyc_profiles.sql (1)
3-36: Schema design is appropriate for progressive KYC collection.The table structure supports the tiered KYC flow with nullable fields for optional/progressive data collection, sensible defaults (tier=0, verified=false), and sound constraints on provider and tier values.
| create trigger update_user_kyc_profiles_updated_at BEFORE | ||
| update on user_kyc_profiles for EACH row | ||
| execute FUNCTION update_user_kyc_profiles_updated_at (); No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the function definition in the migration file
find . -name "create_user_kyc_profiles.sql" -type f -exec cat {} \;Repository: paycrest/noblocks
Length of output: 2386
🏁 Script executed:
#!/bin/bash
# Search for the function definition across all migration files and code
rg "create or replace function update_user_kyc_profiles_updated_at|create function update_user_kyc_profiles_updated_at" --type sqlRepository: paycrest/noblocks
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Also check for any variant function names
rg "update_user_kyc_profiles_updated_at" --type sqlRepository: paycrest/noblocks
Length of output: 273
The trigger references a function that does not exist in the codebase.
The trigger update_user_kyc_profiles_updated_at calls a function update_user_kyc_profiles_updated_at() that is not defined anywhere. This migration will fail with "function update_user_kyc_profiles_updated_at() does not exist" when executed.
Define the function before the trigger or verify it exists in a separate migration that runs first.
🤖 Prompt for AI Agents
In supabase/migrations/create_user_kyc_profiles.sql around lines 47-49, the
trigger creation references a function update_user_kyc_profiles_updated_at()
that does not exist; add or ensure a prior migration defines a trigger function
with that exact name and a trigger signature returning trigger (for example a
function that sets NEW.updated_at = now() and returns NEW), or move the CREATE
FUNCTION statement above this CREATE TRIGGER so the trigger can reference the
existing function; ensure names and schema match and migration ordering
guarantees the function exists before the trigger is created.
… before marking as verified
…ng OTP processes and updating verification logic
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/pages/TransactionForm.tsx (1)
72-87: Fix critical currency mismatch and type safety in transaction limit checkIn
handleSwap, the transaction limit check has two critical issues:
Currency unit mismatch –
amountReceivedis in the selected payout currency (e.g., GHS, NGN), buttransactionSummary.monthlySpentfrom the backend is accumulated fromamount_sentvalues in the source currency (typically USD). PassingamountReceiveddirectly tocanTransact()compares incompatible units and breaks the limit guard. You must convertamountReceivedto the same currency asmonthlySpentbefore checking limits.Type safety –
getValues()can return strings; explicitly coerce to number:- const formData = getValues(); - const usdAmount = formData.amountReceived || 0; + const { amountReceived } = getValues(); + const usdAmount = + typeof amountReceived === "number" + ? amountReceived + : parseFloat(String(amountReceived ?? "0")) || 0; + + // TODO: Convert payout currency to source currency (USD) before limit checkAdditionally,
TransactionLimitModal.onCloseshould callrefreshStatus()after the user completes KYC upgrades inside the modal, matching the pattern inProfileDrawer. Otherwise, the parent form's limit checks remain stale if the user upgrades their tier.
🧹 Nitpick comments (6)
app/api/aggregator.ts (1)
776-830: Consider adding a strongly-typed payload interface.The function correctly implements analytics tracking (without PII), error handling, and Bearer token authentication. However, the
payloadparameter is typed asany, which bypasses TypeScript's type safety.Define a proper interface in
app/types.tsfor the SmileID payload structure and use it here:// In app/types.ts (example structure - adjust based on actual SmileID requirements) export type SmileIDPayload = { images: { selfie_image: string; id_image?: string; }; partner_params: { user_id: string; job_id: string; job_type: number; }; // ... other required fields };Then update the function signature:
export const submitSmileIDData = async ( - payload: any, + payload: SmileIDPayload, accessToken: string, ): Promise<SmileIDSubmissionResponse> => {app/api/phone/verify-otp/route.ts (3)
87-123: Consider DB-side atomic increment for stricter attempt tracking.The current approach computes
verification.attempts + 1client-side (using the value fetched at line 36) then writes it with a.lt('attempts', MAX_ATTEMPTS)boundary check. While the boundary prevents exceedingMAX_ATTEMPTS, concurrent requests with incorrect OTPs could read the same stale value and overwrite each other, causing some failed attempts to go uncounted (e.g., three concurrent requests all readattempts=1, all writeattempts=2, but the user actually made three attempts).For strict counting in this security-sensitive context, use a Postgres function that increments atomically:
const { data: updated, error: attemptsError } = await supabaseAdmin.rpc( 'increment_otp_attempts', { wallet_addr: walletAddress.toLowerCase(), max_attempts: MAX_ATTEMPTS } );Create the function in your migration:
CREATE OR REPLACE FUNCTION increment_otp_attempts(wallet_addr text, max_attempts int) RETURNS TABLE(attempts int) AS $$ UPDATE user_kyc_profiles SET attempts = user_kyc_profiles.attempts + 1 WHERE wallet_address = wallet_addr AND user_kyc_profiles.attempts < max_attempts RETURNING user_kyc_profiles.attempts; $$ LANGUAGE sql;This ensures every failed attempt is counted, even under concurrency, while retaining your existing boundary logic.
126-134: Optional: Replaceanytype with a specific type forupdateData.The
anyannotation bypasses TypeScript's type checking. Consider using a more specific type or defining an inline object type to maintain type safety:- const updateData: any = { + const updateData: { + verified: boolean; + verified_at: string; + tier?: number; + } = { verified: true, verified_at: new Date().toISOString() };Alternatively, if you have a
UserKycProfiletype, usePartial<Pick<UserKycProfile, 'verified' | 'verified_at' | 'tier'>>.
157-157: Optional: Return formatted phone number for UX consistency.The response returns
phoneNumberas received in the request body, which may vary in format. Thesend-otpendpoint returnsvalidation.internationalFormatfor consistent, readable display (e.g.,"+1 234 567 890"). Consider aligning here:- phoneNumber: phoneNumber + phoneNumber: validation.internationalFormatThis ensures clients always receive a predictable, user-friendly format.
app/hooks/useKYCStatus.ts (1)
21-38: Consider defensive handling for unknown tiers and documenting per-instance state
getCurrentLimitsassumes that anytier > 0has a corresponding entry inKYC_TIERS, andcanTransactrelies on that. If the backend ever introduces a new tier or returns an unexpected value,KYC_TIERS[tier]will beundefinedand.limitswill throw at runtime.You could defensively fall back to
{ monthly: 0 }whenKYC_TIERS[tier]is missing, and/or clamptierto the known set before indexing. Also, since eachuseKYCStatuscall maintains its own React state (only the fetch guards are shared viawindow.__KYC_FETCH_GUARDS__), it’s worth keeping in mind that callers must invoke their ownrefreshStatus()to keep local state in sync after verification flows.Also applies to: 71-87
app/components/TransactionLimitModal.tsx (1)
18-22: WiretransactionAmountinto the UI or remove it for now
TransactionLimitModalaccepts an optionaltransactionAmountand passesonVerified={handlePhoneVerified}, but:
transactionAmountis never used in the component (no copy or calculations reference it).handlePhoneVerifiedignores itsphoneNumberargument and only closes the modals.If the intent is to explain “We blocked a $X transaction because it exceeds your limit,” it would be good to surface
transactionAmountin the copy (e.g., near the header or progress text). Otherwise, consider removing the prop and the unusedphoneNumberparameter fromhandlePhoneVerifiedto keep the API tighter until you actually need it.-interface TransactionLimitModalProps { - isOpen: boolean; - onClose: () => void; - transactionAmount?: number; -} +interface TransactionLimitModalProps { + isOpen: boolean; + onClose: () => void; + transactionAmount?: number; // use in copy once UX is finalized +} ... - const handlePhoneVerified = async (phoneNumber: string) => { + const handlePhoneVerified = async () => { setIsPhoneModalOpen(false); onClose(); };If you do plan to show the blocked amount, now is a good time to thread it through the text.
Also applies to: 24-28, 60-63, 193-200
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/api/aggregator.ts(2 hunks)app/api/phone/send-otp/route.ts(1 hunks)app/api/phone/verify-otp/route.ts(1 hunks)app/components/ProfileDrawer.tsx(1 hunks)app/components/TransactionLimitModal.tsx(1 hunks)app/hooks/useKYCStatus.ts(1 hunks)app/pages/TransactionForm.tsx(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/api/phone/send-otp/route.ts
🧰 Additional context used
🧠 Learnings (3)
📚 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/pages/TransactionForm.tsxapp/components/TransactionLimitModal.tsxapp/api/phone/verify-otp/route.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/pages/TransactionForm.tsxapp/components/TransactionLimitModal.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/pages/TransactionForm.tsxapp/components/TransactionLimitModal.tsx
🧬 Code graph analysis (4)
app/pages/TransactionForm.tsx (2)
app/hooks/useKYCStatus.ts (1)
useKYCStatus(58-191)app/components/TransactionLimitModal.tsx (1)
TransactionLimitModal(24-217)
app/api/aggregator.ts (2)
app/types.ts (1)
SmileIDSubmissionResponse(240-248)app/lib/server-analytics.ts (3)
trackServerEvent(130-175)trackApiResponse(241-258)trackBusinessEvent(362-375)
app/api/phone/verify-otp/route.ts (3)
app/api/phone/send-otp/route.ts (1)
POST(12-124)app/lib/phone-verification.ts (1)
validatePhoneNumber(43-75)app/lib/supabase.ts (1)
supabaseAdmin(11-20)
app/hooks/useKYCStatus.ts (1)
app/api/aggregator.ts (1)
fetchKYCStatus(307-353)
🔇 Additional comments (2)
app/api/aggregator.ts (1)
10-10: LGTM!The import of
SmileIDSubmissionResponseis correctly added to support the newsubmitSmileIDDatafunction and matches the type definition inapp/types.ts.app/api/phone/verify-otp/route.ts (1)
1-169: LGTM—Solid OTP verification implementation with good error handling.The endpoint properly validates inputs, normalizes phone numbers to E.164, splits fetch errors from not-found cases, handles race conditions with a boundary check, preserves higher tiers, and tracks analytics consistently. The improvements from past reviews (error handling on increment, fetch error differentiation) are well-integrated.
… and status refresh
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/pages/TransactionForm.tsx (2)
272-287: EnsureisUserVerifiedis updated after completing KYC insideTransactionLimitModalIn this component,
setIsUserVerified(true)is only invoked in thecheckKycStatuseffect whenfetchKYCStatusreturns"success"(primarily on mount/address change). After wiringbuttonActionto openTransactionLimitModalinstead of this component’sKycModal,TransactionLimitModalno longer receivessetIsUserVerified, so KYC completion inside that modal won’t directly flipisUserVerifiedhere unless the parent updates it from some other source (e.g., a globaluseKYCStatuslistener).That can leave this form thinking the user is unverified during the same session (affecting
useSwapButtonbehavior and theisUserVerifiedgate forRecipientDetailsForm), even though KYC has just succeeded.Please verify that:
- The parent does, in fact, update
isUserVerifiedin response to KYC success (e.g., viauseKYCStatus+refreshStatus), or- You pass a callback (or derive
isUserVerifiedfromtier) such that KYC completion inTransactionLimitModalreliably updates this prop without requiring a full reload.If not, consider plumbing
setIsUserVerifiedintoTransactionLimitModalor derivingisUserVerifieddirectly fromuseKYCStatusto avoid this desync.Also applies to: 861-868, 898-899
78-79: Remove now-unreachableKycModalstate/rendering fromTransactionFormWith
buttonActionnow using() => setIsLimitModalOpen(true)as the KYC/limits entry point, there’s no remaining code path in this component that setsisKycModalOpentotrue. TheKycModalinside theAnimatePresenceblock therefore appears unreachable fromTransactionFormitself, whileTransactionLimitModalis intended to orchestrate the KYC flow.To reduce confusion and keep the modal graph focused, consider removing:
isKycModalOpenstate, and- The
<AnimatedModal><KycModal … /></AnimatedModal>block here,and letting
TransactionLimitModalown the embedded KYC experience end‑to‑end.Also applies to: 847-858, 898-899
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/components/ProfileDrawer.tsx(1 hunks)app/pages/TransactionForm.tsx(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/components/ProfileDrawer.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 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/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/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/pages/TransactionForm.tsx
🔇 Additional comments (1)
app/pages/TransactionForm.tsx (1)
72-86: Clarify canTransact input units and TransactionLimitModaltransactionAmountsemantics
handleSwaplabelsamountReceivedasusdAmountwhen passing it tocanTransact, but in this formamountReceivedrepresents the payout amount in the selectedcurrency, not necessarily USD. If the backend expects USD-equivalent values for limit checking, this will incorrectly enforce (or fail to enforce) limits for non-USD payouts and propagate a misaligned amount intoblockedTransactionAmount/TransactionLimitModal.transactionAmount.Verify:
- Whether
canTransactand backend limits expect raw USD or the local currency amount- Whether
amountReceivedis normalized to USD before callingcanTransact- Whether
TransactionLimitModalassumestransactionAmountis in USD or just displays the raw valueIf limits are USD-based, explicitly convert to USD (or pass both amount and currency to the API) rather than relying on an implicit assumption that
amountReceivedis already normalized.
…ic across components
…nsactional messages and link to privacy policy
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (3)
app/lib/smileID.ts (1)
84-91: Avoid logging potentially sensitive SmileID response data.The error handler logs
error.response?.datadirectly, which per SmileID docs may contain job metadata, presigned URLs with tokens, or reference IDs that should be treated as sensitive. This was flagged in a prior review.Restrict logging to high-level fields only:
} catch (error: any) { console.error('SmileID API Error:', { status: error.response?.status, statusText: error.response?.statusText, - data: error.response?.data, message: error.message, }); throw error; }app/components/TransactionLimitModal.tsx (1)
60-63: Missing status refresh after phone verification.
handlePhoneVerifiedcloses both modals but doesn't callrefreshStatus(). After successful phone verification, the KYC tier should update from 0 to 1, but without a refresh, the parent component may show stale tier information.const handlePhoneVerified = async (phoneNumber: string) => { + await refreshStatus(); setIsPhoneModalOpen(false); onClose(); };app/components/ProfileDrawer.tsx (1)
302-350: KYC tier card & upgrade CTA logic looks correct and aligns with backend semanticsThe “Current Tier Status” and “Increase limit” CTA conditions now line up with the documented behavior:
tier >= 1gates the current tier card.- The upgrade CTA is shown only for Tier 1 (
tier < 2 && !isFullyVerified), so Tier 2 users don’t see an invalid “upgrade” to a non‑existent tier.- The next tiers list is restricted to
tierData.level > tier, so only higher tiers are shown.This matches the tiered KYC flow (Tier 1 = phone‑verified, Tier 2 = full KYC) and avoids the undefined next‑tier issue from the earlier version.
Also applies to: 354-356
🧹 Nitpick comments (5)
app/lib/smileID.ts (2)
29-34: Consider adding types forimagesandpartner_params.Both parameters are typed as
any, which reduces type safety. When feasible, introduce minimal interfaces (e.g.,SmileIdPartnerParams,SmileIdImage) to catch shape mismatches at compile time.
62-64: Minor collision risk in job_id generation.The job ID uses
Date.now()and the first 8 characters of the wallet address. If a user submits multiple jobs within the same millisecond, IDs could collide. Consider adding a random suffix or using a UUID library if uniqueness is critical.+ import { randomBytes } from 'crypto'; // Generate unique IDs const timestamp = Date.now(); - const job_id = `job-${timestamp}-${walletAddress.slice(0, 8)}`; + const job_id = `job-${timestamp}-${walletAddress.slice(0, 8)}-${randomBytes(4).toString('hex')}`;app/components/PhoneVerificationModal.tsx (1)
501-513: State reset logic is duplicated fromhandleClose.The verified success callback duplicates the state reset logic from
handleClose(lines 233-239). Extract to a shared helper to reduce maintenance burden.+ const resetState = () => { + setStep(STEPS.ENTER_PHONE); + setPhoneNumber(''); + setFormattedPhone(''); + setOtpCode(''); + setAttemptsRemaining(3); + setIsCountryDropdownOpen(false); + setCountrySearch(''); + }; + const handleClose = () => { onClose(); - setStep(STEPS.ENTER_PHONE); - setPhoneNumber(''); - // ... rest of resets + resetState(); };app/components/ProfileDrawer.tsx (2)
132-187: Tier section UX: tighten icon logic and arrow stateThe tier accordion works, but there are a couple of polish opportunities:
- Arrow icon rotation – the
classNameconditional currently doesn’t change anything:className={classNames( "size-4 ... transition-transform", isExpanded ? "" : "" )}You likely meant to rotate when expanded:
className={classNames( "size-4 ... transition-transform", isExpanded ? "rotate-180" : "rotate-0" )}
- Requirement → icon mapping – the string‑based checks (
includes('number'),includes('verification'),includes('ID')) are brittle; copy changes will silently break icon selection. If possible, makerequirementsstructured (e.g.{ label, type: 'phone' | 'liveness' | 'id' }) and maptypeto icons instead of parsing the text.Not strictly blocking, but it will make this section more maintainable as copy evolves.
216-236: Dialog & refresh behavior: generally solid, only minor resilience nitsOverall the modal wiring looks good:
- Headless UI
Dialogis controlled viaopen={isOpen}andonClose={onClose}and wrapped inAnimatePresence.- Close button and backdrop both call
onClose, which is expected.TransactionLimitModaltriggersrefreshStatus()on close, ensuring the drawer reflects updated limits.Two small resilience improvements to consider:
Handle
refreshStatuserrors – in both theuseEffectandonClosecallback you ignore failures. Adding a.catch(() => toast.error("Unable to refresh verification status"))(or at leastconsole.error) would make issues more visible.Optional: use
Dialog.Titlefor accessibility – wrapping the “Profile” heading with<Dialog.Title>would improve screen reader labeling:<Dialog.Title as="h2" className="text-lg font-semibold ..."> Profile </Dialog.Title>Neither is blocking, but they’ll make this flow more robust and accessible.
Also applies to: 239-251, 367-373
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
app/components/PhoneVerificationModal.tsx(1 hunks)app/components/ProfileDrawer.tsx(1 hunks)app/components/TransactionLimitModal.tsx(1 hunks)app/hooks/useKYCStatus.ts(1 hunks)app/hooks/useSwapButton.ts(2 hunks)app/lib/smileID.ts(1 hunks)app/pages/TransactionForm.tsx(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/hooks/useKYCStatus.ts
🧰 Additional context used
🧠 Learnings (3)
📚 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/pages/TransactionForm.tsxapp/components/TransactionLimitModal.tsxapp/components/ProfileDrawer.tsxapp/components/PhoneVerificationModal.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/pages/TransactionForm.tsxapp/components/TransactionLimitModal.tsxapp/components/ProfileDrawer.tsxapp/components/PhoneVerificationModal.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/pages/TransactionForm.tsxapp/components/TransactionLimitModal.tsxapp/components/PhoneVerificationModal.tsx
🧬 Code graph analysis (1)
app/components/PhoneVerificationModal.tsx (2)
app/lib/countries.ts (4)
Country(1-6)fetchCountries(15-60)searchCountries(95-104)getPopularCountries(88-90)app/components/index.ts (5)
fadeInOut(21-21)primaryBtnClasses(47-47)AnimatedComponent(15-15)slideInOut(19-19)secondaryBtnClasses(48-48)
🔇 Additional comments (7)
app/hooks/useSwapButton.ts (1)
101-118: LGTM! Clean refactor to integrate the new limit modal.The logic correctly gates the limit modal on
!hasInsufficientBalance, ensuring users with insufficient funds are directed to fund their wallet first rather than being shown the verification flow. The parameter rename fromsetIsKycModalOpentosetIsLimitModalOpenaligns with the new modal naming convention.app/components/PhoneVerificationModal.tsx (2)
400-401: Phone masking regex may not handle all formats correctly.The regex
formattedPhone.replace(/(\+\d+\s+\d{3})[\s\d]+(\d{2})/, '$1**$2')assumes a specific format with spaces. IfformattedPhoneuses different separators (e.g., dashes, no separators), the masking won't work as intended.
203-228:handleResendOtpomits thenamefield present in initial OTP request.
handlePhoneSubmitsends{ phoneNumber, walletAddress, name }, buthandleResendOtponly sends{ phoneNumber, walletAddress }. If the backend usesnamefor OTP metadata or logging, this inconsistency could cause issues. Verify whether the backend requiresnameon resend.#!/bin/bash # Check if send-otp route expects name field on all requests rg -nA 20 'export.*POST' app/api/phone/send-otp/route.ts | head -40app/pages/TransactionForm.tsx (2)
271-282: LGTM! KYC status refresh on wallet changes.The effect correctly triggers
refreshStatus()when the wallet context changes, ensuring KYC state stays synchronized.
839-846: LGTM! Modal closes with status refresh.The
onClosehandler properly refreshes KYC status after the modal closes, addressing the stale state concern from the prior review.app/components/TransactionLimitModal.tsx (2)
112-126: LGTM! Progress bar guards against division by zero.The progress bar width calculation properly handles the case when
currentLimits.monthlyis 0, avoiding NaN/Infinity.
42-47:refreshStatusis properly memoized and stable; no infinite loop concern.
refreshStatusis correctly wrapped withuseCallbackin theuseKYCStatushook with appropriate dependencies. ThefetchGuardsglobal object pattern ensures stable callback identities across renders, preventing infinite effect loops in TransactionLimitModal.
| By clicking "Verify and start", you consent to recieving transactional text messages for notifications and alerts from Noblocks. Reply STOP to opt out. you agree to our <a href="https://paycrest.io/privacy-policy" className='text-lavender-600'>Privacy Policy and terms & conditions.</a>. | ||
| </p> |
There was a problem hiding this comment.
Typo: "recieving" should be "receiving".
User-facing text has a spelling error.
-By clicking "Verify and start", you consent to recieving transactional text messages for notifications and alerts from Noblocks.
+By clicking "Verify and start", you consent to receiving transactional text messages for notifications and alerts from Noblocks.📝 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.
| By clicking "Verify and start", you consent to recieving transactional text messages for notifications and alerts from Noblocks. Reply STOP to opt out. you agree to our <a href="https://paycrest.io/privacy-policy" className='text-lavender-600'>Privacy Policy and terms & conditions.</a>. | |
| </p> | |
| By clicking "Verify and start", you consent to receiving transactional text messages for notifications and alerts from Noblocks. Reply STOP to opt out. you agree to our <a href="https://paycrest.io/privacy-policy" className='text-lavender-600'>Privacy Policy and terms & conditions.</a>. | |
| </p> |
🤖 Prompt for AI Agents
In app/components/PhoneVerificationModal.tsx around lines 377 to 378, the
user-facing string contains a spelling error: change "recieving" to "receiving"
in the paragraph text; update the JSX string accordingly (ensure the rest of the
sentence and anchor remain unchanged and run a quick lint/format check to
confirm no formatting issues).
| const currentLimits = getCurrentLimits(); | ||
| const monthlyProgress = (transactionSummary.monthlySpent / currentLimits.monthly) * 100; | ||
|
|
There was a problem hiding this comment.
Guard against zero / missing monthly limit in progress calculation
monthlyProgress divides by currentLimits.monthly without guarding against 0 or undefined. If an unverified tier (or misconfig) sets a monthly limit of 0, you’ll get NaN (0/0) and end up with style={{ width: 'NaN%' }}, which is invalid CSS.
Consider a defensive clamp:
const monthlyLimit = currentLimits.monthly || 0;
const monthlyProgress =
monthlyLimit > 0
? (transactionSummary.monthlySpent / monthlyLimit) * 100
: 0;This keeps the bar stable even if limits are missing or zeroed.
🤖 Prompt for AI Agents
In app/components/ProfileDrawer.tsx around lines 43-45, the calculation for
monthlyProgress divides by currentLimits.monthly without guarding against zero
or undefined which can produce NaN and invalid CSS; fix by reading the monthly
limit into a local variable (defaulting to 0), then compute monthlyProgress only
if that limit is > 0, otherwise set progress to 0 (and optionally clamp the
computed percent to the 0-100 range) so the resulting style width is always a
valid percentage.
| const { linkEmail } = useLinkAccount({ | ||
| onSuccess: ({ user }) => { | ||
| toast.success(`${user.email} linked successfully`); | ||
| }, | ||
| onError: () => { | ||
| toast.error("Error linking account", { | ||
| description: "You might have this email linked already", | ||
| }); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "ProfileDrawer.tsx" --exec cat -n {}Repository: paycrest/noblocks
Length of output: 24307
🏁 Script executed:
rg -t ts -t tsx "useLinkAccount" --type-list | head -20Repository: paycrest/noblocks
Length of output: 450
🏁 Script executed:
rg -t ts -t tsx "interface.*User|type.*User" --max-count=10 -A 5Repository: paycrest/noblocks
Length of output: 88
🏁 Script executed:
# Look for Privy type definitions or where useLinkAccount is defined
rg "from.*privy" -A 5 -B 5Repository: paycrest/noblocks
Length of output: 23200
🏁 Script executed:
rg "useLinkAccount" -A 10 --max-count=20Repository: paycrest/noblocks
Length of output: 3053
Fix email success toast to use user.email.address instead of user.email
In the account card you render the email as user.email.address (line 267), but in the onSuccess callback you interpolate user.email directly (line 63):
onSuccess: ({ user }) => {
toast.success(`${user.email} linked successfully`);
}Since user.email is an object, this will produce "[object Object] linked successfully". Use the same field as the UI:
onSuccess: ({ user }) => {
toast.success(`${user.email?.address} linked successfully`);
}This also applies to lines 264-268 where the email is displayed in the account card.
🤖 Prompt for AI Agents
In app/components/ProfileDrawer.tsx around lines 61 to 70 (and related UI at
lines 264 to 268), the success toast currently interpolates user.email which is
an object and yields "[object Object]"; change the onSuccess toast to use
user.email?.address (or a safe accessor) so it shows the actual email string,
and likewise update the account card rendering at lines 264-268 to render
user.email?.address with optional chaining/guarding to avoid runtime errors when
email is undefined.
| const handleCopyAddress = () => { | ||
| if (walletAddress) { | ||
| navigator.clipboard.writeText(walletAddress); | ||
| setIsAddressCopied(true); | ||
| toast.success("Address copied to clipboard"); | ||
| setTimeout(() => setIsAddressCopied(false), 2000); | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
Improve wallet address UX: hide when missing and handle copy errors
Right now the wallet row always renders and shortenAddress(walletAddress ?? "", 10) is called even when there is no walletAddress, and handleCopyAddress ignores clipboard errors:
- Empty/undefined addresses will still render some placeholder from
shortenAddress, which is confusing. navigator.clipboard.writeTextis not available or may reject in some environments without any feedback.
Consider:
const hasWallet = Boolean(walletAddress);
{hasWallet && (
<div className="flex items-center justify-between w-full">
<p className="text-sm font-light ...">
{shortenAddress(walletAddress!, 10)}
</p>
<button ... onClick={handleCopyAddress}>...</button>
</div>
)}And in handleCopyAddress:
if (walletAddress && navigator.clipboard?.writeText) {
navigator.clipboard.writeText(walletAddress)
.then(() => {
setIsAddressCopied(true);
toast.success("Address copied to clipboard");
setTimeout(() => setIsAddressCopied(false), 2000);
})
.catch(() => {
toast.error("Unable to copy address");
});
}This avoids odd UI when no wallet is present and makes copy behavior more robust.
Also applies to: 279-296
🤖 Prompt for AI Agents
In app/components/ProfileDrawer.tsx around lines 72-80 (and also apply the same
fix at 279-296): the wallet row currently renders even when walletAddress is
empty and calls shortenAddress on a missing value, and handleCopyAddress calls
navigator.clipboard.writeText without feature detection or error handling. Wrap
the wallet UI in a hasWallet conditional (e.g., Boolean(walletAddress)) so
shortenAddress is only called when walletAddress exists; update
handleCopyAddress to check walletAddress and navigator.clipboard?.writeText, use
the returned promise to set isAddressCopied and show success toast on resolve,
and catch errors to show an error toast (and do not call setIsAddressCopied on
failure). Ensure any timeout to clear isAddressCopied still runs only after a
successful copy.
| useEffect(() => { | ||
| if (isOpen && !isLoading && ((tier < 1 && !phoneNumber) || tier === undefined)) { | ||
| setIsPhoneModalOpen(true); | ||
| } | ||
| }, [isOpen, isLoading, tier]); |
There was a problem hiding this comment.
Missing phoneNumber in dependency array.
The condition references phoneNumber but the dependency array only includes [isOpen, isLoading, tier]. If phoneNumber changes after initial load, the auto-open logic won't re-evaluate.
useEffect(() => {
if (isOpen && !isLoading && ((tier < 1 && !phoneNumber) || tier === undefined)) {
setIsPhoneModalOpen(true);
}
- }, [isOpen, isLoading, tier]);
+ }, [isOpen, isLoading, tier, phoneNumber]);🤖 Prompt for AI Agents
In app/components/TransactionLimitModal.tsx around lines 50 to 54, the useEffect
condition reads phoneNumber but the dependency array omits it, so changes to
phoneNumber won't re-evaluate the auto-open logic; add phoneNumber to the
dependency array (e.g. [isOpen, isLoading, tier, phoneNumber]) so the effect
runs when phoneNumber changes and ensure the effect logic still behaves
correctly with the added dependency.
| <KycModal | ||
| setIsKycModalOpen={setIsKycModalOpen} | ||
| setIsUserVerified={() => { }} | ||
| /> |
There was a problem hiding this comment.
setIsUserVerified is a no-op—KYC completion has no effect.
KycModal receives setIsUserVerified={() => {}}, so successful KYC verification won't update any state. This breaks the upgrade flow: after completing full KYC, the user's verified status won't be reflected.
Consider passing a meaningful callback that refreshes status and closes the modal:
<KycModal
setIsKycModalOpen={setIsKycModalOpen}
- setIsUserVerified={() => { }}
+ setIsUserVerified={async () => {
+ await refreshStatus();
+ setIsKycModalOpen(false);
+ onClose();
+ }}
/>📝 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.
| <KycModal | |
| setIsKycModalOpen={setIsKycModalOpen} | |
| setIsUserVerified={() => { }} | |
| /> | |
| <KycModal | |
| setIsKycModalOpen={setIsKycModalOpen} | |
| setIsUserVerified={async () => { | |
| await refreshStatus(); | |
| setIsKycModalOpen(false); | |
| onClose(); | |
| }} | |
| /> |
|
|
||
| // Calculate the USD amount for transaction limit checking | ||
| const formData = getValues(); | ||
| const usdAmount = formData.amountReceived || 0; | ||
|
|
||
| // Check transaction limits based on KYC tier | ||
| const limitCheck = canTransact(usdAmount); | ||
|
|
||
| if (!limitCheck.allowed) { | ||
| setBlockedTransactionAmount(usdAmount); | ||
| setIsLimitModalOpen(true); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the canTransact function implementation and what it expects
rg -nA 15 'canTransact' app/hooks/useKYCStatus.tsRepository: paycrest/noblocks
Length of output: 1268
🏁 Script executed:
#!/bin/bash
# Check the form field definitions to understand what amountReceived and amountSent represent
rg -nB 5 -A 5 'amountReceived\|amountSent' app/pages/TransactionForm.tsx | head -100Repository: paycrest/noblocks
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check how tier limits are defined
rg -nA 10 'tier.*limit\|transactionLimit' app/hooks/useKYCStatus.tsRepository: paycrest/noblocks
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check getRemainingLimits implementation
rg -nB 5 -A 20 'getRemainingLimits' app/hooks/useKYCStatus.tsRepository: paycrest/noblocks
Length of output: 3049
🏁 Script executed:
#!/bin/bash
# Check transaction limits and tier definitions
rg -nB 2 -A 10 'TransactionLimits\|TIER.*LIMIT\|tier.*Limit' app/hooks/useKYCStatus.tsRepository: paycrest/noblocks
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check the form schema or validation to understand field definitions
fd -t f 'TransactionForm' app/pages/ --exec grep -l 'amountReceived\|amountSent' {} \;Repository: paycrest/noblocks
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Look at the actual form field setup more directly
sed -n '1,150p' app/pages/TransactionForm.tsx | grep -n 'amountReceived\|amountSent\|useForm'Repository: paycrest/noblocks
Length of output: 237
🏁 Script executed:
#!/bin/bash
# Get the full getCurrentLimits implementation
rg -nB 5 -A 15 'getCurrentLimits.*useCallback' app/hooks/useKYCStatus.tsRepository: paycrest/noblocks
Length of output: 745
🏁 Script executed:
#!/bin/bash
# Search for tier limit constants or definitions
rg -n 'TIER_LIMITS\|tier.*limit\|500.*1000.*5000' app/hooks/useKYCStatus.tsRepository: paycrest/noblocks
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check the form schema to understand amountReceived vs amountSent context
rg -nB 10 -A 3 'amountReceived.*:.*number\|amountSent.*:.*number' app/pages/TransactionForm.tsxRepository: paycrest/noblocks
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check the backend API to understand what monthlySpent represents
find app -type f -name '*.ts' -o -name '*.tsx' | xargs grep -l 'transaction-summary' | head -5Repository: paycrest/noblocks
Length of output: 127
🏁 Script executed:
#!/bin/bash
# Find KYC_TIERS definition
rg -n 'KYC_TIERS\s*=' app/hooks/useKYCStatus.tsRepository: paycrest/noblocks
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Search for KYC_TIERS in the entire codebase
rg -n 'KYC_TIERS' app/Repository: paycrest/noblocks
Length of output: 1224
🏁 Script executed:
#!/bin/bash
# Check the transaction-summary API route
cat -n app/api/kyc/transaction-summary/route.ts | head -80Repository: paycrest/noblocks
Length of output: 3218
🏁 Script executed:
#!/bin/bash
# Get the full KYC_TIERS definition
sed -n '21,50p' app/hooks/useKYCStatus.tsRepository: paycrest/noblocks
Length of output: 634
🏁 Script executed:
#!/bin/bash
# Check form field definitions and labels for amountSent and amountReceived
rg -nB 5 -A 5 'name.*amountSent\|name.*amountReceived' app/pages/TransactionForm.tsx | head -60Repository: paycrest/noblocks
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Look at the transaction form to understand field purposes
sed -n '440,460p' app/pages/TransactionForm.tsxRepository: paycrest/noblocks
Length of output: 692
Use amountSent instead of amountReceived for transaction limit checking—tier limits are in USD, not local currency.
The KYC tier limits (Tier 1: $100, Tier 2: $15,000) and monthlySpent tracking from the backend are both in stablecoin/USD. However, formData.amountReceived is the local currency amount (NGN, KES, etc.) that the recipient gets, not the USD amount being sent. Using it for canTransact() causes incorrect limit checks.
Change line 443 from:
- const usdAmount = formData.amountReceived || 0;
+ const usdAmount = formData.amountSent || 0;amountSent is the stablecoin amount and matches the currency of the tier limits and monthlySpent tracking.
📝 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.
| // Calculate the USD amount for transaction limit checking | |
| const formData = getValues(); | |
| const usdAmount = formData.amountReceived || 0; | |
| // Check transaction limits based on KYC tier | |
| const limitCheck = canTransact(usdAmount); | |
| if (!limitCheck.allowed) { | |
| setBlockedTransactionAmount(usdAmount); | |
| setIsLimitModalOpen(true); | |
| return; | |
| } | |
| // Calculate the USD amount for transaction limit checking | |
| const formData = getValues(); | |
| const usdAmount = formData.amountSent || 0; | |
| // Check transaction limits based on KYC tier | |
| const limitCheck = canTransact(usdAmount); | |
| if (!limitCheck.allowed) { | |
| setBlockedTransactionAmount(usdAmount); | |
| setIsLimitModalOpen(true); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In app/pages/TransactionForm.tsx around lines 440 to 452, the code uses
formData.amountReceived (local currency) to check KYC USD tier limits, causing
incorrect limit checks; replace usage with formData.amountSent (the
stablecoin/USD amount) by reading const formData = getValues(); const usdAmount
= formData.amountSent || 0; then call canTransact(usdAmount) and
setBlockedTransactionAmount(usdAmount) so the limit check and blocked amount
align with backend monthlySpent and tier USD values.
Description
This pull request introduces a comprehensive KYC (Know Your Customer) and phone verification flow, including new API endpoints for phone OTP (One-Time Password) verification, SmileID biometric verification, and KYC status/transaction summaries. It also updates the frontend modal to support biometric capture and phone verification, and adds configuration for third-party phone verification providers.
New KYC and Phone Verification APIs:
Frontend KYC Modal Enhancements:
KycModalto support a new biometric capture step using SmileID web components, including dynamic import and state management for camera and signature data. [1] [2] [3]Third-Party Provider Configuration:
.env.example, preparing the app for international and African phone number support.Aggregator API Utilities:
submitSmileIDDatautility for submitting biometric KYC data to the backend, with analytics and error tracking.New API Endpoints for KYC and Phone Verification
app/api/phone/send-otp/route.ts: Endpoint to send OTP via Termii or Twilio, validate phone number, store OTP and metadata, and handle analytics.app/api/phone/verify-otp/route.ts: Endpoint to verify OTP, enforce max attempts, mark phone as verified, and update user tier.app/api/phone/status/route.ts: Endpoint to check phone verification status for a given wallet address.app/api/kyc/smile-id/route.ts: Endpoint to submit SmileID biometric data, validate required fields, update KYC profile, and return verification result.app/api/kyc/status/route.ts: Endpoint to check overall KYC status (phone and SmileID) and determine verification tier.app/api/kyc/transaction-summary/route.ts: Endpoint to summarize user's daily/monthly transaction volume and last transaction date.Frontend KYC Modal and Aggregator Integration
app/components/KycModal.tsx: Added biometric capture step, SmileID web component integration, and state for signature/camera. Updated to use new aggregator API methods. [1] [2] [3]app/api/aggregator.ts: AddedsubmitSmileIDDatautility for submitting biometric data and tracking analytics.Configuration for Third-Party Providers
.env.example: Added Termii and Twilio API key placeholders for phone verification services.References
closes #281
closes #282
closes #283
Testing
Here's a loom video recording of what this looks like.
Loom recording of the kyc flow
Checklist
mainBy submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.
Summary by CodeRabbit
New Features
Infrastructure
✏️ Tip: You can customize this high-level summary in your review settings.