From 0025c9eae41654c204ecf31f7e134b91dc473a75 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Mon, 26 Nov 2018 11:17:38 -0500 Subject: [mining] segwit option must be set in GBT Calling getblocktemplate without the segwit rule specified is most likely a client error, since it results in lower fees for the miner. Prevent this client error by failing getblocktemplate if called without the segwit rule specified. --- src/bench/block_assemble.cpp | 2 +- src/miner.cpp | 4 ++-- src/miner.h | 2 +- src/rpc/mining.cpp | 23 +++++++++-------------- src/test/validation_block_tests.cpp | 2 +- 5 files changed, 14 insertions(+), 19 deletions(-) (limited to 'src') diff --git a/src/bench/block_assemble.cpp b/src/bench/block_assemble.cpp index 2def0b23e2..7cf27f9f5b 100644 --- a/src/bench/block_assemble.cpp +++ b/src/bench/block_assemble.cpp @@ -27,7 +27,7 @@ static std::shared_ptr PrepareBlock(const CScript& coinbase_scriptPubKey { auto block = std::make_shared( BlockAssembler{Params()} - .CreateNewBlock(coinbase_scriptPubKey, /* fMineWitnessTx */ true) + .CreateNewBlock(coinbase_scriptPubKey) ->block); block->nTime = ::chainActive.Tip()->GetMedianTimePast() + 1; diff --git a/src/miner.cpp b/src/miner.cpp index 96c9cd6d2a..ef48a86e32 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -95,7 +95,7 @@ void BlockAssembler::resetBlock() nFees = 0; } -std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& scriptPubKeyIn, bool fMineWitnessTx) +std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& scriptPubKeyIn) { int64_t nTimeStart = GetTimeMicros(); @@ -139,7 +139,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc // not activated. // TODO: replace this with a call to main to assess validity of a mempool // transaction (which in most cases can be a no-op). - fIncludeWitness = IsWitnessEnabled(pindexPrev, chainparams.GetConsensus()) && fMineWitnessTx; + fIncludeWitness = IsWitnessEnabled(pindexPrev, chainparams.GetConsensus()); int nPackagesSelected = 0; int nDescendantsUpdated = 0; diff --git a/src/miner.h b/src/miner.h index 8cdcf7133b..44c50b01ad 100644 --- a/src/miner.h +++ b/src/miner.h @@ -157,7 +157,7 @@ public: BlockAssembler(const CChainParams& params, const Options& options); /** Construct a new block template with coinbase to scriptPubKeyIn */ - std::unique_ptr CreateNewBlock(const CScript& scriptPubKeyIn, bool fMineWitnessTx=true); + std::unique_ptr CreateNewBlock(const CScript& scriptPubKeyIn); private: // utility functions diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 93fa3a2728..d4d1adbb50 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -311,7 +311,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) " https://github.com/bitcoin/bips/blob/master/bip-0009.mediawiki#getblocktemplate_changes\n" " https://github.com/bitcoin/bips/blob/master/bip-0145.mediawiki\n", { - {"template_request", RPCArg::Type::OBJ, /* opt */ true, /* default_val */ "", "A json object in the following spec", + {"template_request", RPCArg::Type::OBJ, /* opt */ false, /* default_val */ "", "A json object in the following spec", { {"mode", RPCArg::Type::STR, /* opt */ true, /* default_val */ "", "This must be set to \"template\", \"proposal\" (see BIP 23), or omitted"}, {"capabilities", RPCArg::Type::ARR, /* opt */ true, /* default_val */ "", "A list of strings", @@ -319,7 +319,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) {"support", RPCArg::Type::STR, /* opt */ true, /* default_val */ "", "client side supported feature, 'longpoll', 'coinbasetxn', 'coinbasevalue', 'proposal', 'serverlist', 'workid'"}, }, }, - {"rules", RPCArg::Type::ARR, /* opt */ true, /* default_val */ "", "A list of strings", + {"rules", RPCArg::Type::ARR, /* opt */ false, /* default_val */ "", "A list of strings", { {"support", RPCArg::Type::STR, /* opt */ true, /* default_val */ "", "client side supported softfork deployment"}, }, @@ -503,21 +503,17 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) } const struct VBDeploymentInfo& segwit_info = VersionBitsDeploymentInfo[Consensus::DEPLOYMENT_SEGWIT]; - // If the caller is indicating segwit support, then allow CreateNewBlock() - // to select witness transactions, after segwit activates (otherwise - // don't). - bool fSupportsSegwit = setClientRules.find(segwit_info.name) != setClientRules.end(); + // GBT must be called with 'segwit' set in the rules + if (setClientRules.count(segwit_info.name) != 1) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "getblocktemplate must be called with the segwit rule set (call with {\"rules\": [\"segwit\"]})"); + } // Update block static CBlockIndex* pindexPrev; static int64_t nStart; static std::unique_ptr pblocktemplate; - // Cache whether the last invocation was with segwit support, to avoid returning - // a segwit-block to a non-segwit caller. - static bool fLastTemplateSupportsSegwit = true; if (pindexPrev != chainActive.Tip() || - (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLast && GetTime() - nStart > 5) || - fLastTemplateSupportsSegwit != fSupportsSegwit) + (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLast && GetTime() - nStart > 5)) { // Clear pindexPrev so future calls make a new block, despite any failures from here on pindexPrev = nullptr; @@ -526,11 +522,10 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) nTransactionsUpdatedLast = mempool.GetTransactionsUpdated(); CBlockIndex* pindexPrevNew = chainActive.Tip(); nStart = GetTime(); - fLastTemplateSupportsSegwit = fSupportsSegwit; // Create new block CScript scriptDummy = CScript() << OP_TRUE; - pblocktemplate = BlockAssembler(Params()).CreateNewBlock(scriptDummy, fSupportsSegwit); + pblocktemplate = BlockAssembler(Params()).CreateNewBlock(scriptDummy); if (!pblocktemplate) throw JSONRPCError(RPC_OUT_OF_MEMORY, "Out of memory"); @@ -682,7 +677,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) result.pushKV("bits", strprintf("%08x", pblock->nBits)); result.pushKV("height", (int64_t)(pindexPrev->nHeight+1)); - if (!pblocktemplate->vchCoinbaseCommitment.empty() && fSupportsSegwit) { + if (!pblocktemplate->vchCoinbaseCommitment.empty()) { result.pushKV("default_witness_commitment", HexStr(pblocktemplate->vchCoinbaseCommitment.begin(), pblocktemplate->vchCoinbaseCommitment.end())); } diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 4316f37999..a9d192e555 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -54,7 +54,7 @@ std::shared_ptr Block(const uint256& prev_hash) CScript pubKey; pubKey << i++ << OP_TRUE; - auto ptemplate = BlockAssembler(Params()).CreateNewBlock(pubKey, false); + auto ptemplate = BlockAssembler(Params()).CreateNewBlock(pubKey); auto pblock = std::make_shared(ptemplate->block); pblock->hashPrevBlock = prev_hash; pblock->nTime = ++time; -- cgit v1.2.3