Conversation
🦋 Changeset detectedLatest commit: 03427ee 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)
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 |
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.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## i18n #885 +/- ##
==========================================
- Coverage 71.41% 70.80% -0.62%
==========================================
Files 227 227
Lines 8397 8230 -167
Branches 2707 2639 -68
==========================================
- Hits 5997 5827 -170
+ Misses 2177 2173 -4
- Partials 223 230 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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
src/components/add-funds/Onboard.tsx
Outdated
| 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
src/components/add-funds/Onboard.tsx
Outdated
| const isCrypto = !!network; | ||
| const isBridge = provider === "bridge"; | ||
|
|
||
| const [acknowledged, setAcknowledged] = useState(true); |
There was a problem hiding this comment.
📝 Info: T&C checkbox starts pre-checked in the Fees component
At src/components/add-funds/Fees.tsx:40, acknowledged is initialized to true, meaning the terms checkbox starts checked and the user can immediately proceed. This matches the old behavior in the deleted Onboard.tsx which also initialized acknowledged to true. While this is intentional for UX continuity, it's worth noting this is a pre-acceptance pattern for terms and conditions, which may have legal implications depending on jurisdiction.
Was this helpful? React with 👍 or 👎 to provide feedback.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
src/components/add-funds/Onboard.tsx
Outdated
| 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.
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.
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.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| 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.
cruzdanilo
force-pushed
the
i18n
branch
3 times, most recently
from
3bacc54 to
9ece904
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| export const bridgeMethods: Partial<Record<Currency, string>> = { | ||
| USD: "ACH or WIRE", | ||
| EUR: "SEPA", | ||
| GBP: "FPS", | ||
| MXN: "SPEI", | ||
| BRL: "PIX", | ||
| }; |
There was a problem hiding this comment.
🚩 BRL/PIX fees missing from bridge fee table
In src/utils/currencies.ts:17, bridgeMethods maps BRL to "PIX". However, rampFees.bridge at src/utils/currencies.ts:34-63 has no PIX key (only ACH, WIRE, SEPA, SPEI, FPS). This means bridgeMethod("BRL") in src/components/add-funds/Fees.tsx:308-313 returns undefined, and the rows() function returns an empty array for BRL/bridge users. The fees screen would display no fee information for this currency. This may be intentional if PIX fees are handled differently, but if BRL is expected to show fee rows like other bridge currencies, a PIX entry needs to be added to rampFees.bridge.
Was this helpful? React with 👍 or 👎 to provide feedback.
| function bridgeMethod(currency: Currency) { | ||
| if (currency === "USD") return "ACH" as const; | ||
| const method = bridgeMethods[currency]; | ||
| if (!method || !(method in rampFees.bridge)) return; | ||
| return method as keyof typeof rampFees.bridge; |
There was a problem hiding this comment.
Bug: The rampFees.bridge object is missing an entry for PIX, causing the fees page to show no fee information for BRL Bridge deposits.
Severity: MEDIUM
Suggested Fix
Add the missing PIX fee definition to the rampFees.bridge object in currencies.ts. This will ensure that the fee information is correctly looked up and displayed for BRL Bridge deposits.
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/Fees.tsx#L315-L319
Potential issue: When a user selects the Bridge provider with BRL currency, the
`bridgeMethod` function in `Fees.tsx` correctly identifies the payment method as
`"PIX"`. However, the `rampFees.bridge` object in `currencies.ts` is missing a `PIX`
key. This causes the lookup to fail and the `bridgeMethod` function to return
`undefined`. As a result, the fees page is rendered without any fee information for this
supported payment flow, making the UI appear broken to the user.
| export const fees = { | ||
| manteca: { | ||
| transfer: { | ||
| fee: "0%-2%", | ||
| }, | ||
| }, | ||
| bridge: { | ||
| ACH: { | ||
| fee: "$0.5 + 0.2%", | ||
| creation: "$1", | ||
| maintenance: "$2", | ||
| }, | ||
| WIRE: { | ||
| fee: "$10", | ||
| creation: "$1", | ||
| maintenance: "$2", | ||
| }, | ||
| SEPA: { | ||
| // cspell:ignore sepa | ||
| fee: "0.35% + mid-market rate", | ||
| creation: "$2", | ||
| maintenance: "$2", | ||
| }, | ||
|
|
||
| SPEI: { | ||
| // cspell:ignore spei | ||
| fee: "0.5% + mid-market rate", | ||
| creation: "~$1.5 (30 MXN)", | ||
| maintenance: "~$1.5 (30 MXN)", | ||
| }, | ||
| FPS: { | ||
| fee: "0.5% + mid-market rate", | ||
| creation: "~$2 (1.5 GBP)", | ||
| maintenance: "~$2 (1.5 GBP)", | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🔴 Missing PIX entry in rampFees.bridge causes empty fee rows for BRL bridge users
bridgeMethods["BRL"] is "PIX" (src/utils/currencies.ts:17), but rampFees.bridge only contains keys ACH, WIRE, SEPA, SPEI, FPS — there is no PIX entry. In the bridgeMethod function at src/components/add-funds/Fees.tsx:315-320, when currency === "BRL", the lookup bridgeMethods["BRL"] returns "PIX", and then !("PIX" in rampFees.bridge) is true, so the function returns undefined. This causes rows() at line 301 to return an empty array, resulting in the Fees screen showing zero fee rows for BRL bridge users.
Prompt for agents
Add a PIX entry to the rampFees.bridge object in src/utils/currencies.ts (around line 57-62), with the same structure as other entries (fee, creation, maintenance). For example:
PIX: {
fee: "<appropriate fee>",
creation: "<appropriate creation fee>",
maintenance: "<appropriate maintenance fee>",
},
The actual fee values need to come from the Bridge provider's fee schedule for PIX transfers.
Was this helpful? React with 👍 or 👎 to provide feedback.
| {typedProvider === "bridge" && ( | ||
| <XStack gap="$s3" alignItems="center"> | ||
| <View flexShrink={0} alignItems="center"> | ||
| <Banknote size={16} color="$uiNeutralPrimary" /> | ||
| </View> | ||
| <Text secondary caption2 color="$uiNeutralPlaceholder" flex={1}> | ||
| {t( | ||
| "We cover incoming transfers to your Bridge accounts in Exa App up to $3,000 or 60 transactions per month. Fees apply after that. {{method}}: {{fee}}.", | ||
| { fee: bridgeFee, method: deposit?.displayName }, | ||
| )} | ||
| </Text> | ||
| </XStack> |
There was a problem hiding this comment.
🟡 Bridge fee info text renders with empty method and fee during loading
In Ramp.tsx lines 220-231, the bridge fee info section is rendered whenever typedProvider === "bridge", regardless of whether deposit data has loaded. When the quote query is still pending (isPending is true), deposit is undefined (from deposits.at(0) on an empty array at line 76), and consequently both bridgeFee and deposit?.displayName are undefined. The i18n interpolation renders these as empty strings, producing broken text like: "...Fees apply after that. : ." with a dangling colon and period. This section should be conditionally hidden while loading or when deposit is undefined.
| {typedProvider === "bridge" && ( | |
| <XStack gap="$s3" alignItems="center"> | |
| <View flexShrink={0} alignItems="center"> | |
| <Banknote size={16} color="$uiNeutralPrimary" /> | |
| </View> | |
| <Text secondary caption2 color="$uiNeutralPlaceholder" flex={1}> | |
| {t( | |
| "We cover incoming transfers to your Bridge accounts in Exa App up to $3,000 or 60 transactions per month. Fees apply after that. {{method}}: {{fee}}.", | |
| { fee: bridgeFee, method: deposit?.displayName }, | |
| )} | |
| </Text> | |
| </XStack> | |
| {typedProvider === "bridge" && deposit && ( | |
| <XStack gap="$s3" alignItems="center"> | |
| <View flexShrink={0} alignItems="center"> | |
| <Banknote size={16} color="$uiNeutralPrimary" /> | |
| </View> | |
| <Text secondary caption2 color="$uiNeutralPlaceholder" flex={1}> | |
| {t( | |
| "We cover incoming transfers to your Bridge accounts in Exa App up to $3,000 or 60 transactions per month. Fees apply after that. {{method}}: {{fee}}.", | |
| { fee: bridgeFee, method: deposit?.displayName }, | |
| )} | |
| </Text> | |
| </XStack> | |
| )} |
Was this helpful? React with 👍 or 👎 to provide feedback.
| </Text> | ||
| )} | ||
| </YStack> | ||
| {deposit && <DepositCard copyToClipboard={copyToClipboard} deposit={deposit} isLoading={isPending} />} |
There was a problem hiding this comment.
🚩 Only the first deposit card is rendered despite header referencing all deposit methods
In Ramp.tsx:76, only the first deposit is selected via deposits.at(0), and only one DepositCard is rendered at line 164. However, the header title at lines 134-138 joins all deposit display names (e.g., "USD via ACH or WIRE"). For bridge providers that may return multiple deposit methods (like USD with both ACH and WIRE), the user sees a title referencing both methods but only sees account details for one. The old manteca-only code also only rendered one deposit, but bridge may have a different API contract with multiple deposit infos per currency.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const isBridge = provider === "bridge" && !!currency && !!network; | ||
|
|
||
| const { data } = useQuery({ | ||
| queryKey: ["ramp", "quote", "bridge", currency, network], |
There was a problem hiding this comment.
🚩 Query key inconsistency between AddCrypto and Ramp for bridge quotes
In AddCrypto.tsx:43, the query key for bridge quotes is ["ramp", "quote", "bridge", currency, network], using the literal string "bridge" as the provider. In Ramp.tsx:54, the key is ["ramp", "quote", typedProvider, typedCurrency] where typedProvider is the validated provider value. These two components use different query key structures for potentially the same data, which means they won't share cache entries. This is likely intentional since AddCrypto includes network in the key (crypto deposits) while Ramp doesn't (fiat deposits), but worth confirming there's no cache staleness issue if both screens could display the same provider+currency combo.
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03427ee36a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const bridgeFee = | ||
| typedProvider === "bridge" && deposit && deposit.displayName in rampFees.bridge | ||
| ? rampFees.bridge[deposit.displayName as keyof typeof rampFees.bridge].fee |
There was a problem hiding this comment.
The bridge fee banner derives bridgeFee from deposit.displayName, but several supported Bridge deposits use display names that are not keys in rampFees.bridge (for example, Faster Payments and PIX BR from server/utils/ramps/bridge.ts). In those cases this expression resolves to undefined, so the user-facing fee message renders without a valid fee value for GBP/BRL flows. Use a normalized key (e.g., based on deposit.network) instead of displayName for fee lookup.
Useful? React with 👍 / 👎.
| const method = bridgeMethods[currency]; | ||
| if (!method || !(method in rampFees.bridge)) return; | ||
| return method as keyof typeof rampFees.bridge; |
There was a problem hiding this comment.
bridgeMethod() rejects methods that are not present in rampFees.bridge. With the new config, bridgeMethods.BRL is "PIX", but there is no corresponding PIX entry in rampFees.bridge, so BRL returns undefined here and rows() returns an empty list. That leaves the fees screen without fee rows for BRL onboarding even though BRL is advertised as supported.
Useful? React with 👍 / 👎.
2 participants
Summary by CodeRabbit
New Features
Improvements