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 <B>" 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.
This commit is contained in:
2026-08-28 23:31:47 -05:00
parent b3e81f1eda
commit 5f40c8ede0

View File

@@ -656,9 +656,18 @@ void CustomizeWork(const StratumClient& client, const StratumWork& current_work,
LogPrint("stratum", "%s\n", msg); LogPrint("stratum", "%s\n", msg);
throw std::runtime_error(msg); throw std::runtime_error(msg);
} }
if (cb.vout[0].scriptPubKey == (CScript() << OP_FALSE)) { // Unconditional. This used to be guarded on the coinbase still carrying the OP_FALSE
cb.vout[0].scriptPubKey = GetScriptForDestination(addr.Get()); // 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; // cb_branch = current_work.m_cb_branch;
@@ -864,20 +873,24 @@ std::string GetWorkUnit(StratumClient& client)
static const std::vector<unsigned char> dummy(32-extranonce1.size(), 0x00); // extranonce2 static const std::vector<unsigned char> dummy(32-extranonce1.size(), 0x00); // extranonce2
CustomizeWork(client, current_work, client.m_addr, extranonce1, dummy, cb, bf, cb_branch); 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; CBlockHeader blkhdr;
// Setup native proof-of-work // 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, SERIALIZE_TRANSACTION_NO_WITNESS);
CDataStream ds(SER_GETHASH, PROTOCOL_VERSION); CDataStream ds(SER_GETHASH, PROTOCOL_VERSION);
ds << cb; ds << cb;
@@ -1012,7 +1025,14 @@ bool SubmitBlock(StratumClient& client, const uint256& job_id, const StratumWork
: std::vector<unsigned char>(sol.begin() + 3, sol.end()); : std::vector<unsigned char>(sol.begin() + 3, sol.end());
blkhdr.hashFinalSaplingRoot = current_work.GetBlock().hashFinalSaplingRoot; 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; blkhdr.nNonce = (uint256) nonce;
// block is constructed, now it's time to VerifyEH // block is constructed, now it's time to VerifyEH