mine: remember whether this node was mining, with the command line winning (#84)

5 files changed, 46 insertions, 15 deletions

diff --git a/src/db.cpp b/src/db.cpp
index 16f2e91..a503cae 100644
--- a/src/db.cpp
+++ b/src/db.cpp
@@ -642,16 +642,29 @@ bool CWalletDB::LoadWallet(vector<unsigned char>& vchDefaultKeyRet)
                 ssKey >> strKey;
                 if (strKey == "nTransactionFee")    ssValue >> nTransactionFee;
                 if (strKey == "addrIncoming")       ssValue >> addrIncoming;
-                // Mining-mode/pool settings (nMineMode, strParticipantPool, strPoolName,
-                // strPoolDashboardUrl, dPoolFeePercent, fGenerateBitcoins) are intentionally
-                // NOT restored here. They used to be, and LoadWallet() runs after CLI flags
-                // are parsed in main_gui.cpp, so a saved value would silently overwrite
-                // whatever was just requested on the command line or in a previous Options
-                // session -- no error, no log line. That's what caused pool name/address to
-                // show correctly in the UI but stay stale in the logs, and mode to change
-                // behavior across restarts. Mode/pool config is decided fresh every launch
-                // instead: CLI flags if given, otherwise the compiled-in default. Nothing to
-                // go stale, nothing to fight over load order.
+
+                // Mining mode is restored, but only when no flag chose one.
+                //
+                // These settings used to be restored unconditionally. LoadWallet()
+                // runs after main_gui.cpp has parsed the command line, so a stored
+                // value silently overwrote what had just been asked for -- no error,
+                // no log line -- and the fix at the time was to stop restoring them
+                // at all. That traded a silent override for a different silent
+                // failure: a machine set to mine came back from every restart as a
+                // plain relay, looking exactly like one that had been asked to
+                // relay. A 32-core miner sat idle for hours that way, and the only
+                // evidence was memory usage that never climbed.
+                //
+                // Precedence is now explicit rather than accidental: a flag wins,
+                // and it says so in the log below. Only the two fields that decide
+                // whether this node mines are restored, plus the pool address a
+                // participant cannot work without.
+                if (!fMineModeFromCommandLine)
+                {
+                    if (strKey == "nMineMode")          ssValue >> nMineMode;
+                    if (strKey == "fGenerateBitcoins")  ssValue >> fGenerateBitcoins;
+                    if (strKey == "strParticipantPool") ssValue >> strParticipantPool;
+                }
             }
         }
     }
@@ -673,8 +686,10 @@ bool CWalletDB::LoadWallet(vector<unsigned char>& vchDefaultKeyRet)
 
     printf("nTransactionFee = %lld\n", nTransactionFee);
     printf("addrIncoming = %s\n", addrIncoming.ToString().c_str());
-    printf("nMineMode = %d, strParticipantPool = %s, fGenerateBitcoins = %d (from CLI/default, not wallet.dat)\n",
-           nMineMode, strParticipantPool.c_str(), fGenerateBitcoins);
+    printf("nMineMode = %d, strParticipantPool = %s, fGenerateBitcoins = %d (%s)\n",
+           nMineMode, strParticipantPool.c_str(), fGenerateBitcoins,
+           fMineModeFromCommandLine ? "from the command line, wallet.dat ignored"
+                                    : "remembered from wallet.dat, or the default");
     printf("strPoolName = %s, dPoolFeePercent = %.2f, strPoolDashboardUrl = %s (from CLI/default, not wallet.dat)\n",
            strPoolName.c_str(), dPoolFeePercent, strPoolDashboardUrl.c_str());
 
diff --git a/src/gui.cpp b/src/gui.cpp
index db6b69e..e2bb5cf 100644
--- a/src/gui.cpp
+++ b/src/gui.cpp
@@ -795,6 +795,13 @@ static void DrawOptionsDialog()
                 if (!vfThreadRunning[3]) StartMinerThreads();
             }
 
+            // Remember the choice. Without this the node came back from every
+            // restart as a plain relay, and a machine that had been mining for
+            // days looked identical to one that had never been asked to.
+            CWalletDB().WriteSetting("nMineMode", nMineMode);
+            CWalletDB().WriteSetting("fGenerateBitcoins", fGenerateBitcoins);
+            CWalletDB().WriteSetting("strParticipantPool", strParticipantPool);
+
             if (nMineMode == MINE_OPERATOR) {
                 if (!gPoolServerRunning) {
                     gPoolServerRunning = true;
diff --git a/src/main.cpp b/src/main.cpp
index 1a4c9ea..f63f575 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -91,6 +91,7 @@ int nMinerThreads = 0;
 int64 nTransactionFee = 0;
 CAddress addrIncoming;
 int    nMineMode        = MINE_RELAY;
+bool   fMineModeFromCommandLine = false;
 string strParticipantPool;           // participant mode: pool .btf address
 string strPoolName       = "Bitflash Pool";
 string strPoolDashboardUrl;
diff --git a/src/main.h b/src/main.h
index 7266e3d..a50485f 100644
--- a/src/main.h
+++ b/src/main.h
@@ -64,9 +64,8 @@ extern CBlockIndex* pindexBest;
 extern unsigned int nTransactionsUpdated;
 extern string strSetDataDir;
 extern int nDropMessagesTest;
-// Mining mode -- persisted in wallet.dat
-// Mining mode -- decided fresh every launch (CLI flags, else default). Never
-// restored from wallet.dat; see LoadWallet() in db.cpp.
+// Mining mode -- remembered in wallet.dat, but the command line always wins.
+// See LoadWallet() in db.cpp for why the precedence has to be explicit.
 //   MINE_RELAY (0):       default. Node runs/syncs, never mines, no pool server.
 //   MINE_SOLO (1):        mine to own wallet, no pool server
 //   MINE_OPERATOR (2):    run pool server, mine to own wallet, distribute to miners
@@ -76,6 +75,10 @@ extern int nDropMessagesTest;
 #define MINE_OPERATOR    2
 #define MINE_PARTICIPANT 3
 extern int    nMineMode;
+// Set by the startup parser when a flag chose the mode. LoadWallet leaves the
+// stored mode alone when this is true, so a flag is never silently overruled by
+// something a previous session wrote.
+extern bool   fMineModeFromCommandLine;
 extern string strParticipantPool; // participant: pool .btf address
 extern string strPoolName;        // operator: announced pool name
 extern string strPoolDashboardUrl; // operator: optional dashboard URL
diff --git a/src/main_gui.cpp b/src/main_gui.cpp
index c593162..5c87fe0 100644
--- a/src/main_gui.cpp
+++ b/src/main_gui.cpp
@@ -97,6 +97,7 @@ static void ParseStartupArguments(int argc, char* argv[])
         // their own mode below, so only the unqualified case is affected.
         if (nMineMode == MINE_RELAY)
             nMineMode = MINE_SOLO;
+        fMineModeFromCommandLine = true;
     }
 
     if (arg(argc,argv,"/solomine") || arg(argc,argv,"-solomine"))
@@ -127,12 +128,16 @@ static void ParseStartupArguments(int argc, char* argv[])
     }
 
     if (arg(argc,argv,"/operator") || arg(argc,argv,"-operator"))
+    {
         nMineMode = MINE_OPERATOR;
+        fMineModeFromCommandLine = true;
+    }
 
     if (arg(argc,argv,"/participant") || arg(argc,argv,"-participant"))
     {
         nMineMode = MINE_PARTICIPANT;
         strParticipantPool = argval2(argc, argv, "/participant", "-participant");
+        fMineModeFromCommandLine = true;
     }
 
     string poolName = argval2(argc, argv, "/poolname", "-poolname");