Skip to content

Fall back to the collection account only on infrastructure failures - #4582

Merged
TaprootFreak merged 2 commits into
developfrom
fix/collection-fallback-only-on-infra-failure
Aug 1, 2026
Merged

Fall back to the collection account only on infrastructure failures#4582
TaprootFreak merged 2 commits into
developfrom
fix/collection-fallback-only-on-infra-failure

Conversation

@TaprootFreak

Copy link
Copy Markdown
Collaborator

Follow-up to #4580. The collection-account fallback there caught issuance failures with .catch(() => null), which is too broad.

Problem

Personal Frick IBAN issuance re-reads and merge-resolves the account owner under its own lock and raises KYC_REQUIRED authoritatively against that fresh owner. The blanket .catch(() => null) swallowed that rejection, and the fallback then re-checked only the request's own selector.userData KYC snapshot. When that snapshot is stale relative to the locked check — an in-flight KYC downgrade, or a merge whose target owner sits below level 50 — a customer the authoritative check had just rejected could still be handed a usable collection account with a valid reference. The same blanket catch also swallowed CURRENCY_UNSUPPORTED and other business rejections.

Change

Tell the two failure kinds apart. An infrastructure failure (the provider is down) is what the fallback exists for and still degrades to null. A business rejection is a BadRequestException — above all KYC_REQUIRED — and is re-thrown so it reaches the caller. Applies to both the explicit and the implicit Frick issuance paths, via a small shared helper.

Verification

A new test covers the discrepancy case: issuance rejects with KYC_REQUIRED while the request snapshot is level 50 → the request rejects with KYC_REQUIRED and the fallback (getBank) is never reached. tsc, ESLint, Prettier clean; buy.service.spec.ts 49 tests green locally.

The collection-account fallback swallowed every issuance error with `.catch(() => null)`, including the authoritative KYC rejection that issuance raises under its own lock against the freshly loaded, merge-resolved owner. The fallback then re-checked only the request's own, possibly stale KYC-50 snapshot, so a customer the locked check had just rejected could still receive a usable collection account with a reference. Only degrade to the fallback on an infrastructure failure; re-throw business rejections (BadRequestException, above all KYC_REQUIRED) so they reach the caller. Applies to both the explicit and the implicit Frick issuance paths. A test covers the discrepancy case.
Cover the explicit personal-IBAN path with the same business-rejection test the implicit path already has: a KYC_REQUIRED raised by issuance for a level-50 request must reach the caller and never reach the collection-account fallback. Move the infrastructure-vs-business helper above the fallback method so each comment sits directly over the method it describes.
@TaprootFreak

Copy link
Copy Markdown
Collaborator Author

Two review passes to reach zero findings.

  • Pass 1 confirmed the classification is correct (infrastructure failures degrade to the collection account, business rejections propagate) and proved by counter-test that the guard now closes the gap. It flagged that only the implicit path had a test for a real business rejection, and a comment sat above the wrong method.
  • Pass 2 (fixes): added the same business-rejection test on the explicit path and moved the helper so each comment sits above its method. Clean on both tracks.

All checks green; buy.service.spec 50 tests green.

@TaprootFreak
TaprootFreak marked this pull request as ready for review August 1, 2026 16:45
@TaprootFreak
TaprootFreak merged commit fd48f8a into develop Aug 1, 2026
12 checks passed
@TaprootFreak
TaprootFreak deleted the fix/collection-fallback-only-on-infra-failure branch August 1, 2026 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant