payout: name the funding block in new payout comments

A height the pool wins, loses to a reorg, and wins again yields two rounds
whose height-only comments are identical -- and identical amounts too when
the same miner held both rounds. New sends now write the funding block hash
into the comment. The legacy fallback's needle stays frozen at the
pre-upgrade format: it exists precisely for transactions that carry nothing
else, and a needle in any other format would make every one of those
payments look unpaid and pay it again.

1 files changed, 93 insertions, 4 deletions

diff --git a/src/rpc.cpp b/src/rpc.cpp
index a4b53ca..f9471c2 100644
--- a/src/rpc.cpp
+++ b/src/rpc.cpp
@@ -1186,11 +1186,35 @@ static bool PayoutBlockStillOnChain(const uint256& blockHash)
     return false;
 }
 
-static std::string PayoutComment(int blockHeight)
+// The comment every pre-upgrade payout transaction carries, and therefore the
+// ONLY needle the legacy fallback in FindRecordedPoolPayout may search with.
+// This format is frozen: change it and every payment those builds made stops
+// matching, every one of those debts looks unpaid, and reconciliation pays
+// them all a second time.
+static std::string LegacyPayoutComment(int blockHeight)
 {
     return strprintf("Pool payout height %d", blockHeight);
 }
 
+// The comment written on new sends. It names the funding block, because the
+// height alone cannot: a height that is won, lost to a reorg, and won again
+// yields two rounds whose height-only comments are identical -- and identical
+// amounts too, whenever the same miner held both rounds. The structured
+// pool_payout_* metadata already keys the payment by hash for the code; this
+// puts the same fact where an operator reading the wallet can see it, and
+// makes a new transaction unmatchable by the legacy needle even if the
+// structured keys are ever lost.
+//
+// A zero hash keeps the legacy form: those records predate hash keying, and
+// there is no hash to name.
+static std::string PayoutComment(int blockHeight, const uint256& blockHash)
+{
+    if (blockHash == 0)
+        return LegacyPayoutComment(blockHeight);
+    return strprintf("Pool payout height %d block %s", blockHeight,
+                     blockHash.ToString().c_str());
+}
+
 // Startup reconciliation.
 //
 // This used to be the crash-recovery mechanism: it scanned the wallet with
@@ -1425,7 +1449,7 @@ static PayoutProbeResult FindRecordedPoolPayout(const std::string& id,
     CScript want;
     if (fHaveScript)
         want << OP_DUP << OP_HASH160 << h160 << OP_EQUALVERIFY << OP_CHECKSIG;
-    const std::string comment = PayoutComment(blockHeight);
+    const std::string comment = LegacyPayoutComment(blockHeight);
 
     // Candidate legacy matches, adjudicated after cs_mapWallet is released.
     //
@@ -2186,7 +2210,7 @@ static void PayoutThreadFn(void*)
             }
 
             CWalletTx wtx;
-            wtx.mapValue["comment"] = PayoutComment(r.blockHeight);
+            wtx.mapValue["comment"] = PayoutComment(r.blockHeight, r.blockHash);
             // Upstream's structured metadata. This is what makes the wallet
             // probe above possible at all -- the old comment-only form could
             // not distinguish two rounds paying the same address the same
@@ -3825,7 +3849,7 @@ int RunPayoutLedgerSelfTest()
         const int    height = 8080;
         const int64  amount = 7 * COIN;
         const std::string id = PayoutIdForBlock(height, 0);
-        const std::string comment = PayoutComment(height);
+        const std::string comment = LegacyPayoutComment(height);
         std::set<std::string> none;
         std::string found;
 
@@ -4419,6 +4443,71 @@ int RunPayoutLedgerSelfTest()
     }
 #endif
 
+    // -- 14. The payout comment names the funding block ----------------------
+    //
+    // Same height, two hashes: the shape a reorg leaves when the pool wins a
+    // height, loses the block, and wins the height again. The legacy needle is
+    // frozen at the pre-upgrade format, so pre-upgrade payments keep matching;
+    // a new-format comment never matches it, so a new send can never be read
+    // as a pre-upgrade payment for a twin round.
+    {
+        PayoutTestResetState();
+        CRITICAL_BLOCK(cs_mapWallet)
+            mapWallet.clear();
+        PayoutTestClearBlockIndex();
+
+        const int   height = 9090;
+        const int64 amount = 3 * COIN;
+        uint256 hashA = 0, hashB = 0;
+        hashA.SetHex("00000000000000000000000000000000000000000000000000000000000000aa");
+        hashB.SetHex("00000000000000000000000000000000000000000000000000000000000000bb");
+
+        check(LegacyPayoutComment(height) ==
+                  strprintf("Pool payout height %d", height),
+              "the legacy needle is frozen at the format pre-upgrade builds"
+              " wrote");
+        check(PayoutComment(height, 0) == LegacyPayoutComment(height),
+              "a grandfathered zero-hash record still writes the legacy form");
+        check(PayoutComment(height, hashA) ==
+                  strprintf("Pool payout height %d block %s", height,
+                            hashA.ToString().c_str()),
+              "a hash-keyed record's comment names its funding block");
+        check(PayoutComment(height, hashA) != PayoutComment(height, hashB),
+              "twin rounds at one height carry distinguishable comments");
+
+        // A transaction wearing the NEW comment format and -- hypothetically
+        // -- no structured metadata must not be matched by the legacy needle:
+        // the fallback exists for pre-upgrade transactions, and the new format
+        // proves this is not one.
+        const std::string idA = PayoutIdForBlock(height, hashA);
+        const std::string idB = PayoutIdForBlock(height, hashB);
+        std::set<std::string> none;
+        std::string found;
+        PayoutTestAddWalletTx(PayoutComment(height, hashA), addrA, amount, 1, "");
+        check(FindRecordedPoolPayout(idB, height, addrA, amount, none, found)
+                  == PAYOUT_PROBE_NONE,
+              "a new-format comment is never legacy evidence for a twin round");
+        check(FindRecordedPoolPayout(idA, height, addrA, amount, none, found)
+                  == PAYOUT_PROBE_NONE,
+              "nor a match channel for its own round -- structured metadata is");
+
+        // Round A's own payment, sent by this build, is found by its
+        // structured id.
+        CRITICAL_BLOCK(cs_mapWallet)
+            mapWallet.clear();
+        PayoutTestClearBlockIndex();
+        const std::string paidA =
+            PayoutTestAddWalletTx(PayoutComment(height, hashA), addrA, amount,
+                                  2, idA);
+        check(FindRecordedPoolPayout(idA, height, addrA, amount, none, found)
+                  == PAYOUT_PROBE_STRUCTURED && found == paidA,
+              "the winning twin's own payment is found by its structured id");
+
+        CRITICAL_BLOCK(cs_mapWallet)
+            mapWallet.clear();
+        PayoutTestClearBlockIndex();
+    }
+
     // -- clean up ------------------------------------------------------------
     PayoutTestResetState();
     CRITICAL_BLOCK(cs_mapWallet)