Skip to content

feat(payments): ENG-573 Phase 0 send guard — attempt budget, amount sanity, daily-limit cap - #506

Open
forge0x wants to merge 12 commits into
mainfrom
feat/eng-573-send-guards-phase0
Open

feat(payments): ENG-573 Phase 0 send guard — attempt budget, amount sanity, daily-limit cap#506
forge0x wants to merge 12 commits into
mainfrom
feat/eng-573-send-guards-phase0

Conversation

@forge0x

@forge0x forge0x commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Linear: ENG-573 — Phase 0.

TL;DR for whoever deploys this

This ships observing, not enforcing. sendGuard.mode defaults to log-only: all three checks run, every would-be rejection posts a transfer / would-reject ops event, and the send goes through. Nothing is refused until someone sets the flag.

sendGuard:
  mode: log-only # default — measure first
  # mode: enforce  # rejections become real
  # mode: off      # returns before Redis / price / decode — see "what `off` misses" below

Rollback is the same flag: back to log-only, or off if the guard itself is the outage. It is not live-reloaded — yamlConfig is read once from --configPath at process start, so a rollback is helm upgrade plus a pod roll (kubectl rollout restart deploy/<api>), not an instant flip. Budget for a roll, not for a switch. Anything unrecognised degrades to log-only — the failure mode of this switch must be "does not block", never "every send is refused". Read docs/send-guard.md before flipping it: it has the rollout procedure, what to count in the ops feed, and the config that goes with it.

Why

Flash has no internal ledger, so Galoy's AccountLimitsChecker reads zero volume for every account and never rejects. The live send resolvers call IBEX with nothing on the Flash side checking the amount. On 2026-09-03 a $999,999,999.99 intraledger request reached IBEX untouched (TEST cluster, demo_accountrewards); prod runs the same path.

What

Payments.authorizeSend (src/app/payments/authorize-send.ts), reached by every user-initiated send mutation:

  1. Attempt budget — two Redis buckets keyed on the account: 10/min burst, 200/day. Consumed first, so a rejected attempt still costs a point and probing the amount space is bounded by the caller's own budget.
  2. Amount sanity — positive and finite. USD/USDT cents may be fractional (USDT settles in micros); sats must be whole.
  3. Daily limit as the per-transaction capamount <= dailyLimit(level), per the ENG-573 decision. Intraledger sends use the intraLedger limit; lightning / LNURL / on-chain use the withdrawal limit. Phase 1 replaces this with the remaining allowance.

Wired into all eight live send mutations: intraLedgerPaymentSend, intraLedgerUsdPaymentSend, lnInvoicePaymentSend, lnNoAmountInvoicePaymentSend, lnNoAmountUsdInvoicePaymentSend, lnurlPaymentSend, onChainUsdPaymentSend, onChainPaymentSendAll.

It runs inside the idempotency wrapper, not ahead of it

On the six rails that accept an idempotencyKey, the guard is handed to withPaymentIdempotency as its authorize hook — inside the lock, after the in-lock cache re-check, immediately before the send. Only the path that is about to pay is judged.

That ordering is load-bearing, not tidiness. Ahead of the wrapper, a client whose $50 send timed out and retried spent a burst point on every retry before ever reaching the replay: past 10/min it got {status:"failed"} + "Too many payment attempts" for a payment that had already settled — and a client that reads "failed" as "retry with a fresh key" then double-pays, which is precisely the ENG-530 class the wrapper exists to prevent. The two sats rails additionally re-priced on each retry, so a settled send near the cap could come back as "Cannot transfer more than $X" after a mid-price tick. A replay must cost nothing and must not be re-judged.

off covers every check the guard runs — and one thing it does not

lnInvoicePaymentSend had to start decoding the bolt11, because the guard needs the amount and the amount is inside the invoice. decodeInvoice refuses anything invoices.parsePaymentRequest can't parse and any invoice with no payment secret — a rejection class that rail never had, since it used to hand the raw bolt11 straight to IBEX. That gate therefore lives inside the same authorize hook and answers to the same switch: on off the resolver doesn't decode at all. If the gate ever refuses an invoice IBEX would have paid, the flag is the remedy, not a deploy.

But off is not a full revert of ENG-573. Reading an account with no level field as level 0 lives in effectiveAccountLevel, applied inside getAccountLimits at the config layer — deliberately, so the guard, Account.limits and remainingLimit cannot disagree about those ~300 accounts. Galoy's own AccountLimitsChecker reads the same function, so it caps an unleveled account at the level-0 limits in every mode, off included, with no ops event and no sendGuard.* span attribute to explain it. It bites only the BTC no-amount lightning pair, the sole user-facing rails still routed through @app/payments; the USD and amount-bearing resolvers pay IBEX directly and never reach the checker, so on those off really is pre-ENG-573. Read What off does not cover before concluding the flag has undone everything.

Fail-closed is bounded

No limit configured for the level, no BTC→USD price for a sats amount, or a rate-limit store fault all reject the send with SendLimitsUnavailableError when enforcing — infrastructure faults, not user error. They are recorded as span exceptions on every occurrence (Critical when enforcing, Warn in log-only): that is the signal to alert on.

Their ops embeds are coalesced to one per minute, carrying muted: N so the feed stays countable. A Redis fault isn't one noisy caller — it makes every send in flight report at the same instant, into a shared 50-deep FIFO that drains sequentially and drops its oldest entries. Unbounded, it would bury the verification / cashout / deposit feed during exactly the incident this switch exists to survive. rate-limited and undecodable-invoice are coalesced the same way and for the same reason — both are unbounded by anything but the caller's own retry loop, and a check with no observable output cannot be read during a log-only rollout at all. over-daily-limit and invalid-amount are never coalesced — they are the per-account facts the log-only rollout exists to read, and each caller's attempt budget already bounds them.

Decisions baked in

These decide what log-only will report, and what enforce would refuse. The point of shipping in log-only is that the second column stops being an assumption before anything is blocked.

Decision Value Why
Accounts with no level field treated as level 0 ($125/tx) ~300 prod accounts lack the field (174 with usernames); mongoose has no default. "An unleveled account is an unverified one" is an assumption — this is the sharpest thing to check in the log-only sample before enforcing.
Level-3 (Business) daily limits same as level 2 ($50k intraledger / $50k withdrawal) 25 prod accounts are L3. Placeholder until the ladder is decided (ENG-573 open question).
Levels 0–2 Galoy schema defaults, except level 1 (L0 $125, L1 $1k, L2 $50k) Started on the schema defaults. Galoy ships L1 split — $1k withdrawal against $2k intraledger — and that split was the only place the guard's rail-vs-destination approximation could change an outcome. Settled 2026-09-08 to a single $1,000, i.e. downward, rather than by raising the external cap to $2k. Every level now carries equal withdrawal and intraLedger limits.
Attempt budget 10/min, 200/day rateLimits.paymentSendAttempt / paymentSendDailyAttempt. blockDuration must be ≥ duration — rate-limiter-flexible rewrites the key TTL to blockDuration on the first breach, so a shorter block hands back a fresh budget early.

Deploy notes

  • No chart/values change required, and none needed to stay safe — the default is log-only.
  • The new rateLimits keys are property-level defaults, not required keys, because prod overrides the whole rateLimits block in deployments/tf-modules/flash/flash-values.tmpl.yaml; a required key there would fail config validation at boot. A unit test pins this.
  • accountLimits levels 0–3 are all required. A values file that overrides accountLimits partially now fails at boot instead of resolving a missing level to NaN and silently blocking that level's sends.
  • Expect 🔁 Transfer — Would Reject embeds in #flash-activity after rollout. In log-only that is the guard measuring, not blocking. Count them by step before flipping — the procedure is in docs/send-guard.md.

Tests

  • authorize-send.spec.ts — all three modes; budget order; store fault reported as an infra fault, not as a rate limit; invalid amounts (0, negative, NaN, ∞, fractional sats, bad strings); exact-limit boundary per kind; the wall-of-nines regression at every level; missing level → 0; sats→USD via mid price incl. price-unavailable; 50 consecutive store faults produce one ops embed and 50 span exceptions, and the muted count lands on the next embed that posts.
  • idempotency.spec.tsauthorize is skipped on a replay, on a lock-busy return and on a fingerprint mismatch; runs before execute on the paying path; a rejection leaves the key retryable.
  • send-intraledger.spec.ts — end to end through the real wrapper: a replayed key runs the guard once, not twice (the burst-budget regression); a rejection sends nothing and the key still works afterwards.
  • send-lightning-ops-events.spec.ts — a rejection on the no-amount BTC rail validates nothing, sends nothing, posts no ops event.
  • Resolver specs for all eight mutations — the guard's args, that it is handed down to the send function rather than run ahead of it, and that a rejection returns a failed payload.
  • ln-invoice-payment-send.spec.tsoff still pays an undecodable invoice and a no-amount invoice and never decodes at all; plus a real mainnet bolt11 through the real decodeInvoice, proving the new gate passes an ordinary amount-bearing invoice to IBEX with its true sats amount.
  • send-limits.spec.ts — a behavioural RateLimiterMemory case proving the daily counter survives a breach for the whole window; partial accountLimits override without level 3 fails Ajv validation; sendGuard defaults and enum.
  • error-map.spec.ts — the three new mappings, and that SendLimitsUnavailableError keeps its generic wording instead of leaking internal limit detail.

Full unit suite: 255 suites, 2906 tests (2903 passed, 3 pre-existing skips). yarn tsc-check, yarn tsc-check-noimplicitany, yarn eslint-check, yarn build, yarn check-yaml and yarn madge-check all clean.

Follow-ups (tracked in ENG-573)

  • Flip to enforce once a week of would-reject data is in — a separate, values-only change.
  • Phase 0b: ops-event env label distinguishes clusters; TEST events to their own channel.
  • Phase 1: Redis rolling-24h allowance with reservations; cap becomes amount <= remaining.
  • Phase 2: truthful Account.limits.remainingLimit; IBEX reconciliation job.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV

…anity, daily-limit cap

Flash has no internal ledger, so Galoy's AccountLimitsChecker reads zero
volume for every account and never rejects; the live send resolvers call
IBEX with nothing on the Flash side checking the amount. On 2026-09-03 a
$999,999,999.99 intraledger request reached IBEX untouched (TEST cluster,
demo_account) — the same code path prod runs.

Add `Payments.authorizeSend`, called by every user-initiated send mutation
before anything reaches IBEX:

1. per-account attempt budget — 10/min and 200/day (configurable); every
   attempt costs a point, rejected ones included, so probing the amount
   space is bounded by the caller's own budget
2. amount sanity — positive and finite; USD/USDT cents may be fractional
   (USDT settles in micros), sats must be whole
3. `amount <= dailyLimit(level)` — per the ENG-573 decision the daily limit
   doubles as the per-transaction cap until the Phase 1 allowance counter
   exists; intraledger sends use the intraLedger limit, everything leaving
   Flash uses the withdrawal limit

Fails closed: no limit configured for the level, or no BTC→USD price for a
sats amount, rejects the send. Every rejection posts a `transfer / rejected`
ops event with account, level, amount and reason.

Config:
- accountLimits gains level 3. Placeholder = the level-2 numbers: 25 prod
  accounts are L3 and would otherwise resolve to NaN and be blocked.
- rateLimits gains paymentSendAttempt / paymentSendDailyAttempt as
  property-level defaults rather than required keys, because prod overrides
  the whole rateLimits block (deployments flash-values.tmpl.yaml).
- accounts with no `level` field (~300 in prod, 174 with usernames) are
  treated as level 0.

lnInvoicePaymentSend now decodes the bolt11 (it needs the amount) and
rejects no-amount invoices up front instead of letting IBEX fail them.

Tests: guard unit spec, config spec including the partial-override Ajv
path, and wiring specs for all seven live send resolvers proving a
rejection returns a failed payload before anything downstream is touched.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
@linear

linear Bot commented Sep 5, 2026

Copy link
Copy Markdown

ENG-573

bobodread876 and others added 11 commits September 5, 2026 10:53
…lt, ordering

Addresses the review on PR #506. The guard now ships observing rather than
enforcing, is flippable without a deploy, and no longer sits ahead of the
idempotency replay.

sendGuard.mode — off / log-only / enforce (default log-only)
  These caps are the first Flash-side amount limits that have ever rejected
  anything and nobody has measured what fraction of real traffic they refuse;
  the ~300 prod accounts with no `level` field (174 with usernames) would have
  been capped at $125/transaction on deploy day on an assumption, not a
  measurement. log-only runs all three checks and posts a
  `transfer / would-reject` ops event, but authorises the send — the assumption
  becomes data instead of an incident. `off` returns before any Redis or price
  call, so the guard cannot itself become the outage: previously a Redis fault
  or a price-pod outage past the 10-minute cache rejected 100% of sends with no
  remedy short of image → chart → OCI → terraform. Property-level AND
  block-level defaults, and getSendGuardMode() coerces anything unrecognised
  back to log-only: this switch must fail to "does not block", never to "every
  send is refused". Rollout + rollback in docs/send-guard.md.

Rate-limit store faults are no longer reported as rate limits
  consumeLimiter returns UnknownRateLimitServiceError on a Redis fault. That is
  an infrastructure failure, not the caller being noisy: it now becomes a
  SendLimitsUnavailableError ("Sending is temporarily unavailable"), not "too
  many attempts" on a user's first send of the day. Every limits-unavailable
  rejection is recorded as a span exception (Critical when enforcing, Warn in
  log-only) so a Redis or price outage surfaces as "the guard is blocking
  sends" rather than a wave of unexplained payment failures.

No ops event for rate-limited rejections
  A blocked caller still consumes, still rejects, and would still post — with
  no ceiling. The ops queue is 50 deep and drops its oldest events, so one
  client in a retry loop (or the multi-account probing this guard exists to
  stop) would bury the verification / cashout / deposit feed. The limiter's
  Redis counters are the record for that reason.

Reason travels in `step`, not `meta`
  buildEmbed runs every meta value through truncateId (12 chars), so the feed
  read `over-dai…` / `limits-u…`. `step` is the one field it leaves alone.

accountLimits level 3 is `required`
  The block carries one default, applied only when the key is absent entirely.
  The moment a values file sets `accountLimits` — the edit this work schedules —
  a missing level 3 resolved to NaN and every Business account silently lost
  the ability to send. Now it fails at boot like levels 0-2.

paymentSendDailyAttempt blockDuration 3600 → 86400
  rate-limiter-flexible rewrites the key's TTL to blockDuration on the first
  breach (RateLimiterStoreAbstract._afterConsume → _block), so a 3600s block on
  an 86400s window threw the daily counter away after an hour and granted a
  fresh 200 points: 200/hour, ~4,800/day, not the 200/day claimed.

Guard runs inside withPaymentIdempotency, not ahead of it
  It is now the wrapper's `authorize` hook, invoked after the in-lock cache
  re-check and immediately before execute — only on the path that will actually
  pay. Ahead of the wrapper, a client retrying a timed-out send burned burst
  budget on every retry and got "Too many payment attempts" instead of the
  cached success of a payment that had already moved money; a sats send near
  the cap could also flip to "Cannot transfer more than $X" because the mid
  price ticked. In ln-noamount-usd the wallet-amount conversion moved inside
  execute() too, so a replay does none of it.

SendRejectionReasons is now the single source of the reason strings — the
union type derives from it, and the guard and specs reference it instead of
retyping literals.

Tests
  - error-map.spec.ts: the three new mappings (mapError's default branch is
    assertUnreachable, so an unmapped guard error is a 500, not a payload), and
    that SendLimitsUnavailableError keeps its generic wording instead of
    leaking "no daily send limit configured for level 3".
  - send-limits.spec.ts: a behavioural RateLimiterMemory case proving the daily
    counter survives a breach for the whole window (the old spec pinned the
    literals, not the claim); accountLimits partial override without level 3
    fails Ajv validation; sendGuard defaults and enum.
  - idempotency.spec.ts: authorize is skipped on a replay, on a lock-busy
    return and on a fingerprint mismatch; runs before execute on the paying
    path; a rejection leaves the key retryable.
  - authorize-send.spec.ts: all three modes, no ops event for rate-limited
    however many retries, store fault as an infra fault with the alert, reason
    in `step`.
  - resolver specs: the idempotency passthrough now mirrors the real wrapper's
    authorize-then-execute contract, plus a case per resolver pinning that the
    guard is handed to the wrapper rather than run ahead of it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
…very keyed rail

Addresses the second review on PR #506.

Guard runs inside withPaymentIdempotency on ALL six keyed rails
  Round 1 landed the ordering fix on three of the six rails that accept an
  idempotencyKey. intraLedgerPaymentSend, intraLedgerUsdPaymentSend and
  lnNoAmountInvoicePaymentSend still awaited authorizeSend in the resolver,
  ahead of Payments.*, which is where withPaymentIdempotency actually lives —
  so the round-1 scenario was still live on the highest-volume rails: a $50
  send with a key, IBEX slow, client retries; every retry burned a burst point
  before reaching the replay, and past 10/min it got {status:"failed"} + "Too
  many payment attempts" for a payment that had already settled. A client that
  reads "failed" as "retry with a fresh key" then double-pays — the ENG-530
  class the wrapper exists to prevent. The two sats rails additionally
  re-priced on retry, so a settled send near the cap could come back "Cannot
  transfer more than $X".

  PaymentSendArgs' siblings gain an optional `authorize` (SendGuardHook);
  send-intraledger and send-lightning thread it into their
  withPaymentIdempotency calls; the three resolvers hand
  `authorize: () => authorizeSend({...})` down instead of awaiting it. On the
  BTC intraledger rail the guard consequently runs after the recipient
  username lookup and routing rather than before them — the ordering the
  wrapper requires.

limits-unavailable ops events are coalesced, not unbounded
  The round-1 ceiling was fitted to `rate-limited`, the one reason whose caller
  is already bounded, and left off the one reason that fires on 100% of sends
  at once. A Redis fault or a price-pod outage past the 10-minute cache made
  every send in flight post an embed into the shared 50-deep FIFO that drains
  sequentially and drops its oldest entries — burying the verification /
  cashout / deposit feed during exactly the incident the mode switch exists to
  survive, and making the log-only sample the rollout depends on silently
  lossy. Now at most one embed per minute, carrying `muted: N` (how many were
  coalesced away) so the feed stays countable; recordExceptionInCurrentSpan
  still fires on every occurrence and remains the durable signal ops alerts on.
  over-daily-limit and invalid-amount are deliberately NOT coalesced: they are
  the per-account facts the rollout exists to read, and each caller's own
  attempt budget already bounds them.

mode: off now actually rolls back lnInvoicePaymentSend
  The decodeInvoice gate and the no-amount rejection sat outside
  withPaymentIdempotency and outside getSendGuardMode(), so they applied in all
  three modes. decodeInvoice refuses any bolt11 invoices.parsePaymentRequest
  cannot parse and any invoice with no payment secret — a rejection class this
  rail never had; it previously handed the raw bolt11 straight to IBEX. If that
  gate refused an invoice IBEX would have paid, flipping to `off` did not
  restore service; it took a deploy. Both now live inside the same `authorize`
  hook behind `if (getSendGuardMode() === "off") return true`, so `off` does
  not even decode. docs/send-guard.md's `off` row is corrected to say so.

Tests
  - send-intraledger.spec.ts: a replayed key runs the guard once, not twice
    (the burst-budget regression, end to end through the real wrapper); a guard
    rejection sends nothing and leaves the key retryable.
  - send-lightning-ops-events.spec.ts: a guard rejection on the no-amount BTC
    rail validates nothing, sends nothing and posts no ops event.
  - resolver specs: the three inverted — they now pin that the guard is handed
    DOWN to the send function, that a replay never consults it, and that a
    rejection still returns a failed payload.
  - authorize-send.spec.ts: 50 consecutive store faults produce one ops event
    and 50 span exceptions; the muted count lands on the next event that posts;
    over-daily-limit is not coalesced.
  - ln-invoice-payment-send.spec.ts: `off` still pays an undecodable invoice, a
    no-amount invoice, and never decodes at all; plus the first case in this
    suite to run a REAL bolt11 through the real decodeInvoice, proving the new
    gate passes an ordinary amount-bearing invoice to IBEX with its real sats.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
…ean log-only

Round-3 review findings on #506.

rate-limited was silenced, not coalesced — so one of the three checks had no
observable output at all. That is unreadable during the very rollout the mode
switch exists for: an operator counts a week of would-reject embeds, sees no
rate-limit signal by construction, flips to enforce, and hands a 30-payment
payout batch twenty TooManyRequestErrors. It is coalesced now, one embed per
minute carrying `muted: N`, which bounds the 50-deep ops queue exactly as it
does for limits-unavailable and still yields a count.

The muted count was dropped on the most common incident shape. A 40-second
Redis blip muted 499 rejections, Redis recovered, no further event of that
reason arrived inside the staleness cutoff, and ops read a feed saying one
send was affected. A count is now cleared only by being delivered, and
carries `mutedAgeS` so a late delivery reads as an older incident rather
than as something that just happened.

lnInvoicePaymentSend's bolt11 decode gate ignored the mode: it returned its
error in log-only too, so the one rail whose guard introduced a brand-new
rejection class (decodeInvoice refuses anything parsePaymentRequest cannot
parse, and any invoice with no payment secret) was enforcing while the docs
and the PR said nothing was being enforced — and the would-reject sample
contained no trace of the invoices it turned away. It now routes through
`gateSend`, which reports `undecodable-invoice` and then defers to the mode:
in log-only the raw bolt11 reaches IBEX exactly as it did pre-ENG-573.

Test isolation: the coalescing windows are module state and a muted count now
survives indefinitely, so the spec resets them explicitly instead of leaning
on the staleness cutoff — which was the bug it was hiding.

docs/send-guard.md updated on all three points.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
…from tracing, require the hook

Six review findings on the Phase 0 send guard.

authorize-send.ts:142 — `undecodable-invoice` was neither coalesced nor
budgeted, so any authenticated caller could post one Discord embed per HTTP
request into the shared 50-deep FIFO. The LnPaymentRequest scalar is
/^ln[a-z0-9]+$/i, so `paymentRequest: "lnx"` reaches decodeInvoice and fails it,
and a retry loop starved the verification / cashout / deposit feed while
truncating the very would-reject census the rollout depends on. The code's own
justification for leaving reasons uncoalesced — "each caller's attempt budget
already bounds them" — was false for this one, because the decode ran ahead of
the budget. Added it to COALESCED_REASONS, and extracted the budget step so
`gateSend` charges it: exactly one charge per request either way, since a
request the gate handles never reaches `authorizeSend`. An exhausted budget is
now reported and returned as `rate-limited`.

authorize-send.ts:206 — `effectiveLevel` invented a level-resolution rule no
other consumer of `getAccountLimits` shared, so for the ~300 unleveled prod
accounts the guard refused at $125 while `Account.limits` / `remainingLimit`
resolved to NaN. Moved the rule to `effectiveAccountLevel` in
@domain/accounts and applied it inside `getAccountLimits`, so every consumer
reads the same numbers.

authorize-send.ts:376 — only `limits-unavailable` had a durable signal. The ops
feed no-ops when OPS_DISCORD_WEBHOOK_URL is unset and drops oldest on overflow
behind an unattributed summary, which makes it a bad instrument for a go/no-go
that is literally a count. `report()` now writes sendGuard.rejection / .mode /
.kind / .level / .error / .cents to the current span for every rejection,
unthrottled.

wallets/index.types.d.ts:111 — `authorize` was optional on
PayNoAmountInvoiceByWalletIdArgs and IntraLedgerPaymentSendWalletIdArgs, so a
send path that forgot the hook compiled and shipped unguarded; two stubbed
on-chain resolvers with commented-out bodies are waiting to hit exactly that.
Made it required and gave the system-credit callers (add-earn, referral payout,
Fygaro top-up credit, debug reimburse) a named opt-out,
SEND_GUARD_NOT_APPLICABLE, which is also the grep for "what still sends without
the guard".

docs/send-guard.md — rewrote the mangled coalescing paragraph an on-call
engineer has to read mid-incident, and documented that the mode flag is NOT
live-reloaded: yamlConfig is read once at process start, so a rollback is helm
upgrade plus a pod roll, not an instant flip.

Tests: gateSend budget/coalescing/span cases, the level-default resolution at
the config layer, the per-rejection span census, and @ts-expect-error
assertions that an unguarded send does not type-check. Each was verified to
fail against the unfixed code.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
…the hook, tell the operator what `off` misses

Three findings from the round-3 review.

1. The census dropped the cohort it exists to count. `addAttributesToCurrentSpan`
   sets an attribute only `if (value)` (src/services/tracing.ts) and
   `AccountLevel.Zero === 0`, so `"sendGuard.level": level` never reached a span
   for a level-0 account — the ~300 unleveled prod accounts plus every genuine
   L0 user. A query for `sendGuard.level = 0` matched nothing, and "attribute
   absent" means level 0 nowhere in the runbook. Both `sendGuard.level` and
   `sendGuard.cents` now go on as strings, matching the ops embed one screen
   below and the existing stringifying call sites (send-lightning.ts,
   ledger/volume.ts). The spec could not catch it: it mocked `@services/tracing`
   as a passthrough and asserted on the arguments. That mock now mirrors the
   real filter, the level cases assert the emitted string for L0, no-level and
   L2, and test/flash/unit/services/tracing.spec.ts pins the filter itself so
   the next mock has something to point at.

2. `authorize` is now required on `withPaymentIdempotency` and on
   `PayInvoiceByWalletIdArgs`, and `payInvoiceByWalletId` actually passes it.
   It was optional on the wrapper and absent from that arg type, so the safety
   claim in index.types.d.ts was false for the rail closest to being re-enabled:
   `ln-invoice-payment-send.ts` is a `Todo: reintroduce` block away from calling
   it, at which point the resolver's inline guard is deleted and nothing
   compiles differently. A new case in send-lightning-ops-events.spec.ts drives
   a rejection through `payInvoiceByWalletId` and asserts it never resolves the
   sender wallet.

   The comment also claimed cover it does not have: the on-chain rails do not go
   through the wrapper at all. It now says so, and the two stubbed resolvers
   carry the same note at the line whoever uncomments them will be reading.

3. `off` is no longer "pre-ENG-573 behaviour", and the runbook said it was.
   Moving the missing-level default into `getAccountLimits` put it in the config
   layer, which Galoy's `AccountLimitsChecker` reads too — so an unleveled
   account is capped at the level-0 $125 on every mode, with no ops event, no
   would-reject embed and no span attribute. docs/send-guard.md gets a "What
   `off` does not cover" section naming the two rails that still enforce it
   (`lnNoAmountInvoicePaymentSend`, `lnNoAmountInvoiceFeeProbe`), and the mode
   table, the rollback section and the `accountLimits` bullet all point at it —
   the three places an operator reads at 3am. limits-checker.spec.ts pins the
   behaviour: level-0 caps rather than NaN, and Galoy's own
   "Cannot transfer more than $125.00 in 24 hours" on a $200 send with zero
   volume.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
…gate it

Round-3 review findings on #506.

`sendGuard.cents` was stringified alongside `sendGuard.level`, which made the
one field whose distribution the rollout actually reads unaggregatable: step 3
of the runbook decides whether to raise a level's limit from the refused
amounts, and that needs MAX and percentiles — string ordering even sorts
"9900" above "125000". The falsy-drop stringifying guarded against is
unreachable: cents is attached on two paths and the over-limit one is provably
`cents > limit >= 12500`. Labels stay strings; measures do not.

docs/send-guard.md now states which span attributes are strings and which is
numeric, so an operator writes `sendGuard.level = "0"` rather than `= 0` and
gets rows back.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
…he docs, empty-wallet send-all

Four review findings, all of them about the rollout being readable and the
guard not renaming ordinary outcomes.

1. The census landed on whatever span was active, which differs by call path.
   On the six keyed rails the guard runs inside
   `LockService().lockPaymentIdempotencyKey` (an active span via
   `wrapAsyncFunctionsToRunInSpan`), so the `sendGuard.*` attributes attached to
   `services.lock.lockPaymentIdempotencyKey`; with no idempotency key, and on
   both on-chain rails, they attached to the GraphQL resolver span. An operator
   scoping the go/no-go query to one name counted a fraction of the traffic, and
   the fraction that went missing was the newer mobile clients that send
   idempotency keys — exactly the traffic the flip to enforce is judged on.
   Both entry points now run inside `asyncRunInSpan(SEND_GUARD_SPAN_NAME, …)` =
   `app.payments.authorizeSend`, with the entry point on `code.function`, and
   the runbook's "What to count from" names it. Evaluation and reporting go
   inside the span, the mode decision stays outside, so only
   `limits-unavailable` still records a span exception.

2. docs/send-guard.md claimed `authorizeSend` was "the only Flash-side check on
   a user-initiated send". Cashout is not behind it: `ValidOffer.execute()` pays
   a bolt11 out of the user's own wallet from the `initiateCashout` mutation
   with no attempt budget and no daily cap. Softened line 3 and added a
   "Not covered by the guard at all" section naming that rail, its compensating
   controls (`cashout.minimum`/`maximum`, balance check, account level) and its
   missing volume limits — alongside system credits and the two stubs.

3. `onchain-payment-send-all` fed `cents: 0n` into the guard for a drained or
   never-funded wallet, which `getBalanceForWallet` reads as `USDAmount.ZERO`
   by design (the post-cutover default for every migrated account's legacy USD
   wallet). Enforcing, a "send all" tap on an empty wallet would have returned
   "Amount must be greater than zero" instead of the rail's balance error; in
   log-only every such tap landed in the `invalid-amount` bucket the runbook
   says should be near zero. The resolver now skips the guard when the balance
   rounds to zero cents and lets `OnchainUsdPaymentValidator` answer, as before
   ENG-573.

4. `export * from "./authorize-send"` put `__resetOpsEventCoalescingForTest` on
   the `Payments` public surface, where calling it drops every accumulated
   `muted` count. Replaced with explicit named exports.

Tests: span name + ordering for both entry points and none on `off`; no span
exception for an enforced over-limit rejection; zero-balance and sub-cent-dust
send-all skip the guard while one cent still reaches it; a type-level assertion
(enforced by `yarn tsc-check`) that the barrel does not re-export the reset.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
Four documentation/test defects from review, all about the guard describing
itself more confidently than it behaves.

`kind` is the rail, not the destination. A bolt11 or LN-address payment to
another Flash user never leaves Flash, but every lightning/lnurl rail hands the
guard `kind: "lightning"` / `"lnurl"` and is judged against `withdrawalLimit` —
the destination is not resolved until the payment flow is built, after the
guard. "Everything leaving Flash uses the withdrawal limit" was therefore an
approximation the guard cannot honour. It is observable at level 1 only, where
the schema defaults differ ($1,000 withdrawal vs $2,000 intraLedger); L0/L2/L3
carry equal limits. docs/send-guard.md (check 3) now states the approximation,
scopes it, and puts the choice — raise `accountLimits.withdrawal.level.1` to
200000, or accept the gap — in front of the operator BEFORE the flip. The
`over-daily-limit` triage bullet says the same thing where it bites: those L1
rows are indistinguishable from real external sends in the census bucket step 3
tells the operator to read.

The unguarded-rail inventory omitted the Bridge USDT withdrawal rail.
`bridgeInitiateWithdrawal` -> `BridgeService.initiateWithdrawal` ->
`IbexClient.sendCrypto` moves the user's own USDT out with no attempt budget, no
`accountLimits` cap and no configured min/max at all — only a KYC-approved Bridge
customer, level >= 1, and an execution-time balance re-check — on the largest
per-transaction amounts on the platform. Same class of gap the cashout bullet was
added for last round, on the bigger rail. Now listed in docs/send-guard.md and
mirrored in the guard's header comment, which also named cashout alone.

"Lets the rail return its own balance error" was false. On a zero balance
`payOnChainByWalletId` reaches `checkOnchainMin`, which returns a bare
`ValidationError("Amount must be greater than 0")`, and `mapError` has no case
for it beyond the catch-all — the client sees the generic unexpected-error
string. The census-bucket reason for skipping the guard is unchanged and still
right; the claim about what the user sees is replaced with what actually
happens, in the resolver comment, the runbook and the wiring spec's comment.

"Keeps the span-exception signal for limits-unavailable only" could not fail for
the reason it stated: the spec's `asyncRunInSpan` mock returned `fn()` untouched,
so the real helper's `if (ret instanceof Error) recordException(span, ret)`
branch was not modelled. Returning `outcome.error` from `runInGuardSpan` kept the
test green while production recorded a span exception for every over-limit
rejection, burying the one signal the runbook says to page on. The mock now
mirrors the branch; verified by mutation — that edit fails the test.

Tests:
- `authorize-send.spec.ts`: an L1 $1,500 inside-Flash lightning send is refused
  against the withdrawal limit and lands in the census as `kind: lightning`,
  while the same amount over a username is allowed; L0/L2/L3 limit equality is
  pinned so raising the L1 withdrawal limit fails the test and forces the doc
  and the decision to move with it.
- `error-map.spec.ts`: `ValidationError("Amount must be greater than 0")` maps
  to UNEXPECTED_CLIENT_ERROR with the interpolated catch-all string, never a
  balance message.
- `send-guard-wiring.spec.ts`: the unguarded-rail inventory is checked both
  ways — cashout and Bridge withdrawal still do not call `authorizeSend`, and
  docs/send-guard.md still names each of them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
The test asserted that `src/app/offers/ValidOffer.ts` and
`src/services/bridge/index.ts` do not contain `authorizeSend` — but all eight
guarded rails call the guard from their GraphQL resolver, never from `@app`
or `@services`. So its stated invariant ("wire one of these rails into the
guard and the first assertion fails") did not hold: someone could guard
cashout tomorrow and this would stay green. Point it at the two resolvers
that would actually do the wiring.

Also finish the Bridge-rail comment fix from the last round: the header
comment's opening paragraph listed both unguarded rails, but its closing
summary — the sentence a maintainer skims for "what still sends unguarded" —
still named cashout alone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
Operator decision, 2026-09-08. Galoy ships level 1 split — $1,000 withdrawal
against $2,000 intraLedger — and the runbook offered to close the gap by
raising the withdrawal limit to $2,000. That would have doubled the real
external cap for every L1 account as a side effect of tidying up a census
ambiguity, so it is closed the other way: `intraLedger.level.1` drops to
100000 and level 1 is one number, $1,000.

Every level now carries equal withdrawal and intraLedger limits, so the
guard's rail-vs-destination approximation (a bolt11 or LN-address payment
that never leaves Flash still arrives as `kind: "lightning"`) cannot change
any outcome at all. The tests assert the whole ladder rather than level 1
alone, so reintroducing a split anywhere fails and sends the author to the
runbook; the kind-to-limit mapping is pinned separately against synthetic
limits so it stays provable when no real level distinguishes them.

Live effect before `enforce`: on lnNoAmountInvoicePaymentSend, Galoy's own
checker applies intraLedgerLimit outside the guard's mode switch, so an L1
inside-Flash no-amount BTC send is capped at $1,000 rather than $2,000 from
merge. Every other rail is unaffected until the switch is flipped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
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