payout: refuse comment-only evidence for a round that has a proof

A pre-upgrade payout comment names only the height, so when a height is
won, orphaned, and won again, the old transaction matches the new round's
(height, address, amount) exactly and the miner is never paid. Round
proofs decide the ambiguity where it is decidable: a proof is written when
a round is created, and structured payout metadata (upstream 0b01d6c)
predates round proofs (upstream b1eb8b6), so a round with a proof was
created by a build whose every payment names its round -- a comment-only
match can only be another round's money. Skipping the fallback cannot
double-pay: any real payment for such a round is already found by the
structured probe. Proofless rounds keep the documented bias.

1 files changed, 139 insertions, 22 deletions

diff --git a/src/rpc.cpp b/src/rpc.cpp
index dbbf06d..310c0d4 100644
--- a/src/rpc.cpp
+++ b/src/rpc.cpp
@@ -1369,6 +1369,24 @@ static void MarkPoolRoundForfeited(const std::string& id,
     PublishPoolRounds(snap);
 }
 
+// Does this round have a published round proof?
+//
+// Existence answers a temporal question the legacy fallback below needs:
+// which builds could have paid this round. RecordPoolRound writes the proof
+// when the round is created, and structured payout metadata (upstream
+// cbdd06d) is an ancestor of round proofs (upstream 83ba822) -- no build
+// able to write a proof has ever sent a payment that only a comment can
+// identify. A round with a proof therefore cannot have a comment-only
+// payment; one that matches it is another round's, colliding on the height.
+static bool PoolRoundHasProof(const std::string& id)
+{
+    std::lock_guard<std::mutex> lk(gPoolRoundsMutex);
+    for (const PoolRoundProof& r : gPoolRounds)
+        if (r.id == id)
+            return true;
+    return false;
+}
+
 // Which wallet transaction, if any, already paid this?
 //
 // Two kinds of evidence, in order of quality.
@@ -1404,11 +1422,22 @@ static void MarkPoolRoundForfeited(const std::string& id,
 //      (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.
+//      satisfy more than N obligations;
+//
+//   4. it is not consulted at all for a round that has a round proof
+//      (fAllowLegacy, decided by the caller from PoolRoundHasProof). Proofs
+//      are written when a round is created, and only by builds whose every
+//      payment names its round exactly -- so a comment-only transaction
+//      cannot be a payment for such a round, only a same-height collision
+//      from an older one, and skipping the fallback cannot hide a real
+//      payment: the structured probe above already found any that exist.
 //
-// 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.
+// What is left unresolved by construction is a comment-only transaction and
+// a PROOFLESS round at the same height paying the same address the same
+// amount: two grandfathered zero-hash twins, or a twin pair from the builds
+// that keyed the ledger by hash before any metadata reached the wallet. The
+// comment carries only the height, so nothing ties the transaction to one
+// twin rather than the other. That needs a reorg.
 //
 // This used to say the losing twin's ledger entry is voided by the orphan
 // check before it can be paid. That is wrong: PayoutBlockStillOnChain()
@@ -1445,6 +1474,7 @@ static PayoutProbeResult FindRecordedPoolPayout(const std::string& id,
                                                 const uint256& blockHash,
                                                 const std::string& address,
                                                 int64 amount,
+                                                bool fAllowLegacy,
                                                 const std::set<std::string>& excluded,
                                                 std::string& txidRet)
 {
@@ -1505,6 +1535,8 @@ static PayoutProbeResult FindRecordedPoolPayout(const std::string& id,
                 return PAYOUT_PROBE_STRUCTURED;
             }
 
+            if (!fAllowLegacy)
+                continue;   // comment-only evidence is inadmissible: rule 4
             if (!fHaveScript)
                 continue;
             // The comment is written only by the payout path, so its presence
@@ -2142,10 +2174,15 @@ static void PayoutThreadFn(void*)
                 std::set<std::string> excluded;
                 CollectForeignPayoutTxids(r.blockHeight, r.blockHash, r.address,
                                           claimKey, excluded);
+                // A round with a published proof was created by a build whose
+                // every payment names its round exactly, so only structured
+                // evidence can settle it -- a comment match would be another
+                // round's payment colliding on the height.
+                const bool fAllowLegacy = !PoolRoundHasProof(id);
                 r.fProbeWallet = false;
                 PayoutProbeResult probe = FindRecordedPoolPayout(
                     id, r.blockHeight, r.blockHash, r.address, r.amount,
-                    excluded, recovered);
+                    fAllowLegacy, excluded, recovered);
                 if (probe != PAYOUT_PROBE_NONE) {
                     if (probe == PAYOUT_PROBE_LEGACY) {
                         // Loud on purpose: this is the upgrade path, where the
@@ -3880,31 +3917,34 @@ int RunPayoutLedgerSelfTest()
         const std::string comment = LegacyPayoutComment(height);
         std::set<std::string> none;
         std::string found;
+        // No round proof exists for these rounds, so legacy evidence is
+        // admissible -- computed exactly as the payout thread computes it.
+        const bool fAllowLegacy = !PoolRoundHasProof(id);
 
-        check(FindRecordedPoolPayout(id, height, 0, addrA, amount, none, found)
+        check(FindRecordedPoolPayout(id, height, 0, addrA, amount, fAllowLegacy, none, found)
                   == PAYOUT_PROBE_NONE,
               "an empty wallet is not evidence that anything was paid");
 
         const std::string legacyTxid =
             PayoutTestAddWalletTx(comment, addrA, amount, 1, "");
-        check(FindRecordedPoolPayout(id, height, 0, addrA, amount, none, found)
+        check(FindRecordedPoolPayout(id, height, 0, addrA, amount, fAllowLegacy, none, found)
                   == PAYOUT_PROBE_LEGACY && found == legacyTxid,
               "a payment carrying only a pre-upgrade comment is recognised as"
               " already paid");
 
-        check(FindRecordedPoolPayout(id, height, 0, addrB, amount, none, found)
+        check(FindRecordedPoolPayout(id, height, 0, addrB, amount, fAllowLegacy, none, found)
                   == PAYOUT_PROBE_NONE,
               "the legacy match requires the output to pay THIS recipient");
-        check(FindRecordedPoolPayout(id, height, 0, addrA, amount + COIN, none, found)
+        check(FindRecordedPoolPayout(id, height, 0, addrA, amount + COIN, fAllowLegacy, none, found)
                   == PAYOUT_PROBE_NONE,
               "the legacy match requires the output to pay exactly this amount");
-        check(FindRecordedPoolPayout(id, height + 1, 0, addrA, amount, none, found)
+        check(FindRecordedPoolPayout(id, height + 1, 0, addrA, amount, fAllowLegacy, none, found)
                   == PAYOUT_PROBE_NONE,
               "the legacy match requires the comment for this block height");
 
         std::set<std::string> spent;
         spent.insert(legacyTxid);
-        check(FindRecordedPoolPayout(id, height, 0, addrA, amount, spent, found)
+        check(FindRecordedPoolPayout(id, height, 0, addrA, amount, fAllowLegacy, spent, found)
                   == PAYOUT_PROBE_NONE,
               "one legacy transaction cannot settle a second debt");
 
@@ -3913,7 +3953,7 @@ int RunPayoutLedgerSelfTest()
         // exactly what the structured metadata exists to prevent.
         const std::string otherId = PayoutIdForBlock(height, 0) + "-other";
         PayoutTestAddWalletTx(comment, addrB, amount, 2, otherId);
-        check(FindRecordedPoolPayout(id, height, 0, addrB, amount, none, found)
+        check(FindRecordedPoolPayout(id, height, 0, addrB, amount, fAllowLegacy, none, found)
                   == PAYOUT_PROBE_NONE,
               "another round's structured payment is not legacy evidence for"
               " this one");
@@ -3921,7 +3961,7 @@ int RunPayoutLedgerSelfTest()
         // The structured path still wins where it applies.
         const std::string structuredTxid =
             PayoutTestAddWalletTx(comment, addrB, amount, 3, id);
-        check(FindRecordedPoolPayout(id, height, 0, addrB, amount, none, found)
+        check(FindRecordedPoolPayout(id, height, 0, addrB, amount, fAllowLegacy, none, found)
                   == PAYOUT_PROBE_STRUCTURED && found == structuredTxid,
               "structured payout metadata is matched in preference to a comment");
 
@@ -3935,7 +3975,7 @@ int RunPayoutLedgerSelfTest()
         PayoutTestClearBlockIndex();
         const std::string addrC = PayoutTestAddress(0x33);
         PayoutTestAddWalletTx(comment, addrC, amount, 4, "", false);
-        check(FindRecordedPoolPayout(id, height, 0, addrC, amount, none, found)
+        check(FindRecordedPoolPayout(id, height, 0, addrC, amount, fAllowLegacy, none, found)
                   == PAYOUT_PROBE_NONE,
               "a pre-upgrade payout that never confirmed is not read as"
               " payment");
@@ -3945,7 +3985,7 @@ int RunPayoutLedgerSelfTest()
             mapWallet.clear();
         const std::string confirmedTxid =
             PayoutTestAddWalletTx(comment, addrC, amount, 4, "", true);
-        check(FindRecordedPoolPayout(id, height, 0, addrC, amount, none, found)
+        check(FindRecordedPoolPayout(id, height, 0, addrC, amount, fAllowLegacy, none, found)
                   == PAYOUT_PROBE_LEGACY && found == confirmedTxid,
               "the same payout, confirmed, is");
 
@@ -3956,7 +3996,7 @@ int RunPayoutLedgerSelfTest()
         PayoutTestAddWalletTx(comment, addrC, amount, 5, "", false);
         std::set<std::string> spentFirst;
         spentFirst.insert(confirmedTxid);
-        check(FindRecordedPoolPayout(id, height, 0, addrC, amount, spentFirst, found)
+        check(FindRecordedPoolPayout(id, height, 0, addrC, amount, fAllowLegacy, spentFirst, found)
                   == PAYOUT_PROBE_LEGACY && found == alsoConfirmed,
               "an unconfirmed transaction does not hide a confirmed one behind"
               " it");
@@ -4511,11 +4551,14 @@ int RunPayoutLedgerSelfTest()
         const std::string idB = PayoutIdForBlock(height, hashB);
         std::set<std::string> none;
         std::string found;
+        // Neither twin has a round proof here (section 15 covers the gate),
+        // and their ids collide, so one answer serves both.
+        const bool fAllowLegacy = !PoolRoundHasProof(idA);
         PayoutTestAddWalletTx(PayoutComment(height, hashA), addrA, amount, 1, "");
-        check(FindRecordedPoolPayout(idB, height, hashB, addrA, amount, none, found)
+        check(FindRecordedPoolPayout(idB, height, hashB, addrA, amount, fAllowLegacy, none, found)
                   == PAYOUT_PROBE_NONE,
               "a new-format comment is never legacy evidence for a twin round");
-        check(FindRecordedPoolPayout(idA, height, hashA, addrA, amount, none, found)
+        check(FindRecordedPoolPayout(idA, height, hashA, addrA, amount, fAllowLegacy, none, found)
                   == PAYOUT_PROBE_NONE,
               "nor a match channel for its own round -- structured metadata is");
 
@@ -4527,7 +4570,7 @@ int RunPayoutLedgerSelfTest()
         const std::string paidA =
             PayoutTestAddWalletTx(PayoutComment(height, hashA), addrA, amount,
                                   2, idA);
-        check(FindRecordedPoolPayout(idA, height, hashA, addrA, amount, none, found)
+        check(FindRecordedPoolPayout(idA, height, hashA, addrA, amount, fAllowLegacy, none, found)
                   == PAYOUT_PROBE_STRUCTURED && found == paidA,
               "the winning twin's own payment is found by its structured id");
 
@@ -4539,7 +4582,7 @@ int RunPayoutLedgerSelfTest()
         check(idA == idB,
               "twin hashes differing only past the sixteenth hex digit collide"
               " in id -- the reason the full hash must decide");
-        check(FindRecordedPoolPayout(idB, height, hashB, addrA, amount, none, found)
+        check(FindRecordedPoolPayout(idB, height, hashB, addrA, amount, fAllowLegacy, none, found)
                   == PAYOUT_PROBE_STRUCTURED,
               "a payment without the block key answers for a colliding twin --"
               " the accepted residual for pre-key transactions");
@@ -4552,10 +4595,10 @@ int RunPayoutLedgerSelfTest()
         const std::string keyedA =
             PayoutTestAddWalletTx(PayoutComment(height, hashA), addrA, amount,
                                   3, idA, true, hashA.ToString());
-        check(FindRecordedPoolPayout(idA, height, hashA, addrA, amount, none, found)
+        check(FindRecordedPoolPayout(idA, height, hashA, addrA, amount, fAllowLegacy, none, found)
                   == PAYOUT_PROBE_STRUCTURED && found == keyedA,
               "a payment carrying the full block hash settles its own round");
-        check(FindRecordedPoolPayout(idB, height, hashB, addrA, amount, none, found)
+        check(FindRecordedPoolPayout(idB, height, hashB, addrA, amount, fAllowLegacy, none, found)
                   == PAYOUT_PROBE_NONE,
               "and is no evidence for the twin, so the re-won height is paid");
 
@@ -4564,6 +4607,80 @@ int RunPayoutLedgerSelfTest()
         PayoutTestClearBlockIndex();
     }
 
+    // -- 15. A proof-bearing round rejects comment-only evidence -------------
+    //
+    // The surviving half of the same-height collision: a pre-upgrade
+    // transaction carries only "Pool payout height H", so if H is won again
+    // after a reorg, it matches the new round's (height, address, amount)
+    // exactly and would settle a debt it never paid. The new round has a
+    // round proof -- written at creation, and only by builds whose payments
+    // all name their round -- so for it, comment-only evidence is
+    // inadmissible and the miner is paid. A round without a proof keeps
+    // today's bias: the evidence is admissible and doubt resolves to
+    // "already paid".
+    {
+        PayoutTestResetState();
+        CRITICAL_BLOCK(cs_mapWallet)
+            mapWallet.clear();
+        PayoutTestClearBlockIndex();
+        const std::string roundsPath = dir + "/pool_rounds_gate_selftest.json";
+        const std::string savedRoundsFile = strPoolRoundsFile;
+        strPoolRoundsFile = roundsPath;
+        remove(roundsPath.c_str());
+
+        const int   height = 7171;
+        const int64 amount = 4 * COIN;
+        uint256 hashB = 0;
+        hashB.SetHex("00000000000000000000000000000000000000000000000000000000000000cc");
+        const std::string idB = PayoutIdForBlock(height, hashB);
+        std::set<std::string> none;
+        std::string found;
+
+        // The colliding pre-upgrade payment: confirmed, comment-only, same
+        // height, same address, same amount -- the strongest wrong evidence
+        // there is.
+        PayoutTestAddWalletTx(LegacyPayoutComment(height), addrA, amount, 1, "");
+
+        check(!PoolRoundHasProof(idB),
+              "a round with no recorded proof does not claim one");
+        check(FindRecordedPoolPayout(idB, height, hashB, addrA, amount,
+                                     !PoolRoundHasProof(idB), none, found)
+                  == PAYOUT_PROBE_LEGACY,
+              "without a proof the doubt is genuine and still resolves to"
+              " already-paid");
+
+        PoolRoundProof round;
+        round.id = idB;
+        round.blockHeight = height;
+        round.matureAtHeight = height + COINBASE_SPENDABLE_DEPTH;
+        round.payouts[addrA] = amount;
+        RecordPoolRound(round);
+        check(PoolRoundHasProof(idB),
+              "recording the round proof marks the round as this lineage's");
+        check(FindRecordedPoolPayout(idB, height, hashB, addrA, amount,
+                                     !PoolRoundHasProof(idB), none, found)
+                  == PAYOUT_PROBE_NONE,
+              "with a proof, a same-height comment-only transaction is not"
+              " evidence, and the re-won height is paid");
+
+        // Structured evidence still is: the round's own payment, recovered.
+        const std::string paidB =
+            PayoutTestAddWalletTx(PayoutComment(height, hashB), addrA, amount,
+                                  2, idB, true, hashB.ToString());
+        check(FindRecordedPoolPayout(idB, height, hashB, addrA, amount,
+                                     !PoolRoundHasProof(idB), none, found)
+                  == PAYOUT_PROBE_STRUCTURED && found == paidB,
+              "the gate does not blind the probe to the round's own structured"
+              " payment");
+
+        CRITICAL_BLOCK(cs_mapWallet)
+            mapWallet.clear();
+        PayoutTestClearBlockIndex();
+        remove(roundsPath.c_str());
+        remove((roundsPath + ".tmp").c_str());
+        strPoolRoundsFile = savedRoundsFile;
+    }
+
     // -- clean up ------------------------------------------------------------
     PayoutTestResetState();
     CRITICAL_BLOCK(cs_mapWallet)