From abf5fd9e0832a1a52be0fd589088be32e7e26944 Mon Sep 17 00:00:00 2001 From: janetpius-cmd Date: Mon, 31 Aug 2026 12:43:46 +0100 Subject: [PATCH 1/4] fix(payments): guard refund() with a row lock + transaction to prevent double-payout Wraps the refund read-check-transfer-write sequence in a DB transaction using manager.findOne(..., { lock: { mode: 'pessimistic_write' } }) instead of a plain findOne() + separate save(). Previously, two concurrent POST /payments/:id/refund calls for the same settled payment (double-click, retried request) could both pass the status !== SETTLED check before either had saved, sending two real Stellar payments for one logical refund. --- src/payments/payments.service.ts | 158 +++++++++++++++++-------------- 1 file changed, 87 insertions(+), 71 deletions(-) diff --git a/src/payments/payments.service.ts b/src/payments/payments.service.ts index d479fbee..6eb891e1 100644 --- a/src/payments/payments.service.ts +++ b/src/payments/payments.service.ts @@ -1,6 +1,6 @@ import { Injectable, NotFoundException, BadRequestException, Logger } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; -import { Repository } from 'typeorm'; +import { DataSource, Repository } from 'typeorm'; import { v4 as uuidv4 } from 'uuid'; import * as QRCode from 'qrcode'; import * as StellarSdk from '@stellar/stellar-sdk'; @@ -39,6 +39,7 @@ export class PaymentsService { private merchants: MerchantsService, private soroban: SorobanService, private analytics: AnalyticsService, + private dataSource: DataSource, ) {} async create(merchantId: string, dto: CreatePaymentDto): Promise { @@ -255,85 +256,100 @@ export class PaymentsService { } async refund(id: string, merchantId: string, dto: RefundPaymentDto): Promise { - const payment = await this.findOne(id, merchantId); - - if (payment.status !== PaymentStatus.SETTLED) { - throw new BadRequestException('Only settled payments can be refunded'); - } - - if (!payment.customerWalletAddress) { - throw new BadRequestException('Customer wallet address is unknown. Manual refund required.'); - } - const merchant = await this.merchants.findOne(merchantId); - // Determine refund amount - const refundAmountUsd = dto.amountUsd || payment.amountUsd; - if (refundAmountUsd > payment.amountUsd) { - throw new BadRequestException('Refund amount cannot exceed original payment amount'); - } - - // Determine asset and amount for Stellar transfer - let asset: StellarSdk.Asset; - let amountStr: string; - - if (payment.amountUsdc) { - asset = this.stellar.getUsdcAsset(); - // If partial refund, we need to calculate USDC amount based on ratio - const ratio = refundAmountUsd / payment.amountUsd; - const amountUsdc = payment.amountUsdc * ratio; - amountStr = amountUsdc.toFixed(7); - } else { - asset = StellarSdk.Asset.native(); - const ratio = refundAmountUsd / payment.amountUsd; - const amountXlm = payment.amountXlm * ratio; - amountStr = amountXlm.toFixed(7); - } - - const memo = `REFUND-${payment.reference.split('-').pop()}`; - + // Read-check-transfer-write happens under a row lock inside a DB + // transaction so two concurrent refund calls for the same payment + // (double-click, retried request) cannot both pass the status check + // before either has saved — preventing a duplicate Stellar payout. try { - const txHash = await this.stellar.sendPayment( - payment.customerWalletAddress, - amountStr, - asset, - memo, - ); - - payment.status = PaymentStatus.REFUNDED; - payment.refundAmountUsd = refundAmountUsd; - payment.refundReason = dto.reason; - payment.refundTxHash = txHash; - payment.refundedAt = new Date(); - - const saved = await this.paymentsRepo.save(payment); - - // Dispatch webhook - await this.webhooks.dispatch(merchantId, 'payment.refunded', { - paymentId: payment.id, - reference: payment.reference, - refundAmountUsd, - refundTxHash: txHash, - reason: dto.reason, - }); + return await this.dataSource.transaction(async (manager) => { + const payment = await manager.findOne(Payment, { + where: { id, merchantId }, + lock: { mode: 'pessimistic_write' }, + }); + if (!payment) throw new NotFoundException('Payment not found'); + + if (payment.status !== PaymentStatus.SETTLED) { + throw new BadRequestException('Only settled payments can be refunded'); + } + + if (!payment.customerWalletAddress) { + throw new BadRequestException( + 'Customer wallet address is unknown. Manual refund required.', + ); + } + + // Determine refund amount + const refundAmountUsd = dto.amountUsd || payment.amountUsd; + if (refundAmountUsd > payment.amountUsd) { + throw new BadRequestException('Refund amount cannot exceed original payment amount'); + } + + // Determine asset and amount for Stellar transfer + let asset: StellarSdk.Asset; + let amountStr: string; + + if (payment.amountUsdc) { + asset = this.stellar.getUsdcAsset(); + // If partial refund, we need to calculate USDC amount based on ratio + const ratio = refundAmountUsd / payment.amountUsd; + const amountUsdc = payment.amountUsdc * ratio; + amountStr = amountUsdc.toFixed(7); + } else { + asset = StellarSdk.Asset.native(); + const ratio = refundAmountUsd / payment.amountUsd; + const amountXlm = payment.amountXlm * ratio; + amountStr = amountXlm.toFixed(7); + } + + const memo = `REFUND-${payment.reference.split('-').pop()}`; + + const txHash = await this.stellar.sendPayment( + payment.customerWalletAddress, + amountStr, + asset, + memo, + ); - // Send emails - await this.notifications.enqueueEmail({ - recipient: merchant.email, - subject: `Refund processed: ${payment.reference}`, - html: `

A refund of $${refundAmountUsd} has been processed for payment ${payment.reference}.

Reason: ${dto.reason}

`, - }); + payment.status = PaymentStatus.REFUNDED; + payment.refundAmountUsd = refundAmountUsd; + payment.refundReason = dto.reason; + payment.refundTxHash = txHash; + payment.refundedAt = new Date(); + + const saved = await manager.save(payment); + + // Dispatch webhook + await this.webhooks.dispatch(merchantId, 'payment.refunded', { + paymentId: payment.id, + reference: payment.reference, + refundAmountUsd, + refundTxHash: txHash, + reason: dto.reason, + }); - if (payment.customerEmail) { + // Send emails await this.notifications.enqueueEmail({ - recipient: payment.customerEmail, - subject: `Refund received from ${merchant.businessName}`, - html: `

A refund of $${refundAmountUsd} has been processed for your payment ${payment.reference}.

The funds have been sent back to your Stellar wallet.

`, + recipient: merchant.email, + subject: `Refund processed: ${payment.reference}`, + html: `

A refund of $${refundAmountUsd} has been processed for payment ${payment.reference}.

Reason: ${dto.reason}

`, }); - } - return saved; + if (payment.customerEmail) { + await this.notifications.enqueueEmail({ + recipient: payment.customerEmail, + subject: `Refund received from ${merchant.businessName}`, + html: `

A refund of $${refundAmountUsd} has been processed for your payment ${payment.reference}.

The funds have been sent back to your Stellar wallet.

`, + }); + } + + return saved; + }); } catch (err) { + if (err instanceof BadRequestException || err instanceof NotFoundException) { + throw err; + } throw new BadRequestException(`Stellar refund failed: ${err.message}`); } } From 24b5af438be8e6190090268a90332d7b16be1a55 Mon Sep 17 00:00:00 2001 From: janetpius-cmd Date: Mon, 31 Aug 2026 12:44:12 +0100 Subject: [PATCH 2/4] fix(payments): add PARTIALLY_REFUNDED status and track cumulative refund amount Adds PaymentStatus.PARTIALLY_REFUNDED and changes refund() to sum refundAmountUsd across multiple calls instead of overwriting it, and to guard against refunding more than the remaining un-refunded balance rather than the full original amount. Previously, a single partial refund incorrectly flipped status to REFUNDED (implying the full amount was returned) and blocked any further legitimate partial refund via the status !== SETTLED check. --- src/payments/entities/payment.entity.ts | 1 + src/payments/payments.service.ts | 28 ++++++++++++++++++------- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/payments/entities/payment.entity.ts b/src/payments/entities/payment.entity.ts index 91444f33..667d2dcc 100644 --- a/src/payments/entities/payment.entity.ts +++ b/src/payments/entities/payment.entity.ts @@ -19,6 +19,7 @@ export enum PaymentStatus { FAILED = 'failed', EXPIRED = 'expired', REFUNDED = 'refunded', + PARTIALLY_REFUNDED = 'partially_refunded', } export enum PaymentNetwork { diff --git a/src/payments/payments.service.ts b/src/payments/payments.service.ts index 6eb891e1..49d8dfa7 100644 --- a/src/payments/payments.service.ts +++ b/src/payments/payments.service.ts @@ -270,7 +270,10 @@ export class PaymentsService { }); if (!payment) throw new NotFoundException('Payment not found'); - if (payment.status !== PaymentStatus.SETTLED) { + if ( + payment.status !== PaymentStatus.SETTLED && + payment.status !== PaymentStatus.PARTIALLY_REFUNDED + ) { throw new BadRequestException('Only settled payments can be refunded'); } @@ -280,10 +283,17 @@ export class PaymentsService { ); } - // Determine refund amount - const refundAmountUsd = dto.amountUsd || payment.amountUsd; - if (refundAmountUsd > payment.amountUsd) { - throw new BadRequestException('Refund amount cannot exceed original payment amount'); + // Track cumulative refunds against the original amount instead of + // overwriting refundAmountUsd on every call, so a payment that has + // already been partially refunded can still be refunded further + // (up to the remaining un-refunded balance). + const alreadyRefundedUsd = payment.refundAmountUsd || 0; + const remainingRefundableUsd = payment.amountUsd - alreadyRefundedUsd; + const refundAmountUsd = dto.amountUsd || remainingRefundableUsd; + if (refundAmountUsd > remainingRefundableUsd) { + throw new BadRequestException( + 'Refund amount cannot exceed the remaining un-refunded balance', + ); } // Determine asset and amount for Stellar transfer @@ -312,8 +322,12 @@ export class PaymentsService { memo, ); - payment.status = PaymentStatus.REFUNDED; - payment.refundAmountUsd = refundAmountUsd; + const totalRefundedUsd = alreadyRefundedUsd + refundAmountUsd; + payment.status = + totalRefundedUsd >= payment.amountUsd + ? PaymentStatus.REFUNDED + : PaymentStatus.PARTIALLY_REFUNDED; + payment.refundAmountUsd = totalRefundedUsd; payment.refundReason = dto.reason; payment.refundTxHash = txHash; payment.refundedAt = new Date(); From 017380cd9d26b9f8347ed349fd27ac0d312f8713 Mon Sep 17 00:00:00 2001 From: janetpius-cmd Date: Mon, 31 Aug 2026 12:45:01 +0100 Subject: [PATCH 3/4] fix(payments): don't report webhook/email failures as a refund failure Splits refund() into two phases: the Stellar transfer + DB save keep their own try/catch (the genuine refund-failure path), while webhook dispatch and notification emails run afterward in separate best-effort try/catch blocks that log on failure instead of throwing. Previously a webhook or email error after a successful Stellar payout was caught by the same catch block and reported to the caller as "Stellar refund failed", which could mislead a merchant into manually re-attempting a refund that had already gone through. --- src/payments/payments.service.ts | 78 +++++++++++++++++++++----------- 1 file changed, 52 insertions(+), 26 deletions(-) diff --git a/src/payments/payments.service.ts b/src/payments/payments.service.ts index 49d8dfa7..22a12158 100644 --- a/src/payments/payments.service.ts +++ b/src/payments/payments.service.ts @@ -258,12 +258,18 @@ export class PaymentsService { async refund(id: string, merchantId: string, dto: RefundPaymentDto): Promise { const merchant = await this.merchants.findOne(merchantId); + let payment: Payment; + let refundAmountUsd: number; + let txHash: string; + + // Step 1: the actual financial action — Stellar transfer + DB save — + // is isolated in its own try/catch (the real refund-failure path). // Read-check-transfer-write happens under a row lock inside a DB // transaction so two concurrent refund calls for the same payment // (double-click, retried request) cannot both pass the status check // before either has saved — preventing a duplicate Stellar payout. try { - return await this.dataSource.transaction(async (manager) => { + const result = await this.dataSource.transaction(async (manager) => { const payment = await manager.findOne(Payment, { where: { id, merchantId }, lock: { mode: 'pessimistic_write' }, @@ -334,37 +340,57 @@ export class PaymentsService { const saved = await manager.save(payment); - // Dispatch webhook - await this.webhooks.dispatch(merchantId, 'payment.refunded', { - paymentId: payment.id, - reference: payment.reference, - refundAmountUsd, - refundTxHash: txHash, - reason: dto.reason, - }); - - // Send emails - await this.notifications.enqueueEmail({ - recipient: merchant.email, - subject: `Refund processed: ${payment.reference}`, - html: `

A refund of $${refundAmountUsd} has been processed for payment ${payment.reference}.

Reason: ${dto.reason}

`, - }); - - if (payment.customerEmail) { - await this.notifications.enqueueEmail({ - recipient: payment.customerEmail, - subject: `Refund received from ${merchant.businessName}`, - html: `

A refund of $${refundAmountUsd} has been processed for your payment ${payment.reference}.

The funds have been sent back to your Stellar wallet.

`, - }); - } - - return saved; + return { saved, refundAmountUsd, txHash }; }); + + payment = result.saved; + refundAmountUsd = result.refundAmountUsd; + txHash = result.txHash; } catch (err) { if (err instanceof BadRequestException || err instanceof NotFoundException) { throw err; } throw new BadRequestException(`Stellar refund failed: ${err.message}`); } + + // Step 2: webhook + email dispatch happens *after* the financial action + // has already succeeded and been saved. Failures here (e.g. a webhook + // queue error) must not be reported as a refund failure — the funds have + // genuinely moved. Log and continue on a best-effort basis instead. + try { + await this.webhooks.dispatch(merchantId, 'payment.refunded', { + paymentId: payment.id, + reference: payment.reference, + refundAmountUsd, + refundTxHash: txHash, + reason: dto.reason, + }); + } catch (err) { + this.logger.error( + `Refund webhook dispatch failed for payment ${payment.id} (refund already completed): ${err.message}`, + ); + } + + try { + await this.notifications.enqueueEmail({ + recipient: merchant.email, + subject: `Refund processed: ${payment.reference}`, + html: `

A refund of $${refundAmountUsd} has been processed for payment ${payment.reference}.

Reason: ${dto.reason}

`, + }); + + if (payment.customerEmail) { + await this.notifications.enqueueEmail({ + recipient: payment.customerEmail, + subject: `Refund received from ${merchant.businessName}`, + html: `

A refund of $${refundAmountUsd} has been processed for your payment ${payment.reference}.

The funds have been sent back to your Stellar wallet.

`, + }); + } + } catch (err) { + this.logger.error( + `Refund notification email failed for payment ${payment.id} (refund already completed): ${err.message}`, + ); + } + + return payment; } } From 3a4d9f77098a9c92cc653ef29013ea815a0504be Mon Sep 17 00:00:00 2001 From: janetpius-cmd Date: Mon, 31 Aug 2026 12:45:20 +0100 Subject: [PATCH 4/4] fix(payments): wrap createBatch() persistence in a DB transaction createBatch()'s docstring claims the entire batch reverts if any single entry is invalid and "no partial writes ever reach the database," but the actual persistence step was a plain paymentsRepo.save(records) on an array, which TypeORM does not guarantee is atomic against DB-level failures (e.g. a unique constraint collision on a later item, or connection loss mid-batch). Wrapping the save in dataSource.transaction() backs that atomicity claim with an actual database transaction. --- src/payments/payments.service.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/payments/payments.service.ts b/src/payments/payments.service.ts index 22a12158..e7959e7f 100644 --- a/src/payments/payments.service.ts +++ b/src/payments/payments.service.ts @@ -203,8 +203,14 @@ export class PaymentsService { }); } - // ── Persist all records in one shot (atomic) ────────────────────────────── - const saved = await this.paymentsRepo.save(records); + // ── Persist all records inside one DB transaction. A plain array save() + // is not guaranteed atomic against constraint violations that only + // manifest at insert time (e.g. a unique-constraint collision) or a + // connection loss mid-batch — wrapping in a transaction backs the + // "no partial writes" contract documented above with the database. ── + const saved = await this.dataSource.transaction(async (manager) => { + return manager.save(records); + }); // ── Emit PaymentCreated event for each entry (mirrors contract event log) ─ for (const event of events) {