fix incorrect balance push notification

This commit is contained in:
Devin Zuczek
2026-08-05 15:27:13 -04:00
parent a986d012f5
commit 1f615bab4f
3 changed files with 97 additions and 26 deletions
+32 -25
View File
@@ -203,23 +203,30 @@ async function pushConsumableAdded(
* Push a StorefrontBalanceUpdate to a player after their balance changes, mirroring the * Push a StorefrontBalanceUpdate to a player after their balance changes, mirroring the
* reference's * reference's
* `HubSendToPlayer(accountID, NotifFrame(StorefrontBalanceUpdate, {Balance, CurrencyType, BalanceType}))`. * `HubSendToPlayer(accountID, NotifFrame(StorefrontBalanceUpdate, {Balance, CurrencyType, BalanceType}))`.
* The client applies it to the shown balance so a purchase debit reflects immediately, * The client applies it to the shown balance so a purchase reflects immediately, without
* without waiting for a `GET /balance` re-fetch. `Balance` is the resulting total in that * waiting for a `GET /balance` re-fetch.
* currency (not the delta), `BalanceType` is -2 (account-wide, all platforms). Best-effort: *
* a hub failure is logged and swallowed, since the balance change has already committed. * `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".
*
* `BalanceType` is -2 (account-wide, all platforms). Best-effort: a hub failure is logged
* and swallowed, since the balance change has already committed.
*/ */
async function pushBalanceUpdate( async function pushBalanceUpdate(
c: Context<App>, c: Context<App>,
accountId: number, accountId: number,
currencyType: number, currencyType: number,
balance: number change: number
): Promise<void> { ): Promise<void> {
try { try {
await c.env.RECFLARE_NOTIFICATIONS_HUB.getByName(HUB_INSTANCE).notifyPlayer( await c.env.RECFLARE_NOTIFICATIONS_HUB.getByName(HUB_INSTANCE).notifyPlayer(
accountId, accountId,
NotificationType.StorefrontBalanceUpdate, NotificationType.StorefrontBalanceUpdate,
{ {
Balance: balance, Balance: change,
CurrencyType: currencyType, CurrencyType: currencyType,
BalanceType: ALL_PLATFORMS, BalanceType: ALL_PLATFORMS,
} }
@@ -1012,7 +1019,8 @@ const app = new Hono<App>({ strict: false })
'still matches, debits the buyer atomically, grants the item (into the inventory or', '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', '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', 'player, but the caller always pays. `Balance` in the response is the CHANGE (negated',
'price), not the new total. Pushes a StorefrontBalanceUpdate socket notification.', 'price), not the new total. Pushes a StorefrontBalanceUpdate socket frame carrying the',
'same change, which the client ADDS to the balance it is showing.',
].join(' '), ].join(' '),
security: AUTHED, security: AUTHED,
requestBody: jsonBody(BuyItemRequest, 'The item, currency, price, and optional Gift'), requestBody: jsonBody(BuyItemRequest, 'The item, currency, price, and optional Gift'),
@@ -1133,11 +1141,11 @@ const app = new Hono<App>({ strict: false })
) )
) )
// Push the buyer's new (reduced) balance over the socket so their client updates the // Push the debit over the socket so the buyer's client updates the shown total
// shown total immediately — the buyer (`id`) is who was debited, in the currency they // immediately — the buyer (`id`) is who was charged, in the currency they spent. The
// spent. Best-effort; the HTTP response still carries the change either way. // frame carries the CHANGE, so a purchase is negative. Best-effort; the HTTP response
const newBalance = await getBalance(c.env.DB, id, currencyType as number, startingTokens) // carries the same change either way.
await pushBalanceUpdate(c, id, currencyType as number, newBalance) await pushBalanceUpdate(c, id, currencyType as number, -price.Price)
// The response mirrors a captured real buyItem: `Balance` is the change applied (the // 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 // negated price), not the resulting balance (the client reads its new total from
@@ -1206,7 +1214,9 @@ const app = new Hono<App>({ strict: false })
'its stored `Price`, debits the buyer and pays the creator that price in', 'its stored `Price`, debits the buyer and pays the creator that price in',
'RecCenterTokens (a free invention moves nothing), records ownership in', 'RecCenterTokens (a free invention moves nothing), records ownership in',
'`inventory_invention`, and returns the invention alongside the buyers resulting', '`inventory_invention`, and returns the invention alongside the buyers resulting',
'balance. Both players get a StorefrontBalanceUpdate push when tokens moved.', 'balance. When tokens moved, both players get a StorefrontBalanceUpdate push carrying',
'their CHANGE (the buyers negative, the creators positive), which the client adds to',
'the balance it is showing — unlike this response body, which replaces it.',
'A GET because that is how the client sends it.', 'A GET because that is how the client sends it.',
].join(' '), ].join(' '),
security: AUTHED, security: AUTHED,
@@ -1294,30 +1304,27 @@ const app = new Hono<App>({ strict: false })
// creator who had never touched their balance would otherwise have the row created // creator who had never touched their balance would otherwise have the row created
// here and lose their starting tokens forever. // here and lose their starting tokens forever.
await ensureStartingBalances(c.env.DB, invention.CreatorPlayerId, startingTokens) await ensureStartingBalances(c.env.DB, invention.CreatorPlayerId, startingTokens)
const creatorBalance = await creditCurrency( await creditCurrency(
c.env.DB, c.env.DB,
invention.CreatorPlayerId, invention.CreatorPlayerId,
CurrencyType.RecCenterTokens, CurrencyType.RecCenterTokens,
price, price,
startingTokens startingTokens
) )
// The creator is a different, probably-online player: push their new total so a // The creator is a different, probably-online player: push the payout so a sale
// sale lands on their shown balance without a re-fetch. Best-effort, as everywhere. // lands on their shown balance without a re-fetch. Positive, because the frame
await pushBalanceUpdate( // carries the change. Best-effort, as everywhere.
c, await pushBalanceUpdate(c, invention.CreatorPlayerId, CurrencyType.RecCenterTokens, price)
invention.CreatorPlayerId,
CurrencyType.RecCenterTokens,
creatorBalance
)
} }
// Unlike buyItem — whose `Balance` is the change applied — the reference server // 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 // answers this one with the RESULTING total (a first read seeds the buyer's starting
// grant, as everywhere else). // grant, as everywhere else). The socket frame below is the other way round: the HTTP
// body REPLACES the shown balance, the push ADDS to it.
const balance = await getBalance(c.env.DB, id, CurrencyType.RecCenterTokens, startingTokens) const balance = await getBalance(c.env.DB, id, CurrencyType.RecCenterTokens, startingTokens)
// A free invention moved nothing, so there is no balance to push for it. // A free invention moved nothing, so there is no change to push for it.
if (price > 0) { if (price > 0) {
await pushBalanceUpdate(c, id, CurrencyType.RecCenterTokens, balance) await pushBalanceUpdate(c, id, CurrencyType.RecCenterTokens, -price)
} }
return c.json({ return c.json({
BalanceUpdateResponse: { BalanceUpdateResponse: {
@@ -701,6 +701,7 @@ describe('econ endpoints', () => {
test('POST /api/storefronts/v2/buyItem debits, grants the item, and hands back a gift box', async () => { test('POST /api/storefronts/v2/buyItem debits, grants the item, and hands back a gift box', async () => {
// Account 20: fresh, so its first balance touch grants the 10000 default. // Account 20: fresh, so its first balance touch grants the 10000 default.
await drainFrames()
const res = await exports.default.fetch(`${ORIGIN}/api/storefronts/v2/buyItem`, { const res = await exports.default.fetch(`${ORIGIN}/api/storefronts/v2/buyItem`, {
method: 'POST', method: 'POST',
headers: { ...(await bearer('20')), 'Content-Type': 'application/json' }, headers: { ...(await bearer('20')), 'Content-Type': 'application/json' },
@@ -728,6 +729,16 @@ describe('econ endpoints', () => {
expect(gift.AvatarItemDesc).not.toBe('') expect(gift.AvatarItemDesc).not.toBe('')
expect(gift.Id).toBeGreaterThan(0) expect(gift.Id).toBeGreaterThan(0)
// The socket frame carries the same change the response does — the client adds it to
// the balance it is showing, so the resulting total here would double-count the 9550.
expect(await drainFrames()).toEqual([
{
accountId: 20,
notificationType: STOREFRONT_BALANCE_UPDATE,
payload: { Balance: -450, CurrencyType: 2, BalanceType: -2 },
},
])
// The balance endpoint reflects the debit (this is the resulting total, 10000 - 450). // 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`, { const bal = await exports.default.fetch(`${ORIGIN}/api/storefronts/v4/balance/2`, {
headers: await bearer('20'), headers: await bearer('20'),
@@ -992,6 +1003,26 @@ describe('econ endpoints', () => {
expect(list.every((i) => i.FriendlyName !== 'Bowtie (White)')).toBe(true) expect(list.every((i) => i.FriendlyName !== 'Bowtie (White)')).toBe(true)
}) })
/**
* The StorefrontBalanceUpdate (and other) frames the worker has pushed since the last
* drain, read back off the stub hub in vitest.config.ts. Notification sends are
* best-effort — the worker logs and swallows a hub failure — so this is the only way a
* test sees what was actually pushed.
*/
const drainFrames = async (): Promise<
Array<{ accountId: number; notificationType: number; payload: Record<string, number> }>
> =>
(
env.RECFLARE_NOTIFICATIONS_HUB.getByName('global') as unknown as {
drainFrames(): Promise<
Array<{ accountId: number; notificationType: number; payload: Record<string, number> }>
>
}
).drainFrames()
/** `NotificationType.StorefrontBalanceUpdate` in the notify worker's enum. */
const STOREFRONT_BALANCE_UPDATE = 61
// buyInvention is a GET with query params — that is how the client sends it. // buyInvention is a GET with query params — that is how the client sends it.
const buyInvention = async (sub: string, inventionId: number, requestedPrice = 0) => const buyInvention = async (sub: string, inventionId: number, requestedPrice = 0) =>
exports.default.fetch( exports.default.fetch(
@@ -1042,6 +1073,7 @@ describe('econ endpoints', () => {
test('GET /api/storefronts/v2/buyInvention pays the creator the buyers tokens', async () => { test('GET /api/storefronts/v2/buyInvention pays the creator the buyers tokens', async () => {
// Invention 9 costs 250 and was made by account 999. Buying it moves 250 tokens from // Invention 9 costs 250 and was made by account 999. Buying it moves 250 tokens from
// the buyer to that creator — no house cut, so the two sides are equal and opposite. // the buyer to that creator — no house cut, so the two sides are equal and opposite.
await drainFrames()
const res = await buyInvention('51', 9, 250) const res = await buyInvention('51', 9, 250)
expect(res.status).toBe(200) expect(res.status).toBe(200)
const body = (await res.json()) as { BalanceUpdateResponse: { Balance: number } } const body = (await res.json()) as { BalanceUpdateResponse: { Balance: number } }
@@ -1056,6 +1088,22 @@ describe('econ endpoints', () => {
await getBalance(env.DB, 999, CurrencyType.RecCenterTokens, DEFAULT_STARTING_TOKENS) await getBalance(env.DB, 999, CurrencyType.RecCenterTokens, DEFAULT_STARTING_TOKENS)
).toBe(DEFAULT_STARTING_TOKENS + 250) ).toBe(DEFAULT_STARTING_TOKENS + 250)
expect(await getOwnedInventionIds(env.DB, 51)).toEqual([9]) expect(await getOwnedInventionIds(env.DB, 51)).toEqual([9])
// Both sides get a socket frame carrying their CHANGE, not their new total: the client
// ADDS what it receives to the balance it is showing, so a total would have the creator
// reading their own balance plus the payout. Equal and opposite, like the ledger.
expect(await drainFrames()).toEqual([
{
accountId: 999,
notificationType: STOREFRONT_BALANCE_UPDATE,
payload: { Balance: 250, CurrencyType: CurrencyType.RecCenterTokens, BalanceType: -2 },
},
{
accountId: 51,
notificationType: STOREFRONT_BALANCE_UPDATE,
payload: { Balance: -250, CurrencyType: CurrencyType.RecCenterTokens, BalanceType: -2 },
},
])
}) })
test('GET /api/storefronts/v2/buyInvention rejects a stale price and an unaffordable one', async () => { test('GET /api/storefronts/v2/buyInvention rejects a stale price and an unaffordable one', async () => {
+17 -1
View File
@@ -14,6 +14,12 @@ export default defineConfig({
// isolated test, so provide a minimal stub exposing the same NotificationsHub // isolated test, so provide a minimal stub exposing the same NotificationsHub
// RPC surface — enough for the runtime to start and for notification sends to // RPC surface — enough for the runtime to start and for notification sends to
// no-op. // no-op.
//
// The stub RECORDS what it was sent (`drainFrames`) rather than discarding it.
// Pushes are best-effort and swallow their own errors, so a frame carrying the
// wrong payload is otherwise invisible here — which is exactly how
// StorefrontBalanceUpdate shipped with the resulting total in a field the
// client adds to what it is already showing.
workers: [ workers: [
{ {
name: 'notify', name: 'notify',
@@ -24,8 +30,18 @@ export default defineConfig({
script: ` script: `
import { DurableObject } from 'cloudflare:workers' import { DurableObject } from 'cloudflare:workers'
export class NotificationsHub extends DurableObject { export class NotificationsHub extends DurableObject {
async notifyPlayer() { return { delivered: 0, queued: true } } frames = []
async notifyPlayer(accountId, notificationType, payload) {
this.frames.push({ accountId, notificationType, payload })
return { delivered: 0, queued: true }
}
async broadcast() { return { delivered: 0 } } async broadcast() { return { delivered: 0 } }
/** Everything pushed since the last call, then forget it. */
async drainFrames() {
const drained = this.frames
this.frames = []
return drained
}
} }
export default { fetch() { return new Response('ok') } } export default { fetch() { return new Response('ok') } }
`, `,