Revert "Revert "backend/feat: Stripe payout dynamic transfer top-up f…#1326
Revert "Revert "backend/feat: Stripe payout dynamic transfer top-up f…#1326karan2cmd wants to merge 1 commit into
Conversation
WalkthroughThis PR adds Stripe balance querying and merchant top-up logic to the payout flow. It introduces new Stripe balance types and API support, exports balance query helpers, extends the payout response to track top-up amounts, implements adjusted transfer computation when fleet balance is insufficient, and updates Juspay to populate the new field. ChangesStripe Merchant Top-Up Payout Flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/mobility-core/src/Kernel/External/Payout/Interface.hs (1)
47-74: 💤 Low valueExtra synchronous Stripe balance calls on every payout.
computeAdjustedTransferalways callsStripe.getBalancefor the fleet account (plus a second call for the platform account on shortfall) on the payout request path. This adds external-call latency and rate-limit pressure to every payout. Consider short-TTL caching of balances or gating the top-up logic behind a config flag if it isn't needed for all flows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/External/Payout/Interface.hs` around lines 47 - 74, The payout path currently calls Stripe.getBalance inside computeAdjustedTransfer for every request (fleetBalance and potentially merchantBalance), causing extra latency and rate-limit pressure; update computeAdjustedTransfer to first consult a short-TTL in-memory cache (keyed by connectedAccountId and currency) for fleetBalance and merchantBalance and only call Stripe.getBalance when the cache is missing/stale, and also gate the top-up check behind a config flag (eg. enableTransferTopUp) so when disabled computeAdjustedTransfer returns (req.transferAmount, Nothing) without any Stripe calls; reference computeAdjustedTransfer, Stripe.getBalance, fleetBalance/fleetAvail, merchantBalance/merchantAvail and merchantTopUpAmount when making these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/mobility-core/src/Kernel/External/Payout/Interface.hs`:
- Around line 58-64: Replace the client-facing error that leaks merchantAvail
and adjustedAmt by logging the exact balances internally and throwing a generic
InvalidRequest message; specifically, in the branch that checks "when
(merchantAvail < adjustedAmt)" record merchantAvail and adjustedAmt to your
internal logger at an appropriate level (e.g., error/debug) and then call
"throwError $ InvalidRequest" with a non-sensitive message like "Merchant
platform account has insufficient balance" (omit numeric details).
In `@lib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hs`:
- Around line 152-155: getAvailableForCurrency is converting Stripe minor-unit
amounts with the hardcoded centsToUsd (/100), which breaks currencies like INR
that have different decimal accuracy; replace the hardcoded conversion by using
the currency's accuracy (e.g. getAccuracy or existing currency-decimal helper)
to compute scale = 10 ^ accuracy and convert each f.amount from minor units to
HighPrecMoney by dividing by that scale (instead of using centsToUsd), keeping
the currency filter on f.currency; update any helper conversions
(usdToCents/eurToCents) or replace them with a generic minorUnit <-> money
converter so fleetAvail and the transferAmount comparison use the correct scale.
---
Nitpick comments:
In `@lib/mobility-core/src/Kernel/External/Payout/Interface.hs`:
- Around line 47-74: The payout path currently calls Stripe.getBalance inside
computeAdjustedTransfer for every request (fleetBalance and potentially
merchantBalance), causing extra latency and rate-limit pressure; update
computeAdjustedTransfer to first consult a short-TTL in-memory cache (keyed by
connectedAccountId and currency) for fleetBalance and merchantBalance and only
call Stripe.getBalance when the cache is missing/stale, and also gate the top-up
check behind a config flag (eg. enableTransferTopUp) so when disabled
computeAdjustedTransfer returns (req.transferAmount, Nothing) without any Stripe
calls; reference computeAdjustedTransfer, Stripe.getBalance,
fleetBalance/fleetAvail, merchantBalance/merchantAvail and merchantTopUpAmount
when making these changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2662eb71-b341-4695-83d8-22d4b60fa664
📒 Files selected for processing (5)
lib/mobility-core/src/Kernel/External/Payout/Interface.hslib/mobility-core/src/Kernel/External/Payout/Interface/Juspay.hslib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hslib/mobility-core/src/Kernel/External/Payout/Interface/Types.hslib/mobility-core/src/Kernel/External/Payout/Stripe/Flow.hs
| when (merchantAvail < adjustedAmt) $ | ||
| throwError $ | ||
| InvalidRequest $ | ||
| "Merchant platform account has insufficient balance. Available: " | ||
| <> show merchantAvail | ||
| <> ", Required: " | ||
| <> show adjustedAmt |
There was a problem hiding this comment.
Don't leak the merchant platform balance in a client-facing error.
InvalidRequest is surfaced to the API caller. Embedding the platform/merchant account's available balance (and exact required amount) in the message discloses internal financial state to whoever triggers the payout. Log the details internally and return a generic message.
🔒 Proposed fix
- when (merchantAvail < adjustedAmt) $
- throwError $
- InvalidRequest $
- "Merchant platform account has insufficient balance. Available: "
- <> show merchantAvail
- <> ", Required: "
- <> show adjustedAmt
+ when (merchantAvail < adjustedAmt) $ do
+ logError $
+ "Merchant platform account has insufficient balance. Available: "
+ <> show merchantAvail
+ <> ", Required: "
+ <> show adjustedAmt
+ throwError $
+ InvalidRequest "Insufficient merchant platform account balance to complete payout."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/mobility-core/src/Kernel/External/Payout/Interface.hs` around lines 58 -
64, Replace the client-facing error that leaks merchantAvail and adjustedAmt by
logging the exact balances internally and throwing a generic InvalidRequest
message; specifically, in the branch that checks "when (merchantAvail <
adjustedAmt)" record merchantAvail and adjustedAmt to your internal logger at an
appropriate level (e.g., error/debug) and then call "throwError $
InvalidRequest" with a non-sensitive message like "Merchant platform account has
insufficient balance" (omit numeric details).
| getAvailableForCurrency :: Currency -> Stripe.BalanceResp -> HighPrecMoney | ||
| getAvailableForCurrency currency Stripe.BalanceResp {available} = | ||
| let currencyText = T.toLower $ show currency | ||
| in sum [centsToUsd f.amount | f <- available, f.currency == currencyText] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the cents/major-unit converters to confirm scale is currency-agnostic.
ast-grep --pattern 'centsToUsd $_ = $$$'
ast-grep --pattern 'eurToCents $_ = $$$'
ast-grep --pattern 'usdToCents $_ = $$$'
rg -nP --type=haskell -C2 '\b(centsToUsd|eurToCents|usdToCents)\b'Repository: nammayatri/shared-kernel
Length of output: 12663
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the Interface.hs referenced by the review comment
fd -e hs -a Interface.hs lib/mobility-core/src | head -n 50
# Search for the specific comparison expression
rg -n --type=haskell "fleetAvail\s*\+\s*req\.transferAmount\s*>?=\s*req\.amount" lib/mobility-core/src || true
# If not found, search for fleetAvail usage near transferAmount/amount
rg -n --type=haskell "fleetAvail" lib/mobility-core/src | head -n 50Repository: nammayatri/shared-kernel
Length of output: 2691
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="lib/mobility-core/src/Kernel/External/Payout/Interface.hs"
# Show the relevant block around the comparison
nl -ba "$FILE" | sed -n '1,140p'
# Find the request record type that contains transferAmount/amount
rg -n --type=haskell "transferAmount" lib/mobility-core/src/Kernel/External/Payout/Interface.hs
rg -n --type=haskell "data .*Transfer.*|data .*Payout.*|record.*Transfer|req\.transferAmount" lib/mobility-core/src/Kernel/External/Payout/Interface.hs
# Broader search for where req.amount / transferAmount are built for payout
rg -n --type=haskell "transferAmount\s*=\s*" lib/mobility-core/src/Kernel/External/Payout lib/mobility-core/src/Kernel/External -S
rg -n --type=haskell "req\.amount" lib/mobility-core/src/Kernel/External/Payout -SRepository: nammayatri/shared-kernel
Length of output: 111
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="lib/mobility-core/src/Kernel/External/Payout/Interface.hs"
# Line-numbered view without nl
cat -n "$FILE" | sed -n '1,120p'
# Find request/type definitions and uses
rg -n --type=haskell "transferAmount" "$FILE"
rg -n --type=haskell "req\.amount|req\.transferAmount|amount\s*=|transferAmount\s*=" lib/mobility-core/src/Kernel/External/Payout -S
rg -n --type=haskell "data .*Payout|data .*Transfer|Create.*Payout|Transfer.*Req|Payout.*Req|record .*Payout" lib/mobility-core/src/Kernel/External/Payout -SRepository: nammayatri/shared-kernel
Length of output: 25372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect payout request types
rg -n --type=haskell "data CreatePayoutOrderReq" lib/mobility-core/src/Kernel/External/Payout/Interface/Types.hs
cat -n lib/mobility-core/src/Kernel/External/Payout/Interface/Types.hs | sed -n '1,220p'
# Inspect Stripe payout conversion of req.amount/transferAmount to Stripe minor units
cat -n lib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hs | sed -n '1,220p'
# Find Currency definition and any decimal/minor-unit scaling logic
rg -n --type=haskell "data Currency|newtype Currency|type Currency" lib/mobility-core/src/Kernel | head -n 50
rg -n --type=haskell "Currency" lib/mobility-core/src/Kernel | rg -n "decim|minor|zero|scale|fraction" | head -n 50 || true
rg -n --type=haskell "zero[-_ ]?decimal|minor unit|decimal" lib/mobility-core/src/Kernel | head -n 50 || trueRepository: nammayatri/shared-kernel
Length of output: 19012
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --type=haskell "HighPrecMoney" lib/mobility-core/src/Kernel/Types/Price.hs lib/mobility-core/src/Kernel | head -n 50
rg -n --type=haskell "newtype HighPrecMoney|type HighPrecMoney" lib/mobility-core/src/Kernel | head -n 50
rg -n --type=haskell "HighPrecMoney.*cents|cents.*HighPrecMoney|minor.*HighPrecMoney" lib/mobility-core/src/Kernel | head -n 50Repository: nammayatri/shared-kernel
Length of output: 6407
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="lib/mobility-core/src/Kernel/Types/Price.hs"
# show around HighPrecMoney definition
sed -n '1,140p' "$FILE" | cat -n | sed -n '40,110p'
# show rest where cents/minor may be defined
sed -n '140,260p' "$FILE" | cat -n | head -n 160Repository: nammayatri/shared-kernel
Length of output: 8353
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --type=haskell "getAvailableForCurrency|centsToUsd|usdToCents|eurToCents" lib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hs lib/mobility-core/src/Kernel/External/Payout/Interface.hs lib/mobility-core/src/Kernel/External/Payment/Interface/Stripe.hs
rg -n --type=haskell "getAccuracy\s+INR|data Currency\s*=\s*INR\s*\|" lib/mobility-core/src/Kernel/Types/Price.hsRepository: nammayatri/shared-kernel
Length of output: 4628
Fix currency minor-unit conversion in getAvailableForCurrency (INR is broken by hardcoded /100).
getAvailableForCurrency filters by f.currency == currencyText, but still converts every matching Stripe amount with centsToUsd:
getAvailableForCurrency :: Currency -> Stripe.BalanceResp -> HighPrecMoney
getAvailableForCurrency currency Stripe.BalanceResp {available} =
let currencyText = T.toLower $ show currency
in sum [centsToUsd f.amount | f <- available, f.currency == currencyText]centsToUsd is hardcoded to / 100, and both usdToCents/eurToCents are hardcoded to * 100—so for Currency = INR (0-decimal in getAccuracy INR = 0), fleetAvail (and the top-up logic guarded by fleetAvail + req.transferAmount >= req.amount in Kernel/External/Payout/Interface.hs) will use the wrong scale.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/mobility-core/src/Kernel/External/Payout/Interface/Stripe.hs` around
lines 152 - 155, getAvailableForCurrency is converting Stripe minor-unit amounts
with the hardcoded centsToUsd (/100), which breaks currencies like INR that have
different decimal accuracy; replace the hardcoded conversion by using the
currency's accuracy (e.g. getAccuracy or existing currency-decimal helper) to
compute scale = 10 ^ accuracy and convert each f.amount from minor units to
HighPrecMoney by dividing by that scale (instead of using centsToUsd), keeping
the currency filter on f.currency; update any helper conversions
(usdToCents/eurToCents) or replace them with a generic minorUnit <-> money
converter so fleetAvail and the transferAmount comparison use the correct scale.
…or Fleet VA insufficient balance (#1320)""
This reverts commit ce882b3.
Type of Change
Description
Additional Changes
Motivation and Context
How did you test it?
Checklist
./dev/format-all-files.shSummary by CodeRabbit