Skip to content

fix(aml): distinguish Scorechain provider failure from real high-risk hit#4207

Merged
TaprootFreak merged 3 commits into
developfrom
fix/aml-scorechain-unavailable-error
Jul 15, 2026
Merged

fix(aml): distinguish Scorechain provider failure from real high-risk hit#4207
TaprootFreak merged 3 commits into
developfrom
fix/aml-scorechain-unavailable-error

Conversation

@TaprootFreak

Copy link
Copy Markdown
Collaborator

Closes #4205

Problem

screenScorechain() collapsed two very different outcomes into the same boolean true:

  • a real high-risk screening hit, and
  • an infrastructure failure (provider 5xx, timeout, monthly quota reached, misconfiguration, any unexpected throw).

Both surfaced as AmlError.SCORECHAIN_HIGH_RISK. Compliance therefore saw a "Scorechain hit" for transactions that were never screened at all — no scorechain_screening row, no compliance PDF. Observed in prod: a buy-crypto stuck on Pending / ManualCheck / comment ScorechainHighRisk after a Scorechain POST /scoringAnalysis failed: status code 500; the manual re-trigger later returned NoCoverage, i.e. no hit.

Change

The screening gate no longer speaks in booleans. New enum ScorechainOutcome (Pass / HighRisk / Unavailable) replaces the scorechainHighRisk boolean end to end (preparation service → entity → AmlHelperService), which also resolves the "magic boolean" anti-pattern from CONTRIBUTING.md.

Case Internal AmlError / comment Settlement
Successful screen, score high risk ScorechainHighRisk (unchanged) PENDING
Screening impossible (5xx / timeout / quota / misconfig / unexpected throw) ScorechainUnavailable (new) PENDING
  • Still fail-closed: an unavailable provider never yields PASS, never settles.
  • The catch path can no longer emit SCORECHAIN_HIGH_RISK — structurally, not by convention.
  • amlReason stays the generic MANUAL_CHECK for both, so the customer-facing story is unchanged (no tipping-off).
  • Mapping via an exhaustive mapped type (ScorechainOutcomeError), so a future outcome cannot be forgotten.
  • SCORECHAIN_UNAVAILABLE is added to neither DelayResultError nor the manual-pass white/blacklist — exactly like SCORECHAIN_HIGH_RISK today. No cross-repo sync of ManualPassWhitelistErrors is required.
  • No migration: comment is a free-text field.

Out of scope, as per the issue: fail-open behaviour, customer-facing copy, and the re-trigger endpoint (POST /buyCrypto/:id/scorechain) are untouched.

Tests

Both AML paths (buy-crypto and buy-fiat) are covered at all three layers:

  • Preparation services: provider throws → Unavailable, explicitly not HighRisk; isHighRisk throwing on a misconfigured risk threshold → Unavailable; real high-risk screen → HighRisk (+ compliance PDF on a fresh screen); disabled / no API key / unsupported chain → Pass.
  • AmlHelperService: Unavailable → comment ScorechainUnavailable, PENDING / ManualCheck; HighRiskScorechainHighRisk as before.
  • Entities: the gate still only screens when the tx would otherwise PASS, and now flips PASSPENDING on Unavailable as well.

Docs

docs/specs/scorechain-integration.md updated: the fail-closed decision now states that a provider failure classifies as ScorechainUnavailable, and that a compliance PDF is only guaranteed for a successful fresh screen.

… hit

Screening failures (provider 5xx, timeout, quota, misconfiguration) were reduced to the
same boolean as a real high-risk hit and surfaced as AmlError.SCORECHAIN_HIGH_RISK, so
compliance saw Scorechain hits for transactions that were never screened (no screening
row, no PDF).

Replace the boolean channel with a ScorechainOutcome enum (Pass/HighRisk/Unavailable) and
add AmlError.SCORECHAIN_UNAVAILABLE. Both outcomes stay fail-closed (CRUCIAL -> PENDING /
generic ManualCheck, no tipping-off), but the internal classification is now distinct.

Closes #4205
…ilure paths

A missing risk threshold makes isHighRisk throw only after the screening row and the
compliance PDF have been persisted, so the blanket claim that neither exists was wrong
for that path. Also drop a tautological assertion from both preparation specs.
… spec

A soft wrap right after the slash rendered as "provider/ infrastructure".
@TaprootFreak

Copy link
Copy Markdown
Collaborator Author

Took 3 review passes (conformance / logic / adversarial, each finding independently verified against the code) to reach zero findings.

Pass 1 surfaced two real defects, both fixed in 7b0bfa7:

  • The new comment and the spec claimed as an invariant that a ScorechainUnavailable tx has no screening row and no compliance PDF. That holds for transport/quota errors, but not for a missing risk threshold: isHighRisk throws only after executeScreening has persisted the row and createScreeningReport has written the PDF. Compliance would have triaged against a false invariant, so the rationale is now scoped to the actual failure paths (the runtime classification was and stays correct).
  • A tautological assertion (resolves.not.toBe(HIGH_RISK) right after resolves.toBe(UNAVAILABLE)) in both preparation specs, which re-invoked the method under test without asserting anything. The HighRisk-vs-Unavailable distinction is covered for real in aml-helper.service.spec.ts.

Pass 2 found a markdown soft wrap that rendered as "provider/ infrastructure" (fixed in 77fe5b1). Pass 3: zero findings.

Verified locally on the final commit: format:check, lint, type-check and the full suite (2974 tests passing).

@TaprootFreak TaprootFreak marked this pull request as ready for review July 14, 2026 16:19
@TaprootFreak TaprootFreak merged commit 21db1a5 into develop Jul 15, 2026
7 checks passed
@TaprootFreak TaprootFreak deleted the fix/aml-scorechain-unavailable-error branch July 15, 2026 07:12
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.

Scorechain: distinguish provider failure from real high-risk hit (do not map 5xx to ScorechainHighRisk)

1 participant