mine: count sigops while selecting, not only when validating

CheckBlock() caps a block at MAX_BLOCK_SIGOPS and neither assembler counted
against it. Bytes do not bound what a block costs to verify: every OP_CHECKSIG
is an elliptic-curve verification, so a template comfortably inside the size
budget can still be one the network refuses for demanding more work than a
block is allowed to.

Counted with CTransaction::GetSigOpCount -- the same function CheckBlock()
sums, not a second implementation of it -- and seeded from the coinbase,
because CheckBlock() sums over all of vtx. The comparison matches too:
CheckBlock() fails on `> MAX_BLOCK_SIGOPS`, so a block exactly on the limit is
valid and selection accepts it. Assembly and validation differing by one is
the shape of the bug this series started with.

Unlike the size budget this takes no margin. There is no use for headroom in a
sigop count, so assembly holds itself to the consensus number exactly.

Assembly only; MAX_BLOCK_SIGOPS and the CheckBlock() test are unchanged.

3 files changed, 38 insertions, 9 deletions

diff --git a/src/main.cpp b/src/main.cpp
index 64eb290..4d8031d 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -4492,6 +4492,17 @@ bool BitcoinMiner(int nThreadId)
             // 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;
@@ -4522,8 +4533,9 @@ bool BitcoinMiner(int nThreadId)
                     // 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);
-                    if (!BlockTemplateFits(nBlockSize, nTxSize))
+                    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
@@ -4537,7 +4549,8 @@ bool BitcoinMiner(int nThreadId)
                     swap(mapTestPool, mapTestPoolTmp);
 
                     pblock->vtx.push_back(tx);
-                    nBlockSize += nTxSize;
+                    nBlockSize   += nTxSize;
+                    nBlockSigOps += nTxSigOps;
                     vfAlreadyAdded[n] = true;
                     fFoundSomething = true;
                 }
diff --git a/src/main.h b/src/main.h
index 1f452b5..30e25ef 100644
--- a/src/main.h
+++ b/src/main.h
@@ -43,15 +43,24 @@ static const unsigned int MAX_BLOCK_SIZE = 1000000;
 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?
+// nBlockSize bytes and spends nBlockSigOps signature operations?
 //
 // 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)
+//
+// Size gets a margin (half the cap); sigops do not. There is no cheap way to
+// spend the sigop budget usefully and no reason to reserve headroom in it, so
+// assembly holds itself to the same number CheckBlock() enforces -- and to the
+// same comparison. CheckBlock() fails on `> MAX_BLOCK_SIGOPS`, so a block
+// sitting exactly on the limit is valid and this has to accept it too.
+// Assembly and validation disagreeing by one is how the size bug started.
+inline bool BlockTemplateFits(unsigned int nBlockSize, unsigned int nBlockSigOps,
+                              unsigned int nTxSize, unsigned int nTxSigOps)
 {
-    return (nBlockSize + nTxSize <= MAX_BLOCK_SIZE_GEN);
+    return (nBlockSize + nTxSize <= MAX_BLOCK_SIZE_GEN &&
+            nBlockSigOps + nTxSigOps <= MAX_BLOCK_SIGOPS);
 }
 static const int64 COIN = 100000000;
 // Total Bitflash emission: identical to Bitcoin (21 million).
diff --git a/src/rpc.cpp b/src/rpc.cpp
index 7c5ef6c..62eec7a 100644
--- a/src/rpc.cpp
+++ b/src/rpc.cpp
@@ -284,6 +284,11 @@ static bool RebuildJob()
                 // Seeded with the header, tx count and coinbase, because
                 // CheckBlock() weighs the whole serialized block.
                 unsigned int sz = ::GetSerializeSize(block, SER_DISK);
+                // And the sigop budget, which CheckBlock() enforces and this
+                // builder never counted against. Same counting function
+                // CheckBlock() sums, seeded from the coinbase because it sums
+                // over all of vtx.
+                unsigned int nSigOps = block.vtx[0].GetSigOpCount();
                 while (fFoundSomething && sz < MAX_BLOCK_SIZE_GEN)
                 {
                     fFoundSomething = false;
@@ -301,8 +306,9 @@ static bool RebuildJob()
                         // 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))
+                        unsigned int nTxSize   = ::GetSerializeSize(tx, SER_NETWORK);
+                        unsigned int nTxSigOps = tx.GetSigOpCount();
+                        if (!BlockTemplateFits(sz, nSigOps, nTxSize, nTxSigOps))
                             continue;
                         map<uint256, CTxIndex> tmp(pool);
                         if (!tx.ConnectInputs(txdb, tmp, CDiskTxPos(1,1,1), 0, nFees,
@@ -310,7 +316,8 @@ static bool RebuildJob()
                             continue;
                         swap(pool, tmp);
                         block.vtx.push_back(tx);
-                        sz += nTxSize;
+                        sz      += nTxSize;
+                        nSigOps += nTxSigOps;
                         vfAlreadyAdded[n] = true;
                         fFoundSomething = true;
                     }