feat(x402): POST /settle — design note and implementation (closes #126) - #149
feat(x402): POST /settle — design note and implementation (closes #126)#149ezedike-evan wants to merge 2 commits into
Conversation
Covers the four decisions Miracle656#126 asks to have agreed before any settlement code is written: which keys the facilitator holds, the idempotency key and when its record is written, how a payload is marked consumed, and how failures map onto SettleResponse. No runtime change — the implementation follows once these are agreed.
Wires @x402/stellar's ExactStellarScheme behind the route rather than reimplementing verification or settlement, per the RFP. What this owns is the surface: the wire contract, key handling, idempotency, replay and failure mapping, as agreed in docs/x402/settle-design.md. - no payer key is held; the facilitator holds fee and sequence-number keys only, and answers explicitly when none is configured - the idempotency key is the inner transaction hash, derived from the payload, and its record is written before submission - a second settle for the same payload replays the stored answer; an in-flight record resolves from the ledger, never by resubmitting - failures keep the SDK's own errorReason values, and no rejection ever leaves errorReason null
|
@ezedike-evan Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Miracle656
left a comment
There was a problem hiding this comment.
The design work here is the best thing in the facilitator so far, and I want to be clear about that before the blocker, because the blocker is timing rather than quality.
What you built is better than what is currently on main. #142 merged about two hours ago with a /settle that verifies and submits. Yours adds the part that actually makes settlement safe to call twice:
SettlementAttemptwith@@unique([network, txHash])as the lock- the record written before submission, so a crash between submit and write can't lose the attempt
submittingrows resolved from the ledger, never by resubmission- terminal failures replaying their failure instead of getting a second submission
- idempotency keyed on the inner transaction hash rather than a caller-supplied header — the caller controls the header, so keying on it would let an attacker force a duplicate submission
That last decision is the one I'd have argued for and the reasoning in the design note is right. It's also, in effect, the idempotency half of #147, which I filed this morning before this PR existed.
Blocker: it collides with what merged
main now has /supported, /verify and /settle in src/api/facilitator.ts. This PR adds a second POST /settle in a new src/routes/facilitator.ts, and src/index.ts ends up with:
import { registerFacilitatorRoutes } from './api/facilitator' // main
import { registerFacilitatorRoutes } from './routes/facilitator' // this PRTwo problems, both fatal rather than cosmetic: a duplicate identifier that won't compile, and — if it did — Fastify raising FST_ERR_DUPLICATED_ROUTE for POST /settle at startup. The service wouldn't boot.
None of this is your fault. You opened against a main that didn't have #142 yet, and #126 explicitly asked for a design note first, which is why yours took longer to arrive than the implementation it now overlaps.
What I'd like instead
Rebase onto main and let your /settle replace the one in src/api/facilitator.ts, rather than adding a parallel route.
Concretely:
- keep
src/x402/facilitator.ts(config/keys, returningnullwhen unconfigured — that's a cleaner seam than what's onmain) - move your handler into the existing
src/api/facilitator.ts, replacing the current/settlebody - drop
src/routes/facilitator.tsand its registration - keep the
SettlementAttemptmodel, the design note, and all offacilitatorSettle.test.ts—mainhas nothing equivalent
The result should be a smaller diff that strictly improves the merged /settle. If that rework is more than you want to take on, say so and I'll do the port myself with attribution to you — I'd rather ask than assume.
Two notes for the rebase
prisma/schema.prismamoved under you: #141 landed thenetworkdiscriminator, and every model now carriesnetworkwith@default("testnet"). Your@@unique([network, txHash])fits that convention exactly, so this should be additive — just make sureSettlementAttemptalso gets the default, orprisma db pushwill refuse on a populated database. That specific mistake cost #141 a round.- Your note that a published settled transaction hash per network is still outstanding is correct and worth keeping in the PR body. It needs funded keys, which is deployment rather than code — but it is the RFP's acceptance criterion, so it shouldn't quietly disappear.
Genuinely good work. The four decisions restated at the end of the design note are exactly the right way to make a reviewer's job possible.
closes #126
The issue asks for the four decisions to be agreed before settlement code exists, so this PR leads with the design note (
docs/x402/settle-design.md) and implements exactly what it describes. The note is the part to argue with — if a decision in it is wrong, the code following it is wrong, and it is cheap to change now.The four decisions
1. Keys. No payer key, ever — the payer authorises through a signed Soroban auth entry inside
paymentPayload.payload.transaction. Two keys exist and both are fee/sequence-number only: the settlement signer(s)ExactStellarSchemetakes, and the optionalfeeBumpSigner. Neither can move user funds, and the SDK's ownvalidateSimulationEventsrejects a payload in which a facilitator address participates in the transfer, so non-custodial is a property of the flow rather than a claim. Secrets live in env (FACILITATOR_SIGNER_SECRETS,FACILITATOR_FEE_BUMP_SECRET), are read once at init, and are never persisted, logged or returned. With none configured,getFacilitator()returns null and/settlefails explicitly instead of half-working. Fee ceiling is configurable (FACILITATOR_MAX_FEE_STROOPS, default 50,000), not hard-wired.2. Idempotency. The key is the inner transaction hash —
TransactionBuilder.fromXDR(transaction, passphrase).hash()— derived from the payload rather than generated by us, identical to the hash the ledger will record, and network-scoped for free because the passphrase is mixed into it. TheSettlementAttemptrow is written before submission; its@@unique([network, txHash])is the lock, so two concurrent settles race to insert and exactly one proceeds. That holds across replicas, which an in-process mutex would not.3. Replay.
/verifyis side-effect free, so/settleis what burns a payload. A payload is consumed the moment its record exists:settledreplays the stored response verbatim,failedreplays the stored failure without resubmitting, andsubmittingis resolved by reading the ledger — never by submitting again, since a row in that state means we genuinely do not know whether the network took it.4. Failure semantics. The SDK's own reasons are returned unaltered (
settle_exact_stellar_transaction_submission_failed,..._transaction_failed,unexpected_settle_error, …), never rewritten into a vocabulary of ours. A failed payment is200+success: false, not a 4xx:HTTPFacilitatorClientraises a transport error on a non-2xx body it cannot recognise, so the reference client only reads failures cleanly in that shape. 4xx is reserved for a request we cannot parse, and even then the body keeps theSettleResponseshape. No rejection ever leaveserrorReasonnull — that is an RFP hard criterion and it is asserted in a test rather than claimed.What was built on, and what was not
Per the RFP correction in the issue, no verification or settlement logic is reimplemented:
ExactStellarSchemefrom@x402/stellaris registered on anx402Facilitatorand the payload is handed to it untouched (there is a test asserting exactly that). SEP-41 handling, 7-decimal amounts,signatureExpirationLedgerbounds and fee sponsorship are the SDK's, not ours.Files
docs/x402/settle-design.md— the note: custody table, idempotency key rationale, replay states, and a failure table mapping every situation to HTTP status /success/errorReason.src/x402/facilitator.ts— configuration and keys only; memoised per network; returns null when unconfigured.src/routes/facilitator.ts—POST /settle, public (a facilitator cannot demand payment to accept one). Also exportsderiveIdempotencyKey.prisma/schema.prisma—SettlementAttempt, with the unique identity that does the locking.src/index.ts,.env.example— wiring and the three new keys.Tests
src/__tests__/facilitatorSettle.test.ts(22), driving the route throughapp.injectagainst real signed envelopes built with@stellar/stellar-sdk, so the hashing is real and not stubbed:submitting) record resolves from the ledger: SUCCESS finalises it settled, FAILED finalises it failed, NOT_FOUND answers "still in flight" with a non-null reason — andsettleis never called in any of thoseC…contract-account payload takes the same path as aG…oneSettleResponseshape, with no record writtensettlethrowing →unexpected_settle_errorand the row finalised failednpx vitest run— 267 passed, 1 skipped (36 files).npx tsc --noEmitclean (afternpx prisma generate; the committed client is stale onmainfor unrelated models). No lint script in this repo.Two notes on merge order
src/routes/facilitator.tsis created here because x402 facilitator: implement GET /supported #124 hasn't merged. If facilitator: implement GET /supported #143 lands first, this route folds into that file — happy to rebase either way.