1 files changed, 165 insertions, 8 deletions

diff --git a/src/rpc.cpp b/src/rpc.cpp
index b0436ba..7d7cc8f 100644
--- a/src/rpc.cpp
+++ b/src/rpc.cpp
@@ -583,6 +583,55 @@ static bool WriteFileAtomic(const std::string& path, const std::string& data,
     return true;
 }
 
+// Paths whose loader threw part-way through, and which must therefore never be
+// written again by this process.
+//
+// All three loaders push entries into the live structure INSIDE the try, so an
+// exception half way down a file leaves a PARTIAL ledger in memory -- the
+// entries before the bad one, and nothing after it. The first publish after
+// that then writes the partial state over the file, and the only record of
+// what was owed is gone. ReconcilePayouts publishes unconditionally at
+// startup, so "the first publish" is a few milliseconds later, before an
+// operator could possibly intervene. a8e2f58 named this failure mode in its
+// own commit message and then only fixed the write side.
+//
+// The file is renamed out of the way instead of being left in place, so the
+// evidence survives even if the operator restarts into the same fault, and so
+// a node that comes up owing nobody anything is visibly missing its ledger
+// rather than quietly appearing to have none.
+static std::set<std::string> gRefusedFiles;
+static std::mutex            gRefusedFilesMutex;
+
+static bool FileIsRefused(const std::string& path)
+{
+    std::lock_guard<std::mutex> lk(gRefusedFilesMutex);
+    return gRefusedFiles.count(path) > 0;
+}
+
+// Quarantine `path` and refuse to write it for the rest of the process.
+//
+// Prints unconditionally: an operator who has just lost a payout ledger must
+// be told at whatever log settings they were already running with, and the
+// name it was moved to is the only way back to the data.
+static void RefuseFileAfterLoadFailure(const std::string& path, const char* what,
+                                       const char* why)
+{
+    {
+        std::lock_guard<std::mutex> lk(gRefusedFilesMutex);
+        gRefusedFiles.insert(path);
+    }
+    std::string bad = strprintf("%s.bad.%lld", path.c_str(), (long long)GetTime());
+    bool moved = (rename(path.c_str(), bad.c_str()) == 0);
+    printf("[payout] ERROR: %s (%s) did not parse: %s\n"
+           "[payout] ERROR: what loaded before the error is a PARTIAL picture,"
+           " so this file will not be written again by this process --"
+           " restart once the cause is fixed\n"
+           "[payout] ERROR: the file %s\n",
+           path.c_str(), what, why,
+           moved ? strprintf("has been kept as %s", bad.c_str()).c_str()
+                 : "could NOT be renamed out of the way -- copy it now");
+}
+
 // Publish a versioned image of some in-memory state to `path`.
 //
 // `fileMutex` serialises writers of `path` and guards `publishedSeq`. A blob
@@ -595,6 +644,11 @@ static void PublishVersioned(const std::string& path, const VersionedBlob& v,
                              std::mutex& fileMutex, uint64& publishedSeq,
                              const char* what)
 {
+    // A file whose loader threw is not a file this process knows the contents
+    // of. Writing it would replace a full record of what was owed with
+    // whatever happened to parse before the error.
+    if (FileIsRefused(path))
+        return;
     std::lock_guard<std::mutex> lk(fileMutex);
     if (v.seq <= publishedSeq)
         return;
@@ -760,8 +814,7 @@ static void LoadPendingPayouts()
         LogPrint("payout", "[payout] loaded %zu pending payout(s) from disk\n",
                  gPendingPayouts.size());
     } catch (const std::exception& e) {
-        LogPrint("payout", "[payout] WARNING: failed to parse %s: %s\n",
-                 path.c_str(), e.what());
+        RefuseFileAfterLoadFailure(path, "pending payout ledger", e.what());
     }
 }
 
@@ -948,8 +1001,7 @@ static void LoadPaying()
         LogPrint("payout", "[payout] loaded %zu in-flight payment(s) from disk\n",
                  gPaying.size());
     } catch (const std::exception& e) {
-        LogPrint("payout", "[payout] WARNING: failed to parse %s: %s\n",
-                 path.c_str(), e.what());
+        RefuseFileAfterLoadFailure(path, "in-flight payment list", e.what());
     }
 }
 
@@ -1095,8 +1147,7 @@ static void LoadPoolRounds()
         LogPrint("payout", "[payout] loaded %zu pool round proof(s) from disk\n",
                  gPoolRounds.size());
     } catch (const std::exception& e) {
-        LogPrint("payout", "[payout] WARNING: failed to parse %s: %s\n",
-                 path.c_str(), e.what());
+        RefuseFileAfterLoadFailure(path, "pool round proofs", e.what());
     }
 }
 
@@ -3021,6 +3072,12 @@ static void PayoutTestResetState()
         std::lock_guard<std::mutex> lk(gClaimedTxidMutex);
         gLegacyClaimedTxids.clear();
     }
+    {
+        // A quarantine is meant to last the life of the process; each section
+        // here is a fresh process as far as the code under test is concerned.
+        std::lock_guard<std::mutex> lk(gRefusedFilesMutex);
+        gRefusedFiles.clear();
+    }
 }
 
 // A BTF address whose hash160 is entirely under the test's control, so the
@@ -3785,7 +3842,107 @@ int RunPayoutLedgerSelfTest()
         strPoolRoundsFile = savedRoundsFile;
     }
 
-    // -- 11. A send that failed for want of coin is not a fault --------------
+    // -- 11. A ledger that failed to load is never written back --------------
+    //
+    // The loaders push entries into the live vector inside the try, so an
+    // exception part way down a file leaves a PARTIAL ledger in memory: the
+    // entries before the bad one and nothing after it. Publishing that -- and
+    // ReconcilePayouts publishes unconditionally, milliseconds into startup --
+    // writes the partial state over the file and destroys the only record of
+    // what was owed.
+    {
+        PayoutTestResetState();
+        remove(ledgerPath.c_str());
+
+        // Two entries, the second with a matureAtHeight that is not a number.
+        // The first loads; the second throws where it is read.
+        const std::string corrupt = strprintf(
+            "[{\"amounts\":{\"%s\":100000000},\"blockHash\":\"%s\","
+            "\"blockHeight\":10,\"matureAtHeight\":10},"
+            "{\"amounts\":{\"%s\":900000000},\"blockHash\":\"%s\","
+            "\"blockHeight\":20,\"matureAtHeight\":\"twenty\"}]",
+            addrA.c_str(), uint256(0).ToString().c_str(),
+            addrB.c_str(), uint256(0).ToString().c_str());
+        check(PayoutTestWriteFile(ledgerPath, corrupt),
+              "wrote a ledger whose second entry does not parse");
+
+        const int64 t0 = GetTime();
+        LoadPendingPayouts();
+        const int64 t1 = GetTime();
+
+        size_t loaded = 0;
+        {
+            std::lock_guard<std::mutex> lk(gPayoutLedgerMutex);
+            loaded = gPendingPayouts.size();
+        }
+        check(loaded == 1,
+              "a mid-file parse error does leave a partial ledger in memory --"
+              " which is the reason for everything below");
+
+        std::string gone;
+        check(!PayoutTestReadFile(ledgerPath, gone),
+              "the file that did not parse is moved out of the way");
+
+        // The rename stamps GetTime(); it was taken between t0 and t1.
+        std::string kept, keptPath;
+        for (int64 t = t0; t <= t1 && keptPath.empty(); t++) {
+            const std::string candidate =
+                strprintf("%s.bad.%lld", ledgerPath.c_str(), (long long)t);
+            if (PayoutTestReadFile(candidate, kept))
+                keptPath = candidate;
+        }
+        check(!keptPath.empty() && kept == corrupt,
+              "it is kept, byte for byte, as <path>.bad.<timestamp>");
+
+        // The publish that used to destroy it.
+        VersionedBlob partial;
+        {
+            std::lock_guard<std::mutex> lk(gPayoutLedgerMutex);
+            partial = SerializePendingPayoutsLocked();
+        }
+        PublishPendingPayouts(partial);
+        check(!PayoutTestReadFile(ledgerPath, gone),
+              "and the partial ledger is not published over it");
+
+        // The refusal is per path and lasts the process: a second attempt,
+        // with more state, is refused too.
+        {
+            std::lock_guard<std::mutex> lk(gPayoutLedgerMutex);
+            PendingPayout pp;
+            pp.blockHeight = 30; pp.blockHash = 0; pp.matureAtHeight = 30;
+            pp.amounts[addrA] = 5 * COIN;
+            gPendingPayouts.push_back(pp);
+            partial = SerializePendingPayoutsLocked();
+        }
+        PublishPendingPayouts(partial);
+        check(!PayoutTestReadFile(ledgerPath, gone),
+              "the refusal is not a one-shot -- the path stays unwritable");
+
+        if (!keptPath.empty())
+            remove(keptPath.c_str());
+        PayoutTestResetState();
+        remove(ledgerPath.c_str());
+
+        // And a file that parses is not quarantined, so the ordinary path is
+        // still the ordinary path.
+        check(PayoutTestWriteFile(ledgerPath, "[]"),
+              "wrote a ledger that parses");
+        LoadPendingPayouts();
+        {
+            std::lock_guard<std::mutex> lk(gPayoutLedgerMutex);
+            PendingPayout pp;
+            pp.blockHeight = 40; pp.blockHash = 0; pp.matureAtHeight = 40;
+            pp.amounts[addrA] = 2 * COIN;
+            gPendingPayouts.push_back(pp);
+            partial = SerializePendingPayoutsLocked();
+        }
+        PublishPendingPayouts(partial);
+        std::string written;
+        check(PayoutTestReadFile(ledgerPath, written) && written == partial.blob,
+              "a ledger that loaded cleanly is still published normally");
+    }
+
+    // -- 12. A send that failed for want of coin is not a fault --------------
     //
     // The defect this replaces: the classifier decided "was this a funding
     // problem?" by re-reading GetBalance() and allowing amount +
@@ -3919,7 +4076,7 @@ int RunPayoutLedgerSelfTest()
             mapWallet.clear();
     }
 
-    // -- 12. No payout mutex is held across the status-file write ------------
+    // -- 13. No payout mutex is held across the status-file write ------------
     //
     // RefreshStats runs on the event loop -- the thread that owns every
     // miner's socket -- and used to hold gStatsMutex and gPayoutLedgerMutex at