1 files changed, 49 insertions, 5 deletions

diff --git a/src/main.cpp b/src/main.cpp
index 6b8e0de..018627d 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -4949,7 +4949,6 @@ bool BitcoinMiner(int nThreadId)
             map<uint256, CTxIndex> mapTestPool;
             vector<char> vfAlreadyAdded(mapTransactions.size());
             bool fFoundSomething = true;
-            unsigned int nBlockSize = 0;
             // Assembly must stop below the consensus cap that CheckBlock()
             // enforces, or we mine a block our own node then rejects.
             //
@@ -4960,9 +4959,30 @@ bool BitcoinMiner(int nThreadId)
             // meant solving a block and throwing it away -- with this pool at
             // ~90% of hashrate, that is the network stalling, not just us.
             //
-            // Half the cap, keeping the original margin: the test runs before
-            // a transaction is added, so the loop can overshoot by one tx.
-            while (fFoundSomething && nBlockSize < MAX_BLOCK_SIZE/2)
+            // The budget test then sat only on this outer while, and the
+            // comment claimed that bounded the overshoot to a single
+            // transaction. It did not. The inner for walks all of
+            // mapTransactions with no test of its own, so one pass could append
+            // the entire mempool: the overshoot was bounded by mempool size,
+            // which is to say not bounded at all. Ask per transaction, before
+            // appending, and the overshoot is zero rather than small.
+            //
+            // Seeded with what the block already weighs -- header, tx count,
+            // coinbase -- because CheckBlock() weighs the whole serialized
+            // block and not just the transactions we chose to add to it.
+            unsigned int nBlockSize = ::GetSerializeSize(*pblock, SER_DISK);
+            // Sigops are a budget too, and CheckBlock() enforces one that no
+            // assembler here counted against. Bytes do not bound validation
+            // cost: every OP_CHECKSIG is an elliptic-curve verification, so a
+            // template well inside the size limit can still be refused for
+            // demanding more work than a block is allowed to. Counted with
+            // CTransaction::GetSigOpCount, the function CheckBlock() itself
+            // sums, so the two cannot disagree about what a transaction costs.
+            //
+            // Seeded from the coinbase because CheckBlock() sums over all of
+            // vtx, coinbase included.
+            unsigned int nBlockSigOps = pblock->vtx[0].GetSigOpCount();
+            while (fFoundSomething && nBlockSize < MAX_BLOCK_SIZE_GEN)
             {
                 fFoundSomething = false;
                 unsigned int n = 0;
@@ -4974,6 +4994,29 @@ bool BitcoinMiner(int nThreadId)
                     if (tx.IsCoinBase() || !tx.IsFinal())
                         continue;
 
+                    // Skip, not break: a transaction too big for the room left
+                    // says nothing about the next one, which may still fit.
+                    //
+                    // This is also the only ceiling on a single transaction's
+                    // size anywhere on the path into a block. CheckTransaction
+                    // does not cap it and neither does AcceptTransaction, so
+                    // the mempool will hold one of any size the wire allows;
+                    // without this test a lone 1.5 MB transaction offering a
+                    // fat fee is selected, and every template built from that
+                    // mempool is one CheckBlock() throws away. The fee is never
+                    // paid, because the block never confirms.
+                    //
+                    // Deliberately before ConnectInputs and not after:
+                    // ConnectInputs adds this transaction's fee to nFees by
+                    // reference on success. Passing it and then declining the
+                    // transaction would credit the coinbase for a fee no
+                    // transaction in the block pays, and the block would be
+                    // rejected for a wholly different reason.
+                    unsigned int nTxSize   = ::GetSerializeSize(tx, SER_NETWORK);
+                    unsigned int nTxSigOps = tx.GetSigOpCount();
+                    if (!BlockTemplateFits(nBlockSize, nBlockSigOps, nTxSize, nTxSigOps))
+                        continue;
+
                     // Transaction fee requirements, mainly only needed for flood control
                     // Under 10K (about 80 inputs) is free for first 100 transactions
                     // Base rate is 0.01 per KB
@@ -4985,7 +5028,8 @@ bool BitcoinMiner(int nThreadId)
                     swap(mapTestPool, mapTestPoolTmp);
 
                     pblock->vtx.push_back(tx);
-                    nBlockSize += ::GetSerializeSize(tx, SER_NETWORK);
+                    nBlockSize   += nTxSize;
+                    nBlockSigOps += nTxSigOps;
                     vfAlreadyAdded[n] = true;
                     fFoundSomething = true;
                 }