From 0646380dd201d60b16273a649367012e07b24531 Mon Sep 17 00:00:00 2001 From: shum Date: Thu, 27 Aug 2026 10:01:56 +0000 Subject: [PATCH] plan: adopt upstream's payment status types --- .../2026-08-21-badges-web-checkout.md | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/plans/badges-codes/2026-08-21-badges-web-checkout.md b/plans/badges-codes/2026-08-21-badges-web-checkout.md index b12fc0068b..8c527ba0d6 100644 --- a/plans/badges-codes/2026-08-21-badges-web-checkout.md +++ b/plans/badges-codes/2026-08-21-badges-web-checkout.md @@ -213,7 +213,7 @@ None of these steps touches a payment provider. A1, A2, A3 and A5 are independen - tagged sums (`BadgeServiceCommand`, `BadgeServiceResponse`, `StatementEntryType`, `StatementCreditType`, `StatementDebitType`, `ServicePayment`, `ServicePaymentMethod`, `ServicePaymentDestination`, `OfferDiscount`): `taggedObjectJSON` with the discriminator `type`, matching the JTD `discriminator` in `docs/protocol/badges-rpc.schema.json` - `SCUnknown` and `SDUnknown` (`Badges/Service.hs:167,177`) must round-trip an unrecognised tag verbatim (RPC §"Statement and balance": "An unknown type is stored as received and decoded after an app upgrade") - `BadgeServiceErrorCode` already has both instances via `TextEncoding` (`Badges/Service.hs:198-246`) and `BSEUnknown` already round-trips an unknown tag. Leave it alone. -- **`BadgePurchaseStatus` (`Badges/Types.hs:64`) and `BadgePaymentStatus` (`:60`) have no instances at all**, not `TextEncoding`, `ToField`, `FromField` or JSON, so nothing in the repo defines how they are spelled in a column. Both are persisted by B1 and C1. Write the `TextEncoding` instances by hand, following `instance TextEncoding BadgeType` (`Badges.hs:91-101`), spelling `PSAcquiring | PSIssued | PSSuperseded | PSFailed` as `acquiring | issued | superseded | failed` and `BPSNew | BPSInvoiced | BPSPending | BPSSettled | BPSFailed | BPSExpired` as `new | invoiced | pending | settled | failed | expired`. Derive the rest from that single spelling: `ToJSON`/`FromJSON` through `textToJSON` and `textParseJSON` (`Badges.hs:103-108`), and `ToField`/`FromField` through `toField . textEncode` and `fromTextField_ textDecode` (`Badges.hs:339-341`). `enumJSON` is an Aeson options builder (`simplexmq Parsers.hs:102`) and cannot define a text codec, so it is not used for these two; it stays correct for the plain enums above. Every SQL literal B1 and C1 write comes from these instances. +- **`BadgePurchaseStatus` (`Badges/Types.hs`), `InvoiceStatus` and `PaymentStatus` (`PaymentService/Types.hs`) have no instances at all**, not `TextEncoding`, `ToField`, `FromField` or JSON, so nothing in the repo defines how they are spelled in a column. All three are persisted by B1, C1 and D0. Write the `TextEncoding` instances by hand, following `instance TextEncoding BadgeType` (`Badges.hs:91-101`), spelling `PSAcquiring | PSIssued | PSSuperseded | PSFailed` as `acquiring | issued | superseded | failed`, `ISOpen | ISPaid | ISExpired` as `open | paid | expired`, and `PSPending | PSSettled | PSFailed` as `pending | settled | failed`. Each instance lives beside the type it encodes, so `InvoiceStatus` and `PaymentStatus` get theirs in `PaymentService/Types.hs` next to `PaymentProvider`'s, not in `Badges/Types.hs`. Derive the rest from that single spelling: `ToField`/`FromField` through `toField . textEncode` and `fromTextField_ textDecode` (`Badges.hs:339-341`); only `BadgePurchaseStatus` also gets `ToJSON`/`FromJSON` through `textToJSON` and `textParseJSON` (`Badges.hs:103-108`), since the two payment statuses never cross the wire. `enumJSON` is an Aeson options builder (`simplexmq Parsers.hs:102`) and cannot define a text codec, so it is not used for these; it stays correct for the plain enums above. Every SQL literal B1, C1 and D0 write comes from these instances. `PaymentStatus`'s `textDecode` cannot rebuild `PSFailed`'s text, which is `payments.exception`, a column of its own — see §9. - `Badges/Types.hs` cannot host any of this as it stands. It declares only `DerivingStrategies`, `DuplicateRecordFields` and `GeneralizedNewtypeDeriving` (`:1-3`), and its `import Simplex.Chat.Badges` is against an explicit export list (`Badges.hs:17-51`) that exports neither `TextEncoding` nor `textEncode`, `textDecode`, `textToJSON`, `textParseJSON` or `fromTextField_`. Add `CPP`, `LambdaCase`, `OverloadedStrings` and `TemplateHaskell` to the pragma block, `import Simplex.Messaging.Encoding.String`, `import Simplex.Messaging.Agent.Store.DB (fromTextField_)`, and the `#if defined(dbPostgres)` `ToField`/`FromField` import block copied verbatim from `Badges.hs:74-80`. There is no import cycle: `Badges/Types.hs` imports `Badges.hs` and not the reverse. - Three record fields cannot decode their own rows, all in types marked "to review" or "unconfirmed draft", so correcting them is not a protocol change; record it in §9 when the step lands. `BadgePayment.paymentId` and `BadgeIssuance.issuanceId` are `Int64` (`Badges/Types.hs:117,177`) against `TEXT NOT NULL PRIMARY KEY` columns (`M20260731_user_badges.hs:43,167`): both become `Text`. `BadgePurchase.paymentId` is `Int64` (`:106`) against `payment_id TEXT REFERENCES @payments`, which is nullable (`M20260731_user_badges.hs:99`): it becomes `Maybe Text`. `DuplicateRecordFields` is already on (`Badges/Types.hs:2`), so the two `paymentId` fields may differ. - `BadgeIssuance` also has no field for `badge_type TEXT NOT NULL` or `credential BLOB NOT NULL` (`M20260731_user_badges.hs:170,174`), and B1's `getIssuanceForRedeemedCode` exists precisely to return that credential. Add `badgeType :: BadgeType` and `credential :: BadgeCredential`, so B1 and C1 decode the whole row into one type rather than defining a second. @@ -399,7 +399,7 @@ The first end-to-end path. No payments yet; codes come from B8's operator toolin - `Store.hs` defines `data ServiceError`, the error type of every store function, covering not-found, conflict and decode failures. - **Transaction discipline.** Every function takes a `DB.Connection` and opens no transaction of its own. `withServiceTransaction :: DBStore -> (DB.Connection -> ExceptT ServiceError IO a) -> IO (Either ServiceError a)` is the only place a transaction is opened, and a `Left` rolls back. Command handlers call it once. This is what makes one transaction per command achievable: functions that open their own transactions cannot be composed into one. -- Purchases and payments: `getPurchaseByKey`; `createPurchase`, writing the row with status `issued`, both badge-type columns set, `purchase_key` from the request's signer key and `master_key` from the badge master key in the verified badge request, all four columns being `NOT NULL` in the shared `badgeSchema` (`M20260731_user_badges.hs:95-98`), since the service has no pre-response row either; `createCodePayment`, writing the `payments` row with a caller-minted UUID as `payment_id` (`payments.payment_id` is `TEXT NOT NULL PRIMARY KEY` with no default, `M20260731_user_badges.hs:43`), `provider = 'code'`, `invoice_id` NULL and `status = 'settled'`, and pointing the purchase row's `payment_id` at it. Statuses go through the `ToField` instances A2 adds for `BadgePurchaseStatus` (`Badges/Types.hs:64`) and `BadgePaymentStatus` (`:60`), which is the same vocabulary the client uses (C1), because both sides share `badgeSchema`. +- Purchases and payments: `getPurchaseByKey`; `createPurchase`, writing the row with status `issued`, both badge-type columns set, `purchase_key` from the request's signer key and `master_key` from the badge master key in the verified badge request, all four columns being `NOT NULL` in the shared `badgeSchema` (`M20260731_user_badges.hs:95-98`), since the service has no pre-response row either; `createCodePayment`, writing the `payments` row with a caller-minted UUID as `payment_id` (`payments.payment_id` is `TEXT NOT NULL PRIMARY KEY` with no default, `M20260731_user_badges.hs:43`), `provider = 'code'`, `invoice_id` NULL and `status = 'settled'`, and pointing the purchase row's `payment_id` at it. Statuses go through the `ToField` instances A2 adds for `BadgePurchaseStatus` (`Badges/Types.hs`) and `PaymentStatus` (`PaymentService/Types.hs`), which is the same vocabulary the client uses (C1), because both sides share `badgeSchema`. - Ledger: `getLastLedgerEntry`, `appendLedgerEntry`, `getLedgerSince`. - Issuances: `getIssuanceForPeriod`; `getIssuanceForRedeemedCode` (code hash → `redeemed_purchase_id` → the issuance whose period contains `redeemed_at`; B7's replay path needs this, since a purchase may have several issuances by then); `createIssuance`. - Codes: `getCodeByHash`, returning the code row **joined to `badge_purchases`** so the caller sees the `purchase_key` behind `redeemed_purchase_id` and can distinguish a replay from another key's use; `markCodeRedeemed`; `unredeemCode`, clearing `redeemed_purchase_id` and `redeemed_at` and setting `unredeemed_at = now`, which both re-enables redemption and reopens E4's disclosure window; `insertCodes`; `revokeCode`; `revokeBatch`, setting `revoked_at` on every unrevoked code of a batch through `@idx_codes_batch` (A3), which is what B8's `codes revoke --batch` calls. @@ -628,7 +628,7 @@ Phase C ends with a chat client that can redeem a code minted by B8 and show the **Do:** -- Statuses are the constructors of `BadgePurchaseStatus` (`Badges/Types.hs:64`) and `BadgePaymentStatus` (`:60`), written through the `ToField` instances A2 adds; no new status is invented. A code redemption writes only `PSIssued`, `PSSuperseded` and `BPSSettled`, spelled `issued`, `superseded` and `settled`; the rest belong to the invoice flow this plan defers (§6). +- Statuses are the constructors of `BadgePurchaseStatus` (`Badges/Types.hs`) and `PaymentStatus` (`PaymentService/Types.hs`), written through the `ToField` instances A2 adds; no new status is invented. A code redemption writes only `PSIssued`, `PSSuperseded` and `PSSettled`, spelled `issued`, `superseded` and `settled`; the rest belong to the invoice flow this plan defers (§6). - `createCodePayment` writes the `payments` row with `provider = 'code'`, `invoice_id` NULL and `status = 'settled'`, and returns its id. `payments.payment_id` and `badge_issuances.issuance_id` are both `TEXT NOT NULL PRIMARY KEY` with no default (`M20260731_user_badges.hs:43,167`), so each caller mints a UUID with `Data.UUID.V4.nextRandom`; `uuid` is already a library dependency (`simplex-chat.cabal:376`). `badge_ledger.entry_uuid` is different: it is authored by the service and copied verbatim (core §1), never minted here. As on the service side, `payments` has no `price_id` or `offer_id` columns (`M20260731_user_badges.hs:42-56`), so core §5's "`price_id` and `offer_id` NULL" does not apply. - `createPurchase` writes the purchase row with the purchase keypair, the badge master key, that `payment_id`, status `issued`, and `initial_badge_type` and `current_badge_type` both set to the badge type the service stated. Both columns are `NOT NULL` with no default (`M20260731_user_badges.hs:97-98`), and a code redemption does not know its badge type until the response arrives, so the row is written **on success only**, as core §5 requires for the code payment. Nothing is persisted before the send; a response lost in flight is recovered with H2's `codes unredeem`, not by reusing a stored key. Core §5 calls this row *the badge row*; this plan calls it the purchase row throughout (§3). - `createIssuance` writes the `badge_issuances` row carrying the verified credential and its period, which C3's worker writes again for each new period. @@ -1399,7 +1399,7 @@ Append here when a step contradicts this plan: the step id, what was wrong, and - **Phase C review — `redeemBadgeCode` reads the reported badge state outside the badge lock. Accepted, not fixed.** The lock is released after `storeRedeemedBadge` and before `presentUserBadgeToContacts` (it must be: `presentUserBadgeToContacts` takes `chatLock`), and `getUserBadgeState` for the response runs after that. A concurrent redemption on the same profile could supersede this one in between, so the response would name the *other* purchase as shown. Cosmetic: both purchases are stored correctly, the rows are right, and the next `APIGetBadgeState` reports the truth. Fixing it would mean either computing the response inside the lock — which is a different value from the one the app will read next — or widening the lock over `chatLock`, which is the inversion C3 exists to avoid. - **Phase C review — a haddock gap fixed, and a style "fix" reverted as wrong.** The review reported `hasIssuanceForPeriod` (`Store/Badges.hs`) as the module's only query not using `[sql| |]`, and it was reformatted to match. That premise was false and the re-review caught it: the module carries bare-string queries at four other sites against six `[sql| |]` blocks, and the convention is by query length — `[sql| |]` for multi-line or multi-column statements, a plain string for a one-liner. Under the real convention the original was already consistent, and the reformat made a one-line `SELECT EXISTS` span five lines, so it was reverted. Recorded because two successive reviews asserted the convention without reading the module for it. And `runBadgeWorker`'s haddock did not mention the `waitChatStartedAndActivated` gate the C3 review round added to the top of its loop, which is the one thing about that loop a reader most needs to know after a suspend; it now names it and the three sibling loops that gate the same way. - **D0 — `PaymentProvider` now has a column codec, and both `codePaymentProviderText = "code"` literals are gone.** B1's and C1's entries above deferred a real `TextEncoding PaymentProvider` until "a second provider needs writing from the service side"; D0's `createOrder` is that step, since `@invoices.provider` is written per order. The instance lives with the type (`PaymentService/Types.hs`), with `ToField`/`FromField` derived from it under the same CPP pattern `Badges/Types.hs` uses, and no JSON: `PaymentProvider` does not cross the wire, so this spelling is only ever read back from a column it was written to. The service's and the client's `createCodePayment` both now write `PPCode`, so the two databases cannot drift. **`@invoices.provider` is derived from `@web_orders.method`, not passed in** — `card → stripe`, `btc | xmr → crypto`, both crypto methods being the one BTCPay instance — so a caller cannot get the pair wrong. E2 and F1 inherit this rather than inventing their own literals; note that `crypto` is the spelling, not `btcpay` (which appears only in B10's hand-written test fixture). -- **D0 — the method enum lives in `Store.hs` as `OrderMethod`, not in D6's `Orders.hs` as `Method`.** D0 has to persist `@web_orders.method` before `Orders.hs` exists, and D0 makes no cabal edit, so the type and its codec are defined beside the column they serve. D6's step text is corrected to import it. The same applies to `WebOrderStatus` (`invoiced | pending | paid | expired | failed`), which is deliberately **not** `BadgePaymentStatus`: that enum has a `new` state an order never occupies and spells settlement `settled`, where A3's CHECK requires `paid`. `orderInvoiceStatus` is the single mapping between the two vocabularies, and it is what keeps A3's "`@invoices.status` in step" invariant true. +- **D0 — the method enum lives in `Store.hs` as `OrderMethod`, not in D6's `Orders.hs` as `Method`.** D0 has to persist `@web_orders.method` before `Orders.hs` exists, and D0 makes no cabal edit, so the type and its codec are defined beside the column they serve. D6's step text is corrected to import it. The same applies to `WebOrderStatus` (`invoiced | pending | paid | expired | failed`), which is deliberately **not** `InvoiceStatus`: an order needs five states because E3 and E5 have to tell an underpaid expiry from a provider failure, where `@invoices.status` has only three. `orderInvoiceStatus` is the single mapping between the two vocabularies — a projection, `invoiced`+`pending` -> `open`, `paid` -> `paid`, `expired`+`failed` -> `expired` — and it is what keeps A3's "`@invoices.status` in step" invariant true. - **D0 — `createOrder` takes an `invoiceId`, which the plan's field list did not name.** `@invoices.invoice_id` is `TEXT NOT NULL PRIMARY KEY` with no default and the store mints no identifiers (it opens no transaction either), so D6 mints it alongside the `orderId`. It must NOT be the `orderId` itself: the order id is a bearer capability for the code (decision 9) and putting it in a second table widens the surface for no gain. Two more shapes the field list left open, both now fixed in code: `@invoices.price` and `@invoices.amount` both carry A4's `offerTotal` with `discount_amount`/`credit_amount` NULL, because an offer's discount is expressed as free months so the total IS the price; and `@invoices.payment_crypto_currency` is not written at all (A3 — `method` is the single source). - **D0 — every order read is an INNER join to `@invoices`, and three new `ServiceError` constructors.** `getOrder`, `getOrderByProviderRef`, `getOrderByShortRef` and `getStuckOrders` all go through one `orderSelect`, joined rather than left-joined: `createOrder` is the only writer of a `@web_orders` row and always writes the invoice with it, so an order without one does not exist and `WebOrder` can hold the invoice's NOT NULL columns unwrapped. `SEOrderNotFound` is what `updateOrderStatus`, `setOrderProviderRef` and `setOrderSettled` throw when the order they name is absent; `SEProviderEventNotFound` is `markProviderEventProcessed`'s, reachable only if it and `recordProviderEvent` disagree about the key. `SEInvoiceNotFound` is the invariant guard: the `@invoices.status` write is an `UPDATE … RETURNING` like every other writer here, so an order that resolves to no invoice fails instead of moving one status and leaving the other behind — `createOrder` makes that unreachable, and the guard is there for whoever next writes this table. **`setOrderSettled` takes `settledAt` and `now` separately**: the provider's reported settlement instant is routinely in the past for an on-chain confirmation, and writing it to `updated_at` would move that column backwards past an earlier `updateOrderStatus`; E3, F2 and H3 pass both. **`setOrderSettled` does not guard on the current status**: settlement is idempotent and monotonic toward `paid` (E3), but that is E3's rule to apply, because the same decision governs whether a code row is written — a guard here would answer `SEOrderNotFound` for a replay, which is worse than no guard. - **D0 — `getStuckOrders`' status filter leaves a recovery gap, implemented as specified. H3 decides.** The step says `invoiced` or `pending`, and that is what shipped. But E3 can move `expired` and `failed` to `paid` on a late webhook, so an order that one webhook marked `expired` and that then settles on chain with THAT webhook missed is never re-read from the provider by H3's pass — the buyer has paid and only support (H2) recovers it. Missed webhooks are exactly what H3 exists for, so this is worth a decision rather than an assumption; widening the filter to all four non-`paid` statuses is a change to H3's contract, not to this query, and the haddock on `getStuckOrders` says so. @@ -1464,6 +1464,15 @@ Append here when a step contradicts this plan: the step id, what was wrong, and - **D3 — a change under `web/dist/` does not invalidate cabal's build, so the embedded site goes stale on a local run.** Measured after this step's rebuild: `cabal build simplex-badge-service` printed `Up to date`, and the already-linked binary still carried D2's site — `Choose an option to continue.` present, D3's `nothingChosenMessage` absent. `web/dist` is not in `extra-source-files`, and cabal compares the *content* of the package's declared sources, so a bare `touch` of `Assets.hs` leaves it `Up to date` too; the file-embed dependency is invisible to it. Any real content change to a Haskell source makes cabal invoke ghc, and the current `dist/` is embedded then. Clean builds are unaffected, so CI and D8's gate are unaffected — but a local run of D4's web examples after a site change asserts against the PREVIOUSLY embedded page unless the suite is forced to rebuild, which is exactly the reassurance a web step reaches for. Forced here, `should serve the index with every token resolved …` and `should serve every module a served module imports under that module's own prefix` both pass with D3's `catalog.js` in the module graph. The remedy for whoever next has reason to touch the cabal file is `extra-source-files` covering `apps/simplex-badge-service/web/dist/**`, `web/index.html` and `web/styles.css`; it was not taken here because this step must not touch the Haskell build. +- **A2/B1/C1/D0 — the payment statuses are upstream's `InvoiceStatus` and `PaymentStatus`; `BadgePaymentStatus` no longer exists and must not be re-added.** The plan was written against `Badges/Types.hs`'s `BadgePaymentStatus` (`BPSNew | BPSInvoiced | BPSPending | BPSSettled | BPSFailed | BPSExpired`). Upstream **deleted** that type in `1c7bc1aed core: refactor badge types (#7387)`, together with `BadgePayment`, and moved the payment model to `PaymentService/Types.hs`: `PaymentId`, `StoredPayment`, `PaymentFunding`, `PaymentTerm` and `PaymentStatus = PSPending | PSSettled | PSFailed {exception}`, beside the `InvoiceStatus = ISOpen | ISPaid | ISExpired` that was already there. A merge of `origin/badges` resurrected `BadgePaymentStatus` because our steps still wrote it; that resolution was wrong and has been undone. **Adopt upstream's types; do not re-declare the deleted one.** Concretely: + - `payments.status` is `PaymentStatus`. C1 and B1 write exactly one value, and `PSSettled` spells `settled` just as `BPSSettled` did, so no stored data changes. + - `@invoices.status` is `InvoiceStatus`. `orderInvoiceStatus` became a 5 -> 3 projection (`invoiced`+`pending` -> `open`, `paid` -> `paid`, `expired`+`failed` -> `expired`). That is acceptable because A3 already states this column "is maintained in step with it in the same transaction and is read by nothing in this plan" — `@web_orders.status` is the authoritative lifecycle column and keeps all five states, with its own `CHECK`. `InvoiceStatus` was **not** widened; the D0 tests that asserted `invoiced`/`pending`/`settled` on this column now assert `open`/`open`/`paid`. + - Neither column has a `CHECK` (`M20261001_user_badges.hs`: both are plain `TEXT NOT NULL`), so these instances are the only thing pinning the spelling — which is why `BadgeTests`'s column-spelling test covers all three enums. + - `PaymentStatus`'s `textDecode` cannot round-trip `PSFailed`: the failure text is `payments.exception`, a column of its own, so `textDecode "failed"` returns an empty one. Encoding is total, and that is the only direction anything uses today. A future reader of a failed payment must select `exception` and fill it in. + - `BadgePurchaseStatus` is untouched — upstream still declares it, and its codec is A2's as written. + - Upstream's draft `BadgePurchase` and the `import Simplex.Chat.Badges hiding (BadgePurchase (..))` that goes with it were **kept**: nothing here needs them gone, `Badges/Types.hs` is full of unused drafts already, and keeping them makes both the module's unused `UserId`/`C` imports live and our diff against `origin/badges` smaller. Our `UserBadge`/`UserBadgeState` still replace upstream's draft `UserBadgeState`, because they are the shipped projection behind `APIGetBadgeState` and `CRBadgeState`. + - `BadgeTests` imports `Simplex.Chat.PaymentService hiding (PaymentStatus (..))`: `BadgePurchaseStatus` and `PaymentStatus` both have a `PSFailed`, and that module spells out both vocabularies. The payment statuses are reached through the qualified `PT` alias. + ## 10. End-to-end verification After F5: