Skip to content

feat(payments): pre-warm the send path with prepareSend [AMB-3038] - #42

Merged
wthrajat merged 7 commits into
mainfrom
jesseva/amb-3038-payments-sdk-pre-warm-the-send-path-with-preparesend-to
Aug 20, 2026
Merged

feat(payments): pre-warm the send path with prepareSend [AMB-3038]#42
wthrajat merged 7 commits into
mainfrom
jesseva/amb-3038-payments-sdk-pre-warm-the-send-path-with-preparesend-to

Conversation

@bufo24

@bufo24 bufo24 commented Aug 19, 2026

Copy link
Copy Markdown
Member

Closes AMB-3038

Problem

Transactions.send() did the full credential derivation on every call: two GraphQL round-trips (GetWalletSendContext, GetWalletNodePermissions) plus two Argon2id passes (m=64 MiB, t=3, p=4) to decrypt the admin macaroon. That is seconds of latency on each payment, all of it redundant after the first send from the same wallet.

What changed

send() is split into a prepare step (wallet context → node permissions → Argon2id → nip44 decrypt) and the payment itself (CreateSendTransaction + node REST call). The prepare step is cached per wallet, so it runs once instead of per send.

New public API on Transactions:

Method Purpose
prepareSend(params) Runs the prepare step ahead of time and caches the macaroon
isSendReady(walletId) Whether a prepared macaroon is resident — false while still deriving
forgetSend(walletId) Drops it, releasing the decrypted macaroon

PaymentsConfig gains an optional send: readonly PrepareSendParams[] to pre-warm wallets from the constructor. Pre-warming runs sequentially — Argon2id is CPU-bound, so concurrency would only delay readiness — and swallows per-wallet failures. A missing serviceApiKey, by contrast, throws from the constructor rather than silently pre-warming nothing.

The one rule the cache runs on

Only prepareSend writes the cache, and only a send that omits password reads it. A send carrying a password derives afresh — exactly as it does in the published version.

That is a deliberate constraint, not an oversight. An earlier revision of this PR keyed the cache on credentials so a password-bearing send could hit it too. Doing that obliges the cache to answer "are these the same credentials?", and that single question generated passwordFingerprint, a Credentials/PendingSlot/ReadySlot trio, an isReusable wildcard-and-provenance chain, an overrodeTeamId flag, a #ready/#pending split, a pending-attempt array — and three successive rounds of bugs:

  1. a wrong password evicting an already-prepared wallet,
  2. a concurrent bad-credential attempt displacing a good in-flight one and discarding its result,
  3. an omitted teamId being answered from a slot derived with an explicit override.

Making the cache credential-blind removes all of it. The three bugs are now impossible by construction rather than fixed: a failing send never touches the map, a wallet has one identity instead of a set to match against, and there is no provenance to track. Net −117 lines, and the README's five cache-behaviour bullets collapse to one — which also closes a contract leak where callers had to model the SDK's internal credential matching to predict whether a send would be fast.

The cost is narrow and explicit: a caller who passes password on every send() gets no speedup. Omit it to use the prepared macaroon.

Security posture

  • Master key is discarded. Only the macaroon is retained; masterKey and masterPasswordHash stay locals of the derivation frame. Verified by tracing closure capture — the cached value and the retained promise reach neither the plaintext password nor the master key. A prepared wallet holds admin access to one node, not the key to every wallet in the team.
  • A failing send harms nothing. It never reads or writes the cache, so a typo'd password fails that one call and leaves the prepared wallet working.
  • forgetSend wins races. A preparation resolving after it is discarded rather than resurrecting the macaroon.
  • Blocking is documented. Argon2id is synchronous; prepareSend is async because of the API calls, not because key derivation yields. Prepare at startup, not per request.

Also in here

  • Re-derived onto current main, so the cached path picks up fee_limit_sat (fix: send a fixed fee limit on Transactions.send [AMB-2979] #37) and the already-COMPLETED short-circuit (fix: return already-completed send without re-paying on the node [AMB-2985] #39). Both sit on the single shared path after the cache lookup, so a cache hit cannot bypass them; pinned by a test.
  • Test-collection fix, unrelated to this feature. tsx --test src/**/*.test.ts was unquoted, and under sh ** collapses to one directory level — so packages/payments/src/client.test.ts had never run. core escaped it only because it has no subdirectory tests, leaving the pattern unexpanded for tsx to glob itself. Quoting it hands the pattern to tsx in both packages: payments goes 44 → 51 tests. Happy to split this out if you'd rather it landed separately.

Release impact

New public API on a published package (npm is at 1.0.3) — release-please will cut a minor. Deliberately not marked breaking: prepareSend does not exist on main, and send({walletId, password}) behaves as the published version already does.

Test plan

  • pnpm run build / typecheck / format:check — pass
  • pnpm run test — 62/62 (11 core, 51 payments)
  • pnpm run typecheck:examples / test:examples — pass (dual ESM/CJS build)
  • A prepared password-less send issues only CreateSendTransaction
  • A send passing a password always re-derives and leaves the prepared wallet intact
  • isSendReady() is false while deriving, true once resolved, false after forgetSend()
  • Sandbox wallets prepare, and still send unprepared, without a password
  • Prepared sends still carry fee_limit_sat
  • send pre-warm without serviceApiKey throws ConfigError from the constructor
  • Manual check against a live wallet before merge

Known follow-up

forgetSend cannot cancel an in-flight Argon2id pass — the derivation runs to completion and its result is discarded. Fixing it means threading an AbortSignal through PrepareSendParams, a public API change beyond this PR.

Docs updated in AGENTS.md, packages/payments/README.md, and docs/INTEGRATION.md.

🤖 Generated with Claude Code

bufo24 added 7 commits August 20, 2026 11:54
send() paid for two GraphQL round-trips and two Argon2id passes (m=64 MiB,
t=3, p=4) before it could touch an invoice — seconds of work, none of it
dependent on the payment. Split that into a prepare step cached per wallet.

- prepareSend({ walletId, password }) resolves the node endpoint and decrypts
  the admin macaroon up front; a later send() then issues a single
  CreateSendTransaction and pays, with no password argument.
- isSendReady(walletId) reports residency, false while still deriving.
- forgetSend(walletId) evicts, releasing node admin access from memory and
  picking up rotated credentials (the cache has no expiry).
- PaymentsConfig.send pre-warms an array of wallets from the constructor,
  sequentially and fire-and-forget.

Only the macaroon is retained — masterKey/masterPasswordHash are dropped after
the decrypt, so a prepared wallet holds access to one node rather than the key
to every wallet in the team. Slots record a sha256 fingerprint of
(password, teamId) so a send() with different credentials re-derives instead of
reusing another password's macaroon, and a rejected prepare evicts itself so a
transient failure can't poison later sends.
The prepare cache short-circuits credential derivation only; the node call
body must keep carrying fee_limit_sat. Pins that so a future cache refactor
cannot silently drop it.
… fail

The prepare cache kept one slot per wallet and installed each new slot the
moment its derivation started, before knowing it would succeed. A send() with
the wrong password therefore displaced an already-prepared wallet, and the
rejection handler then deleted the entry outright — so one caller's typo left
every later password-less send() throwing "A team password is required", and a
concurrent password-less send() in that window was handed the doomed promise.

Split the cache in two: #pending holds derivations still running, so concurrent
callers still share one Argon2 pass, and #ready holds finished ones. Only a
successful derivation graduates to #ready, and a rejection drops nothing but
its own #pending slot. Lookups check #ready first, so a wallet that is already
prepared wins over an in-flight derivation that may yet fail.

The slot fingerprint also hashed the raw teamId param rather than the one the
derivation actually used, so a wallet pre-warmed without teamId never matched a
send() passing the wallet's own team id explicitly — the documented override —
and silently re-ran both GraphQL calls and both Argon2 passes. Fingerprint the
password alone and compare the resolved teamId now recorded on the slot.
Two defects in the send cache, both found by review.

#pending held one slot per wallet, so a second caller's wrong password
overwrote a good derivation already in flight. The good one then resolved
into a handler that no longer recognised its slot and dropped the result,
leaving isSendReady() false after a prepareSend() that had succeeded, and
password-less sends failing. It now holds every in-flight attempt for a
wallet, so siblings settle independently.

isReusable treated an omitted teamId as matching any slot, so a send with
a password but no teamId could be answered from a slot derived with an
explicit override -- silently using a salt the caller never named, when
omitting teamId is documented to resolve it from the wallet. Slots now
record whether their salt was overridden and only auto-resolved ones
answer an omitted teamId.
Cleanup pass over the prepareSend diff.

The pre-warm loop reached Transactions through the getter inside its
per-wallet try, so requireServiceApiKey's ConfigError was swallowed like a
bad password: constructing with `send` but no serviceApiKey pre-warmed
nothing, forever, with no signal. The resource is now resolved once in the
constructor, where that misconfiguration throws.

Also quote the test glob. Under sh, src/**/*.test.ts means one directory
level, so packages/payments/src/client.test.ts was never collected -- six
tests had never run. core got away with it only because it has no
subdirectory tests, leaving the pattern unexpanded for tsx to glob itself.
Quoting hands the pattern to tsx in both packages: payments 44 -> 51 tests,
all passing.

Drops a fire-and-forget IIFE, de-duplicates two wrong-password test
setups, and splits the AGENTS.md cache-invariant run-on into one bullet
per invariant.
The cache was keyed on credentials, so it had to answer "are these the
same credentials?" -- and that question, not prewarming, was where the
complexity lived. It produced passwordFingerprint, the Credentials/
PendingSlot/ReadySlot trio, isReusable's wildcard-and-provenance chain,
overrodeTeamId, the ready/pending split and the pending array, plus three
rounds of bugs: wrong-password eviction, a concurrent attempt displacing a
good one, and an omitted teamId answered from an overridden slot.

Only prepareSend writes the cache now, and only a send that omits password
reads it. A send carrying a password derives afresh, as it did before this
feature existed. All three bug classes become impossible by construction:
a failing send never touches the map, a wallet has one identity rather
than a set to match against, and there is no provenance to track.

Net -117 lines and the README's five cache-behaviour bullets collapse to
one, removing the contract leak where callers had to model the SDK's
internal credential matching to predict whether a send would be fast.

Not marked breaking: prepareSend is unreleased (absent from main, npm is
at 1.0.3), and send(walletId, password) behaves exactly as the published
version does. Only this branch's own earlier commits are affected.
@bufo24
bufo24 force-pushed the jesseva/amb-3038-payments-sdk-pre-warm-the-send-path-with-preparesend-to branch from 4e5e706 to 1355062 Compare August 20, 2026 09:54
@wthrajat
wthrajat merged commit bd505a3 into main Aug 20, 2026
2 checks passed
@apotdevin apotdevin mentioned this pull request Aug 20, 2026
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