feat: add Smile ID KYC submission functionality and integrate smart camera component in KycModal#286
Conversation
…amera component in KycModal
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds end-to-end tiered KYC and phone verification: new server routes (OTP, SMS/Twilio, SmileID, tier3 Dojah), DB migrations and schema, client KYC context and UI (phone modal, KYC modal, profile/limits), helper libs (SmileID, Dojah, phone validation, countries), middleware and package deps. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant App as Client App
participant OTPRoute as /api/phone/send-otp
participant SMSProvider as SMS Service<br/>KudiSMS / Twilio
participant DB as Supabase<br/>user_kyc_profiles
User->>App: Submit phoneNumber
App->>OTPRoute: POST /api/phone/send-otp
activate OTPRoute
OTPRoute->>OTPRoute: Validate & normalize phone (libphonenumber-js)
OTPRoute->>OTPRoute: Choose provider (Nigeria → KudiSMS, else Twilio)
OTPRoute->>DB: Upsert otp_code, expires_at, provider
OTPRoute->>SMSProvider: Send OTP
SMSProvider-->>OTPRoute: Acknowledgement
OTPRoute-->>App: success (provider, intlPhone)
deactivate OTPRoute
User->>App: Submit OTP
App->>/api/phone/verify-otp: POST
activate OTPRoute
OTPRoute->>DB: Fetch profile by wallet + phone
alt provider == twilio
OTPRoute->>SMSProvider: Verify via Twilio Verify
else
OTPRoute->>OTPRoute: Compare stored otp_code, check expiry, attempts
end
OTPRoute->>DB: Update verified, verified_at, set tier=1
OTPRoute-->>App: verified: true
deactivate OTPRoute
sequenceDiagram
actor User
participant App as Client App
participant Camera as smart-camera-web
participant Aggregator as app/api/aggregator.ts
participant KYCRoute as /api/kyc/smile-id
participant SmileIDAPI as SmileID Web API
participant DB as Supabase<br/>user_kyc_profiles
User->>App: Open KYC modal, select id_type/country
App->>Camera: Initialize SmileID capture
User->>Camera: Capture ID + selfie
Camera-->>App: publish { images, id_info }
App->>Aggregator: submitSmileIDData(payload, accessToken)
activate Aggregator
Aggregator->>KYCRoute: POST /api/kyc/smile-id
activate KYCRoute
KYCRoute->>SmileIDAPI: submitSmileIDJob(images, id_info, partner_params)
SmileIDAPI-->>KYCRoute: { jobId, jobStatus, ... }
alt verification success
KYCRoute->>DB: Upsert profile (verified=true/tier=2, id fields, job ref)
KYCRoute-->>Aggregator: success response
else failure
KYCRoute-->>Aggregator: error response
end
Aggregator-->>App: SmileIDSubmissionResponse
deactivate KYCRoute
deactivate Aggregator
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- 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.
…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.
… 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.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@app/components/PhoneVerificationModal.tsx`:
- Around line 136-167: In handlePhoneSubmit add a guard to require a non-empty
full name: check name.trim() alongside phoneNumber and walletAddress and show a
toast error like "Please enter your full name" and return if missing; also
update the UI submit control (the submit button handler/state referenced near
lines 432-436) to be disabled when name.trim() is falsy so users cannot send OTP
without providing a full name. Ensure you reference the same
selectedCountry.code/fullPhoneNumber logic and keep setIsLoading behavior
unchanged.
In `@app/lib/phone-verification.ts`:
- Around line 1-2: The OTP generation currently uses Math.random which is not
cryptographically secure; replace that logic by using Node's crypto.randomInt to
produce the required numeric OTP entropy, import crypto, and generate the digits
(e.g., loop crypto.randomInt(0,10) per digit or call crypto.randomInt(0,
10**length) and pad) instead of Math.random; update the function that constructs
the OTP (the code site using Math.random) to use crypto.randomInt and ensure
output length/padding remains the same and tests still pass.
In `@app/pages/TransactionForm.tsx`:
- Around line 71-72: The gating currently uses isFullyVerified (tier ≥2) which
blocks Tier 1 users; update the checks in TransactionForm to gate on canTransact
instead of isFullyVerified so phone‑verified users can proceed to the recipient
form and submit swaps. Locate the usages of isFullyVerified inside the
TransactionForm component (the checks around the recipient form and submit flow)
and replace them with canTransact, making sure any UI/disabled state, early
returns, or error messages that referenced isFullyVerified are adjusted to
reflect the canTransact semantics and still call refreshStatus where
appropriate.
In `@supabase/migrations/create_user_kyc_profiles.sql`:
- Around line 3-45: Add a PL/pgSQL trigger function named
update_user_kyc_profiles_updated_at() before the trigger definition so the
trigger can call it; implement it to set NEW.updated_at = now() and return NEW
(matching the pattern used in create_saved_recipients.sql), then keep the
existing trigger that calls update_user_kyc_profiles_updated_at(); ensure the
function signature is "create or replace function
public.update_user_kyc_profiles_updated_at() returns trigger language plpgsql"
so the trigger can find it.
♻️ Duplicate comments (5)
supabase/migrations/create_user_kyc_profiles.sql (1)
62-64: The trigger references a function that is not defined in this migration.The trigger calls
update_user_kyc_profiles_updated_at()but this function is not created anywhere in the file. The migration will fail with:ERROR: function update_user_kyc_profiles_updated_at() does not existAdd the function definition before the trigger:
Proposed fix
create index IF not exists idx_user_kyc_profiles_verified on public.user_kyc_profiles using btree (verified) TABLESPACE pg_default; +-- Function to automatically update updated_at timestamp +create or replace function update_user_kyc_profiles_updated_at() +returns trigger as $$ +begin + new.updated_at = now(); + return new; +end; +$$ language plpgsql; + 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 ();.env.example (1)
98-123: Standardize KudiSMS/SmileID env formatting for consistencyThe new blocks still trigger dotenv‑linter warnings (ordering, quoting, spacing). It’s cosmetic, but consistent formatting avoids subtle parsing differences across tools. This overlaps with earlier feedback, so tagging as duplicate.
♻️ Suggested tidy-up (formatting + ordering)
# KudiSMS (for African phone numbers) # Get from: KudiSMS Dashboard → Settings → API Keys -KUDISMS_API_KEY=your_kudisms_api_key -KUDISMS_APP_NAME_CODE=your_app_name_code -KUDISMS_TEMPLATE_CODE=your_template_code -KUDISMS_SENDER_ID=Noblocks +KUDISMS_API_KEY="your_kudisms_api_key" +KUDISMS_APP_NAME_CODE="your_app_name_code" +KUDISMS_SENDER_ID="Noblocks" +KUDISMS_TEMPLATE_CODE="your_template_code" # ============================================================================= # SmileID KYC Verification Services # ============================================================================= -SMILE_IDENTITY_BASE_URL=XXXXXX `#get` from docs -SMILE_IDENTITY_API_KEY=your_api_key_here -SMILE_IDENTITY_PARTNER_ID=your_partner_id_here -SMILE_ID_CALLBACK_URL= `#optional` callback url -SMILE_IDENTITY_SERVER=0 # 0 for sandbox, 1 for production +SMILE_IDENTITY_API_KEY="your_api_key_here" +SMILE_IDENTITY_PARTNER_ID="your_partner_id_here" +SMILE_IDENTITY_BASE_URL="https://sandbox.smileid.io" # or https://api.smileid.io for production +SMILE_IDENTITY_SERVER="0" # 0 for sandbox, 1 for production +SMILE_ID_CALLBACK_URL="" # optional callback URLapp/components/ProfileDrawer.tsx (1)
80-87: Guard clipboard copy and hide wallet row when address is missingCurrently
shortenAddressruns on empty strings andnavigator.clipboard.writeTextisn’t guarded. This is a small UX/robustness improvement; similar to earlier feedback.♻️ Suggested guardrails
const walletAddress = user?.linkedAccounts.find( (account) => account.type === "smart_wallet", )?.address; + const hasWallet = Boolean(walletAddress); @@ - const handleCopyAddress = () => { - if (walletAddress) { - navigator.clipboard.writeText(walletAddress); - setIsAddressCopied(true); - toast.success("Address copied to clipboard"); - setTimeout(() => setIsAddressCopied(false), 2000); - } - }; + const handleCopyAddress = async () => { + if (!walletAddress || !navigator.clipboard?.writeText) { + toast.error("Unable to copy address"); + return; + } + try { + await navigator.clipboard.writeText(walletAddress); + setIsAddressCopied(true); + toast.success("Address copied to clipboard"); + setTimeout(() => setIsAddressCopied(false), 2000); + } catch { + toast.error("Unable to copy address"); + } + }; @@ - {/* Wallet Address */} - <div className="flex w-full items-center justify-between"> - <p className="text-sm font-light text-text-body dark:text-white/70"> - {shortenAddress(walletAddress ?? "", 10)} - </p> - <button - type="button" - onClick={handleCopyAddress} - title="Copy wallet address" - className="rounded-lg p-2 transition-colors hover:bg-accent-gray dark:hover:bg-white/10" - > - {isAddressCopied ? ( - <PiCheck className="size-4 text-green-500" /> - ) : ( - <Copy01Icon className="size-4 text-outline-gray dark:text-white/50" /> - )} - </button> - </div> + {/* Wallet Address */} + {hasWallet && ( + <div className="flex w-full items-center justify-between"> + <p className="text-sm font-light text-text-body dark:text-white/70"> + {shortenAddress(walletAddress!, 10)} + </p> + <button + type="button" + onClick={handleCopyAddress} + title="Copy wallet address" + className="rounded-lg p-2 transition-colors hover:bg-accent-gray dark:hover:bg-white/10" + > + {isAddressCopied ? ( + <PiCheck className="size-4 text-green-500" /> + ) : ( + <Copy01Icon className="size-4 text-outline-gray dark:text-white/50" /> + )} + </button> + </div> + )}Also applies to: 314-331
app/components/PhoneVerificationModal.tsx (1)
419-421: Fix typo in consent text (“recieving”)User‑facing typo remains.
✍️ Quick fix
- By clicking "Verify and start", you consent to recieving transactional + By clicking "Verify and start", you consent to receiving transactionalapp/components/TransactionLimitModal.tsx (1)
64-67: Avoid double-callingonCloseafter phone verificationPhoneVerificationModal calls
onVerifiedandonCloseon success. WithhandlePhoneVerifiedalso callingonClose, the parent close runs twice.🧹 Suggested adjustment
- const handlePhoneVerified = async (phoneNumber: string) => { - setIsPhoneModalOpen(false); - onClose(); - }; + const handlePhoneVerified = async (_phoneNumber: string) => { + // Let PhoneVerificationModal's onClose handler close modals + await refreshStatus(); + };Also applies to: 220-226
…TP generation method
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/components/PhoneVerificationModal.tsx`:
- Around line 262-272: The modal close handler handleClose resets phone/OTP
state but doesn't clear the user's full name, risking PII carryover; update
handleClose to also clear the name state (call the appropriate setter, e.g.,
setName or setFullName) and likewise clear the name in the flow's
success/completion handler (e.g., verify success handler or handleSuccess at the
end of the modal flow) so the name is reset both when closing and when the modal
completes successfully.
♻️ Duplicate comments (1)
app/components/PhoneVerificationModal.tsx (1)
423-424: Typo in consent copy (“recieving”).Line 423 should be “receiving”.
✏️ Proposed fix
- By clicking "Verify and start", you consent to recieving transactional + By clicking "Verify and start", you consent to receiving transactional
🧹 Nitpick comments (1)
app/components/PhoneVerificationModal.tsx (1)
60-61: Align provider type with all supported providers.
data.providercan be"termii"(see Line 183), but the state union only allows"kudisms" | "twilio". Widen the union to keep the type aligned with real responses.♻️ Proposed fix
- const [provider, setProvider] = useState<"kudisms" | "twilio">("kudisms"); + const [provider, setProvider] = useState< + "kudisms" | "termii" | "twilio" + >("kudisms");Also applies to: 176-185
…cies - Added new fields: network and order_id to the transactions table. - Updated status check to include 'fulfilling'. - Improved formatting for better readability. - Adjusted RLS policies for clarity and consistency.
…emove unnecessary state ref
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Fix all issues with AI agents
In `@app/api/kyc/transaction-summary/route.ts`:
- Around line 22-32: The comment above the Supabase query is incorrect—update it
to say you're fetching transactions from the start of the current calendar month
(not "last 30 days"), and make the date boundaries timezone-safe by constructing
UTC-based dates (e.g., replace new Date(now.getFullYear(), now.getMonth(), ...)
with new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), ...)) for today
and monthStart) so the .gte('created_at', monthStart.toISOString()) call uses an
explicit UTC boundary; adjust any references to today/monthStart in this file
(route.ts) accordingly around the supabaseAdmin.from('transactions') query that
uses walletAddress and monthStart.
- Around line 5-20: The GET handler in app/api/kyc/transaction-summary/route.ts
currently trusts a walletAddress query param and is unauthenticated; update the
handler to read the authenticated wallet address from the middleware-injected
header (e.g., request.headers.get('x-wallet-address')) instead of searchParams,
return 401 when the header is missing (mirroring app/api/kyc/status/route.ts
behavior), and remove/ignore the walletAddress query param; also update
client-side call site in app/context/KYCContext.tsx (fetchTransactionSummary) to
send the session/auth header so the server can use the middleware-provided
x-wallet-address.
In `@app/api/phone/send-otp/route.ts`:
- Around line 107-109: Trim the incoming name before assigning full_name in the
response/DB payload: replace uses of the raw name variable with a trimmed
version (e.g., const trimmedName = name?.trim()) and set full_name to
trimmedName || existingProfile?.full_name || null so pure-whitespace names are
not stored; update the assignment that currently sets full_name: name ||
existingProfile?.full_name || null accordingly in the send-otp route handler.
In `@app/components/ProfileDrawer.tsx`:
- Around line 54-59: The effect using useEffect with deps [isOpen] is missing an
eslint disable comment and should explicitly suppress the
react-hooks/exhaustive-deps warning or stabilize refreshStatus; add an inline
eslint directive (// eslint-disable-next-line react-hooks/exhaustive-deps)
immediately above the useEffect to silence the warning, or alternatively ensure
refreshStatus is memoized (e.g., wrap refreshStatus in useCallback where it is
defined) so you can safely include it in the dependency array; update the effect
around useEffect, isOpen, setIsLoading, and refreshStatus accordingly.
In `@app/components/TransactionLimitModal.tsx`:
- Around line 88-94: In TransactionLimitModal, the span that renders the monthly
amount is using a hardcoded "text-white" class which will be invisible in light
mode; update the span's className on the element that wraps
formatNumberWithCommas(currentLimits.monthly) to use a theme-aware utility such
as "text-black dark:text-white" or your project's neutral text token (e.g.,
"text-text-primary dark:text-white") so the value is readable in both light and
dark modes and preserves existing font-weight styling.
- Around line 165-169: In TransactionLimitModal.tsx the arrow icon's className
ternary currently yields an empty string for both states, so add the rotation
class when the modal is expanded: update the span (the arrow icon element) to
conditionally include "rotate-180" when the expansion boolean (e.g., expanded /
isExpanded / the state variable that controls the collapsed/expanded branch) is
true, preserving any existing transition classes (e.g., "transition-transform
duration-200") for smooth animation; ensure the falsy branch remains an empty
string or "rotate-0" so the arrow returns to its default orientation when
collapsed.
In `@app/context/KYCContext.tsx`:
- Around line 118-143: fetchTransactionSummary currently calls
/api/kyc/transaction-summary without an Authorization header; update it to
obtain the current access token (e.g., from your auth/session provider or a
getAccessToken function available in this component/context) before the fetch,
return early if no token, and include Authorization: `Bearer ${accessToken}` in
the fetch headers; ensure the token source (or accessToken) is referenced in the
useCallback dependencies so fetchTransactionSummary uses the latest token and
still sets state via setTransactionSummary and updates fetchGuardsRef as before.
- Around line 187-194: The current useEffect resets fetchGuardsRef.current then
calls refreshStatus, but in-flight fetches can still resolve and call
setTier/setTransactionSummary with stale data; modify the logic to cancel or
ignore stale responses by introducing a generation counter (fetchGenRef) or an
AbortController: increment fetchGenRef.current (or create/abort controller)
whenever walletAddress changes (in the useEffect), capture the current gen (or
signal) at the start of fetchKYCStatus and fetchTransactionSummary, and before
calling any state setters (setTier, setTransactionSummary or updating
fetchGuardsRef.current) check that the captured gen matches fetchGenRef.current
(or that the signal is not aborted); this ensures only the latest wallet’s
responses update state.
In `@app/pages/TransactionForm.tsx`:
- Around line 318-329: The useEffect named updateVerificationStatus currently
sets isUserVerified based on both tier and amountSent via canTransact, which
hides the recipient form when the entered amount exceeds the tier limit; change
it so isUserVerified is derived only from tier (e.g., setIsUserVerified(tier >=
1) and remove amountSent/canTransact from that logic and dependencies) and keep
per-transaction blocking in the existing canTransact/handleSwap flow (handleSwap
remains responsible for rejecting submissions when limits are exceeded).
In `@supabase/migrations/create_transactions_table.sql`:
- Around line 78-85: The UPDATE policy "Users can update own transactions" on
table transactions currently only has a USING clause and therefore doesn't
prevent changing wallet_address in the new row; update the policy to include a
WITH CHECK clause that enforces the new row's wallet_address equals
current_setting('app.current_wallet_address', TRUE) so updates cannot reassign
transactions to another wallet; modify the policy definition for "Users can
update own transactions" to add the WITH CHECK(...) condition mirroring the
USING condition.
- Around line 52-58: The SQL currently grants EXECUTE on the function
set_current_wallet_address(TEXT) to anon and authenticated which allows any
client to impersonate wallets; remove the GRANT lines for anon and authenticated
and ensure only the service_role is granted EXECUTE (or explicitly REVOKE
EXECUTE FROM anon, authenticated and GRANT to service_role) so only the trusted
backend can call set_current_wallet_address; locate the GRANT statements
referencing set_current_wallet_address in the migration and modify them to
restrict execution to service_role only.
- Around line 29-30: The updated_at column currently only has a DEFAULT on
INSERT and won't change on UPDATE; add a trigger function (e.g.,
update_updated_at or update_timestamp) that sets NEW.updated_at =
timezone('utc', now()) and returns NEW, then create a BEFORE UPDATE trigger
(e.g., set_updated_at_trigger) on the transactions table to call that function
for each row so updated_at is refreshed automatically on every update. Ensure
the trigger function and trigger operate on the same table and use TIMESTAMP
WITH TIME ZONE to preserve UTC timestamps.
🧹 Nitpick comments (9)
supabase/migrations/create_transactions_table.sql (1)
35-37: Consider a composite index for monthly volume queries.The
transaction-summaryendpoint will query bywallet_address+created_atrange. A composite index covers both that query and plain wallet lookups (via leftmost prefix), making the standaloneidx_transactions_wallet_addressredundant.-CREATE INDEX idx_transactions_wallet_address ON transactions (wallet_address); - -CREATE INDEX idx_transactions_created_at ON transactions (created_at DESC); +CREATE INDEX idx_transactions_wallet_created ON transactions (wallet_address, created_at DESC);app/api/phone/verify-otp/route.ts (2)
15-19: No rate limiting on the verify-otp endpoint.The
send-otpendpoint appliesrateLimit(request), butverify-otpdoes not. While the KudiSMS path enforcesMAX_ATTEMPTS = 3and Twilio has its own rate limiting, an attacker can still send rapid concurrent requests to this endpoint. Adding the same rate limiter used insend-otpwould provide defense-in-depth.Proposed fix
import { NextRequest, NextResponse } from "next/server"; import { supabaseAdmin } from "@/app/lib/supabase"; +import { rateLimit } from "@/app/lib/rate-limit"; ... export async function POST(request: NextRequest) { const startTime = Date.now(); try { + const rateLimitResult = await rateLimit(request); + if (!rateLimitResult.success) { + return NextResponse.json( + { success: false, error: "Too many requests. Please try again later." }, + { status: 429 }, + ); + } + trackApiRequest(request, "/api/phone/verify-otp", "POST");
116-173: Duplicate success-update logic between Twilio and KudiSMS paths.Lines 140–164 (Twilio success) and lines 242–264 (KudiSMS success) perform identical operations: set
verified = true,verified_at, conditionally upgradetierto 1, and handle update errors. Extract a shared helper to reduce duplication and ensure future changes apply to both paths.Also, line 242 uses
anywhile line 140 usesRecord<string, unknown>— prefer the stricter type consistently.Also applies to: 241-264
app/context/KYCContext.tsx (1)
12-17: Dead global type declaration —window.__KYC_FETCH_GUARDS__is never used.The
declare global { interface Window { __KYC_FETCH_GUARDS__?: ... } }augmentation pollutes the globalWindowtype but is never referenced. The actual fetch guard usesfetchGuardsRef(a React ref at line 74). Remove this dead declaration.app/pages/TransactionPreview.tsx (1)
439-443: Intentionally never resetting the ref means no retry on transient failures.If
saveTransactionDatathrows (e.g., network blip), the ref staystrueand the user must reload to retry. This is a deliberate trade-off (prevent duplicates > allow retries), but worth noting — consider whether a user-facing error toast or a manual retry button would improve UX for genuine save failures.app/components/TransactionLimitModal.tsx (1)
19-23:transactionAmountprop is accepted but never used.The prop is declared in the interface and has a default value, but it's never referenced in the component body. Either use it (e.g., to display the blocked amount) or remove it to avoid confusion.
Also applies to: 28-29
app/components/ProfileDrawer.tsx (3)
76-83:navigator.clipboard.writeTextcan reject — add error handling.In non-HTTPS contexts or when the document lacks focus,
writeTextthrows. A rejected promise here will be an unhandled rejection.Suggested fix
const handleCopyAddress = () => { if (walletAddress) { - navigator.clipboard.writeText(walletAddress); - setIsAddressCopied(true); - toast.success("Address copied to clipboard"); - setTimeout(() => setIsAddressCopied(false), 2000); + navigator.clipboard.writeText(walletAddress) + .then(() => { + setIsAddressCopied(true); + toast.success("Address copied to clipboard"); + setTimeout(() => setIsAddressCopied(false), 2000); + }) + .catch(() => { + toast.error("Failed to copy address"); + }); } };
216-216: Use strict equality (===) instead of loose equality (==).Line 216 uses
tier == 0while the rest of the codebase consistently uses===. Sincetieris typed as0 | 1 | 2, this works, but===is the idiomatic choice.- {tier == 0 && tierLevel === tier + 1 && ( + {tier === 0 && tierLevel === tier + 1 && (
333-333:tier !== undefinedis a dead check.
tieris typed as0 | 1 | 2and initialized to0in KYCContext, so it's neverundefined. The>= 1check already handles everything.- {tier >= 1 && tier !== undefined && ( + {tier >= 1 && (
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In @.env.example:
- Around line 129-135: The committed Dojah credentials must be rotated and
removed from the repo: immediately revoke/rotate the keys shown and replace the
hard-coded values for the variables DOJAH_APP_ID and DOJAH_SECRET_KEY in
.env.example with non-sensitive placeholders (e.g.
DOJAH_APP_ID=<YOUR_DOJAH_APP_ID> and DOJAH_SECRET_KEY=<YOUR_DOJAH_SECRET_KEY>),
keep DOJAH_BASE_URL and KYC_DOCUMENTS_BUCKET as example/default values only, and
ensure no real secrets remain in the example file or future commits (also
consider removing the exposed values from git history if required).
In `@app/api/kyc/tier3-verify/route.ts`:
- Around line 39-54: Validate the uploaded file before reading into memory:
check the incoming file object's size and MIME type (the same file used in
Buffer.from(await file.arrayBuffer()) and for deriving ext/path) and reject if
size > 10 * 1024 * 1024 (10MB) or if file.type is not in an allowlist like
image/jpeg, image/png, image/webp; return an appropriate JSON error response
(e.g., 400 or 413) and only call file.arrayBuffer() after these checks; also
derive the file extension from a safe mapping of MIME types when file.name has
no extension to avoid trusting user-provided names.
- Around line 126-134: Before creating updatePayload with tier: 3, fetch the
user's current verification state (e.g., load the user record or call the
existing user service used in this route handler) and assert that Tier 1 and
Tier 2 prerequisites are complete (for example check phone/phone_verified or
tier >= 1 and SmileID/smileid_verified or tier >= 2); if those checks fail,
return a 4xx response and do not proceed to build or apply updatePayload. Locate
the route handler where updatePayload is constructed and add the prerequisite
checks there (verify currentUser.tier or currentUser.phoneVerified and
currentUser.smileIdVerified) before allowing tier to be set to 3.
In `@app/components/kyc/DocumentRequirementsModal.tsx`:
- Around line 39-70: The modal content currently renders directly inside a
motion.div (inside DocumentRequirementsModal) so Headless UI v2 cannot detect
the modal boundary; wrap that motion.div with Headless UI's DialogPanel
component (import DialogPanel from Headless UI if not already) so the motion.div
becomes a child of <DialogPanel> — keep existing props/children (fadeInOut,
className, close button, list rendering, onClose handler) intact and remove any
duplicate outer wrappers so click-outside-to-close behavior is restored while
preserving the Cancel01Icon button and list rendering logic.
In `@app/components/KycModal.tsx`:
- Around line 835-843: handleTier3FileChange and handleTier3Drop currently
silently return on invalid file type/size; update both handlers to set a
user-visible error state instead of silently returning: when ext not in
["JPG","PNG","PDF","DOC","JPEG","DOCX"] call the error state setter (e.g.,
setTier3Error or setTier3ErrorMessage) with a message like "Invalid file type;
allowed: JPG, PNG, PDF, DOC, JPEG, DOCX" and return; when file.size > 5 * 1024 *
1024 set the error state to "File too large; maximum 5 MB" and return; on
successful validation clear the error state before calling
setTier3UploadedFile(file). Ensure you apply the same changes to both
handleTier3FileChange and handleTier3Drop and use the same validation logic and
messages so users get clear feedback.
- Around line 957-964: The current rendering uses
tier3UploadedFile.type.split("/")[1].toUpperCase(), which throws if
tier3UploadedFile.type is empty; update the JSX that displays the file format
(the expression inside the second <p> for tier3UploadedFile) to defensively
handle missing MIME type by first checking tier3UploadedFile.type (e.g., use
optional chaining or a conditional) and falling back to a safe value derived
from the filename extension or a literal like "UNKNOWN" before calling
toUpperCase(); ensure the final value is always a string so toUpperCase() cannot
throw.
In `@app/components/ProfileDrawer.tsx`:
- Around line 380-387: The progress bar gradient in ProfileDrawer.tsx is using
identical colors ("from-white to-white") so it is invisible on light
backgrounds; update the inner div (the element with className "h-2.5
rounded-full bg-gradient-to-r from-white to-white transition-all duration-500")
to use distinct, accessible gradient colors (e.g., Tailwind classes like
from-sky-500 to-indigo-500) and add dark-mode variants (e.g., dark:from-sky-400
dark:to-indigo-400) or use semantic CSS variables (e.g., from-[var(--accent-1)]
to-[var(--accent-2)]) so the gradient is visible in both light and dark themes
while preserving the existing width transition logic.
- Around line 167-172: The ArrowDown01Icon rotation classes are missing because
classNames is given empty strings for both branches; update the classNames call
in ProfileDrawer.tsx (for ArrowDown01Icon) to conditionally add a rotation class
based on isExpanded (e.g., add "rotate-180" when isExpanded is true and "" or
"rotate-0" when false) and keep the existing transition classes so the chevron
visually rotates when toggling; reference the ArrowDown01Icon element, the
isExpanded boolean, and the classNames helper when making this change.
In `@app/components/TransactionLimitModal.tsx`:
- Around line 65-69: In handlePhoneVerified, await the async refreshStatus()
call and remove the explicit onClose() invocation to avoid closing the modal
prematurely and duplicating the close action; specifically update the
handlePhoneVerified function so it calls await refreshStatus() (ensuring the
KYC/tier state is refreshed before UI updates) and delete the onClose() call
there, relying on PhoneVerificationModal's own onClose prop (triggered alongside
onVerified) to close the modal.
In `@app/context/KYCContext.tsx`:
- Around line 136-161: The fetchTransactionSummary (and fetchKYCStatus)
functions call response.json() without checking HTTP status; update both to
first test response.ok (or response.status) after the fetch and handle non-OK
responses by reading/attempting to parse the body for error details or logging
response.status/text and treating the call as failed (set guards via
fetchGuardsRef.current using the same key pattern `${guardKey}_tx` / appropriate
key, and bail out or throw so the error is caught and logged), ensuring you
still set guards[key] = "done" in finally; this prevents attempting to parse
non-JSON error pages and surfaces 4xx/5xx failures to logs and callers.
🧹 Nitpick comments (9)
app/lib/dojah.ts (2)
37-51: Add a timeout to the Dojah API call.The
fetchto Dojah has no timeout. If the external service is slow or unresponsive, this request will hang indefinitely, tying up a serverless function invocation.Proposed fix
const res = await fetch( `${DOJAH_BASE_URL}/api/v1/document/analysis/utility_bill`, { method: "POST", headers: { "Content-Type": "application/json", AppId: DOJAH_APP_ID, Authorization: DOJAH_SECRET_KEY, }, body: JSON.stringify({ input_type: "url", input_value: imageUrl, }), + signal: AbortSignal.timeout(30_000), } );
53-58: Non-JSON error responses will produce a confusing error.If Dojah returns a non-JSON response (e.g., HTML 502 page),
res.json()on line 53 will throw aSyntaxErrorbefore the!res.okcheck on line 55 can run. The caller's catch block will handle it, but the error message will be unhelpful (e.g., "Unexpected token < in JSON").Consider checking
res.okbefore parsing, or catching the JSON parse error:Proposed fix
+ if (!res.ok) { + let message = res.statusText; + try { + const errData = await res.json(); + message = errData?.message || errData?.entity?.result?.message || message; + } catch { /* non-JSON body */ } + throw new Error(message || `Dojah request failed: ${res.status}`); + } + - const data = (await res.json()) as DojahUtilityBillResponse & { message?: string }; - - if (!res.ok) { - const message = data?.message || data?.entity?.result?.message || res.statusText; - throw new Error(message || `Dojah request failed: ${res.status}`); - } + const data = (await res.json()) as DojahUtilityBillResponse; return data;app/api/kyc/tier3-verify/route.ts (1)
94-103: Uploaded document persists in storage even if Dojah verification fails.When
isDojahVerificationSuccessreturnsfalse(line 95), the endpoint returns a 400 but leaves the uploaded file in the storage bucket. Over time, failed verification attempts will accumulate orphaned files, increasing storage costs.Consider cleaning up on failure:
Proposed addition
if (!isDojahVerificationSuccess(dojahResult)) { + // Clean up the uploaded file since verification failed + await supabaseAdmin.storage.from(KYC_BUCKET).remove([path]); const msg = dojahResult?.entity?.result?.message ||app/context/KYCContext.tsx (1)
12-17: Unused global type declaration.
Window.__KYC_FETCH_GUARDS__is declared but never assigned or read anywhere. The guard mechanism usesfetchGuardsRefexclusively. Remove this dead declaration to avoid confusion.Proposed fix
-// Extend the Window interface for our in-memory fetch guard -declare global { - interface Window { - __KYC_FETCH_GUARDS__?: Record<string, string>; - } -}app/components/ProfileDrawer.tsx (1)
346-404: Minor: redundanttier !== undefinedguard and loose comparison at Line 230.On Line 347,
tieris typedKYCTierLevel(always0|1|2|3|4), sotier !== undefinedis always true. On Line 230 (tier == 0), prefer strict equality (===).app/components/KycModal.tsx (4)
8-14: Global JSXIntrinsicElementsusesany— consider a minimal shape.Using
anydefeats type-checking for every attribute on<smart-camera-web>. A narrower type improves safety.Suggested improvement
declare global { namespace JSX { interface IntrinsicElements { - "smart-camera-web": any; + "smart-camera-web": React.DetailedHTMLProps< + React.HTMLAttributes<HTMLElement> & { + "theme-color"?: string; + "capture-id"?: boolean; + }, + HTMLElement + >; } } }
92-96:getAllCountries()is called on every render — memoize or hoist.
getAllCountries()sorts the full country list on each render. SinceidTypesDatais a static JSON import, the result never changes. Hoist it to module scope.Proposed fix
-const getAllCountries = (): Country[] => { - return idTypesData.continents - .flatMap((continent) => continent.countries) - .sort((a, b) => a.name.localeCompare(b.name)); -}; +const ALL_COUNTRIES: Country[] = idTypesData.continents + .flatMap((continent) => continent.countries) + .sort((a, b) => a.name.localeCompare(b.name)); // inside component: -const countries = getAllCountries(); +const countries = ALL_COUNTRIES;Also applies to: 153-153
178-190: Removeconsole.logfrom production path.Line 182 logs on every successful SmileID load. Use a debug-level logger or remove.
1259-1276: All steps rendered eagerly via object-lookup pattern.Every render function (including tier3 forms, capture, etc.) is called on every render, regardless of which step is active. This is wasteful — the SmileID camera component gets mounted/unmounted implicitly. Consider lazy evaluation:
Sketch
- { - { - [STEPS.TERMS]: renderTerms(), - ... - }[step] - } + {step === STEPS.TERMS && renderTerms()} + {step === STEPS.ID_INFO && renderIdInfo()} + {step === STEPS.CAPTURE && renderCapture()} + {/* ... etc */}This avoids constructing unused JSX trees and prevents unintended side-effects from render functions (like
renderTier3Uploaddefining handlers on every render).
|
@coderabbitai review |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/components/SettingsDropdown.tsx (1)
281-307:⚠️ Potential issue | 🔴 CriticalFix JSX parse error from stray
*/}/ unmatched conditional.
Line 290 introduces an unexpected token and breaks parsing. Remove the stray*/and the extra closing)}so JSX compiles.🛠️ Suggested fix
- )} */} + )} <li role="menuitem" className="flex cursor-pointer items-center justify-between gap-2 rounded-lg transition-all duration-300 hover:bg-accent-gray dark:hover:bg-neutral-700" > <button type="button" className="group flex w-full items-center gap-2.5" onClick={() => { setIsProfileDrawerOpen(true); setIsOpen(false); }} > <FaceIdIcon className="size-5 text-icon-outline-secondary dark:text-white/50" /> <p>Profile</p> </button> </li> - )}
🤖 Fix all issues with AI agents
In `@app/api/kyc/tier3-verify/route.ts`:
- Around line 11-13: The server-side allowlist (ALLOWED_MIME_TYPES and
MIME_TO_EXT) only contains image MIME types while the UI accepts PDFs/DOCs;
update ALLOWED_MIME_TYPES to include "application/pdf", "application/msword",
"application/vnd.openxmlformats-officedocument.wordprocessingml.document" and
add corresponding entries to MIME_TO_EXT (e.g., "pdf" / "doc" / "docx"), then
ensure the route's validation logic (the handler referencing MAX_FILE_BYTES,
ALLOWED_MIME_TYPES, MIME_TO_EXT) uses the updated constants; also update the
other occurrence noted around lines 65-73 to reference the same allowlist so
client and server validations match.
- Around line 59-63: The error message for the file size check is inconsistent:
the check uses file.size > MAX_FILE_BYTES but the NextResponse.json message says
"maximum 10 MB"; update the message to reflect the actual MAX_FILE_BYTES limit
(e.g., "maximum 5 MB") or build the message dynamically from MAX_FILE_BYTES so
they stay in sync; modify the NextResponse.json call in the branch that checks
file.size > MAX_FILE_BYTES to use the corrected text or a computed
human-readable value based on MAX_FILE_BYTES.
In `@app/components/ProfileDrawer.tsx`:
- Around line 304-307: The <img> in ProfileDrawer.tsx is missing an alt
attribute; update the image element used for the profile avatar (in the
ProfileDrawer component) to include a descriptive alt string (e.g., the user's
name like `${user.name} avatar` or a fallback like "profile picture") and ensure
the alt is set consistently whether you're rendering the placeholder
("/icons/placeholder.png") or a real avatar URL so screen readers receive
meaningful text.
🧹 Nitpick comments (2)
app/context/KYCContext.tsx (2)
12-17: Unused global declaration.
Window.__KYC_FETCH_GUARDS__is declared but never assigned or read anywhere in this file. The actual guards are stored infetchGuardsRef.current. This declaration appears to be leftover from an earlier design.Proposed fix: remove unused declaration
-// Extend the Window interface for our in-memory fetch guard -declare global { - interface Window { - __KYC_FETCH_GUARDS__?: Record<string, string>; - } -} -
24-31: Type mismatch betweenKYCTierLevelandKYCTier.level.
KYCTierLevelincludes0, butKYCTier.levelis typed as1 | 2 | 3 | 4. This inconsistency could cause confusion since tier 0 (unverified) is a valid runtime state but has no correspondingKYCTierentry.Consider aligning the types or documenting that tier 0 is intentionally excluded from
KYC_TIERS:Option: Add explicit documentation
+// Tier 0 represents unverified users and has no KYCTier entry in KYC_TIERS export type KYCTierLevel = 0 | 1 | 2 | 3 | 4;
| const MAX_FILE_BYTES = 5 * 1024 * 1024; // 5MB | ||
| const ALLOWED_MIME_TYPES = ["image/jpeg", "image/png", "image/webp"] as const; | ||
| const MIME_TO_EXT: Record<string, string> = { |
There was a problem hiding this comment.
Backend allowlist rejects file types the UI accepts.
The Tier 3 UI accepts PDF/DOC/DOCX, but this route only allows image MIME types. Users can pass client validation and still hit a 400 here. Align server + client allowlists (either tighten the UI to images or expand the server list to match).
🛠️ Example if PDFs are intended to be allowed
-const ALLOWED_MIME_TYPES = ["image/jpeg", "image/png", "image/webp"] as const;
+const ALLOWED_MIME_TYPES = [
+ "image/jpeg",
+ "image/png",
+ "image/webp",
+ "application/pdf",
+] as const;
const MIME_TO_EXT: Record<string, string> = {
"image/jpeg": "jpg",
"image/png": "png",
"image/webp": "webp",
+ "application/pdf": "pdf",
};Also applies to: 65-73
🤖 Prompt for AI Agents
In `@app/api/kyc/tier3-verify/route.ts` around lines 11 - 13, The server-side
allowlist (ALLOWED_MIME_TYPES and MIME_TO_EXT) only contains image MIME types
while the UI accepts PDFs/DOCs; update ALLOWED_MIME_TYPES to include
"application/pdf", "application/msword",
"application/vnd.openxmlformats-officedocument.wordprocessingml.document" and
add corresponding entries to MIME_TO_EXT (e.g., "pdf" / "doc" / "docx"), then
ensure the route's validation logic (the handler referencing MAX_FILE_BYTES,
ALLOWED_MIME_TYPES, MIME_TO_EXT) uses the updated constants; also update the
other occurrence noted around lines 65-73 to reference the same allowlist so
client and server validations match.
| <img | ||
| src="/icons/placeholder.png" | ||
| className="object-fit h-[44px] w-[44px] rounded-full" | ||
| /> |
There was a problem hiding this comment.
Missing alt attribute on image element.
The <img> element is missing an alt attribute, which is an accessibility requirement. Screen readers need alternative text to describe the image.
Proposed fix
<img
src="/icons/placeholder.png"
className="object-fit h-[44px] w-[44px] rounded-full"
+ alt="User profile"
/>🤖 Prompt for AI Agents
In `@app/components/ProfileDrawer.tsx` around lines 304 - 307, The <img> in
ProfileDrawer.tsx is missing an alt attribute; update the image element used for
the profile avatar (in the ProfileDrawer component) to include a descriptive alt
string (e.g., the user's name like `${user.name} avatar` or a fallback like
"profile picture") and ensure the alt is set consistently whether you're
rendering the placeholder ("/icons/placeholder.png") or a real avatar URL so
screen readers receive meaningful text.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@app/api/phone/verify-otp/route.ts`:
- Around line 197-205: The current read-modify-write that updates attempts using
verification.attempts is non-atomic and can lose increments under concurrent
requests; create a Postgres function (e.g., increment_kyc_attempts) that
performs "update user_kyc_profiles set attempts = attempts + 1 where
wallet_address = p_wallet_address and attempts < MAX_ATTEMPTS returning
attempts" and then replace the .update(...) call on supabaseAdmin in the verify
flow with a single .rpc('increment_kyc_attempts', { p_wallet_address:
walletAddress }) call (handle the returned data/error like the previous
updated/attemptsError and still enforce MAX_ATTEMPTS constant in the DB
function).
In `@app/components/KycModal.tsx`:
- Around line 1174-1179: In KycModal, the Typeform container div using
typeformContainerRef has an invalid empty width (style={{ width: "", minHeight:
"11px" }}) and an unusually small minHeight; remove the empty width property (or
set a sensible value like "100%" or a px value) and increase minHeight to a
reasonable size for an embed (e.g., ~200px) or replace the inline styles with a
named CSS class; also remove the redundant empty className to keep the element
clean.
- Around line 1054-1089: The onClick handler for Tier 3 submission doesn't check
HTTP status and can exit early without resetting tier3Submitting; update the
handler (the async onClick that calls getAccessToken and
fetch("/api/kyc/tier3-verify")) so that after calling getAccessToken you do not
return without clearing setTier3Submitting(false) (i.e., if no token, call
setTier3Submitting(false) then toast and return), and after fetch check res.ok
before calling res.json(); if !res.ok attempt to parse JSON inside a try/catch
and fall back to await res.text() to capture error details, then
setStep(STEPS.STATUS.FAILED) and surface a helpful toast/log including the
parsed error message; only treat success when res.ok and parsed data.success is
true, then call refreshStatus(), setIsUserVerified(true) and setStep SUCCESS.
Ensure all early-return paths clear tier3Submitting so the finally is no longer
the only mechanism relied upon.
In `@app/components/ProfileDrawer.tsx`:
- Around line 186-208: The icon container in the tierData.requirements.map
rendering can end up empty because the chained conditional returns false when no
condition matches (the `req.includes("ID") && (...)` yields false), so update
the JSX in ProfileDrawer.tsx inside the map (the <li> and its inner <div> that
currently renders icons) to ensure the expression always returns a React
element: replace the chained ternaries with a clear branching that returns the
appropriate icon for "number", "Selfie verification", "Address verification",
"Business verification", or "ID", and add an explicit fallback icon (e.g.,
DefaultIcon) for any other requirement string so the container never renders
empty. Ensure the final expression always yields a valid element (not a boolean)
so the icon placeholder is always populated.
🧹 Nitpick comments (7)
app/api/phone/verify-otp/route.ts (1)
29-32: Middleware already normalizes wallet addresses; add explicit lowercasing for defensive consistency.The
x-wallet-addressheader is set by middleware after normalizing throughprivy.ts(which returns.toLowerCase()), so wallet addresses reaching this route are already lowercase. However, other routes like/api/v1/wallets/deprecateexplicitly call.toLowerCase()on this header for defensive consistency. Consider adopting the same pattern here to match the codebase convention and protect against future middleware changes.Suggested adjustment
- const walletAddress = request.headers.get("x-wallet-address"); + const walletAddress = request.headers.get("x-wallet-address")?.toLowerCase();app/components/ProfileDrawer.tsx (2)
77-84: Add error handling for clipboard write failure.
navigator.clipboard.writeTextcan fail in certain environments (e.g., non-HTTPS, iframe restrictions). Currently errors are silently swallowed and the success state is set unconditionally.Proposed fix
const handleCopyAddress = () => { - if (walletAddress) { - navigator.clipboard.writeText(walletAddress); - setIsAddressCopied(true); - toast.success("Address copied to clipboard"); - setTimeout(() => setIsAddressCopied(false), 2000); + 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"); + }); } };
93-101: Auto-expand effect only triggers fortier < 1, missing higher tiers.The effect only auto-expands the next tier section when
tier < 1(i.e., Tier 0). Users at Tier 1, 2, or 3 won't see their next tier auto-expanded. Consider expanding the next tier for all users who have a next tier available.Proposed fix
// Auto-expand next tier section useEffect(() => { - if (tier < 1) { + const nextTier = tier + 1; + if (KYC_TIERS[nextTier]) { setExpandedTiers((prev) => ({ ...prev, - [tier + 1]: true, + [nextTier]: true, })); } }, [tier]);app/context/KYCContext.tsx (2)
202-213:refreshStatussignature change may break callers expecting a no-arg function.The function signature is
refreshStatus = useCallback(async (force = false) => {...}), but the context type at line 80 declaresrefreshStatus: () => Promise<void>. Callers won't get type errors when omitting the argument (due to the default), but the type mismatch could cause confusion.Update the type to reflect the optional parameter
interface KYCContextType { // ... - refreshStatus: () => Promise<void>; + refreshStatus: (force?: boolean) => Promise<void>; }
122-137:canTransactreason message doesn't format currency consistently.The reason string uses
$${amount}and$${remaining.monthly}without formatting. For large amounts, this could show$15000instead of$15,000. Consider usingformatNumberWithCommasfor consistency with the rest of the app.app/components/KycModal.tsx (2)
194-206: SmileID load failure only shows a toast — consider blocking progression.When the dynamic import fails, a toast is shown but
smileIdLoadedremainsfalse. If the user somehow reaches the CAPTURE step (e.g., via rapid clicking or state manipulation), they'll see a broken camera component. Consider also setting an error state that prevents navigation to CAPTURE.Proposed enhancement
+ const [smileIdError, setSmileIdError] = useState(false); useEffect(() => { if (targetTier === 3 || targetTier === 4) return; if (typeof window !== "undefined" && !smileIdLoaded) { import("@smileid/web-components/smart-camera-web") .then(() => { setSmileIdLoaded(true); }) .catch(() => { toast.error("Failed to load verification component"); + setSmileIdError(true); }); } }, [smileIdLoaded, targetTier]); // Then in renderIdInfo, disable Continue button if smileIdError: - disabled={!selectedCountry || !selectedIdType || (!needsDocCapture && !idNumber)} + disabled={!selectedCountry || !selectedIdType || (!needsDocCapture && !idNumber) || smileIdError}
1109-1144: Typeform script loading has potential issues.
- The
spinnerTimersetsisTypeformReadyto true after 2 seconds regardless of whether the script actually loaded, potentially hiding the spinner before the form is ready.- No cleanup removes the injected script on unmount, which could cause issues if the component remounts.
Proposed improvement
useEffect(() => { if (step !== STEPS.TIER4_TYPEFORM) return; setIsTypeformReady(false); const loadScript = () => { if (typeformScriptLoaded.current && window.tf) { window.tf.load(); + setIsTypeformReady(true); return; } const existing = document.querySelector( 'script[src*="embed.typeform.com/next/embed.js"]', ); if (existing) { typeformScriptLoaded.current = true; window.tf?.load(); + setIsTypeformReady(true); return; } const script = document.createElement("script"); script.src = "//embed.typeform.com/next/embed.js"; script.async = true; script.onload = () => { typeformScriptLoaded.current = true; + setIsTypeformReady(true); }; document.head.appendChild(script); }; const scriptTimer = setTimeout(loadScript, 100); - const spinnerTimer = setTimeout(() => setIsTypeformReady(true), 2000); return () => { clearTimeout(scriptTimer); - clearTimeout(spinnerTimer); }; }, [step]);
| if (verification.otp_code !== hashOTP(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) | ||
| .lt("attempts", MAX_ATTEMPTS) | ||
| .select("attempts") | ||
| .single(); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Supabase PostgREST atomic increment update RETURNING keyword
💡 Result:
Supabase’s REST API is PostgREST, so you don’t send a SQL RETURNING keyword. Instead, you control “return updated row(s)” with the Prefer header:
- Default for writes: no body (
Prefer: return=minimal) - To get updated rows back:
Prefer: return=representation(optionally combine with?select=...) [1]
Example (REST / PostgREST):
curl -X PATCH 'https://<project_ref>.supabase.co/rest/v1/counters?id=eq.42&select=id,count' \
-H "apikey: <anon>" \
-H "Authorization: Bearer <jwt-or-anon>" \
-H "Content-Type: application/json" \
-H "Prefer: return=representation" \
-d '{ "count": 123 }'Atomic increment
A true atomic increment is SET count = count + 1 inside Postgres. Because PostgREST updates are JSON-based, the common Supabase/PostgREST approach is to do the increment in a Postgres function and call it via RPC [2][4]:
create or replace function increment_counter(row_id bigint, inc int default 1)
returns bigint
language sql
as $$
update counters
set count = count + inc
where id = row_id
returning count;
$$;Call it:
- JS:
const { data, error } = await supabase.rpc('increment_counter', { row_id: 42, inc: 1 })[2]
- REST:
POST https://<project_ref>.supabase.co/rest/v1/rpc/increment_counter(body:{"row_id":42,"inc":1}) [4]
🌐 Web query:
Supabase JavaScript client v2 increment column value atomic update
💡 Result:
To do an atomic increment with @supabase/supabase-js v2, run the increment inside Postgres (single UPDATE ... SET col = col + n ... RETURNING ...) and call it via RPC from the client. Supabase’s JS client supports calling Postgres functions with supabase.rpc(...). [1]
1) Create a Postgres function (atomic increment)
create or replace function increment_counter(p_id uuid, p_by int)
returns bigint
language sql
as $$
update my_table
set counter = counter + p_by
where id = p_id
returning counter;
$$;2) Call it from supabase-js v2
const { data, error } = await supabase.rpc('increment_counter', {
p_id: someId,
p_by: 1,
})
if (error) throw error
// data is the returned bigint (new counter value)Notes
- This avoids the non-atomic “select then update” race condition by doing the math in the database in one statement.
- Adjust parameter/return types to match your table (e.g.,
bigint,int, etc.).
Source
- Supabase docs: calling Postgres functions via
supabase.rpc[1]
References:
[1] Supabase Docs — “JavaScript: Call a Postgres function” https://supabase.com/docs/client/rpc
🌐 Web query:
Supabase race condition concurrent updates attempts counter
💡 Result:
To avoid a race condition when multiple clients concurrently increment an “attempts” counter in Supabase, don’t do read → increment in app → write. Instead, make the increment happen inside Postgres as a single statement (atomic at the row level).
Best pattern: atomic increment (no read-modify-write)
Use:
update my_table
set attempts = attempts + 1
where id = _id
returning attempts;This avoids lost updates because the database computes attempts + 1 during the UPDATE, rather than you sending a precomputed value from the client. [1]
Supabase: expose it via an RPC (recommended)
Since Supabase clients generally operate through PostgREST, put the update in a function and call it with rpc() [1]:
create or replace function increment_attempts(_id uuid)
returns bigint
language sql
as $$
update my_table
set attempts = attempts + 1
where id = _id
returning attempts;
$$;JS:
const { data, error } = await supabase.rpc('increment_attempts', { _id: id })If you need more complex logic: lock the row in a transaction
If you must read other columns and make decisions before updating, do it in a single DB function using row-level locks (e.g., SELECT ... FOR UPDATE / FOR NO KEY UPDATE) so concurrent transactions wait rather than overwrite each other. PostgreSQL documents these row locks and their blocking behavior. [3]
Supabase/RLS gotcha (common “it didn’t update” symptom)
With RLS enabled, an UPDATE may “not work as expected” unless the role also has a matching SELECT policy (because Postgres needs row visibility for updates). Supabase explicitly notes that UPDATE requires a corresponding SELECT policy. [2]
Sources: [1] [2] [3]
🏁 Script executed:
cat -n app/api/phone/verify-otp/route.ts | head -250 | tail -100Repository: paycrest/noblocks
Length of output: 3686
🏁 Script executed:
rg "supabase.rpc|increment|attempts" app/api/phone/verify-otp/route.ts -B2 -A2Repository: paycrest/noblocks
Length of output: 2127
🏁 Script executed:
fd "route.ts" --type f | xargs rg "rpc\(" -l | head -5Repository: paycrest/noblocks
Length of output: 105
Replace client-side read-modify-write with a database-side atomic increment to prevent lost OTP-attempt updates under concurrent requests.
The current pattern attempts: verification.attempts + 1 performs a non-atomic read-modify-write at the application level. While the .lt("attempts", MAX_ATTEMPTS) filter prevents exceeding the limit, it does not prevent concurrent requests from reading the same stale attempts value and overwriting each other's increments. For example, if two concurrent invalid OTPs both read attempts=5, both will write 6, losing one increment and weakening the rate limit.
Create a Postgres function that increments atomically within the database:
Example atomic increment function
create or replace function increment_kyc_attempts(p_wallet_address text)
returns bigint
language sql
as $$
update user_kyc_profiles
set attempts = attempts + 1
where wallet_address = p_wallet_address
and attempts < 5 -- MAX_ATTEMPTS as constant
returning attempts;
$$;Then replace lines 199–205 with:
const { data: updated, error: attemptsError } = await supabaseAdmin
.rpc('increment_kyc_attempts', { p_wallet_address: walletAddress })
.single();This ensures the increment and boundary check execute as a single database operation, eliminating the race condition.
🤖 Prompt for AI Agents
In `@app/api/phone/verify-otp/route.ts` around lines 197 - 205, The current
read-modify-write that updates attempts using verification.attempts is
non-atomic and can lose increments under concurrent requests; create a Postgres
function (e.g., increment_kyc_attempts) that performs "update user_kyc_profiles
set attempts = attempts + 1 where wallet_address = p_wallet_address and attempts
< MAX_ATTEMPTS returning attempts" and then replace the .update(...) call on
supabaseAdmin in the verify flow with a single .rpc('increment_kyc_attempts', {
p_wallet_address: walletAddress }) call (handle the returned data/error like the
previous updated/attemptsError and still enforce MAX_ATTEMPTS constant in the DB
function).
| onClick={async () => { | ||
| if (!tier3UploadedFile || tier3Submitting) return; | ||
| setTier3Submitting(true); | ||
| try { | ||
| const accessToken = await getAccessToken(); | ||
| if (!accessToken) { | ||
| toast.error("Session expired. Please sign in again."); | ||
| return; | ||
| } | ||
| const formData = new FormData(); | ||
| formData.append("file", tier3UploadedFile); | ||
| formData.append("countryCode", tier3CountryCode); | ||
| formData.append("documentType", tier3DocumentType); | ||
| formData.append("houseNumber", tier3HouseNumber); | ||
| formData.append("streetAddress", tier3StreetAddress); | ||
| formData.append("county", tier3County); | ||
| formData.append("postalCode", tier3PostalCode); | ||
| const res = await fetch("/api/kyc/tier3-verify", { | ||
| method: "POST", | ||
| headers: { Authorization: `Bearer ${accessToken}` }, | ||
| body: formData, | ||
| }); | ||
| const data = await res.json(); | ||
| if (data.success) { | ||
| await refreshStatus(); | ||
| setIsUserVerified(true); | ||
| setStep(STEPS.STATUS.SUCCESS); | ||
| } else { | ||
| setStep(STEPS.STATUS.FAILED); | ||
| } | ||
| } catch (e) { | ||
| setStep(STEPS.STATUS.FAILED); | ||
| } finally { | ||
| setTier3Submitting(false); | ||
| } | ||
| }} |
There was a problem hiding this comment.
Tier 3 submission doesn't handle non-2xx responses properly.
At line 1076, await res.json() is called without first checking res.ok. If the server returns a 4xx/5xx error with a non-JSON body (e.g., HTML error page), this will throw a JSON parse error, masking the actual error. Additionally, the finally block always sets tier3Submitting to false, but if getAccessToken() returns null, the function returns early without reaching finally.
Proposed fix
const res = await fetch("/api/kyc/tier3-verify", {
method: "POST",
headers: { Authorization: `Bearer ${accessToken}` },
body: formData,
});
+ if (!res.ok) {
+ const errorText = await res.text();
+ throw new Error(errorText || `HTTP ${res.status}`);
+ }
const data = await res.json();
if (data.success) {
await refreshStatus();
setIsUserVerified(true);
setStep(STEPS.STATUS.SUCCESS);
} else {
+ toast.error(data.message || "Verification failed");
setStep(STEPS.STATUS.FAILED);
}Also fix the early return:
if (!accessToken) {
toast.error("Session expired. Please sign in again.");
+ setTier3Submitting(false);
return;
}🤖 Prompt for AI Agents
In `@app/components/KycModal.tsx` around lines 1054 - 1089, The onClick handler
for Tier 3 submission doesn't check HTTP status and can exit early without
resetting tier3Submitting; update the handler (the async onClick that calls
getAccessToken and fetch("/api/kyc/tier3-verify")) so that after calling
getAccessToken you do not return without clearing setTier3Submitting(false)
(i.e., if no token, call setTier3Submitting(false) then toast and return), and
after fetch check res.ok before calling res.json(); if !res.ok attempt to parse
JSON inside a try/catch and fall back to await res.text() to capture error
details, then setStep(STEPS.STATUS.FAILED) and surface a helpful toast/log
including the parsed error message; only treat success when res.ok and parsed
data.success is true, then call refreshStatus(), setIsUserVerified(true) and
setStep SUCCESS. Ensure all early-return paths clear tier3Submitting so the
finally is no longer the only mechanism relied upon.
| <div | ||
| ref={typeformContainerRef} | ||
| data-tf-live="01KH3NC0M1G4MNHBCWM320WBP9" | ||
| style={{ width: "", minHeight: "11px" }} | ||
| className="" | ||
| /> |
There was a problem hiding this comment.
Typeform container has empty width style and unclear purpose.
The inline style style={{ width: "", minHeight: "11px" }} sets an empty width string, which is likely unintentional. This may cause layout issues. Also, minHeight: "11px" seems unusually small for a Typeform embed trigger.
Proposed fix
<div
ref={typeformContainerRef}
data-tf-live="01KH3NC0M1G4MNHBCWM320WBP9"
- style={{ width: "", minHeight: "11px" }}
- className=""
+ style={{ minHeight: "44px" }}
+ className="flex-1"
/>📝 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.
| <div | |
| ref={typeformContainerRef} | |
| data-tf-live="01KH3NC0M1G4MNHBCWM320WBP9" | |
| style={{ width: "", minHeight: "11px" }} | |
| className="" | |
| /> | |
| <div | |
| ref={typeformContainerRef} | |
| data-tf-live="01KH3NC0M1G4MNHBCWM320WBP9" | |
| style={{ minHeight: "44px" }} | |
| className="flex-1" | |
| /> |
🤖 Prompt for AI Agents
In `@app/components/KycModal.tsx` around lines 1174 - 1179, In KycModal, the
Typeform container div using typeformContainerRef has an invalid empty width
(style={{ width: "", minHeight: "11px" }}) and an unusually small minHeight;
remove the empty width property (or set a sensible value like "100%" or a px
value) and increase minHeight to a reasonable size for an embed (e.g., ~200px)
or replace the inline styles with a named CSS class; also remove the redundant
empty className to keep the element clean.
| {tierData.requirements.map((req, index) => ( | ||
| <li | ||
| key={index} | ||
| className="flex items-center gap-2 text-xs text-text-secondary dark:text-white/60" | ||
| > | ||
| <div className="rounded-lg bg-white/10 p-0.5"> | ||
| {req.includes("number") ? ( | ||
| <CallingIcon className="size-4 text-outline-gray dark:text-white/50" /> | ||
| ) : req.includes("Selfie verification") ? ( | ||
| <FaceIdIcon className="size-4 text-outline-gray dark:text-white/70" /> | ||
| ) : req.includes("Address verification") ? ( | ||
| <MapPinpoint01Icon className="size-4 text-outline-gray dark:text-white/50" /> | ||
| ) : req.includes("Business verification") ? ( | ||
| <WorkAlertIcon className="size-4 text-outline-gray dark:text-white/50" /> | ||
| ) : ( | ||
| req.includes("ID") && ( | ||
| <TbIdBadge className="size-4 text-outline-gray dark:text-white/50" /> | ||
| ) | ||
| )} | ||
| </div> | ||
| <span>{req}</span> | ||
| </li> | ||
| ))} |
There was a problem hiding this comment.
Empty icon placeholder when requirement doesn't match any condition.
When a requirement string doesn't contain "number", "Selfie verification", "Address verification", "Business verification", or "ID", no icon is rendered. The last branch req.includes("ID") && (...) returns false (not an element) when the condition isn't met, leaving an empty icon container.
Proposed fix — add a fallback icon
{req.includes("number") ? (
<CallingIcon className="size-4 text-outline-gray dark:text-white/50" />
) : req.includes("Selfie verification") ? (
<FaceIdIcon className="size-4 text-outline-gray dark:text-white/70" />
) : req.includes("Address verification") ? (
<MapPinpoint01Icon className="size-4 text-outline-gray dark:text-white/50" />
) : req.includes("Business verification") ? (
<WorkAlertIcon className="size-4 text-outline-gray dark:text-white/50" />
- ) : (
- req.includes("ID") && (
- <TbIdBadge className="size-4 text-outline-gray dark:text-white/50" />
- )
+ ) : req.includes("ID") ? (
+ <TbIdBadge className="size-4 text-outline-gray dark:text-white/50" />
+ ) : (
+ <TbIdBadge className="size-4 text-outline-gray dark:text-white/50" />
)}🤖 Prompt for AI Agents
In `@app/components/ProfileDrawer.tsx` around lines 186 - 208, The icon container
in the tierData.requirements.map rendering can end up empty because the chained
conditional returns false when no condition matches (the `req.includes("ID") &&
(...)` yields false), so update the JSX in ProfileDrawer.tsx inside the map (the
<li> and its inner <div> that currently renders icons) to ensure the expression
always returns a React element: replace the chained ternaries with a clear
branching that returns the appropriate icon for "number", "Selfie verification",
"Address verification", "Business verification", or "ID", and add an explicit
fallback icon (e.g., DefaultIcon) for any other requirement string so the
container never renders empty. Ensure the final expression always yields a valid
element (not a boolean) so the icon placeholder is always populated.
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)
package.json (1)
43-81:⚠️ Potential issue | 🟠 MajorUpgrade
twilioto 5.12.1 to patch CVE-2025-15284 in transitive dependencyqs.
twilio@5.10.5(line 81) is vulnerable via itsqsdependency. Version 5.12.0+ patches this. Node 22 is supported.smile-identity-core@3.1.0(line 76) is current; Node 22 supported.@smileid/web-components@11.0.2(line 43) is 1 minor version behind (11.1.0 available) but no advisories found.libphonenumber-js@1.12.27(line 60) is 9 patches behind (1.12.36 available) but no advisories found.app/components/SettingsDropdown.tsx (1)
83-92:⚠️ Potential issue | 🟡 MinorFix email success toast to use
user.email?.addressinstead ofuser.email.The
onSuccesscallback interpolatesuser.emaildirectly, butuser.emailis an object. This will produce"[object Object] linked successfully"in the toast.Proposed fix
const { linkEmail } = useLinkAccount({ onSuccess: ({ user }) => { - toast.success(`${user.email} linked successfully`); + toast.success(`${user.email?.address} linked successfully`); },
🤖 Fix all issues with AI agents
In `@app/components/KycModal.tsx`:
- Around line 473-479: The Confirm/Proceed button currently enables navigation
to STEPS.CAPTURE based only on selectedCountry, selectedIdType, needsDocCapture,
and idNumber; add validation to ensure smileIdLoaded is true before allowing the
transition. Update the button's disabled condition to include !smileIdLoaded and
guard the onClick handler that calls setStep(STEPS.CAPTURE) to no-op (or show an
inline error/toast) when smileIdLoaded is false; reference the same state
variables (selectedCountry, selectedIdType, needsDocCapture, idNumber,
smileIdLoaded) and the setStep/STEPS.CAPTURE call so the capture step only
proceeds when SmileID components are loaded.
🧹 Nitpick comments (4)
app/api/kyc/tier3-verify/route.ts (1)
76-78: Prefer MIME-derived extension over filename.
Line 76-78 trustsfile.nameforext; using the MIME map avoids spoofed extensions and keeps storage consistent.♻️ Suggested tweak
- const nameExt = file.name?.split(".").pop(); - const ext = (nameExt && nameExt.length <= 4 ? nameExt : MIME_TO_EXT[mime]) || "jpg"; + const ext = MIME_TO_EXT[mime] || "jpg";app/api/kyc/smile-id/route.ts (1)
53-57: Consider replacinganywith a more specific type for SmileID responses.The local
SmileIdResultTypeusesanyforid_infoand[key: string]: any. While this works, it reduces type safety. If SmileID provides response types or you have a known schema, consider using those.app/components/ProfileDrawer.tsx (2)
77-84: Handle clipboard API errors gracefully.
navigator.clipboard.writeTextcan fail in some environments. Consider adding error handling to provide user feedback.Proposed fix
const handleCopyAddress = () => { - if (walletAddress) { - navigator.clipboard.writeText(walletAddress); - setIsAddressCopied(true); - toast.success("Address copied to clipboard"); - setTimeout(() => setIsAddressCopied(false), 2000); + 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"); + }); } };
322-325: Wallet address row renders placeholder whenwalletAddressis undefined.When
walletAddressis undefined,shortenAddress(walletAddress ?? "", 10)is called with an empty string, which may produce an odd placeholder. Consider hiding the wallet row when no address exists.Proposed fix
- {/* Wallet Address */} - <div className="flex w-full items-center justify-between"> - <p className="text-sm font-light text-text-body dark:text-white/70"> - {shortenAddress(walletAddress ?? "", 10)} - </p> + {/* Wallet Address */} + {walletAddress && ( + <div className="flex w-full items-center justify-between"> + <p className="text-sm font-light text-text-body dark:text-white/70"> + {shortenAddress(walletAddress, 10)} + </p>
| disabled={ | ||
| !selectedCountry || | ||
| !selectedIdType || | ||
| (!needsDocCapture && !idNumber) | ||
| } | ||
| onClick={() => setStep(STEPS.CAPTURE)} | ||
| > |
There was a problem hiding this comment.
Add validation that SmileID components have loaded before proceeding to CAPTURE.
The button at line 478 proceeds to STEPS.CAPTURE without checking if smileIdLoaded is true. If the SmileID components failed to load, the user will see a broken capture screen.
Proposed fix
disabled={
!selectedCountry ||
!selectedIdType ||
- (!needsDocCapture && !idNumber)
+ (!needsDocCapture && !idNumber) ||
+ !smileIdLoaded
}
- onClick={() => setStep(STEPS.CAPTURE)}
+ onClick={() => {
+ if (!smileIdLoaded) {
+ toast.error("Verification component not ready. Please try again.");
+ return;
+ }
+ setStep(STEPS.CAPTURE);
+ }}🤖 Prompt for AI Agents
In `@app/components/KycModal.tsx` around lines 473 - 479, The Confirm/Proceed
button currently enables navigation to STEPS.CAPTURE based only on
selectedCountry, selectedIdType, needsDocCapture, and idNumber; add validation
to ensure smileIdLoaded is true before allowing the transition. Update the
button's disabled condition to include !smileIdLoaded and guard the onClick
handler that calls setStep(STEPS.CAPTURE) to no-op (or show an inline
error/toast) when smileIdLoaded is false; reference the same state variables
(selectedCountry, selectedIdType, needsDocCapture, idNumber, smileIdLoaded) and
the setStep/STEPS.CAPTURE call so the capture step only proceeds when SmileID
components are loaded.
1 participant
Description
This pull request introduces a comprehensive KYC (Know Your Customer) verification flow, adding new API endpoints for ID, address, and status checks, as well as transaction summaries. It also updates environment configuration to support multiple verification services. The changes enable tiered KYC verification, integrate SmileID and Dojah for ID and address checks, and provide analytics tracking and error handling throughout the process.
KYC Verification APIs and Flow
app/api/kyc/smile-id/route.tsfor submitting SmileID biometric/enhanced ID verification jobs, validating input, updating user KYC profiles in Supabase, and handling verification results and errors.app/api/kyc/tier3-verify/route.tsfor Tier 3 address/proof-of-address verification using Dojah, including file upload to Supabase Storage, document verification, and KYC profile updates.app/api/kyc/status/route.tsto check KYC status, including phone and SmileID verification, returning tier and verification flags for the user.app/api/kyc/transaction-summary/route.tsto provide daily and monthly transaction summaries for a wallet, supporting compliance and limit checks.API Integration and Aggregator Updates
submitSmileIDDatafunction inapp/api/aggregator.tsto call the SmileID KYC API, track analytics, and handle responses and errors. Also importedSmileIDSubmissionResponsetype for type safety. [1] [2]Environment Configuration
.env.exampleto include configuration variables for KudiSMS, Twilio, SmileID, Dojah, and Supabase Storage bucket, supporting phone, ID, and address verification services.References
closes #281
closes #282
closes #283
closes #288
closes #303
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
Updates
Chores