Conversation
🦋 Changeset detectedLatest commit: 41fd673 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds provider-aware bridge support across the add-funds flow: new UI components, provider/network URL params, provider-specific onboarding/KYC (bridge vs manteca), multi-deposit ramp quoting (including PIX/BR), server/client API updates, i18n entries, and related utilities/resources. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client as AddFunds/Onboard/KYC UI
participant Server
participant Provider as Bridge/Manteca
participant KYC as Persona
User->>Client: Select currency + provider (+network)
Client->>Server: Fetch ramp providers & tosLink (country, redirectURL)
Client->>User: Show TOS if bridge / show onboarding UI
alt Bridge flow (TOS)
User->>Client: Accept TOS (redirect with signed_agreement_id)
Client->>Server: startRampOnboarding(provider=bridge, acceptedTermsId, network)
else Manteca flow
Client->>Server: startRampOnboarding(provider=manteca)
end
Server->>Provider: Initiate onboarding request
Provider-->>Server: Return inquiryId/sessionToken or none
alt inquiry present
Server-->>Client: Tokens (inquiryId, sessionToken)
Client->>KYC: startAddressKYC or startMantecaKYC (type-based)
KYC-->>Client: KYC result (complete/cancel/error)
Client->>Client: Redirect to /add-funds/kyc or /add-funds/status with provider/network
else no inquiry
Server-->>Client: No inquiry → redirect to status (pending)
end
sequenceDiagram
participant User
participant RampUI as Ramp
participant Server
participant Provider as Bridge/Manteca
participant QRModal
User->>RampUI: Open deposit view (provider, currency, network)
RampUI->>Server: Request quote + deposits (provider-aware)
Server->>Provider: Request deposit details
Provider-->>Server: Return deposits list (PIX/BR/bank/crypto)
Server-->>RampUI: Deliver quote + deposits
RampUI->>User: Render deposits + toggle
alt PIX/BR deposit selected
User->>QRModal: Open QR modal
QRModal-->>User: Display QR/BR code
else bank/crypto deposit
RampUI->>User: Show account/address rows with copy/share
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can disable the changed files summary in the walkthrough.Disable the |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the add funds functionality by introducing Bridge support and refactoring the fiat onboarding flow. It includes UI improvements, new asset and network logos, and a TOS view for Bridge. The changes allow users to deposit funds from various networks and bank accounts, providing a more seamless and user-friendly experience. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for a new 'bridge' provider, significantly enhancing the on-ramp capabilities for both fiat and crypto. The 'Add Funds' flow has been thoughtfully refactored into separate 'Crypto' and 'Fiat' sections, improving user experience and code organization. Key changes include the introduction of generic components like AddRampButton and CopyAddressSheet to handle multiple providers and networks, and the generalization of the KYC and onboarding processes. The addition of new assets, networks, and corresponding UI elements is well-integrated. Overall, this is a substantial and well-executed feature addition. I've included a couple of specific suggestions for improvement.
src/components/add-funds/Ramp.tsx
Outdated
| } | ||
|
|
||
| const styles = StyleSheet.create({ | ||
| pill: { position: "absolute", top: 2, bottom: 2, left: 2, right: 2, borderRadius: 9999 }, |
There was a problem hiding this comment.
In the pill style, right: 2 is redundant and potentially confusing since left and width are both specified for the animated view that uses this style. When left and width are set, right is ignored. Removing it would make the styling clearer.
| pill: { position: "absolute", top: 2, bottom: 2, left: 2, right: 2, borderRadius: 9999 }, | |
| pill: { position: "absolute", top: 2, bottom: 2, left: 2, borderRadius: 9999 }, |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Prevent Tests Dashboard |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/add-funds/Ramp.tsx (2)
98-103:⚠️ Potential issue | 🟠 Major
Minis wired to the FX quote.Line 101 derives
minAmountfromquote.buyRate, and Lines 241-248 render that same field again as the exchange rate. The footer can therefore show the FX rate under aMinlabel instead of the actual deposit floor. Use the real minimum-amount field from the quote/limits payload, or hideMinuntil that value exists.Also applies to: 200-213, 241-248
267-285:⚠️ Potential issue | 🟠 MajorThe QR sheet still shows manteca-only transfer rules in bridge flows.
The main screen already hides the
under your namealert for bridge, but the modal always renders it. That contradicts the bridge copy added insrc/components/add-funds/AddRampButton.tsx(Any account) and can misdirect users right before a transfer.🐛 Proposed fix
- <InfoAlert title={t("All deposits must be from bank accounts under your name.")} /> + {typedProvider === "manteca" && ( + <InfoAlert title={t("All deposits must be from bank accounts under your name.")} /> + )}src/components/add-funds/KYC.tsx (1)
71-79:⚠️ Potential issue | 🟠 MajorKeep
kyc-tokenswhen the user cancels out of KYC.This back handler now clears the cached tokens on cancel, which forces the user to restart onboarding instead of immediately retrying the same KYC step.
Based on learnings, canceling the KYC flow should preserve these tokens; only success or error paths are meant to clear them.🐛 Proposed fix
<Pressable onPress={() => { - queryClient.removeQueries({ queryKey: ["ramp", "kyc-tokens"] }); if (router.canGoBack()) { router.back(); } else { router.replace("/(main)/(home)"); }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1302b94b-88c6-4f98-a8b0-be7e9c522596
⛔ Files ignored due to path filters (16)
src/assets/images/ars-back.svgis excluded by!**/*.svgsrc/assets/images/ars-usdc.svgis excluded by!**/*.svgsrc/assets/images/background.svgis excluded by!**/*.svgsrc/assets/images/brl-back.svgis excluded by!**/*.svgsrc/assets/images/brl-usdc.svgis excluded by!**/*.svgsrc/assets/images/euro-back.svgis excluded by!**/*.svgsrc/assets/images/mxn-back.svgis excluded by!**/*.svgsrc/assets/images/pounds-back.svgis excluded by!**/*.svgsrc/assets/images/solana-network.svgis excluded by!**/*.svgsrc/assets/images/stellar-network.svgis excluded by!**/*.svgsrc/assets/images/tron-network.svgis excluded by!**/*.svgsrc/assets/images/usd-back.svgis excluded by!**/*.svgsrc/assets/images/usd-usdc.svgis excluded by!**/*.svgsrc/assets/images/usdc-centered.svgis excluded by!**/*.svgsrc/assets/images/usdc-front.svgis excluded by!**/*.svgsrc/assets/images/usdt-centered.svgis excluded by!**/*.svg
📒 Files selected for processing (21)
.changeset/modern-jeans-write.mdsrc/components/add-funds/AddCrypto.tsxsrc/components/add-funds/AddFiatButton.tsxsrc/components/add-funds/AddFunds.tsxsrc/components/add-funds/AddFundsOption.tsxsrc/components/add-funds/AddRampButton.tsxsrc/components/add-funds/BridgeDisclaimer.tsxsrc/components/add-funds/KYC.tsxsrc/components/add-funds/MantecaDisclaimer.tsxsrc/components/add-funds/Onboard.tsxsrc/components/add-funds/Ramp.tsxsrc/components/add-funds/Status.tsxsrc/components/shared/CopyAddressSheet.tsxsrc/i18n/es.jsonsrc/i18n/pt.jsonsrc/utils/completeOnboarding.tssrc/utils/currencies.tssrc/utils/networkLogos.tssrc/utils/persona.tssrc/utils/queryClient.tssrc/utils/server.ts
💤 Files with no reviewable changes (1)
- src/components/add-funds/AddFiatButton.tsx
| if (Platform.OS === "web") { | ||
| return React.createElement("iframe", { | ||
| src: uri, | ||
| style: { flex: 1, border: "none", width: "100%", height: "100%" }, | ||
| onLoad: (event: { target: { contentWindow?: { location: { href: string } } } }) => { | ||
| try { | ||
| const url = (event.target as { contentWindow?: { location: { href: string } } }).contentWindow?.location.href; | ||
| if (url?.startsWith(redirectURL)) handleRedirect(url); | ||
| } catch {} // eslint-disable-line no-empty -- cross-origin expected | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Redundant type assertion on event.target.
Line 311 casts event.target to the same type already declared in the onLoad parameter type annotation. This duplication can be removed.
♻️ Proposed simplification
onLoad: (event: { target: { contentWindow?: { location: { href: string } } } }) => {
try {
- const url = (event.target as { contentWindow?: { location: { href: string } } }).contentWindow?.location.href;
+ const url = event.target.contentWindow?.location.href;
if (url?.startsWith(redirectURL)) handleRedirect(url);
} catch {} // eslint-disable-line no-empty -- cross-origin expected
},📝 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.
| if (Platform.OS === "web") { | |
| return React.createElement("iframe", { | |
| src: uri, | |
| style: { flex: 1, border: "none", width: "100%", height: "100%" }, | |
| onLoad: (event: { target: { contentWindow?: { location: { href: string } } } }) => { | |
| try { | |
| const url = (event.target as { contentWindow?: { location: { href: string } } }).contentWindow?.location.href; | |
| if (url?.startsWith(redirectURL)) handleRedirect(url); | |
| } catch {} // eslint-disable-line no-empty -- cross-origin expected | |
| }, | |
| }); | |
| } | |
| if (Platform.OS === "web") { | |
| return React.createElement("iframe", { | |
| src: uri, | |
| style: { flex: 1, border: "none", width: "100%", height: "100%" }, | |
| onLoad: (event: { target: { contentWindow?: { location: { href: string } } } }) => { | |
| try { | |
| const url = event.target.contentWindow?.location.href; | |
| if (url?.startsWith(redirectURL)) handleRedirect(url); | |
| } catch {} // eslint-disable-line no-empty -- cross-origin expected | |
| }, | |
| }); | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/add-funds/KYC.tsx (1)
71-79:⚠️ Potential issue | 🟠 MajorKeep the cached KYC tokens when the user backs out.
This clears
["ramp", "kyc-tokens"]before navigation, so returning to the KYC step forces a full restart instead of resuming. The success and error paths already remove the cache entry; the back path should preserve it.↩️ Proposed fix
<Pressable onPress={() => { - queryClient.removeQueries({ queryKey: ["ramp", "kyc-tokens"] }); if (router.canGoBack()) { router.back(); } else {Based on learnings, the add-funds KYC flow intentionally keeps these tokens on cancel/back-out paths and only clears them on success or error.
♻️ Duplicate comments (2)
src/components/add-funds/Ramp.tsx (1)
344-394:⚠️ Potential issue | 🟠 MajorLocalize the deposit detail labels.
depositRows()hard-codes the new bank-detail labels in English, so translated builds will still show mixed-language instructions on this screen. Pass these labels throught(), or build the rows where translation is available.Based on learnings, i18n keys in this repo should use the full user-facing message verbatim.
src/components/add-funds/Onboard.tsx (1)
305-316: 🧹 Nitpick | 🔵 TrivialRedundant type assertion on
event.target.Line 311 casts
event.targetto the same type already declared in theonLoadparameter type annotation. This duplication can be removed.♻️ Proposed simplification
onLoad: (event: { target: { contentWindow?: { location: { href: string } } } }) => { try { - const url = (event.target as { contentWindow?: { location: { href: string } } }).contentWindow?.location.href; + const url = event.target.contentWindow?.location.href; if (url?.startsWith(redirectURL)) handleRedirect(url); } catch {} // eslint-disable-line no-empty -- cross-origin expected },
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: afae625d-60b7-49c5-b2f6-14e6d97471b4
⛔ Files ignored due to path filters (16)
src/assets/images/ars-back.svgis excluded by!**/*.svgsrc/assets/images/ars-usdc.svgis excluded by!**/*.svgsrc/assets/images/background.svgis excluded by!**/*.svgsrc/assets/images/brl-back.svgis excluded by!**/*.svgsrc/assets/images/brl-usdc.svgis excluded by!**/*.svgsrc/assets/images/euro-back.svgis excluded by!**/*.svgsrc/assets/images/mxn-back.svgis excluded by!**/*.svgsrc/assets/images/pounds-back.svgis excluded by!**/*.svgsrc/assets/images/solana-network.svgis excluded by!**/*.svgsrc/assets/images/stellar-network.svgis excluded by!**/*.svgsrc/assets/images/tron-network.svgis excluded by!**/*.svgsrc/assets/images/usd-back.svgis excluded by!**/*.svgsrc/assets/images/usd-usdc.svgis excluded by!**/*.svgsrc/assets/images/usdc-centered.svgis excluded by!**/*.svgsrc/assets/images/usdc-front.svgis excluded by!**/*.svgsrc/assets/images/usdt-centered.svgis excluded by!**/*.svg
📒 Files selected for processing (21)
.changeset/modern-jeans-write.mdsrc/components/add-funds/AddCrypto.tsxsrc/components/add-funds/AddFiatButton.tsxsrc/components/add-funds/AddFunds.tsxsrc/components/add-funds/AddFundsOption.tsxsrc/components/add-funds/AddRampButton.tsxsrc/components/add-funds/BridgeDisclaimer.tsxsrc/components/add-funds/KYC.tsxsrc/components/add-funds/MantecaDisclaimer.tsxsrc/components/add-funds/Onboard.tsxsrc/components/add-funds/Ramp.tsxsrc/components/add-funds/Status.tsxsrc/components/shared/CopyAddressSheet.tsxsrc/i18n/es.jsonsrc/i18n/pt.jsonsrc/utils/completeOnboarding.tssrc/utils/currencies.tssrc/utils/networkLogos.tssrc/utils/persona.tssrc/utils/queryClient.tssrc/utils/server.ts
💤 Files with no reviewable changes (1)
- src/components/add-funds/AddFiatButton.tsx
| const mantecaOnramp = providers?.manteca.onramp; | ||
| const limits = mantecaOnramp && "limits" in mantecaOnramp ? mantecaOnramp.limits : undefined; | ||
| const limitCurrency = limits?.monthly.symbol; | ||
| const minAmount = quote?.buyRate ? Number(quote.buyRate) : undefined; | ||
| const maxAmount = limits?.monthly.available ? Number(limits.monthly.available) : undefined; |
There was a problem hiding this comment.
Don't display the exchange rate as the minimum deposit.
Line 101 derives minAmount from quote.buyRate, but Lines 241-249 already use that same field as the exchange rate. The footer will now show the FX rate under the Min label, which is incorrect. Read the minimum from the actual limits/quote field, or hide that label until the API exposes one.
cruzdanilo
force-pushed
the
i18n
branch
2 times, most recently
from
580ed79 to
7acee10
Compare
| const isCrypto = !!network; | ||
| const isBridge = provider === "bridge"; | ||
|
|
||
| const [acknowledged, setAcknowledged] = useState(true); |
There was a problem hiding this comment.
🚩 Acknowledged checkbox defaults to true (pre-checked)
At src/components/add-funds/Onboard.tsx:77, acknowledged is initialized to true, meaning the terms checkbox starts pre-checked. This was present in the old code too (same useState(true)), so it's not a regression. Whether this is appropriate UX for a terms acceptance flow (especially for the new Bridge provider) is a product decision, but regulators sometimes require affirmative opt-in (unchecked by default). Worth flagging for product review.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
src/components/add-funds/Ramp.tsx (2)
100-103:⚠️ Potential issue | 🟠 MajorDo not derive “Min” amount from exchange rate.
Line 101 sets
minAmountfromquote.buyRate, butbuyRateis already displayed as exchange rate (Line 248). This shows incorrect data under the Min label.
344-393:⚠️ Potential issue | 🟠 MajorLocalize deposit-row labels instead of hard-coded English text.
depositRows()returns raw English labels (for example, “Beneficiary name”, “Account number”), so localized builds will show mixed-language account instructions.Based on learnings, i18n keys should use full-sentence strings and avoid non-localized descriptive keys/text in UI.
src/components/add-funds/AddRampButton.tsx (1)
34-36:⚠️ Potential issue | 🟠 MajorPreserve
pending: "true"for ONBOARDING navigation.Line 35 drops the pending flag, so the status screen treats the flow as non-pending and skips the delayed activation re-check.
🧭 Proposed fix
- router.push({ pathname: "/add-funds/status", params: { ...params, status: "ONBOARDING" } }); + router.push({ pathname: "/add-funds/status", params: { ...params, status: "ONBOARDING", pending: "true" } });Based on learnings,
src/components/add-funds/Status.tsxintentionally performs a single delayed provider check only for pending routes.src/components/add-funds/Onboard.tsx (1)
309-320: 🧹 Nitpick | 🔵 TrivialRedundant type assertion on
event.target.Line 315 casts
event.targetto the same type already declared in theonLoadparameter type annotation. This duplication can be removed.♻️ Proposed simplification
onLoad: (event: { target: { contentWindow?: { location: { href: string } } } }) => { try { - const url = (event.target as { contentWindow?: { location: { href: string } } }).contentWindow?.location.href; + const url = event.target.contentWindow?.location.href; if (url?.startsWith(redirectURL)) handleRedirect(url); } catch {} // eslint-disable-line no-empty -- cross-origin expected },src/components/add-funds/AddCrypto.tsx (1)
46-51: 🧹 Nitpick | 🔵 TrivialType assertion bypasses compile-time safety.
Line 48 uses
as Parameters<typeof getRampQuote>[0]to force-cast the object. Since the query is only enabled whenisBridgeis true (which requirescurrencyandnetworkto be defined), this is safe at runtime. However, if theisBridgecondition logic changes, this could introduce silent failures.Consider adding explicit type narrowing or a type guard to make the relationship clearer.
♻️ Alternative with explicit narrowing
const { data } = useQuery({ queryKey: ["ramp", "quote", "bridge", currency, network], - queryFn: () => getRampQuote({ provider: "bridge", currency, network } as Parameters<typeof getRampQuote>[0]), + queryFn: () => { + if (!currency || !network) throw new Error("missing currency or network"); + return getRampQuote({ provider: "bridge", currency, network }); + }, enabled: isBridge, staleTime: 10_000, });
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f9a0e244-6dfe-4abb-abbb-e34bf29cc395
⛔ Files ignored due to path filters (16)
src/assets/images/ars-back.svgis excluded by!**/*.svgsrc/assets/images/ars-usdc.svgis excluded by!**/*.svgsrc/assets/images/background.svgis excluded by!**/*.svgsrc/assets/images/brl-back.svgis excluded by!**/*.svgsrc/assets/images/brl-usdc.svgis excluded by!**/*.svgsrc/assets/images/euro-back.svgis excluded by!**/*.svgsrc/assets/images/mxn-back.svgis excluded by!**/*.svgsrc/assets/images/pounds-back.svgis excluded by!**/*.svgsrc/assets/images/solana-network.svgis excluded by!**/*.svgsrc/assets/images/stellar-network.svgis excluded by!**/*.svgsrc/assets/images/tron-network.svgis excluded by!**/*.svgsrc/assets/images/usd-back.svgis excluded by!**/*.svgsrc/assets/images/usd-usdc.svgis excluded by!**/*.svgsrc/assets/images/usdc-centered.svgis excluded by!**/*.svgsrc/assets/images/usdc-front.svgis excluded by!**/*.svgsrc/assets/images/usdt-centered.svgis excluded by!**/*.svg
📒 Files selected for processing (22)
.changeset/modern-jeans-write.mdcspell.jsonsrc/components/add-funds/AddCrypto.tsxsrc/components/add-funds/AddFiatButton.tsxsrc/components/add-funds/AddFunds.tsxsrc/components/add-funds/AddFundsOption.tsxsrc/components/add-funds/AddRampButton.tsxsrc/components/add-funds/BridgeDisclaimer.tsxsrc/components/add-funds/KYC.tsxsrc/components/add-funds/MantecaDisclaimer.tsxsrc/components/add-funds/Onboard.tsxsrc/components/add-funds/Ramp.tsxsrc/components/add-funds/Status.tsxsrc/components/shared/CopyAddressSheet.tsxsrc/i18n/es.jsonsrc/i18n/pt.jsonsrc/utils/completeOnboarding.tssrc/utils/currencies.tssrc/utils/networkLogos.tssrc/utils/persona.tssrc/utils/queryClient.tssrc/utils/server.ts
💤 Files with no reviewable changes (1)
- src/components/add-funds/AddFiatButton.tsx
| const { | ||
| i18n: { language }, | ||
| } = useTranslation(); | ||
| const locale = language === "es" ? "es" : "en"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/components/add-funds/MantecaDisclaimer.tsx | head -50Repository: exactly/exa
Length of output: 2230
🌐 Web query:
react-i18next i18n.language locale format codes
💡 Result:
react-i18next just exposes the underlying i18next instance, so i18n.language is whatever language code string i18next has currently selected/detected (e.g. "en", "en-US", "pt-BR", "zh-Hant-HK"). i18next calls these values “language codes”. [1][2]
What format should those codes be?
- You can use any format, but i18next strongly recommends BCP 47 language tags if you want built-in features (plural rules, fallback resolution) to work as intended:
lng-(script)-REGION-(extensions)likeen,en-US,en-GB,zh-Hant-HK. [1] - When your language code contains a dash (
-), i18next will try to format it usingIntl.getCanonicalLocales(i.e., canonical BCP 47 normalization). [1]
Related i18next options that affect the language string you’ll see in i18n.language
load: controls which variants i18next will look up when the current language is specific (e.g.en-US):'all'→ tries['en-US', 'en', 'dev']'currentOnly'→ onlyen-US'languageOnly'→ onlyen[3]
lowerCaseLng: if enabled, lowercases the full locale (e.g.en-US→en-us). [3]cleanCode: if enabled, lowercases just the main language (e.g.EN→en) while keeping region/script casing for full tags (e.g.en-US). [3]
Practical tip (using i18n.language as an Intl “locale”)
If you pass i18n.language to things like Intl.DateTimeFormat, prefer keeping it BCP-47-ish (hyphenated, not underscored) and don’t enable lowerCaseLng unless you really want all-lowercase tags. [1][3]
Sources:
[1] i18next FAQ – language code formatting (BCP 47, canonicalization)
[2] i18next “Translation Resolution” – examples of language codes (en-US, pt-BR, etc.)
[3] i18next Configuration Options – load, lowerCaseLng, cleanCode
Handle regional Spanish locales in locale mapping.
Line 15 only matches "es" exactly. i18n.language can include regional values (e.g., es-AR, es-MX, es-CO), which would incorrectly fall back to English terms.
🔧 Proposed fix
- const locale = language === "es" ? "es" : "en";
+ const locale = language.toLowerCase().startsWith("es") ? "es" : "en";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { | |
| i18n: { language }, | |
| } = useTranslation(); | |
| const locale = language === "es" ? "es" : "en"; | |
| const { | |
| i18n: { language }, | |
| } = useTranslation(); | |
| const locale = language.toLowerCase().startsWith("es") ? "es" : "en"; |
| if (Platform.OS === "web") { | ||
| return React.createElement("iframe", { | ||
| src: uri, | ||
| style: { flex: 1, border: "none", width: "100%", height: "100%" }, | ||
| onLoad: (event: { target: { contentWindow?: { location: { href: string } } } }) => { | ||
| try { | ||
| const url = (event.target as { contentWindow?: { location: { href: string } } }).contentWindow?.location.href; | ||
| if (url?.startsWith(redirectURL)) handleRedirect(url); | ||
| } catch {} // eslint-disable-line no-empty -- cross-origin expected | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🚩 TOSView iframe redirect detection relies on same-origin access pattern
The TOSView web implementation at src/components/add-funds/Onboard.tsx:309-319 uses an iframe with an onLoad handler that tries to read contentWindow.location.href. This will throw cross-origin errors for the initial Bridge TOS page (hosted on bridge.xyz). The empty catch block silently handles this. When Bridge redirects to the app's redirectURL (same-origin https://{domain}/add-funds), the onLoad fires again and the same-origin URL is now readable. This pattern works for server-side redirects (HTTP 302) but may fail if Bridge uses client-side JavaScript navigation within the iframe, as onLoad doesn't fire for SPA-style route changes. The native WebView path (onShouldStartLoadWithRequest) is more robust since it intercepts all navigation requests. Worth verifying Bridge's redirect mechanism.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
src/components/add-funds/Ramp.tsx (2)
101-101:⚠️ Potential issue | 🟠 MajorDon’t derive “Min” from the exchange rate.
Line 101 assigns
minAmountfromquote.buyRate, while Lines 241-249 already treatbuyRateas FX. That makes the footer show exchange rate values under the Min label.Also applies to: 207-213, 241-249
344-394:⚠️ Potential issue | 🟠 MajorLocalize deposit detail labels before rendering.
Lines 344-394 hard-code labels in English, and
DetailRowrenders them as-is, so localized screens still show mixed-language account details.Based on learnings, i18n in this repository should use user-facing message keys consistently, and these labels already have translated entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 23cced28-30d9-44ff-ae64-099d05d611d6
⛔ Files ignored due to path filters (16)
src/assets/images/ars-back.svgis excluded by!**/*.svgsrc/assets/images/ars-usdc.svgis excluded by!**/*.svgsrc/assets/images/background.svgis excluded by!**/*.svgsrc/assets/images/brl-back.svgis excluded by!**/*.svgsrc/assets/images/brl-usdc.svgis excluded by!**/*.svgsrc/assets/images/euro-back.svgis excluded by!**/*.svgsrc/assets/images/mxn-back.svgis excluded by!**/*.svgsrc/assets/images/pounds-back.svgis excluded by!**/*.svgsrc/assets/images/solana-network.svgis excluded by!**/*.svgsrc/assets/images/stellar-network.svgis excluded by!**/*.svgsrc/assets/images/tron-network.svgis excluded by!**/*.svgsrc/assets/images/usd-back.svgis excluded by!**/*.svgsrc/assets/images/usd-usdc.svgis excluded by!**/*.svgsrc/assets/images/usdc-centered.svgis excluded by!**/*.svgsrc/assets/images/usdc-front.svgis excluded by!**/*.svgsrc/assets/images/usdt-centered.svgis excluded by!**/*.svg
📒 Files selected for processing (22)
.changeset/modern-jeans-write.mdcspell.jsonsrc/components/add-funds/AddCrypto.tsxsrc/components/add-funds/AddFiatButton.tsxsrc/components/add-funds/AddFunds.tsxsrc/components/add-funds/AddFundsOption.tsxsrc/components/add-funds/AddRampButton.tsxsrc/components/add-funds/BridgeDisclaimer.tsxsrc/components/add-funds/KYC.tsxsrc/components/add-funds/MantecaDisclaimer.tsxsrc/components/add-funds/Onboard.tsxsrc/components/add-funds/Ramp.tsxsrc/components/add-funds/Status.tsxsrc/components/shared/CopyAddressSheet.tsxsrc/i18n/es.jsonsrc/i18n/pt.jsonsrc/utils/completeOnboarding.tssrc/utils/currencies.tssrc/utils/networkLogos.tssrc/utils/persona.tssrc/utils/queryClient.tssrc/utils/server.ts
💤 Files with no reviewable changes (1)
- src/components/add-funds/AddFiatButton.tsx
| const { mutateAsync: handleContinue, isPending } = useMutation({ | ||
| mutationKey: ["kyc", "complete", "manteca"], | ||
| mutationKey: ["kyc", "complete", provider], | ||
| async mutationFn() { | ||
| if (!currency) return; | ||
| const result = await startMantecaKYC(invalidLegalId); | ||
| const result = isAddress ? await startAddressKYC(storedTokens) : await startMantecaKYC(storedTokens); | ||
| if (result.status === "cancel") return; | ||
| if (result.status !== "complete") { | ||
| queryClient.removeQueries({ queryKey: ["ramp", "invalid-legal-id"] }); | ||
| router.replace({ pathname: "/add-funds/status", params: { status: "error", currency } }); | ||
| queryClient.removeQueries({ queryKey: ["ramp", "kyc-tokens"] }); | ||
| router.replace({ | ||
| pathname: "/add-funds/status", | ||
| params: { status: "error", currency, provider, network }, | ||
| }); | ||
| return; | ||
| } | ||
| queryClient.removeQueries({ queryKey: ["ramp", "invalid-legal-id"] }); | ||
| await completeOnboarding(router, currency); | ||
| queryClient.removeQueries({ queryKey: ["ramp", "kyc-tokens"] }); | ||
| await completeOnboarding(router, currency, isBridge ? "bridge" : "manteca", acceptedTermsId, network); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Mutation key may contain undefined provider.
The mutation key ["kyc", "complete", provider] can include undefined when provider is not in the URL params. While this works functionally, it may cause unintended cache collisions with queries that also use undefined in keys.
♻️ Consider defaulting provider in the key
const { mutateAsync: handleContinue, isPending } = useMutation({
- mutationKey: ["kyc", "complete", provider],
+ mutationKey: ["kyc", "complete", provider ?? "manteca"],| t, | ||
| i18n: { language }, | ||
| } = useTranslation(); | ||
| const locale = language === "es" ? "es" : "en"; |
There was a problem hiding this comment.
Handle regional Spanish locales in locale mapping.
Same issue as MantecaDisclaimer.tsx: exact match on "es" will miss regional variants like es-AR, es-MX.
🐛 Proposed fix
- const locale = language === "es" ? "es" : "en";
+ const locale = language.toLowerCase().startsWith("es") ? "es" : "en";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const locale = language === "es" ? "es" : "en"; | |
| const locale = language.toLowerCase().startsWith("es") ? "es" : "en"; |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/add-funds/KYC.tsx (1)
71-79:⚠️ Potential issue | 🟡 MinorKeep
kyc-tokenson the back path.This back handler now clears the cached inquiry/session before the user leaves the screen, so “go back and retry” becomes a full restart instead of a resume.
♻️ Suggested fix
<Pressable onPress={() => { - queryClient.removeQueries({ queryKey: ["ramp", "kyc-tokens"] }); if (router.canGoBack()) { router.back(); } else {Based on learnings, the
["ramp", "kyc-tokens"]entry should stay cached on KYC cancellation and only be cleared on success or error paths.src/components/add-funds/Onboard.tsx (1)
235-254:⚠️ Potential issue | 🟠 MajorAdd checkbox semantics to the consent control.
This is a required acknowledgement gate, but the tappable square is only an
XStackwith no checkbox role or checked state, so screen readers can’t announce or operate it reliably.♿ Suggested fix
<XStack cursor="pointer" + accessibilityRole="checkbox" + accessibilityState={{ checked: acknowledged }} + accessibilityLabel={t("I accept the Terms and Conditions.")} onPress={() => { setAcknowledged(!acknowledged); }} >
♻️ Duplicate comments (7)
src/components/add-funds/MantecaDisclaimer.tsx (1)
15-15:⚠️ Potential issue | 🟡 MinorLocale mapping misses regional Spanish codes.
Line 15 only matches
"es"exactly. Ifi18n.languageises-AR,es-MX, etc., the URL falls back to/en/unintentionally.🔧 Proposed fix
- const locale = language === "es" ? "es" : "en"; + const locale = language.toLowerCase().startsWith("es") ? "es" : "en";In i18next/react-i18next, can `i18n.language` be a regional tag like `es-AR` or `es-MX`, and what is the recommended way to branch locale URLs for Spanish?Also applies to: 41-41
src/components/add-funds/AddFundsOption.tsx (1)
23-31:⚠️ Potential issue | 🟡 MinorExpose disabled semantics to accessibility APIs.
This card is visually/functionally disabled, but assistive tech still gets no explicit button role or disabled state. Add those here so the unavailable option is announced correctly.
♿ Suggested fix
<YStack + accessibilityRole="button" + accessibilityState={{ disabled: !!disabled }} padding="$s4_5" backgroundColor="$backgroundSoft" borderRadius="$r5" borderWidth={1}src/components/add-funds/AddRampButton.tsx (1)
34-35:⚠️ Potential issue | 🟠 MajorPreserve
pending: "true"on ONBOARDING routes.
Status.tsxonly runs its delayed provider recheck when that flag is present. Without it, onboarding users land on the static status screen and never auto-redirect when the provider becomes ACTIVE.🧭 Suggested fix
case "ONBOARDING": - router.push({ pathname: "/add-funds/status", params: { ...params, status: "ONBOARDING" } }); + router.push({ pathname: "/add-funds/status", params: { ...params, status: "ONBOARDING", pending: "true" } }); break;Based on learnings,
src/components/add-funds/Status.tsxintentionally performs the delayed provider activation check only for pending routes.src/components/add-funds/Onboard.tsx (1)
60-64:⚠️ Potential issue | 🟡 MinorNormalize locale prefixes before composing help URLs.
Exact equality on
languagedrops supported regional locales likees-AR,es-MX, andpt-BRto the English article. Match on the prefix instead of the full locale code here.Also applies to: 160-162
src/components/add-funds/Ramp.tsx (2)
98-102:⚠️ Potential issue | 🟠 Major
minAmountis still derived fromquote.buyRate, which is the exchange rate.This was flagged in a previous review. Line 101 sets
minAmountfrombuyRate, but Lines 241-249 display that same value as the exchange rate. The footer will show the FX rate under the Min label, which is misleading. Either read the actual minimum from the API limits or remove this display until a proper field is available.
344-394:⚠️ Potential issue | 🟠 MajorHard-coded English labels in
depositRows()break localization.This was flagged in a previous review. All row labels ("Beneficiary name", "Account number", "Routing number", etc.) are in English. Localized builds will display mixed-language bank instructions. Route these through
t().src/components/add-funds/AddCrypto.tsx (1)
148-151: 🧹 Nitpick | 🔵 TrivialConditional spread pattern for optional props.
This was noted in a prior review as a nitpick. The pattern works correctly—when
isBridge && network && depositAddressis truthy, the additional props are spread. While explicit prop assignment would be clearer, this is functionally correct and the component handles missing props gracefully.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: abc7f336-516e-4142-babb-d084b0796d23
⛔ Files ignored due to path filters (16)
src/assets/images/ars-back.svgis excluded by!**/*.svgsrc/assets/images/ars-usdc.svgis excluded by!**/*.svgsrc/assets/images/background.svgis excluded by!**/*.svgsrc/assets/images/brl-back.svgis excluded by!**/*.svgsrc/assets/images/brl-usdc.svgis excluded by!**/*.svgsrc/assets/images/euro-back.svgis excluded by!**/*.svgsrc/assets/images/mxn-back.svgis excluded by!**/*.svgsrc/assets/images/pounds-back.svgis excluded by!**/*.svgsrc/assets/images/solana-network.svgis excluded by!**/*.svgsrc/assets/images/stellar-network.svgis excluded by!**/*.svgsrc/assets/images/tron-network.svgis excluded by!**/*.svgsrc/assets/images/usd-back.svgis excluded by!**/*.svgsrc/assets/images/usd-usdc.svgis excluded by!**/*.svgsrc/assets/images/usdc-centered.svgis excluded by!**/*.svgsrc/assets/images/usdc-front.svgis excluded by!**/*.svgsrc/assets/images/usdt-centered.svgis excluded by!**/*.svg
📒 Files selected for processing (22)
.changeset/modern-jeans-write.mdcspell.jsonsrc/components/add-funds/AddCrypto.tsxsrc/components/add-funds/AddFiatButton.tsxsrc/components/add-funds/AddFunds.tsxsrc/components/add-funds/AddFundsOption.tsxsrc/components/add-funds/AddRampButton.tsxsrc/components/add-funds/BridgeDisclaimer.tsxsrc/components/add-funds/KYC.tsxsrc/components/add-funds/MantecaDisclaimer.tsxsrc/components/add-funds/Onboard.tsxsrc/components/add-funds/Ramp.tsxsrc/components/add-funds/Status.tsxsrc/components/shared/CopyAddressSheet.tsxsrc/i18n/es.jsonsrc/i18n/pt.jsonsrc/utils/completeOnboarding.tssrc/utils/currencies.tssrc/utils/networkLogos.tssrc/utils/persona.tssrc/utils/queryClient.tssrc/utils/server.ts
💤 Files with no reviewable changes (1)
- src/components/add-funds/AddFiatButton.tsx
| const deposits = data?.depositInfo ?? []; | ||
| const quote = data?.quote; | ||
| const beneficiaryName = depositInfo && "beneficiaryName" in depositInfo ? depositInfo.beneficiaryName : undefined; | ||
| const pixKey = depositInfo?.network === "PIX" ? depositInfo.pixKey : undefined; | ||
| const pix = useMemo(() => { | ||
| if (!pixKey || !beneficiaryName) return null; | ||
| const activeDeposit = deposits.find((d) => d.network === selectedNetwork) ?? deposits.at(0); | ||
| const pixDeposit = deposits.find((d) => d.network === "PIX" || d.network === "PIX-BR"); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
selectedNetwork state is never reset when deposits change.
If the user switches currencies or providers and deposits changes, selectedNetwork may reference a network that no longer exists in the new array. The fallback deposits.at(0) handles this gracefully, but the stale state remains. Consider resetting selectedNetwork when the query key changes.
| const displayAddress = overrideAddress ?? accountAddress; | ||
| const displayNetwork = network ?? chain.name; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Inline single-use display aliases to match repository style rules.
displayAddress and displayNetwork are only used once each, so keep them inline.
♻️ Proposed refactor
- const displayAddress = overrideAddress ?? accountAddress;
- const displayNetwork = network ?? chain.name;
const displayAssets = assets ?? supportedAssets;
@@
- {displayAddress}
+ {overrideAddress ?? accountAddress}
@@
- {displayNetwork}
+ {network ?? chain.name}As per coding guidelines: do not extract a value into a variable unless it is used in two or more places.
Also applies to: 65-65, 90-90
src/utils/currencies.ts
Outdated
| EUR: { name: "Euros", emoji: "🇪🇺" }, | ||
| GBP: { name: "British Pounds", emoji: "🇬🇧" }, | ||
| MXN: { name: "Mexican Pesos", emoji: "🇲🇽" }, |
There was a problem hiding this comment.
Global Currency now includes values invalid for Manteca quote paths.
Line 6–8 broadens Currency for all flows, but server quote validation is provider-specific. EUR/GBP/MXN are valid for Bridge and rejected for Manteca, so this weakens compile-time guarantees and can surface as runtime 4xxs.
🛠️ Suggested direction
export const currencies = {
ARS: { name: "Argentine Pesos", emoji: "🇦🇷" },
BRL: { name: "Brazilian Real", emoji: "🇧🇷" },
EUR: { name: "Euros", emoji: "🇪🇺" },
GBP: { name: "British Pounds", emoji: "🇬🇧" },
MXN: { name: "Mexican Pesos", emoji: "🇲🇽" },
USD: { name: "US Dollars", emoji: "🇺🇸" },
} as const;
+export const mantecaCurrencies = ["ARS", "BRL", "USD"] as const;
+export type MantecaCurrency = (typeof mantecaCurrencies)[number];
+export const bridgeFiatCurrencies = ["BRL", "EUR", "GBP", "MXN", "USD"] as const;
+export type BridgeFiatCurrency = (typeof bridgeFiatCurrencies)[number];Then type provider-specific request builders with MantecaCurrency vs BridgeFiatCurrency.
📝 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.
| EUR: { name: "Euros", emoji: "🇪🇺" }, | |
| GBP: { name: "British Pounds", emoji: "🇬🇧" }, | |
| MXN: { name: "Mexican Pesos", emoji: "🇲🇽" }, | |
| export const currencies = { | |
| ARS: { name: "Argentine Pesos", emoji: "🇦🇷" }, | |
| BRL: { name: "Brazilian Real", emoji: "🇧🇷" }, | |
| EUR: { name: "Euros", emoji: "🇪🇺" }, | |
| GBP: { name: "British Pounds", emoji: "🇬🇧" }, | |
| MXN: { name: "Mexican Pesos", emoji: "🇲🇽" }, | |
| USD: { name: "US Dollars", emoji: "🇺🇸" }, | |
| } as const; | |
| export const mantecaCurrencies = ["ARS", "BRL", "USD"] as const; | |
| export type MantecaCurrency = (typeof mantecaCurrencies)[number]; | |
| export const bridgeFiatCurrencies = ["BRL", "EUR", "GBP", "MXN", "USD"] as const; | |
| export type BridgeFiatCurrency = (typeof bridgeFiatCurrencies)[number]; |
| const networkLogos: Record<string, string> = { | ||
| SOLANA: "https://raw.githubusercontent.com/trustwallet/assets/master/blockchains/solana/info/logo.png", | ||
| STELLAR: "https://raw.githubusercontent.com/trustwallet/assets/master/blockchains/stellar/info/logo.png", | ||
| TRON: "https://raw.githubusercontent.com/trustwallet/assets/master/blockchains/tron/info/logo.png", | ||
| }; | ||
|
|
||
| export default networkLogos; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Inline the default export to satisfy repository export/extraction rules.
networkLogos is extracted only for a single export, and default export is placed at the bottom. Inline export keeps this file guideline-compliant.
♻️ Proposed refactor
-const networkLogos: Record<string, string> = {
+export default {
SOLANA: "https://raw.githubusercontent.com/trustwallet/assets/master/blockchains/solana/info/logo.png",
STELLAR: "https://raw.githubusercontent.com/trustwallet/assets/master/blockchains/stellar/info/logo.png",
TRON: "https://raw.githubusercontent.com/trustwallet/assets/master/blockchains/tron/info/logo.png",
-};
-
-export default networkLogos;
+} satisfies Record<string, string>;As per coding guidelines: place the default export at the top, and do not extract single-use values into variables.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const networkLogos: Record<string, string> = { | |
| SOLANA: "https://raw.githubusercontent.com/trustwallet/assets/master/blockchains/solana/info/logo.png", | |
| STELLAR: "https://raw.githubusercontent.com/trustwallet/assets/master/blockchains/stellar/info/logo.png", | |
| TRON: "https://raw.githubusercontent.com/trustwallet/assets/master/blockchains/tron/info/logo.png", | |
| }; | |
| export default networkLogos; | |
| export default { | |
| SOLANA: "https://raw.githubusercontent.com/trustwallet/assets/master/blockchains/solana/info/logo.png", | |
| STELLAR: "https://raw.githubusercontent.com/trustwallet/assets/master/blockchains/stellar/info/logo.png", | |
| TRON: "https://raw.githubusercontent.com/trustwallet/assets/master/blockchains/tron/info/logo.png", | |
| } satisfies Record<string, string>; |
| function startRampKYC(type: "bridge" | "manteca", tokens?: { inquiryId: string; sessionToken: string }) { | ||
| if (current && !current.controller.signal.aborted && current.type === type && current.tokens === tokens) | ||
| return current.promise; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Deduplication check compares tokens by reference.
Line 134 checks current.tokens === tokens using reference equality. If the caller creates a new object with the same inquiryId and sessionToken values, this check will fail and a new inquiry will start. This is likely intentional (tokens from different calls are treated as different flows), but worth noting.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| case "WIRE": | ||
| return [ | ||
| { label: "Beneficiary name", value: deposit.beneficiaryName }, | ||
| { label: "Beneficiary address", value: deposit.beneficiaryAddress }, |
There was a problem hiding this comment.
🔴 String literal instead of property access for beneficiary address in ACH/WIRE deposits
In depositRows for ACH/WIRE networks, the beneficiary address value is a string literal "deposit.beneficiaryAddress" instead of the property access deposit.beneficiaryAddress. Every other row in this function and neighboring cases correctly uses property access (e.g., deposit.beneficiaryName, deposit.accountNumber). This causes the UI to display the literal text "deposit.beneficiaryAddress" to users and copy that literal string when they tap the copy button, instead of showing and copying the actual beneficiary address.
| { label: "Beneficiary address", value: deposit.beneficiaryAddress }, | |
| { label: "Beneficiary address", value: deposit.beneficiaryAddress }, |
Was this helpful? React with 👍 or 👎 to provide feedback.
| </View> | ||
| } | ||
| title={t("{{currency}} from {{network}}", { currency, network })} | ||
| subtitle={t("Receive USDC on Optimism")} |
There was a problem hiding this comment.
🚩 Hardcoded "Receive USDC on Optimism" subtitle for crypto bridge options
At src/components/add-funds/AddRampButton.tsx:58, the crypto bridge subtitle is hardcoded as t("Receive USDC on Optimism") regardless of the source currency (could be USDT) or destination. This appears intentional since the bridge converts all incoming crypto to USDC on the Exa wallet's chain (Optimism), but it's worth confirming this is the desired UX — especially if other destination assets become supported in the future. The non-crypto fiat subtitle at line 76 similarly says t("Receive USDC") which is consistent.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const pixKey = depositInfo?.network === "PIX" ? depositInfo.pixKey : undefined; | ||
| const pix = useMemo(() => { | ||
| if (!pixKey || !beneficiaryName) return null; | ||
| const deposit = deposits.at(0); |
There was a problem hiding this comment.
🚩 DepositCard only renders the first deposit, other methods may be missed
In src/components/add-funds/Ramp.tsx:76,160, deposit is set to deposits.at(0) and only this first deposit is rendered in a DepositCard. When Bridge returns multiple deposit methods (e.g., ACH and WIRE for USD), only the first one gets a detail card. The header does show all deposit display names joined with "or" (line 131-133), so the user sees the method names, but can only view/copy details for the first one. This may be intentional as a simplified first implementation, but could confuse users who expect to see details for all listed methods.
Was this helpful? React with 👍 or 👎 to provide feedback.
| case "WIRE": | ||
| return [ | ||
| { label: "Beneficiary name", value: deposit.beneficiaryName }, | ||
| { label: "Beneficiary address", value: deposit.beneficiaryAddress }, | ||
| { label: "Account number", value: deposit.accountNumber }, | ||
| { label: "Routing number", value: deposit.routingNumber }, | ||
| { label: "Bank name", value: deposit.bankName }, |
There was a problem hiding this comment.
Bug: The client code accesses deposit.beneficiaryAddress for ACH/WIRE deposits, but the server API does not return this field, causing it to be undefined and hidden in the UI.
Severity: HIGH
Suggested Fix
Update the Zod schemas for ACH and WIRE deposits in server/api/ramp.ts to include the beneficiaryAddress field. Then, in server/utils/ramps/bridge.ts, map the virtualAccount.source_deposit_instructions.bank_beneficiary_address to the beneficiaryAddress field in the returned deposit objects.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/components/add-funds/Ramp.tsx#L362-L368
Potential issue: The server API for ACH and WIRE deposits does not include the
`beneficiaryAddress` field in its response. The server schema in `server/api/ramp.ts`
omits this field, and the `getDepositDetailsFromVirtualAccount` function in
`server/utils/ramps/bridge.ts` fails to map the `bank_beneficiary_address` from the
underlying Bridge API into the response object. Consequently, when the client component
`Ramp.tsx` attempts to display `deposit.beneficiaryAddress`, the value is `undefined`.
This results in the "Beneficiary address" row being empty for users, hiding important
information required for them to complete ACH or WIRE transfers.
| const pixKey = depositInfo?.network === "PIX" ? depositInfo.pixKey : undefined; | ||
| const pix = useMemo(() => { | ||
| if (!pixKey || !beneficiaryName) return null; | ||
| const deposit = deposits.at(0); |
There was a problem hiding this comment.
🔴 Only first deposit card rendered when multiple deposit methods exist (USD via bridge has ACH + WIRE)
In Ramp.tsx, deposit = deposits.at(0) extracts only the first element of the depositInfo array, and only one DepositCard is rendered at line 160. For USD via bridge, the server returns two deposit details — ACH and WIRE (server/utils/ramps/bridge.ts:895-918). The title bar at lines 130-134 correctly joins all display names (deposits.map((d) => d.displayName).join(...)) showing "USD via ACH or WIRE", but only the ACH deposit card is actually rendered. Users see a title implying both methods are available but can only view ACH account details. The WIRE deposit details (which have a different estimatedProcessingTime of 300 seconds vs "1 - 3 business days") are completely inaccessible.
Prompt for agents
In src/components/add-funds/Ramp.tsx, line 76 extracts only the first deposit with `const deposit = deposits.at(0)`, and line 160 renders a single DepositCard. For currencies with multiple deposit methods (USD via bridge returns both ACH and WIRE), all deposit cards should be rendered.
1. Remove the single `deposit` variable at line 76 (keep `deposits` which is already correct).
2. At line 160, replace the single DepositCard rendering with a map over all deposits:
Replace `{deposit && <DepositCard copyToClipboard={copyToClipboard} deposit={deposit} isLoading={isPending} />}` with `{deposits.map((d) => <DepositCard key={d.network} copyToClipboard={copyToClipboard} deposit={d} isLoading={isPending} />)}`
3. Update any other references to the single `deposit` variable (lines 249 for fee display) to iterate over all deposits or pick appropriately.
Was this helpful? React with 👍 or 👎 to provide feedback.
| case "WIRE": | ||
| return [ | ||
| { label: "Beneficiary name", value: deposit.beneficiaryName }, | ||
| { label: "Beneficiary address", value: deposit.beneficiaryAddress }, |
There was a problem hiding this comment.
🚩 depositRows for ACH/WIRE references beneficiaryAddress which may not exist in the server response
The depositRows function at src/components/add-funds/Ramp.tsx:365 includes { label: "Beneficiary address", value: deposit.beneficiaryAddress } for ACH and WIRE cases. However, the server's DepositDetails schema for ACH/WIRE (server/api/ramp.ts:271-291) does not include a beneficiaryAddress field — only beneficiaryName, routingNumber, accountNumber, bankName, and bankAddress. The bridge API response does have bank_beneficiary_address (server/utils/ramps/bridge.ts:797), but the server's getDepositDetailsFromVirtualAccount function (server/utils/ramps/bridge.ts:899) maps it to... nothing. TypeScript's discriminated union narrowing should catch this as a compile error (the narrowed type for ACH doesn't have beneficiaryAddress). If it somehow passes compilation, the row would show a perpetual loading Skeleton. Either the server should expose this field in the DepositDetails schema, or the client should remove this row.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Summary by CodeRabbit
New Features
Improvements