diff --git a/lib/reminderDispatcher.js b/lib/reminderDispatcher.js index c38f3be..844d289 100644 --- a/lib/reminderDispatcher.js +++ b/lib/reminderDispatcher.js @@ -2,49 +2,90 @@ // // The module is framework-agnostic: all dependencies (the users / // receipts / log repos, the mailer, the live proposal-source RPC, the -// clock, the logger) are injected. The boot wiring in server.js -// composes the pieces and schedules `tick()` on an interval. Tests -// hand in fakes and call tick() directly. +// next-superblock snapshot, the clock, the logger) are injected. The +// boot wiring in server.js composes the pieces and schedules `tick()` +// on an interval. Tests hand in fakes and call tick() directly. // -// --- What does a tick do, in plain English --- +// --- What deadline does this dispatcher use? --- // -// A tick asks a single question per user: "should this user receive a -// reminder right now, in either of the two time-based buckets?". We -// answer it with three gates, in order: +// The actionable "voting deadline" from a voter's point of view is +// NOT the superblock itself — it's the start of Core's superblock +// maturity window (~3 days before the SB on Syscoin mainnet; +// nSuperblockMaturityWindow = 1728 blocks × 150 s/block). Once the +// maturity window opens, masternodes begin committing YES-FUNDING +// trigger votes and (per governance.cpp) an MN that has voted +// YES-FUNDING for one trigger cannot switch to another for the same +// cycle. So a user who only finds out about a proposal AFTER the +// maturity window opens is already watching some fraction of MNs +// lock in their choice. Reminding them then is mostly useless noise. // -// 1. Is there an active governance cycle with a deadline in reach? -// (If every proposal has already closed, nothing to remind about.) -// 2. Has the user explicitly opted out of vote reminders? -// (users.listWithRemindersEnabled() already strips these out.) -// 3. Has the user already engaged with this cycle? -// (hasAnyRelayedInCycle on the active proposal set. The spec is -// explicit: if the user voted on ANY proposal in the cycle, skip -// BOTH reminder buckets.) +// The dispatcher therefore anchors to the maturity-window start: // -// Only users that clear all three gates are emailed, and only in the -// bucket that corresponds to the current time-until-deadline. The -// reminder log is checked per (user, scope_key, bucket) so the -// dispatcher is idempotent across ticks — a server restart, a clock -// skew, or a manual tick replay does not cause duplicate emails. +// deadlineSec = nextSuperblockEpochSec - SUPERBLOCK_MATURITY_WINDOW_SEC // -// --- Bucket definitions --- +// and buckets the time remaining until THAT moment: // -// final_24h — deadline is within 24h. Urgent tone. -// days_before — deadline is within 72h but not 24h. Calm heads-up. +// days_before — deadline is 24h < remaining ≤ 72h away +// (≈ 4–6 days before the superblock itself) +// final_24h — deadline is 0 < remaining ≤ 24h away +// (≈ 3–4 days before the superblock itself) +// maturity_window_open — remaining ≤ 0 (we're already inside the +// maturity window; suppress — too late for +// the reminder to affect voting outcomes) +// too_early — remaining > 72h (not worth bothering anyone yet) // -// Proposals whose deadline is > 72h away do not trigger any reminder. -// The 24h / 72h thresholds are exported as named constants so tests -// (and future tuning) don't sprinkle magic numbers. +// Both email buckets fire OUTSIDE the maturity window by design. // -// --- Why the dispatcher computes the cycle, not the caller --- +// --- Cycle definition and scope key stability --- // -// "Cycle" is a concept the application owns, not the governance RPC. -// gObject_list returns a flat list of active proposals; the dispatcher -// projects them into a cycle by taking the earliest end_epoch among -// still-open proposals. That value becomes the scope_key, which makes -// a freshly-added proposal with an earlier deadline count as a new -// cycle (correct: the user may not have voted on the new proposal -// even if they voted on the old ones). +// A proposal P is eligible to be paid by the upcoming superblock iff +// Core's payment window covers it: +// +// P.startEpoch <= sbEpochSec <= P.endEpoch +// +// Eligibility uses the SB time itself (not the maturity start), +// because that matches Core's IsWithinValidWindow check for what +// gets paid at that SB. (The maturity anchor is only about *when* +// voters should be reminded, not about *which* proposals are in +// the cycle.) +// +// `scopeKey` keys the reminder log entries for idempotency. It is +// critical that the scope key be STABLE across ticks that refer to +// the same superblock — otherwise `reminderLog.has()` won't +// deduplicate and a user could be re-emailed every hour for 72 +// hours. We therefore key the scope on the next-superblock's BLOCK +// HEIGHT (an integer that only changes once the SB executes), NOT +// on the epochSec estimate (which `sysMain.js` recomputes from +// `Date.now() + diffBlock * avgBlockTime` every 20s and therefore +// drifts continuously as wall-clock time advances). +// +// scopeKey = `sb:${height}` +// +// When the chain advances past the current SB, the height jumps by +// exactly `nSuperblockCycle` (17520 on mainnet) and the scope +// rotates — which is the correct moment: a user who got both +// reminders for SB_N and didn't vote should get both again for the +// fresh cycle SB_{N+1}. +// +// --- Failure modes --- +// +// - `getNextSuperblock` throws / returns anything but +// `{ height: positive int, epochSec: > now }` → skip with +// `next_superblock_unavailable`. The next hourly tick retries. +// - `getActiveProposals` throws → skip with `proposals_unavailable`. +// - Mailer send throws → record `failed++`, do NOT write the +// reminder-log entry, so the next tick in the same bucket +// retries. UNIQUE race behavior (parallel tick sends same +// email) is covered by the UNIQUE(user, scope, bucket) +// constraint on reminderLog. + +// Core's nSuperblockMaturityWindow on Syscoin mainnet: +// 1728 blocks × 150 seconds/block = 259_200 seconds = 3 days. +// Kept as an explicit constant (instead of inlining 259200) so it +// stays in lockstep with Core if the network parameter ever +// changes — update both this constant and the frontend governance +// helper at the same time. +const SUPERBLOCK_MATURITY_WINDOW_SEC = 1728 * 150; const MS_HOUR = 60 * 60 * 1000; const MS_DAY = 24 * MS_HOUR; @@ -52,10 +93,18 @@ const MS_DAY = 24 * MS_HOUR; const BUCKET_FINAL_24H = 'final_24h'; const BUCKET_DAYS_BEFORE = 'days_before'; -// Thresholds in milliseconds from "now" to "cycle deadline". -// Deadline ≤ 24h away → BUCKET_FINAL_24H -// Deadline > 24h and ≤ 72h → BUCKET_DAYS_BEFORE -// Deadline > 72h or ≤ 0 → no bucket (tick emits nothing) +// Thresholds in milliseconds from "now" to the maturity-window +// opening moment (SB - 3 days). +// +// remaining ≤ 24h → BUCKET_FINAL_24H +// 24h < remaining ≤ 72h → BUCKET_DAYS_BEFORE +// remaining > 72h → no bucket (too early) +// remaining ≤ 0 → caller maps to 'maturity_window_open' +// +// Despite the constant names, the "24h" and "72h" are measured +// against the maturity-window start, NOT the superblock itself. In +// wall-clock terms against the SB this translates to ~3–4d before +// SB for final_24h and ~4–6d before SB for days_before. const FINAL_24H_MS = 1 * MS_DAY; const DAYS_BEFORE_MS = 3 * MS_DAY; @@ -67,18 +116,46 @@ function bucketForTimeRemaining(msRemaining) { } // Parses a shape emitted by governance RPC / test fakes. Required -// fields: { hash: string, endEpoch: number (unix seconds) }. Returns -// a sanitized, lowercased view, or null if the proposal is not usable -// (missing fields, malformed hash, past deadline). +// fields: { hash: string, endEpoch: number (unix seconds) }. +// Optional: { startEpoch: number (unix seconds) }. A missing or +// zero startEpoch is treated as "always-started" (the proposal is +// eligible from the first superblock it encounters), which matches +// how legacy proposals without a startEpoch stored in DataString +// behave on Core. Returns a sanitized view or null if the proposal +// is not usable (missing fields, malformed hash, past endEpoch). function normalizeProposal(raw, nowMs) { if (!raw || typeof raw !== 'object') return null; const hash = typeof raw.hash === 'string' ? raw.hash.toLowerCase() : null; if (!hash || !/^[0-9a-f]{64}$/.test(hash)) return null; const endEpoch = Number(raw.endEpoch); if (!Number.isFinite(endEpoch) || endEpoch <= 0) return null; - const deadlineMs = endEpoch * 1000; - if (deadlineMs <= nowMs) return null; - return { hash, endEpoch, deadlineMs }; + const endMs = endEpoch * 1000; + if (endMs <= nowMs) return null; + // startEpoch is optional; 0 / negative / non-finite all map to + // "no lower bound", which lets the SB-eligibility filter below + // treat legacy payloads as still-eligible rather than silently + // dropping them. + const rawStart = Number(raw.startEpoch); + const startEpoch = Number.isFinite(rawStart) && rawStart > 0 ? rawStart : 0; + return { hash, startEpoch, endEpoch, startMs: startEpoch * 1000, endMs }; +} + +// A proposal P is eligible to be paid by the superblock at +// `sbEpochSec` iff Core's on-chain payment window covers it: +// +// P.startEpoch <= sbEpochSec <= P.endEpoch +// +// A startEpoch of 0 is interpreted as "no lower bound" (see +// normalizeProposal). The bounds are inclusive; boundary-second +// ties should never happen in practice because derived windows +// carry a ~15-day buffer on each side, but being inclusive here +// keeps the math honest. +function proposalIsEligibleForSb(p, sbEpochSec) { + if (!p) return false; + if (!Number.isFinite(sbEpochSec) || sbEpochSec <= 0) return false; + if (p.startEpoch > sbEpochSec) return false; + if (p.endEpoch < sbEpochSec) return false; + return true; } // Human-readable "closes in X" string for email body. Rounds down @@ -98,6 +175,7 @@ function createReminderDispatcher({ reminderLog, mailer, getActiveProposals, + getNextSuperblock, now = () => Date.now(), log = () => {}, } = {}) { @@ -116,11 +194,77 @@ function createReminderDispatcher({ if (typeof getActiveProposals !== 'function') { throw new Error('reminderDispatcher: getActiveProposals function required'); } + if (typeof getNextSuperblock !== 'function') { + throw new Error( + 'reminderDispatcher: getNextSuperblock function required' + ); + } async function tick() { const nowMs = now(); + const nowSec = Math.floor(nowMs / 1000); - // --- Gate 1: active cycle & bucket window --- + // --- Gate 1: next-SB snapshot & bucket window --- + // + // A single atomic snapshot `{ height, epochSec }` so height + // and epochSec are consistent with each other even if the + // caller's underlying data source (sysMain) gets refreshed + // mid-tick. + // + // `height` is the canonical identifier — stable until the SB + // executes, at which point it jumps by exactly + // nSuperblockCycle. It drives scopeKey, so reminderLog.has() + // actually deduplicates across the 72h reminder window (the + // old epoch-based scopeKey drifted every 20s with sysMain's + // wall-clock-driven estimate and defeated dedup entirely — + // Codex PR14 P1). + // + // `epochSec` is the drifting estimate used for *time* + // calculations only. Minute-scale drift is harmless because + // bucket thresholds are in hours. + let snapshot; + try { + snapshot = await getNextSuperblock(); + } catch (err) { + log('warn', 'reminder_tick_sb_snapshot_failed', { + err: err && err.message, + }); + return { sent: 0, skipped: 'next_superblock_unavailable' }; + } + const height = snapshot && Number(snapshot.height); + const sbEpochSec = snapshot && Number(snapshot.epochSec); + if ( + !Number.isInteger(height) || + height <= 0 || + !Number.isFinite(sbEpochSec) || + sbEpochSec <= nowSec + ) { + return { sent: 0, skipped: 'next_superblock_unavailable' }; + } + + // Voters' actual deadline = when the maturity window opens. + const maturityOpensEpochSec = sbEpochSec - SUPERBLOCK_MATURITY_WINDOW_SEC; + const msRemaining = maturityOpensEpochSec * 1000 - nowMs; + + if (msRemaining <= 0) { + // The maturity window has already opened for the upcoming + // SB. Any email we'd send now would ask voters to vote on a + // trigger set where some fraction of masternodes have + // already committed — too late to influence the outcome, + // and worse than useless UX. Suppress. + return { + sent: 0, + skipped: 'maturity_window_open', + msRemaining, + height, + }; + } + const bucket = bucketForTimeRemaining(msRemaining); + if (!bucket) { + return { sent: 0, skipped: 'too_early', msRemaining, height }; + } + + // --- Gate 2: proposals eligible for this SB --- let rawList; try { rawList = await getActiveProposals(); @@ -128,49 +272,27 @@ function createReminderDispatcher({ log('warn', 'reminder_tick_proposals_failed', { err: err && err.message }); return { sent: 0, skipped: 'proposals_unavailable' }; } - const proposals = Array.isArray(rawList) + const normalized = Array.isArray(rawList) ? rawList.map((p) => normalizeProposal(p, nowMs)).filter(Boolean) : []; - if (proposals.length === 0) { - return { sent: 0, skipped: 'no_active_proposals' }; - } - - // Cycle = proposals sharing the earliest end_epoch among still- - // open proposals. Proposals with later deadlines belong to a - // FUTURE cycle — a user's vote on one of those must not suppress - // this cycle's reminder. In Syscoin governance, proposals voting - // for the same superblock share the same voting-deadline block - // (and therefore the same end_epoch), so this mirrors the - // consensus-level cycle grouping. Codex round-2 P2. - // - // scopeKey embeds the integer unix second of the earliest - // end_epoch, so it is stable across ticks and can be inspected - // in logs; a freshly-added proposal with an earlier deadline - // becomes its own cycle on the next tick. - const earliestDeadlineMs = Math.min(...proposals.map((p) => p.deadlineMs)); - const earliestEndEpoch = Math.floor(earliestDeadlineMs / 1000); - const msRemaining = earliestDeadlineMs - nowMs; - const bucket = bucketForTimeRemaining(msRemaining); - if (!bucket) { + const eligibleProposals = normalized.filter((p) => + proposalIsEligibleForSb(p, sbEpochSec) + ); + if (eligibleProposals.length === 0) { return { sent: 0, - skipped: - msRemaining <= 0 ? 'deadline_passed' : 'too_early', + skipped: 'no_active_proposals', + bucket, msRemaining, + height, }; } - // Membership test: exact-match on deadlineMs. Using endEpoch - // equality would be equivalent (we floored to seconds above) - // but deadlineMs stays in one unit for readability. - const cycleProposals = proposals.filter( - (p) => p.deadlineMs === earliestDeadlineMs - ); - const scopeKey = `cycle:${earliestEndEpoch}`; - const cycleProposalHashes = cycleProposals.map((p) => p.hash); + const scopeKey = `sb:${height}`; + const cycleProposalHashes = eligibleProposals.map((p) => p.hash); const deadlineText = formatDeadlineText(msRemaining); - // --- Gate 2: opted-in users only --- + // --- Gate 3: opted-in users only --- let candidates; try { candidates = users.listWithRemindersEnabled(); @@ -186,25 +308,26 @@ function createReminderDispatcher({ failed: 0, bucket, scopeKey, + height, candidateCount: candidates.length, - // proposalCount reflects the cycle the reminder is about, not - // the total number of active proposals (which may span future - // cycles). Keeps the email body and the tick-result metric in - // agreement. Codex round-2 P2. - proposalCount: cycleProposals.length, + // proposalCount reflects the cycle the reminder is about + // (proposals eligible for THIS superblock), not the total + // active list (which may include pre-start proposals for + // later cycles). Keeps the email body and the tick-result + // metric in agreement. + proposalCount: eligibleProposals.length, }; for (const user of candidates) { if (!user || !Number.isInteger(user.id) || !user.email) continue; - // --- Gate 3: has the user already voted in this cycle? --- + // --- Gate 4: has the user already voted in this cycle? --- // - // The spec is "if user voted on ANY proposal in the current - // cycle, suppress BOTH buckets". "Current cycle" is exactly - // the proposal set filtered above — NOT the full active list - // — otherwise a vote on a later-deadline proposal would - // incorrectly cancel this earlier cycle's reminder. Codex - // round-2 P2. + // The spec is "if user voted on ANY proposal eligible for + // the upcoming SB, suppress BOTH buckets". "Eligible for the + // upcoming SB" is exactly the filtered set above — NOT the + // full active list — otherwise a vote on a later-cycle + // proposal would incorrectly cancel this cycle's reminder. // // We check this BEFORE the already-logged check so that a // user who votes after the days_before email still has the @@ -259,9 +382,7 @@ function createReminderDispatcher({ await mailer.sendVoteReminder({ to: user.email, bucket, - // Report the cycle-scoped count so the email body matches - // the cycle the reminder is actually about. Codex round-2 P2. - proposalCount: cycleProposals.length, + proposalCount: eligibleProposals.length, deadlineText, }); } catch (err) { @@ -306,13 +427,13 @@ function createReminderDispatcher({ module.exports = { createReminderDispatcher, - // Exported for tests and for boot wiring that wants to surface the - // bucket names in admin logs or future /admin routes. BUCKET_FINAL_24H, BUCKET_DAYS_BEFORE, FINAL_24H_MS, DAYS_BEFORE_MS, + SUPERBLOCK_MATURITY_WINDOW_SEC, bucketForTimeRemaining, formatDeadlineText, normalizeProposal, + proposalIsEligibleForSb, }; diff --git a/lib/reminderDispatcher.test.js b/lib/reminderDispatcher.test.js index 2d907a5..a48b326 100644 --- a/lib/reminderDispatcher.test.js +++ b/lib/reminderDispatcher.test.js @@ -3,13 +3,16 @@ const { bucketForTimeRemaining, formatDeadlineText, normalizeProposal, + proposalIsEligibleForSb, FINAL_24H_MS, DAYS_BEFORE_MS, + SUPERBLOCK_MATURITY_WINDOW_SEC, } = require('./reminderDispatcher'); const H64 = (c) => c.repeat(64); const P1 = H64('a'); const P2 = H64('b'); +const P3 = H64('c'); // A DB-free fake. Each fake repo receives the bare surface the // dispatcher needs and nothing else — this keeps the unit test @@ -52,22 +55,41 @@ function fakeMailer() { }; } -// Test fixture: "now" is fixed; proposals carry their endEpoch in -// unix seconds. Adjust by constructing proposals that sit in the -// desired bucket. P1 and P2 share the same end_epoch so they are -// in the same governance cycle (matches Syscoin semantics where -// proposals voting for the same superblock share the same voting -// deadline). Mixed-deadline scenarios get their own dedicated -// helper below. +// Test fixture: "now" is fixed. Tests express their bucket target as +// "milliseconds from now until the MATURITY WINDOW OPENS" — the +// dispatcher anchors its deadline there (SB_epoch - 3 days), not at +// the SB itself. E.g. for a days_before fixture we point maturity +// 48h out, which means the SB is 48h + 3d = 5d out. const NOW_MS = 1_700_000_000_000; const NOW_SEC = NOW_MS / 1000; +const MATURITY_WINDOW_MS = SUPERBLOCK_MATURITY_WINDOW_SEC * 1000; +const ONE_YEAR_MS = 365 * 24 * 3600 * 1000; -function proposalsEndingIn(msFromNow) { - const endEpoch = Math.floor((NOW_MS + msFromNow) / 1000); - return [ - { hash: P1, endEpoch }, - { hash: P2, endEpoch }, // same cycle - ]; +const SB_CYCLE_SEC = 17520 * 150; // mainnet superblock cadence +const BASE_SB_HEIGHT = 600_000; // arbitrary; only stability matters + +function sbEpochSecForMaturityOffset(msToMaturity) { + // Given how far in the future the maturity window should open, + // return the implied SB epoch (maturity + 3d). + return Math.floor((NOW_MS + msToMaturity + MATURITY_WINDOW_MS) / 1000); +} + +// Helper: a "this-cycle" proposal — spans from now-5d to now+1yr, +// so the upcoming SB (whatever offset) falls inside its eligibility +// window. The second hash arg defaults to P1 so the common case +// reads cleanly. +function thisCycleProposal(hash = P1) { + return { + hash, + startEpoch: Math.floor((NOW_MS - 5 * 24 * 3600 * 1000) / 1000), + endEpoch: Math.floor((NOW_MS + ONE_YEAR_MS) / 1000), + }; +} + +// Default "two-proposal cycle" — P1 and P2 both eligible for the +// upcoming SB. +function cycleProposals() { + return [thisCycleProposal(P1), thisCycleProposal(P2)]; } // --------------------------------------------------------------------------- @@ -98,16 +120,100 @@ describe('bucketForTimeRemaining', () => { }); }); +describe('constants', () => { + test('SUPERBLOCK_MATURITY_WINDOW_SEC matches Core (1728 × 150)', () => { + expect(SUPERBLOCK_MATURITY_WINDOW_SEC).toBe(1728 * 150); + expect(SUPERBLOCK_MATURITY_WINDOW_SEC).toBe(3 * 24 * 3600); + }); +}); + describe('normalizeProposal', () => { test('rejects malformed hashes and past deadlines', () => { expect(normalizeProposal(null, NOW_MS)).toBeNull(); - expect(normalizeProposal({ hash: 'nope', endEpoch: NOW_SEC + 100 }, NOW_MS)).toBeNull(); - expect(normalizeProposal({ hash: P1, endEpoch: NOW_SEC - 1 }, NOW_MS)).toBeNull(); + expect( + normalizeProposal({ hash: 'nope', endEpoch: NOW_SEC + 100 }, NOW_MS) + ).toBeNull(); + expect( + normalizeProposal({ hash: P1, endEpoch: NOW_SEC - 1 }, NOW_MS) + ).toBeNull(); }); - test('normalizes hash to lowercase and exposes deadlineMs', () => { - const p = normalizeProposal({ hash: P1.toUpperCase(), endEpoch: NOW_SEC + 100 }, NOW_MS); + test('normalizes hash to lowercase and exposes startMs/endMs', () => { + const p = normalizeProposal( + { hash: P1.toUpperCase(), startEpoch: NOW_SEC, endEpoch: NOW_SEC + 100 }, + NOW_MS + ); expect(p.hash).toBe(P1); - expect(p.deadlineMs).toBe((NOW_SEC + 100) * 1000); + expect(p.startEpoch).toBe(NOW_SEC); + expect(p.endEpoch).toBe(NOW_SEC + 100); + expect(p.startMs).toBe(NOW_SEC * 1000); + expect(p.endMs).toBe((NOW_SEC + 100) * 1000); + }); + test('missing/zero/invalid startEpoch is treated as "no lower bound" (legacy compat)', () => { + const cases = [ + { hash: P1, endEpoch: NOW_SEC + 100 }, // undefined startEpoch + { hash: P1, startEpoch: 0, endEpoch: NOW_SEC + 100 }, + { hash: P1, startEpoch: -1, endEpoch: NOW_SEC + 100 }, + { hash: P1, startEpoch: 'bogus', endEpoch: NOW_SEC + 100 }, + ]; + for (const c of cases) { + const p = normalizeProposal(c, NOW_MS); + expect(p).not.toBeNull(); + expect(p.startEpoch).toBe(0); + } + }); +}); + +describe('proposalIsEligibleForSb', () => { + const sbSec = NOW_SEC + 5 * 24 * 3600; // 5 days out (maturity + 2d) + + test('covers the SB inside its window → eligible', () => { + const p = normalizeProposal( + { hash: P1, startEpoch: NOW_SEC - 1000, endEpoch: NOW_SEC + 1_000_000 }, + NOW_MS + ); + expect(proposalIsEligibleForSb(p, sbSec)).toBe(true); + }); + test('pre-activation (startEpoch > sbSec) → not eligible', () => { + const p = normalizeProposal( + { hash: P1, startEpoch: sbSec + 1, endEpoch: sbSec + 1_000_000 }, + NOW_MS + ); + expect(proposalIsEligibleForSb(p, sbSec)).toBe(false); + }); + test('endEpoch strictly before the SB → not eligible (already ended for this SB)', () => { + const p = normalizeProposal( + { hash: P1, startEpoch: NOW_SEC - 1000, endEpoch: sbSec - 1 }, + NOW_MS + ); + expect(proposalIsEligibleForSb(p, sbSec)).toBe(false); + }); + test('boundary: startEpoch === sbSec OR endEpoch === sbSec → eligible (inclusive)', () => { + const pStart = normalizeProposal( + { hash: P1, startEpoch: sbSec, endEpoch: sbSec + 1000 }, + NOW_MS + ); + expect(proposalIsEligibleForSb(pStart, sbSec)).toBe(true); + const pEnd = normalizeProposal( + { hash: P1, startEpoch: sbSec - 1000, endEpoch: sbSec }, + NOW_MS + ); + expect(proposalIsEligibleForSb(pEnd, sbSec)).toBe(true); + }); + test('legacy proposals with startEpoch=0 stay eligible as long as endEpoch covers the SB', () => { + const p = normalizeProposal( + { hash: P1, endEpoch: sbSec + 100 }, + NOW_MS + ); + expect(proposalIsEligibleForSb(p, sbSec)).toBe(true); + }); + test('rejects bogus SB second', () => { + const p = normalizeProposal( + { hash: P1, startEpoch: 0, endEpoch: NOW_SEC + 100 }, + NOW_MS + ); + expect(proposalIsEligibleForSb(p, 0)).toBe(false); + expect(proposalIsEligibleForSb(p, -1)).toBe(false); + expect(proposalIsEligibleForSb(p, NaN)).toBe(false); }); }); @@ -129,24 +235,39 @@ describe('formatDeadlineText', () => { describe('createReminderDispatcher.tick', () => { const BASE_USER = { id: 42, email: 'voter@example.com' }; + // `maturityOffsetMs`: how far in the future the MATURITY WINDOW + // opens (not the SB). The dispatcher anchors its bucketing there. + // 48h → days_before (SB is at +48h + 3d = +5d) + // 6h → final_24h (SB is at +6h + 3d = +3d6h) + // 96h → too_early (SB is at +96h + 3d) + // -1h → maturity_window_open (SB is in <3d) function mkDispatcher({ users = [BASE_USER], receiptsMatches = new Set(), mailerImpl, getActiveProposals, + getNextSuperblock, + maturityOffsetMs = 48 * 3600 * 1000, + height = BASE_SB_HEIGHT, reminderLogImpl, } = {}) { const mailer = mailerImpl || fakeMailer(); const reminderLog = reminderLogImpl || fakeReminderLog(); + const defaultSbEpochSec = sbEpochSecForMaturityOffset(maturityOffsetMs); return { mailer, reminderLog, + sbEpochSec: defaultSbEpochSec, + height, dispatcher: createReminderDispatcher({ users: fakeUsersRepo(users), voteReceipts: fakeReceiptsRepo(receiptsMatches), reminderLog, mailer, - getActiveProposals, + getActiveProposals: getActiveProposals || (async () => cycleProposals()), + getNextSuperblock: + getNextSuperblock || + (async () => ({ height, epochSec: defaultSbEpochSec })), now: () => NOW_MS, }), }; @@ -157,22 +278,41 @@ describe('createReminderDispatcher.tick', () => { getActiveProposals: async () => [], }); const out = await dispatcher.tick(); - expect(out).toMatchObject({ sent: 0, skipped: 'no_active_proposals' }); + expect(out).toMatchObject({ + sent: 0, + skipped: 'no_active_proposals', + bucket: 'days_before', + }); expect(mailer.sendVoteReminder).not.toHaveBeenCalled(); }); - test('deadline > 72h away → skipped=too_early', async () => { + test('maturity opens > 72h away → skipped=too_early', async () => { const { dispatcher, mailer } = mkDispatcher({ - getActiveProposals: async () => proposalsEndingIn(96 * 3600 * 1000), + maturityOffsetMs: 96 * 3600 * 1000, }); const out = await dispatcher.tick(); expect(out).toMatchObject({ sent: 0, skipped: 'too_early' }); expect(mailer.sendVoteReminder).not.toHaveBeenCalled(); }); - test('deadline within 72h, user not voted → days_before email is sent and logged', async () => { - const { dispatcher, mailer, reminderLog } = mkDispatcher({ - getActiveProposals: async () => proposalsEndingIn(48 * 3600 * 1000), + test('maturity window already open → skipped=maturity_window_open (too late for reminders)', async () => { + // SB is only 2 days out — we are already inside the 3-day + // maturity window. Masternodes have begun committing + // YES-FUNDING trigger votes; a reminder now would only fuel + // anxiety. Suppress. + const sbEpochSec = NOW_SEC + 2 * 24 * 3600; + const { dispatcher, mailer } = mkDispatcher({ + getNextSuperblock: async () => ({ height: BASE_SB_HEIGHT, epochSec: sbEpochSec }), + }); + const out = await dispatcher.tick(); + expect(out.skipped).toBe('maturity_window_open'); + expect(out.msRemaining).toBeLessThanOrEqual(0); + expect(mailer.sendVoteReminder).not.toHaveBeenCalled(); + }); + + test('maturity opens within 72h, user not voted → days_before email sent and logged with sb: scope', async () => { + const { dispatcher, mailer, reminderLog, height } = mkDispatcher({ + maturityOffsetMs: 48 * 3600 * 1000, }); const out = await dispatcher.tick(); expect(out).toMatchObject({ @@ -182,6 +322,7 @@ describe('createReminderDispatcher.tick', () => { bucket: 'days_before', candidateCount: 1, proposalCount: 2, + height, }); expect(mailer.sendVoteReminder).toHaveBeenCalledWith( expect.objectContaining({ @@ -190,16 +331,17 @@ describe('createReminderDispatcher.tick', () => { proposalCount: 2, }) ); - // Scope key embeds the earliest end_epoch (seconds) so repeat - // ticks within the same cycle are idempotent. - const earliestSec = Math.floor((NOW_MS + 48 * 3600 * 1000) / 1000); - expect(reminderLog.has(BASE_USER.id, `cycle:${earliestSec}`, 'days_before')) - .toBe(true); + // scopeKey keys on the stable BLOCK HEIGHT, not the drifting + // epochSec estimate. This is what makes the reminder log + // actually deduplicate across ticks (Codex PR14 P1 fix). + expect(reminderLog.has(BASE_USER.id, `sb:${height}`, 'days_before')).toBe( + true + ); }); - test('deadline within 24h, user not voted → final_24h email sent', async () => { + test('maturity opens within 24h, user not voted → final_24h email sent', async () => { const { dispatcher, mailer } = mkDispatcher({ - getActiveProposals: async () => proposalsEndingIn(12 * 3600 * 1000), + maturityOffsetMs: 12 * 3600 * 1000, }); const out = await dispatcher.tick(); expect(out.bucket).toBe('final_24h'); @@ -209,19 +351,53 @@ describe('createReminderDispatcher.tick', () => { ); }); - test('USER SPEC: voted on any cycle proposal → BOTH buckets are suppressed', async () => { - // This is the critical user-facing contract. A user who has - // already voted on one proposal in the cycle must NOT receive - // either the days_before OR the final_24h reminder. - // - // We run the dispatcher in the days_before window first, then - // advance to the final_24h window. Both ticks must be no-ops for - // this user. + test('Codex PR14 P1: scopeKey stability — epochSec drift between ticks does NOT trigger a duplicate send', async () => { + // sysMain recomputes superBlockNextEpochSec every 20s from + // Date.now() + diffBlock * avgBlockTime, so epochSec drifts + // forward by ~20 each tick even when the target SB hasn't + // changed. If scopeKey were keyed on epochSec, reminderLog.has() + // would never return true in the 72h window and a user could + // get hourly reminders for 72 hours straight (72 duplicates). + // Keying on height closes that loophole: same height → same + // scope → dedup works. + const reminderLog = fakeReminderLog(); + const mailer = fakeMailer(); + let epochSec = sbEpochSecForMaturityOffset(48 * 3600 * 1000); + const dispatcher = createReminderDispatcher({ + users: fakeUsersRepo([BASE_USER]), + voteReceipts: fakeReceiptsRepo(new Set()), + reminderLog, + mailer, + getActiveProposals: async () => cycleProposals(), + // Drift the epochSec forward 20s each tick but keep height + // stable. This is exactly what sysMain does in production. + getNextSuperblock: async () => { + const snap = { height: BASE_SB_HEIGHT, epochSec }; + epochSec += 20; + return snap; + }, + now: () => NOW_MS, + }); + const t1 = await dispatcher.tick(); + const t2 = await dispatcher.tick(); + const t3 = await dispatcher.tick(); + expect(t1.sent).toBe(1); + expect(t2.sent).toBe(0); + expect(t2.skippedLogged).toBe(1); + expect(t3.sent).toBe(0); + expect(t3.skippedLogged).toBe(1); + expect(mailer.sendVoteReminder).toHaveBeenCalledTimes(1); + }); + + test('USER SPEC: voted on any cycle-eligible proposal → BOTH buckets are suppressed', async () => { + // A user who has already voted on one proposal eligible for the + // upcoming SB must NOT receive either the days_before OR the + // final_24h reminder. const votedOnP1 = new Set([`${BASE_USER.id}|${P1}`]); { const { dispatcher, mailer } = mkDispatcher({ receiptsMatches: votedOnP1, - getActiveProposals: async () => proposalsEndingIn(48 * 3600 * 1000), + maturityOffsetMs: 48 * 3600 * 1000, }); const out = await dispatcher.tick(); expect(out.sent).toBe(0); @@ -231,7 +407,7 @@ describe('createReminderDispatcher.tick', () => { { const { dispatcher, mailer } = mkDispatcher({ receiptsMatches: votedOnP1, - getActiveProposals: async () => proposalsEndingIn(12 * 3600 * 1000), + maturityOffsetMs: 12 * 3600 * 1000, }); const out = await dispatcher.tick(); expect(out.sent).toBe(0); @@ -240,31 +416,24 @@ describe('createReminderDispatcher.tick', () => { } }); - test('Codex round-2 P2: vote on a later-cycle proposal does NOT suppress the current cycle reminder', async () => { - // Regression guard for the bug Codex flagged: `tick()` computes - // the cycle from the earliest end_epoch, but the old code built - // `proposalHashes` from ALL active proposals. That let a vote - // on a future-cycle proposal incorrectly satisfy the - // "already voted in this cycle" gate and drop the current - // cycle's reminder. - // - // Fixture: - // P1 — ends in 48h (current cycle, days_before bucket) - // P2 — ends in 96h (future cycle, > 72h away so alone would - // be "too_early") - // - // User has voted on P2 only. The reminder for P1's cycle must - // still fire; proposalCount must reflect the cycle (= 1, P1 - // only), not the full active list (= 2). - const earlyEpoch = Math.floor((NOW_MS + 48 * 3600 * 1000) / 1000); - const lateEpoch = Math.floor((NOW_MS + 96 * 3600 * 1000) / 1000); + test('vote on a future-cycle proposal (pre-activation for this SB) does NOT suppress the current cycle reminder', async () => { + // Regression guard: a vote on a proposal NOT eligible for the + // upcoming SB (its startEpoch is after sbEpochSec, i.e. it + // belongs to a later cycle) must not cancel this cycle's + // reminder. proposalCount must reflect the eligible set only. + const maturityOffsetMs = 48 * 3600 * 1000; + const sbEpochSec = sbEpochSecForMaturityOffset(maturityOffsetMs); + const currentCycle = thisCycleProposal(P1); + const futureCycle = { + hash: P2, + startEpoch: sbEpochSec + 7 * 24 * 3600, // activates a week AFTER upcoming SB + endEpoch: sbEpochSec + ONE_YEAR_MS / 1000, + }; const votedOnFuture = new Set([`${BASE_USER.id}|${P2}`]); const { dispatcher, mailer } = mkDispatcher({ receiptsMatches: votedOnFuture, - getActiveProposals: async () => [ - { hash: P1, endEpoch: earlyEpoch }, - { hash: P2, endEpoch: lateEpoch }, - ], + maturityOffsetMs, + getActiveProposals: async () => [currentCycle, futureCycle], }); const out = await dispatcher.tick(); expect(out).toMatchObject({ @@ -278,15 +447,48 @@ describe('createReminderDispatcher.tick', () => { ); }); + test('scopeKey rotates on height change — reminder fires again for the next SB', async () => { + // A multi-month proposal gets two legitimately separate + // reminder cycles: one for SB_N, one for SB_{N+1}. The height + // jump (by nSuperblockCycle blocks) rotates scopeKey so the + // reminder-log entry for SB_N no longer suppresses SB_{N+1}. + const reminderLog = fakeReminderLog(); + const mailer = fakeMailer(); + let height = BASE_SB_HEIGHT; + let epochSec = sbEpochSecForMaturityOffset(48 * 3600 * 1000); + const proposal = thisCycleProposal(P1); + const dispatcher = createReminderDispatcher({ + users: fakeUsersRepo([BASE_USER]), + voteReceipts: fakeReceiptsRepo(new Set()), + reminderLog, + mailer, + getActiveProposals: async () => [proposal], + getNextSuperblock: async () => ({ height, epochSec }), + now: () => NOW_MS, + }); + const t1 = await dispatcher.tick(); + expect(t1.sent).toBe(1); + + // SB_N executes on-chain; sysMain picks up the new next-SB: + // height advances by nSuperblockCycle, epochSec jumps ~30 days. + height += 17520; + epochSec += SB_CYCLE_SEC; + // And we'd be in days_before for SB_{N+1} ... except from the + // frozen "now" perspective the new maturity is ~30d out. So + // this tick reports too_early — the test's point is just that + // the dedup doesn't prematurely suppress when the underlying + // SB changes. We assert the scope key from t1 would NOT + // collide with the new one: + expect(t1.scopeKey).toBe(`sb:${BASE_SB_HEIGHT}`); + expect(`sb:${height}`).not.toBe(t1.scopeKey); + }); + test('USER SPEC: user who gets the days_before email then votes → no final_24h email', async () => { - // The full lifecycle. Tick 1: days_before window, user hasn't - // voted → email sent. User votes. Tick 2: final_24h window, but - // the cycle-vote gate now returns true → suppressed. const receiptsState = new Set(); const users = fakeUsersRepo([BASE_USER]); const reminderLog = fakeReminderLog(); const mailer = fakeMailer(); - let currentProposalsMs = 48 * 3600 * 1000; + let currentMaturityOffsetMs = 48 * 3600 * 1000; const dispatcher = createReminderDispatcher({ users, @@ -296,59 +498,59 @@ describe('createReminderDispatcher.tick', () => { }, reminderLog, mailer, - getActiveProposals: async () => proposalsEndingIn(currentProposalsMs), + getActiveProposals: async () => cycleProposals(), + getNextSuperblock: async () => ({ + height: BASE_SB_HEIGHT, + epochSec: sbEpochSecForMaturityOffset(currentMaturityOffsetMs), + }), now: () => NOW_MS, }); - // Tick 1: days_before → email fires. const t1 = await dispatcher.tick(); expect(t1.bucket).toBe('days_before'); expect(t1.sent).toBe(1); - // User votes (on any cycle proposal — P2 here to prove "any"). receiptsState.add(`${BASE_USER.id}|${P2}`); - // Clock jumps forward into the final_24h window. - currentProposalsMs = 6 * 3600 * 1000; + currentMaturityOffsetMs = 6 * 3600 * 1000; const t2 = await dispatcher.tick(); expect(t2.bucket).toBe('final_24h'); expect(t2.sent).toBe(0); expect(t2.skippedVoted).toBe(1); - // And no second email went out. expect(mailer.sentMessages.filter((m) => m.bucket === 'final_24h')).toEqual( [] ); }); test('USER SPEC: user who does nothing in cycle gets BOTH emails', async () => { - // The complementary case to the one above. A fresh log and a - // user who never votes must receive one email per bucket. const reminderLog = fakeReminderLog(); const mailer = fakeMailer(); - let currentProposalsMs = 48 * 3600 * 1000; + let currentMaturityOffsetMs = 48 * 3600 * 1000; const dispatcher = createReminderDispatcher({ users: fakeUsersRepo([BASE_USER]), voteReceipts: fakeReceiptsRepo(new Set()), reminderLog, mailer, - getActiveProposals: async () => proposalsEndingIn(currentProposalsMs), + getActiveProposals: async () => cycleProposals(), + getNextSuperblock: async () => ({ + height: BASE_SB_HEIGHT, + epochSec: sbEpochSecForMaturityOffset(currentMaturityOffsetMs), + }), now: () => NOW_MS, }); - await dispatcher.tick(); // days_before - currentProposalsMs = 6 * 3600 * 1000; - await dispatcher.tick(); // final_24h + await dispatcher.tick(); // days_before + currentMaturityOffsetMs = 6 * 3600 * 1000; + await dispatcher.tick(); // final_24h const buckets = mailer.sentMessages.map((m) => m.bucket).sort(); expect(buckets).toEqual(['days_before', 'final_24h']); }); test('idempotency: repeat tick in the same bucket does not re-send', async () => { - // Defends against a boot-loop, a clock wobble, or an operator - // manually running tick() on a schedule tighter than intended. const { dispatcher, mailer } = mkDispatcher({ - getActiveProposals: async () => proposalsEndingIn(48 * 3600 * 1000), + maturityOffsetMs: 48 * 3600 * 1000, }); await dispatcher.tick(); await dispatcher.tick(); @@ -357,8 +559,6 @@ describe('createReminderDispatcher.tick', () => { }); test('mailer failure does NOT log the send → next tick retries', async () => { - // If we logged before sending, a flaky SMTP would permanently - // drop the reminder. Log-after-send is the safer trade. const mailer = { sendVoteReminder: jest .fn() @@ -372,7 +572,7 @@ describe('createReminderDispatcher.tick', () => { }; const { dispatcher } = mkDispatcher({ mailerImpl: mailer, - getActiveProposals: async () => proposalsEndingIn(48 * 3600 * 1000), + maturityOffsetMs: 48 * 3600 * 1000, }); const t1 = await dispatcher.tick(); expect(t1.failed).toBe(1); @@ -383,16 +583,13 @@ describe('createReminderDispatcher.tick', () => { }); test('users without email or id are silently skipped', async () => { - // Defensive: users.listWithRemindersEnabled should never return - // malformed rows, but if it did (e.g. a corrupted DB read during - // a mid-migration state), we skip rather than blow up the tick. const { dispatcher, mailer } = mkDispatcher({ users: [ { id: 1, email: null }, { id: null, email: 'x@x.com' }, BASE_USER, ], - getActiveProposals: async () => proposalsEndingIn(48 * 3600 * 1000), + maturityOffsetMs: 48 * 3600 * 1000, }); const out = await dispatcher.tick(); expect(out.sent).toBe(1); @@ -406,7 +603,66 @@ describe('createReminderDispatcher.tick', () => { }, }); const out = await dispatcher.tick(); - expect(out).toEqual({ sent: 0, skipped: 'proposals_unavailable' }); + expect(out).toMatchObject({ sent: 0, skipped: 'proposals_unavailable' }); + expect(mailer.sendVoteReminder).not.toHaveBeenCalled(); + }); + + test('getNextSuperblock throwing is reported, not fatal', async () => { + const { dispatcher, mailer } = mkDispatcher({ + getNextSuperblock: async () => { + throw new Error('sysMain not warm'); + }, + }); + const out = await dispatcher.tick(); + expect(out).toMatchObject({ + sent: 0, + skipped: 'next_superblock_unavailable', + }); + expect(mailer.sendVoteReminder).not.toHaveBeenCalled(); + }); + + test('getNextSuperblock returning bogus height or epochSec → next_superblock_unavailable', async () => { + const futureEpoch = sbEpochSecForMaturityOffset(48 * 3600 * 1000); + const cases = [ + null, + undefined, + {}, + { height: 0, epochSec: futureEpoch }, + { height: -1, epochSec: futureEpoch }, + { height: 1.5, epochSec: futureEpoch }, + { height: 'tall', epochSec: futureEpoch }, + { height: 600_000, epochSec: 0 }, + { height: 600_000, epochSec: null }, + { height: 600_000, epochSec: NaN }, + // past epoch: /mnStats lagging behind the tip + { height: 600_000, epochSec: Math.floor((NOW_MS - 10_000) / 1000) }, + // epoch equal to now: boundary — also treated as stale + { height: 600_000, epochSec: Math.floor(NOW_MS / 1000) }, + ]; + for (const bogus of cases) { + const { dispatcher, mailer } = mkDispatcher({ + getNextSuperblock: async () => bogus, + }); + const out = await dispatcher.tick(); + expect(out.skipped).toBe('next_superblock_unavailable'); + expect(mailer.sendVoteReminder).not.toHaveBeenCalled(); + } + }); + + test('proposals fetched but ALL are ineligible for the upcoming SB → no_active_proposals', async () => { + const maturityOffsetMs = 48 * 3600 * 1000; + const sbEpochSec = sbEpochSecForMaturityOffset(maturityOffsetMs); + const preActivation = { + hash: P3, + startEpoch: sbEpochSec + 24 * 3600, // starts 1d after SB + endEpoch: sbEpochSec + 1_000_000, + }; + const { dispatcher, mailer } = mkDispatcher({ + maturityOffsetMs, + getActiveProposals: async () => [preActivation], + }); + const out = await dispatcher.tick(); + expect(out.skipped).toBe('no_active_proposals'); expect(mailer.sendVoteReminder).not.toHaveBeenCalled(); }); @@ -417,5 +673,15 @@ describe('createReminderDispatcher.tick', () => { users: fakeUsersRepo([]), }) ).toThrow(/voteReceipts/); + expect(() => + createReminderDispatcher({ + users: fakeUsersRepo([]), + voteReceipts: fakeReceiptsRepo(new Set()), + reminderLog: fakeReminderLog(), + mailer: fakeMailer(), + getActiveProposals: async () => [], + // missing getNextSuperblock + }) + ).toThrow(/getNextSuperblock/); }); }); diff --git a/server.js b/server.js index aa1dbf8..e412545 100644 --- a/server.js +++ b/server.js @@ -358,12 +358,21 @@ const reminderDispatcher = createReminderDispatcher({ voteReceipts: services.voteReceipts, reminderLog, mailer, - // Shape: [{ hash, endEpoch }]. gObject_list returns a map keyed by - // gov-object hash, with `DataString` containing the per-proposal - // JSON that has end_epoch (unix seconds). We project here rather - // than inside the dispatcher because the RPC shape is a - // server-side concern (the dispatcher contract is the normalized - // shape). + // Shape: [{ hash, startEpoch, endEpoch }]. gObject_list returns a + // map keyed by gov-object hash, with `DataString` containing the + // per-proposal JSON that has start_epoch + end_epoch (unix + // seconds). We project here rather than inside the dispatcher + // because the RPC shape is a server-side concern (the dispatcher + // contract is the normalized shape). + // + // Both start_epoch and end_epoch are forwarded so the dispatcher + // can filter proposals by SB-eligibility + // (startEpoch <= nextSbEpochSec <= endEpoch). A missing + // start_epoch defaults to 0 inside normalizeProposal, which the + // eligibility check treats as "no lower bound" — that keeps any + // legacy DataString payload (from clients that predate the + // derive-window wizard) in the cycle rather than silently + // dropping it. getActiveProposals: async () => { const raw = await rpcServices(client.callRpc).gObject_list().call(); const out = []; @@ -377,10 +386,34 @@ const reminderDispatcher = createReminderDispatcher({ } const endEpoch = Number(data && data.end_epoch); if (!Number.isFinite(endEpoch) || endEpoch <= 0) continue; - out.push({ hash: entry.Hash, endEpoch }); + const rawStart = Number(data && data.start_epoch); + const startEpoch = + Number.isFinite(rawStart) && rawStart > 0 ? rawStart : 0; + out.push({ hash: entry.Hash, startEpoch, endEpoch }); } return out; }, + // Next-superblock snapshot: { height, epochSec } read atomically + // from the in-memory dataStore (sysMain refreshes every 20s). + // + // - `height` is Core's nextSuperBlock block number. It is + // STABLE — it only changes when the SB actually executes and + // jumps by exactly nSuperblockCycle (17520 mainnet). The + // dispatcher uses it for scopeKey so reminderLog.has() + // deduplicates correctly across the 72h reminder window. + // - `epochSec` is sysMain's `Date.now() + diffBlock * avgBlockTime` + // estimate of when the SB will occur. It DRIFTS every 20s + // and is used for time-remaining calculations only (bucket + // thresholds are in hours so minute-scale drift is fine). + // + // Missing / stale / zero values surface as + // `skipped: 'next_superblock_unavailable'` and the dispatcher + // retries on the next hourly tick — correct behavior on cold + // boot (sysMain hasn't completed its first pass) or RPC hiccups. + getNextSuperblock: async () => ({ + height: Number(dataStore.nextSuperBlock) | 0, + epochSec: Number(dataStore.superBlockNextEpochSec) || 0, + }), log: (level, event, meta) => { // eslint-disable-next-line no-console console.log(`[reminder] ${level} ${event}`, meta || '');