Conversation
🦋 Changeset detectedLatest commit: ba90b46 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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:
<review_stack_artifact> </review_stack_artifact> 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 188fbc9c-c51a-4982-af69-203c14ad6608
📒 Files selected for processing (10)
.changeset/swift-bridges-cross.mdserver/api/ramp.tsserver/hooks/bridge.tsserver/i18n/es.jsonserver/i18n/pt.jsonserver/test/api/ramp.test.tsserver/test/hooks/bridge.test.tsserver/test/utils/bridge.test.tsserver/utils/ramps/bridge.tsserver/utils/segment.ts
![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 bridge offramp functionality, allowing users to withdraw funds to external bank accounts. Key changes include new API endpoints for managing external accounts (POST, GET, PATCH, DELETE /external-account), support for offramp directions in the ramp API, and handling of Bridge webhook events for transfer status updates. The update also includes push notifications for withdrawal progress and Segment tracking for offramp events. Feedback focuses on relaxing validation constraints for street addresses and bank account numbers to accommodate shorter valid inputs, as well as refactoring repeated authentication and customer retrieval logic into a shared middleware.
There was a problem hiding this comment.
Actionable comments posted: 9
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 86f72493-a0b9-4f8a-89db-f41aa19e65b9
📒 Files selected for processing (10)
.changeset/swift-bridges-cross.mdserver/api/ramp.tsserver/hooks/bridge.tsserver/i18n/es.jsonserver/i18n/pt.jsonserver/test/api/ramp.test.tsserver/test/hooks/bridge.test.tsserver/test/utils/bridge.test.tsserver/utils/ramps/bridge.tsserver/utils/segment.ts
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1024 +/- ##
==========================================
+ Coverage 73.87% 74.97% +1.10%
==========================================
Files 243 243
Lines 10852 11542 +690
Branches 3678 3972 +294
==========================================
+ Hits 8017 8654 +637
- Misses 2528 2564 +36
- Partials 307 324 +17
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:
|
mainqueg
force-pushed
the
offramp
branch
3 times, most recently
from
846895d to
f4612a6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/hooks/bridge.ts (1)
132-193:⚠️ Potential issue | 🟠 Major | ⚡ Quick winShort-circuit ignored transfer events before credential lookup.
transfer.updated.status_transitionedevents that you later ignore (external_account_id == nullor any state other thanfunds_received/payment_processed) still resolvebridgeIdand can return{ code: "credential not found" }first. That turns an intended no-op into Sentry noise wheneveron_behalf_ofis unpaired.🛠️ Suggested guard placement
+ if ( + payload.event_type === "transfer.updated.status_transitioned" && + (!payload.event_object.destination.external_account_id || + (payload.event_object.state !== "funds_received" && + payload.event_object.state !== "payment_processed")) + ) { + return c.json({ code: "ok" }, 200); + } + const bridgeId = payload.event_type === "customer.updated.status_transitioned" ? payload.event_object.id : payload.event_type === "transfer.updated.status_transitioned" ? payload.event_object.on_behalf_ofAlso applies to: 259-293
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dd0dcddd-cfdb-437b-8f7c-5d34cf75216d
📒 Files selected for processing (10)
.changeset/swift-bridges-cross.mdserver/api/ramp.tsserver/hooks/bridge.tsserver/i18n/es.jsonserver/i18n/pt.jsonserver/test/api/ramp.test.tsserver/test/hooks/bridge.test.tsserver/test/utils/bridge.test.tsserver/utils/ramps/bridge.tsserver/utils/segment.ts
![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: f4612a6090
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e083f4f853
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
server/utils/ramps/bridge.ts (2)
780-798:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winPass a stable idempotency key when creating the static template.
This read-then-create branch is still race-prone. Two concurrent requests can both miss
templates.find(...)and create separateawaiting_fundstransfers becausecreateTransfer()is called without a deterministic key.removeExternalAccount()only deletes one of them.Proposed fix
const templates = await getStaticTemplates(customer.id); + const idempotencyKey = crypto + .createHash("sha256") + .update(["offramp", customer.id, externalAccountId, supportedChain, account.toLowerCase()].join(":")) + .digest("hex"); let transfer = templates.find( ({ destination, source }) => destination.external_account_id === externalAccountId && source.payment_rail === supportedChain && source.currency === "usdc", ); transfer ??= await createTransfer({ on_behalf_of: customer.id, client_reference_id: account, source: { currency: "usdc", payment_rail: supportedChain }, destination: { currency: externalAccount.currency, payment_rail: paymentRail, external_account_id: externalAccountId, }, features: { flexible_amount: true, static_template: true, allow_any_from_address: true }, - }); + }, idempotencyKey);Does Bridge require an `Idempotency-Key` header for POST transfer creation, and for how long does it deduplicate repeated requests?
1667-1670:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSanitize the validation context before sending it to Sentry.
Spreading
resultintosetContext()can send raw Bridge payloads on schema failures. In this module that includes bank details, addresses, and transfer metadata, so this is a privacy/compliance leak. Record only sanitized issues plus request metadata.Proposed fix
const result = safeParse(schema, JSON.parse(new TextDecoder().decode(rawBody))); if (!result.success) { - setContext("validation", { ...result, flatten: flatten(result.issues) }); + setContext("validation", { url, issues: flatten(result.issues) }); throw new ValiError(result.issues); }Does valibot `safeParse` include the original `input` in the failure result object it returns?
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 75cb7714-1d84-4f55-a9f8-e920d6faf07a
📒 Files selected for processing (10)
.changeset/swift-bridges-cross.mdserver/api/ramp.tsserver/hooks/bridge.tsserver/i18n/es.jsonserver/i18n/pt.jsonserver/test/api/ramp.test.tsserver/test/hooks/bridge.test.tsserver/test/utils/bridge.test.tsserver/utils/ramps/bridge.tsserver/utils/segment.ts
mainqueg
force-pushed
the
offramp
branch
2 times, most recently
from
45c2696 to
db233d9
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/utils/ramps/bridge.ts (1)
1643-1675: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueRemove commented-out debug code.
Line 1673 contains commented-out debug code that should be removed per coding guidelines: "do not use comments. the only exceptions are static analysis annotations...and TODO/HACK/FIXME markers."
Suggested fix
if (!result.success) { setContext("validation", { ...result, flatten: flatten(result.issues) }); throw new ValiError(result.issues); } - // console.dir(result.output, { depth: null }); return result.output;
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e74768f0-928f-4a94-977f-ba0cf6902b21
📒 Files selected for processing (10)
.changeset/swift-bridges-cross.mdserver/api/ramp.tsserver/hooks/bridge.tsserver/i18n/es.jsonserver/i18n/pt.jsonserver/test/api/ramp.test.tsserver/test/hooks/bridge.test.tsserver/test/utils/bridge.test.tsserver/utils/ramps/bridge.tsserver/utils/segment.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db233d97c7
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f405766bb
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46470e2b12
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2cd8eeef86
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2980c839ae
ℹ️ 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".
| bankName: externalAccount.bankName, | ||
| currency: externalAccount.currency, | ||
| id, | ||
| ownerName: externalAccount.accountOwnerName, |
There was a problem hiding this comment.
Map bankName and ownerName from the BridgeExternalAccount response instead of echoing request input. The current code returns externalAccount.bankName / externalAccount.accountOwnerName, so if the client omits bankName or Bridge normalizes/corrects values, the API responds with stale or missing data even though Bridge returned the authoritative fields. This can immediately desync UI state after account creation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 617580bc0d
ℹ️ 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".
| ], | ||
| }, | ||
| onramp: { currencies: [...currencies, ...approvedCurrencies] }, | ||
| offramp: { currencies: [{ currency: "USDT" as const, network: "TRON" as const }, ...approvedCurrencies] }, |
There was a problem hiding this comment.
getCryptoOfframpDepositDetails and the /quote validator support USDC offramps on BASE, SOLANA, and STELLAR, but this provider payload only exposes { currency: "USDT", network: "TRON" } plus fiat currencies. Clients that rely on /ramp capabilities to render available flows will not surface the supported USDC crypto offramp routes, making part of the new offramp functionality effectively unreachable.
Useful? React with 👍 / 👎.
| let transfer = templates.find( | ||
| ({ destination, source, state }) => | ||
| destination.external_account_id === externalAccountId && | ||
| source.payment_rail === supportedChain && | ||
| source.currency === "usdc" && | ||
| state !== "canceled", | ||
| ); | ||
| if (transfer && transfer.state !== "awaiting_funds") throw new Error(ErrorCodes.TRANSFER_IN_USE); |
There was a problem hiding this comment.
getOfframpDepositDetails inspects only the first non-canceled matching template, then decides whether to throw TRANSFER_IN_USE. If multiple matching templates exist and the first is awaiting_funds while another is already in-flight (for example funds_received), this path will incorrectly allow reuse and return deposit instructions instead of blocking the new withdrawal attempt.
Useful? React with 👍 / 👎.
1 participant
Summary by CodeRabbit
New Features
Analytics & Notifications
Localization
Tests