Skip to content

Fix /verify and /settle silently 500ing on transient RPC failure - #4

Merged
jUXTAPOSITION1 merged 1 commit into
mainfrom
fix/verify-settle-rpc-failure-handling
Jul 18, 2026
Merged

Fix /verify and /settle silently 500ing on transient RPC failure#4
jUXTAPOSITION1 merged 1 commit into
mainfrom
fix/verify-settle-rpc-failure-handling

Conversation

@jUXTAPOSITION1

@jUXTAPOSITION1 jUXTAPOSITION1 commented Jul 18, 2026

Copy link
Copy Markdown
Owner

Root cause

verifyPayment()'s on-chain reads (authorizationState/balanceOf) and its risk-scan step (scanAddress(), which shares the same RPC client) had no error handling at all. Any transient RPC trouble — a rate limit, a timeout, a non-archive node rejecting a historical query, or no RPC URL configured — threw straight out of verifyPayment(), past the route handler (Express 5 auto-forwards a rejected async handler to errorMiddleware), and became an unrecorded 500.

Since /settle re-runs verifyPayment() internally before ever broadcasting (prepareSettlementverifyPayment), both endpoints shared this exact fragility.

Because the throw happens before recordVerification/recordSettlement or verifyOutcomesTotal/settleOutcomesTotal are ever reached, the failure was invisible in VAPOR's own /stats too — from a resource server's perspective this is indistinguishable from "the facilitator is down," so it silently falls back to whatever facilitator is next in its chain, and VAPOR never shows a trace the attempt happened at all.

Fix

  • Wrap the on-chain state read (getPublicClient + authorizationState/balanceOf) in a try/catch that returns a clean, recorded {isValid: false, invalidReason: "temporarily unable to verify on-chain state (RPC error)"} instead of throwing.
  • Wrap the risk-scan step so a scan failure degrades to {isValid: true, payer} with no riskAssessment — which is what verifyPayment's own docstring already said should happen ("a risk-scan failure still returns isValid: true... VAPOR informs, it doesn't unilaterally decide") but the code didn't actually deliver on for an infra failure, only for a completed-but-negative scan.

Tests

Adds tests/unit/verification.service.test.ts (new — no prior unit test covered verifyPayment's own branching at all), covering both new degrade paths plus the existing success/nonce-reuse/insufficient-balance/policy-denial behavior so this can't silently regress again.

npx tsc --noEmit clean. New test file: 7/7 passing. Pre-existing failures in tests/integration/{app,api-key-scoping,route-scoped-middleware}.test.ts (all /stats-related, DB/Prisma setup issue) confirmed present identically on main without this change — unrelated to this fix.


Generated by Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Payment verification now handles temporary blockchain connection failures gracefully.
    • Risk-scanning service interruptions no longer cause verification requests to fail unexpectedly.
    • Authorization, balance, and risk policies continue to be enforced when required data is available.
  • Tests

    • Added coverage for successful verification, policy violations, insufficient balances, reused authorizations, and service failures.

Root cause of "VAPOR never receives real transactions as a facilitator":
verifyPayment()'s on-chain reads (authorizationState/balanceOf) and its
risk-scan step (scanAddress(), which shares the same RPC client) had no
error handling at all. Any transient RPC trouble — a rate limit, a
timeout, a non-archive node rejecting a historical query, or simply no
RPC URL configured — threw straight out of verifyPayment(), past the
route handler (Express 5 auto-forwards a rejected async handler to
errorMiddleware), and became an unrecorded 500.

Since /settle re-runs verifyPayment() internally before ever broadcasting
(prepareSettlement -> verifyPayment), both endpoints shared this exact
fragility. And because the throw happens before recordVerification/
recordSettlement or verifyOutcomesTotal/settleOutcomesTotal are ever
reached, the failure was invisible in VAPOR's own /stats too — from a
resource server's perspective this is indistinguishable from "the
facilitator is down," so it falls back to whatever facilitator is next
in its own chain, and VAPOR never shows a trace of the attempt.

Fix: wrap the on-chain state read in a try/catch that returns a clean,
recorded `{isValid: false, invalidReason: "temporarily unable to verify
on-chain state (RPC error)"}` instead of throwing, and wrap the risk-scan
step so a scan failure degrades to `{isValid: true, payer}` with no
riskAssessment — which is what verifyPayment's own docstring already said
should happen ("a risk-scan failure still returns isValid: true... VAPOR
informs, it doesn't unilaterally decide") but the code didn't actually
deliver on for an infra failure, only for a completed-but-negative scan.

Adds tests/unit/verification.service.test.ts (new — no prior unit test
covered verifyPayment's own branching at all) covering both degrade paths
plus the existing success/nonce-reuse/insufficient-balance/policy-denial
behavior, so this can't silently regress again.
@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 26e68b2b-7409-46c4-946a-8a8692945c0f

📥 Commits

Reviewing files that changed from the base of the PR and between 69d27f7 and 419d2e3.

📒 Files selected for processing (2)
  • src/core/verification/verification.service.ts
  • tests/unit/verification.service.test.ts

📝 Walkthrough

Walkthrough

verifyPayment now handles on-chain RPC and risk-scanning failures without propagating exceptions. New unit tests cover successful verification, invalid authorization states, insufficient balances, policy rejection, and degraded infrastructure paths.

Changes

Verification resilience

Layer / File(s) Summary
Guard verification control flow
src/core/verification/verification.service.ts
On-chain read failures return a deterministic invalid result, while risk-scanning failures return a valid result without riskAssessment.
Validate verification outcomes
tests/unit/verification.service.test.ts
Mocks verification dependencies and tests success, RPC failures, risk-scan failures, policy rejection, nonce replay, and insufficient balance handling.

Estimated code review effort: 3 (Moderate) | ~20 minutes

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/verify-settle-rpc-failure-handling

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@jUXTAPOSITION1
jUXTAPOSITION1 merged commit b9c5122 into main Jul 18, 2026
1 of 2 checks passed
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.

2 participants