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)