diff --git a/ads.js b/ads.js index 8d67468..19632a4 100644 Binary files a/ads.js and b/ads.js differ diff --git a/nas.js b/nas.js index 207879e..5d21c43 100644 --- a/nas.js +++ b/nas.js @@ -100,11 +100,22 @@ async function readServed(nasAdId) { return { assigned, remaining, served, clicks: Number(rows[0].hits) || 0 }; } -// stop a syndicated ad (budget spent, paused, or expired) — remaining:0 halts serving. +// Stop a syndicated ad (budget spent, paused, or expired). NAS has no stop flag: its +// serving query only picks rows with remaining>0, so zeroing that counter is the only +// way to halt an ad. +// +// THE TRAP (found 2026-09-17): served is derived as assigned-remaining, so zeroing also +// makes a stopped ad read back as 100% DELIVERED. That inflated every stopped campaign's +// reported delivery, and worse, reconcileNas charges members for the difference — so +// pausing a campaign could bill for impressions that were never served. A naturally +// exhausted ad also ends at remaining=0, so the two cases are indistinguishable +// afterwards. The true figure therefore has to be captured BEFORE the stop. async function deactivate(nasAdId) { if (!enabled()) return { skipped: 'flag-off' }; + let served = null; + try { const s = await readServed(nasAdId); if (s) served = s.served; } catch (e) {} await q('UPDATE sponsorads SET remaining=0, EDate=NOW() WHERE ID=?', [Number(nasAdId)]); - return { ok: true }; + return { ok: true, served }; // callers MUST persist this as the final served count } // top up a syndicated ad with more impressions (buy-more-views / reactivate). diff --git a/qa/nas-served.mjs b/qa/nas-served.mjs new file mode 100644 index 0000000..861b000 --- /dev/null +++ b/qa/nas-served.mjs @@ -0,0 +1,102 @@ +// The Network Ad Space served-count trap (found 2026-09-17). +// +// NAS has no stop flag: its serving query only picks rows with remaining>0, so zeroing +// that counter is the only way to halt an ad. But served is derived as +// assigned - remaining, so a STOPPED ad reads back as 100% delivered. Two harms: +// 1. reported delivery was inflated for every stopped campaign; +// 2. reconcileNas CHARGES MEMBER CREDITS off that figure, so pausing a campaign could +// bill someone for impressions that were never served. +// +// Runs entirely against the JSON store with the NAS layer stubbed. Touches no real data. +// node qa/nas-served.mjs +import { createRequire } from 'node:module'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +const require = createRequire(import.meta.url); + +const ok = [], bad = []; +const t = (n, c, extra) => { (c ? ok : bad).push(n + (c || !extra ? '' : ' -> ' + extra)); }; + +const DATA = path.join(os.tmpdir(), 'iap-nas-served-' + Date.now()); +fs.mkdirSync(DATA, { recursive: true }); + +// stub the NAS layer BEFORE ads.js uses it (ads.js holds the module object, so mutating +// its exports is enough) +const nas = require('../nas.js'); +let nasRow = { assigned: 10000, remaining: 7000, hits: 3 }; // 3,000 genuinely served +let deactivated = 0; +nas.enabled = () => true; +nas.impressionsPerCredit = () => 1; +nas.readServed = async () => ({ assigned: nasRow.assigned, remaining: nasRow.remaining, + served: Math.max(0, Math.min(nasRow.assigned, nasRow.assigned - nasRow.remaining)), clicks: nasRow.hits }); +nas.deactivate = async () => { + const served = Math.max(0, Math.min(nasRow.assigned, nasRow.assigned - nasRow.remaining)); + nasRow.remaining = 0; // what the real stop does + deactivated++; + return { ok: true, served }; // the fix: report the TRUE figure captured first +}; +nas.topUp = async () => ({ ok: true }); + +const ads = require('../ads.js'); +ads.init({ dataDir: DATA, chain: null }); + +// ---- 1. stopping an ad must record what it really delivered, not its whole allocation +// Seed the store directly: going through createCampaign would drag in balances and the +// chain, and neither is what this test is about. +const FILE = path.join(DATA, 'campaigns.json'); // NOT ads.json +const id = 9001; +const seed = (status, nasServed) => { + fs.writeFileSync(FILE, JSON.stringify({ + v: 1, nextId: 9002, campaigns: [{ id, owner: 'tester@example.com', memberId: 0, type: 'banner', name: 'served test', + targetUrl: 'https://example.com', imageUrl: 'https://example.com/b.png', width: 728, height: 90, + budget: 10000, spent: 0, accrued: 0, imps: 0, clicks: 0, status, nasAdId: 4242, + nasServed, created: Date.now() }], + burnsPending: [], v: 1, nextId: 9002 + })); + ads.init({ dataDir: DATA, chain: null }); +}; +const store = () => JSON.parse(fs.readFileSync(FILE, 'utf8')); +const findC = () => (store().campaigns || []).find(c => c.id === id); + +seed('active', 0); +t('test campaign seeded', !!findC(), 'missing'); +let c; + +// pause it: the stop path runs, and the TRUE served (3,000) must be what is recorded +await ads.setStatus('tester@example.com', id, 'paused').catch(() => {}); +c = findC(); +t('pausing stopped the NAS ad', deactivated >= 1, String(deactivated)); +t('the recorded served count is the REAL 3,000, not the 10,000 allocation', + c && c.nasServed === 3000, c ? 'nasServed=' + c.nasServed : 'campaign missing'); +t('and it is not the full allocation', !c || c.nasServed !== 10000, c && String(c.nasServed)); + +// ---- 2. a stopped campaign must never be reconciled again (this is the billing guard) +const spentBefore = c ? (c.spent || 0) + (c.accrued || 0) : 0; +await ads.reconcileNas(); +c = findC(); +const spentAfter = c ? (c.spent || 0) + (c.accrued || 0) : 0; +t('a paused campaign is NOT charged after its NAS ad was stopped', + spentAfter === spentBefore, 'before ' + spentBefore + ' after ' + spentAfter); +t('and its served figure did not jump to the allocation', + c && c.nasServed === 3000, c && String(c.nasServed)); + +// ---- 3. an ACTIVE campaign still reconciles normally, so the guard did not break delivery +nasRow = { assigned: 10000, remaining: 5000, hits: 9 }; // 5,000 served now +c = findC(); +if (c) { c.status = 'active'; c.nasServed = 3000; + const s = store(); const i = s.campaigns.findIndex(x => x.id === id); s.campaigns[i] = c; + fs.writeFileSync(FILE, JSON.stringify(s)); ads.init({ dataDir: DATA, chain: null }); } +await ads.reconcileNas(); +c = findC(); +t('an active campaign still reconciles real delivery', c && c.nasServed === 5000, c && String(c.nasServed)); +// The seeded member has no funded balance, so the charge itself cannot land; what matters +// is that reconcile ATTEMPTED it and the campaign was retired as exhausted rather than +// silently continuing to accrue undelivered impressions. +t('reconcile acted on the new impressions and retired the campaign', + c && c.status === 'out', c && 'status=' + c.status); + +console.log('PASS ' + ok.length); +for (const b of bad) console.log('FAIL ' + b); +try { fs.rmSync(DATA, { recursive: true, force: true }); } catch (e) {} +process.exit(bad.length ? 1 : 0);