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/rpc.cpp | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) (limited to 'src/rpc.cpp') diff --git a/src/rpc.cpp b/src/rpc.cpp index a4b53ca..7c5ef6c 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -267,16 +267,24 @@ static bool RebuildJob() map pool; vector vfAlreadyAdded(mapTransactions.size()); bool fFoundSomething = true; - unsigned int sz = 0; - // The cap is MAX_BLOCK_SIZE/2, not MAX_SIZE/2. MAX_SIZE is the - // 32 MB serialisation ceiling, so half of it is 16 MB -- this - // builder would happily assemble a template that its own + // The cap is MAX_BLOCK_SIZE_GEN, not MAX_SIZE/2. MAX_SIZE is + // the 32 MB serialisation ceiling, so half of it is 16 MB -- + // this builder would happily assemble a template that its own // CheckBlock() rejects at the 1 MB consensus cap. This is the // pool's job builder, so a block over the cap is not one // wasted solve, it is every miner on the pool working a - // template the network will reject. The inner break bounds the - // overshoot to a single transaction within a pass. - while (fFoundSomething && sz < MAX_BLOCK_SIZE / 2) + // template the network will reject. + // + // The inner break used to bound the overshoot to one + // transaction per pass. One transaction is still too many: + // nothing caps how big a single transaction may be, so that + // bound was worth up to MAX_SIZE. Asking whether each one fits + // before appending it bounds the overshoot at zero instead. + // + // Seeded with the header, tx count and coinbase, because + // CheckBlock() weighs the whole serialized block. + unsigned int sz = ::GetSerializeSize(block, SER_DISK); + while (fFoundSomething && sz < MAX_BLOCK_SIZE_GEN) { fFoundSomething = false; unsigned int n = 0; @@ -287,16 +295,24 @@ static bool RebuildJob() continue; CTransaction& tx = (*mi).second; if (tx.IsCoinBase() || !tx.IsFinal()) continue; + // Skip rather than stop: one transaction too big for + // the room left does not mean the next is. Ordered + // before ConnectInputs because ConnectInputs credits + // nFees by reference on success, and a transaction we + // then decline would leave the coinbase claiming a fee + // this block does not collect. + unsigned int nTxSize = ::GetSerializeSize(tx, SER_NETWORK); + if (!BlockTemplateFits(sz, nTxSize)) + continue; map tmp(pool); if (!tx.ConnectInputs(txdb, tmp, CDiskTxPos(1,1,1), 0, nFees, false, true, tx.GetMinFee(block.vtx.size() < 100))) continue; swap(pool, tmp); block.vtx.push_back(tx); - sz += ::GetSerializeSize(tx, SER_NETWORK); + sz += nTxSize; vfAlreadyAdded[n] = true; fFoundSomething = true; - if (sz > MAX_BLOCK_SIZE / 2) break; } } } -- cgit v1.2.3