From 6c37f59ea136cbace8b0cfccefdad555ff3d619f Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 08:54:01 +0000 Subject: 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. --- src/rpc.cpp | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 93 insertions(+), 4 deletions(-) (limited to 'src/rpc.cpp') 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 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 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) -- cgit v1.2.3 From a2e1f27b32442bcea92eeda3ac09432f6c5ec2c5 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 08:56:46 +0000 Subject: payout: record the full funding block hash and require it to agree pool_payout_id keeps only the first sixteen hex digits of the block hash, which is the end proof-of-work fills with zeros -- so twin rounds at one height can carry the SAME id, and one twin's payment then settles the other's debt through the structured probe: the same-height collision the comment match had, one layer down. New sends record the full hash as pool_payout_block and a structured match must agree with it. A transaction without the key predates it and is matched by id as before; anything stricter would make every payment already sent look unpaid and pay it again. --- src/rpc.cpp | 96 ++++++++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 76 insertions(+), 20 deletions(-) (limited to 'src/rpc.cpp') diff --git a/src/rpc.cpp b/src/rpc.cpp index f9471c2..dbbf06d 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -1373,10 +1373,15 @@ static void MarkPoolRoundForfeited(const std::string& id, // // Two kinds of evidence, in order of quality. // -// Structured (pool_payout_id / pool_payout_address / pool_payout_amount) is -// upstream's, and it is exact: the id is derived from the block hash, so it -// names one round and one round only. Every transaction this build sends -// carries it. +// Structured (pool_payout_id / pool_payout_address / pool_payout_amount, +// plus pool_payout_block since this series) is upstream's, extended. The id +// is derived from the block hash, but only from the FIRST sixteen hex digits +// of it -- and proof-of-work concentrates zeros exactly there, so the id can +// name two same-height rounds identically. New sends therefore record the +// full hash as pool_payout_block, and a structured match must agree with it +// wherever both sides have one; a transaction without the key was sent +// before it existed, and for those the id is the best evidence there is. +// Every transaction this build sends carries all four keys. // // But NO transaction this pool has ever sent carries it. Every payout in the // existing wallet has only a "comment". So on the first boot after this @@ -1437,6 +1442,7 @@ enum PayoutProbeResult { static PayoutProbeResult FindRecordedPoolPayout(const std::string& id, int blockHeight, + const uint256& blockHash, const std::string& address, int64 amount, const std::set& excluded, @@ -1482,6 +1488,19 @@ static PayoutProbeResult FindRecordedPoolPayout(const std::string& id, map::const_iterator amtIt = wtx.mapValue.find("pool_payout_amount"); if (amtIt == wtx.mapValue.end() || amtIt->second != strprintf("%lld", amount)) continue; + // The id alone cannot tell twin rounds at one height apart + // (see above); the full hash can, where the transaction + // recorded one. Where it did not, the id decides, as it + // always has -- refusing those transactions would make every + // payment sent before the key existed look unpaid and pay it + // again. + if (blockHash != 0) { + map::const_iterator blkIt = + wtx.mapValue.find("pool_payout_block"); + if (blkIt != wtx.mapValue.end() + && blkIt->second != blockHash.ToString()) + continue; // a twin round's payment, not this round's + } txidRet = wtx.GetHash().GetHex(); return PAYOUT_PROBE_STRUCTURED; } @@ -2125,7 +2144,8 @@ static void PayoutThreadFn(void*) claimKey, excluded); r.fProbeWallet = false; PayoutProbeResult probe = FindRecordedPoolPayout( - id, r.blockHeight, r.address, r.amount, excluded, recovered); + id, r.blockHeight, r.blockHash, r.address, r.amount, + excluded, recovered); if (probe != PAYOUT_PROBE_NONE) { if (probe == PAYOUT_PROBE_LEGACY) { // Loud on purpose: this is the upgrade path, where the @@ -2219,6 +2239,11 @@ static void PayoutThreadFn(void*) wtx.mapValue["pool_payout_id"] = id; wtx.mapValue["pool_payout_address"] = r.address; wtx.mapValue["pool_payout_amount"] = strprintf("%lld", r.amount); + // The full hash, not only the id's sixteen-digit prefix: the + // prefix of a proof-of-work hash is where the zeros live, and + // the probe needs to tell twin rounds at one height apart. + if (r.blockHash != 0) + wtx.mapValue["pool_payout_block"] = r.blockHash.ToString(); SendMoneyOutcome outcome; if (SendMoney(sc, r.amount, wtx, outcome)) { @@ -3285,7 +3310,8 @@ static std::string PayoutTestAddWalletTx(const std::string& comment, int64 amount, int nonce, const std::string& structuredId, - bool fConfirmed = true) + bool fConfirmed = true, + const std::string& structuredBlock = "") { CWalletTx wtx; wtx.vin.resize(1); @@ -3300,6 +3326,8 @@ static std::string PayoutTestAddWalletTx(const std::string& comment, wtx.mapValue["pool_payout_id"] = structuredId; wtx.mapValue["pool_payout_address"] = address; wtx.mapValue["pool_payout_amount"] = strprintf("%lld", amount); + if (!structuredBlock.empty()) + wtx.mapValue["pool_payout_block"] = structuredBlock; } uint256 hash = wtx.GetHash(); if (fConfirmed) { @@ -3853,30 +3881,30 @@ int RunPayoutLedgerSelfTest() std::set none; std::string found; - check(FindRecordedPoolPayout(id, height, addrA, amount, none, found) + check(FindRecordedPoolPayout(id, height, 0, addrA, amount, 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, addrA, amount, none, found) + check(FindRecordedPoolPayout(id, height, 0, addrA, amount, none, found) == PAYOUT_PROBE_LEGACY && found == legacyTxid, "a payment carrying only a pre-upgrade comment is recognised as" " already paid"); - check(FindRecordedPoolPayout(id, height, addrB, amount, none, found) + check(FindRecordedPoolPayout(id, height, 0, addrB, amount, none, found) == PAYOUT_PROBE_NONE, "the legacy match requires the output to pay THIS recipient"); - check(FindRecordedPoolPayout(id, height, addrA, amount + COIN, none, found) + check(FindRecordedPoolPayout(id, height, 0, addrA, amount + COIN, none, found) == PAYOUT_PROBE_NONE, "the legacy match requires the output to pay exactly this amount"); - check(FindRecordedPoolPayout(id, height + 1, addrA, amount, none, found) + check(FindRecordedPoolPayout(id, height + 1, 0, addrA, amount, none, found) == PAYOUT_PROBE_NONE, "the legacy match requires the comment for this block height"); std::set spent; spent.insert(legacyTxid); - check(FindRecordedPoolPayout(id, height, addrA, amount, spent, found) + check(FindRecordedPoolPayout(id, height, 0, addrA, amount, spent, found) == PAYOUT_PROBE_NONE, "one legacy transaction cannot settle a second debt"); @@ -3885,7 +3913,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, addrB, amount, none, found) + check(FindRecordedPoolPayout(id, height, 0, addrB, amount, none, found) == PAYOUT_PROBE_NONE, "another round's structured payment is not legacy evidence for" " this one"); @@ -3893,7 +3921,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, addrB, amount, none, found) + check(FindRecordedPoolPayout(id, height, 0, addrB, amount, none, found) == PAYOUT_PROBE_STRUCTURED && found == structuredTxid, "structured payout metadata is matched in preference to a comment"); @@ -3907,7 +3935,7 @@ int RunPayoutLedgerSelfTest() PayoutTestClearBlockIndex(); const std::string addrC = PayoutTestAddress(0x33); PayoutTestAddWalletTx(comment, addrC, amount, 4, "", false); - check(FindRecordedPoolPayout(id, height, addrC, amount, none, found) + check(FindRecordedPoolPayout(id, height, 0, addrC, amount, none, found) == PAYOUT_PROBE_NONE, "a pre-upgrade payout that never confirmed is not read as" " payment"); @@ -3917,7 +3945,7 @@ int RunPayoutLedgerSelfTest() mapWallet.clear(); const std::string confirmedTxid = PayoutTestAddWalletTx(comment, addrC, amount, 4, "", true); - check(FindRecordedPoolPayout(id, height, addrC, amount, none, found) + check(FindRecordedPoolPayout(id, height, 0, addrC, amount, none, found) == PAYOUT_PROBE_LEGACY && found == confirmedTxid, "the same payout, confirmed, is"); @@ -3928,7 +3956,7 @@ int RunPayoutLedgerSelfTest() PayoutTestAddWalletTx(comment, addrC, amount, 5, "", false); std::set spentFirst; spentFirst.insert(confirmedTxid); - check(FindRecordedPoolPayout(id, height, addrC, amount, spentFirst, found) + check(FindRecordedPoolPayout(id, height, 0, addrC, amount, spentFirst, found) == PAYOUT_PROBE_LEGACY && found == alsoConfirmed, "an unconfirmed transaction does not hide a confirmed one behind" " it"); @@ -4484,10 +4512,10 @@ int RunPayoutLedgerSelfTest() std::set none; std::string found; PayoutTestAddWalletTx(PayoutComment(height, hashA), addrA, amount, 1, ""); - check(FindRecordedPoolPayout(idB, height, addrA, amount, none, found) + check(FindRecordedPoolPayout(idB, height, hashB, 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) + check(FindRecordedPoolPayout(idA, height, hashA, addrA, amount, none, found) == PAYOUT_PROBE_NONE, "nor a match channel for its own round -- structured metadata is"); @@ -4499,10 +4527,38 @@ int RunPayoutLedgerSelfTest() const std::string paidA = PayoutTestAddWalletTx(PayoutComment(height, hashA), addrA, amount, 2, idA); - check(FindRecordedPoolPayout(idA, height, addrA, amount, none, found) + check(FindRecordedPoolPayout(idA, height, hashA, addrA, amount, none, found) == PAYOUT_PROBE_STRUCTURED && found == paidA, "the winning twin's own payment is found by its structured id"); + // The id is derived from the FIRST sixteen hex digits of the hash -- + // the end proof-of-work fills with zeros -- so twin hashes that agree + // there collide in id, and a payment carrying no pool_payout_block + // answers for either twin. That is the accepted trade for payments + // sent before the key existed, pinned here so it stays a decision. + 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) + == PAYOUT_PROBE_STRUCTURED, + "a payment without the block key answers for a colliding twin --" + " the accepted residual for pre-key transactions"); + + // With the full hash recorded -- as every send now records it -- the + // twins are told apart and the re-won height is paid. + CRITICAL_BLOCK(cs_mapWallet) + mapWallet.clear(); + PayoutTestClearBlockIndex(); + const std::string keyedA = + PayoutTestAddWalletTx(PayoutComment(height, hashA), addrA, amount, + 3, idA, true, hashA.ToString()); + check(FindRecordedPoolPayout(idA, height, hashA, addrA, amount, 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) + == PAYOUT_PROBE_NONE, + "and is no evidence for the twin, so the re-won height is paid"); + CRITICAL_BLOCK(cs_mapWallet) mapWallet.clear(); PayoutTestClearBlockIndex(); -- cgit v1.2.3 From d8eb3aeec198e9e6753b89a53d25f4f91f8ab3d6 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 08:59:21 +0000 Subject: 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. --- src/rpc.cpp | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 139 insertions(+), 22 deletions(-) (limited to 'src/rpc.cpp') 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 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& 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 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 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 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 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 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 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) -- cgit v1.2.3 From 7221bb35d50e9bac11ce5e4b3252c34abe03c124 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 09:01:34 +0000 Subject: payout: treat a malformed ledger block hash as corruption, not as a hash SetHex() parses whatever hex prefix it finds and stops, so a corrupt blockHash loaded as a DIFFERENT hash: garbage with a hex-looking prefix became a hash no block ever had, which the orphan check reads as an orphaned block -- and the next flush voided the debt on the strength of file corruption. Garbage with no hex prefix became 0 and skipped the orphan check instead. A field that is not the 64-digit hex string this tree writes now fails the load, and the file is quarantined like any other parse error. --- src/rpc.cpp | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 99 insertions(+), 2 deletions(-) (limited to 'src/rpc.cpp') diff --git a/src/rpc.cpp b/src/rpc.cpp index 310c0d4..be6c80d 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -776,6 +776,29 @@ static void PublishPendingPayouts(const VersionedBlob& v) gPayoutLedgerPublishedSeq, "pending payout ledger"); } +// Parse a persisted block-hash field, refusing to guess. SetHex() reads +// whatever hex prefix it finds and stops at the first character it does not +// recognise, so a corrupt value does not fail to parse -- it parses as a +// DIFFERENT hash. Both shapes of that are wrong in a dangerous direction: a +// value with a hex-looking prefix ("deadbeef...") becomes a hash no block +// ever had, which PayoutBlockStillOnChain reads as an orphaned block and +// FlushMaturePayouts then VOIDS the debt; a value with no hex prefix at all +// becomes 0, which is grandfathered past the orphan check on the strength +// of garbage. A field that is not the 64-digit hex string ToString() wrote +// is corruption, and corruption is a load failure like any other parse +// error -- the file is quarantined, not read around. +static uint256 ParsePayoutBlockHash(const std::string& str) +{ + if (str.size() != 64 || + str.find_first_not_of("0123456789abcdefABCDEF") != std::string::npos) + throw std::runtime_error( + strprintf("blockHash \"%s\" is not a 64-digit hex string", + str.c_str())); + uint256 h; + h.SetHex(str); + return h; +} + static void LoadPendingPayouts() { std::string path = PayoutFilePath(); @@ -797,7 +820,8 @@ static void LoadPendingPayouts() // field. Leave blockHash zero and they are grandfathered past the // orphan check rather than voided on a guess. if (obj.contains("blockHash") && obj["blockHash"].is_string()) - pp.blockHash.SetHex(obj["blockHash"].get()); + pp.blockHash = + ParsePayoutBlockHash(obj["blockHash"].get()); if (obj.contains("blockHeight") && obj["blockHeight"].is_number()) pp.blockHeight = obj["blockHeight"].get(); // Upstream's tolerance for an entry with no usable height, kept @@ -976,7 +1000,8 @@ static void LoadPaying() r.amount = obj["amount"].get(); r.matureAtHeight = obj["matureAtHeight"].get(); if (obj.contains("blockHash") && obj["blockHash"].is_string()) - r.blockHash.SetHex(obj["blockHash"].get()); + r.blockHash = + ParsePayoutBlockHash(obj["blockHash"].get()); if (obj.contains("blockHeight") && obj["blockHeight"].is_number()) r.blockHeight = obj["blockHeight"].get(); if (obj.contains("retries") && obj["retries"].is_number()) @@ -4681,6 +4706,78 @@ int RunPayoutLedgerSelfTest() strPoolRoundsFile = savedRoundsFile; } + // -- 16. A malformed block hash is corruption, not a hash ---------------- + // + // SetHex() parses whatever hex prefix it finds and stops, so a corrupt + // blockHash did not fail to load -- it loaded as a DIFFERENT hash. + // "deadbeef..." became 0xdeadbeef, a hash no block ever had, which the + // orphan check reads as an orphaned block and the next flush VOIDS the + // debt; a value with no hex prefix became 0, which is grandfathered past + // the orphan check instead. Either way a corrupt file silently changed + // what the pool owed. It is a load failure now, handled like every other + // parse error: quarantine, and never rewrite. + { + PayoutTestResetState(); + remove(ledgerPath.c_str()); + remove(payingPath.c_str()); + + const std::string badLedger = strprintf( + "[{\"amounts\":{\"%s\":100000000}," + "\"blockHash\":\"deadbeef oops, not a hash\"," + "\"blockHeight\":10,\"matureAtHeight\":10}]", addrA.c_str()); + check(PayoutTestWriteFile(ledgerPath, badLedger), + "wrote a ledger whose blockHash is not a 64-digit hex string"); + LoadPendingPayouts(); + { + std::lock_guard lk(gPayoutLedgerMutex); + check(gPendingPayouts.empty(), + "no entry is loaded with a guessed hash"); + } + std::string gone; + check(!PayoutTestReadFile(ledgerPath, gone), + "the ledger with the corrupt hash is quarantined"); + + const std::string badPaying = strprintf( + "[{\"address\":\"%s\",\"amount\":100000000," + "\"blockHash\":\"00000000cafe\",\"blockHeight\":10," + "\"matureAtHeight\":10}]", addrA.c_str()); + check(PayoutTestWriteFile(payingPath, badPaying), + "wrote an in-flight list whose blockHash is truncated"); + LoadPaying(); + { + std::lock_guard lk(gPayoutExecMutex); + check(gPaying.empty(), + "no in-flight record is loaded with a guessed hash"); + } + check(!PayoutTestReadFile(payingPath, gone), + "the in-flight list with the corrupt hash is quarantined"); + + // The two spellings every real file uses still load: the 64-digit + // hash of a keyed round, and the 64 zeros this build writes for a + // grandfathered entry. + PayoutTestResetState(); + remove(ledgerPath.c_str()); + const std::string goodLedger = strprintf( + "[{\"amounts\":{\"%s\":100000000}," + "\"blockHash\":\"00000000000000000000000000000000000000000000000000000000000000cd\"," + "\"blockHeight\":10,\"matureAtHeight\":10}," + "{\"amounts\":{\"%s\":200000000}," + "\"blockHash\":\"%s\"," + "\"blockHeight\":20,\"matureAtHeight\":20}]", + addrA.c_str(), addrB.c_str(), uint256(0).ToString().c_str()); + check(PayoutTestWriteFile(ledgerPath, goodLedger), + "wrote a ledger in both real hash spellings"); + LoadPendingPayouts(); + { + std::lock_guard lk(gPayoutLedgerMutex); + check(gPendingPayouts.size() == 2 && + gPendingPayouts[0].blockHash != 0 && + gPendingPayouts[1].blockHash == 0, + "a real hash and a serialised zero hash both still load"); + } + remove(ledgerPath.c_str()); + } + // -- clean up ------------------------------------------------------------ PayoutTestResetState(); CRITICAL_BLOCK(cs_mapWallet) -- cgit v1.2.3