diff --git a/CLAUDE.md b/CLAUDE.md index 5ba82b9..6efa49a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -121,16 +121,28 @@ inconsistency here without checking the client first. the owner whether to load the latest or the published version and resolves it from the `/subrooms/:sid/saves` list — the matchmake call is identical either way. Don't make this server-side: it would put two people in one instance on different versions. -- Every `StorefrontBalance*` socket frame (`econ` → `notify` hub) is ADDITIVE: the client - ADDS the frame's `Balance` to the total it is already showing. That includes - `StorefrontBalancePurchase`, whose `Delta`/`BalanceAddType` fields make it look like an - idempotent "here is your new total" frame — it isn't, and the client never applies - `Delta` itself. So never send a total, and never push a frame to the player who is - reading the HTTP response for the same change: they apply both. A storefront purchase - (`/api/storefronts/v2/buyItem`) therefore pushes NOTHING — the buyer applies the body's - `Balance` (the negated price) — and `buyInvention` pushes only the CREATOR's payout, not - the buyer's debit. Pushing the resulting total on a buy showed 33,200 tokens to a player - who spent 900 of 17,500 (the correct 16,600, twice); pushing the change debits twice. +- A balance lives in a `(CurrencyType, Platform)` BUCKET and the client shows the SUM of the + buckets, so `Platform` is a balance's identity, not a label. This server uses exactly one + bucket per currency — `ALL_PLATFORMS`, -2 `NonPurchasedNotUsableInP2P` — and every surface + must name it: the balance DTO (`econ`: `GET /api/storefronts/v4/balance/:type`), the + `BalanceType` the storefront bodies echo, and the `Platform` on every `StorefrontBalance*` + socket frame. Two traps, which produced two "balance doubling" bugs that both looked like + the frames being additive when they are not: + - Each frame SETS the bucket it names to an absolute value — `Balance` is the RESULTING + TOTAL, never the change (`StorefrontBalancePurchase`'s `Delta`/`BalanceAddType` are + display-only; the client logs them and stores `Balance` outright). Send a change and the + balance becomes that change. Being absolute, a frame is idempotent: re-sending one, or + racing a `GET /balance`, cannot drift the total, so the player reading the HTTP response + for the same change gets a frame too. + - The bucket key on the wire is `Platform`. The client's property is named `BalanceType` + but carries a `[DataMember]` rename, and its decoder drops unknown members silently, so + a frame saying `BalanceType` lands in `Platform` 0 (`SteamPurchased`) and adds a phantom + balance to the real one — 10,000 tokens + a 250 reward read 20,250. Sending a real-but- + different platform does the same: `Platform: RecNet` on a buy showed 34,100 to a player + who spent 900 of 17,500, then 33,200 once the body's -900 reached the true bucket. + The payload shapes are recovered from the client's own decoder in + `apps/notify/src/notification-payloads.ts` — build frames against those interfaces (econ + does) so a renamed key fails the build instead of silently vanishing on the wire. - Accessibility is sent as the `RoomAccessibility` enum NAME on `rooms` `PUT /rooms/:id/subrooms/:sid/accessibility` (`accessibility=Private`), not the ordinal the room-level `/rooms/:id/accessibility` takes. The enum has five members diff --git a/apps/econ/src/balance-db.ts b/apps/econ/src/balance-db.ts index 242d6e2..dd6e3a9 100644 --- a/apps/econ/src/balance-db.ts +++ b/apps/econ/src/balance-db.ts @@ -1,3 +1,5 @@ +import { BalancePlatform } from '../../notify/src/notification-payloads' + /** * Currency balances on the shared `recflare` D1 database. * @@ -91,75 +93,23 @@ export function startingBalances( } /** - * The client's `Platform` enum (obfuscated `DEKBHBDENBA`) — WHERE a balance came from, - * carried on the balance DTO and on the `StorefrontBalance*` socket frames. The store - * platforms (Steam … Pico) mean tokens bought with real money there; the negative and - * 100+ members are the "not purchased" kinds, split by whether they may be spent - * player-to-player. + * The ONE balance bucket this server uses: `NonPurchasedNotUsableInP2P` (-2). * - * We sell nothing, so only one of these is ever on the wire from us: balances read back as - * `NonPurchasedNotUsableInP2P` (see `ALL_PLATFORMS`). The rest is recorded for when a frame - * from a real capture has to be read. - */ -export const Platform = { - NonPurchasedNotUsableInP2P: -2, - NonPurchasedDefault: -1, - Steam: 0, - Oculus: 1, - PlayStation: 2, - Microsoft: 3, - RecNet: 4, - IOS: 5, - GooglePlay: 6, - Pico: 8, - PlayStationNonPurchasedP2P: 100, - NonPlayStationNonPurchasedP2P: 101, - NonPurchasedEarnedByP2P: 1000, -} as const - -/** - * `Platform` in the client's balance DTO: -2, `NonPurchasedNotUsableInP2P`. We don't track - * per-platform wallets (real RecNet did, for platform-purchased tokens), and everything we - * hand out is minted rather than bought, so one account-wide balance answers for all of them. - */ -export const ALL_PLATFORMS: number = Platform.NonPurchasedNotUsableInP2P - -/** - * The client's `BalanceAddType` enum (obfuscated `EPJJLKAOOLD`) — WHY a balance changed, - * tagged onto a `StorefrontBalance*` frame. Log-only: the client shows/records the reason - * but never derives the balance from it, so a wrong value here is cosmetic, not a wrong - * number on screen. + * The client keys a balance by `(CurrencyType, Platform)` and shows the SUM of the buckets, + * so which Platform a balance is reported under is not cosmetic — it is the bucket's + * identity. Everything we hand out is minted rather than bought, and we track no + * per-platform wallets (real RecNet did, for tokens paid for on each store), so one + * account-wide bucket per currency answers for all of them. * - * Nothing here sends one today: the `StorefrontBalanceUpdate` frames this worker pushes - * carry only `{ Balance, CurrencyType, BalanceType }`, and the purchase paths push no frame - * at all (see `pushBalanceUpdate` in econ.app.ts). Recorded because the reasons a balance - * moves (challenges, level-ups, creator payouts, manual grants) are paths this worker will - * grow into, and for reading a frame out of a real capture. + * Every surface that names the bucket must name THIS one: the balance DTO's `Platform`, the + * `BalanceType` the storefront HTTP bodies echo, and the `Platform` on every + * `StorefrontBalance*` socket frame. Naming a second one there invents a balance the client + * adds to the real total — see the frame rule in econ.app.ts. + * + * The enum itself lives in the notify worker's `notification-payloads.ts`, recovered from + * the client's decoder, rather than being duplicated here. */ -export const BalanceAddType = { - Invalid: 0, - DirectBalanceWithMultiplier: 1, - FromGiftBox: 2, - NUXChallenge: 10, - AllNUXChallenges: 11, - DailyChallenge: 100, - AllDailyChallenges: 101, - FinishActivity: 200, - RecRoyaleMatchFinished: 250, - ChecklistCredit: 303, - WonGame: 1000, - LostGame: 1001, - WonGameRateLimited: 1002, - WonGamePartial: 1003, - LevelUp: 1100, - Registered: 1200, - CreatorReward: 1300, - CommercePurchase: 1400, - CommercePurchaseRevoked: 1401, - Manual_Refund: 2000, - Manual_Thanks: 2010, - Manual_Apology: 2020, -} as const +export const ALL_PLATFORMS: BalancePlatform = BalancePlatform.NonPurchasedNotUsableInP2P /** Schema DDL (mirror of migrations 0001_balance.sql) — also used to build the table in tests. */ export const BALANCE_SCHEMA_DDL: string[] = [ diff --git a/apps/econ/src/econ.app.ts b/apps/econ/src/econ.app.ts index 122360b..f7d4b35 100644 --- a/apps/econ/src/econ.app.ts +++ b/apps/econ/src/econ.app.ts @@ -20,8 +20,11 @@ import { validateAndGetAccountId } from '@repo/jwt' // Imported directly rather than copied: these are plain D1 helpers with no bindings of // their own, and buyInvention has to read the very rows `api` writes. import { getInventionById, toSaveResult } from '../../api/src/inventions-db' -// The notification-type ids the hub carries (owned by the `notify` worker). Imported -// as a value — the enum has no runtime dependencies. +// The notification-type ids the hub carries, and the payload shapes recovered from the +// client's own decoder (both owned by the `notify` worker). Imported rather than copied so +// the frames this worker builds are typed by the shapes the client actually parses — a +// wrong or renamed key (see the `Platform`/`BalanceType` trap) fails the build here. +import { BalanceAddType } from '../../notify/src/notification-payloads' import { NotificationType } from '../../notify/src/notification-types' import adCarouselItems from '../static/ad-carousel-items.json' import defaultAvatarItems from '../static/default-avatar-items.json' @@ -87,6 +90,10 @@ import { claimReward } from './reward-db' import type { Context } from 'hono' import type { GiftContent, Progression, StoredGift, XpGrant } from '@repo/domain' +import type { + BalanceResponsePayload, + PurchaseBalanceModificationPayload, +} from '../../notify/src/notification-payloads' import type { Avatar } from './avatar-db' import type { ConsumeResult } from './consumables-db' import type { App } from './context' @@ -223,46 +230,63 @@ async function pushConsumableAdded( } /** - * Push a StorefrontBalanceUpdate to a player after their balance changes, mirroring the - * reference's - * `HubSendToPlayer(accountID, NotifFrame(StorefrontBalanceUpdate, {Balance, CurrencyType, BalanceType}))`. - * The client applies it to the shown balance so a change reflects immediately, without - * waiting for a `GET /balance` re-fetch. + * THE BALANCE-FRAME RULE, which both balance bugs came from getting wrong. * - * `Balance` is the CHANGE — negative for a debit, positive for a payout — not the - * resulting total. The client ADDS what it receives to the balance it is already showing, - * so sending the total made a 10,000-token player who earned 250 read 20,250: their own - * balance plus the new total. That also makes this frame non-idempotent, so push exactly - * once per change and never re-send it as a "refresh". + * The client holds a balance PER `(CurrencyType, Platform)` bucket and shows the SUM of the + * buckets. Every `StorefrontBalance*` frame is an absolute SET of the one bucket it names — + * not a change to apply — so: * - * Every StorefrontBalance* frame is additive this way, StorefrontBalancePurchase included - * — it is NOT the idempotent "here is your new total" frame it looks like. Sending the - * total on a purchase doubled the buyer's balance on screen (17,500 − 900 spent showed - * 33,200: the correct 16,600 twice over), which is why the purchase paths below push - * nothing to the buyer at all. + * 1. `Balance` is the RESULTING TOTAL. Sending the change sets the bucket TO that change. + * 2. The bucket key on the wire is `Platform`. The client's property is called + * `BalanceType` but carries a `[DataMember]` rename, and its decoder drops unknown + * members in silence — so a frame that says `BalanceType` lands in `Platform` 0, + * `SteamPurchased`, and creates a SECOND bucket that is added to the real one forever. + * 3. That bucket must be the same one `GET /api/storefronts/v4/balance/:type` reports, + * `ALL_PLATFORMS`. One account-wide bucket per currency is the whole model here; a + * frame naming any other Platform is a phantom balance, not a per-store nicety. * - * So: a frame goes to a player whose client is NOT reading this response — the invention - * creator collecting a payout. The caller learns their own new balance from the HTTP body - * and must not also be pushed one, or they apply both. + * Both live bugs were rule 2 or 3, and both looked like the frame being "additive": + * - A player who earned 250 on 10,000 read 20,250 — `BalanceType: -2` was dropped, so the + * total landed in a phantom Steam bucket beside the real one. + * - A player who spent 900 of 17,500 read 34,100, then 33,200 once the purchase response's + * -900 reached the real bucket — same phantom bucket, this time from `Platform: RecNet`. + * Neither was additivity: the totals were right, the bucket was wrong. Frames as specified + * here are idempotent, so re-sending one or racing a `GET /balance` cannot drift the total. * - * `BalanceType` is -2 (account-wide, all platforms). Best-effort: a hub failure is logged - * and swallowed, since the balance change has already committed. + * See apps/notify/src/notification-payloads.ts for the payload shapes this is recovered + * from — the interfaces there type these calls, so a wrong key is now a build error. + */ + +/** + * Push a StorefrontBalanceUpdate (61) — "your balance in this bucket is now X" — after a + * player's balance changes for a reason that is not their own purchase. `balance` is their + * resulting TOTAL in that currency, per the rule above. + * + * A player who is reading the HTTP response for the same change gets this too: it sets the + * bucket to the same total the body reports, so the two agree rather than compound. Pushing + * it is what saves them a `GET /balance` re-fetch. + * + * Best-effort: a hub failure is logged and swallowed, since the change has already committed. */ async function pushBalanceUpdate( c: Context, accountId: number, currencyType: number, - change: number + balance: number ): Promise { + // `satisfies` rather than a type annotation: the hub takes a Record, and + // an interface (unlike an inferred object type) has no implicit index signature to match + // it. This still checks every key against the shape the client's decoder parses. + const payload = { + Balance: balance, + CurrencyType: currencyType, + Platform: ALL_PLATFORMS, + } satisfies BalanceResponsePayload try { await c.env.RECFLARE_NOTIFICATIONS_HUB.getByName(HUB_INSTANCE).notifyPlayer( accountId, NotificationType.StorefrontBalanceUpdate, - { - Balance: change, - CurrencyType: currencyType, - BalanceType: ALL_PLATFORMS, - } + payload ) } catch (err) { logger.error('failed to push StorefrontBalanceUpdate notification', { @@ -272,6 +296,45 @@ async function pushBalanceUpdate( } } +/** + * Push a StorefrontBalancePurchase (62) — the frame the reference sends when the balance + * moved because the player BOUGHT something, as opposed to the plain update above. Same + * absolute-set semantics: `balance` is the resulting total. + * + * `Delta` (the negated price) and `BalanceAddType` are display/telemetry only — the client + * logs them and then stores `Balance` outright, so a correct `Delta` beside a stale + * `Balance` still leaves the player's balance wrong. `Platform` is `ALL_PLATFORMS`, NOT + * `RecNetPurchased`: it has to name the bucket `GET /balance` reports, and sending RecNet + * here is exactly what doubled a buyer's tokens on screen. Best-effort, as above. + */ +async function pushBalancePurchase( + c: Context, + accountId: number, + currencyType: number, + delta: number, + balance: number +): Promise { + const payload = { + BalanceAddType: BalanceAddType.CommercePurchase, + Delta: delta, + Balance: balance, + Platform: ALL_PLATFORMS, + CurrencyType: currencyType, + } satisfies PurchaseBalanceModificationPayload + try { + await c.env.RECFLARE_NOTIFICATIONS_HUB.getByName(HUB_INSTANCE).notifyPlayer( + accountId, + NotificationType.StorefrontBalancePurchase, + payload + ) + } catch (err) { + logger.error('failed to push StorefrontBalancePurchase notification', { + accountId, + error: err instanceof Error ? err.message : String(err), + }) + } +} + /** * Project a stored avatar into the public render subset returned by * `GET /api/avatar/v2/:id` — the fields needed to draw another player's avatar @@ -1713,9 +1776,9 @@ const app = new Hono({ strict: false }) 'still matches, debits the buyer atomically, grants the item (into the inventory or', 'consumable table), and returns a gift box. A `Gift` block routes the item to another', 'player, but the caller always pays. `Balance` in the response is the CHANGE (negated', - 'price), not the new total. No balance socket frame is pushed: the buyer is the caller,', - 'and the client ADDS any StorefrontBalance* frame on top of the change it already', - 'applied from this body — pushing the total here doubled the balance on screen.', + 'price), not the new total. Pushes a StorefrontBalancePurchase socket frame that SETS the', + 'buyer’s account-wide bucket to the RESULTING total, so the frame, this body and a', + '`GET /balance` re-fetch all agree (`Delta` there is display-only).', ].join(' '), security: AUTHED, requestBody: jsonBody(BuyItemRequest, 'The item, currency, price, and optional Gift'), @@ -1803,13 +1866,13 @@ const app = new Hono({ strict: false }) message ) - // NO balance frame is pushed here, deliberately. The buyer is the caller: they get - // the debit from the response below (and re-read `GET /balance`), and the client ADDS - // any StorefrontBalance* frame on top of that — including StorefrontBalancePurchase, - // which is additive like the rest despite carrying a `Delta` field. Pushing the - // resulting total doubled the shown balance (17,500 − 900 read 33,200 = 16,600 twice); - // pushing the change debited it twice. Only a player who is NOT reading this response - // needs a frame — see the invention creator's payout in buyInvention. + // Push the spend to the buyer (`id` — the caller is who was charged) so their client + // updates without waiting for a `GET /balance` re-fetch. StorefrontBalancePurchase + // SETS the account-wide bucket to the resulting total read back from D1, so it agrees + // with both the response body below and any re-fetch instead of compounding with them + // — see the frame rule above pushBalanceUpdate. Best-effort. + const newBalance = await getBalance(c.env.DB, id, currencyType as number, startingTokens) + await pushBalancePurchase(c, id, currencyType as number, -price.Price, newBalance) // The response mirrors a captured real buyItem: `Balance` is the change applied (the // negated price), not the resulting balance (the client reads its new total from @@ -1879,9 +1942,9 @@ const app = new Hono({ strict: false }) 'its stored `Price`, debits the buyer and pays the creator that price in', 'RecCenterTokens (a free invention moves nothing), records ownership in', '`inventory_invention`, and returns the invention alongside the buyer’s resulting', - 'balance. When tokens moved, the CREATOR gets a StorefrontBalanceUpdate push carrying', - 'their payout, which their client adds to the balance it is showing. The buyer gets no', - 'push: this response body already replaces the balance their client shows.', + 'balance. When tokens moved, both players get a socket push carrying their RESULTING', + 'total — the buyer a StorefrontBalancePurchase, the CREATOR a StorefrontBalanceUpdate —', + 'which sets the account-wide bucket their client shows, agreeing with this body.', 'A GET because that is how the client sends it.', ].join(' '), security: AUTHED, @@ -1969,26 +2032,35 @@ const app = new Hono({ strict: false }) // creator who had never touched their balance would otherwise have the row created // here and lose their starting tokens forever. await ensureStartingBalances(c.env.DB, invention.CreatorPlayerId, startingTokens) - await creditCurrency( + const creatorBalance = await creditCurrency( c.env.DB, invention.CreatorPlayerId, CurrencyType.RecCenterTokens, price, startingTokens ) - // The creator is a different, probably-online player: push the payout so a sale - // lands on their shown balance without a re-fetch. Positive, because the frame - // carries the change. Best-effort, as everywhere. - await pushBalanceUpdate(c, invention.CreatorPlayerId, CurrencyType.RecCenterTokens, price) + // The creator is a different, probably-online player with no response to read: + // push the sale so it lands on their shown balance without a re-fetch. The frame + // carries their resulting TOTAL (what `creditCurrency` returns), not the payout — + // sending the payout would set their whole balance to it. A plain update rather + // than a purchase frame: they sold, they didn't buy. Best-effort, as everywhere. + await pushBalanceUpdate( + c, + invention.CreatorPlayerId, + CurrencyType.RecCenterTokens, + creatorBalance + ) } // Unlike buyItem — whose `Balance` is the change applied — the reference server // answers this one with the RESULTING total (a first read seeds the buyer's starting - // grant, as everywhere else). That total REPLACES the balance the buyer's client is - // showing, which is why the buyer gets no socket frame: a StorefrontBalance* push is - // ADDED to what the client shows, so one here would debit them a second time on - // screen. The creator, whose client never sees this response, is pushed above. + // grant, as everywhere else). The buyer's frame carries that same total, so the body + // and the push land the client on one number. const balance = await getBalance(c.env.DB, id, CurrencyType.RecCenterTokens, startingTokens) + // A free invention moved nothing, so there is no purchase to report. + if (price > 0) { + await pushBalancePurchase(c, id, CurrencyType.RecCenterTokens, -price, balance) + } return c.json({ BalanceUpdateResponse: { Balance: balance, diff --git a/apps/econ/src/test/integration/api.test.ts b/apps/econ/src/test/integration/api.test.ts index e192956..4b21d80 100644 --- a/apps/econ/src/test/integration/api.test.ts +++ b/apps/econ/src/test/integration/api.test.ts @@ -777,12 +777,27 @@ describe('econ endpoints', () => { expect(gift.AvatarItemDesc).not.toBe('') expect(gift.Id).toBeGreaterThan(0) - // A purchase pushes NO balance frame. The buyer is the caller: they apply the change - // from the body above, and the client ADDS any StorefrontBalance* frame on top of it — - // StorefrontBalancePurchase included, despite its `Delta` field. Pushing the resulting - // total is what made a live 17,500-token player read 33,200 after spending 900 (16,600 - // twice over); pushing the change would debit them twice instead. - expect(await drainFrames()).toEqual([]) + // A purchase pushes StorefrontBalancePurchase, which SETS one (CurrencyType, Platform) + // bucket to an absolute value: `Balance` is the resulting total (10000 - 450) and `Delta` + // is display-only. The bucket key is `Platform`, and it MUST be the -2 the balance + // endpoint reports below — the client sums its buckets, so a frame naming any other + // platform (or spelling the key `BalanceType`, which the client's decoder drops) invents + // a second balance beside the real one. That is what showed a live player 34,100 tokens + // after spending 900 of 17,500, then 33,200 once the body's -900 landed. + expect(await drainFrames()).toEqual([ + { + accountId: 20, + notificationType: NotificationType.StorefrontBalancePurchase, + payload: { + // 1400 = CommercePurchase; -2 = NonPurchasedNotUsableInP2P, the only bucket we use. + BalanceAddType: 1400, + Delta: -450, + Balance: 9550, + Platform: -2, + CurrencyType: 2, + }, + }, + ]) // The balance endpoint reflects the debit (this is the resulting total, 10000 - 450). const bal = await exports.default.fetch(`${ORIGIN}/api/storefronts/v4/balance/2`, { @@ -1131,16 +1146,31 @@ describe('econ endpoints', () => { ).toBe(DEFAULT_STARTING_TOKENS + 250) expect(await getOwnedInventionIds(env.DB, 51)).toEqual([9]) - // Only the CREATOR gets a socket frame, carrying their CHANGE rather than their new - // total: the client ADDS what it receives to the balance it is showing, so a total would - // have them reading their own balance plus the payout. The buyer gets none — the - // response body already replaced the balance their client shows, and a frame on top of - // it would debit them twice on screen. + // Both sides get a frame carrying their RESULTING TOTAL, into the same -2 bucket the + // balance endpoint reports — a StorefrontBalance* push SETS that bucket, so sending the + // change (250 / -250) would set their whole balance to it. The creator sold, so theirs is + // a plain update; the buyer bought, so theirs is a purchase frame with a display-only + // `Delta`. Note the key is `Platform`: the client renames `BalanceType` away and drops it. expect(await drainFrames()).toEqual([ { accountId: 999, notificationType: NotificationType.StorefrontBalanceUpdate, - payload: { Balance: 250, CurrencyType: CurrencyType.RecCenterTokens, BalanceType: -2 }, + payload: { + Balance: DEFAULT_STARTING_TOKENS + 250, + CurrencyType: CurrencyType.RecCenterTokens, + Platform: -2, + }, + }, + { + accountId: 51, + notificationType: NotificationType.StorefrontBalancePurchase, + payload: { + BalanceAddType: 1400, + Delta: -250, + Balance: DEFAULT_STARTING_TOKENS - 250, + Platform: -2, + CurrencyType: CurrencyType.RecCenterTokens, + }, }, ]) })