From 46864cdf3c90d15d5d21a330bb54f43696aa7d3b Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Sun, 23 Aug 2026 11:50:12 +0000 Subject: payout: say why a grandfathered pair is safe, since it is not the orphan check The comment on FindRecordedPoolPayout justified the legacy fallback by saying that where two pre-upgrade rounds share a height, the losing one's ledger entry is voided by the orphan check before it can be paid. That check is PayoutBlockStillOnChain(), which returns true for a zero hash -- and every pre-upgrade entry has a zero hash, because they were written before payouts recorded one. So it is a no-op for exactly the population the fallback exists to serve. The stated reason for a safety property was not the reason. The property does hold, by a different mechanism. SamePayoutRound() treats two zero-hash entries at the same height as the same round, so the pair is indistinguishable to every consumer, not just to this one: FlushMaturePayouts explodes one and skips the other as already queued, PaymentInFlightLocked refuses a second record for that (round, address), and RecordPayoutTxid settles against the first match. The pair presents as one debt and can be paid at most once. The cost of that -- only one of a genuine same-height pair would ever be paid -- is now written down too, rather than left as an implication. Both halves are asserted rather than reasoned about: the self-test checks that the orphan check passes a zero hash, and that two identical zero-hash entries at one height produce exactly one in-flight record. Property: the comment describes the mechanism that is actually load-bearing, and a test fails if either half of it stops being true. --- src/rpc.cpp | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 5 deletions(-) (limited to 'src/rpc.cpp') diff --git a/src/rpc.cpp b/src/rpc.cpp index 7b45fea..7ea0597 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -1379,11 +1379,32 @@ static void MarkPoolRoundForfeited(const std::string& id, // // 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. +// carries only the height. 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() +// returns true for a zero hash, and EVERY pre-upgrade entry has a zero hash, +// so the orphan check is a no-op for exactly the population this fallback +// serves. +// +// The outcome is still safe, by a different mechanism. SamePayoutRound() +// treats two zero-hash entries at the same height as the SAME round, so the +// pair is indistinguishable everywhere, not only here: FlushMaturePayouts +// explodes one of them and skips the other as already queued, +// PaymentInFlightLocked refuses a second record for that (round, address), and +// RecordPayoutTxid settles against the first match. The pair therefore +// presents as ONE debt and can be paid at most once -- which is the direction +// that matters for a fallback whose failure mode is treating one payment as +// proof of two. +// +// The cost of that mechanism, stated plainly: only one of a genuine +// same-height pair would ever be paid. It takes a reorg AND both rounds +// predating hash keying, and no file this tree has written since can produce +// one, so it is a property of history rather than a live risk. +// +// 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 @@ -3594,6 +3615,33 @@ int RunPayoutLedgerSelfTest() PayoutRoundIdentifiable(7, 0), "an entry with neither hash nor height is not adjudicable"); + // What actually makes a grandfathered same-height pair safe, since the + // comment on FindRecordedPoolPayout used to claim it was the orphan + // check. It is not: the orphan check passes every zero-hash entry, and + // every pre-upgrade entry has a zero hash. + check(PayoutBlockStillOnChain(0), + "the orphan check passes every grandfathered entry, so it cannot" + " be what voids a losing twin"); + PayoutTestResetState(); + remove(ledgerPath.c_str()); + remove(payingPath.c_str()); + { + std::lock_guard lk(gPayoutLedgerMutex); + PendingPayout twin; + twin.blockHeight = 300; twin.blockHash = 0; twin.matureAtHeight = 300; + twin.amounts[addrA] = 1 * COIN; + gPendingPayouts.push_back(twin); + gPendingPayouts.push_back(twin); // the losing side of a reorg + } + nBestHeight = 5000; + FlushMaturePayouts(); + { + std::lock_guard lk(gPayoutExecMutex); + check(gPaying.size() == 1, + "two grandfathered rounds at one height are one round, so the" + " pair presents as one debt and is paid at most once"); + } + // End to end: recording a payment against one grandfathered round must // not settle the other's debt. PayoutTestResetState(); -- cgit v1.2.3