1 files changed, 65 insertions, 7 deletions

diff --git a/src/rpc.cpp b/src/rpc.cpp
index 76867ab..b6b26f1 100644
--- a/src/rpc.cpp
+++ b/src/rpc.cpp
@@ -530,6 +530,42 @@ static std::string PayoutIdForBlock(int blockHeight, const uint256& blockHash)
                      blockHash.ToString().substr(0, 16).c_str());
 }
 
+// Do these two references name the same payout round?
+//
+// The block hash alone does not decide it, and every ledger lookup used to
+// assume it did. Entries grandfathered in from a file written before payouts
+// were keyed by hash all carry hash 0, so under a hash-only comparison the
+// first zero-hash entry answers for every other one: RecordPayoutTxid writes
+// a txid against the wrong round (which is then erased as "fully paid" while
+// the real debt still stands), and ReconcilePayouts drops a live in-flight
+// record because a *different* zero-hash round happened to pay that address.
+//
+// A non-zero hash is unique and decides on its own -- that is the whole point
+// of keying on it. A zero hash falls back to the height, which
+// LoadPendingPayouts supplies for any entry that can name its own maturity.
+// Mixed hash-ness never matches: refusing to match is the safe direction for
+// every caller here, because each of them treats a match as "this debt is
+// already dealt with".
+static bool SamePayoutRound(int aHeight, const uint256& aHash,
+                            int bHeight, const uint256& bHash)
+{
+    if (aHash != bHash)
+        return false;
+    if (aHash != 0)
+        return true;
+    return aHeight == bHeight;
+}
+
+// An entry with neither a hash nor a height cannot be told apart from any
+// other entry in the same state, so nothing may be recorded against it: a
+// txid written to the wrong one reports a debt as settled that is still owed.
+// Such an entry is never paid and is logged instead -- refusing to act is the
+// safe failure, and no file this tree has ever written can produce one.
+static bool PayoutRoundIdentifiable(int height, const uint256& hash)
+{
+    return hash != 0 || height != 0;
+}
+
 static bool PayoutFullyPaid(const PendingPayout& pp)
 {
     for (const auto& kv : pp.amounts)
@@ -938,7 +974,9 @@ static void ReconcilePayouts()
     for (auto it = gPaying.begin(); it != gPaying.end(); ) {
         bool paid = false;
         for (const PendingPayout& pp : gPendingPayouts)
-            if (pp.blockHash == it->blockHash && pp.paidTxids.count(it->address)) {
+            if (SamePayoutRound(pp.blockHeight, pp.blockHash,
+                                it->blockHeight, it->blockHash)
+                && pp.paidTxids.count(it->address)) {
                 paid = true;
                 break;
             }
@@ -1237,6 +1275,18 @@ static void FlushMaturePayouts()
                 continue;
             if (PayoutFullyPaid(pp))
                 continue;
+            // An entry with neither a hash nor a height cannot be told apart
+            // from another in the same state, so a txid recorded against it
+            // could settle the wrong debt. Paying it is the unsafe move;
+            // leaving it standing and saying so is not.
+            if (!PayoutRoundIdentifiable(pp.blockHeight, pp.blockHash)) {
+                LogPrint("payout", "[payout] WARNING: ledger entry maturing at"
+                         " height %d names neither a block hash nor a height --"
+                         " refusing to pay %zu recipient(s) that cannot be told"
+                         " apart from another round's\n",
+                         pp.matureAtHeight, pp.amounts.size());
+                continue;
+            }
             ready.push_back(pp);
         }
     }
@@ -1285,8 +1335,8 @@ static void FlushMaturePayouts()
 // amounts is a map keyed by address, so (block, address) is already unique.
 static bool SamePayment(const PayingRecord& a, const PayingRecord& b)
 {
-    return a.matureAtHeight == b.matureAtHeight
-        && a.blockHash      == b.blockHash
+    return SamePayoutRound(a.blockHeight, a.blockHash, b.blockHeight, b.blockHash)
+        && a.matureAtHeight == b.matureAtHeight
         && a.address        == b.address
         && a.amount         == b.amount;
 }
@@ -1309,7 +1359,11 @@ static void ForgetPayment(const PayingRecord& r)
         // standing).
         bool stillInFlight = false;
         for (const auto& e : gPaying)
-            if (e.blockHash == r.blockHash) { stillInFlight = true; break; }
+            if (SamePayoutRound(e.blockHeight, e.blockHash,
+                                r.blockHeight, r.blockHash)) {
+                stillInFlight = true;
+                break;
+            }
         if (!stillInFlight)
             gQueuedPayouts.erase(PayoutIdForBlock(r.blockHeight, r.blockHash));
 
@@ -1355,7 +1409,8 @@ static void RecordPayoutTxid(const PayingRecord& r, const std::string& txid)
     {
         std::lock_guard<std::mutex> lk(gPayoutLedgerMutex);
         for (auto it = gPendingPayouts.begin(); it != gPendingPayouts.end(); ++it) {
-            if (it->blockHash != r.blockHash)
+            if (!SamePayoutRound(it->blockHeight, it->blockHash,
+                                 r.blockHeight, r.blockHash))
                 continue;
             it->paidTxids[r.address] = txid;
             it->retryAfter = 0;
@@ -1389,7 +1444,8 @@ static void ForfeitPayoutRecipient(const PayingRecord& r)
     {
         std::lock_guard<std::mutex> lk(gPayoutLedgerMutex);
         for (auto it = gPendingPayouts.begin(); it != gPendingPayouts.end(); ++it) {
-            if (it->blockHash != r.blockHash)
+            if (!SamePayoutRound(it->blockHeight, it->blockHash,
+                                 r.blockHeight, r.blockHash))
                 continue;
             it->amounts.erase(r.address);
             if (it->amounts.empty() || PayoutFullyPaid(*it))
@@ -1443,7 +1499,9 @@ static void PayoutThreadFn(void*)
                 {
                     std::lock_guard<std::mutex> lk(gPayoutLedgerMutex);
                     for (const PendingPayout& pp : gPendingPayouts)
-                        if (pp.blockHash == r.blockHash && pp.paidTxids.count(r.address)) {
+                        if (SamePayoutRound(pp.blockHeight, pp.blockHash,
+                                            r.blockHeight, r.blockHash)
+                            && pp.paidTxids.count(r.address)) {
                             alreadyPaid = true;
                             break;
                         }