From 74c068829967bbff60da3506613844df4da1ae80 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 14:14:48 +0000 Subject: mine: ask whether a transaction fits before appending it The node miner tested its size budget only on the outer while loop. The inner for walked all of mapTransactions with no test, so a single pass could append the entire mempool -- the overshoot was bounded by mempool size, not by one transaction as the comment claimed. The pool builder did test, but only after the push, which bounds the overshoot at one transaction. Nothing caps how big one transaction may be: neither CheckTransaction nor AcceptTransaction limits it, so the mempool will hold one of any size the wire allows and "one transaction" is worth up to MAX_SIZE. A lone 1.5 MB transaction offering a large fee is therefore enough to make every template unmineable, and the fee is never paid because the block never confirms. Both now ask before appending and skip what does not fit, so the overshoot is zero and an oversized transaction costs the template nothing rather than invalidating it. The budget moves to MAX_BLOCK_SIZE_GEN so the two assemblers cannot drift on it again -- drift is what produced this. The test is ordered before ConnectInputs on purpose: ConnectInputs credits nFees by reference on success, so declining a transaction after calling it would leave the coinbase claiming a fee the block does not collect. Assembly only. CheckBlock, CheckTransaction and AcceptTransaction are untouched: this changes what we produce, not what the network accepts. --- src/main.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'src/main.h') diff --git a/src/main.h b/src/main.h index 382dd9c..1f452b5 100644 --- a/src/main.h +++ b/src/main.h @@ -33,6 +33,26 @@ static const unsigned int MAX_BLOCK_SIGOPS = 20000; // limit (32 MB), not a block rule. Bitcoin 0.1.0 predated the later 1 MB block // cap, so this tree inherited no real block-size consensus limit. static const unsigned int MAX_BLOCK_SIZE = 1000000; +// How much of that cap a template we build may fill. Half, which is what both +// assemblers already aimed at -- this only gives the number a name and one +// place to live. They had already drifted on how it was enforced, which is the +// bug this exists to stop repeating. +// +// NOT a consensus rule. CheckBlock() decides what the network accepts; this +// decides what we are willing to produce, and it has to stay under that. +static const unsigned int MAX_BLOCK_SIZE_GEN = MAX_BLOCK_SIZE / 2; + +// Room for one more transaction in a template that currently weighs +// nBlockSize bytes? +// +// Asked BEFORE the transaction is appended, never after. A template that has +// already overshot is one CheckBlock() rejects, and asking afterwards only +// measures the waste. Both assemblers call this, so neither can drift into a +// different idea of what fits. +inline bool BlockTemplateFits(unsigned int nBlockSize, unsigned int nTxSize) +{ + return (nBlockSize + nTxSize <= MAX_BLOCK_SIZE_GEN); +} static const int64 COIN = 100000000; // Total Bitflash emission: identical to Bitcoin (21 million). // MAX_MONEY guards against the value overflow bug (CVE-2010-5139), which in -- cgit v1.2.3