Reminder dispatcher: key on next superblock, not end_epoch#14
Conversation
…end_epoch
The reminder dispatcher used to key off each proposal's end_epoch:
deadline = earliest endEpoch, bucket by `deadline - now`,
scope = `cycle:<earliestEndEpoch>`. That worked when proposers set
end_epoch close to their last paid superblock, but after the derive-
window wizard landed it no longer matches reality — end_epoch is now
intentionally ~15 days AFTER the last payout SB so Core prunes
cleanly without allocating an extra superblock. With that layout,
"≤72h to end_epoch" fires ~12 days AFTER every meaningful voting
decision has been made. The reminder is useless at best and
misleading at worst (a user who thinks "vote today, it closes in 3
days!" has actually missed the vote by almost two weeks).
Rework the dispatcher to key off the next upcoming superblock —
the event that actually matters to voters:
deadline = nextSuperblockEpochSec (from dataStore, populated by
sysMain every 20s; exposed
via /mnStats.superblock_
next_epoch_sec as of PR#13)
cycle = {P : P.startEpoch <= nextSbEpoch <= P.endEpoch}
(matches Core's superblock-payment window check)
scope = `sb:${nextSbEpochSec}`
(naturally rotates once the SB executes and sysMain
picks up the next anchor — a user who got both
reminders for SB_N and didn't vote gets both again
for SB_{N+1})
buckets = unchanged: ≤24h final_24h, ≤72h days_before
Changes:
lib/reminderDispatcher.js
- new required dep: getNextSuperblockEpochSec()
- normalizeProposal accepts optional startEpoch; missing/zero
mapped to "no lower bound" so legacy DataString payloads
without start_epoch stay eligible
- proposalIsEligibleForSb(p, sbEpochSec) — exported helper
that implements the startEpoch <= sb <= endEpoch check
- tick() new skip reason `next_superblock_unavailable` for
fetch-throws / zero / NaN / past (stale sysMain) anchors
- cycle-vote gate now operates on the SB-eligible subset only,
so a vote on a later-cycle proposal (pre-activation for this
SB) cannot incorrectly suppress the current cycle's reminder
lib/reminderDispatcher.test.js
- rewritten fixture: proposalsForSbIn → thisCycleProposal /
cycleProposals, which builds [startEpoch, endEpoch] spans
that cover the upcoming SB by construction
- scopeKey assertion updated to `sb:<epoch>`
- new regressions:
* new proposalIsEligibleForSb suite (6 tests)
* normalizeProposal legacy-compat (missing start_epoch)
* getNextSuperblockEpochSec throwing / 0 / null / NaN
* getNextSuperblockEpochSec returning past timestamp (stale)
* all-ineligible active list → no_active_proposals
* multi-month proposal in both its covered cycles (SB_1 +
SB_2 via distinct scopeKeys)
- 33/33 tests green in the suite; 854/854 across the repo
server.js
- getActiveProposals now forwards start_epoch alongside
end_epoch so the dispatcher can filter by SB-eligibility
- wires getNextSuperblockEpochSec to
dataStore.superBlockNextEpochSec
Failure modes handled conservatively: if /mnStats is mid-refresh
and returns 0/past, the dispatcher skips the tick rather than fire
an email keyed to a bogus or already-executed SB. Next tick (hourly
default) retries automatically once sysMain recovers.
Made-with: Cursor
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6de877a19
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ); | ||
| const scopeKey = `cycle:${earliestEndEpoch}`; | ||
| const cycleProposalHashes = cycleProposals.map((p) => p.hash); | ||
| const scopeKey = `sb:${sbEpochSec}`; |
There was a problem hiding this comment.
scopeKey is built from sbEpochSec, but with the current wiring this value comes from dataStore.superBlockNextEpochSec, which services/sysMain.js recomputes every 20s from Date.now() + diffBlock * avgBlockTime. That estimate can change between ticks even when the target superblock has not changed, so the same user/bucket gets a new scope key and reminderLog.has() no longer deduplicates; in the 72h window, users who have not voted can be emailed repeatedly for the same superblock.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in d97bb67. scopeKey now keys on dataStore.nextSuperBlock (block height), not the drifting epoch estimate.
Replaced the single getNextSuperblockEpochSec dep with an atomic getNextSuperblock: () => ({ height, epochSec }) snapshot:
height— Core'snextSuperBlockinteger block number, stable until the SB executes (then jumps by exactlynSuperblockCycle= 17520 mainnet). scopeKey =sb:${height}.epochSec— still used only for time-remaining math (buckets are in hours, so minute-scale drift is fine).
Snapshot is atomic so height and epochSec come from the same sysMain pass — no race where sysMain refreshes between separate reads.
Added a direct regression (Codex PR14 P1: scopeKey stability) that drifts epochSec forward +20s per tick while keeping height fixed and asserts reminderLog.has() correctly deduplicates after the first send — only 1 email goes out across 3 ticks.
While I was there I also corrected a separate intent bug that you didn't flag but @sidhujag reminded me of: e6de877 anchored bucketing to the SB itself, so both reminders fired inside the 3-day maturity window. Since MNs start committing YES-FUNDING during that window, any email sent then is asking voters to race a decision some fraction of the network has already made. Re-anchored to sbEpochSec - SUPERBLOCK_MATURITY_WINDOW_SEC (maturity-window start = 3 days before SB). Existing 24h/72h bucket thresholds preserved — they now measure time until the maturity window opens, which maps to ~3–6 days before the superblock in wall-clock terms. New skip reason maturity_window_open for the case where the tick fires after the window has already opened.
35/35 tests in the dispatcher suite, 856/856 across the repo.
…block height Two corrections stacked on e6de877 — one intent bug, one P1 from Codex: 1. ANCHOR CORRECTION (intent). e6de877 anchored reminder bucketing to the superblock itself, so both emails (72h and 24h before "deadline") fired INSIDE Core's 3-day maturity window. That's exactly the wrong time: masternodes begin committing YES-FUNDING trigger votes as soon as the maturity window opens, and per governance.cpp an MN that has voted YES-FUNDING for one trigger cannot switch for the same cycle. A user rushing to vote after our email would find some fraction of MNs already locked in. The actionable voter deadline is the maturity-window START (SB - 3d on mainnet), not the SB. Re-anchor: deadlineSec = nextSbEpochSec - SUPERBLOCK_MATURITY_WINDOW_SEC msRemaining = deadlineSec * 1000 - nowMs bucket = bucketForTimeRemaining(msRemaining) Existing 24h/72h bucket thresholds kept; they now measure time until the maturity window opens, not time until the SB. Wall- clock equivalents vs SB: days_before — 24h < remaining ≤ 72h before maturity opens = ~4–6 days before the superblock final_24h — 0 < remaining ≤ 24h before maturity opens = ~3–4 days before the superblock New skip reason `maturity_window_open` for the case where the tick fires after the window has already opened (msRemaining ≤ 0): suppress rather than send a reminder that asks voters to race a commit decision some MNs have already made. 2. CODEX PR14 P1 (scopeKey stability). scopeKey was `sb:<epochSec>`, but sysMain recomputes superBlockNextEpochSec every 20s as `Date.now() + diffBlock * avgBlockTime`. That estimate drifts forward by ~20s each tick even when the target SB hasn't changed, so the scopeKey moved continuously and reminderLog.has() never returned true. A user who never voted could receive a days_before email every hour for 72 hours straight — 72 duplicate emails per cycle. Replace the single `getNextSuperblockEpochSec` dep with an atomic snapshot getter `getNextSuperblock: () => ({ height, epochSec })`. height (from dataStore.nextSuperBlock) is Core's canonical integer block number for the upcoming SB — stable until the SB executes, at which point it jumps by exactly nSuperblockCycle. Key scope on it: scopeKey = `sb:${height}` epochSec is still used for time-remaining calculation only, where minute-scale drift is harmless (thresholds are in hours). Snapshot is atomic so height and epochSec come from the same sysMain pass — no race where sysMain refreshes between reads. 3. Validation tightened. Missing/zero/non-integer height, or non-finite/past epochSec, all surface as `skipped: next_superblock_unavailable` (next hourly tick retries). /mnStats lagging past the SB (epochSec ≤ nowSec) no longer leaks into bucketing with a past anchor. Tests (35/35 green in the suite; 856/856 across the repo): * new `Codex PR14 P1: scopeKey stability` regression that drifts epochSec forward +20s per tick with stable height and asserts reminderLog.has() correctly deduplicates → only 1 send. * new `maturity_window_open` test verifying reminders are suppressed once we're inside the maturity window. * new `scopeKey rotates on height change` test verifying the legitimate case where the SB advances does produce a fresh scope key. * rewrote mkDispatcher to take `maturityOffsetMs` (time until maturity opens) rather than `sbOffsetMs`, so test intent matches the new anchor. * expanded `getNextSuperblock bogus values` to cover all the shapes the snapshot validator must reject. Server wiring updated to pass { height, epochSec } atomically from dataStore.nextSuperBlock + dataStore.superBlockNextEpochSec. Made-with: Cursor
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Context
After the derive-window wizard shipped (sysnode-info PR#20 + this repo's PR#13
superblock_next_epoch_sec),end_epochon proposals is now intentionally set ~15 days after the last payout superblock so Core prunes cleanly without allocating an extra SB. The old reminder dispatcher keyed its deadline off the earliestend_epoch, which means the "≤72h to close" reminder now fires ~12 days after the actual voting decisions are made — useless at best, misleading at worst.What changed
Reminder deadline now tracks the upcoming superblock — the event that actually matters to voters:
proposal.endEpochnextSuperblockEpochSec(fromdataStore, viasysMain→/mnStats)endEpochstartEpoch <= nextSb <= endEpoch(matches Core's payment window)cycle:<earliestEndEpoch>sb:<nextSbEpochSec>— rotates naturally as the chain advancesnext_superblock_unavailable(fetch throws, zero/NaN, or past timestamp)Fail-closed on a stale/missing SB anchor: if
sysMainhasn't warmed up or/mnStatsis lagging past the superblock, the tick skips rather than fire an email keyed to a bogus/executed SB. Next hourly tick retries.Cycle-vote gate fix (carried forward)
The old dispatcher built
cycleProposalHashesfrom the earliest-endEpoch set, but a vote on any later-cycle proposal would still cancel this cycle's reminder becausehasAnyRelayedInCyclescanned the whole active list. The new implementation restricts the hash set to the SB-eligible subset, so a vote on a future-cycle (pre-activation) proposal cannot suppress this cycle's reminder. Regression test included.Files
Test plan
Made with Cursor