[FEAT] RLUSD 토큰 에스크로 결제 지원 및 서비스 레이어 리팩토링#21
Merged
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- CRUD(create/findAll/findById)를 EscrowPaymentsCrudService로 추출 - createXrplEscrow를 EscrowCreateProcessor로 직접 이동 - recoverSubmittingEscrow를 EscrowSubmitRecoveryScheduler로 직접 이동 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- makeCrudServiceTestingModule / makeProcessorTestingModule 헬퍼 추가 - xrpl.spec.ts를 EscrowCreateProcessor 기반으로 재작성 - recovery-scheduler.spec.ts에 recoverSubmittingEscrow 직접 테스트 추가 - XrplService RLUSD 메서드 단위 테스트 추가 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- escrow-rlusd-mock.e2e-spec.ts / jest-e2e-rlusd-testnet.json 신규 추가 - jest-e2e.json에 rlusd-testnet 제외 패턴 추가 (mock 실행 시 분리) - 기존 XRP mock/testnet e2e에서 EscrowCreateProcessor 직접 주입으로 수정 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthrough이 PR은 XRPL 에스크로에 RLUSD(IOU) 지원을 추가하고, 결제 CRUD를 전용 서비스로 분리하며 XRPL 에스크로 제출 로직을 프로세서로 이동하고 관련 XRPL/복구/테스트/CI를 확장합니다. ChangesRLUSD 에스크로 & 시스템 리팩토링
Sequence DiagramsequenceDiagram
participant Client
participant API
participant Processor
participant XrplService
participant Ledger
Client->>API: POST /escrow-payments (currency=RLUSD)
API->>API: persist via EscrowPaymentsCrudService.create
API->>Processor: enqueue createXrplEscrow job
Processor->>XrplService: createEscrow(..., currency="RLUSD")
XrplService->>Ledger: submit EscrowCreate (IOU amount)
Ledger-->>XrplService: tx result (tesSUCCESS / error)
XrplService-->>Processor: txHash/sequence
Processor->>API: update escrow -> ESCROWED / handle rollback
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 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 |
10 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/modules/escrow-payments/escrow-payments.controller.ts (1)
174-225:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift참여자 검증 컨텍스트 누락으로 IDOR 위험이 남아 있습니다.
Line 189, Line 220의 엔드포인트는
@CurrentUser()를 받지 않아 사용자 식별자를 서비스로 전달하지 않습니다. 이 구조에서는 결제 참여자 검증이 불가능해, 인증된 제3자가 ID를 아는 경우 상태 조회/취소를 시도할 수 있습니다.🔧 수정 예시 (컨트롤러 기준)
getEscrowStatus( `@Param`("id", ParseMongoIdPipe) id: string, `@Param`("escrowId", ParseMongoIdPipe) escrowId: string, + `@CurrentUser`() user: SessionUser, ) { - return this.service.getEscrowStatus(id, escrowId); + return this.service.getEscrowStatus(id, escrowId, user.userId); } cancelEscrowItem( `@Param`("id", ParseMongoIdPipe) id: string, `@Param`("escrowId", ParseMongoIdPipe) escrowId: string, + `@CurrentUser`() user: SessionUser, ) { - return this.service.cancelEscrowItem(id, escrowId); + return this.service.cancelEscrowItem(id, escrowId, user.userId); }🤖 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 `@src/modules/escrow-payments/escrow-payments.controller.ts` around lines 174 - 225, The getEscrowStatus and cancelEscrowItem endpoints are missing participant verification because they don't accept the current user; add a `@CurrentUser`() parameter (e.g., user or userId) to both controller methods (getEscrowStatus and cancelEscrowItem) and pass that identifier into this.service.getEscrowStatus(...) and this.service.cancelEscrowItem(...), and then update the corresponding service method signatures to require and enforce participant authorization so the service validates the caller is a payment participant before returning status or performing cancel.
♻️ Duplicate comments (1)
src/modules/escrow-payments/escrow-create.processor.ts (1)
177-222:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPre-flight / 롤백 쿼리도
$positional + 다중 조건 패턴 — 위 스케줄러 코멘트와 동일 이슈L177-191 (pre-flight)와 L215-222 (XRPL 실패 시 PENDING_ESCROW 복구) 모두
{ "escrows._id": ..., "escrows.status": "..." }형태로 같은 배열에 두 조건을 분리해 두고$.status를 갱신합니다. 한 결제에 여러 escrow가 있고 일부만 같은 상태일 수 있는 본 도메인에서는 다른 escrow가 갱신될 수 있으니,$elemMatch로 묶어 안전하게 만들어 주세요(scheduler 코멘트 참조).특히 pre-flight는 PROCESSING 결제에서 동시에 여러 escrow를 처리할 때 race로 인해
PENDING_ESCROW인 다른 escrow가 SUBMITTING으로 잘못 전이될 수 있어 영향이 큽니다.🛠️ pre-flight / 실패 복구 둘 다 $elemMatch로
const preFlighted = await this.escrowPaymentModel.findOneAndUpdate( - { - _id: paymentId, - "escrows._id": escrow._id, - "escrows.status": "PENDING_ESCROW", - }, + { + _id: paymentId, + escrows: { + $elemMatch: { _id: escrow._id, status: "PENDING_ESCROW" }, + }, + }, { ... }, ); ... // Case A await this.escrowPaymentModel.findOneAndUpdate( - { - _id: paymentId, - "escrows._id": escrow._id, - "escrows.status": "SUBMITTING", - }, + { + _id: paymentId, + escrows: { + $elemMatch: { _id: escrow._id, status: "SUBMITTING" }, + }, + }, { $set: { "escrows.$.status": "PENDING_ESCROW" } }, );🤖 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 `@src/modules/escrow-payments/escrow-create.processor.ts` around lines 177 - 222, The pre-flight and rollback findOneAndUpdate queries in escrow-create.processor.ts (the one that sets preFlighted and the rollback call inside the XRPL catch) currently match "escrows._id" and "escrows.status" as separate top-level predicates which can update the wrong array element; change both queries to use an $elemMatch on "escrows" (e.g. "escrows": { $elemMatch: { _id: escrow._id, status: "PENDING_ESCROW" } } for pre-flight and similarly status "SUBMITTING" for the rollback) so the positional $ update targets the same escrow element; keep the $set targets ("escrows.$.status", "escrows.$.condition", etc.) and variable names (preFlighted, txHash/sequence) the same.
🧹 Nitpick comments (8)
src/modules/escrow-payments/escrow-payments-crud.service.ts (1)
82-92: 💤 Low valuelean() 반환 타입과 선언 타입 불일치
lean()메서드는 Mongoose Document가 아닌 순수 JavaScript 객체(POJO)를 반환하지만, 반환 타입이EscrowPaymentDocument[](Document를 상속함)로 선언되어 있습니다. 읽기 전용 작업에서는 실제 런타임 동작에 문제가 없지만, 타입 안정성 측면에서 정확하지 않습니다.더 정확한 타입을 위해 반환 타입을
Array<Omit<EscrowPaymentDocument, keyof Document>>또는 별도의 read DTO 타입으로 변경하는 것을 고려해보세요. 다만 현재 컨트롤러에서 Document 타입을 기대한다면 구조 변경이 필요할 수 있습니다.🤖 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 `@src/modules/escrow-payments/escrow-payments-crud.service.ts` around lines 82 - 92, The query uses this.escrowPaymentModel.find(...).lean() but the service returns data typed as EscrowPaymentDocument[], which is incorrect because lean() returns POJOs; update the return type to a read-safe type (e.g., Array<Omit<EscrowPaymentDocument, keyof Document>> or a dedicated EscrowPaymentReadDto) and adjust any callers if they expect full Mongoose Documents; locate usages around this.escrowPaymentModel.find(...).lean() and the return statement "data: data as unknown as EscrowPaymentDocument[]" and replace the cast with the correct DTO/type to preserve type safety.test/escrow-rlusd-testnet.e2e-spec.ts (1)
244-268: 💤 Low valueDTO
currency: "RLUSD"인데 XRPL에는TST로 발행 — 의도라면 코멘트 보강 필요
iouCurrencyCode는TST로 설정되어 있고 실제 XRPL 트랜잭션의Amount.currency도TST로 나가지만, DTO에는currency: "RLUSD"를 넣고 있어 코드만 보면 맞지 않아 보입니다. 서비스 분기 키(payment.currency === "RLUSD")와 XRPL 발행 통화 코드(iouCurrencyCode)가 분리되어 있다는 사실이 의도라면, 테스트 상단 주석이나currency필드 옆에 한 줄 보강해 두면 향후 유지보수자가 헷갈리지 않을 것 같습니다. 동시에, DB 필드 자체도currency: "IOU"같은 일반 키로 추상화하는 편이 더 견고할 수 있는지 함께 고민해보시면 좋겠습니다.🤖 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 `@test/escrow-rlusd-testnet.e2e-spec.ts` around lines 244 - 268, The test sets DTO currency to "RLUSD" while XRPL uses iouCurrencyCode "TST"; clarify this intentional separation by adding an inline comment near the payment DTO (next to currency: "RLUSD") explaining that payment.currency is a service-branching key (used in conditions like payment.currency === "RLUSD") while iouCurrencyCode controls the XRPL Amount.currency, and also add a TODO note to consider using an abstract DB field name (e.g., currencyType or IOU currency) for clarity in the payment model and related code paths (create/escrows handling).src/modules/escrow-payments/escrow-create.processor.ts (2)
25-39: ⚡ Quick winNon-retryable 코드 매칭이
String.includes기반 — 부분 일치 오탐 우려
NON_RETRYABLE_CODES.some((code) => err.message.includes(code))방식은err.message에 우연히 같은 substring이 들어가면 false positive 가 됩니다. 또한 XRPL 라이브러리/네트워크에 따라 메시지 포맷이 바뀌면 매칭이 깨지는 취약점도 있습니다.가능하면
xrpl.service.ts의XrplTransactionFailedException에engineResult필드를 노출해 코드 단위 비교(err.engineResult === "tecNO_LINE")로 옮기는 게 안전합니다. 본 PR에서 IOU용tecNO_LINE/tecPATH_DRY/tecNO_PERMISSION이 추가되어 매칭 대상이 늘어난 만큼, 이 시점에 한 번 정리하는 비용 대비 효용이 좋습니다.♻️ 코드 비교로 전환
// xrpl.service.ts: XrplTransactionFailedException에 engineResult 필드 추가 // throw new XrplTransactionFailedException("EscrowCreate", meta?.TransactionResult ?? "unknown"); // ^ 이 값을 .engineResult로 보존 // processor: function isNonRetryable(err: Error): boolean { const code = (err as any).engineResult as string | undefined; return code !== undefined && NON_RETRYABLE_CODES.includes(code); }🤖 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 `@src/modules/escrow-payments/escrow-create.processor.ts` around lines 25 - 39, Expose the XRPL engineResult on the thrown XrplTransactionFailedException in xrpl.service.ts (preserve meta?.TransactionResult as an engineResult property on the exception instance), then update isNonRetryable to read that property (const code = (err as any).engineResult) and perform exact membership checking against NON_RETRYABLE_CODES (use NON_RETRYABLE_CODES.includes(code)) instead of string.includes; keep a safe fallback (return false) when engineResult is undefined.
209-212: 🏗️ Heavy lift
amountXrp필드 명을 RLUSD에 그대로 재사용 — 도메인 의미와 어긋남스키마/DTO/프로세서 전반에서
amountXrp를 RLUSD(IOU) 금액으로도 사용 중인데, 필드 이름이 단위까지 박혀 있어 코드 리딩/감사/마이그레이션 모두에서 혼란이 큽니다(escrow-rlusd-testnet.e2e-spec.ts에서도amountXrp: 10 // TST 단위식으로 별도 주석을 다는 형국). 외부 API 호환성을 깨지 않는 선에서 단계적으로amount+currency로 정리하는 마이그레이션 플랜을 검토해 보시기 바랍니다.당장 PR에서 처리할 일은 아니지만, RLUSD가 정식 지원되는 시점에는 큰 부채가 될 수 있어 별도 이슈로 관리하시는 걸 추천드립니다.
원하시면 신규 필드(
amount,currency)로 단계적 마이그레이션하는 백필/리네임 ADR 초안을 별도 이슈로 정리해 드릴 수 있습니다.🤖 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 `@src/modules/escrow-payments/escrow-create.processor.ts` around lines 209 - 212, The code reuses the field escrow.amountXrp for non-XRP IOUs which embeds a unit in the name and will cause confusion; add new neutral fields (e.g., escrow.amount and escrow.currency) and update the processor to read amount from escrow.amount (falling back to escrow.amountXrp) and currency from escrow.currency (falling back to payment.currency ?? "XRP") so behavior stays backward-compatible; update DTO/schema creation flows that set escrow.amountXrp to also set escrow.amount and escrow.currency, and file a follow-up migration/ADR issue to phase out amountXrp once consumers are migrated.src/modules/escrow-payments/escrow-payments.service.ts (2)
106-115: 💤 Low value
select("")는 의도가 불분명 — 조건부 select 자체를 분기로 빼는 편이 명확Mongoose에서 빈 문자열
select("")는 사실상 no-op이지만 "필드 선택 안 함"이라는 의도가 코드에 잘 드러나지 않고, 향후+wallet.seed처럼select: false필드가 추가될 때 실수로 따라붙을 위험이 있습니다. seed가 필요한지 여부를 분기 자체로 분리하는 편이 의도가 명확합니다.♻️ 분기 분리 예시
- // RLUSD는 TrustSet 서명에 seed 필요 → +wallet.seed 포함 조회 - const withSeed = payment.currency === "RLUSD"; - const [buyerUser, sellerUser] = await Promise.all([ - this.userModel - .findById(payment.buyerId) - .select(withSeed ? "+wallet.seed" : ""), - this.userModel - .findById(payment.sellerId) - .select(withSeed ? "+wallet.seed" : ""), - ]); + // RLUSD는 TrustSet 서명에 seed 필요 → +wallet.seed 포함 조회 + const buyerQuery = this.userModel.findById(payment.buyerId); + const sellerQuery = this.userModel.findById(payment.sellerId); + if (payment.currency === "RLUSD") { + buyerQuery.select("+wallet.seed"); + sellerQuery.select("+wallet.seed"); + } + const [buyerUser, sellerUser] = await Promise.all([buyerQuery, sellerQuery]);🤖 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 `@src/modules/escrow-payments/escrow-payments.service.ts` around lines 106 - 115, The conditional use of .select(withSeed ? "+wallet.seed" : "") is unclear because select("") is a no-op; instead split the branch: compute withSeed = payment.currency === "RLUSD", then for buyer/seller call this.userModel.findById(id).select("+wallet.seed") only when withSeed is true and call this.userModel.findById(id) without .select() when false (i.e., avoid passing an empty string to .select). Update the Promise.all that builds [buyerUser, sellerUser] to use these two distinct call sites so the intent is explicit.
120-143: 💤 Low valueRLUSD pre-flight 부분 실패 시 trust line 잔존 — 무해하지만 운영 관점에서 인지 필요
Promise.all로 buyer/sellerensureRlusdTrustLine을 동시 실행하기 때문에 한쪽이 성공하고 다른 쪽이 실패하면 XRPL에는 trust line이 부분적으로 남고 결제는APPROVED로 머무릅니다. trust line은 멱등이라 재시도 시 중복 발행은 없지만, 한쪽이 영구 실패(tecNO_PERMISSION등)인 경우 사용자는 이유 없이 계속 reserve 2 XRP를 점유당하는 상태가 됩니다. 운영 알림/메트릭 측면에서 두 호출 결과를 구분 가능한 로그로 남기는 것을 추천드립니다(특히 본 서비스는 해외 바이어 + 국내 셀러 구조라 IOU 권한 이슈가 한쪽에서만 발생할 가능성이 큽니다).🤖 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 `@src/modules/escrow-payments/escrow-payments.service.ts` around lines 120 - 143, The concurrent Promise.all call to ensureRlusdTrustLine can leave a partial trust line if one call fails; update the RLUSD pre-flight logic in escrow-payments.service (the block around payment.currency === "RLUSD" and the ensureRlusdTrustLine calls) to call ensureRlusdTrustLine for buyer and seller separately (or wrap each call in its own try/catch) so you can log success/failure distinctly for buyerUser.wallet.address and sellerUser.wallet.address; on per-call failures record the error and relevant addresses in structured logs/metrics and only proceed to xrplService.validateRlusdFunds when both trust-line attempts either succeeded or have recorded fatal errors, preserving the WalletNotAvailableException behavior for missing seeds/addresses.src/modules/xrpl/xrpl.service.ts (1)
215-253: 💤 Low valueIOU 에스크로 시 추가 Flags 불필요 — XLS-85 명세 확인 완료
XLS-85 최종 명세에 따르면 IOU 토큰 에스크로는 EscrowCreate 트랜잭션의 Flags 필드에
tfFulfillmentDelivery,tfPartialPayment등 추가 플래그가 필요하지 않습니다. 현재 구현이 이미 필수 요건을 충족하고 있습니다:
- 발행자의
asfAllowTrustLineLocking요구사항:enableTrustLineLocking에서 처리 ✓CancelAfter필드(IOU 에스크로는 필수): 코드에 포함 ✓다만 토큰 발행 설정에 따라 송금자/수취인의 사전 인증(pre-authorization) 여부를 확인할 수 있는 로직 추가를 권장합니다. 테스트넷 e2e 통과 후 mainnet 전환 직전에 토큰별 인증 정책을 한 번 더 검토해 주세요.
🤖 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 `@src/modules/xrpl/xrpl.service.ts` around lines 215 - 253, The review notes that IOU escrow does not require extra Flags and the current createEscrow implementation is fine, but asks you to add a pre-authorization check for IOU tokens before submitting EscrowCreate; update createEscrow to detect when currency !== "XRP" (or when buildEscrowAmount returns an IOU) and call a new helper (e.g., isPreauthorizedIssuerOrCounterparty or checkTrustlinePreauth) to verify the sender/recipient are pre-authorized per the token's issuer settings (using ledger trustline/issuer flags via this.client or relevant XRPL helper), and if not pre-authorized, throw a clear error or return a distinct rejection before autofill/sign/submit; keep existing CancelAfter handling and do not add extra Flags.ci-check.sh (1)
30-30: ⚡ Quick winLine 30:
&&체인의|| fail패턴을 명시적 조건문으로 개선 권장현재 코드는 기능상 문제없이 동작하지만,
A && B || C패턴은 ShellCheck에서 SC2015 경고를 발생시킵니다. 이 파일의 다른 검사들(14, 18, 22, 26줄)은 단일 명령이지만, 30줄은 두 명령을 연결하는 유일한 경우라 더 명확한 형태로 개선할 가치가 있습니다.개선 예시
-npm run test:e2e:testnet && npm run test:e2e:rlusd-testnet || fail "E2E 테스트 실패" +if ! npm run test:e2e:testnet || ! npm run test:e2e:rlusd-testnet; then + fail "E2E 테스트 실패" +fi🤖 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 `@ci-check.sh` at line 30, Replace the terse A && B || C pattern on the line running "npm run test:e2e:testnet && npm run test:e2e:rlusd-testnet || fail ..." with an explicit conditional: run "npm run test:e2e:testnet" and if it succeeds run "npm run test:e2e:rlusd-testnet", otherwise call fail; implement this using an if ... then ... else ... fi block so the intent is clear and ShellCheck SC2015 is avoided while keeping the same behavior.
🤖 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 `@src/modules/escrow-payments/escrow-create.processor.ts`:
- Around line 199-212: The log always prints "XRP" regardless of payment
currency; update the Escrow submission log in the block around logger.log and
the createEscrow call to derive a displayCurrency = payment.currency ?? "XRP"
and log the corresponding amount with that currency (use escrow.amountXrp for
XRP and the appropriate RLUSD amount field when displayCurrency !== "XRP" or at
minimum log the chosen displayCurrency alongside escrow.amountXrp), and ensure
the value passed to xrplService.createEscrow stays payment.currency ?? "XRP" so
the log and the createEscrow call use the same currency variable.
In `@src/modules/escrow-payments/escrow-submit-recovery.scheduler.ts`:
- Around line 118-149: In recoverSubmittingEscrow: make the findOneAndUpdate
call safer by adding a guard to the query to require "escrows.status":
"SUBMITTING" (so use escrowPaymentModel.findOneAndUpdate with { _id: paymentId,
"escrows._id": new Types.ObjectId(escrowId), "escrows.status": "SUBMITTING" })
and after the call check if result is null; if null, treat it as already-handled
and return "recovered" instead of dereferencing result; only run the allEscrowed
check and the subsequent escrowPaymentModel.findByIdAndUpdate when result is
non-null. Keep cancelSubmittingEscrowAndRollback unchanged for the XRPL-missing
branch.
- Around line 151-166: The query that matches escrows uses separate dot-notation
conditions which can match different array elements; change each such query to
use a single $elemMatch (e.g. { escrows: { $elemMatch: { _id: new
Types.ObjectId(escrowId), status: "SUBMITTING" } } }) so the match is guaranteed
to be the same subdocument, then keep the update using the positional operator
("escrows.$.status": "CANCELLED") or use arrayFilters if you prefer named
filters; apply this fix in cancelSubmittingEscrowAndRollback
(escrow-submit-recovery.scheduler.ts) and the two analogous
pre-flight/failure-recovery queries in escrow-create.processor.ts so the
intended escrow element is updated atomically.
In `@src/modules/xrpl/xrpl.service.ts`:
- Around line 61-67: The constructor currently throws when xrpl.issuerAddress is
missing, forcing an issuer in XRP-only environments; remove the immediate throw
and instead set this.iouIssuer from config (allow undefined) while keeping
this.iouCurrencyCode defaulting to "RLUSD"; then add lazy validation in
RLUSD-related entry points—e.g., ensureRlusdTrustLine, validateRlusdFunds,
sendIssuedCurrencyPayment—to check that this.iouCurrencyCode indicates an issued
currency and that this.iouIssuer is present before proceeding, and only then
throw a descriptive error if the issuer is required but not configured.
- Around line 651-663: buildEscrowAmount currently forces IOU precision with
amount.toFixed(6), which truncates significant digits; change buildEscrowAmount
to accept and preserve a string or BigNumber for IOU values (e.g., amount:
string | BigNumber | number) and when currency !== "XRP" set value to the
original string representation (no toFixed) so Amount.value retains full
precision, leaving currency as this.iouCurrencyCode and issuer as
this.iouIssuer; then update all callers (and related validateRlusdFunds) to
pass/validate amounts as string or BigNumber and ensure validateRlusdFunds uses
the same representation for consistency between validation and issuance.
- Around line 581-613: validateRlusdFunds에서 parseFloat로 IOU 잔액을 부동소수점으로 변환해 비교하는
대신, buildEscrowAmount가 사용한 정밀도(예: toFixed(6))와 동일하게 정규화해 정수 비교로 바꾸세요:
account_lines에서 받은 line.balance(문자열)를 소수점 6자리로 맞춰 양쪽 값(잔액과 escrows 합계)을 ×10^6
등으로 정규화한 후 BigInt로 변환해 비교하고, 부족할 경우 기존대로 InsufficientXrpBalanceException을 던지도록
변경합니다; 관련 식별자: validateRlusdFunds, buildEscrowAmount, validateEscrowFunds,
iouCurrencyCode, iouIssuer.
In `@test/escrow-rlusd-mock.e2e-spec.ts`:
- Around line 372-375: The test incorrectly throws an XRP-specific exception for
an RLUSD balance check; update the mocked rejection on
mockXrplService.validateRlusdFunds to use an RLUSD-specific or currency-neutral
exception (e.g., RLusdInsufficientBalanceException or
InsufficientBalanceException with currency='RLUSD') and adjust any assertions
that check the error type/message accordingly so the test contract reflects
RLUSD rather than XRP; locate the mock call to
mockXrplService.validateRlusdFunds and replace the
InsufficientXrpBalanceException instantiation with the appropriate RLUSD-aware
exception and matching expected response assertions.
In `@test/escrow-rlusd-testnet.e2e-spec.ts`:
- Around line 63-69: The CANCELLED error message in the escrow check is too
prescriptive (claims XLS-85) and should instead include the actual status and
available context for debugging; update the throw in the CANCELLED branch to
construct a neutral, informative message that references escrowId, item.status
and any available context such as txHashCreate and meta.TransactionResult (or
the whole item object) so CI failures show the real cause rather than asserting
temDISABLED/XLS-85.
---
Outside diff comments:
In `@src/modules/escrow-payments/escrow-payments.controller.ts`:
- Around line 174-225: The getEscrowStatus and cancelEscrowItem endpoints are
missing participant verification because they don't accept the current user; add
a `@CurrentUser`() parameter (e.g., user or userId) to both controller methods
(getEscrowStatus and cancelEscrowItem) and pass that identifier into
this.service.getEscrowStatus(...) and this.service.cancelEscrowItem(...), and
then update the corresponding service method signatures to require and enforce
participant authorization so the service validates the caller is a payment
participant before returning status or performing cancel.
---
Duplicate comments:
In `@src/modules/escrow-payments/escrow-create.processor.ts`:
- Around line 177-222: The pre-flight and rollback findOneAndUpdate queries in
escrow-create.processor.ts (the one that sets preFlighted and the rollback call
inside the XRPL catch) currently match "escrows._id" and "escrows.status" as
separate top-level predicates which can update the wrong array element; change
both queries to use an $elemMatch on "escrows" (e.g. "escrows": { $elemMatch: {
_id: escrow._id, status: "PENDING_ESCROW" } } for pre-flight and similarly
status "SUBMITTING" for the rollback) so the positional $ update targets the
same escrow element; keep the $set targets ("escrows.$.status",
"escrows.$.condition", etc.) and variable names (preFlighted, txHash/sequence)
the same.
---
Nitpick comments:
In `@ci-check.sh`:
- Line 30: Replace the terse A && B || C pattern on the line running "npm run
test:e2e:testnet && npm run test:e2e:rlusd-testnet || fail ..." with an explicit
conditional: run "npm run test:e2e:testnet" and if it succeeds run "npm run
test:e2e:rlusd-testnet", otherwise call fail; implement this using an if ...
then ... else ... fi block so the intent is clear and ShellCheck SC2015 is
avoided while keeping the same behavior.
In `@src/modules/escrow-payments/escrow-create.processor.ts`:
- Around line 25-39: Expose the XRPL engineResult on the thrown
XrplTransactionFailedException in xrpl.service.ts (preserve
meta?.TransactionResult as an engineResult property on the exception instance),
then update isNonRetryable to read that property (const code = (err as
any).engineResult) and perform exact membership checking against
NON_RETRYABLE_CODES (use NON_RETRYABLE_CODES.includes(code)) instead of
string.includes; keep a safe fallback (return false) when engineResult is
undefined.
- Around line 209-212: The code reuses the field escrow.amountXrp for non-XRP
IOUs which embeds a unit in the name and will cause confusion; add new neutral
fields (e.g., escrow.amount and escrow.currency) and update the processor to
read amount from escrow.amount (falling back to escrow.amountXrp) and currency
from escrow.currency (falling back to payment.currency ?? "XRP") so behavior
stays backward-compatible; update DTO/schema creation flows that set
escrow.amountXrp to also set escrow.amount and escrow.currency, and file a
follow-up migration/ADR issue to phase out amountXrp once consumers are
migrated.
In `@src/modules/escrow-payments/escrow-payments-crud.service.ts`:
- Around line 82-92: The query uses this.escrowPaymentModel.find(...).lean() but
the service returns data typed as EscrowPaymentDocument[], which is incorrect
because lean() returns POJOs; update the return type to a read-safe type (e.g.,
Array<Omit<EscrowPaymentDocument, keyof Document>> or a dedicated
EscrowPaymentReadDto) and adjust any callers if they expect full Mongoose
Documents; locate usages around this.escrowPaymentModel.find(...).lean() and the
return statement "data: data as unknown as EscrowPaymentDocument[]" and replace
the cast with the correct DTO/type to preserve type safety.
In `@src/modules/escrow-payments/escrow-payments.service.ts`:
- Around line 106-115: The conditional use of .select(withSeed ? "+wallet.seed"
: "") is unclear because select("") is a no-op; instead split the branch:
compute withSeed = payment.currency === "RLUSD", then for buyer/seller call
this.userModel.findById(id).select("+wallet.seed") only when withSeed is true
and call this.userModel.findById(id) without .select() when false (i.e., avoid
passing an empty string to .select). Update the Promise.all that builds
[buyerUser, sellerUser] to use these two distinct call sites so the intent is
explicit.
- Around line 120-143: The concurrent Promise.all call to ensureRlusdTrustLine
can leave a partial trust line if one call fails; update the RLUSD pre-flight
logic in escrow-payments.service (the block around payment.currency === "RLUSD"
and the ensureRlusdTrustLine calls) to call ensureRlusdTrustLine for buyer and
seller separately (or wrap each call in its own try/catch) so you can log
success/failure distinctly for buyerUser.wallet.address and
sellerUser.wallet.address; on per-call failures record the error and relevant
addresses in structured logs/metrics and only proceed to
xrplService.validateRlusdFunds when both trust-line attempts either succeeded or
have recorded fatal errors, preserving the WalletNotAvailableException behavior
for missing seeds/addresses.
In `@src/modules/xrpl/xrpl.service.ts`:
- Around line 215-253: The review notes that IOU escrow does not require extra
Flags and the current createEscrow implementation is fine, but asks you to add a
pre-authorization check for IOU tokens before submitting EscrowCreate; update
createEscrow to detect when currency !== "XRP" (or when buildEscrowAmount
returns an IOU) and call a new helper (e.g., isPreauthorizedIssuerOrCounterparty
or checkTrustlinePreauth) to verify the sender/recipient are pre-authorized per
the token's issuer settings (using ledger trustline/issuer flags via this.client
or relevant XRPL helper), and if not pre-authorized, throw a clear error or
return a distinct rejection before autofill/sign/submit; keep existing
CancelAfter handling and do not add extra Flags.
In `@test/escrow-rlusd-testnet.e2e-spec.ts`:
- Around line 244-268: The test sets DTO currency to "RLUSD" while XRPL uses
iouCurrencyCode "TST"; clarify this intentional separation by adding an inline
comment near the payment DTO (next to currency: "RLUSD") explaining that
payment.currency is a service-branching key (used in conditions like
payment.currency === "RLUSD") while iouCurrencyCode controls the XRPL
Amount.currency, and also add a TODO note to consider using an abstract DB field
name (e.g., currencyType or IOU currency) for clarity in the payment model and
related code paths (create/escrows handling).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 52f63bb3-6ca7-4031-a2c3-ec1a8698028d
📒 Files selected for processing (23)
ci-check.shpackage.jsonsrc/modules/escrow-payments/__tests__/create.spec.tssrc/modules/escrow-payments/__tests__/helpers.tssrc/modules/escrow-payments/__tests__/query.spec.tssrc/modules/escrow-payments/__tests__/recovery-scheduler.spec.tssrc/modules/escrow-payments/__tests__/xrpl.spec.tssrc/modules/escrow-payments/dto/create-escrow-payment.dto.tssrc/modules/escrow-payments/escrow-create.processor.tssrc/modules/escrow-payments/escrow-payments-crud.service.tssrc/modules/escrow-payments/escrow-payments.controller.tssrc/modules/escrow-payments/escrow-payments.module.tssrc/modules/escrow-payments/escrow-payments.service.tssrc/modules/escrow-payments/escrow-submit-recovery.scheduler.tssrc/modules/escrow-payments/schemas/escrow-payment.schema.tssrc/modules/xrpl/xrpl.service.spec.tssrc/modules/xrpl/xrpl.service.tstest/escrow-mock.e2e-spec.tstest/escrow-rlusd-mock.e2e-spec.tstest/escrow-rlusd-testnet.e2e-spec.tstest/escrow-testnet.e2e-spec.tstest/jest-e2e-rlusd-testnet.jsontest/jest-e2e.json
dot-notation 분리 조건은 서로 다른 배열 원소에서 각각 충족되어도 통과되므로 $elemMatch로 한 원소 내에서 모든 조건을 동시에 만족하도록 수정 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
XRP 잔고 부족과 RLUSD 잔고 부족을 별도 예외로 구분 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ion 반영 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced May 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #15
개요
XRP 에스크로에 이어 RLUSD(IOU 토큰) 에스크로 결제를 지원합니다.
XLS-85(Token-Enabled Escrows) 어멘드먼트 기반으로 동작하며,
기존 XRP 플로우와 동일한 인터페이스(
currency: "RLUSD"필드 추가)로 사용됩니다.서비스 계층도 함께 정리했습니다 —
EscrowPaymentsService가 담당하던 CRUD,XRPL 생성, 복구 로직을 각 책임 레이어로 분리했습니다.
주요 변경사항
RLUSD 지원 (
XrplService)createEscrow/finishEscrow에currency분기 추가 (XRP ↔ RLUSD Amount 형식 분리)ensureRlusdTrustLine— buyer/seller Trust Line 자동 설정validateRlusdFunds— 토큰 잔고 검증enableTrustLineLocking— issuer 계정asfAllowTrustLineLocking플래그 설정sendIssuedCurrencyPayment— issuer → 계정 토큰 전송 (테스트/초기화용)서비스 레이어 리팩토링
EscrowPaymentsService)create / findAll / findByIdEscrowPaymentsCrudServicecreateXrplEscrowEscrowCreateProcessor(직접 실행)recoverSubmittingEscrow/cancelSubmittingEscrowAndRollbackEscrowSubmitRecoveryScheduler(직접 실행)Controller는
CrudService+EscrowPaymentsService둘 다 주입받아 라우팅합니다.테스트
makeCrudServiceTestingModule/makeProcessorTestingModule헬퍼 추가, 각 레이어 독립 테스트escrow-rlusd-mock.e2e-spec.ts신규 추가 (MongoMemoryReplSet + mock XRPL)escrow-rlusd-testnet.e2e-spec.ts신규 추가 (실제 testnet,npm run test:e2e:rlusd-testnet)Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항
테스트
CI
오류 처리