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