payout: recognise a payment a pre-upgrade build already made

FindRecordedPoolPayout matches on pool_payout_id / pool_payout_address /
pool_payout_amount. That metadata is written only by this build. Every payout
the pool has actually sent, in the wallet right now, carries nothing but a
"comment".

So the first boot after this upgrade is the dangerous one: swap the binary
after an unclean shutdown with payments in flight, and the recovery probe
finds no evidence for any of them and pays every one of those recipients a
second time. The deleted WalletAlreadyPaid() matched on what the payment WAS
-- the comment plus an output paying exactly that amount to exactly that
script -- and that evidence is still sitting in the wallet. It just stopped
being read.

The probe now answers with which kind of evidence it found, and the legacy
form is restored as a fallback under three constraints that keep it from
answering for a round that is not the one being asked about:

  1. it is consulted only when the structured lookup found nothing, so the
     exact path stays primary and strictly better going forward;
  2. any transaction carrying pool_payout_id is excluded from it outright.
     That key means this build wrote the transaction, and this build's
     transactions are adjudicated exactly -- so a newer round's payment can
     never be offered as proof for an older one. Only pre-upgrade
     transactions, which is the entire population the fallback exists for,
     are eligible;
  3. a txid already credited to a DIFFERENT (round, address) is excluded --
     from the ledger's paidTxids, which is durable across restarts, and from
     what this process has already claimed. One legacy transaction therefore
     settles at most one debt, so N transactions can never discharge more than
     N obligations. Two debts with one transaction pays the second, which is
     the correct reading of the evidence.

What cannot be resolved from a comment alone is two PRE-UPGRADE rounds at the
same height paying the same address the same amount, because the comment
carries only the height. That requires a reorg, and the losing round's ledger
entry is voided by the orphan check before it can be paid, so the two cannot
both be live. Where the evidence genuinely runs out the fallback does not
match and the payment is made: paying a debt that cannot be proven settled is
the correct default for a pool, and it is the direction that does not keep a
miner's money.

Recovery on legacy evidence logs loudly and says exactly what it matched on,
because it is a judgement made about a binary that is no longer running.

The two lookups share one pass over mapWallet. The structured path used to
scan alone; adding a second scan would have doubled a linear walk taken under
cs_mapWallet, which the whole node contends on.

No on-disk format change.

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;