Block accounting: candidates are not blocks, and PPS needs a floor - #44
Merged
Conversation
A share that meets network difficulty makes a block CANDIDATE. submitblock refuses stale, duplicate and high-hash candidates routinely, and on a low-difficulty chain that is nearly all of them — but the result was thrown away. block_submit_fn returned void, so on_block_cb logged the refusal and on_block_found_cb, which has no knowledge of it, filed the candidate in blocks_found with its full reward. That is not a display problem. health.js computes pool solvency as the sum of reward_sats across blocks_found minus credited shares, so phantom rewards make the margin permanently positive and a genuinely insolvent pool reports healthy. In pps-classic that check is the only thing standing between an operator and paying out more than was ever mined. On one alphanet pool the table held 158,326 rows claiming 3,072,992 BTC — roughly 50x everything the chain could have minted — and a sample of 25 was orphaned 25 times out of 25. So thread the result through: block_submit_fn returns int and fills the node's reason, and blocks_found gains status/confirmations/submit_error/checked_via. A refused candidate is recorded as 'rejected' with the reason rather than dropped — a silent reject is how this survived — and an accepted one is only 'pending', because nothing on the submit path can know a block is in the chain. Only 'confirmed' is ever revenue, and nothing here writes it. A failed block assembly now counts as refused too. It never reached the node, so it was the one candidate that could not possibly have been found, and it was being recorded as one anyway. height <= 0 is refused outright. bitcoind_parse_template already rejects a template without a numeric height, so a zero reaching the store means the template was never parsed. The share itself is untouched. The miner did the work at the difficulty they did it at, and in pps-classic absorbing this variance is exactly what the pool is for. The UNIQUE index on hash is deliberately absent: it fails outright on a table that already holds duplicate hashes, and the migration runner only tolerates "duplicate column", so landing it here would leave it silently missing on exactly the databases that need it. It belongs after the reconciliation pass. Confirmation follow-up and the reporting queries are separate commits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An accepted candidate is only 'pending': submitblock says the node took it, not that it stayed. On a low-difficulty chain almost every one loses a race, and nothing was ever asking. getblockhash is the obvious way to ask, and it is the one thing this pool cannot rely on. bitcoind.c speaks getblockchaininfo, getblocktemplate and submitblock, and that is deliberate — the CUSF enforcer, which is the backend a drivechain pool must point at, answers "Method not found" to everything but the latter two, which is already why resolve_network() infers the network from the operator address instead of asking. A confirmation loop built on getblockhash would simply never run where this matters most. So use it where it exists and fall back where it does not. Every getblocktemplate poll is an observation of the node's tip: a template building height H+1 with prev_hash X says the tip at H was X, and `templates` already keeps one row per materially distinct template. That makes it a historical chain of tips the pool itself observed, and one SQL pass over it settles every candidate whose next height was ever seen — no RPC at all. checked_via records which source answered, the way pool_meta.network_source separates an authoritative answer from an inferred one. The pass compares against the LATEST observation at height+1, not merely any of them: after a reorg both the winning and losing prev_hash have been seen at that height, so "some template built on us" would call a reorged-out block confirmed forever. Every non-rejected status is re-examined, so the pass is idempotent and symmetric — confirmed demotes to orphaned when the chain moves on, and an orphan is restored if a later reorg brings it back. Only 'rejected' is terminal: the node never accepted that candidate, so no reorg can put it in the chain. The node path stays narrower (pending/confirmed, shallow only) because re-checking every settled orphan over RPC forever is one call per row per tick, which on this chain is the whole table. Nothing invents a verdict. A candidate neither path can speak to stays pending, and pending counts as nothing. At startup one bulk pass classifies whatever is already on record — rows predating the column are all 'pending', which is correct but useless — and only then is the UNIQUE index on hash created. That ordering is the point: the index fails outright on a table still holding duplicates, and the migration runner swallows anything that is not "duplicate column", so as a migration it would be silently absent on exactly the databases that needed it. The surviving duplicate keeps the earliest sighting but inherits any resolved status its copies reached, so collapsing them cannot lose a confirmation. Competing candidates at one height have different hashes and are all kept — several rows per height is expected here, and status is what stops them counting. Reporting still reads these rows without filtering on status; that is next. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Everything reading blocks_found treated every row as a block the pool mined. With status now on the rows, the counts, the sums and the pages have to follow, or the record is honest and nothing that reads it is. Solvency is the one that matters: health.js computed margin as the sum of reward_sats across the whole table minus credited shares, so candidates the node refused and blocks the chain reorged out funded the pool on paper. It now sums confirmed rows only. A young pps-classic pool will read red here — it credits shares before its first confirmed block — and that is the honest number rather than a fault in the check. shares.is_block was a second, independent block counter: admin.js counted it per worker and per day, so fixing blocks_found alone would have left the admin page still reporting every candidate as a block that miner found. Those counters now require a confirmed blocks_found row. is_block itself is unchanged and still means "this hash met the network target" — it is what the miner did, it stays true whatever the chain later decided, and it is load-bearing on the PPS credit path. Only the verdict shown beside it moves. New health check on how many recent candidates reached the chain. On the alphanet forknet that was effectively zero, and it was invisible precisely because every candidate counted as a block — the pool looked like it was winning constantly while earning nothing. Only settled candidates count toward the ratio; a pool with nothing settled yet has not failed at anything. The near-misses are surfaced, not hidden. Orphaned, rejected and unverified totals sit next to the block count, and every listing carries its verdict. Rows that are not confirmed show no reward, because printing what they would have paid is how 3,072,992 BTC got claimed on a chain that had issued a fiftieth of it. A status never renders blank — a row from before the column existed reads "unverified", which is what it is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The operator-facing audit in CLASSIC_PAYOUTS.md and docs/simplepool.html published the unfiltered solvency query — sum reward_sats across blocks_found, subtract credited shares. That is the bug itself, handed to operators as guidance: anyone checking their pool by hand reproduced the phantom revenue exactly. Both now filter on confirmed, and both gain the query that says how the candidates actually settled. The troubleshooting table in NONCE_AND_SHARES.md still told operators that is_block=1 with no blocks_found row means submitblock failed. There is a row now, and it says 'rejected' with the node's own reason. Replaced with the three states an operator will actually meet, including that rows resting at 'pending' are expected against a backend that does not serve getblockhash. payout/audit.js was a third independent block counter, and this one is right to stay as it is. Block-withholding asks whether a miner quietly discarded the submission that solves a block — so what matters is what they submitted, not what the chain did with it afterwards. A miner whose solution was refused or reorged out has withheld nothing, and filtering on confirmed would flag honest miners on exactly the low-difficulty chains where orphans are routine. Both sides of the ratio come from the same column, so the statistic is unchanged. What was wrong there was only the label: it printed blocks=N, which an operator would compare against a dashboard number that now means something else. Renamed to solutions throughout, with the reason recorded next to it in both READMEs, so the two numbers differing reads as intended rather than as a discrepancy to chase. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The 19.4 BTC average candidate value has no explanation yet, and inspection alone will not produce one. So add the check that decides it. A template reports the coinbase value in one field and per-transaction fees in another, and bitcoind.c reads them independently — so value minus fees is an independent estimate of the subsidy, and the subsidy is not a free parameter. It is 50 BTC halved once an era and nothing else. A difference that is not a halving value means the pool is being told a block is worth something it is not, and the check names both numbers rather than merely going red. Why this is worth a check of its own rather than a one-off query: the two template paths fail differently and only one of them is loud. On the coinbasevalue path the proxy builds the coinbase from this number, so an inflated value is a consensus-invalid block the node refuses — visible now that submitblock's reason is recorded. On the coinbasetxn path, which is the CUSF enforcer and therefore the drivechain case, the backend supplies the coinbase and the block stays valid. Nothing complains. But the same number feeds refresh_pps_rate, so an inflated value overpays every share by the same factor and the pool owes real money it never earned — with the solvency guard, until this branch, unable to say so. Both directions of that parse are covered. Value far above the subsidy means the value field is over-read; value near zero means coinbasetxn's `fee` is being taken as BIP22-strict fees rather than total output value, which is an assumption src/bitcoind.c:390 states in a comment and never verifies. Fees legitimately push a coinbase above the subsidy, so up to 2x passes. The check immediately caught the health fixture claiming a 3.125 BTC block at height 2, where the subsidy is 50 — an inconsistency that had been sitting in the tests. Fixture moved to a height where its value is real. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
find_job released its lock and returned a borrowed pointer. The comment said that was safe "so long as set_job hasn't replaced it", and on a chain serving templates several times a second that assumption does not survive contact: RECENT_JOBS is 8, so the retention ring recycles in seconds and retire_job frees jobs on the tip watcher thread while a submit is still reading one. This reached production. The pps pool's blocks_found holds rows whose height is 0, 2 and 550 on a chain mining past 963,000, two of them carrying rewards of 1.29 million BTC — a freed job's fields, read after the free. The matching shares are entirely normal (difficulty 1.0 and 4.0, credited 3.0938 BTC), because share difficulty comes from the connection while height and value come from the job. No template ever held those numbers; the templates recorded at those exact timestamps read height 963,727+ and value 3.125 BTC. It is not only a reporting problem. is_block is decided by comparing against job->network_target_be, so a freed job can make an ordinary hash look like a solved block — which is how rows at height 2 came to exist at all. So reference count the jobs. find_job retains under the same lock that guards the slot, so nothing can be destroyed between finding it and claiming it, and stratum_job_free becomes the release — every existing caller already held exactly one reference, so their semantics are unchanged. Readers that only touch a job under the lock (send_current_notify, current_net_diff) needed nothing. handle_submit is split so that reference is released on exactly one path. The body has nine exits and a release on each is a leak, or a double free, waiting for the next edit. Also add `make asan`. A lifetime bug here does not crash — it corrupts a field and carries on, which is why this one shipped and then hid inside a table nobody could reconcile. The new regression test aborts under ASan with the refcount removed, at stratum.c in find_job, and passes clean with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first `make asan` run found it. worker_diff_to_target clamped with `if (scaled >= 2^128) scaled = 2^128 - 1.0`, which does nothing at all: a double carries 53 bits of mantissa, so near 2^128 the gap between representable values is 2^75 and 2^128 - 1 rounds straight back to 2^128. The conversion to unsigned __int128 that followed was then out of range. The two plausible results of that undefined conversion are opposites — a zero target rejects every share the pool receives, an all-ones target accepts every share regardless of work — and which one you get is the compiler's choice. Reachable below a worker difficulty of roughly 2.3e-10, which both initial_diff and vardiff_min can be configured to. Write the easiest possible target directly instead. That is what a difficulty that small means, and it is already what the diff <= 0 branch does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
block_value / network_difficulty is a share's expected value, and it is right only while every share the pool produces has a real chance of becoming a block. That needs difficulty calibrated to hashrate. A chain accepts one block per interval however fast work arrives, so once the pool's own difficulty throughput passes a block's worth per interval, the formula is promising blocks that will never be minted — overstating by exactly that ratio, with nothing anywhere to notice. The production pps pool ran at 40.15 TH/s (9,349 difficulty/s) on a forknet that started at difficulty 1 and retargeted upward. It needed difficulty 5,609,561 and had 1. In under four hours it accrued 15,561,471 BTC of liability against 943.60 BTC of confirmed blocks, 84% of it during the five minutes at difficulty 1. Nothing was paid only because the payout worker had not run. Two guards, because each covers the other's blind spot. The floor is the operator's: pps_min_network_difficulty, the difficulty at which this pool alone would find one block per interval — hashrate * block_interval / 2^32. Below it nothing accrues. It works from the very first share, which is what the ceiling cannot do. The ceiling is automatic and needs no configuration: the chain mints one block_value per interval across every miner in existence, so no pool can earn faster than that. Given the pool's observed difficulty throughput the highest defensible rate is (value/interval)/throughput. It needs a minute of history, so it is the backstop, not the primary guard. Replayed against the real difficulty-1 window it brings 13 million BTC down to the 0.55 blocks the chain actually minted, while still paying miners something. While the floor holds accrual off, the pool refuses mining.authorize and rejects submits by default. Accepting shares it has already decided not to credit means miners hashing for free with no way to tell — worse than being turned away, which at least lets them go elsewhere. Operators who know their miners can switch that off. Both are reported rather than left silent: the proxy logs the difficulty it observes to be necessary even when no floor is configured, and the dashboard computes the same check independently from the shares table, so it is a check on the proxy's number rather than a repetition of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
blocks_foundwas written for every share that met network difficulty, whateverthe node said and whatever the chain did next. On a low-difficulty chain that is
nearly every candidate, and because
reward_satsfeeds the solvency check, thepool's only insolvency guard was being funded by blocks that never existed.
Investigating that against the production
pps-classicdatabase turned up threemore defects, two of them worse than the one we set out to fix.
Verified against the production pool
A 62MB snapshot of the live
pps-classicpool (233,275 shares, 158,326candidate rows, 3h52m). Every number below is measured, not estimated.
The backfill classified 99.8% of the table with no RPC: 305 confirmed, 157,734
orphaned, 287 pending.
1 — Candidates recorded as blocks
block_submit_fnreturnedvoid, sosubmitblock's verdict was logged anddiscarded while the candidate was filed as a found block regardless. It now
returns the node's reason, and
blocks_foundcarriesstatus/confirmations/submit_error/checked_via. Refused →rejectedwiththe reason; accepted →
pendingonly, because nothing on the submit path canknow a block is in the chain.
Confirmation prefers
getblockhash, but the CUSF enforcer — the backend adrivechain pool must use — serves only
getblocktemplateandsubmitblock. Sothere is a fallback with no RPC at all: a template building height H+1 with
prev_hashX says the tip at H was X, andtemplatesalready records that.checked_viasays which answered. Anything neither can speak to stayspendingand counts as nothing.
Counts, sums and pages now filter on
confirmed, including the separateshares.is_blockcounters inadmin.js.payout/audit.jsdeliberately doesnot — block-withholding is about what a miner submitted — so its counters are
renamed to
solutionsto stop the two numbers reading as a discrepancy.2 — Use-after-free in
find_job(memory safety)find_jobreleased its lock and returned a borrowed pointer;retire_jobfreesjobs from the tip-watcher thread. With
RECENT_JOBSat 8 and templates arrivingseveral times a second, the ring recycles in seconds.
This reached production: rows at heights 0, 2 and 550 on a chain mining past
963,000, two carrying rewards of 1.29M BTC. Their shares are entirely normal
(difficulty 1.0/4.0, credited 3.0938 BTC) because share difficulty comes from
the connection while height and value come from the job. No template ever held
those values. A freed
network_target_becan also make an ordinary hash looklike a solved block, which is how the rows existed at all.
Jobs are now reference counted, with the retain under the lock that guards the
slot. Proven with the new regression test built against a neutered refcount:
Added
make asan, because this class of bug corrupts a field rather thancrashing — which is exactly how it shipped and then hid in a table nobody could
reconcile.
3 — Undefined behaviour in
worker_diff_to_targetFound by the first
make asanrun. The clampif (scaled >= 2^128) scaled = 2^128 - 1.0does nothing: a double has 53mantissa bits, so near 2^128 the spacing is 2^75 and
2^128 - 1rounds back to2^128. The conversion tounsigned __int128was out of range. The twoplausible outcomes are opposites — a zero target rejects every share, an
all-ones target accepts every share regardless of work — and the compiler
picks. Reachable below worker difficulty ~2.3e-10, which
initial_diffandvardiff_mincan both be set to. Now saturates.4 — PPS on a chain the pool can outrun
block_value / network_difficultyis a share's expected value, correct onlywhile the pool's solutions can actually become blocks. A chain accepts one block
per interval however fast work arrives, so once the pool's difficulty throughput
passes a block's worth per interval, the formula promises blocks that will never
be minted.
The production pool ran at 40.15 TH/s (9,349 difficulty/s) on a forknet that
started at difficulty 1. It needed difficulty 5,609,561 and had 1. It accrued
15,561,471 BTC against 943.60 BTC mined — 84% of it in the five minutes at
difficulty 1. Nothing was paid only because the payout worker had not run.
Two guards, each covering the other's blind spot:
pps_min_network_difficulty, the difficulty at which this poolalone would find one block per interval (
hashrate * interval / 2^32). Belowit nothing accrues. Works from the first share.
block_valueperinterval across all miners, so the highest defensible rate is
(value/interval)/throughput. Needs ~60s of history, so it is the backstop.Replayed against the real difficulty-1 window, the ceiling brings 13 million BTC
down to the 0.55 blocks the chain actually minted, while still paying miners.
While the floor holds accrual off the pool refuses
mining.authorizeandrejects submits by default: accepting shares it has decided not to credit means
miners hashing for free with no way to tell. Operators who know their miners can
switch it off.
Health checks added
value - feesis an independentestimate of the subsidy, and the subsidy is not a free parameter. Caught an
inconsistency sitting in the test fixtures on its first run.
sharestable,so it checks the proxy's number rather than repeating it.
effectively 0% and invisible.
Testing
C 166/63/91 plus store/coinbase/broadcast/thunder · dashboard 113 · payout 56 ·
all clean under ASan + UBSan · clean build under
-Werror.Notes for review
pps_min_network_difficultydefaults to 0 (off), so existing mainnetdeployments do not break on upgrade. That leaves a fresh forknet deploy
unprotected unless someone reads the config. Making it required when
pool_mode = pps-classicis a one-line change and there is a good argumentfor it — worth a decision here.
hashis created after the backfill, not as amigration.
CREATE UNIQUE INDEXfails outright on a table holdingduplicates and the migration runner only tolerates
duplicate column, so as amigration it would be silently absent on exactly the databases that needed it.
The production data confirms the shape: 8,938 duplicate heights, 0
duplicate hashes.
as guidance. Corrected in
CLASSIC_PAYOUTS.md,docs/simplepool.html,NONCE_AND_SHARES.mdandREADME.md.🤖 Generated with Claude Code