From 63740a1b00d78441713a2b909aa40429098184d2 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Sun, 23 Aug 2026 07:35:31 +0000 Subject: payout: stop grandfathered zero-hash rounds answering for each other Every ledger lookup identified an entry by its block hash alone -- ReconcilePayouts, the payout thread's already-paid check, RecordPayoutTxid, ForfeitPayoutRecipient, ForgetPayment's still-in-flight test, and SamePayment. Entries restored from a file written before payouts were keyed by hash all carry hash 0. Two of them therefore compare equal, and the first one found answers for the second: - RecordPayoutTxid writes a txid against the wrong round. That round can then satisfy PayoutFullyPaid and be erased from the ledger while the debt it recorded is still owed, and nothing on disk remembers it. - ReconcilePayouts drops a live in-flight record on startup because a different zero-hash round had already paid that same address. SamePayoutRound() decides it properly: a non-zero hash is unique and settles the question on its own, a zero hash falls back to the block height (which LoadPendingPayouts supplies for any entry able to name its own maturity), and a zero hash never matches a real one. Refusing to match is the safe direction at all six sites, because each treats a match as "this debt is already handled". The orphan-void scan is deliberately left alone: PayoutBlockStillOnChain returns true for a zero hash, so a grandfathered entry can never enter the orphaned list and there is nothing there to cross-match. For the residual case where an entry has neither hash nor height -- which no file this tree has written can produce -- FlushMaturePayouts now refuses to explode it and logs why, rather than paying against an identity that could belong to another round. Not paying is recoverable; recording a payment against the wrong debt is not. PayoutIdForBlock's grandfathered "block-" fallback is unchanged and still safe: that id is only ever matched against pool_payout_id, which no pre-upgrade transaction carries. No on-disk format change. --- src/rpc.cpp | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 65 insertions(+), 7 deletions(-) (limited to 'src/rpc.cpp') diff --git a/src/rpc.cpp b/src/rpc.cpp index 76867ab..b6b26f1 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -530,6 +530,42 @@ static std::string PayoutIdForBlock(int blockHeight, const uint256& blockHash) blockHash.ToString().substr(0, 16).c_str()); } +// Do these two references name the same payout round? +// +// The block hash alone does not decide it, and every ledger lookup used to +// assume it did. Entries grandfathered in from a file written before payouts +// were keyed by hash all carry hash 0, so under a hash-only comparison the +// first zero-hash entry answers for every other one: RecordPayoutTxid writes +// a txid against the wrong round (which is then erased as "fully paid" while +// the real debt still stands), and ReconcilePayouts drops a live in-flight +// record because a *different* zero-hash round happened to pay that address. +// +// A non-zero hash is unique and decides on its own -- that is the whole point +// of keying on it. A zero hash falls back to the height, which +// LoadPendingPayouts supplies for any entry that can name its own maturity. +// Mixed hash-ness never matches: refusing to match is the safe direction for +// every caller here, because each of them treats a match as "this debt is +// already dealt with". +static bool SamePayoutRound(int aHeight, const uint256& aHash, + int bHeight, const uint256& bHash) +{ + if (aHash != bHash) + return false; + if (aHash != 0) + return true; + return aHeight == bHeight; +} + +// An entry with neither a hash nor a height cannot be told apart from any +// other entry in the same state, so nothing may be recorded against it: a +// txid written to the wrong one reports a debt as settled that is still owed. +// Such an entry is never paid and is logged instead -- refusing to act is the +// safe failure, and no file this tree has ever written can produce one. +static bool PayoutRoundIdentifiable(int height, const uint256& hash) +{ + return hash != 0 || height != 0; +} + static bool PayoutFullyPaid(const PendingPayout& pp) { for (const auto& kv : pp.amounts) @@ -938,7 +974,9 @@ static void ReconcilePayouts() for (auto it = gPaying.begin(); it != gPaying.end(); ) { bool paid = false; for (const PendingPayout& pp : gPendingPayouts) - if (pp.blockHash == it->blockHash && pp.paidTxids.count(it->address)) { + if (SamePayoutRound(pp.blockHeight, pp.blockHash, + it->blockHeight, it->blockHash) + && pp.paidTxids.count(it->address)) { paid = true; break; } @@ -1237,6 +1275,18 @@ static void FlushMaturePayouts() continue; if (PayoutFullyPaid(pp)) continue; + // An entry with neither a hash nor a height cannot be told apart + // from another in the same state, so a txid recorded against it + // could settle the wrong debt. Paying it is the unsafe move; + // leaving it standing and saying so is not. + if (!PayoutRoundIdentifiable(pp.blockHeight, pp.blockHash)) { + LogPrint("payout", "[payout] WARNING: ledger entry maturing at" + " height %d names neither a block hash nor a height --" + " refusing to pay %zu recipient(s) that cannot be told" + " apart from another round's\n", + pp.matureAtHeight, pp.amounts.size()); + continue; + } ready.push_back(pp); } } @@ -1285,8 +1335,8 @@ static void FlushMaturePayouts() // amounts is a map keyed by address, so (block, address) is already unique. static bool SamePayment(const PayingRecord& a, const PayingRecord& b) { - return a.matureAtHeight == b.matureAtHeight - && a.blockHash == b.blockHash + return SamePayoutRound(a.blockHeight, a.blockHash, b.blockHeight, b.blockHash) + && a.matureAtHeight == b.matureAtHeight && a.address == b.address && a.amount == b.amount; } @@ -1309,7 +1359,11 @@ static void ForgetPayment(const PayingRecord& r) // standing). bool stillInFlight = false; for (const auto& e : gPaying) - if (e.blockHash == r.blockHash) { stillInFlight = true; break; } + if (SamePayoutRound(e.blockHeight, e.blockHash, + r.blockHeight, r.blockHash)) { + stillInFlight = true; + break; + } if (!stillInFlight) gQueuedPayouts.erase(PayoutIdForBlock(r.blockHeight, r.blockHash)); @@ -1355,7 +1409,8 @@ static void RecordPayoutTxid(const PayingRecord& r, const std::string& txid) { std::lock_guard lk(gPayoutLedgerMutex); for (auto it = gPendingPayouts.begin(); it != gPendingPayouts.end(); ++it) { - if (it->blockHash != r.blockHash) + if (!SamePayoutRound(it->blockHeight, it->blockHash, + r.blockHeight, r.blockHash)) continue; it->paidTxids[r.address] = txid; it->retryAfter = 0; @@ -1389,7 +1444,8 @@ static void ForfeitPayoutRecipient(const PayingRecord& r) { std::lock_guard lk(gPayoutLedgerMutex); for (auto it = gPendingPayouts.begin(); it != gPendingPayouts.end(); ++it) { - if (it->blockHash != r.blockHash) + if (!SamePayoutRound(it->blockHeight, it->blockHash, + r.blockHeight, r.blockHash)) continue; it->amounts.erase(r.address); if (it->amounts.empty() || PayoutFullyPaid(*it)) @@ -1443,7 +1499,9 @@ static void PayoutThreadFn(void*) { std::lock_guard lk(gPayoutLedgerMutex); for (const PendingPayout& pp : gPendingPayouts) - if (pp.blockHash == r.blockHash && pp.paidTxids.count(r.address)) { + if (SamePayoutRound(pp.blockHeight, pp.blockHash, + r.blockHeight, r.blockHash) + && pp.paidTxids.count(r.address)) { alreadyPaid = true; break; } -- cgit v1.2.3