payout: stop grandfathered zero-hash rounds answering for each other

Every ledger lookup identified an entry by its block hash alone --
ReconcilePayouts, the payout thread's already-paid check, RecordPayoutTxid,
ForfeitPayoutRecipient, ForgetPayment's still-in-flight test, and SamePayment.

Entries restored from a file written before payouts were keyed by hash all
carry hash 0. Two of them therefore compare equal, and the first one found
answers for the second:

  - RecordPayoutTxid writes a txid against the wrong round. That round can
    then satisfy PayoutFullyPaid and be erased from the ledger while the debt
    it recorded is still owed, and nothing on disk remembers it.
  - ReconcilePayouts drops a live in-flight record on startup because a
    different zero-hash round had already paid that same address.

SamePayoutRound() decides it properly: a non-zero hash is unique and settles
the question on its own, a zero hash falls back to the block height (which
LoadPendingPayouts supplies for any entry able to name its own maturity), and
a zero hash never matches a real one. Refusing to match is the safe direction
at all six sites, because each treats a match as "this debt is already
handled".

The orphan-void scan is deliberately left alone: PayoutBlockStillOnChain
returns true for a zero hash, so a grandfathered entry can never enter the
orphaned list and there is nothing there to cross-match.

For the residual case where an entry has neither hash nor height -- which no
file this tree has written can produce -- FlushMaturePayouts now refuses to
explode it and logs why, rather than paying against an identity that could
belong to another round. Not paying is recoverable; recording a payment
against the wrong debt is not.

PayoutIdForBlock's grandfathered "block-<height>" fallback is unchanged and
still safe: that id is only ever matched against pool_payout_id, which no
pre-upgrade transaction carries.

No on-disk format change.

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;
                         }