1 files changed, 213 insertions, 12 deletions

diff --git a/src/selftest.cpp b/src/selftest.cpp
index 1a72e3a..357d532 100644
--- a/src/selftest.cpp
+++ b/src/selftest.cpp
@@ -19,6 +19,7 @@
 #include "wallet_sqlite.h"
 
 extern int RunPoolStratumSelfTest();
+extern int RunPayoutLedgerSelfTest();
 
 // Test results go to the terminal, not to debug.log.
 //
@@ -2579,7 +2580,11 @@ static int RunWalletBackendDefaultSelfTest()
 
     try
     {
-        // 1. Fresh datadir, no flag -> new wallet.sqlite, marker "sqlite", no wallet.dat.
+        // 1. Fresh datadir, no flag -> this fork refuses, where upstream creates.
+        //    An empty datadir on a headless node is far more often a mount or
+        //    backup fault than a first run, and creating a wallet also pins the
+        //    backend marker, after which restoring the real wallet is a no-op.
+        //    Nothing may be left behind: no wallet of either kind, no marker.
         string d1 = tmp + "/fresh";
         nFail += Check(MakeDirLocal(d1), "fresh scenario dir can be created") ? 0 : 1;
         vector<string> a1;
@@ -2587,19 +2592,31 @@ static int RunWalletBackendDefaultSelfTest()
         a1.push_back("-nomanagedtor");
         a1.push_back("-nogui");
         a1.push_back("-newaddress");
-        int r1 = RunBitflashChild(strExe, a1);
+        int r0 = RunBitflashChild(strExe, a1);
+        nFail += Check(r0 != 0 &&
+                       !FileExists((d1 + "/wallet.sqlite").c_str()) &&
+                       !FileExists((d1 + "/wallet.dat").c_str()) &&
+                       !FileExists((d1 + "/wallet-backend").c_str()),
+                       "a flagless headless start refuses to create a wallet") ? 0 : 1;
+
+        // 1a. -createwallet says it was meant: new wallet.sqlite, marker
+        //     "sqlite", no wallet.dat.
+        vector<string> a1c = a1;
+        a1c.push_back("-createwallet");
+        int r1 = RunBitflashChild(strExe, a1c);
         nFail += Check(r1 == 0 &&
                        FileExists((d1 + "/wallet.sqlite").c_str()) &&
                        !FileExists((d1 + "/wallet.dat").c_str()) &&
                        FileContainsText(d1 + "/wallet-backend", "sqlite"),
-                       "a fresh install starts on SQLite (new default) and records the marker") ? 0 : 1;
+                       "-createwallet starts a fresh install on SQLite and records the marker") ? 0 : 1;
 
-        // 1b. Second run reuses it, still SQLite, still no wallet.dat.
+        // 1b. Second run reuses it without the flag -- the refusal only guards
+        //     creation, so an existing wallet still starts flagless.
         int r1b = RunBitflashChild(strExe, a1);
         nFail += Check(r1b == 0 &&
                        FileExists((d1 + "/wallet.sqlite").c_str()) &&
                        !FileExists((d1 + "/wallet.dat").c_str()),
-                       "the recorded SQLite choice survives a restart") ? 0 : 1;
+                       "the recorded SQLite choice survives a flagless restart") ? 0 : 1;
 
         // 2. Existing wallet.dat, no flag -> stays on Berkeley DB.
         string d2 = tmp + "/existing-bdb";
@@ -3023,6 +3040,13 @@ static int RunNetMessageSelfTest()
     int nFail = 0;
     try
     {
+        // A peer that has not yet been kept waiting must not look like one that
+        // has been waiting since the epoch. This is the whole bug: the stall
+        // clock lives in CNode, so only the constructor can start it at zero.
+        CNode opened(INVALID_SOCKET, CAddress("127.0.0.1"));
+        nFail += Check(opened.nMsgWaitSince == 0 && opened.nMsgWaitSize == 0,
+                       "a new peer starts with its stall clock stopped") ? 0 : 1;
+
         CNode complete(INVALID_SOCKET, CAddress("127.0.0.1"));
         complete.nVersion = VERSION;
         complete.vRecv << CMessageHeader("ping", 0);
@@ -3037,20 +3061,38 @@ static int RunNetMessageSelfTest()
         nFail += Check(ProcessMessages(&partial), "an incomplete message returns cleanly") ? 0 : 1;
         nFail += Check(partial.vRecv.size() == nPartialBefore,
                        "an incomplete message keeps one header in the buffer") ? 0 : 1;
-        nFail += Check(partial.nIncompleteMessageStart != 0,
-                       "an incomplete message starts a timeout clock") ? 0 : 1;
+        nFail += Check(partial.nMsgWaitSince != 0,
+                       "an incomplete message starts the stall clock") ? 0 : 1;
+        nFail += Check(partial.nMsgWaitSize == 0,
+                       "the stall clock records how much payload had arrived") ? 0 : 1;
         nFail += Check(!partial.fDisconnect,
                        "a fresh incomplete message does not disconnect immediately") ? 0 : 1;
 
+        // Still arriving, just slowly: more payload is here than when the clock
+        // was started, so the clock restarts however old it was. Timing the
+        // wait instead of the stall would drop this peer.
+        CNode slow(INVALID_SOCKET, CAddress("127.0.0.1"));
+        slow.nVersion = VERSION;
+        slow.vRecv << CMessageHeader("block", 100);
+        slow.vRecv.insert(slow.vRecv.end(), 50, '\0');
+        slow.nMsgWaitSince = GetTime() - BTF_MSG_WAIT_MAX_SECS - 1;
+        slow.nMsgWaitSize = 10;
+        ProcessMessages(&slow);
+        nFail += Check(!slow.fDisconnect,
+                       "a peer still sending payload is not dropped") ? 0 : 1;
+        nFail += Check(slow.nMsgWaitSize == 50,
+                       "progress restarts the stall clock") ? 0 : 1;
+
+        // Nothing more has arrived since the clock started, and the clock is
+        // older than the limit.
         CNode stale(INVALID_SOCKET, CAddress("127.0.0.1"));
         stale.nVersion = VERSION;
         stale.vRecv << CMessageHeader("block", 100);
-        stale.nIncompleteMessageStart = GetTime() - BTF_INCOMPLETE_MESSAGE_TIMEOUT_SECS - 1;
-        stale.nIncompleteMessageSize = 100;
-        stale.strIncompleteMessageCommand = "block";
+        stale.nMsgWaitSince = GetTime() - BTF_MSG_WAIT_MAX_SECS - 1;
+        stale.nMsgWaitSize = 0;
         ProcessMessages(&stale);
         nFail += Check(stale.fDisconnect,
-                       "a stale incomplete message disconnects the peer") ? 0 : 1;
+                       "a stalled incomplete message disconnects the peer") ? 0 : 1;
 
         CNode oversized(INVALID_SOCKET, CAddress("127.0.0.1"));
         oversized.nVersion = VERSION;
@@ -3174,6 +3216,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<CTransaction>& 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);
@@ -3197,6 +3307,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<CTransaction> 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<CTransaction> 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<CTransaction> 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)
     {
@@ -3604,6 +3803,8 @@ int RunSelfTest(const std::string& name)
         return RunConsensusLimitsSelfTest();
     if (name == "pool-stratum")
         return RunPoolStratumSelfTest();
+    if (name == "payout-ledger")
+        return RunPayoutLedgerSelfTest();
     if (name == "parse-money")
         return RunParseMoneySelfTest();
     if (name == "socks5-proxy")
@@ -3612,6 +3813,6 @@ int RunSelfTest(const std::string& name)
         return RunManagedTorSelfTest();
 
     printf("Unknown self-test '%s'\n", name.c_str());
-    printf("Known self-tests: wallet-keypool, wallet-hd, wallet-format, wallet-storage-sanity, db-env-reopen, wallet-sqlite, wallet-sqlite-migration, wallet-crypto, wallet-encrypt, wallet-sqlite-encrypt, wallet-backend-default, wallet-convert, wallet-portability, net-message, network-params, consensus-limits, pool-stratum, parse-money, socks5-proxy, managed-tor\n");
+    printf("Known self-tests: wallet-keypool, wallet-hd, wallet-format, wallet-storage-sanity, db-env-reopen, wallet-sqlite, wallet-sqlite-migration, wallet-crypto, wallet-encrypt, wallet-sqlite-encrypt, wallet-backend-default, wallet-convert, wallet-portability, net-message, network-params, consensus-limits, pool-stratum, payout-ledger, parse-money, socks5-proxy, managed-tor\n");
     return 1;
 }