✨ server: add bridge fee window tracking#925
Conversation
🦋 Changeset detectedLatest commit: 4f99113 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:
WalkthroughIntroduces windowed sponsored-fee tracking for bridge flows: adds feeRule exports and enabling/disabling logic, surfaces Changes
Sequence DiagramsequenceDiagram
participant Webhook as Bridge Webhook
participant Hook as Webhook Handler
participant FeeRule as Fee Rule (feeRule)
participant Redis as Redis (storage)
participant API as Provider API / Bridge helpers
Webhook->>Hook: POST event (payment_processed / payment_submitted / drain)
Hook->>Hook: validate event_object.created_at (must parse to Date)
Hook->>Hook: parse numeric amount (receipt.final_amount / outgoing_amount)
alt amount is NaN
Hook->>Hook: captureException & return { code: "invalid amount" }
else amount valid
Hook->>FeeRule: report({ bridgeId, eventId, amount: Math.round(amount*100), timestamp })
FeeRule->>Redis: increment/record volume & count for window
FeeRule->>FeeRule: evaluate thresholds
alt Threshold exceeded
FeeRule->>API: enableFees(customerId) (PUT VAs & liquidation addresses)
FeeRule->>API: captureEvent("bridge fees enabled")
else Threshold not exceeded
FeeRule->>FeeRule: continue accumulating
end
Hook->>Hook: proceed with notifications/tracking after report
end
API->>FeeRule: evaluateSponsoredFees(customerId) when building deposit details
FeeRule->>Redis: read current window state
FeeRule->>API: return sponsoredFees (window, available, thresholds) or undefined on error
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 docstrings
🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
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}
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 @@
## window-rule #925 +/- ##
===============================================
+ Coverage 72.01% 72.09% +0.08%
===============================================
Files 229 229
Lines 8400 8436 +36
Branches 2695 2701 +6
===============================================
+ Hits 6049 6082 +33
- Misses 2114 2119 +5
+ Partials 237 235 -2
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:
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
mainqueg
force-pushed
the
window-rule
branch
3 times, most recently
from
54d0250 to
295d566
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
server/utils/ramps/bridge.ts (2)
422-429:⚠️ Potential issue | 🟠 MajorConsult the fee window before creating a new rail.
These branches create the instruction first and only then read
evaluateSponsoredFees(...). If the customer already exhausted sponsorship on another rail, the new account/address is still created fee-free, so the response can come back withfee: "0.0"whilesponsoredFees.availableis already0.Also applies to: 456-468
1148-1158:⚠️ Potential issue | 🔴 CriticalNormalize volume before comparing it to a USD threshold.
This rule stores only a raw
amountstring, but it is fed from native-currency Bridge events and later surfaced asUSD.Math.round(Number(event.amount)) * 100nalso drops cent precision. That means fee activation can move materially on non-USD or fractional transfers.Run this to confirm the rule has no currency context and currently rounds to whole dollars:
#!/bin/bash rg -n -C2 'schema: object\(|symbol: "USD"|Math\.round\(Number\(event\.amount\)\) \* 100n' server/utils/ramps/bridge.ts rg -n -C3 'feeRule\.report\(|initial_amount|outgoing_amount|currency:' server/hooks/bridge.ts python - <<'PY' import math for amount in ["0.49", "0.50", "99.999", "200.01"]: cents = math.floor(float(amount) + 0.5) * 100 print(f"{amount} -> current cents: {cents}") PYExpected: the grep output shows the window is labeled
USDwhile the stored event has onlyamount, and the Python output shows current whole-dollar rounding.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 578da5c6-fb6a-41e5-9e56-908634d46066
📒 Files selected for processing (8)
.changeset/swift-foxes-track.mdserver/api/ramp.tsserver/hooks/bridge.tsserver/index.tsserver/test/api/ramp.test.tsserver/test/hooks/bridge.test.tsserver/test/utils/bridge.test.tsserver/utils/ramps/bridge.ts
![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.
![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.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?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.
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.
| Promise.allSettled([feeRule.stop(), closeSentry(), closeSegment(), database.$client.end()]) | ||
| .then(async (results) => { | ||
| await closeRedis(); | ||
| if (error) reject(error); |
There was a problem hiding this comment.
Bug: The server shutdown sequence can hang indefinitely if a BullMQ job is stuck, preventing Redis connections from being closed.
Severity: HIGH
Suggested Fix
Wrap the feeRule.stop() call in a Promise.race with a timeout. Alternatively, use BullMQ's force-close options to ensure the worker is terminated after a reasonable period, allowing the shutdown sequence to proceed and close all connections.
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: server/index.ts#L326-L329
Potential issue: The `feeRule.stop()` method calls `worker.close()` without a timeout.
Since `worker.close()` waits indefinitely for active jobs to finish, a single hanging
job (e.g., waiting on an external API) will block the promise from resolving. This
prevents the `Promise.allSettled` in the main shutdown sequence from completing, meaning
`closeRedis()` is never called. This results in a resource leak and prevents a graceful
server shutdown.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
server/utils/ramps/bridge.ts (1)
422-429:⚠️ Potential issue | 🟠 MajorSeed newly created Bridge rails from the current fee-window state.
Both provisioning paths still create new rails with zero fees. If the customer already crossed the sponsored-fee threshold before their first ACH/WIRE/SEPA/SPEI/PIX-BR or crypto rail is created,
/quotecan return exhaustedsponsoredFeeswhile the freshly created rail still hasfee: "0.0", and that rail stays free until a later trigger/expiry cycle updates it.🛠️ fix sketch
+ const fee = await feeRule + .read(customer.id) + .then(({ result }) => (result.trigger ? fees[CurrencyToBridge[currency]] : "0.0")) + .catch(() => "0.0"); virtualAccount ??= await createVirtualAccount(customer.id, { source: { currency: CurrencyToBridge[currency] }, - developer_fee_percent: "0.0", + developer_fee_percent: fee, destination: { currency: "usdc", payment_rail: supportedChainId, address: account }, });Apply the same
feederivation tocustom_developer_fee_percentwhen creating liquidation addresses, or haveevaluateSponsoredFees()return the trigger state so both branches can reuse a single read.Also applies to: 456-468
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 52045bdb-0a91-4aa5-9c4b-4050b157b5fa
📒 Files selected for processing (8)
.changeset/swift-foxes-track.mdserver/api/ramp.tsserver/hooks/bridge.tsserver/index.tsserver/test/api/ramp.test.tsserver/test/hooks/bridge.test.tsserver/test/utils/bridge.test.tsserver/utils/ramps/bridge.ts
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Tests