Merge #17693: rpc: Add generateblock to mine a custom set of transactions

7524b6479cb20471d827aec5500925c86c62ce1c Add tests for generateblock (Andrew Toth)
dcc8332543f8fb6d1bb47cb270fcbb6a814a7d6e Add generateblock rpc (Andrew Toth)

Pull request description:

  The existing block generation rpcs for regtest, `generatetoaddress` and `generatetodescriptor`, mine everything in the mempool up to the block weight limit. This makes it difficult to test a system for several scenarios where a different set of transactions are mined. For example:
  - Testing the common scenario where a transaction is replaced in the mempool but the replaced transaction is mined instead.
  - Testing for a double-spent transaction where a transaction that conflicts with the mempool is mined.
  - Testing for non-standard transactions that are mined.
  - Testing the scenario where several blocks are mined without a specific transaction in the mempool being included in a block.

  This PR introduces a new rpc, `generateblock`, that takes an array of raw transactions and txids and mines only those and the coinbase. Any txids must be in the mempool, but the raw txs can be anything conforming to consensus rules. The coinbase can be specified as either an address or descriptor.

  This reopens #17653 since it was closed by mistake.

  Thanks to instagibbs for code suggestions that I used here.

ACKs for top commit:
  MarcoFalke:
    re-ACK 7524b6479cb20471d827aec5500925c86c62ce1c 📁

Tree-SHA512: 857106007465b5b9b8a84b6d07c17cbf8378a33a72d32ff79abea1d5ab4babb4d53a11ddbb14595aa1fac9dfa1391e3a11403d742f69951beea2f683e8a01cd4

6 files changed, 303 insertions, 31 deletions

diff --git a/src/miner.cpp b/src/miner.cpp
index 61d27d17c1..5ab23c7f4f 100644
--- a/src/miner.cpp
+++ b/src/miner.cpp
@@ -39,6 +39,17 @@ int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParam
     return nNewTime - nOldTime;
 }
 
+void RegenerateCommitments(CBlock& block)
+{
+    CMutableTransaction tx{*block.vtx.at(0)};
+    tx.vout.erase(tx.vout.begin() + GetWitnessCommitmentIndex(block));
+    block.vtx.at(0) = MakeTransactionRef(tx);
+
+    GenerateCoinbaseCommitment(block, WITH_LOCK(cs_main, return LookupBlockIndex(block.hashPrevBlock)), Params().GetConsensus());
+
+    block.hashMerkleRoot = BlockMerkleRoot(block);
+}
+
 BlockAssembler::Options::Options() {
     blockMinFeeRate = CFeeRate(DEFAULT_BLOCK_MIN_TX_FEE);
     nBlockMaxWeight = DEFAULT_BLOCK_MAX_WEIGHT;
diff --git a/src/miner.h b/src/miner.h
index cc8fc31a9f..b9ffb34a2d 100644
--- a/src/miner.h
+++ b/src/miner.h
@@ -203,4 +203,7 @@ private:
 void IncrementExtraNonce(CBlock* pblock, const CBlockIndex* pindexPrev, unsigned int& nExtraNonce);
 int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev);
 
+/** Update an old GenerateCoinbaseCommitment from CreateNewBlock after the block txs have changed */
+void RegenerateCommitments(CBlock& block);
+
 #endif // BITCOIN_MINER_H
diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp
index c1762483e9..64c7401199 100644
--- a/src/rpc/client.cpp
+++ b/src/rpc/client.cpp
@@ -33,6 +33,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "generatetoaddress", 2, "maxtries" },
     { "generatetodescriptor", 0, "num_blocks" },
     { "generatetodescriptor", 2, "maxtries" },
+    { "generateblock", 1, "transactions" },
     { "getnetworkhashps", 0, "nblocks" },
     { "getnetworkhashps", 1, "height" },
     { "sendtoaddress", 1, "amount" },
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index da9d583fa7..b812f3005f 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -101,6 +101,36 @@ static UniValue getnetworkhashps(const JSONRPCRequest& request)
     return GetNetworkHashPS(!request.params[0].isNull() ? request.params[0].get_int() : 120, !request.params[1].isNull() ? request.params[1].get_int() : -1);
 }
 
+static bool GenerateBlock(CBlock& block, uint64_t& max_tries, unsigned int& extra_nonce, uint256& block_hash)
+{
+    block_hash.SetNull();
+
+    {
+        LOCK(cs_main);
+        IncrementExtraNonce(&block, ::ChainActive().Tip(), extra_nonce);
+    }
+
+    CChainParams chainparams(Params());
+
+    while (max_tries > 0 && block.nNonce < std::numeric_limits<uint32_t>::max() && !CheckProofOfWork(block.GetHash(), block.nBits, chainparams.GetConsensus()) && !ShutdownRequested()) {
+        ++block.nNonce;
+        --max_tries;
+    }
+    if (max_tries == 0 || ShutdownRequested()) {
+        return false;
+    }
+    if (block.nNonce == std::numeric_limits<uint32_t>::max()) {
+        return true;
+    }
+
+    std::shared_ptr<const CBlock> shared_pblock = std::make_shared<const CBlock>(block);
+    if (!ProcessNewBlock(chainparams, shared_pblock, true, nullptr))
+        throw JSONRPCError(RPC_INTERNAL_ERROR, "ProcessNewBlock, block not accepted");
+
+    block_hash = block.GetHash();
+    return true;
+}
+
 static UniValue generateBlocks(const CTxMemPool& mempool, const CScript& coinbase_script, int nGenerate, uint64_t nMaxTries)
 {
     int nHeightEnd = 0;
@@ -119,29 +149,54 @@ static UniValue generateBlocks(const CTxMemPool& mempool, const CScript& coinbas
         if (!pblocktemplate.get())
             throw JSONRPCError(RPC_INTERNAL_ERROR, "Couldn't create new block");
         CBlock *pblock = &pblocktemplate->block;
-        {
-            LOCK(cs_main);
-            IncrementExtraNonce(pblock, ::ChainActive().Tip(), nExtraNonce);
-        }
-        while (nMaxTries > 0 && pblock->nNonce < std::numeric_limits<uint32_t>::max() && !CheckProofOfWork(pblock->GetHash(), pblock->nBits, Params().GetConsensus()) && !ShutdownRequested()) {
-            ++pblock->nNonce;
-            --nMaxTries;
-        }
-        if (nMaxTries == 0 || ShutdownRequested()) {
+
+        uint256 block_hash;
+        if (!GenerateBlock(*pblock, nMaxTries, nExtraNonce, block_hash)) {
             break;
         }
-        if (pblock->nNonce == std::numeric_limits<uint32_t>::max()) {
-            continue;
+
+        if (!block_hash.IsNull()) {
+            ++nHeight;
+            blockHashes.push_back(block_hash.GetHex());
         }
-        std::shared_ptr<const CBlock> shared_pblock = std::make_shared<const CBlock>(*pblock);
-        if (!ProcessNewBlock(Params(), shared_pblock, true, nullptr))
-            throw JSONRPCError(RPC_INTERNAL_ERROR, "ProcessNewBlock, block not accepted");
-        ++nHeight;
-        blockHashes.push_back(pblock->GetHash().GetHex());
     }
     return blockHashes;
 }
 
+static bool getScriptFromDescriptor(const std::string& descriptor, CScript& script, std::string& error)
+{
+    FlatSigningProvider key_provider;
+    const auto desc = Parse(descriptor, key_provider, error, /* require_checksum = */ false);
+    if (desc) {
+        if (desc->IsRange()) {
+            throw JSONRPCError(RPC_INVALID_PARAMETER, "Ranged descriptor not accepted. Maybe pass through deriveaddresses first?");
+        }
+
+        FlatSigningProvider provider;
+        std::vector<CScript> scripts;
+        if (!desc->Expand(0, key_provider, scripts, provider)) {
+            throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Cannot derive script without private keys"));
+        }
+
+        // Combo desriptors can have 2 or 4 scripts, so we can't just check scripts.size() == 1
+        CHECK_NONFATAL(scripts.size() > 0 && scripts.size() <= 4);
+
+        if (scripts.size() == 1) {
+            script = scripts.at(0);
+        } else if (scripts.size() == 4) {
+            // For uncompressed keys, take the 3rd script, since it is p2wpkh
+            script = scripts.at(2);
+        } else {
+            // Else take the 2nd script, since it is p2pkh
+            script = scripts.at(1);
+        }
+
+        return true;
+    } else {
+        return false;
+    }
+}
+
 static UniValue generatetodescriptor(const JSONRPCRequest& request)
 {
     RPCHelpMan{
@@ -166,27 +221,15 @@ static UniValue generatetodescriptor(const JSONRPCRequest& request)
     const int num_blocks{request.params[0].get_int()};
     const int64_t max_tries{request.params[2].isNull() ? 1000000 : request.params[2].get_int()};
 
-    FlatSigningProvider key_provider;
+    CScript coinbase_script;
     std::string error;
-    const auto desc = Parse(request.params[1].get_str(), key_provider, error, /* require_checksum = */ false);
-    if (!desc) {
+    if (!getScriptFromDescriptor(request.params[1].get_str(), coinbase_script, error)) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, error);
     }
-    if (desc->IsRange()) {
-        throw JSONRPCError(RPC_INVALID_PARAMETER, "Ranged descriptor not accepted. Maybe pass through deriveaddresses first?");
-    }
-
-    FlatSigningProvider provider;
-    std::vector<CScript> coinbase_script;
-    if (!desc->Expand(0, key_provider, coinbase_script, provider)) {
-        throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Cannot derive script without private keys"));
-    }
 
     const CTxMemPool& mempool = EnsureMemPool();
 
-    CHECK_NONFATAL(coinbase_script.size() == 1);
-
-    return generateBlocks(mempool, coinbase_script.at(0), num_blocks, max_tries);
+    return generateBlocks(mempool, coinbase_script, num_blocks, max_tries);
 }
 
 static UniValue generatetoaddress(const JSONRPCRequest& request)
@@ -229,6 +272,113 @@ static UniValue generatetoaddress(const JSONRPCRequest& request)
     return generateBlocks(mempool, coinbase_script, nGenerate, nMaxTries);
 }
 
+static UniValue generateblock(const JSONRPCRequest& request)
+{
+    RPCHelpMan{"generateblock",
+        "\nMine a block with a set of ordered transactions immediately to a specified address or descriptor (before the RPC call returns)\n",
+        {
+            {"address/descriptor", RPCArg::Type::STR, RPCArg::Optional::NO, "The address or descriptor to send the newly generated bitcoin to."},
+            {"transactions", RPCArg::Type::ARR, RPCArg::Optional::NO, "An array of hex strings which are either txids or raw transactions.\n"
+                "Txids must reference transactions currently in the mempool.\n"
+                "All transactions must be valid and in valid order, otherwise the block will be rejected.",
+                {
+                    {"rawtx/txid", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, ""},
+                },
+            }
+        },
+        RPCResult{
+            RPCResult::Type::OBJ, "", "",
+            {
+                {RPCResult::Type::STR_HEX, "hash", "hash of generated block"}
+            }
+        },
+        RPCExamples{
+            "\nGenerate a block to myaddress, with txs rawtx and mempool_txid\n"
+            + HelpExampleCli("generateblock", R"("myaddress" '["rawtx", "mempool_txid"]')")
+        },
+    }.Check(request);
+
+    const auto address_or_descriptor = request.params[0].get_str();
+    CScript coinbase_script;
+    std::string error;
+
+    if (!getScriptFromDescriptor(address_or_descriptor, coinbase_script, error)) {
+        const auto destination = DecodeDestination(address_or_descriptor);
+        if (!IsValidDestination(destination)) {
+            throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Error: Invalid address or descriptor");
+        }
+
+        coinbase_script = GetScriptForDestination(destination);
+    }
+
+    const CTxMemPool& mempool = EnsureMemPool();
+
+    std::vector<CTransactionRef> txs;
+    const auto raw_txs_or_txids = request.params[1].get_array();
+    for (size_t i = 0; i < raw_txs_or_txids.size(); i++) {
+        const auto str(raw_txs_or_txids[i].get_str());
+
+        uint256 hash;
+        CMutableTransaction mtx;
+        if (ParseHashStr(str, hash)) {
+
+            const auto tx = mempool.get(hash);
+            if (!tx) {
+                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Transaction %s not in mempool.", str));
+            }
+
+            txs.emplace_back(tx);
+
+        } else if (DecodeHexTx(mtx, str)) {
+            txs.push_back(MakeTransactionRef(std::move(mtx)));
+
+        } else {
+            throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("Transaction decode failed for %s", str));
+        }
+    }
+
+    CChainParams chainparams(Params());
+    CBlock block;
+
+    {
+        LOCK(cs_main);
+
+        CTxMemPool empty_mempool;
+        std::unique_ptr<CBlockTemplate> blocktemplate(BlockAssembler(empty_mempool, chainparams).CreateNewBlock(coinbase_script));
+        if (!blocktemplate) {
+            throw JSONRPCError(RPC_INTERNAL_ERROR, "Couldn't create new block");
+        }
+        block = blocktemplate->block;
+    }
+
+    CHECK_NONFATAL(block.vtx.size() == 1);
+
+    // Add transactions
+    block.vtx.insert(block.vtx.end(), txs.begin(), txs.end());
+    RegenerateCommitments(block);
+
+    {
+        LOCK(cs_main);
+
+        BlockValidationState state;
+        if (!TestBlockValidity(state, chainparams, block, LookupBlockIndex(block.hashPrevBlock), false, false)) {
+            throw JSONRPCError(RPC_VERIFY_ERROR, strprintf("TestBlockValidity failed: %s", state.ToString()));
+        }
+    }
+
+    uint256 block_hash;
+    uint64_t max_tries{1000000};
+    unsigned int extra_nonce{0};
+
+    if (!GenerateBlock(block, max_tries, extra_nonce, block_hash) || block_hash.IsNull()) {
+        throw JSONRPCError(RPC_MISC_ERROR, "Failed to make block.");
+    }
+
+    UniValue obj(UniValue::VOBJ);
+    obj.pushKV("hash", block_hash.GetHex());
+    return obj;
+}
+
 static UniValue getmininginfo(const JSONRPCRequest& request)
 {
             RPCHelpMan{"getmininginfo",
@@ -1038,6 +1188,7 @@ static const CRPCCommand commands[] =
 
     { "generating",         "generatetoaddress",      &generatetoaddress,      {"nblocks","address","maxtries"} },
     { "generating",         "generatetodescriptor",   &generatetodescriptor,   {"num_blocks","descriptor","maxtries"} },
+    { "generating",         "generateblock",          &generateblock,    {"address","transactions"} },
 
     { "util",               "estimatesmartfee",       &estimatesmartfee,       {"conf_target", "estimate_mode"} },
 
diff --git a/test/functional/rpc_generateblock.py b/test/functional/rpc_generateblock.py
new file mode 100755
index 0000000000..f23d9ec556
--- /dev/null
+++ b/test/functional/rpc_generateblock.py
@@ -0,0 +1,105 @@
+#!/usr/bin/env python3
+# Copyright (c) 2020 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+'''Test generateblock rpc.
+'''
+
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import (
+    assert_equal,
+    assert_raises_rpc_error,
+)
+
+class GenerateBlockTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 1
+
+    def skip_test_if_missing_module(self):
+        self.skip_if_no_wallet()
+
+    def run_test(self):
+        node = self.nodes[0]
+
+        self.log.info('Generate an empty block to address')
+        address = node.getnewaddress()
+        hash = node.generateblock(address, [])['hash']
+        block = node.getblock(hash, 2)
+        assert_equal(len(block['tx']), 1)
+        assert_equal(block['tx'][0]['vout'][0]['scriptPubKey']['addresses'][0], address)
+
+        self.log.info('Generate an empty block to a descriptor')
+        hash = node.generateblock('addr(' + address + ')', [])['hash']
+        block = node.getblock(hash, 2)
+        assert_equal(len(block['tx']), 1)
+        assert_equal(block['tx'][0]['vout'][0]['scriptPubKey']['addresses'][0], address)
+
+        self.log.info('Generate an empty block to a combo descriptor with compressed pubkey')
+        combo_key = '0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798'
+        combo_address = 'bcrt1qw508d6qejxtdg4y5r3zarvary0c5xw7kygt080'
+        hash = node.generateblock('combo(' + combo_key + ')', [])['hash']
+        block = node.getblock(hash, 2)
+        assert_equal(len(block['tx']), 1)
+        assert_equal(block['tx'][0]['vout'][0]['scriptPubKey']['addresses'][0], combo_address)
+
+        self.log.info('Generate an empty block to a combo descriptor with uncompressed pubkey')
+        combo_key = '0408ef68c46d20596cc3f6ddf7c8794f71913add807f1dc55949fa805d764d191c0b7ce6894c126fce0babc6663042f3dde9b0cf76467ea315514e5a6731149c67'
+        combo_address = 'mkc9STceoCcjoXEXe6cm66iJbmjM6zR9B2'
+        hash = node.generateblock('combo(' + combo_key + ')', [])['hash']
+        block = node.getblock(hash, 2)
+        assert_equal(len(block['tx']), 1)
+        assert_equal(block['tx'][0]['vout'][0]['scriptPubKey']['addresses'][0], combo_address)
+
+        # Generate 110 blocks to spend
+        node.generatetoaddress(110, address)
+
+        # Generate some extra mempool transactions to verify they don't get mined
+        for i in range(10):
+            node.sendtoaddress(address, 0.001)
+
+        self.log.info('Generate block with txid')
+        txid = node.sendtoaddress(address, 1)
+        hash = node.generateblock(address, [txid])['hash']
+        block = node.getblock(hash, 1)
+        assert_equal(len(block['tx']), 2)
+        assert_equal(block['tx'][1], txid)
+
+        self.log.info('Generate block with raw tx')
+        utxos = node.listunspent(addresses=[address])
+        raw = node.createrawtransaction([{'txid':utxos[0]['txid'], 'vout':utxos[0]['vout']}],[{address:1}])
+        signed_raw = node.signrawtransactionwithwallet(raw)['hex']
+        hash = node.generateblock(address, [signed_raw])['hash']
+        block = node.getblock(hash, 1)
+        assert_equal(len(block['tx']), 2)
+        txid = block['tx'][1]
+        assert_equal(node.gettransaction(txid)['hex'], signed_raw)
+
+        self.log.info('Fail to generate block with out of order txs')
+        raw1 = node.createrawtransaction([{'txid':txid, 'vout':0}],[{address:0.9999}])
+        signed_raw1 = node.signrawtransactionwithwallet(raw1)['hex']
+        txid1 = node.sendrawtransaction(signed_raw1)
+        raw2 = node.createrawtransaction([{'txid':txid1, 'vout':0}],[{address:0.999}])
+        signed_raw2 = node.signrawtransactionwithwallet(raw2)['hex']
+        assert_raises_rpc_error(-25, 'TestBlockValidity failed: bad-txns-inputs-missingorspent', node.generateblock, address, [signed_raw2, txid1])
+
+        self.log.info('Fail to generate block with txid not in mempool')
+        missing_txid = '0000000000000000000000000000000000000000000000000000000000000000'
+        assert_raises_rpc_error(-5, 'Transaction ' + missing_txid + ' not in mempool.', node.generateblock, address, [missing_txid])
+
+        self.log.info('Fail to generate block with invalid raw tx')
+        invalid_raw_tx = '0000'
+        assert_raises_rpc_error(-22, 'Transaction decode failed for ' + invalid_raw_tx, node.generateblock, address, [invalid_raw_tx])
+
+        self.log.info('Fail to generate block with invalid address/descriptor')
+        assert_raises_rpc_error(-5, 'Invalid address or descriptor', node.generateblock, '1234', [])
+
+        self.log.info('Fail to generate block with a ranged descriptor')
+        ranged_descriptor = 'pkh(tpubD6NzVbkrYhZ4XgiXtGrdW5XDAPFCL9h7we1vwNCpn8tGbBcgfVYjXyhWo4E1xkh56hjod1RhGjxbaTLV3X4FyWuejifB9jusQ46QzG87VKp/0/*)'
+        assert_raises_rpc_error(-8, 'Ranged descriptor not accepted. Maybe pass through deriveaddresses first?', node.generateblock, ranged_descriptor, [])
+
+        self.log.info('Fail to generate block with a descriptor missing a private key')
+        child_descriptor = 'pkh(tpubD6NzVbkrYhZ4XgiXtGrdW5XDAPFCL9h7we1vwNCpn8tGbBcgfVYjXyhWo4E1xkh56hjod1RhGjxbaTLV3X4FyWuejifB9jusQ46QzG87VKp/0\'/0)'
+        assert_raises_rpc_error(-5, 'Cannot derive script without private keys', node.generateblock, child_descriptor, [])
+
+if __name__ == '__main__':
+    GenerateBlockTest().main()
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index faa2dee4ed..ee71de3310 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -173,6 +173,7 @@ BASE_SCRIPTS = [
     'wallet_importprunedfunds.py',
     'p2p_leak_tx.py',
     'rpc_signmessage.py',
+    'rpc_generateblock.py',
     'wallet_balance.py',
     'feature_nulldummy.py',
     'mempool_accept.py',