From 5f40c8ede057adae3f98916677405b94227748d6 Mon Sep 17 00:00:00 2001 From: DanS Date: Fri, 28 Aug 2026 23:31:47 -0500 Subject: [PATCH] stratum: stop paying every miner's blocks to whoever asked for work first CreateNewBlock() builds the stratum template with an OP_FALSE placeholder in the coinbase, and CustomizeWork() substituted the miner's own payout address only while that placeholder was still intact. GetWorkUnit() then wrote the customized coinbase straight back into the shared template: current_work.GetBlock().vtx[0] = cb; current_work.GetBlock().hashMerkleRoot = ...BuildMerkleTree(); which consumed the placeholder for everyone. The first client to request work after a tip change therefore captured the template. Every later client on that job got a mining.notify whose merkle root already committed to the first client's coinbase, CustomizeWork() was a no-op for them on submit, and SubmitBlock() read the shared root back -- so a block found by miner B was accepted paying miner A. It was silent: the daemon logged "GOT BLOCK!!! by " while the coinbase paid A. With untrusted miners that is a reward-theft primitive, and it is cheap: mining.authorize sets m_send_work, so re-sending it in a loop wins the race after every tip. Leave the template pristine and derive each client's header from a local copy, in GetWorkUnit for the notify and again in SubmitBlock from the coinbase CustomizeWork() just produced for that client. This is the refactor the TODO removed here was asking for. CustomizeWork() now stamps the payout script unconditionally, so a coinbase that somehow arrives already customized can never be inherited by another miner, and rejects an invalid address rather than silently building a coinbase that pays no one. Single-miner behaviour is unchanged, which is why this survived: it is only observable with two miners on one template. --- src/stratum.cpp | 44 ++++++++++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/src/stratum.cpp b/src/stratum.cpp index 16736e6e2..7877dd171 100644 --- a/src/stratum.cpp +++ b/src/stratum.cpp @@ -656,9 +656,18 @@ void CustomizeWork(const StratumClient& client, const StratumWork& current_work, LogPrint("stratum", "%s\n", msg); throw std::runtime_error(msg); } - if (cb.vout[0].scriptPubKey == (CScript() << OP_FALSE)) { - cb.vout[0].scriptPubKey = GetScriptForDestination(addr.Get()); + // Unconditional. This used to be guarded on the coinbase still carrying the OP_FALSE + // placeholder, which made it a no-op for every client after the first once a customized + // coinbase had been written back into the shared template -- so those miners silently + // mined the first miner's payout address. The template is now left pristine (see + // GetWorkUnit), and stamping unconditionally means a coinbase that somehow arrives + // already customized can never be inherited by a different miner. + if (!addr.IsValid()) { + const std::string msg = strprintf("%s: no valid payout address for this client; unable to customize work", __func__); + LogPrint("stratum", "%s\n", msg); + throw std::runtime_error(msg); } + cb.vout[0].scriptPubKey = GetScriptForDestination(addr.Get()); } // cb_branch = current_work.m_cb_branch; @@ -864,20 +873,24 @@ std::string GetWorkUnit(StratumClient& client) static const std::vector dummy(32-extranonce1.size(), 0x00); // extranonce2 CustomizeWork(client, current_work, client.m_addr, extranonce1, dummy, cb, bf, cb_branch); - // without 2 lines below equihash solutinon on SubmitWork will be incorrect, bcz we should - // change vtx[0] in current work and re-calc hashMerkleRoot - // TODO: refactor all of these ... may be change this in current_work directly is bad idea, - // and we should do all checks and hashMerkleRoot at SubmitBlock(...) - - current_work.GetBlock().vtx[0] = cb; - current_work.GetBlock().hashMerkleRoot = current_work.GetBlock().BuildMerkleTree(); - } CBlockHeader blkhdr; // Setup native proof-of-work - blkhdr = current_work.GetBlock().GetBlockHeader(); // copy entire blockheader created with CreateNewBlock to blkhdr + // The shared template MUST keep its pristine OP_FALSE coinbase. This previously did + // current_work.GetBlock().vtx[0] = cb; + // current_work.GetBlock().hashMerkleRoot = current_work.GetBlock().BuildMerkleTree(); + // which published one client's coinbase to every other client on the same job: the merkle + // root they were told to mine, and the block they eventually submitted, both committed to + // the first client's payout address. Derive this client's header from a local copy instead, + // which is what the TODO that used to sit here was asking for. + { + CBlock tmp(current_work.GetBlock()); + tmp.vtx[0] = cb; + blkhdr = tmp.GetBlockHeader(); + blkhdr.hashMerkleRoot = tmp.BuildMerkleTree(); + } // CDataStream ds(SER_GETHASH, SERIALIZE_TRANSACTION_NO_WITNESS); CDataStream ds(SER_GETHASH, PROTOCOL_VERSION); ds << cb; @@ -1012,7 +1025,14 @@ bool SubmitBlock(StratumClient& client, const uint256& job_id, const StratumWork : std::vector(sol.begin() + 3, sol.end()); blkhdr.hashFinalSaplingRoot = current_work.GetBlock().hashFinalSaplingRoot; - blkhdr.hashMerkleRoot = current_work.GetBlock().hashMerkleRoot; + // Recompute from the coinbase CustomizeWork() just derived for THIS client. Reading the + // shared template's root would be wrong now that the template is left pristine, and was + // wrong before too -- it carried whichever client happened to request work first. + { + CBlock tmp(current_work.GetBlock()); + tmp.vtx[0] = cb; + blkhdr.hashMerkleRoot = tmp.BuildMerkleTree(); + } blkhdr.nNonce = (uint256) nonce; // block is constructed, now it's time to VerifyEH