From ff6af901eaab61b6dbba41bb80f7bb2f40980880 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 14:23:50 +0000 Subject: selftest: cover the assembly half of the block limits consensus-limits only ever asked what CheckBlock() refuses. Nothing asked what the assemblers produce, which is why a miner able to overshoot the cap by an entire mempool sat here unnoticed through both the deployed and candidate builds. Adds the other half: a template built from a 3 MB mempool stays inside the budget and passes CheckSizeLimits(); a transaction too big for the budget is skipped while the ones that fit are still taken; sigop-heavy transactions stop selection at MAX_BLOCK_SIGOPS while the block is still small. Plus the rule's edges, including that sigops exactly on the limit fit -- CheckBlock() fails on `>`, so assembly must accept what validation accepts. The test drives BlockTemplateFits the way the assemblers drive it rather than calling them: BitcoinMiner needs a CTxDB, a wallet key and cs_main, and RebuildJob is static in rpc.cpp. What that proves is the shared rule and the property it exists to give, which is where the defect was -- the budget was right and the enforcement was not. Checked against the bug: with the fit test neutered, eight of these fail, including both templates that CheckBlock() would have rejected. With the sigop comparison stricter than CheckBlock's by one, the boundary case fails. --- src/selftest.cpp | 157 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 157 insertions(+) (limited to 'src/selftest.cpp') diff --git a/src/selftest.cpp b/src/selftest.cpp index 9f7106f..d11fdd8 100644 --- a/src/selftest.cpp +++ b/src/selftest.cpp @@ -1235,6 +1235,74 @@ static CBlock MakeSizedConsensusBlock(size_t nScriptBytes) return block; } +// A distinct transaction of a chosen weight and verification cost. nUnique +// only keeps the txids apart, so a mempool built from these is not a pile of +// duplicates a block could never carry anyway. +static CTransaction MakeSizedTx(size_t nPadBytes, unsigned int nSigOps, int nUnique) +{ + CTransaction tx; + tx.vin.push_back(CTxIn(COutPoint(), CScript() << (int64)nUnique << 1)); + tx.vout.push_back(CTxOut(0, CScript())); + for (unsigned int i = 0; i < nSigOps; i++) + tx.vout[0].scriptPubKey << OP_CHECKSIG; + tx.vout[0].scriptPubKey.insert(tx.vout[0].scriptPubKey.end(), nPadBytes, 0); + return tx; +} + +static CBlock MakeCoinbaseOnlyTemplate() +{ + CTransaction txNew; + txNew.vin.resize(1); + txNew.vin[0].prevout.SetNull(); + txNew.vin[0].scriptSig << 1 << 1 << 1; + txNew.vout.resize(1); + txNew.vout[0].scriptPubKey << OP_CHECKSIG; + + CBlock block; + block.vtx.push_back(txNew); + block.nVersion = 1; + block.nTime = GetAdjustedTime(); + block.nBits = bnProofOfWorkLimit.GetCompact(); + block.nNonce = 0; + return block; +} + +// The sum CheckBlock() takes, spelled the same way. +static unsigned int BlockSigOpCount(const CBlock& block) +{ + unsigned int n = 0; + for (int i = 0; i < (int)block.vtx.size(); i++) + n += block.vtx[i].GetSigOpCount(); + return n; +} + +// Drive the selection rule the way both assemblers drive it: ask +// BlockTemplateFits before appending, skip what does not fit, never stop early +// on one that does not. +// +// This mirrors the loops in BitcoinMiner() and RebuildJob() rather than calling +// them -- one needs a CTxDB, a wallet key and cs_main, the other is static in +// rpc.cpp, so neither is reachable from here. What that leaves proven is the +// rule the two of them share and the property it is meant to guarantee, which +// is where the defect actually lived: the budget was right and the enforcement +// was not. +static void SelectIntoTemplate(CBlock& block, const vector& vMempool) +{ + unsigned int nBlockSize = ::GetSerializeSize(block, SER_DISK); + unsigned int nBlockSigOps = block.vtx[0].GetSigOpCount(); + for (int i = 0; i < (int)vMempool.size(); i++) + { + const CTransaction& tx = vMempool[i]; + unsigned int nTxSize = ::GetSerializeSize(tx, SER_NETWORK); + unsigned int nTxSigOps = tx.GetSigOpCount(); + if (!BlockTemplateFits(nBlockSize, nBlockSigOps, nTxSize, nTxSigOps)) + continue; + block.vtx.push_back(tx); + nBlockSize += nTxSize; + nBlockSigOps += nTxSigOps; + } +} + static int RunConsensusLimitsSelfTest() { fflush(stdout); @@ -1258,6 +1326,95 @@ static int RunConsensusLimitsSelfTest() "the test block sits between 1 MB and the old 32 MB cap") ? 0 : 1; nFail += Check(!oversized.CheckSizeLimits(), "a block above 1 MB is outside the consensus size limit") ? 0 : 1; + + // The other half of the cap. Everything above asks what validation + // refuses; nothing asked what assembly produces, which is why a miner + // that could overshoot the cap by an entire mempool went unnoticed. + nFail += Check(MAX_BLOCK_SIZE_GEN < MAX_BLOCK_SIZE, + "the assembly budget stays under the consensus cap") ? 0 : 1; + + // The rule itself, at its edges. The sigop comparison has to match + // CheckBlock(), which fails on `> MAX_BLOCK_SIGOPS` and so accepts a + // block sitting exactly on the limit. + nFail += Check(BlockTemplateFits(MAX_BLOCK_SIZE_GEN - 100, 0, 100, 0), + "a transaction that exactly fills the size budget fits") ? 0 : 1; + nFail += Check(!BlockTemplateFits(MAX_BLOCK_SIZE_GEN - 100, 0, 101, 0), + "one byte past the size budget does not fit") ? 0 : 1; + nFail += Check(BlockTemplateFits(0, 0, 0, MAX_BLOCK_SIGOPS), + "sigops exactly on the limit fit, as CheckBlock() allows") ? 0 : 1; + nFail += Check(!BlockTemplateFits(0, 0, 0, MAX_BLOCK_SIGOPS + 1), + "one sigop past the limit does not fit") ? 0 : 1; + + // A mempool several times the consensus cap, in ordinary-sized + // transactions. The old miner tested its budget only on the outer + // loop, so a single pass appended all of this. + { + CBlock block = MakeCoinbaseOnlyTemplate(); + vector vMempool; + for (int i = 0; i < 300; i++) + vMempool.push_back(MakeSizedTx(10000, 1, i)); + SelectIntoTemplate(block, vMempool); + block.hashMerkleRoot = block.BuildMerkleTree(); + + nFail += Check(block.vtx.size() > 1, + "a full mempool still fills a template") ? 0 : 1; + nFail += Check(::GetSerializeSize(block, SER_DISK) <= MAX_BLOCK_SIZE_GEN, + "a template built from a 3 MB mempool respects the budget") ? 0 : 1; + nFail += Check(block.CheckSizeLimits(), + "that template is one CheckBlock() would accept on size") ? 0 : 1; + } + + // One transaction too big for the budget. Nothing on the path into the + // mempool caps transaction size, so this is a transaction a peer can + // simply send. It must be skipped and the rest still taken -- skipped, + // not stopped on. + { + CBlock block = MakeCoinbaseOnlyTemplate(); + vector vMempool; + vMempool.push_back(MakeSizedTx(1500000, 1, 9999)); + for (int i = 0; i < 10; i++) + vMempool.push_back(MakeSizedTx(1000, 1, i)); + + unsigned int nHugeSize = ::GetSerializeSize(vMempool[0], SER_NETWORK); + nFail += Check(nHugeSize > MAX_BLOCK_SIZE && nHugeSize <= MAX_SIZE, + "the outsized transaction is one the mempool would hold") ? 0 : 1; + + SelectIntoTemplate(block, vMempool); + block.hashMerkleRoot = block.BuildMerkleTree(); + + bool fTookHuge = false; + for (int i = 0; i < (int)block.vtx.size(); i++) + if (block.vtx[i].GetHash() == vMempool[0].GetHash()) + fTookHuge = true; + + nFail += Check(!fTookHuge, + "a transaction larger than the budget is left out") ? 0 : 1; + nFail += Check(block.vtx.size() == 11, + "the ten that do fit are taken anyway") ? 0 : 1; + nFail += Check(block.CheckSizeLimits(), + "one outsized transaction cannot invalidate a template") ? 0 : 1; + } + + // Transactions that are cheap in bytes and expensive to verify. Size + // alone does not bound what a block costs, and CheckBlock() refuses on + // the count regardless of how small the block is. + { + CBlock block = MakeCoinbaseOnlyTemplate(); + vector vMempool; + for (int i = 0; i < 100; i++) + vMempool.push_back(MakeSizedTx(0, 1000, i)); + SelectIntoTemplate(block, vMempool); + block.hashMerkleRoot = block.BuildMerkleTree(); + + nFail += Check(block.vtx.size() > 1, + "sigop-heavy transactions are still selected") ? 0 : 1; + nFail += Check(BlockSigOpCount(block) <= MAX_BLOCK_SIGOPS, + "selection stops at the sigop cap CheckBlock() enforces") ? 0 : 1; + nFail += Check(::GetSerializeSize(block, SER_DISK) < MAX_BLOCK_SIZE_GEN, + "and stops on sigops while still well inside the size budget") ? 0 : 1; + nFail += Check(block.CheckSizeLimits(), + "the sigop-bounded template is within the size limit too") ? 0 : 1; + } } catch (const std::exception& e) { -- cgit v1.2.3