Fix/remove duplicate stellar sdk#294
Merged
Junman140 merged 83 commits intoApr 29, 2026
Merged
Conversation
…xit (Pi-Defi-world#220) - Add BackButton component with fallback-safe navigation pattern - Component uses router.back() only when in-app history exists - Falls back to explicit in-app route when arriving via direct URL/external link - SSR-safe with window access guard - Audit confirms 0 router.back() calls exist in codebase - All existing back navigation already uses explicit Link components
…EMENT docs Closes Pi-Defi-world#228 - Add MetricLabel component with Info icon tooltip on each health metric - Metrics covered: Total reserves, Total ACBU supply, Collateral ratio, Health - Per-currency metrics: Balance, USD value, Target weight, Actual weight - Each tooltip links to /docs/RESERVE_MANAGEMENT for deeper reading - Addresses F-057: users misinterpreting health metrics on reserves page
…p-passcode-enforcement Harden signup passcode policy and add client-side strength meter
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
- Remove API key storage from sessionStorage (XSS vulnerability) - Implement httpOnly cookie-based authentication - Move passcode to secure in-memory storage - Add Content Security Policy headers via middleware - Add security headers (X-Frame-Options, X-Content-Type-Options, etc.) BREAKING CHANGE: Users will need to re-authenticate after deployment. Backend must set httpOnly session cookies on login. Fixes critical security vulnerability where long-lived API keys stored in sessionStorage could be exfiltrated via XSS attacks. Files changed: - contexts/auth-context.tsx: Remove API key state - lib/api/client.ts: Remove Bearer token headers - app/auth/signin/page.tsx: Update login flow - app/auth/2fa/page.tsx: Update 2FA flow - lib/passcode-manager.ts: NEW - In-memory passcode storage - lib/wallet-storage.ts: Use in-memory passcode - middleware.ts: NEW - CSP and security headers - SECURITY_FIX_SUMMARY.md: Complete documentation - DEPLOYMENT_CHECKLIST.md: Deployment guide
- Create new Help Center page with FAQ section (top 10 questions) - Add system status banner with link to status page - Add contact support form for ticket submission - Add quick links to documentation, status, and community - Add Help Center link to Me page navigation Addresses low severity issue: Help page too thin Impact: Reduces support load by enabling self-service for common questions Features: - FAQ accordion with 10 most common questions - Contact form with name, email, subject, and message fields - Status page integration - Links to external resources (docs, community) - Security contact information for urgent issues - Mobile-responsive design Acceptance criteria met: Users can self-serve top 10 questions via FAQ Status page link for service uptime Ticket form for support requests Quick access from Me page
- Remove token-based auth remnants - Fix passcode storage shadowing in signin - Add session validation on page load - Tighten CSP for production - Replace plaintext wallet storage with encryption - Add 2FA passcode validation guard
- Store passcode in memory instead of sessionStorage - Fix passcode storage before 2FA redirect - Remove API key from sessionStorage - Use secure in-memory passcode manager
- Fix stale comment in wallet-setup-modal - Add passcode guard in wallet-setup page to prevent dead-end state - Improve wallet page error handling with logout and redirect - Fix double handler registration in auth context with cleanup
Distinguish between 401 (invalid session) and network errors. Only clear auth state on 401, preserve it on transient failures.
- Remove legacy stellar-sdk@^13.3.0 from dependencies - Add pnpm override to force all transitive deps to @stellar/stellar-sdk@^15.0.1 - Add peerDependencyRules to silence the trezor plugin peer warning - Lockfile now resolves a single stellar-sdk version (15.0.1) Closes Pi-Defi-world#181
- Add KycBadge component driven by user.kyc_status from the existing /users/me endpoint (UserMe already exposes kyc_status) - Four status variants: verified/approved → green, pending/under_review/ submitted → yellow, rejected/failed → red, default → grey 'KYC Required' - Badge shows animated skeleton while profile data is loading - No extra API call needed — kyc_status is already returned by getMe() Closes Pi-Defi-world#200
… change confirmation
…pi-key-from-sessionstorage fix: remove API key from sessionStorage to prevent XSS attacks
…rehensive-help-page feat: add comprehensive help center page
…uccess-message fix: improve signin security and passcode handling
…e-stellar-sdk fix(deps): remove duplicate stellar-sdk, keep only @stellar/stellar-sdk
…ge-dynamic fix(me): bind KYC badge to live kyc_status — F-029
…rn-tab-real-flow fix(mint): route burn tab to /burn with prefilled state — F-023
…ings-withdraw-recipient-editable Feature/f 027 savings withdraw recipient editable
…-ctas-inert F-017: Fix SME CTA route and activate application mailto flow
…avings-mock-data fix: remove mock savings account constants, derive balance from API only
…urity-settings-placeholder F-022: Add security settings management UI with sessions and API keys
…avings-deposit-handler fix: wire savings deposit dialog to API and show pending/completed st…
…ce-middleware fix(security): add strict CSP via nonce middleware — F-064
- Remove legacy stellar-sdk@^13.3.0 from dependencies - Add pnpm.overrides to force all transitive deps to @stellar/stellar-sdk@^15.0.1 - Add peerDependencyRules entry to silence the trezor plugin peer warning - Lockfile now resolves a single stellar-sdk version (15.0.1) Closes Pi-Defi-world#181
Contributor
📝 WalkthroughWalkthroughThis PR implements comprehensive authentication hardening and security infrastructure updates. It removes sessionStorage-based API keys and passcodes, switches to httpOnly cookie-based authentication, introduces an in-memory passcode manager, adds application-wide error reporting and handling, centralizes API error state management, and adds features for lending and help center support. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (15)
app/wallet/page.tsx (1)
69-86:⚠️ Potential issue | 🟠 MajorReorder: persist the encrypted secret only after the backend accepts the new wallet address.
Right now the encrypted secret is written to local storage on line 73 before
putWalletAddressruns on line 76. If the PUT fails (network error, 4xx, or backend mismatch onresult.ok), thethrowat line 78 leaves a local key for an address the backend doesn't know about — exactly the divergence thatcomponents/wallet-setup-modal.tsx:71-81explicitly warns against and is structured to avoid (PUT → store → confirm).The same ordering issue exists in
handleImportSeed(lines 122 → 125).🐛 Proposed fix (handleGenerateConfirm)
const kp = Keypair.fromSecret(passphrase); const newAddress = kp.publicKey(); - // Store encrypted with passcode (secure) - await storeWalletSecret(userId, passphrase, passcode); - // Sync public key to backend. const result = await userApi.putWalletAddress(newAddress, opts); if (!result?.ok) { throw new Error("Backend did not accept the new wallet address. Please retry."); } + // Backend accepted the address — now persist the encrypted secret. + await storeWalletSecret(userId, passphrase, passcode); + // Confirm wallet activation on backend try { await userApi.postWalletConfirm({ wallet_address: newAddress }, opts);Apply the same swap to
handleImportSeed(move thestoreWalletSecret(userId, importSeed, passcode)call to after theputWalletAddressok-check).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/wallet/page.tsx` around lines 69 - 86, The code stores the encrypted secret (storeWalletSecret) before the backend accepts the new wallet address, which can leave a local key for an unknown address; update both handleGenerateConfirm and handleImportSeed so they first call userApi.putWalletAddress(newAddress/...) and verify result?.ok, only then call storeWalletSecret(userId, passphrase/importSeed, passcode), and finally call userApi.postWalletConfirm(...) (preserving the existing try/catch around confirm). Ensure you throw on non-ok PUT results before calling storeWalletSecret so local persistence only happens after a successful backend acceptance.app/auth/signin/page.tsx (2)
84-88:⚠️ Potential issue | 🟡 MinorClear in-memory passcode on signin failure.
If
authApi.signinsucceeds (sostorePasscode(passcode)runs on the 2FA branch at line 60) but a subsequent step throws — e.g.,result.challenge_tokenis missing,sessionStorage.setItemfails, orrouter.pushrejects — the catch handler surfaces the error but leaves the passcode lingering in memory tied to a session that never completed. Recommend callingclearPasscode()(and removing2fa_challenge_token) in thecatchto keep state consistent with a failed login.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/auth/signin/page.tsx` around lines 84 - 88, When signin fails after a successful authApi.signin that previously called storePasscode(passcode), ensure the catch block in page.tsx clears that transient state by invoking clearPasscode() and removing the stored 2FA token (e.g., sessionStorage.removeItem("2fa_challenge_token") or the equivalent token-clearing helper) before setting the error; this guarantees any in-memory passcode and leftover challenge token are cleaned up when the login flow aborts.
77-82:⚠️ Potential issue | 🟠 MajorWallet passphrase still persisted in
sessionStorage— undermines the in-memory passcode story.Lines 60 and 73 deliberately move the passcode from
sessionStorageto in-memory storage to reduce XSS exposure. But on the wallet-creation branch the freshly-issuedresult.passphrase(the Stellar wallet seed) is written straight tosessionStorage, where any XSS payload can read it for the lifetime of the tab — and the passphrase is far more sensitive than the passcode (full custody of the wallet).If the goal is to harden secrets handling, the passphrase should ride the same in-memory channel as the passcode (e.g. a sibling helper in
lib/passcode-manager.tsor a dedicatedwallet-bootstrapstore) and be consumed byapp/auth/wallet-setup/page.tsxfrom memory. As written, the threat model improvement is only partial.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/auth/signin/page.tsx` around lines 77 - 82, The code writes the newly-issued wallet seed (result.passphrase) into sessionStorage (sessionStorage.setItem) which defeats the in-memory passcode model; instead stop persisting the passphrase to sessionStorage in the wallet-creation branch and store it in the same in-memory manager used for the passcode (e.g. add a setTempPassphrase/getTempPassphrase API to lib/passcode-manager.ts or a dedicated wallet-bootstrap store), then navigate to wallet-setup (router.push('/auth/wallet-setup')) where app/auth/wallet-setup/page.tsx reads and consumes the passphrase from that in-memory API (and ensure it is cleared after consumption); remove the sessionStorage.setItem call and any fallbacks that persist the seed to browser storage.app/mint/page.tsx (1)
328-403:⚠️ Potential issue | 🟡 MinorDead code —
handleExecuteBurnis no longer reachable.Now that
handleBurnConfirmdeep-links to/burnandhandleExecuteonly dispatches onactiveTab === "mint", this entire ~75-line burn execution path is unreachable (ESLint also flags it:'handleExecuteBurn' is assigned a value but never used). Removing it will also let you drop thesubmitBurnRedeemSingleClientimport (used only here) and shrink the bundle. A bonus: the "Confirm Burn" branch in the success/confirm<AlertDialog>(lines 705-718, 753-756) becomes dead too —steponly flips to"confirm"on mint now — so the dialog can be simplified to mint-only copy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/mint/page.tsx` around lines 328 - 403, The handleExecuteBurn function (and its import usage) is dead code because burning is no longer reachable; remove the entire handleExecuteBurn function definition and any references to it, delete the now-unused submitBurnRedeemSingleClient import, and simplify the success/confirm AlertDialog by removing the "Confirm Burn" branch and any logic that sets step to "confirm" for burn flows (leave only mint-related confirm/success logic). Ensure you also remove any state or helpers used exclusively by handleExecuteBurn (e.g., burn-specific setTxId/setBurnApiError usage if they become unused) to avoid leftover unused variables and ESLint warnings.pr.md (1)
1-199:⚠️ Potential issue | 🟠 MajorDelete
pr.md— appears to be an accidentally committed PR description, and for the wrong PR.Two problems:
- Wrong content. This file is a pull-request description for "fix(currency): replace mock balance/rates with real API + add toasts" closing
#190. PR#294is "Fix/remove duplicate stellar sdk" closing#181. None of the changes described here (app/currency/page.tsx,<Toaster />mount inapp/layout.tsx,useBalance()rewiring, etc.) are part of the file set under review.- Wrong location. PR descriptions belong in the GitHub PR body, not committed to the repo. As a tracked file, it will pollute search results, drift out of sync with reality, and ship with every release artifact.
Recommend
git rm pr.mdand pasting the relevant prose into the PR body on GitHub instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pr.md` around lines 1 - 199, pr.md is an accidental committed PR description that doesn't belong in the repo; remove the tracked file by running git rm pr.md, commit the removal (e.g., "chore: remove accidental pr.md"), and push the change so it no longer ships or pollutes search results; then copy any useful text into the GitHub PR body (not the repository) and ensure no other commits reintroduce a PR body file.app/savings/page.tsx (2)
178-197:⚠️ Potential issue | 🟠 Major"Total Savings" card now duplicates "Savings balance (API)".
After the change, both the "Savings balance (API)" card (lines 160-176) and the "Total Savings" card (lines 178-197) render the same
apiBalancevalue (totalSavings = apiBalanceon line 135). The only differences are:
- A still-hardcoded "Earning 8% APY interest" caption.
- A still-hardcoded
(totalSavings * 0.08) / 12"+this month" line that has no relation to any backend rate.A previously-retrieved learning notes that the "Total Savings" card was intentional mock/placeholder data on the route-grouped variant of this page. Now that it shows the real balance alongside another card showing the same real balance, the duplication is user-visible and the 8% APY copy is misleading. Either:
- delete this card entirely (was that the intent?), or
- replace 8% APY + "+this month" with values sourced from the savings API.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/savings/page.tsx` around lines 178 - 197, The "Total Savings" Card duplicates the "Savings balance (API)" by showing totalSavings (which equals apiBalance) and uses a hardcoded 8% APY and derived monthly amount; either remove the Card or wire it to real API fields—update the Card that renders totalSavings/positionsLoading/formatAmount to instead read the savings APY and monthly interest from the API response (e.g., use the API field name such as savings.apy or savingsRate if available), compute monthlyInterest = totalSavings * apy / 12, replace the hardcoded "Earning 8% APY interest" and the hardcoded +this month line with values derived from that apy and monthlyInterest, or delete the Card entirely if duplication was intended.
51-59:⚠️ Potential issue | 🔴 CriticalRemove the dead
SavingsAccountinterface with unimportedLucideIcontype reference.The
SavingsAccountinterface (lines 51–59) is completely unused throughout the codebase and referencesLucideIconwhich is not imported in this file. Since the entire Savings Accounts UI was removed, this interface should be deleted.Additionally, the "Total Savings" card (lines 178–197) duplicates the "Savings balance (API)" card immediately above it—both display the same
positionsBalancevalue. Consider removing the duplicate card or clarifying its purpose.♻️ Suggested change
-interface SavingsAccount { - id: string; - name: string; - apy: number; - balance: number; - icon: LucideIcon; - description: string; - color: string; -} - interface SavingsGoal {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/savings/page.tsx` around lines 51 - 59, Remove the dead SavingsAccount interface (named SavingsAccount) that references the unimported LucideIcon type; delete the entire interface declaration and any unused imports related to LucideIcon. Also remove or consolidate the duplicate UI card that displays positionsBalance: locate the two cards labeled "Savings balance (API)" and "Total Savings" (both rendering positionsBalance) and either delete the "Total Savings" card or change its data/label so it is not a duplicate (ensure only one card shows positionsBalance or that each card displays distinct computed values).app/savings/withdraw/page.tsx (1)
1-285:⚠️ Potential issue | 🔴 CriticalCritical: this file is broken — multiple undefined references, missing imports, and dead refactor remnants. Will not compile.
Going through the actual identifiers used:
- Line 48:
RecipientResponseis referenced as a type but is not imported in this file. Addimport type { RecipientResponse } from "@/types/api";.- Line 76:
setUser(resolved)— there is nouser/setUserstate in this file. Lines 217 and 276 also read a non-existentuser(and calluser.trim()), so the form will throwReferenceErrorat render time. Either rename tohomeRecipient/setHomeRecipientconsistently, or reintroduce auserstate.- Line 100:
recipientApi.resolveRecipient(...)— the import on line 15 isimport { resolveRecipient } from "@/lib/api/recipient";. There is norecipientApinamespace import, so this throwsReferenceErrorthe first timevalidateRecipientruns.- Line 151:
setApiError(e)—useApiErroris imported (line 11) but never called/destructured.setApiErroris undefined here. AlsoApiErrorDisplayis imported but never rendered (the catch block falls back to a manual destructive<div>at lines 188-193 keyed off the legacyerrorstring state).- Line 39 / 53:
setHomeRecipientis declared but never called, sohomeRecipientstays"";isRecipientChanged = recipient.trim() !== homeRecipient.trim()then becomestrueas soon as the user types anything, which forces the "confirm different recipient" checkbox / submission gating on every flow — including the "default" one.- Lines 49–50, 94–114, 157–172:
recipientValidationLoading,recipientValidationError,validateRecipient, andhandleRecipientChangeare all defined but never wired to the recipient<Input>(which itself is missing entirely from the JSX — only the read-onlyid="withdraw-account"Input bound to the brokenuserexists). The "Change/Reset" button toggleseditingRecipientbut the rest of the JSX never branches on it.In its current state the page won't render and the "edit recipient" feature is unreachable. Suggest either:
- Reverting the recipient-edit feature for now and shipping only the
setApiError/ApiErrorDisplaymigration; or- Completing the migration: drop the legacy
user/setUserreferences, add the missing recipient<Input>and confirmation checkbox bound torecipient/handleRecipientChange, destructureuseApiError(), render<ApiErrorDisplay />instead of the manual destructive<div>at 188–193, and importRecipientResponse.I'd recommend (1) for this PR since the recipient-validation flow has no UI, then a follow-up PR to fully add the editable recipient experience.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/savings/withdraw/page.tsx` around lines 1 - 285, This file contains unfinished recipient-edit refactor bits and undefined identifiers; revert the incomplete edit flow for this PR: remove or ignore recipient-edit related state and helpers (recipient, setRecipient, editingRecipient, resolvedRecipient, recipientValidationLoading, recipientValidationError, confirmDifferentRecipient, validateRecipient, handleRecipientChange, handleToggleEdit), ensure the initial receive address is saved via setHomeRecipient (replace the incorrect setUser(resolved) call in the useEffect with setHomeRecipient(resolved) and use homeRecipient everywhere instead of user), destructure useApiError to get setApiError and call setApiError(e) in handleSubmit catch, replace the manual destructive error block with rendering <ApiErrorDisplay /> using the api error state, and remove or stop referencing the nonexistent recipientApi and RecipientResponse types for now; this yields a compileable page and allows reintroducing the full recipient-edit/validation flow in a follow-up.app/burn/page.tsx (1)
70-368:⚠️ Potential issue | 🔴 CriticalCritical: file is in an inconsistent half-migrated state and will not compile/run.
This file mixes the old
useState-based form with a partialreact-hook-formmigration, leaving multiple undefined references and a broken JSX structure. Concretely:
- Line 117–122, 136–137, 169–170, 177–178, 184:
values.*is referenced butvaluesis never defined inhandleSubmit's scope (the function signature is(e: React.FormEvent)). These would throwReferenceErrorat runtime.- Line 230:
<form onSubmit={formHandleSubmit(onSubmit)}>— neitherformHandleSubmitnoronSubmitexist; this matches the Biome parse errors.- Line 360: stray
</div>with no matching opening<div>— the JSX tree is unbalanced (Biome reports missing closing tags for<Form>/<form>).- Lines 75–104 still maintain the old per-field
useStateand theisValidderivation, but the inputs now bind toform.controlonly — so the old state is dead/contradictory andisValidno longer reflects what the user typed.The submit path needs to be a single
react-hook-formflow, e.g.:🔧 Sketch of the corrected wiring
- const isValid = ... old logic ... - - const handleSubmit = async (e: React.FormEvent) => { - e.preventDefault(); - if (!isValid) return; - ... - const recipientAccount: BurnRecipientAccount = { - account_number: values.accountNumber.trim(), - ... - }; - ... - }; + const onSubmit = async (values: BurnFormValues) => { + clearError(); + setLoading(true); + setTxId(null); + try { + if (!userId) throw new Error("Not signed in"); + if (!stellarAddress) throw new Error("No linked Stellar wallet address."); + const recipientAccount: BurnRecipientAccount = { + account_number: values.accountNumber.trim(), + bank_code: values.bankCode.trim(), + account_name: values.accountName.trim(), + type: "bank", + }; + // ... rest using `values.acbuAmount` / `values.currency` + form.reset({ ...values, acbuAmount: "" }); + } catch (e) { + setApiError(e); + } finally { + setLoading(false); + } + }; @@ - <Form {...form}> - <form onSubmit={formHandleSubmit(onSubmit)} className="space-y-4"> + <Form {...form}> + <form onSubmit={form.handleSubmit(onSubmit)} className="space-y-4"> @@ - /> - </div> - <Button + /> + <Button type="submit" - disabled={!isValid || loading || isSubmitDisabled} + disabled={!form.formState.isValid || loading || isSubmitDisabled}Also drop the now-dead per-field
useStatedeclarations and the oldisValid.Additionally, the duplicate
txIdblocks at 214–218 and 220–227 render the transaction ID twice (plain paragraph + success banner) — the plain-text version on 214–218 should be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/burn/page.tsx` around lines 70 - 368, This file mixes old useState form bits with react-hook-form causing undefined references and broken JSX; remove the per-field useState declarations (acbuAmount, currency, accountNumber, bankCode, accountName) and the old isValid logic, then replace the current handleSubmit(e) with a react-hook-form onSubmit that accepts values (e.g., const onSubmit = async (values: BurnFormValues) => { ... }); inside onSubmit use values.accountNumber, values.bankCode, values.accountName, values.acbuAmount and values.currency, keep the existing secret/kit/burn flow (submitBurnRedeemSingleClient, getWalletSecretAnyLocal, burnApi.burnAcbu) but reference form.reset and form.formState.isValid for disabling submit instead of the old isValid; change the JSX form tag to use form.handleSubmit(onSubmit) (not formHandleSubmit/onSubmit mismatch), remove the duplicate plain txId paragraph so only the success banner remains, and fix the unbalanced JSX by ensuring every opened <Form> and wrapper <div> is properly closed around the <form> block.lib/api/client.ts (2)
93-129:⚠️ Potential issue | 🟠 MajorAbortSignal handling: pre-aborted check missing, and listener leaks for long-lived caller signals.
Two related issues in the new abort/timeout block:
- Pre-aborted signal is ignored. If a caller passes an
opts.signalthat is already aborted, line 100 attaches an'abort'listener that will never fire (the event already happened), and the request still hits the network. Fetch will then be issued with the controller's non-aborted signal. The fix is to abort the local controller immediately:if (opts.signal) { if (opts.signal.aborted) { controller.abort(); } else { opts.signal.addEventListener('abort', () => controller.abort(), { once: true }); } }- Listener leak on long-lived caller signals.
addEventListener('abort', ..., { once: true })is only auto-removed when the event fires. Ifopts.signalbelongs to a component-scoped or app-scopedAbortController(the typical case), every completed request leaves a stale listener attached until that signal eventually aborts. Over a long session this accumulates. Remove the listener when the request resolves:const onCallerAbort = () => controller.abort(); if (opts.signal) { if (opts.signal.aborted) controller.abort(); else opts.signal.addEventListener('abort', onCallerAbort, { once: true }); } try { // ... fetch } finally { clearTimeout(timeoutId); opts.signal?.removeEventListener('abort', onCallerAbort); }This also lets you collapse the duplicated
clearTimeoutcalls (lines 119 and 129) into a singlefinallyblock.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/api/client.ts` around lines 93 - 129, Handle pre-aborted and leaking caller signals: if opts.signal exists, immediately abort the local AbortController when opts.signal.aborted is true instead of only attaching a listener; otherwise attach a named handler (e.g., onCallerAbort = () => controller.abort()) to opts.signal and ensure you remove that listener after the request completes (opts.signal.removeEventListener('abort', onCallerAbort)). Move clearTimeout(timeoutId) into a single finally block that also removes the caller listener to avoid stale handlers, and keep using the local controller.signal for fetch; update references to controller, signal, opts.signal, timeoutId and DEFAULT_TIMEOUT accordingly.
56-60:⚠️ Potential issue | 🟡 MinorBackend must set
XSRF-TOKENcookie withouthttpOnlyflag — the frontend reads this token viadocument.cookie(line 58) and sends it asX-XSRF-TOKENheader for CSRF protection. If the backend incorrectly setshttpOnly=true, CSRF protection silently degrades without visible errors.Recommended XSRF-TOKEN cookie flags:
httpOnly=false(must be JavaScript-readable)secure=truesameSite=strictorlaxWhile
SECURITY_FIX_SUMMARY.mddocuments that the session/auth cookie must behttpOnly=true, the XSRF-TOKEN cookie configuration is not explicitly documented. Add explicit backend requirements to clarify this distinction to prevent accidental misconfigurations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/api/client.ts` around lines 56 - 60, The frontend's getCsrfToken function reads the XSRF-TOKEN cookie (match for "XSRF-TOKEN" and then sent as X-XSRF-TOKEN header), so ensure the backend explicitly sets the XSRF-TOKEN cookie with httpOnly=false (so document.cookie can read it), secure=true, and sameSite=lax or strict; update backend cookie-setting logic and add clear documentation (e.g., in SECURITY_FIX_SUMMARY.md or backend auth config) that session/auth cookies remain httpOnly=true while XSRF-TOKEN must be non-httpOnly and JavaScript-readable to preserve CSRF protection.app/currency/page.tsx (2)
658-676:⚠️ Potential issue | 🟡 MinorConfirmation dialog: passing raw string amounts to
formatAmountand not showing the computed local amount for burn/international.Two small UX issues in the confirm dialog:
formatAmount(burnAmount)/formatAmount(intlAmount)(lines 659, 661, 669, 670) pass user-typed strings; if the user typed"10."or"10.5e2"the formatted output may surprise. Prefer the already-coercedburnNumeric/intlNumeric/mintNumeric.- The confirm dialog still says "Calculated by backend" / "Processing fee" without surfacing the computed
estimatedBurnNgnorestimatedIntlLocalyou already have in scope — the user just clicked through a screen showing "₦X" but the confirm screen hides it. Consider mirroring those estimates here so the confirmation matches the input screen.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/currency/page.tsx` around lines 658 - 676, The confirm dialog is formatting raw string inputs (formatAmount(burnAmount)/formatAmount(intlAmount)) and hiding backend-computed local estimates; change uses of formatAmount to accept the already-coerced numeric values (burnNumeric, intlNumeric, mintNumeric) instead of the raw burnAmount/intlAmount strings, and replace the static "Calculated by backend" processing fee row with the corresponding computed estimates (estimatedBurnNgn and estimatedIntlLocal) so the confirmation dialog mirrors the amounts shown earlier; update references around AlertDialogDescription / the "Amount:" and "Processing fee:" spans to display these numeric/coerced values and the estimatedBurnNgn or estimatedIntlLocal when activeTab is 'burn' or 'international'.
80-200:⚠️ Potential issue | 🔴 CriticalCritical:
balance,balanceLoading,refreshBalance, andtoastare referenced but never declared/imported in this file.After the refactor, the component body uses several identifiers that have no corresponding hook call or import:
balance,balanceLoading— used at lines 119, 235, 240, 242, 408, 412, 485 with nouseBalance()call and no import.refreshBalance()— called at line 191 after a successful submit.toast({...})— called at lines 144–147, 168–171, 185–188; there is nouseToast()call and notoastimport.The page will throw a
ReferenceError(or TS compile error in strict mode) on render andhandleExecutewill throw beforesetStep("success")runs, so users will see a stale error and the success dialog will never open.🐛 Likely fix — restore the missing hooks and imports
import { useApiOpts } from "@/hooks/use-api"; import { useApiError } from "@/hooks/use-api-error"; import { ApiErrorDisplay } from "@/components/ui/api-error-display"; +import { useBalance } from "@/hooks/use-balance"; +import { useToast } from "@/hooks/use-toast"; import * as mintApi from "@/lib/api/mint"; @@ export default function CurrencyPage() { const opts = useApiOpts(); const { uiError, setApiError, clearError, isSubmitDisabled } = useApiError(); + const { balance, loading: balanceLoading, refresh: refreshBalance } = useBalance(); + const { toast } = useToast();Adjust to whatever the actual hook names/exports are in this codebase.
#!/bin/bash # Confirm none of these identifiers are declared in app/currency/page.tsx. rg -n -C1 -P '\b(balance|balanceLoading|refreshBalance|toast|useToast|useBalance)\b' app/currency/page.tsx # Confirm the hook signatures we'd be importing. fd -t f use-balance hooks fd -t f use-toast hooks components ast-grep --pattern 'export function useBalance($$$) { $$$ }' ast-grep --pattern 'export function useToast($$$) { $$$ }'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/currency/page.tsx` around lines 80 - 200, The file is referencing balance, balanceLoading, refreshBalance and toast but never imports or calls the hooks that provide them; add the appropriate hook imports (e.g. import { useBalance } and import { useToast }) and call them at the top of the component to expose those identifiers (for example call const { balance, balanceLoading, refreshBalance } = useBalance(/*args*/); and const toast = useToast();), then use those values in handleExecute and throughout the component where balance, balanceLoading, refreshBalance, and toast are used so the references resolve and refreshBalance() and toast(...) work as intended.app/send/page.tsx (1)
152-242:⚠️ Potential issue | 🔴 CriticalStale closure:
confirmedAmountis missing fromhandleConfirmTransfer's dependency array.
handleConfirmTransferreadsconfirmedAmountat lines 176, 209, 217, and 225, and writes it tosetLastSentAmountat line 225, butconfirmedAmountis not in the deps array (line 242).Repro: user types amount →
setConfirmedAmount(amount)runs on Continue → React re-renders, but becauseamountdidn't change again between the previous render and now,useCallbackreturns the previously memoized closure whereconfirmedAmountis still"". Clicking "Send ACBU" then submitsamount_acbu: ""totransfersApi.createTransferand signs a Stellar payment for"". Best case: backend 400. Worst case: the Stellar SDK interprets it as 0 and signs a no-op transaction, but the user sees "Transfer Sent!" anyway.Also worth adding to deps:
clearErrorandsetApiError(custom-hook setters that aren't necessarily stable across renders).🐛 Proposed fix
- }, [amount, getToValue, note, userId, stellarAddress, kit, opts, loadTransfers, refreshBalance]); + }, [ + amount, + confirmedAmount, + getToValue, + note, + userId, + stellarAddress, + kit, + opts, + loadTransfers, + refreshBalance, + clearError, + setApiError, + ]);If
useApiError's setters are guaranteed stable (returned fromuseCallback/useMemo), they can be omitted, but lock that down with a comment in the hook so future refactors don't silently break this callback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/send/page.tsx` around lines 152 - 242, handleConfirmTransfer closes over confirmedAmount (and uses clearError and setApiError) but doesn't include them in its useCallback dependency array, which can cause stale values to be submitted; update the deps for useCallback that defines handleConfirmTransfer to include confirmedAmount, clearError, and setApiError (or ensure those setters are stable and document that in useApiError) so the latest confirmedAmount and error handlers are captured when creating transfers and setting API errors.SECURITY_FIX_SUMMARY.md (1)
1-122:⚠️ Potential issue | 🟠 MajorPR scope mismatch with stated objectives.
This PR's title is "Fix/remove duplicate stellar sdk" and it claims to close issue
#181(duplicate@stellar/stellar-sdkandstellar-sdk). However, this file (and the entire diff under review) documents and implements a large auth/security hardening effort — httpOnly cookie auth, in-memory passcode manager, CSP middleware, error-reporting infrastructure, lending admin page, etc. None of the reviewed files touchpackage.json,pnpm-lock.yaml, or stellar-sdk imports.Either:
- The PR title/description is wrong (these security changes belong to a different ticket and need their own description, security review, and acceptance criteria), or
- The dedup commits were rebased into the wrong branch and the security changes here are unintended for this PR.
Please reconcile before merging — reviewers/deployers will read "remove duplicate stellar-sdk" and miss the substantive auth/CSP changes that need careful sign-off.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@SECURITY_FIX_SUMMARY.md` around lines 1 - 122, The PR title/issue (`#181`) and branch appear mismatched: SECURITY_FIX_SUMMARY.md documents large auth/CSP/security changes (files like contexts/auth-context.tsx, lib/api/client.ts, lib/passcode-manager.ts, middleware.ts, lib/wallet-storage.ts) but the PR claims to "remove duplicate stellar-sdk" and contains no package.json/pnpm-lock changes; reconcile by either (A) updating the PR title, description, and issue reference to accurately reflect the security hardening work (list the affected symbols/files in the description and add acceptance criteria from SECURITY_FIX_SUMMARY.md) or (B) remove/revert the security commits from this branch and open a separate PR for the security changes (use git revert/cherry-pick to isolate commits touching contexts/auth-context.tsx, lib/api/client.ts, lib/passcode-manager.ts, middleware.ts, lib/wallet-storage.ts, SECURITY_FIX_SUMMARY.md), then ensure the "remove duplicate stellar-sdk" PR only contains package.json/pnpm-lock and actual import fixes for `@stellar/stellar-sdk` vs stellar-sdk before re-submitting.
🟡 Minor comments (14)
app/business/sme/page.tsx-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorRemove unused
ArrowRightimport.ESLint flags
ArrowRightas imported but never used in this file.🧹 Proposed fix
-import { ArrowLeft, Briefcase, Mail, ArrowRight } from 'lucide-react'; +import { ArrowLeft, Briefcase, Mail } from 'lucide-react';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/business/sme/page.tsx` at line 8, The import list includes an unused symbol ArrowRight which ESLint flags; remove ArrowRight from the named import (the import statement that currently reads something like import { ArrowLeft, Briefcase, Mail, ArrowRight } from 'lucide-react') so only used icons (ArrowLeft, Briefcase, Mail) remain, then run lint to confirm the warning is gone.components/navigation/BackButton.tsx-47-58 (1)
47-58:⚠️ Potential issue | 🟡 MinorRedundant accessible name when no
childrenare provided.The
<button>already hasaria-label={label}, which becomes the accessible name and causes screen readers to ignore the button's inner text content. The<span className="sr-only">{label}</span>on Line 56 therefore contributes nothing for assistive tech in the default (icon-only) case, and when consumers pass customchildrencontaining visible text,aria-labelwill override that visible text as the accessible name — usually the opposite of what's intended (WCAG 2.5.3 "Label in Name").Consider only setting
aria-labelwhen there are no visible-text children, and dropping the redundantsr-onlyspan:♻️ Suggested change
return ( <button onClick={handleBack} className={className} - aria-label={label} + aria-label={children ? undefined : label} type="button" > {children || ( <> <ArrowLeft className="w-5 h-5 text-primary" aria-hidden="true" /> - {label && <span className="sr-only">{label}</span>} </> )} </button> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/navigation/BackButton.tsx` around lines 47 - 58, The BackButton component currently always sets aria-label={label} and also renders a visually hidden <span className="sr-only">{label}</span>, causing redundant/overriding accessible names; update the component so it only applies aria-label when there are no visible-text children (i.e., when children is falsy or only icon), remove the redundant sr-only span from the default rendering, and ensure the button still renders the ArrowLeft icon and respects className and onClick (handleBack) while letting provided children serve as the accessible name when present.HELP_PAGE_SUMMARY.md-158-158 (1)
158-158:⚠️ Potential issue | 🟡 MinorStale/incorrect PR link.
The "PR Link" points to
coderchris1234/acbu-frontendon afeature/comprehensive-help-pagebranch, but this PR is onPi-Defi-world/acbu-frontendfromfix/remove-duplicate-stellar-sdk. Update or remove the link to avoid confusion for future readers of this doc.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HELP_PAGE_SUMMARY.md` at line 158, The "PR Link" entry in HELP_PAGE_SUMMARY.md is stale; update or remove it. Either replace the URL under the "**PR Link:**" line with the correct PR for Pi-Defi-world/acbu-frontend from the fix/remove-duplicate-stellar-sdk branch, or remove the PR Link line entirely to avoid confusion; make the change to the "**PR Link:**" line so the document references the correct repository/branch.HELP_PAGE_SUMMARY.md-67-70 (1)
67-70:⚠️ Potential issue | 🟡 MinorAdd a language to the fenced code block (MD040).
Flagged by markdownlint.
📝 Proposed fix
-``` +```text app/help/page.tsx - Main help center page app/me/page.tsx - Updated with help link</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@HELP_PAGE_SUMMARY.mdaround lines 67 - 70, The fenced code block in
HELP_PAGE_SUMMARY.md is missing a language tag (MD040); update the opening fence
fromtotext for the block that contains the lines "app/help/page.tsx
- Main help center page" and "app/me/page.tsx - Updated with help
link" so markdownlint recognizes the language and the rule is satisfied.</details> </blockquote></details> <details> <summary>package.json-88-91 (1)</summary><blockquote> `88-91`: _⚠️ Potential issue_ | _🟡 Minor_ **Remove the `eslint-plugin-react>eslint: "^10"` allowedVersions entry.** eslint-plugin-react@7.37.5 already declares native peer support for `eslint: ^3 || ^4 || ^5 || ^6 || ^7 || ^8 || ^9.7`. Since the installed eslint is pinned to `^9.11.1` (line 73), which falls within `^9.7`, no override rule is needed. The `^10` entry doesn't match the installed version and serves no purpose—it's a leftover from before the eslint downgrade. <details> <summary>Proposed fix</summary> ```diff "peerDependencyRules": { "allowedVersions": { - "eslint-plugin-react>eslint": "^10", "@trezor/connect-plugin-stellar>@stellar/stellar-sdk": "^15.0.1" } } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 88 - 91, Remove the unnecessary allowedVersions override for "eslint-plugin-react>eslint": "^10" from the package.json allowedVersions block; locate the allowedVersions object (key "allowedVersions") and delete the entry with the key "eslint-plugin-react>eslint" so that eslint-plugin-react's native peer range is used (no other changes needed). ``` </details> </blockquote></details> <details> <summary>app/auth/error.tsx-25-29 (1)</summary><blockquote> `25-29`: _⚠️ Potential issue_ | _🟡 Minor_ **Possible recovery loop if the error originates on `/auth/signin` itself.** This boundary covers the entire `app/auth/*` segment, so an error thrown while rendering `/auth/signin` will mount this component, and the "Sign in" button will redirect right back to the same broken route. Consider either pointing the recovery action at a stable landing (e.g. `/`) when `window.location.pathname` already starts with `/auth/signin`, or letting `reset()` be the only retry path for that case. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@app/auth/error.tsx` around lines 25 - 29, handleGoToSignIn currently always sets window.location.href to '/auth/signin' which can cause a recovery redirect loop if the error occurred on /auth/signin; modify handleGoToSignIn to first check window.location.pathname (or startsWith('/auth/signin')) and if already on that path call the provided reset() retry function (or redirect to a stable landing like '/'), otherwise navigate to '/auth/signin'; update references in the error boundary component so the button uses this new logic instead of unconditionally setting window.location.href. ``` </details> </blockquote></details> <details> <summary>lib/error-reporting.ts-13-13 (1)</summary><blockquote> `13-13`: _⚠️ Potential issue_ | _🟡 Minor_ **Replace `any` with `unknown` (or a narrower record type).** ESLint is flagging this. `Record<string, unknown>` is the safer default; consumers that need to index into it can narrow at the call site. <details> <summary>♻️ Diff</summary> ```diff - context?: Record<string, any>; + context?: Record<string, unknown>; ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@lib/error-reporting.ts` at line 13, Replace the permissive type on the context property with a safer type: change the declaration of context (currently "context?: Record<string, any>;") to use Record<string, unknown> (or a narrower record type) in lib/error-reporting.ts; update any call sites that index into context to perform proper type narrowing or type assertions before using properties so consumers can safely access keys. ``` </details> </blockquote></details> <details> <summary>lib/error-reporting.ts-73-82 (1)</summary><blockquote> `73-82`: _⚠️ Potential issue_ | _🟡 Minor_ **`getStoredErrors` doesn't validate that the parsed value is an array.** If `localStorage['app_errors']` is ever corrupted to a non-array (manual edit, version skew, an older entry written by a future change), `JSON.parse` will succeed, the cast to `ErrorReport[]` lies, and the very next call site — `errors.push(report)` at line 59 — throws a `TypeError` on whatever is returned. That throw lands in the outer catch (line 65) and gets logged, but every subsequent error report silently fails the same way until storage is cleared. <details> <summary>♻️ Suggested guard</summary> ```diff - const stored = localStorage.getItem('app_errors'); - return stored ? JSON.parse(stored) : []; + const stored = localStorage.getItem('app_errors'); + if (!stored) return []; + const parsed = JSON.parse(stored); + return Array.isArray(parsed) ? (parsed as ErrorReport[]) : []; ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@lib/error-reporting.ts` around lines 73 - 82, The getStoredErrors function should validate that JSON.parse(localStorage.getItem('app_errors')) returns an array before treating it as ErrorReport[]; replace the blind cast with a check (e.g., if (!Array.isArray(parsed)) return []) and optionally verify elements are objects with the expected keys, returning [] on any validation failure so callers like the errors.push(report) site won't hit a TypeError when storage is corrupted or holds non-array data. ``` </details> </blockquote></details> <details> <summary>app/send/page.test.tsx-1-198 (1)</summary><blockquote> `1-198`: _⚠️ Potential issue_ | _🟡 Minor_ **This `.test.tsx` file contains no executable tests — it exports test source as strings.** Two practical problems: 1. Jest/Vitest will discover this file (via the `.test.tsx` suffix) but find no `test(...)`/`it(...)` calls. Most default configs treat that as a hard failure ("Your test suite must contain at least one test"). At best it's a no-op; at worst it breaks CI. 2. Tests pasted as template literals never actually execute — they will silently rot as `app/send/page.tsx` evolves. Recommendation: either convert these to real RTL tests (one `test(...)` per scenario, importing `SendPage` directly), or move the documentation portion to a `.md`/`README` and delete this file. If you want, I can draft a real `app/send/page.test.tsx` that exercises `confirm-amount` end-to-end. Also, `import React from 'react'` on line 24 is unused (flagged by ESLint). <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@app/send/page.test.tsx` around lines 1 - 198, This file currently exports test strings (TEST_1_CONFIRM_AMOUNT_DISPLAYED, TEST_2_AMOUNT_PRESERVED_AFTER_CANCEL, TEST_3_AMOUNT_CLEARED_AFTER_SUCCESS) and has an unused import React, so CI will fail and the tests won't run; either (A) convert those string fixtures into real RTL tests by replacing the exported template literals with actual test(...) blocks that import SendPage and use `@testing-library/react` and user-event to assert data-testid="confirm-amount" behavior (create tests named to match the TEST_... constants), or (B) move the long documentation block into a markdown/README and delete this .test.tsx so Jest/Vitest no longer discovers it; in both cases remove the unused "import React from 'react'" and ensure at least one real test(...) exists in the final file. ``` </details> </blockquote></details> <details> <summary>app/global-error.tsx-31-35 (1)</summary><blockquote> `31-35`: _⚠️ Potential issue_ | _🟡 Minor_ **"Reload page" button navigates to home instead of reloading.** The button is labeled "Reload page" but `handleReload` sets `window.location.href = '/'`, which navigates away (and discards any URL/query state useful for debugging). If the intent is to truly reload, use `window.location.reload()`. If the intent is to escape to home, rename the button accordingly (e.g., "Go home"). <details> <summary>🔧 Proposed fix</summary> ```diff const handleReload = () => { if (typeof window !== 'undefined') { - window.location.href = '/'; + window.location.reload(); } }; ``` </details> Also applies to: 82-84 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@app/global-error.tsx` around lines 31 - 35, The "Reload page" button handler handleReload currently sets window.location.href = '/' which navigates home instead of reloading; change handleReload to call window.location.reload() so the button truly reloads the current page (and update any duplicate implementation at the other occurrence around the same file), or if you intend to go home instead, rename the button label to "Go home" and keep the navigation behavior—adjust both handleReload implementations and the corresponding button text to be consistent. ``` </details> </blockquote></details> <details> <summary>components/ui/api-error-display.tsx-29-46 (1)</summary><blockquote> `29-46`: _⚠️ Potential issue_ | _🟡 Minor_ **Countdown timer can be inadvertently reset by parent re-renders.** The effect depends on the whole `error` object. If a parent rebuilds the `UIError` on each render (or callers wrap the same logical error in a new object), this timer restarts mid-countdown and the rate-limit gating is unreliable. Depend on the actual scalar that drives the timer instead: ```diff - }, [error]); + }, [error.action?.disableSubmitFor]); ``` That way, identical 30s windows don't reset just because the parent rendered. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@components/ui/api-error-display.tsx` around lines 29 - 46, The useEffect for the countdown is depending on the whole error object which causes mid-countdown resets when parents recreate the object; change the effect to depend only on the scalar that drives the timer (error.action?.disableSubmitFor) instead of error, initialize total via Math.ceil(error.action.disableSubmitFor / 1000) only when that scalar is present, keep the interval logic and clearInterval cleanup using the same local interval variable, and ensure setSecondsLeft is set to null when disableSubmitFor is falsy; adjust the dependency array to [error.action?.disableSubmitFor] so the timer only restarts when the actual timeout value changes. ``` </details> </blockquote></details> <details> <summary>app/lending/page.tsx-118-136 (1)</summary><blockquote> `118-136`: _⚠️ Potential issue_ | _🟡 Minor_ **Failed balance fetch is rendered as `ACBU 0` — misleading for a financial UI.** When `getLendingBalance` throws, the catch sets `balance` to `null`, and the JSX falls back to `formatAmount(balance ?? 0)`, displaying `ACBU 0`. A user who is signed in but whose balance call failed will appear to have zero — which they may treat as authoritative. Distinguish "unavailable" from "zero": <details> <summary>🔧 Proposed fix</summary> ```diff - <p className="text-2xl font-bold text-foreground"> - {balanceLoading - ? '—' - : apiUser - ? `ACBU ${formatAmount(balance ?? 0)}` - : 'Sign in to view'} - </p> + <p className="text-2xl font-bold text-foreground"> + {balanceLoading + ? '—' + : !apiUser + ? 'Sign in to view' + : balance == null + ? 'Unavailable' + : `ACBU ${formatAmount(balance)}`} + </p> ``` </details> Also applies to: 259-265 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@app/lending/page.tsx` around lines 118 - 136, The effect that calls lendingApi.getLendingBalance sets balance to null on error, and the UI uses formatAmount(balance ?? 0) which renders "ACBU 0" for failures; change the catch behavior and rendering so failed/unknown balances are distinguished from zero: in the useEffect (the getLendingBalance promise handlers where setBalance and setBalanceLoading are called) set a sentinel for "unavailable" (e.g., undefined or a specific state) instead of treating errors as zero, and update the JSX that uses formatAmount(balance ?? 0) to branch — only call formatAmount when balance is a real number and otherwise render a placeholder like "—" or "Unavailable"; apply the same change to the other occurrence around the commented 259-265 block that uses formatAmount with balance. ``` </details> </blockquote></details> <details> <summary>app/send/page.tsx-84-84 (1)</summary><blockquote> `84-84`: _⚠️ Potential issue_ | _🟡 Minor_ **Two small follow-ups: leftover `useToast` and inconsistent label.** 1. Line 84 still destructures `toast` from `useToast()`, but the AI summary states the file moved away from toast notifications to `ApiErrorDisplay`. If `toast` is no longer called anywhere in the file, drop the hook to avoid an unused-variable warning and unnecessary subscription. 2. Line 554 displays `Send ACBU ${amount}` on the confirm action button. Since the rest of the confirm dialog uses `confirmedAmount` (line 539) — which is a stable snapshot — the button should match, otherwise editing the input while the dialog is open creates a label/value mismatch (the dialog shows X but submits Y). <details> <summary>♻️ Suggested fix for the label</summary> ```diff - <AlertDialogAction onClick={handleConfirmTransfer} className="flex-1 bg-primary text-primary-foreground hover:bg-primary/90" disabled={sending || isSubmitDisabled}>{sending ? 'Sending...' : `Send ACBU ${amount}`}</AlertDialogAction> + <AlertDialogAction onClick={handleConfirmTransfer} className="flex-1 bg-primary text-primary-foreground hover:bg-primary/90" disabled={sending || isSubmitDisabled}>{sending ? 'Sending...' : `Send ACBU ${confirmedAmount}`}</AlertDialogAction> ``` </details> Also applies to: 554-554 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@app/send/page.tsx` at line 84, Remove the unused hook by deleting the useToast() destructure (remove "const { toast } = useToast();" and any remaining references to toast in this file) to avoid unused-variable/subscription warnings, and update the confirm action button label to use the stable snapshot variable confirmedAmount instead of amount (ensure the confirm dialog’s submit button text displays `confirmedAmount` so the dialog label/value matches what will be submitted). ``` </details> </blockquote></details> <details> <summary>SECURITY_FIX_SUMMARY.md-76-84 (1)</summary><blockquote> `76-84`: _⚠️ Potential issue_ | _🟡 Minor_ **Replace unsupported backend claim with verifiable proof.** Line 84 claims the backend "already supports" httpOnly, secure, and sameSite=strict cookies "based on code comments," but this is not verifiable. The frontend code correctly expects these flags (e.g., `credentials: 'include'` in API calls), but without explicit backend verification (e.g., a production-tested PR/commit showing the cookie configuration), this claim is on faith alone. If the backend deploys with `secure=false` in preview environments or `sameSite=lax` to allow OAuth redirects, the login flow silently breaks. Add a direct link to the backend PR/commit implementing the cookie contract, and include an explicit integration test (login → assert `Set-Cookie` headers contain `httpOnly`, `secure`, `sameSite=strict`) before merge. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@SECURITY_FIX_SUMMARY.md` around lines 76 - 84, Replace the unverifiable claim that the backend "already supports" httpOnly/secure/sameSite=strict cookies with a concrete link to the backend PR/commit that implemented the cookie contract and add an integration test that performs a real login and asserts the Set-Cookie header includes httpOnly, Secure, and SameSite=Strict; specifically, update SECURITY_FIX_SUMMARY to cite the exact backend change (the auth login handler / POST /api/auth/login or auth.login implementation that sets the cookie) and add a test that sends credentials to the login endpoint, captures the Set-Cookie header, and asserts presence of "HttpOnly", "Secure", and "SameSite=Strict" (and fail the CI if any flag is missing). ``` </details> </blockquote></details> </blockquote></details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `a9781804-04cb-4b7a-b338-0aa340cec470` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between b791eee54522cd4aabe61a53d04eb538bf84711d and ceb5c01b171da85471845ffe0da51901c42dcaf0. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `pnpm-lock.yaml` is excluded by `!**/pnpm-lock.yaml` </details> <details> <summary>📒 Files selected for processing (61)</summary> * `.claude/settings.local.json` * `.vscode/settings.json` * `DEPLOYMENT_CHECKLIST.md` * `HELP_PAGE_SUMMARY.md` * `SECURITY_FIXES_PART2.md` * `SECURITY_FIX_SUMMARY.md` * `app/api/csp-report/route.ts` * `app/auth/2fa/page.tsx` * `app/auth/error.tsx` * `app/auth/signin/page.tsx` * `app/auth/signup/page.tsx` * `app/auth/wallet-setup/page.tsx` * `app/bills/error.tsx` * `app/burn/page.tsx` * `app/business/page.tsx` * `app/business/sme/page.tsx` * `app/currency/page.tsx` * `app/error.tsx` * `app/fiat/page.tsx` * `app/global-error.tsx` * `app/help/page.tsx` * `app/layout.tsx` * `app/lending/admin/page.tsx` * `app/lending/page.tsx` * `app/me/error.tsx` * `app/me/page.tsx` * `app/me/settings/security/page.tsx` * `app/mint/page.tsx` * `app/page.tsx` * `app/rates/page.tsx` * `app/reserves/page.tsx` * `app/savings/deposit/page.tsx` * `app/savings/error.tsx` * `app/savings/page.tsx` * `app/savings/withdraw/page.tsx` * `app/send/error.tsx` * `app/send/page.test.tsx` * `app/send/page.tsx` * `app/transactions/error.tsx` * `app/wallet/error.tsx` * `app/wallet/page.tsx` * `components/error-boundary.tsx` * `components/error-test-trigger.tsx` * `components/global-error-handler.tsx` * `components/mobile-nav.tsx` * `components/navigation/BackButton.tsx` * `components/ui/api-error-display.tsx` * `components/wallet-setup-modal.tsx` * `contexts/auth-context.tsx` * `hooks/use-api-error.ts` * `lib/api/client.ts` * `lib/api/user.ts` * `lib/error-reporting.ts` * `lib/features.ts` * `lib/lending-store.ts` * `lib/passcode-manager.ts` * `lib/wallet-storage.ts` * `middleware.ts` * `next.config.mjs` * `package.json` * `pr.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| import { Badge } from '@/components/ui/badge'; | ||
| import { useRouter } from 'next/navigation'; | ||
| import { ArrowRight, User, Settings, LogOut, Eye, Clock, Building2, Shield } from 'lucide-react'; | ||
| import { ArrowRight, User, Settings, LogOut, Eye, Clock, Building2, Shield, HelpCircle } from 'lucide-react'; |
Contributor
There was a problem hiding this comment.
Missing icon imports — KycBadge will crash at build/render.
getKycBadgeConfig references CheckCircle2 (Line 40), Clock3 (Line 49), XCircle (Line 57), and AlertCircle (Line 64), but none of these are in the lucide-react import on Line 8. This will fail TypeScript compilation (or throw ReferenceError at runtime if it slips past). The library summary explicitly mentions these icons should be imported here.
🐛 Proposed fix
-import { ArrowRight, User, Settings, LogOut, Eye, Clock, Building2, Shield, HelpCircle } from 'lucide-react';
+import {
+ ArrowRight,
+ User,
+ Settings,
+ LogOut,
+ Eye,
+ Clock,
+ Building2,
+ Shield,
+ HelpCircle,
+ CheckCircle2,
+ Clock3,
+ XCircle,
+ AlertCircle,
+} from 'lucide-react';📝 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.
Suggested change
| import { ArrowRight, User, Settings, LogOut, Eye, Clock, Building2, Shield, HelpCircle } from 'lucide-react'; | |
| import { | |
| ArrowRight, | |
| User, | |
| Settings, | |
| LogOut, | |
| Eye, | |
| Clock, | |
| Building2, | |
| Shield, | |
| HelpCircle, | |
| CheckCircle2, | |
| Clock3, | |
| XCircle, | |
| AlertCircle, | |
| } from 'lucide-react'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/me/page.tsx` at line 8, The import list at the top is missing icons used
by getKycBadgeConfig (CheckCircle2, Clock3, XCircle, AlertCircle), causing
build/runtime failures; update the lucide-react import that currently lists
ArrowRight, User, Settings, LogOut, Eye, Clock, Building2, Shield, HelpCircle to
also include CheckCircle2, Clock3, XCircle and AlertCircle so the symbols
referenced in getKycBadgeConfig resolve correctly and TypeScript compiles.
Comment on lines
+684
to
689
| rateRows.map((r) => ( | ||
| <Card key={r.currency} className="border-border p-4"> | ||
| <div className="flex justify-between"> | ||
| <p className="font-semibold text-foreground">ACBU/{r.currency ?? 'Rate'}</p> | ||
| <p className="text-lg font-bold text-primary">{r.rate != null ? String(r.rate) : '—'}</p> | ||
| <p className="font-semibold text-foreground">ACBU/{r.currency}</p> | ||
| <p className="text-lg font-bold text-primary">{formatRate(r.rate)}</p> | ||
| </div> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify formatRate is imported into app/mint/page.tsx.
rg -nP '\bformatRate\b' app/mint/page.tsx
rg -nP '^\s*export\s+(?:const|function)\s+formatRate\b' lib/Repository: Pi-Defi-world/acbu-frontend
Length of output: 168
🏁 Script executed:
#!/bin/bash
# Search for import statements containing formatRate
echo "=== Searching for formatRate imports in app/mint/page.tsx ==="
rg -nP 'import.*formatRate' app/mint/page.tsx
# Search for any formatRate definition in the entire codebase
echo -e "\n=== Searching for formatRate definition in the codebase ==="
rg -nP '(?:export\s+)?(?:const|function|async\s+function)\s+formatRate\b' --type ts --type tsx
# Check the import statement around line 33 to see what's actually imported from `@/lib/utils`
echo -e "\n=== Checking imports around line 33 in app/mint/page.tsx ==="
sed -n '30,40p' app/mint/page.tsxRepository: Pi-Defi-world/acbu-frontend
Length of output: 712
🏁 Script executed:
#!/bin/bash
# Get file size to see if we can review the entire file
echo "=== File info ==="
wc -l app/mint/page.tsx
# Broader search for formatRate without file type restrictions
echo -e "\n=== Searching for formatRate definition anywhere in repo ==="
rg -nP 'formatRate' --type-list | head -5 # See what types are available
# Try with broader approach
echo -e "\n=== Search without type filter ==="
rg -nP '\bformatRate\b' | grep -v 'node_modules'
# Check if it might be defined in the same file
echo -e "\n=== Checking first 100 lines of app/mint/page.tsx for any formatRate definition or local function ==="
head -100 app/mint/page.tsx | grep -n formatRateRepository: Pi-Defi-world/acbu-frontend
Length of output: 991
Import formatRate from app/rates/page.tsx.
formatRate is used on line 688 but never imported or defined in this file. formatRate is defined in app/rates/page.tsx and must be imported to avoid a ReferenceError at runtime.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/mint/page.tsx` around lines 684 - 689, The render uses formatRate inside
the rateRows.map in app/mint/page.tsx but formatRate is not defined here; import
the helper from app/rates/page.tsx (where formatRate is declared) and add an
import for formatRate at the top of app/mint/page.tsx so the JSX line using
formatRate(r.rate) resolves correctly.
Member
|
@DavisVT resolve conflicts |
Member
|
@DavisVT RESOLVE |
1 similar comment
Member
|
@DavisVT RESOLVE |
Contributor
Author
|
@Junman140 Done |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
app/send/page.tsx (3)
3-52:⚠️ Potential issue | 🔴 CriticalAdd the missing imports for
useToastandApiErrorDisplay.Line 83 calls
useToast()and line 530 rendersApiErrorDisplay, but neither symbol is imported in this file. That leaves the page in a compile-failing state even after the parser issue below is fixed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/send/page.tsx` around lines 3 - 52, The file is missing imports for useToast and ApiErrorDisplay which causes compile errors; add the appropriate imports at the top of the module: import useToast from the toast hook (or the named export used elsewhere in the repo) and import ApiErrorDisplay from its UI/error component, then use those symbols (useToast used where called on line ~83 and ApiErrorDisplay where rendered around line ~530) so the references resolve; ensure the import paths match your project's existing convention for toast and error-display components.
161-251:⚠️ Potential issue | 🔴 CriticalWire this handler to
useApiErrorinstead of the removed submit-error helpers.Line 164 still calls
clearSubmitError()and line 247 still callshandleSubmitError(e), but the file now exposesclearError/setApiErrorfromuseApiError. As written, this path won't compile.Suggested fix
- clearSubmitError(); + clearError(); … - handleSubmitError(e); + setApiError(e);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/send/page.tsx` around lines 161 - 251, The handler handleConfirmTransfer still calls removed helpers clearSubmitError() and handleSubmitError(e); replace clearSubmitError() with clearError() and replace handleSubmitError(e) with setApiError(e) (imported from useApiError), ensure useApiError's clearError and setApiError are used in this component and included in the handleConfirmTransfer dependency array, and remove any leftover imports/usages of the old submit-error helpers.
161-251:⚠️ Potential issue | 🔴 CriticalInclude
confirmedAmountin the submit callback's source of truth.This callback submits
confirmedAmount, but it is memoized withoutconfirmedAmountin the dependency list. After clicking Continue, the dialog shows the new snapshot whilehandleConfirmTransfercan still close over the previous value and create the transfer with""or an older amount. The validation guard should also use the snapshot, not the live input.Suggested fix
- if (!amount || parseFloat(amount) <= 0 || !to) return; + if (!confirmedAmount || parseFloat(confirmedAmount) <= 0 || !to) return; … - }, [amount, getToValue, note, userId, stellarAddress, kit, opts, loadTransfers, refreshBalance]); + }, [confirmedAmount, getToValue, note, userId, stellarAddress, kit, opts, loadTransfers, refreshBalance]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/send/page.tsx` around lines 161 - 251, handleConfirmTransfer is closing over confirmedAmount and validating against the live amount, causing stale submissions; update the validation to use the snapshot confirmedAmount (e.g. change the guard to if (!confirmedAmount || parseFloat(confirmedAmount) <= 0 || !to) return) and add confirmedAmount to the useCallback dependency array so handleConfirmTransfer is re-memoized when the confirmed value changes; keep references to getToValue, userId, stellarAddress, kit, opts, loadTransfers, refreshBalance as-is.lib/api/client.ts (1)
56-58:⚠️ Potential issue | 🔴 CriticalFinish removing the bearer-token path.
RequestOptionsno longer has atokenproperty, but therequest()function at line 84 still readsopts.tokenand references an undefinedcurrentTokenvariable, andapiOpts()at lines 175–176 still attempts to return a token field. This causes TypeScript compilation to fail:RequestOptionsdoes not definetoken, making the type mismatch atapiOpts()a contract violation and theopts.tokenaccess at line 84 a property-not-found error.Complete the cookie-only auth migration by removing all token-handling code and updating the interface contract.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/api/client.ts` around lines 56 - 58, RequestOptions was stripped of token but code still reads opts.token and references currentToken; remove all token-handling: delete any access to opts.token and the currentToken variable usage in the request() function, and update apiOpts() to stop returning a token field (only return headers/cookie or whatever remains), ensuring RequestOptions and apiOpts() signatures match; search for and remove or refactor any token-related logic in request(), apiOpts(), and any helpers so compilation no longer references the undefined token/currentToken symbols.app/currency/page.tsx (1)
130-198:⚠️ Potential issue | 🟠 MajorFinish the migration to
useApiError()in the execute path.The UI now clears/displays
uiError, but the catch block still writes tosetSubmitError(...). That means submission failures won't show up inApiErrorDisplay, and if the old submit-error state was removed this path no longer compiles.Suggested fix
- } catch (e) { - logger.error(`Currency operation failed: ${activeTab}`, e); // <-- ADD LOGGER - setSubmitError(e instanceof Error ? e.message : "Operation failed"); + } catch (e) { + logger.error(`Currency operation failed: ${activeTab}`, e); + setApiError(e); } finally { setSubmitting(false); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/currency/page.tsx` around lines 130 - 198, The catch in handleExecute still calls setSubmitError(...) — replace that with the useApiError hook's setter (e.g., setUiError or setApiError) so the ApiErrorDisplay sees the error; update the catch to call setUiError(e instanceof Error ? e.message : "Operation failed") (and keep logger.error(`Currency operation failed: ${activeTab}`, e) and clearError/clearApiError usage intact), and ensure the setter from useApiError (setUiError/setApiError) is imported/declared in the component scope where handleExecute is defined.app/mint/page.tsx (1)
242-317:⚠️ Potential issue | 🔴 CriticalRemove the duplicate
handleExecuteBurndeclaration.The file contains two
const handleExecuteBurn = async () => { ... }declarations (lines 242 and 318) in the same scope, which creates a duplicate identifier error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/mint/page.tsx` around lines 242 - 317, There are two duplicate declarations of the async handler const handleExecuteBurn in the same scope; remove the redundant one and keep the correct implementation (ensure you preserve the version that wires up getWalletSecretAnyLocal, Kit fallback flow, submitBurnRedeemSingleClient, fiatApi.postOffRamp, setTxId and setStep("success")). After deleting the duplicate, update any references to handleExecuteBurn if needed (e.g., button onClick or useEffect) so they point to the remaining handleExecuteBurn and run the app/build to confirm the duplicate identifier error is resolved.app/burn/page.tsx (2)
109-187:⚠️ Potential issue | 🔴 CriticalReplace
handleSubmithandler with proper form submission pattern using react-hook-form.The code references
values.accountNumber,values.bankCode,values.accountName,values.acbuAmount, andvalues.currency(lines 121–123, 139–140, 172–173, 180–181, 187), butvaluesis never defined. The function should be integrated withform.handleSubmit()to receive validated form values:const handleSubmit = async (values: BurnFormValues) => { // Remove the e.preventDefault() and signature—react-hook-form handles this if (!isValid) return; // ... rest of the handler };Then bind it in the form:
<form onSubmit={form.handleSubmit(handleSubmit)}>(currently line 233 references undefinedformHandleSubmitandonSubmit).Also fix: undefined
uiError(should beerror, line 214), undefinedisSubmitDisabled(line 366), missingCheckCircleandApiErrorDisplayimports (lines 215, 225), and duplicateuseSearchParamsimport (lines 5, 11).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/burn/page.tsx` around lines 109 - 187, The submit handler currently uses an undefined "values" and DOM event; change handleSubmit to accept validated values from react-hook-form (signature: const handleSubmit = async (values: BurnFormValues) => { ... }) remove e.preventDefault and the React.FormEvent parameter, and replace all uses of values.* in handleSubmit with that parameter; bind the form element to form.handleSubmit(handleSubmit). Also rename any uiError references to error (or the actual error state variable), replace the undefined isSubmitDisabled usage with a computed boolean (e.g., !isValid || loading), add missing imports for CheckCircle and ApiErrorDisplay, and remove the duplicate useSearchParams import so identifiers (handleSubmit, form.handleSubmit, error, isSubmitDisabled, CheckCircle, ApiErrorDisplay) resolve.
232-371:⚠️ Potential issue | 🔴 CriticalReplace
formHandleSubmit(onSubmit)withhandleSubmitand add missing closing</Form>tag.The code has three syntax errors:
formHandleSubmitandonSubmitare undefined. The functionhandleSubmitis defined in the component but never used. Change line 233 from<form onSubmit={formHandleSubmit(onSubmit)}>to<form onSubmit={handleSubmit}>.The
<Form>wrapper opened on line 232 is never closed. Add</Form>after</form>on line 368.The
</div>on line 361 closes a div with no corresponding opening tag in this block. Either remove this closing tag or add a matching<div>opening tag if a wrapper is intended around the form fields.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/burn/page.tsx` around lines 232 - 371, The form markup has three issues: replace the undefined formHandleSubmit/onSubmit usage with the existing handleSubmit by changing the form's onSubmit to use handleSubmit; close the Form component by adding a closing </Form> tag after the inner </form>; and remove the stray </div> (or add the corresponding opening <div> wrapper if intended) so that tags balance—look for the Form wrapper, the inner <form onSubmit={handleSubmit}> and the stray </div> near the end of the block when making the fixes.
♻️ Duplicate comments (4)
app/send/page.tsx (2)
499-505:⚠️ Potential issue | 🔴 CriticalUse
isValidhere instead of the removedisFormValid().This still calls an undefined function, so the dialog will throw as soon as React evaluates
disabled. The memoized boolean at lines 256-261 is the value this button should be using.Suggested fix
- disabled={!isFormValid()} + disabled={!isValid}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/send/page.tsx` around lines 499 - 505, The Button currently references a removed function isFormValid(), causing an undefined call; update the disabled prop to use the memoized boolean isValid instead (i.e., replace disabled={!isFormValid()} with disabled={!isValid}) so the Button (component using onClick that calls setConfirmedAmount and setShowConfirmDialog) uses the existing isValid state; ensure you only change the disabled expression and leave the onClick handlers (setConfirmedAmount, setShowConfirmDialog) and className intact.
103-148:⚠️ Potential issue | 🔴 CriticalResolve the broken merge/conflict artifact in the loader section.
The stray
devtokens and duplicatedloadContactsbranches leave thisuseCallbackblock unbalanced, which is why Biome now sees thereturnat line 315 as being outside the component. This needs to be cleaned up before the file will build.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/send/page.tsx` around lines 103 - 148, Remove the stray "dev" tokens and the duplicated/conflicting loadContacts blocks so the useCallback is a single well-formed function; specifically, keep one loadContacts useCallback that (1) clears errors via setContactsError(""), (2) awaits userApi.getContacts(opts) and calls setContacts(data.contacts ?? []), (3) catches errors and sets setContactsError(message) and calls toast({...}) with the same message, and (4) calls setLoadingContacts(false) in finally; also ensure there is no duplicated closing braces or parentheses left over from the merge and that related symbols (loadContacts, useCallback, userApi.getContacts, setContacts, setContactsError, setLoadingContacts, toast) are referenced consistently with loadTransfers.app/savings/deposit/page.tsx (1)
25-27:⚠️ Potential issue | 🔴 CriticalThis resolution flow is missing its backing state/imports.
setResolving,setError,resolveUserUri,resolving, andAlertCircleare all referenced here without a declaration/import in this file, so the page will not build. Finish wiring the new flow end-to-end or revert these references to the previous implementation.Also applies to: 38-39, 84-85, 102-108
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/savings/deposit/page.tsx` around lines 25 - 27, The page references missing state and imports: declare resolving and error state via React useState (e.g., const [resolving, setResolving] = useState(false); const [error, setError] = useState(""); import React/useState if needed), import or implement resolveUserUri (or import its module) and import the AlertCircle component from your icon library (e.g., lucide-react) so setResolving, setError, resolveUserUri, resolving, and AlertCircle are defined; alternatively revert the new flow to the previous implementation by removing these references. Ensure the cancellation flag (cancelled) is handled consistently with the resolved flow and update any useEffect cleanup where used.app/mint/page.tsx (1)
662-666:⚠️ Potential issue | 🔴 Critical
formatRateis still undefined in this module.The Rates tab still renders
formatRate(r.rate), but this file does not import or define that helper.Recon script
#!/bin/bash rg -nP '\bformatRate\b' app/mint/page.tsx echo rg -nP '^import .*formatRate|function\s+formatRate\b|const\s+formatRate\s*=' app/mint/page.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/mint/page.tsx` around lines 662 - 666, The Rates tab uses formatRate(r.rate) but formatRate is not defined or imported in this module; fix it by either importing the helper (e.g., add an import for formatRate from its utility module) or add a local formatRate function in app/mint/page.tsx and use it where rateRows.map calls formatRate; locate the usage in the JSX (rateRows.map((r) => ... formatRate(r.rate))) and ensure the symbol is exported from the chosen module or declared before the component so the reference resolves.
🟠 Major comments (10)
app/send/page.tsx-117-136 (1)
117-136:⚠️ Potential issue | 🟠 MajorDon't share one
loadErroracross both async loaders.
loadTransfers()andloadContacts()both clear and overwrite the same error state while running concurrently on mount. A contacts success can therefore clear a transfers failure (and vice versa), so the banner can disappear even though one section never loaded. Keep separate error states or derive a combined banner from both results.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/send/page.tsx` around lines 117 - 136, loadTransfers and loadContacts share the same loadError state (setLoadError) so one loader can clear the other's error; introduce separate error state variables (e.g., transfersError and contactsError with setTransfersError and setContactsError) and update loadTransfers to setTransfersError (and not touch contactsError) and update loadContacts to setContactsError (and not touch transfersError), keep setLoadingTransfers and setTransfers/setContacts behavior unchanged, and render any combined banner by deriving it from both transfersError and contactsError instead of a single shared loadError.components/error-boundary.tsx-17-17 (1)
17-17:⚠️ Potential issue | 🟠 MajorWire the new
onErrorAPI throughcomponentDidCatch.The
onErrorcallback anderrorInfostate property were added to the public API but are never used.componentDidCatchonly logs the error; it does not storeerrorInfoto state or invoke the callback, making the new API non-functional.Suggested fix
componentDidCatch(error: Error, errorInfo: React.ErrorInfo): void { + this.setState({ errorInfo }); logger.error('ErrorBoundary caught an error:', { error, errorInfo }); + this.props.onError?.(error, errorInfo); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/error-boundary.tsx` at line 17, componentDidCatch currently only logs errors and doesn't wire the new onError API or persist errorInfo; update the ErrorBoundary.componentDidCatch method to call this.setState({ error, errorInfo }) to store the React.ErrorInfo in state (referencing the state field errorInfo) and invoke this.props.onError?.(error, errorInfo) so consumers receive the callback; ensure you reference the onError prop and the errorInfo state property names exactly as declared.app/savings/withdraw/page.tsx-88-90 (1)
88-90:⚠️ Potential issue | 🟠 MajorThe new confirmation checkbox is not enforced.
handleSubmitand the submitdisabledcondition ignoreconfirmDifferentRecipient,recipientValidationLoading, andrecipientValidationError. Once the editor is wired up, a form submit can still proceed with an unconfirmed or unresolved alternate recipient. Gate the handler itself as well as the button state.Also applies to: 214-230
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/savings/withdraw/page.tsx` around lines 88 - 90, The form currently allows submission even when the new recipient confirmation or validation are unresolved; update the submit gating in handleSubmit to also require confirmDifferentRecipient === true and that recipientValidationLoading is false and recipientValidationError is falsy, and mirror those same checks in the submit button's disabled condition so the button is disabled while recipientValidationLoading is true, recipientValidationError exists, or confirmDifferentRecipient is false (in addition to the existing user/amount checks); reference the handleSubmit function, the confirmDifferentRecipient flag, recipientValidationLoading, recipientValidationError, and the submit button disabled logic to locate and apply these changes.app/savings/page.tsx-33-51 (1)
33-51:⚠️ Potential issue | 🟠 MajorThe new resolver is dead code, so this page still uses the raw identifier.
resolveUserUri()is added but never called, which is why ESLint flags it on Line 39. More importantly,setApiUser(uri)keeps feedinggetSavingsPositions()the unresolved alias/pay identifier instead of the canonical value the helper was added to produce.Suggested fix
useEffect(() => { setReceiveError(""); userApi.getReceive(opts).then(async (data) => { const uri = (data.pay_uri ?? data.alias) as string | undefined; - if (uri && typeof uri === "string") setApiUser(uri); + if (uri && typeof uri === "string") { + const resolved = await resolveUserUri(uri, opts); + setApiUser(resolved); + } setReceiveError(""); }).catch((e) => { logger.error("Failed to load user info", e); setReceiveError(e instanceof Error ? e.message : "Failed to load user info"); });Also applies to: 107-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/savings/page.tsx` around lines 33 - 51, resolveUserUri is defined but never used so setApiUser/getSavingsPositions keep receiving raw identifiers; update the callers to await resolveUserUri(rawIdentifier, opts) and pass the returned canonical URI into setApiUser (and into the subsequent getSavingsPositions invocation) instead of the raw value; specifically locate usages that feed setApiUser and the call sites around the block referenced (including the other occurrence around lines 107-109) and replace feeding raw with the awaited resolveUserUri(...) result, preserving the same opts parameter shape passed to resolveRecipient.app/savings/withdraw/page.tsx-153-179 (1)
153-179:⚠️ Potential issue | 🟠 Major
Changenever makes the recipient editable.The field is still
readOnlyand bound touser, sorecipient,handleRecipientChange,validateRecipient, and the new confirmation flow never participate. As implemented, withdrawing to a different recipient is impossible.Suggested fix
- <Input - id="withdraw-account" - value={resolving ? "Resolving…" : user} - readOnly - className="border-border font-mono text-sm bg-muted" - /> + <Input + id="withdraw-recipient" + value={resolving ? "Resolving…" : recipient} + readOnly={!editingRecipient} + onChange={(e) => handleRecipientChange(e.target.value)} + onBlur={() => { + const value = recipient.trim(); + if (editingRecipient && value) void validateRecipient(value); + }} + className="border-border font-mono text-sm bg-muted" + />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/savings/withdraw/page.tsx` around lines 153 - 179, The "Change" button toggles editingRecipient but the Input (id="withdraw-account") is always readOnly and bound to user, so recipient, handleRecipientChange, validateRecipient and the confirmation flow are never used; update the Input to use value={editingRecipient ? recipient : (resolving ? "Resolving…" : user)}, remove/readOnly only when editingRecipient is true (i.e., set readOnly={!editingRecipient}), and wire its onChange to handleRecipientChange (so validateRecipient can run) and ensure handleToggleEdit resets recipient to user when cancelling (or preserves when switching to edit) so the confirmation flow can operate on the edited recipient.app/wallet/page.tsx-70-74 (1)
70-74:⚠️ Potential issue | 🟠 MajorPersist the new seed only after the backend accepts the wallet change.
Both flows overwrite local wallet material before
putWalletAddress()/postWalletConfirm()succeeds. If the backend update fails, the device now holds a secret for a different account than the backend still references.app/send/page.tsx:174-183explicitly rejects that mismatch, so a partial failure can strand the user until they re-import the old seed.Suggested fix
- // Store encrypted with passcode (secure) - await storeWalletSecret(userId, passphrase, passcode); - // Sync public key to backend. const result = await userApi.putWalletAddress(newAddress, opts); if (!result?.ok) { throw new Error("Backend did not accept the new wallet address. Please retry."); } @@ try { await userApi.postWalletConfirm({ wallet_address: newAddress }, opts); } catch (err) { console.warn("Wallet confirm failed, but wallet address was set. User can continue.", err); } + + await storeWalletSecret(userId, passphrase, passcode);Also applies to: 119-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/wallet/page.tsx` around lines 70 - 74, The code currently calls storeWalletSecret(userId, passphrase, passcode) before contacting the backend (userApi.putWalletAddress and the similar postWalletConfirm flow), which can leave local secrets inconsistent if the backend call fails; change the flow so you only call storeWalletSecret (and any local overwrite of the seed/secret) after the backend call returns a successful response (check result.ok / HTTP 2xx or equivalent) for userApi.putWalletAddress(newAddress, opts) and the postWalletConfirm path, and if the backend call fails surface the error to the caller and do not persist or overwrite local wallet material (or implement an explicit rollback if needed).app/wallet/page.tsx-299-303 (1)
299-303:⚠️ Potential issue | 🟠 MajorDon’t promise “stored securely” with the current implementation.
These banners say the seed is securely encrypted on-device, but
lib/wallet-storage.ts:26-29still “encrypts” withbtoa(\${passcode}:${secret}`)`, which is trivially reversible. That copy overpromises the protection level for a wallet seed.Also applies to: 324-328
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/wallet/page.tsx` around lines 299 - 303, The banner overpromises security because lib/wallet-storage.ts currently uses btoa(`${passcode}:${secret}`) to “encrypt” the seed; replace this with real crypto: in the functions that save/load the secret in lib/wallet-storage.ts (the code path using btoa(`${passcode}:${secret}`)) derive an encryption key from the passcode (e.g., PBKDF2 with a per-item random salt and sufficient iterations), encrypt the secret with a modern algorithm (AES-GCM) using a random IV, and persist the ciphertext together with the salt and IV; update the corresponding load/decrypt function to reverse this (derive key from stored salt and passcode, then decrypt) and add robust error handling for failed decryption, and if you keep the UI text, change “stored securely” to a less absolute phrase unless you can guarantee stronger protections.lib/api/client.ts-95-98 (1)
95-98:⚠️ Potential issue | 🟠 MajorHandle already-aborted caller signals before
fetch().When
opts.signal.abortedis alreadytrue, theaddEventListener('abort', ...)call does not register the listener (per AbortSignal spec), so the internal controller never gets aborted and the request proceeds to the backend. Callers expect cancellation to fail fast without hitting the backend.Suggested fix
// If caller provides signal, abort our controller when caller's aborts if (opts.signal) { - opts.signal.addEventListener('abort', () => controller.abort(), { once: true }); + if (opts.signal.aborted) { + controller.abort(opts.signal.reason); + } else { + opts.signal.addEventListener('abort', () => controller.abort(opts.signal.reason), { once: true }); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/api/client.ts` around lines 95 - 98, The current logic adds an 'abort' listener to opts.signal but doesn't handle the case where opts.signal.aborted is already true, so the internal AbortController (controller) never gets aborted and the request proceeds; update the code around the opts.signal handling in lib/api/client.ts to first check if opts.signal.aborted is true and if so call controller.abort() immediately, otherwise register the opts.signal.addEventListener('abort', () => controller.abort(), { once: true }); so that the internal controller is always aborted when the caller's signal is already aborted or aborts later (refer to opts.signal and controller in the surrounding function).app/burn/page.tsx-364-367 (1)
364-367:⚠️ Potential issue | 🟠 MajorDrive the submit button from RHF state, not the removed local state.
isValidis still computed from the olduseStatefields, so after this migration it no longer tracks what the user typed into the form.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/burn/page.tsx` around lines 364 - 367, The submit button is still using the removed local isValid state; update the form to derive validity from React Hook Form instead: ensure your useForm call includes a mode that validates on change (e.g., useForm({... , mode: "onChange"})) and destructure the RHF formState (const { formState: { isValid } } = useForm(...)) or read form.getState().isValid, remove the old local isValid state and any setIsValid handlers, and change the Button disabled prop to disabled={!isValid || loading || isSubmitDisabled} so the button is driven by RHF's isValid; keep loading and isSubmitDisabled as-is.app/burn/page.tsx-86-96 (1)
86-96:⚠️ Potential issue | 🟠 MajorKeep the deep-link defaults in the form state.
The page still reads
amountandcurrencyfrom the URL, but the new form ignores them and resets both fields to empty/NGN. That breaks the/mint -> /burnprefill flow.Suggested fix
const form = useForm<BurnFormValues>({ resolver: zodResolver(burnSchema), defaultValues: { - acbuAmount: "", - currency: "NGN", + acbuAmount: searchParams.get("amount") || "", + currency: (searchParams.get("currency") || "NGN").toUpperCase(), accountNumber: "", bankCode: "", accountName: "",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/burn/page.tsx` around lines 86 - 96, The form's defaultValues are overwriting deep-link params; update the useForm initialization (useForm<BurnFormValues> with resolver zodResolver(burnSchema)) so defaultValues for acbuAmount and currency are derived from the existing URL/deep-link values (e.g., the amount and currency you already read elsewhere on the page) instead of hardcoding "" and "NGN"; preserve the current fallbacks (empty string and "NGN") if the URL values are absent and ensure types match BurnFormValues.
🟡 Minor comments (1)
app/layout.tsx-9-9 (1)
9-9:⚠️ Potential issue | 🟡 MinorRemove the stale
AuthGuardimport while the wrapper stays commented out.ESLint is already flagging this import as unused in the current state. Either drop the import now or restore the wrapper in the same change.
Also applies to: 86-88
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/layout.tsx` at line 9, The AuthGuard import is stale while its wrapper is commented out; remove the unused import statement "AuthGuard" (and any duplicate unused imports around the other occurrences mentioned at lines 86-88) from the module, or alternatively restore the commented wrapper that uses AuthGuard so the import is actually used—choose one approach and make the code and imports consistent (update imports/usages for the AuthGuard symbol accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/auth/2fa/page.tsx`:
- Around line 72-73: The call to login is passing the API key as the first
argument (login(result.api_key!, ...)) which no longer matches the new signature
login(userId, stellarAddress?), causing the API key to be stored as userId;
update the call in the authApi.verify2fa result handling to call
login(result.user_id, result.stellar_address) (omitting result.api_key) so
userId and optional stellarAddress map correctly to the new login(userId,
stellarAddress?) signature.
In `@app/burn/page.tsx`:
- Around line 214-230: The JSX uses undeclared symbols: replace uiError with the
error returned from useApiError (the hook referenced as useApiError/ error in
this file), and add the missing imports for CheckCircle (import from
'lucide-react', similar to the existing ArrowLeft import) and ApiErrorDisplay
(import the component where it’s defined); ensure the top-of-file imports
include CheckCircle and ApiErrorDisplay and that the conditional rendering uses
error (not uiError) and the existing txId logic remains unchanged.
In `@app/currency/page.tsx`:
- Around line 29-55: Import the missing types and APIs and initialize the
missing state: add "RatesResponse" from "@/types/api", "ratesApi" from
"@/lib/api/rates", and the "toast" helper (where your project keeps it) to the
top of the file; initialize React state for "balance" and "balanceLoading"
(e.g., useState<number | null>(...) and useState<boolean>(...)) where component
state is declared so code referencing balance/balanceLoading compiles; replace
any calls to the nonexistent setSubmitError with the already-destructured
setApiError from useApiError(); ensure the functions that use RatesResponse
(localPerAcbu, estimateAcbuFromUsd, estimateLocalFromAcbu) keep their signatures
and now refer to the imported RatesResponse type.
- Around line 146-149: The component calls toast(...) (at the places that use
res.transaction_id and res.status) but never imports or initializes the hook;
import useToast (from "@/hooks/use-toast") at the top of the file and initialize
the hook inside the component to get the toast function (e.g., call useToast and
destructure toast) so that the toast variable used in the mint handlers and
other locations is defined; update references to toast in the component
accordingly.
- Around line 118-125: The component is using balance, balanceLoading, and
refreshBalance but never imports or calls the useBalance hook; import useBalance
and call it near the top of the component to destructure the missing values
(e.g., const { balance, balanceLoading, refreshBalance } = useBalance()), then
remove any hardcoded or undefined references and use these variables in the
existing calculations and handlers (references: useBalance, balance,
balanceLoading, refreshBalance, estimateAcbuFromUsd). Ensure the import is added
to the module imports and the hook call is executed before any code that reads
balance, balanceLoading, or refreshBalance.
In `@app/error.tsx`:
- Line 27: The fallback component renders Lucide icons AlertTriangle, RefreshCw,
and Home but they are not imported, causing a compilation error; add named
imports for AlertTriangle, RefreshCw, and Home from the lucide-react package
(e.g., import { AlertTriangle, RefreshCw, Home } from 'lucide-react') at the top
of the file where the component is defined so the symbols used in the JSX
(AlertTriangle, RefreshCw, Home) are resolved.
In `@app/fiat/page.tsx`:
- Around line 20-23: Consolidate the two useApiError() calls into a single hook
instance so the component uses the same uiError, setApiError, clearError,
isSubmitDisabled and handleError bindings (remove the duplicate declaration that
redeclares clearError and merge error/handleError into the first destructure),
and update all references (calls to handleError on lines where it's used) to use
that single instance; also add the missing import for ApiErrorDisplay so the
component can render the error UI that reads uiError.
- Around line 109-110: The component ApiErrorDisplay is used in the render
(wrapped by the uiError check) but not imported; add an import for
ApiErrorDisplay from '@/components/ui/api-error-display' at the top of the
module so the JSX <ApiErrorDisplay error={uiError} onDismiss={clearError} />
resolves correctly and the symbols ApiErrorDisplay, uiError and clearError
remain unchanged in the component.
In `@app/layout.tsx`:
- Around line 73-78: There are two declarations of const nonce in RootLayout
causing a syntax error; remove the duplicate by keeping a single nonce
assignment that reads the header once (e.g., drop the first "const nonce =
(await headers()).get('x-nonce') || undefined;" or drop the later
headersList/duplicate line), and if you need headersList elsewhere rename or
reuse it so only one nonce variable is declared (referencing nonce and
headersList in RootLayout).
In `@app/mint/page.tsx`:
- Around line 66-67: There are duplicate const bindings for clearMintError and
clearBurnError caused by two useApiError() destructurings; remove the redundant
properties from the first destructuring (the one that defines mintUiError,
setMintApiError, isMintDisabled, and burnUiError, setBurnApiError,
isBurnDisabled) so that clearMintError and clearBurnError are only declared once
(where they are re-declared later), keeping the remaining useApiError()
destructures as mintUiError/setMintApiError/isMintDisabled and
burnUiError/setBurnApiError/isBurnDisabled.
In `@app/reserves/page.tsx`:
- Around line 20-27: The new Tooltip and icon components used in the JSX
(Tooltip, TooltipTrigger, TooltipContent, and Info) are not imported, causing
compilation errors; add the missing imports at the top of the file by importing
Tooltip, TooltipTrigger, TooltipContent from your Tooltip/ui component module
(or the library you use) and import the Info icon from your icons library (or
icon set) so the JSX references resolve, then run the build to verify no
unused-import warnings remain.
In `@app/savings/deposit/page.tsx`:
- Around line 25-45: The effect never clears the resolving state except in the
!uri branch; ensure setResolving(false) is called after successful resolution
and in the catch path (but only when not cancelled) so the UI unblocks — update
the promise chain handling around userApi.getReceive / resolveUserUri: after
await resolveUserUri(uri, opts) and setUser(resolved) call setResolving(false)
if not cancelled, and in the .catch((e) => { ... }) branch call
setResolving(false) (guarded by !cancelled) in addition to logging via
logger.error; apply the same fix to the other occurrence referenced (lines
~147-149) to avoid leaving resolving true.
In `@app/savings/withdraw/page.tsx`:
- Around line 17-27: The file mixes old "user"/"targetRecipient" flow with the
new "recipient"/"resolvedRecipient" state; remove references to undefined
symbols (user, setUser, targetRecipient, setError, setResolving, resolveUserUri,
recipientApi) and consistently wire the new flow using the declared state:
recipient, setRecipient, homeRecipient, setHomeRecipient, resolvedRecipient,
setResolvedRecipient, recipientValidationLoading, setRecipientValidationLoading,
recipientValidationError, setRecipientValidationError, editingRecipient,
setEditingRecipient; update effect/validation/submit handlers to call the new
resolver/validator (or import and use the correct resolve/recipient API and
RecipientResponse type) and to use clearError/handleError and success/loading
state variables instead of the old setError/setResolving symbols so the
component type-checks and renders consistently.
In `@app/wallet/page.tsx`:
- Around line 59-64: The re-auth branches call undefined logout and router; fix
by calling the required hooks at the top of the component: invoke useAuth() and
destructure logout (e.g., const { logout } = useAuth()) and call useRouter() to
get router (e.g., const router = useRouter()) before any conditional returns so
both logout and router.push(...) are defined when used in the getPasscode()
failure branches in the page component (references: logout, useAuth, router,
useRouter, router.push, getPasscode).
In `@components/error-boundary.tsx`:
- Line 59: The JSX references missing Lucide icons AlertTriangle, RefreshCw, and
Home causing a compile error; add imports for these icons (e.g., import {
AlertTriangle, RefreshCw, Home } from 'lucide-react') near the top of
components/error-boundary.tsx (with your other imports) so the fallback UI can
render without errors and the references in the ErrorBoundary component resolve.
In `@contexts/auth-context.tsx`:
- Line 24: Remove the stray shell text "git checkout -b
fix/f-043-remove-console-logs" from contexts/auth-context.tsx so the module can
parse, then fix imports: remove the non-existent setToken import from
"@/lib/api/client" and replace it by importing the actual token setter exported
by your codebase (find and import the correct symbol used to set auth tokens
where setToken was referenced), and add an import for clearPasscode from
"@/lib/passcode-manager" so calls to clearPasscode() (used around the clear/auth
flows) resolve; update the import list at the top of contexts/auth-context.tsx
accordingly so all references (setToken usage and clearPasscode calls) point to
real exported functions.
- Around line 79-87: Remove the now-unused setToken import from
"@/lib/api/client" and add the missing import for clearPasscode (the passcode
utility used in this file) so clearPasscode() calls resolve; e.g., import {
clearPasscode } from the passcode utilities module (for example "@/lib/passcode"
or the module in your project that exports clearPasscode), then remove any
leftover references to setToken to avoid unused-import errors and ensure
clearPasscode is imported once for its calls in auth-context (used around the
86/159/166 call sites).
---
Outside diff comments:
In `@app/burn/page.tsx`:
- Around line 109-187: The submit handler currently uses an undefined "values"
and DOM event; change handleSubmit to accept validated values from
react-hook-form (signature: const handleSubmit = async (values: BurnFormValues)
=> { ... }) remove e.preventDefault and the React.FormEvent parameter, and
replace all uses of values.* in handleSubmit with that parameter; bind the form
element to form.handleSubmit(handleSubmit). Also rename any uiError references
to error (or the actual error state variable), replace the undefined
isSubmitDisabled usage with a computed boolean (e.g., !isValid || loading), add
missing imports for CheckCircle and ApiErrorDisplay, and remove the duplicate
useSearchParams import so identifiers (handleSubmit, form.handleSubmit, error,
isSubmitDisabled, CheckCircle, ApiErrorDisplay) resolve.
- Around line 232-371: The form markup has three issues: replace the undefined
formHandleSubmit/onSubmit usage with the existing handleSubmit by changing the
form's onSubmit to use handleSubmit; close the Form component by adding a
closing </Form> tag after the inner </form>; and remove the stray </div> (or add
the corresponding opening <div> wrapper if intended) so that tags balance—look
for the Form wrapper, the inner <form onSubmit={handleSubmit}> and the stray
</div> near the end of the block when making the fixes.
In `@app/currency/page.tsx`:
- Around line 130-198: The catch in handleExecute still calls
setSubmitError(...) — replace that with the useApiError hook's setter (e.g.,
setUiError or setApiError) so the ApiErrorDisplay sees the error; update the
catch to call setUiError(e instanceof Error ? e.message : "Operation failed")
(and keep logger.error(`Currency operation failed: ${activeTab}`, e) and
clearError/clearApiError usage intact), and ensure the setter from useApiError
(setUiError/setApiError) is imported/declared in the component scope where
handleExecute is defined.
In `@app/mint/page.tsx`:
- Around line 242-317: There are two duplicate declarations of the async handler
const handleExecuteBurn in the same scope; remove the redundant one and keep the
correct implementation (ensure you preserve the version that wires up
getWalletSecretAnyLocal, Kit fallback flow, submitBurnRedeemSingleClient,
fiatApi.postOffRamp, setTxId and setStep("success")). After deleting the
duplicate, update any references to handleExecuteBurn if needed (e.g., button
onClick or useEffect) so they point to the remaining handleExecuteBurn and run
the app/build to confirm the duplicate identifier error is resolved.
In `@app/send/page.tsx`:
- Around line 3-52: The file is missing imports for useToast and ApiErrorDisplay
which causes compile errors; add the appropriate imports at the top of the
module: import useToast from the toast hook (or the named export used elsewhere
in the repo) and import ApiErrorDisplay from its UI/error component, then use
those symbols (useToast used where called on line ~83 and ApiErrorDisplay where
rendered around line ~530) so the references resolve; ensure the import paths
match your project's existing convention for toast and error-display components.
- Around line 161-251: The handler handleConfirmTransfer still calls removed
helpers clearSubmitError() and handleSubmitError(e); replace clearSubmitError()
with clearError() and replace handleSubmitError(e) with setApiError(e) (imported
from useApiError), ensure useApiError's clearError and setApiError are used in
this component and included in the handleConfirmTransfer dependency array, and
remove any leftover imports/usages of the old submit-error helpers.
- Around line 161-251: handleConfirmTransfer is closing over confirmedAmount and
validating against the live amount, causing stale submissions; update the
validation to use the snapshot confirmedAmount (e.g. change the guard to if
(!confirmedAmount || parseFloat(confirmedAmount) <= 0 || !to) return) and add
confirmedAmount to the useCallback dependency array so handleConfirmTransfer is
re-memoized when the confirmed value changes; keep references to getToValue,
userId, stellarAddress, kit, opts, loadTransfers, refreshBalance as-is.
In `@lib/api/client.ts`:
- Around line 56-58: RequestOptions was stripped of token but code still reads
opts.token and references currentToken; remove all token-handling: delete any
access to opts.token and the currentToken variable usage in the request()
function, and update apiOpts() to stop returning a token field (only return
headers/cookie or whatever remains), ensuring RequestOptions and apiOpts()
signatures match; search for and remove or refactor any token-related logic in
request(), apiOpts(), and any helpers so compilation no longer references the
undefined token/currentToken symbols.
---
Major comments:
In `@app/burn/page.tsx`:
- Around line 364-367: The submit button is still using the removed local
isValid state; update the form to derive validity from React Hook Form instead:
ensure your useForm call includes a mode that validates on change (e.g.,
useForm({... , mode: "onChange"})) and destructure the RHF formState (const {
formState: { isValid } } = useForm(...)) or read form.getState().isValid, remove
the old local isValid state and any setIsValid handlers, and change the Button
disabled prop to disabled={!isValid || loading || isSubmitDisabled} so the
button is driven by RHF's isValid; keep loading and isSubmitDisabled as-is.
- Around line 86-96: The form's defaultValues are overwriting deep-link params;
update the useForm initialization (useForm<BurnFormValues> with resolver
zodResolver(burnSchema)) so defaultValues for acbuAmount and currency are
derived from the existing URL/deep-link values (e.g., the amount and currency
you already read elsewhere on the page) instead of hardcoding "" and "NGN";
preserve the current fallbacks (empty string and "NGN") if the URL values are
absent and ensure types match BurnFormValues.
In `@app/savings/page.tsx`:
- Around line 33-51: resolveUserUri is defined but never used so
setApiUser/getSavingsPositions keep receiving raw identifiers; update the
callers to await resolveUserUri(rawIdentifier, opts) and pass the returned
canonical URI into setApiUser (and into the subsequent getSavingsPositions
invocation) instead of the raw value; specifically locate usages that feed
setApiUser and the call sites around the block referenced (including the other
occurrence around lines 107-109) and replace feeding raw with the awaited
resolveUserUri(...) result, preserving the same opts parameter shape passed to
resolveRecipient.
In `@app/savings/withdraw/page.tsx`:
- Around line 88-90: The form currently allows submission even when the new
recipient confirmation or validation are unresolved; update the submit gating in
handleSubmit to also require confirmDifferentRecipient === true and that
recipientValidationLoading is false and recipientValidationError is falsy, and
mirror those same checks in the submit button's disabled condition so the button
is disabled while recipientValidationLoading is true, recipientValidationError
exists, or confirmDifferentRecipient is false (in addition to the existing
user/amount checks); reference the handleSubmit function, the
confirmDifferentRecipient flag, recipientValidationLoading,
recipientValidationError, and the submit button disabled logic to locate and
apply these changes.
- Around line 153-179: The "Change" button toggles editingRecipient but the
Input (id="withdraw-account") is always readOnly and bound to user, so
recipient, handleRecipientChange, validateRecipient and the confirmation flow
are never used; update the Input to use value={editingRecipient ? recipient :
(resolving ? "Resolving…" : user)}, remove/readOnly only when editingRecipient
is true (i.e., set readOnly={!editingRecipient}), and wire its onChange to
handleRecipientChange (so validateRecipient can run) and ensure handleToggleEdit
resets recipient to user when cancelling (or preserves when switching to edit)
so the confirmation flow can operate on the edited recipient.
In `@app/send/page.tsx`:
- Around line 117-136: loadTransfers and loadContacts share the same loadError
state (setLoadError) so one loader can clear the other's error; introduce
separate error state variables (e.g., transfersError and contactsError with
setTransfersError and setContactsError) and update loadTransfers to
setTransfersError (and not touch contactsError) and update loadContacts to
setContactsError (and not touch transfersError), keep setLoadingTransfers and
setTransfers/setContacts behavior unchanged, and render any combined banner by
deriving it from both transfersError and contactsError instead of a single
shared loadError.
In `@app/wallet/page.tsx`:
- Around line 70-74: The code currently calls storeWalletSecret(userId,
passphrase, passcode) before contacting the backend (userApi.putWalletAddress
and the similar postWalletConfirm flow), which can leave local secrets
inconsistent if the backend call fails; change the flow so you only call
storeWalletSecret (and any local overwrite of the seed/secret) after the backend
call returns a successful response (check result.ok / HTTP 2xx or equivalent)
for userApi.putWalletAddress(newAddress, opts) and the postWalletConfirm path,
and if the backend call fails surface the error to the caller and do not persist
or overwrite local wallet material (or implement an explicit rollback if
needed).
- Around line 299-303: The banner overpromises security because
lib/wallet-storage.ts currently uses btoa(`${passcode}:${secret}`) to “encrypt”
the seed; replace this with real crypto: in the functions that save/load the
secret in lib/wallet-storage.ts (the code path using
btoa(`${passcode}:${secret}`)) derive an encryption key from the passcode (e.g.,
PBKDF2 with a per-item random salt and sufficient iterations), encrypt the
secret with a modern algorithm (AES-GCM) using a random IV, and persist the
ciphertext together with the salt and IV; update the corresponding load/decrypt
function to reverse this (derive key from stored salt and passcode, then
decrypt) and add robust error handling for failed decryption, and if you keep
the UI text, change “stored securely” to a less absolute phrase unless you can
guarantee stronger protections.
In `@components/error-boundary.tsx`:
- Line 17: componentDidCatch currently only logs errors and doesn't wire the new
onError API or persist errorInfo; update the ErrorBoundary.componentDidCatch
method to call this.setState({ error, errorInfo }) to store the React.ErrorInfo
in state (referencing the state field errorInfo) and invoke
this.props.onError?.(error, errorInfo) so consumers receive the callback; ensure
you reference the onError prop and the errorInfo state property names exactly as
declared.
In `@lib/api/client.ts`:
- Around line 95-98: The current logic adds an 'abort' listener to opts.signal
but doesn't handle the case where opts.signal.aborted is already true, so the
internal AbortController (controller) never gets aborted and the request
proceeds; update the code around the opts.signal handling in lib/api/client.ts
to first check if opts.signal.aborted is true and if so call controller.abort()
immediately, otherwise register the opts.signal.addEventListener('abort', () =>
controller.abort(), { once: true }); so that the internal controller is always
aborted when the caller's signal is already aborted or aborts later (refer to
opts.signal and controller in the surrounding function).
---
Minor comments:
In `@app/layout.tsx`:
- Line 9: The AuthGuard import is stale while its wrapper is commented out;
remove the unused import statement "AuthGuard" (and any duplicate unused imports
around the other occurrences mentioned at lines 86-88) from the module, or
alternatively restore the commented wrapper that uses AuthGuard so the import is
actually used—choose one approach and make the code and imports consistent
(update imports/usages for the AuthGuard symbol accordingly).
---
Duplicate comments:
In `@app/mint/page.tsx`:
- Around line 662-666: The Rates tab uses formatRate(r.rate) but formatRate is
not defined or imported in this module; fix it by either importing the helper
(e.g., add an import for formatRate from its utility module) or add a local
formatRate function in app/mint/page.tsx and use it where rateRows.map calls
formatRate; locate the usage in the JSX (rateRows.map((r) => ...
formatRate(r.rate))) and ensure the symbol is exported from the chosen module or
declared before the component so the reference resolves.
In `@app/savings/deposit/page.tsx`:
- Around line 25-27: The page references missing state and imports: declare
resolving and error state via React useState (e.g., const [resolving,
setResolving] = useState(false); const [error, setError] = useState(""); import
React/useState if needed), import or implement resolveUserUri (or import its
module) and import the AlertCircle component from your icon library (e.g.,
lucide-react) so setResolving, setError, resolveUserUri, resolving, and
AlertCircle are defined; alternatively revert the new flow to the previous
implementation by removing these references. Ensure the cancellation flag
(cancelled) is handled consistently with the resolved flow and update any
useEffect cleanup where used.
In `@app/send/page.tsx`:
- Around line 499-505: The Button currently references a removed function
isFormValid(), causing an undefined call; update the disabled prop to use the
memoized boolean isValid instead (i.e., replace disabled={!isFormValid()} with
disabled={!isValid}) so the Button (component using onClick that calls
setConfirmedAmount and setShowConfirmDialog) uses the existing isValid state;
ensure you only change the disabled expression and leave the onClick handlers
(setConfirmedAmount, setShowConfirmDialog) and className intact.
- Around line 103-148: Remove the stray "dev" tokens and the
duplicated/conflicting loadContacts blocks so the useCallback is a single
well-formed function; specifically, keep one loadContacts useCallback that (1)
clears errors via setContactsError(""), (2) awaits userApi.getContacts(opts) and
calls setContacts(data.contacts ?? []), (3) catches errors and sets
setContactsError(message) and calls toast({...}) with the same message, and (4)
calls setLoadingContacts(false) in finally; also ensure there is no duplicated
closing braces or parentheses left over from the merge and that related symbols
(loadContacts, useCallback, userApi.getContacts, setContacts, setContactsError,
setLoadingContacts, toast) are referenced consistently with loadTransfers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 06b87c10-0d7f-41c6-b271-7fd40fce8fb8
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
app/auth/2fa/page.tsxapp/burn/page.tsxapp/business/page.tsxapp/currency/page.tsxapp/error.tsxapp/fiat/page.tsxapp/layout.tsxapp/me/page.tsxapp/mint/page.tsxapp/page.tsxapp/rates/page.tsxapp/reserves/page.tsxapp/savings/deposit/page.tsxapp/savings/page.tsxapp/savings/withdraw/page.tsxapp/send/page.tsxapp/wallet/page.tsxcomponents/error-boundary.tsxcontexts/auth-context.tsxlib/api/client.tslib/wallet-storage.tspackage.json
✅ Files skipped from review due to trivial changes (1)
- app/business/page.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- app/page.tsx
- app/rates/page.tsx
- package.json
- app/me/page.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
19 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #181
Summary by CodeRabbit
Release Notes
New Features
Security Improvements
Bug Fixes & Improvements