Wire consumer fiat accounts, balances, transfers and unified funding flows - #98
Gift-Stack wants to merge 26 commits into
Conversation
| const payout = | ||
| BigInt(order.plan.recipientMinor) + | ||
| BigInt(order.plan.payoutFeeMinor); | ||
| if (input.action === 'fail') { |
There was a problem hiding this comment.
[Altitude, medium (simulation-only)] The unified simulation is a second money engine that reimplements the production funding state machine, and its advance('fail') branch releases a reservation from any non-terminal state (including sending), whereas the production reducer (funding-state.ts:313-322) forbids releasing after payout_started without reconciled payout_not_debited evidence. As the reference/demo model it teaches a failure semantics the real engine correctly refuses, and the two will drift. Drive the simulator through the real reducer plus SimulatorFiatProvider rather than a parallel implementation.
There was a problem hiding this comment.
Partially addressed in 894cf79: the consumer-facing unified simulator screen has been removed (the legacy route redirects to observed balances), and fail is now rejected once an order reaches sending, preserving the reservation for reconciliation. The backend simulator still remains a separate dev-only engine rather than being fully driven through the production reducer, so I’m leaving this architectural thread unresolved rather than claiming the consolidation is complete.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 3cf92d74e9
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@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: 177cb68c80
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await apiClient.quoteNairaTransfer( | ||
| account, | ||
| minor!, | ||
| requestKey(`quote:${account}:${minor}`) |
There was a problem hiding this comment.
When a preview expires, requesting the same recipient and amount reuses this permanently cached idempotency key. The backend therefore replays the original expired quote before creating a new one, so the confirmation button remains disabled and the user cannot obtain a fresh preview without changing an input or remounting the screen. Generate a new quote key when retrying an expired preview while retaining the existing key only for an in-flight request retry.
Useful? React with 👍 / 👎.
| routes.data?.routes.filter( | ||
| (r) => r.direction === direction && r.environment !== "simulation" | ||
| ) ?? []; |
There was a problem hiding this comment.
Filtering out every simulation route makes the only executable implementation in this slice unreachable from the mobile flow. With FIAT_ENABLED_PROVIDERS=simulator, the API returns valid receive/send routes but this screen reports that no options are available; when Fonbnk is also enabled, users can only preview its quote-only routes because those advertise orderAvailable: false. Allow simulator routes in development (and likewise retain their history) so the configured durable simulation flow can actually be exercised.
Useful? React with 👍 / 👎.
| } catch { | ||
| // No resubmission, guessed settlement, refund or balance credit after uncertain execution. | ||
| record = { | ||
| ...record, | ||
| status: 'needs_attention', | ||
| updatedAt: new Date().toISOString(), |
There was a problem hiding this comment.
When Paga conclusively rejects a transfer—for example with a non-retryable 4xx response or a nonzero provider response code—PagaProvider throws PagaError('REJECTED'), but this catch maps it to needs_attention just like a timeout. That state is included in the earlier pending-transfer check, so a definitive non-debit permanently blocks every subsequent outgoing transfer for the account despite requiring no settlement reconciliation. Preserve needs_attention for UNKNOWN_OUTCOME/ambiguous responses, but move confirmed rejections to a terminal non-debited state that releases the outgoing-send block.
Useful? React with 👍 / 👎.
| providerReference: outcome.providerReference, | ||
| updatedAt: new Date().toISOString(), | ||
| }; | ||
| } catch { |
There was a problem hiding this comment.
Every error from transferSubsidiary becomes needs_attention, including clean rejections that moved no money (a non-zero statusCode such as insufficient funds, a 401 from a rotated key, INVALID_INPUT). The pending check at L238-244 then refuses every later send from that account, and no code path moves a transfer out of needs_attention: there is no transfer reconcile endpoint or job, and sweepStale only adds more. One Paga rejection means every later send gets 409 for good. Definite rejections should be marked failed, and needs_attention transfers need a way to be reconciled.
| await apiClient.quoteNairaTransfer( | ||
| account, | ||
| minor!, | ||
| requestKey(`quote:${account}:${minor}`) |
There was a problem hiding this comment.
The quote key is cached per account:amount for the life of the screen, and the server quote replay (transfers.service.ts:84-97) returns the stored record whatever its status or expiry. After a completed ₦1,000 send, previewing the same recipient and amount again returns the old completed record. Confirm then reuses keys.current[quote.id], send() returns the existing idempotency row, and no money moves while the screen looks successful. An expired preview is also stuck on "Preview expired" until the amount changes. Drop the cached key after a successful send, and make the server replay only a still-quoted, unexpired record.
| WHERE id = ${id} AND owner_id = ${ownerId} RETURNING * | ||
| `); | ||
| return view(saved.rows[0] as AccountRow); | ||
| } catch { |
There was a problem hiding this comment.
Every error lands here as needs_attention, including failures where no account can exist: the Nomba token exchange failing before any bank request (the adapter says this cannot make a write uncertain), or Paga returning 400 for a rejected BVN. After that, ON CONFLICT (owner_id, provider, environment) DO NOTHING makes every create return this stuck row, and reconcile calls retrieveAccount for an account that was never created, which fails. The user can never get a naira account for this provider. Pre-submission and definite-rejection errors should release the claim.
| if (!observedSource) | ||
| throw new ConflictException('Active source account required.'); | ||
| let balance: Awaited<ReturnType<PagaProvider['getBalance']>>; | ||
| try { |
There was a problem hiding this comment.
The balance is read before the FOR UPDATE on L207 and not re-read after the lock. Say the balance is ₦100 and the user has two ₦100 quotes. Send B reads 100 while send A is submitting. A completes, B takes the lock, the pending check passes, and 100 < 100 is false, so B is submitted with ₦0 available. Paga rejects it, and the needs_attention handling at L279 then blocks the account. Re-read the balance after taking the lock, or subtract transfers that completed since the read.
| async quote(ownerId: string, raw: NairaTransferQuoteInput) { | ||
| const input = NairaTransferQuoteBody.parse(raw); | ||
| const provider = this.provider(); | ||
| const replay = await this.db.client |
There was a problem hiding this comment.
Quote idempotency is a plain SELECT on record->>'quoteRequestKey' with no unique index behind it. A client retry that arrives while the first request is still waiting on Paga retrieveAccount misses the replay check, so two quoted rows with different ids get inserted for one key. Later replays return whichever is newest by created_at, which may not be the id the client holds. The 10-active-quote cap is also a non-atomic count. A partial unique index on (owner_id, (record->>'quoteRequestKey')) would close this.
| PAGA_SANDBOX_PUBLIC_KEY: Joi.string().allow('').optional(), | ||
| PAGA_SANDBOX_SECRET_KEY: Joi.string().allow('').optional(), | ||
| PAGA_SANDBOX_HASH_KEY: Joi.string().allow('').optional(), | ||
| FONBNK_ENV: Joi.string().valid('sandbox').optional(), |
There was a problem hiding this comment.
FONBNK_ENV is missing the .allow('') every sibling fiat variable has. .env.example ships FONBNK_ENV=sandbox, so blanking it is the natural way to turn Fonbnk off, and Joi then fails with "not allowed to be empty" and the backend won't boot, production included.
| "idx": 41, | ||
| "version": "7", | ||
| "when": 1788885236359, | ||
| "tag": "0041_consumer_fiat_orders", |
There was a problem hiding this comment.
These fiat migrations use idx and numbers 0041-0047, but main already has 0041_payment_pricing_currency through 0055_api_key_rotation_grace. Landing the stack gives conflicts in _journal.json, schema.ts and the 0044/0046/0047 snapshots. Taking both sides of the journal gives duplicate idx entries, and drizzle skips or mis-orders the fiat migrations while migrate still reports success. Regenerate them after 0055 before merging.
| segments[1] === "naira-send") | ||
| ) | ||
| return screens; | ||
| if (!isAuthenticated) return <Redirect href="/(fiat)/unified" withAnchor />; |
There was a problem hiding this comment.
With EXPO_PUBLIC_FIAT_LOCAL_DEMO=true, this runs before the loading, auth-group and app-lock branches. !isAuthenticated is also true while auth is still null, so /login can never render and signed-in users are redirected on every cold start. The four fiat screens return screens directly, which skips the lock and obscure overlays. This is dev-only, but it makes the auth flow unusable while the flag is on.
| source, | ||
| input.destinationCurrency, | ||
| debit, | ||
| source === 'NGN' ? debit * 6n : debit / 6n, |
There was a problem hiding this comment.
debit / 6n rounds down to 0 for 1-5 micro-USDC, and planSendAll throws INVALID_QUOTE when the converted amount is 0. With NGN 100000 and USDC 5 micro, send-all to NGN returns 409, so the user can't send their naira while the dust remains. When USDC isn't a multiple of 6, the remainder is debited but never credited.
| return structuredClone(await cached.value); | ||
| } | ||
| const value = this.load(ownerId); | ||
| this.cache.set(ownerId, { expiresAt: Date.now() + 15_000, value }); |
There was a problem hiding this comment.
Nothing evicts entries from this Map. Every distinct owner who opens balances keeps a resolved ObservedBalances object for the life of the process. Also, if an older load() rejects after a newer request has replaced the entry, L62 deletes the newer entry. Delete an entry only while it still holds the same promise, and prune expired entries.
There was a problem hiding this comment.
💡 Codex Review
xend-global/apps/backend/src/fiat/banking/nomba-auth.ts
Lines 91 to 92 in c3ecd33
Discard rejected refresh credentials before retrying
If a cached Nomba token is near expiry and the refresh endpoint rejects an invalid or expired refresh token with a deterministic client status other than 401/403, such as HTTP 400, this branch throws NOMBA_AUTH_REJECTED without clearing this.token. Every subsequent operation therefore selects /refresh again with the same rejected token and can never return to the client-credentials /issue flow until the process restarts. Clear the cached token for rejected refresh attempts while retaining it only for transient 429/5xx failures.
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const accountsQuery = useQuery({ | ||
| queryKey, | ||
| queryFn: () => apiClient.bankAccounts(), | ||
| enabled: isAuthenticated === true, |
There was a problem hiding this comment.
When EXPO_PUBLIC_FIAT_LOCAL_DEMO=true is used without signing in, this query remains disabled even though apiClient.bankAccounts() explicitly redirects local-demo requests to the guarded /dev/fiat/banking/accounts route without authentication. React Query consequently leaves the query pending and the bank-details sheet displays its spinner indefinitely, making the intended login-free local provider-account flow inaccessible. Gate this like useUnifiedFiat, allowing either local demo mode or an authenticated user.
Useful? React with 👍 / 👎.
|
|
||
| const email = authEmail || (SEED_DEMO ? SEED_USER.email : ""); | ||
| const data = accountsQuery.data; | ||
| const record = data?.accounts[0] ?? null; |
There was a problem hiding this comment.
When an owner has sandbox accounts for more than one provider—for example after switching FIAT_NGN_ACCOUNT_PROVIDER from Nomba to Paga—the backend returns every account ordered oldest first, but this always renders accounts[0]. The sheet can therefore show the obsolete provider, suppress creation of the newly selected provider, or repeatedly attempt to reconcile an old needs_attention account that the service now rejects because its provider is not selected. Choose the account whose provider matches data.provider rather than the first row.
Useful? React with 👍 / 👎.
| presenceProof?: string; | ||
| }; | ||
|
|
||
| let pendingCrypto: { key: string; submission: PendingSubmission } | null = null; |
There was a problem hiding this comment.
After an ambiguous crypto-submit failure, this module-level value survives sheet unmounts and account changes but is keyed only by recipient and amount. If Consumer A's submit actually succeeded while both responses were lost, then Consumer B signs in and sends the same amount to the same recipient, the app replays A's intent without a new biometric; TransferService.submit returns the existing transfer before checking intent ownership, so B is shown “Sent” even though no transfer was made from B's account. Include the consumer identity in this cache and clear it whenever that identity changes.
Useful? React with 👍 / 👎.
1 participant
Connects bank adapters to owner-scoped consumer APIs and production-shaped mobile flows for NGN account creation/recovery, provider and devnet balance observations, and Paga subsidiary transfers. Adds destination-currency-first funding plans, durable reservations and reconciliation state. Consumer-facing simulator controls have been removed; the legacy unified route redirects to observed balances, and fiat entry points remain development-only until production providers are enabled.
Nomba is the selected sandbox provider. An active account was created through the provisioning service, matched against an authenticated provider read, and displayed through the running backend in the iPhone simulator. The mobile account screen only offers the Paga-specific send flow when Paga is selected.
Adds
POST /webhooks/nomba/sandboxand migration 0046 for durable provider notifications. The receiver verifies the signature, timestamp and configured merchant, deduplicates concurrent deliveries across restarts, and retains only signed identity fields. Nomba does not sign amount or beneficiary; notifications cannot credit a balance. The receiver is disabled in production and without sandbox configuration. Migration 0047 adds automatic authenticated requery with durable claims, bounded jittered retries, crash recovery and stale-response fencing. Conflicting records are retained as needs_attention. The dashboard’s unsigned empty connectivity check is acknowledged without persistence; unsigned payment notifications remain rejected. Migrations 0041–0045 support the preceding account/order/transfer flows, and matching 0046/0047 Drizzle snapshots keep future generation safe.Hardening in the final integration commit also:
Validation on the combined tree:
drizzle-kit generatereports no schema changes.checks,sdkandmigratejobs.This remains a sandbox-only integration. The sandbox payout requery returned mismatched destination details, and nonexistent transaction references returned SUCCESS. The documented ₦100 test bank deposit produced conflicting callback and authenticated lookup fee evidence, so the worker correctly retained TRANSACTION_IDENTITY_MISMATCH. Account attribution and ledger credits, funded NGN payouts, executable NGN↔USDC conversion, signed Solana linkage and actual unified spending remain intentionally disabled. Observed provider balances are never treated as spendable.
Stack (bottom → top): foundation #93 → Paga #96 → Nomba #97 → consumer integration #98. Native GitHub stack #99. Merge from the bottom upward.