1 files changed, 164 insertions, 18 deletions

diff --git a/src/rpc.cpp b/src/rpc.cpp
index 7badb7c..b71457c 100644
--- a/src/rpc.cpp
+++ b/src/rpc.cpp
@@ -1132,11 +1132,71 @@ static void MarkPoolRoundPaid(const std::string& id,
     PublishPoolRounds(snap);
 }
 
-static bool FindRecordedPoolPayout(const std::string& id,
-                                   const std::string& address,
-                                   int64 amount,
-                                   std::string& txidRet)
+// Which wallet transaction, if any, already paid this?
+//
+// Two kinds of evidence, in order of quality.
+//
+// Structured (pool_payout_id / pool_payout_address / pool_payout_amount) is
+// upstream's, and it is exact: the id is derived from the block hash, so it
+// names one round and one round only. Every transaction this build sends
+// carries it.
+//
+// But NO transaction this pool has ever sent carries it. Every payout in the
+// existing wallet has only a "comment". So on the first boot after this
+// upgrade -- precisely the boot where the binary is swapped after an unclean
+// shutdown with payments in flight -- a structured-only probe finds nothing
+// and every one of those recipients is paid a second time. The deleted
+// WalletAlreadyPaid() matched on what the payment WAS, and that evidence is
+// still there; it just has to be read.
+//
+// The legacy fallback is therefore restored, with three constraints that
+// together stop it answering for a round that is not this one:
+//
+//   1. it is only consulted when the structured lookup found nothing;
+//   2. it ignores any transaction carrying pool_payout_id. That key means this
+//      build wrote the transaction, and this build's transactions are
+//      adjudicated by the exact path -- so a NEW round's payment can never be
+//      offered as evidence for an old one. Only pre-upgrade transactions,
+//      which is exactly the population the fallback exists for, are eligible;
+//   3. `excluded` holds every txid already credited to some OTHER
+//      (round, address) pair -- from the ledger's paidTxids, which is durable,
+//      and from what this process has already claimed. One legacy transaction
+//      can therefore settle at most one debt, so N transactions can never
+//      satisfy more than N obligations.
+//
+// What is left unresolved by construction is two pre-upgrade rounds at the
+// SAME height paying the same address the same amount, since the comment
+// carries only the height. That needs a reorg, and the losing round's ledger
+// entry is voided by the orphan check before it can be paid, so the pair
+// cannot both be live. Where the evidence genuinely runs out the fallback
+// simply does not match, and the payment is made -- paying a debt we cannot
+// prove was settled is the correct default for a pool.
+enum PayoutProbeResult {
+    PAYOUT_PROBE_NONE = 0,   // the wallet holds no record of this payment
+    PAYOUT_PROBE_STRUCTURED, // matched this build's pool_payout_* metadata
+    PAYOUT_PROBE_LEGACY      // matched a pre-upgrade comment and output
+};
+
+static PayoutProbeResult FindRecordedPoolPayout(const std::string& id,
+                                                int blockHeight,
+                                                const std::string& address,
+                                                int64 amount,
+                                                const std::set<std::string>& excluded,
+                                                std::string& txidRet)
 {
+    // Built before cs_mapWallet is taken: an invalid address has no script to
+    // compare against, and the caller has already rejected those.
+    uint160 h160;
+    bool fHaveScript = AddressToHash160(address, h160);
+    CScript want;
+    if (fHaveScript)
+        want << OP_DUP << OP_HASH160 << h160 << OP_EQUALVERIFY << OP_CHECKSIG;
+    const std::string comment = PayoutComment(blockHeight);
+
+    std::string legacyTxid;
+
+    // One pass, not two: this scan runs under cs_mapWallet, which the whole
+    // node contends on, and the merge is already paying for it once per record.
     CRITICAL_BLOCK(cs_mapWallet)
     {
         for (map<uint256, CWalletTx>::const_iterator it = mapWallet.begin();
@@ -1144,19 +1204,85 @@ static bool FindRecordedPoolPayout(const std::string& id,
         {
             const CWalletTx& wtx = it->second;
             map<string, string>::const_iterator idIt = wtx.mapValue.find("pool_payout_id");
-            if (idIt == wtx.mapValue.end() || idIt->second != id)
-                continue;
-            map<string, string>::const_iterator addrIt = wtx.mapValue.find("pool_payout_address");
-            if (addrIt == wtx.mapValue.end() || addrIt->second != address)
+            if (idIt != wtx.mapValue.end()) {
+                // Written by this build. Adjudicated exactly, and never
+                // offered to the fallback below.
+                if (idIt->second != id)
+                    continue;
+                map<string, string>::const_iterator addrIt = wtx.mapValue.find("pool_payout_address");
+                if (addrIt == wtx.mapValue.end() || addrIt->second != address)
+                    continue;
+                map<string, string>::const_iterator amtIt = wtx.mapValue.find("pool_payout_amount");
+                if (amtIt == wtx.mapValue.end() || amtIt->second != strprintf("%lld", amount))
+                    continue;
+                txidRet = wtx.GetHash().GetHex();
+                return PAYOUT_PROBE_STRUCTURED;
+            }
+
+            if (!fHaveScript || !legacyTxid.empty())
                 continue;
-            map<string, string>::const_iterator amtIt = wtx.mapValue.find("pool_payout_amount");
-            if (amtIt == wtx.mapValue.end() || amtIt->second != strprintf("%lld", amount))
+            // The comment is written only by the payout path, so its presence
+            // already identifies the transaction as one of ours.
+            map<string, string>::const_iterator ci = wtx.mapValue.find("comment");
+            if (ci == wtx.mapValue.end() || ci->second != comment)
                 continue;
-            txidRet = wtx.GetHash().GetHex();
-            return true;
+            std::string txid = wtx.GetHash().GetHex();
+            if (excluded.count(txid))
+                continue;   // already credited to a different debt
+            foreach (const CTxOut& txout, wtx.vout)
+                if (txout.nValue == amount && txout.scriptPubKey == want) {
+                    legacyTxid = txid;
+                    break;
+                }
         }
     }
-    return false;
+
+    if (!legacyTxid.empty()) {
+        txidRet = legacyTxid;
+        return PAYOUT_PROBE_LEGACY;
+    }
+    return PAYOUT_PROBE_NONE;
+}
+
+// txid -> "<round id>|<address>", for payments this run has already credited
+// to a debt on legacy evidence. Stops one pre-upgrade transaction being read
+// as proof that two different obligations were met. The ledger's paidTxids
+// covers the same ground durably; this covers the window before the recovered
+// txid reaches the ledger, and the case where the ledger entry has already
+// been retired.
+static std::map<std::string, std::string> gLegacyClaimedTxids;
+static std::mutex                         gClaimedTxidMutex;
+
+// Every txid already spoken for by some payment OTHER than this one. Collected
+// before cs_mapWallet is taken: the ledger mutex must not be acquired under a
+// wallet lock.
+static void CollectForeignPayoutTxids(int blockHeight, const uint256& blockHash,
+                                      const std::string& address,
+                                      const std::string& claimKey,
+                                      std::set<std::string>& out)
+{
+    {
+        std::lock_guard<std::mutex> lk(gPayoutLedgerMutex);
+        for (const PendingPayout& pp : gPendingPayouts) {
+            bool sameRound = SamePayoutRound(pp.blockHeight, pp.blockHash,
+                                             blockHeight, blockHash);
+            for (const auto& kv : pp.paidTxids)
+                if (!(sameRound && kv.first == address))
+                    out.insert(kv.second);
+        }
+    }
+    {
+        std::lock_guard<std::mutex> lk(gClaimedTxidMutex);
+        for (const auto& kv : gLegacyClaimedTxids)
+            if (kv.second != claimKey)
+                out.insert(kv.first);
+    }
+}
+
+static void ClaimLegacyPayoutTxid(const std::string& txid, const std::string& claimKey)
+{
+    std::lock_guard<std::mutex> lk(gClaimedTxidMutex);
+    gLegacyClaimedTxids[txid] = claimKey;
 }
 
 // blockHash stays a uint256 and not upstream's display string: it is the key
@@ -1603,12 +1729,32 @@ static void PayoutThreadFn(void*)
             // Only asked once per record -- see PayingRecord::fProbeWallet.
             if (r.fProbeWallet) {
                 std::string recovered;
+                const std::string claimKey = id + "|" + r.address;
+                std::set<std::string> excluded;
+                CollectForeignPayoutTxids(r.blockHeight, r.blockHash, r.address,
+                                          claimKey, excluded);
                 r.fProbeWallet = false;
-                if (FindRecordedPoolPayout(id, r.address, r.amount, recovered)) {
-                    LogPrint("payout", "[payout] recovered recorded payout %s to %s"
-                             " from wallet tx %s -- not resending\n",
-                             FormatMoney(r.amount).c_str(), r.address.c_str(),
-                             recovered.c_str());
+                PayoutProbeResult probe = FindRecordedPoolPayout(
+                    id, r.blockHeight, r.address, r.amount, excluded, recovered);
+                if (probe != PAYOUT_PROBE_NONE) {
+                    if (probe == PAYOUT_PROBE_LEGACY) {
+                        // Loud on purpose: this is the upgrade path, where the
+                        // only evidence a payment went out is a comment written
+                        // by a binary that is no longer running.
+                        LogPrint("payout", "[payout] %s to %s was already paid by"
+                                 " a pre-upgrade build (wallet tx %s carries the"
+                                 " payout comment for height %d and an output of"
+                                 " exactly this amount, and no structured payout"
+                                 " metadata) -- not resending\n",
+                                 FormatMoney(r.amount).c_str(), r.address.c_str(),
+                                 recovered.c_str(), r.blockHeight);
+                        ClaimLegacyPayoutTxid(recovered, claimKey);
+                    } else {
+                        LogPrint("payout", "[payout] recovered recorded payout %s to %s"
+                                 " from wallet tx %s -- not resending\n",
+                                 FormatMoney(r.amount).c_str(), r.address.c_str(),
+                                 recovered.c_str());
+                    }
                     RecordPayoutTxid(r, recovered);
                     ForgetPayment(r);
                     continue;