consensus: cap block size at 1 MB, and stop assembling blocks we would reject

CheckBlock() validated block size against MAX_SIZE, the serializer's generic
32 MB guard, not a block rule. MAX_BLOCK_SIZE did not exist in this tree at
all: Bitcoin's 1 MB cap arrived in 0.3.x, after the 0.1.0 this fork came from.
A miner could produce a 32 MB block and every node would download, verify and
store it. Upstream #111 (from #110 by mtg0001).

The cap is a tightening, so a node that does not upgrade is the permissive one.
Measured upstream over 5709 blocks: largest 34,290 bytes, median 216, so 1 MB
rejects nothing that exists.

Upstream shipped the validation half only. Both places that ASSEMBLE a block
still filled to MAX_SIZE/2 (16 MB), which was consistent against a 32 MB
validation cap and is inverted by a 1 MB one -- the node would solve a block
and then reject it itself. Fixed both:

  main.cpp  CreateNewBlock, the node's own miner
  rpc.cpp   RebuildJob, the POOL's job builder -- the one that matters here,
            because every miner on the pool would be working a template the
            network rejects, and this pool is ~90% of hashrate

Half the cap in both, preserving the original margin: each test runs against a
transaction that is already counted, so the loop can overshoot by one.

(cherry picked from commit a3be9f04903447fa2741aff4eb529fe45f916877)

2 files changed, 20 insertions, 2 deletions

diff --git a/src/main.cpp b/src/main.cpp
index 5d0b70a..37a15c6 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -4226,7 +4226,19 @@ bool BitcoinMiner(int nThreadId)
             vector<char> vfAlreadyAdded(mapTransactions.size());
             bool fFoundSomething = true;
             unsigned int nBlockSize = 0;
-            while (fFoundSomething && nBlockSize < MAX_SIZE/2)
+            // Assembly must stop below the consensus cap that CheckBlock()
+            // enforces, or we mine a block our own node then rejects.
+            //
+            // This used to read MAX_SIZE/2 (16 MB), which was consistent while
+            // CheckBlock() also tested against MAX_SIZE (32 MB). Adding
+            // MAX_BLOCK_SIZE inverted it: assembly could fill 16 MB and
+            // validation would refuse anything over 1 MB, so a full mempool
+            // 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)
             {
                 fFoundSomething = false;
                 unsigned int n = 0;
diff --git a/src/rpc.cpp b/src/rpc.cpp
index da22b1d..03d1f44 100644
--- a/src/rpc.cpp
+++ b/src/rpc.cpp
@@ -253,7 +253,13 @@ static bool RebuildJob()
                     pool = tmp;
                     block.vtx.push_back(tx);
                     sz += ::GetSerializeSize(tx, SER_NETWORK);
-                    if (sz > MAX_SIZE / 2) break;
+                    // Must stay under the consensus cap CheckBlock() enforces.
+                    // This is the pool's job builder, so a block over the cap
+                    // is not just one wasted solve -- it is every miner on the
+                    // pool working a template the network will reject. The
+                    // transaction is already pushed above, so this can overshoot
+                    // by one; half the cap leaves room for that.
+                    if (sz > MAX_BLOCK_SIZE / 2) break;
                 }
             }
         }