wallet: generate keys ahead of use so a backup outlives the next block (#85)

The wallet made a key per address and per block mined, so a backup only ever
contained the keys that existed the moment it was taken -- mine one block and
the coin lands on a key the backup has never heard of. The README warned about
it, which is an admission rather than a fix.

Keys are now generated a hundred at a time and handed out one at a time, each
written to wallet.dat before it can be pointed at. A backup covers the next
hundred addresses. Deterministic derivation (#47) stays open; this buys back the
distance between a backup and the next mistake.

5 files changed, 118 insertions, 13 deletions

diff --git a/src/db.cpp b/src/db.cpp
index a503cae..3af87fb 100644
--- a/src/db.cpp
+++ b/src/db.cpp
@@ -636,6 +636,14 @@ bool CWalletDB::LoadWallet(vector<unsigned char>& vchDefaultKeyRet)
             {
                 ssValue >> vchDefaultKeyRet;
             }
+            else if (strType == "pool")
+            {
+                int64 nIndex;
+                ssKey >> nIndex;
+                vector<unsigned char> vchPubKey;
+                ssValue >> vchPubKey;
+                mapKeyPool[nIndex] = vchPubKey;
+            }
             else if (strType == "setting")  /// or settings or option or options or config?
             {
                 string strKey;
diff --git a/src/db.h b/src/db.h
index 22efc77..f51047e 100644
--- a/src/db.h
+++ b/src/db.h
@@ -386,6 +386,19 @@ public:
         return Write(string("defaultkey"), vchPubKey);
     }
 
+    // Keys generated ahead of time and not yet handed out. The private key of
+    // each is already stored under its own "key" record by AddKey; a "pool"
+    // record only says that this one is still unspoken for.
+    bool WritePool(int64 nIndex, const vector<unsigned char>& vchPubKey)
+    {
+        return Write(make_pair(string("pool"), nIndex), vchPubKey);
+    }
+
+    bool ErasePool(int64 nIndex)
+    {
+        return Erase(make_pair(string("pool"), nIndex));
+    }
+
     template<typename T>
     bool ReadSetting(const string& strKey, T& value)
     {
diff --git a/src/main.cpp b/src/main.cpp
index f63f575..b027cef 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -175,6 +175,58 @@ vector<unsigned char> GenerateNewKey()
     return key.GetPubKey();
 }
 
+CCriticalSection cs_keyPool;
+map<int64, vector<unsigned char> > mapKeyPool;
+
+void TopUpKeyPool()
+{
+    CRITICAL_BLOCK(cs_keyPool)
+    {
+        while ((int)mapKeyPool.size() < KEYPOOL_SIZE)
+        {
+            // Indices only ever go up. Reusing one would mean two pool records
+            // racing for the same slot after an erase.
+            int64 nIndex = 1;
+            if (!mapKeyPool.empty())
+                nIndex = (--mapKeyPool.end())->first + 1;
+
+            CKey key;
+            key.MakeNewKey();
+            // AddKey writes the private key. Order matters: the key is in
+            // wallet.dat before anything can point at it, so a crash between
+            // the two writes costs an unused key, never a usable one.
+            if (!AddKey(key))
+                return;
+            vector<unsigned char> vchPubKey = key.GetPubKey();
+            if (!CWalletDB().WritePool(nIndex, vchPubKey))
+                return;
+            mapKeyPool[nIndex] = vchPubKey;
+        }
+    }
+}
+
+vector<unsigned char> GetKeyFromPool()
+{
+    CRITICAL_BLOCK(cs_keyPool)
+    {
+        TopUpKeyPool();
+        if (!mapKeyPool.empty())
+        {
+            map<int64, vector<unsigned char> >::iterator mi = mapKeyPool.begin();
+            int64 nIndex = mi->first;
+            vector<unsigned char> vchPubKey = mi->second;
+            mapKeyPool.erase(mi);
+            CWalletDB().ErasePool(nIndex);
+            return vchPubKey;
+        }
+    }
+    // Nothing in the pool and nothing could be added -- disk full, wallet
+    // locked open elsewhere. Generating on demand is what this code did for
+    // its whole life, so fall back to it rather than refuse to mine.
+    printf("GetKeyFromPool() : pool empty, generating a key on demand\n");
+    return GenerateNewKey();
+}
+
 
 
 
@@ -3061,8 +3113,11 @@ bool BitcoinMiner(int nThreadId)
         RandomXInitDataset(nThreads > 0 ? (int)nThreads : 1);
     }
 
-    CKey key;
-    key.MakeNewKey();
+    // Drawn from the pool, so it is already in wallet.dat before a single hash
+    // is computed. The old code generated a key here and only saved it if this
+    // thread happened to win a block, which meant a backup taken while mining
+    // could not spend what the very next block paid.
+    vector<unsigned char> vchPubKey = GetKeyFromPool();
     CBigNum bnExtraNonce = 0;
     while (fGenerateBitcoins)
     {
@@ -3113,7 +3168,7 @@ bool BitcoinMiner(int nThreadId)
         txNew.vout.resize(1);
         // Always pay coinbase to own wallet key. In operator mode, the pool
         // server distributes shares to miners via SendMoney() after each block.
-        txNew.vout[0].scriptPubKey << key.GetPubKey() << OP_CHECKSIG;
+        txNew.vout[0].scriptPubKey << vchPubKey << OP_CHECKSIG;
 
 
         //
@@ -3234,13 +3289,13 @@ bool BitcoinMiner(int nThreadId)
                     }
                     else
                     {
-                        // Save key
-                        if (!AddKey(key))
-                        {
-                            RandomXDestroyMinerVM(rxvm);
-                            return false;
-                        }
-                        key.MakeNewKey();
+                        // The key that just got paid was written to wallet.dat
+                        // when it left the pool, so there is nothing to save
+                        // here -- only a fresh one to take for the next block.
+                        // Two threads never draw the same key: the pool hands
+                        // them out one at a time under cs_keyPool, which is
+                        // what keeps concurrent coinbases distinct (#58).
+                        vchPubKey = GetKeyFromPool();
 
                         // Process this block the same as if we had received it from another node
                         if (!ProcessBlock(NULL, pblock.release()))
diff --git a/src/main.h b/src/main.h
index a50485f..1d9efac 100644
--- a/src/main.h
+++ b/src/main.h
@@ -124,6 +124,32 @@ FILE* OpenBlockFile(unsigned int nFile, unsigned int nBlockPos, const char* pszM
 FILE* AppendBlockFile(unsigned int& nFileRet);
 bool AddKey(const CKey& key);
 vector<unsigned char> GenerateNewKey();
+
+// --- Key pool -------------------------------------------------------------
+//
+// A backup of this wallet only ever contained the keys that existed the moment
+// it was taken. Mine a block or take a new address afterwards and the coin
+// lands on a key the backup has never heard of -- it is on the chain, and the
+// file you carefully put away cannot spend it. The LEIA-ME shipped with a
+// warning about this, which is an admission, not a fix.
+//
+// Keys are now generated a hundred at a time and handed out one by one, so a
+// backup covers the next hundred addresses this node will use. It is the same
+// answer Bitcoin arrived at, and it is not deterministic derivation (#47) --
+// it just buys back the distance between a backup and the next mistake.
+static const int KEYPOOL_SIZE = 100;
+extern CCriticalSection cs_keyPool;
+extern map<int64, vector<unsigned char> > mapKeyPool;
+
+// Fill the pool back up to KEYPOOL_SIZE. Every key it creates is written to
+// wallet.dat before it is offered to anybody.
+void TopUpKeyPool();
+
+// Hand out the oldest unused key. Falls back to generating one on the spot if
+// the pool cannot be filled, which is exactly what this code did before the
+// pool existed -- a wallet that cannot pre-generate should still be able to
+// mine.
+vector<unsigned char> GetKeyFromPool();
 bool AddToWallet(const CWalletTx& wtxIn);
 void ReacceptWalletTransactions();
 // Returns how many wallet transactions the chain corrected.
diff --git a/src/rpc.cpp b/src/rpc.cpp
index 3db8b94..97d8429 100644
--- a/src/rpc.cpp
+++ b/src/rpc.cpp
@@ -146,7 +146,9 @@ static bool RebuildJob()
         CBlockIndex* pindexPrev = pindexBest;
         unsigned int nBits = GetNextWorkRequired(pindexPrev);
 
-        CKey key; key.MakeNewKey();
+        // From the pool, so the template's payout key is in wallet.dat before
+        // any miner is handed work against it.
+        vector<unsigned char> vchPubKey = GetKeyFromPool();
         static std::atomic<uint32_t> sExtra{0};
         uint32_t extraNonce = ++sExtra;
 
@@ -160,7 +162,7 @@ static bool RebuildJob()
         // is mined and submitted like any other. #58.
         txNew.vin[0].scriptSig << (pindexPrev ? pindexPrev->nHeight + 1 : 0) << nBits << (CBigNum)extraNonce;
         txNew.vout.resize(1);
-        txNew.vout[0].scriptPubKey << key.GetPubKey() << OP_CHECKSIG;
+        txNew.vout[0].scriptPubKey << vchPubKey << OP_CHECKSIG;
 
         CBlock block;
         block.vtx.push_back(txNew);
@@ -194,7 +196,8 @@ static bool RebuildJob()
                           (unsigned int)GetAdjustedTime());
         block.nBits  = nBits;
         block.nNonce = 1;
-        AddKey(key);
+        // No AddKey here: the pool wrote this key to wallet.dat before it was
+        // handed out.
 
         StratumJob job;
         job.jobId       = ToHex(&extraNonce, 4);