Merge pull request #4484 from vijaydasmp/backport_v19_vijay_batch_1

merge bitcoin#15685,#15335,#15642,#15686,#15629

doc/developer-notes.md

@@ -1033,8 +1033,7 @@ A few guidelines for introducing and reviewing new RPC interfaces:
     from there.
 
 - A RPC method must either be a wallet method or a non-wallet method. Do not
-  introduce new methods such as `signrawtransaction` that differ in behavior
+  introduce new methods that differ in behavior based on presence of a wallet.
-  based on presence of a wallet.
 
   - *Rationale*: as well as complicating the implementation and interfering
     with the introduction of multi-wallet, wallet and non-wallet code should be

src/coins.h

@@ -331,6 +331,10 @@ public:
     bool HaveInputs(const CTransaction& tx) const;
 
 private:
+    /**
+     * @note this is marked const, but may actually append to `cacheCoins`, increasing
+     * memory usage.
+     */
     CCoinsMap::iterator FetchCoin(const COutPoint &outpoint) const;
 };
 

src/init.cpp

@@ -1170,19 +1170,6 @@ void InitParameterInteraction()
     if (gArgs.IsArgSet("-masternodeblsprivkey") && gArgs.SoftSetBoolArg("-disablewallet", true)) {
         LogPrintf("%s: parameter interaction: -masternodeblsprivkey set -> setting -disablewallet=1\n", __func__);
     }
-
-    // Warn if network-specific options (-addnode, -connect, etc) are
-    // specified in default section of config file, but not overridden
-    // on the command line or in this network's section of the config file.
-    std::string network = gArgs.GetChainName();
-    for (const auto& arg : gArgs.GetUnsuitableSectionOnlyArgs()) {
-        InitWarning(strprintf(_("Config setting for %s only applied on %s network when in [%s] section."), arg, network, network));
-    }
-
-    // Warn if unrecognized section name are present in the config file.
-    for (const auto& section : gArgs.GetUnrecognizedSections()) {
-        InitWarning(strprintf(_("Section [%s] is not recognized."), section));
-    }
 }
 
 static std::string ResolveErrMsg(const char * const optname, const std::string& strBind)
@@ -1297,6 +1284,19 @@ bool AppInitParameterInteraction()
 
     // also see: InitParameterInteraction()
 
+    // Warn if network-specific options (-addnode, -connect, etc) are
+    // specified in default section of config file, but not overridden
+    // on the command line or in this network's section of the config file.
+    std::string network = gArgs.GetChainName();
+    for (const auto& arg : gArgs.GetUnsuitableSectionOnlyArgs()) {
+        return InitError(strprintf(_("Config setting for %s only applied on %s network when in [%s] section."), arg, network, network));
+    }
+
+    // Warn if unrecognized section name are present in the config file.
+    for (const auto& section : gArgs.GetUnrecognizedSections()) {
+        InitWarning(strprintf("%s:%i " + _("Section [%s] is not recognized."), section.m_file, section.m_line, section.m_name));
+    }
+
     if (!fs::is_directory(GetBlocksDir())) {
         return InitError(strprintf(_("Specified blocks directory \"%s\" does not exist."), gArgs.GetArg("-blocksdir", "").c_str()));
     }

src/qt/rpcconsole.cpp

@@ -72,7 +72,6 @@ const QStringList historyFilter = QStringList()
     << "importprivkey"
     << "importmulti"
     << "signmessagewithprivkey"
-    << "signrawtransaction"
     << "signrawtransactionwithkey"
     << "walletpassphrase"
     << "walletpassphrasechange"

src/rpc/mining.cpp

@@ -488,16 +488,16 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
         throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
 
     if (g_connman->GetNodeCount(CConnman::CONNECTIONS_ALL) == 0)
-        throw JSONRPCError(RPC_CLIENT_NOT_CONNECTED, "Dash Core is not connected!");
+        throw JSONRPCError(RPC_CLIENT_NOT_CONNECTED, PACKAGE_NAME " is not connected!");
 
     if (IsInitialBlockDownload())
-        throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, "Dash Core is downloading blocks...");
+        throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, PACKAGE_NAME " is in initial sync and waiting for blocks...");
 
     // next bock is a superblock and we need governance info to correctly construct it
     if (AreSuperblocksEnabled()
         && !masternodeSync.IsSynced()
         && CSuperblock::IsValidBlockHeight(chainActive.Height() + 1))
-            throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, "Dash Core is syncing with network...");
+            throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, PACKAGE_NAME "is syncing with network...");
 
     static unsigned int nTransactionsUpdatedLast;
 

src/rpc/rawtransaction.cpp

@@ -1015,14 +1015,6 @@ static UniValue signrawtransactionwithkey(const JSONRPCRequest& request)
     return SignTransaction(*g_rpc_interfaces->chain, mtx, request.params[2], &keystore, true, request.params[3]);
 }
 
-UniValue signrawtransaction(const JSONRPCRequest& request)
-{
-    // This method should be removed entirely in V0.19, along with the entries in the
-    // CRPCCommand table and rpc/client.cpp.
-    throw JSONRPCError(RPC_METHOD_DEPRECATED, "signrawtransaction was removed in v0.18.\n"
-        "Clients should transition to using signrawtransactionwithkey and signrawtransactionwithwallet");
-}
-
 UniValue sendrawtransaction(const JSONRPCRequest& request)
 {
     if (request.fHelp || request.params.size() < 1 || request.params.size() > 4)
@@ -1684,7 +1676,6 @@ static const CRPCCommand commands[] =
     { "rawtransactions",    "decodescript",                 &decodescript,              {"hexstring"} },
     { "rawtransactions",    "sendrawtransaction",           &sendrawtransaction,        {"hexstring","allowhighfees","instantsend","bypasslimits"} },
     { "rawtransactions",    "combinerawtransaction",        &combinerawtransaction,     {"txs"} },
-    { "hidden",             "signrawtransaction",           &signrawtransaction,        {"hexstring","prevtxs","privkeys","sighashtype"} },
     { "rawtransactions",    "signrawtransactionwithkey",    &signrawtransactionwithkey, {"hexstring","privkeys","prevtxs","sighashtype"} },
     { "rawtransactions",    "testmempoolaccept",            &testmempoolaccept,         {"rawtxs","allowhighfees"} },
     { "rawtransactions",    "decodepsbt",                   &decodepsbt,                {"psbt"} },

src/test/util_tests.cpp

@@ -132,9 +132,10 @@ struct TestArgsManager : public ArgsManager
         {
             LOCK(cs_args);
             m_config_args.clear();
+            m_config_sections.clear();
         }
         std::string error;
-        BOOST_REQUIRE(ReadConfigStream(streamConfig, error));
+        BOOST_REQUIRE(ReadConfigStream(streamConfig, "", error));
     }
     void SetNetworkOnlyArg(const std::string arg)
     {

src/util/system.cpp

@@ -382,8 +382,7 @@ const std::set<std::string> ArgsManager::GetUnsuitableSectionOnlyArgs() const
     return unsuitables;
 }
 
-
+const std::list<SectionInfo> ArgsManager::GetUnrecognizedSections() const
-const std::set<std::string> ArgsManager::GetUnrecognizedSections() const
 {
     // Section names to be recognized in the config file.
     static const std::set<std::string> available_sections{
@@ -392,14 +391,11 @@ const std::set<std::string> ArgsManager::GetUnrecognizedSections() const
         CBaseChainParams::MAIN,
         CBaseChainParams::DEVNET
     };
-    std::set<std::string> diff;
 
     LOCK(cs_args);
-    std::set_difference(
+    std::list<SectionInfo> unrecognized = m_config_sections;
-        m_config_sections.begin(), m_config_sections.end(),
+    unrecognized.remove_if([](const SectionInfo& appeared){ return available_sections.find(appeared.m_name) != available_sections.end(); });
-        available_sections.begin(), available_sections.end(),
+    return unrecognized;
-        std::inserter(diff, diff.end()));
-    return diff;
 }
 
 void ArgsManager::SelectConfigNetwork(const std::string& network)
@@ -860,7 +856,7 @@ static std::string TrimString(const std::string& str, const std::string& pattern
     return str.substr(front, end - front + 1);
 }
 
-static bool GetConfigOptions(std::istream& stream, std::string& error, std::vector<std::pair<std::string, std::string>>& options, std::set<std::string>& sections)
+static bool GetConfigOptions(std::istream& stream, const std::string& filepath, std::string& error, std::vector<std::pair<std::string, std::string>>& options, std::list<SectionInfo>& sections)
 {
     std::string str, prefix;
     std::string::size_type pos;
@@ -876,7 +872,7 @@ static bool GetConfigOptions(std::istream& stream, std::string& error, std::vect
         if (!str.empty()) {
             if (*str.begin() == '[' && *str.rbegin() == ']') {
                 const std::string section = str.substr(1, str.size() - 2);
-                sections.insert(section);
+                sections.emplace_back(SectionInfo{section, filepath, linenr});
                 prefix = section + '.';
             } else if (*str.begin() == '-') {
                 error = strprintf("parse error on line %i: %s, options in configuration file must be specified without leading -", linenr, str);
@@ -889,8 +885,8 @@ static bool GetConfigOptions(std::istream& stream, std::string& error, std::vect
                     return false;
                 }
                 options.emplace_back(name, value);
-                if ((pos = name.rfind('.')) != std::string::npos) {
+                if ((pos = name.rfind('.')) != std::string::npos && prefix.length() <= pos) {
-                    sections.insert(name.substr(0, pos));
+                    sections.emplace_back(SectionInfo{name.substr(0, pos), filepath, linenr});
                 }
             } else {
                 error = strprintf("parse error on line %i: %s", linenr, str);
@@ -905,12 +901,11 @@ static bool GetConfigOptions(std::istream& stream, std::string& error, std::vect
     return true;
 }
 
-bool ArgsManager::ReadConfigStream(std::istream& stream, std::string& error, bool ignore_invalid_keys)
+bool ArgsManager::ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys)
 {
     LOCK(cs_args);
     std::vector<std::pair<std::string, std::string>> options;
-    m_config_sections.clear();
+    if (!GetConfigOptions(stream, filepath, error, options, m_config_sections)) {
-    if (!GetConfigOptions(stream, error, options, m_config_sections)) {
         return false;
     }
     for (const std::pair<std::string, std::string>& option : options) {
@@ -941,13 +936,14 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
     {
         LOCK(cs_args);
         m_config_args.clear();
+        m_config_sections.clear();
     }
 
     const std::string confPath = GetArg("-conf", BITCOIN_CONF_FILENAME);
     fsbridge::ifstream stream(GetConfigFile(confPath));
 
     if (stream.good()) {
-        if (!ReadConfigStream(stream, error, ignore_invalid_keys)) {
+        if (!ReadConfigStream(stream, confPath, error, ignore_invalid_keys)) {
             return false;
         }
         // if there is an -includeconf in the override args, but it is empty, that means the user
@@ -978,7 +974,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
             for (const std::string& to_include : includeconf) {
                 fsbridge::ifstream include_config(GetConfigFile(to_include));
                 if (include_config.good()) {
-                    if (!ReadConfigStream(include_config, error, ignore_invalid_keys)) {
+                    if (!ReadConfigStream(include_config, to_include, error, ignore_invalid_keys)) {
                         return false;
                     }
                     LogPrintf("Included configuration file %s\n", to_include.c_str());

src/util/system.h

@@ -158,6 +158,13 @@ enum class OptionsCategory {
     HIDDEN // Always the last option to avoid printing these in the help
 };
 
+struct SectionInfo
+{
+    std::string m_name;
+    std::string m_file;
+    int m_line;
+};
+
 class ArgsManager
 {
 protected:
@@ -178,9 +185,9 @@ protected:
     std::string m_network GUARDED_BY(cs_args);
     std::set<std::string> m_network_only_args GUARDED_BY(cs_args);
     std::map<OptionsCategory, std::map<std::string, Arg>> m_available_args GUARDED_BY(cs_args);
-    std::set<std::string> m_config_sections GUARDED_BY(cs_args);
+    std::list<SectionInfo> m_config_sections GUARDED_BY(cs_args);
 
-    [[nodiscard]] bool ReadConfigStream(std::istream& stream, std::string& error, bool ignore_invalid_keys = false);
+    [[nodiscard]] bool ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys = false);
 
 public:
     ArgsManager();
@@ -204,7 +211,7 @@ public:
     /**
      * Log warnings for unrecognized section names in the config file.
      */
-    const std::set<std::string> GetUnrecognizedSections() const;
+    const std::list<SectionInfo> GetUnrecognizedSections() const;
 
     /**
      * Return a vector of strings of the given argument

src/validation.cpp

@@ -646,6 +646,13 @@ static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, CValidationSt
     return CheckInputs(tx, state, view, true, flags, cacheSigStore, true, txdata);
 }
 
+/**
+ * @param[out] coins_to_uncache   Return any outpoints which were not previously present in the
+ *                                coins cache, but were added as a result of validating the tx
+ *                                for mempool acceptance. This allows the caller to optionally
+ *                                remove the cache additions if the associated transaction ends
+ *                                up being rejected by the mempool.
+ */
 static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool& pool, CValidationState& state, const CTransactionRef& ptx,
                                      bool* pfMissingInputs, int64_t nAcceptTime, bool bypass_limits,
                                      const CAmount& nAbsurdFee, std::vector<COutPoint>& coins_to_uncache, bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
@@ -733,6 +740,10 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
             if (!pcoinsTip->HaveCoinInCache(txin.prevout)) {
                 coins_to_uncache.push_back(txin.prevout);
             }
+
+            // Note: this call may add txin.prevout to the coins cache
+            // (pcoinsTip.cacheCoins) by way of FetchCoin(). It should be removed
+            // later (via coins_to_uncache) if this tx turns out to be invalid.
             if (!view.HaveCoin(txin.prevout)) {
                 // Are inputs missing because we already have the tx?
                 for (size_t out = 0; out < tx.vout.size(); out++) {
@@ -927,6 +938,11 @@ static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPo
     bool res = AcceptToMemoryPoolWorker(chainparams, pool, state, tx, pfMissingInputs, nAcceptTime, bypass_limits, nAbsurdFee, coins_to_uncache, test_accept);
     if (!res || test_accept) {
         if(!res) LogPrint(BCLog::MEMPOOL, "%s: %s %s (%s)\n", __func__, tx->GetHash().ToString(), state.GetRejectReason(), state.GetDebugMessage());
+           // Remove coins that were not present in the coins cache before calling ATMPW;
+           // this is to prevent memory DoS in case we receive a large number of
+           // invalid transactions that attempt to overrun the in-memory coins cache
+           // (`CCoinsViewCache::cacheCoins`).
+
         for (const COutPoint& hashTx : coins_to_uncache)
             pcoinsTip->Uncache(hashTx);
     }

test/functional/feature_config_args.py

@@ -35,6 +35,10 @@ class ConfArgsTest(BitcoinTestFramework):
             conf.write('-dash=1\n')
         self.nodes[0].assert_start_raises_init_error(expected_msg='Error reading configuration file: parse error on line 1: -dash=1, options in configuration file must be specified without leading -')
 
+        with open(inc_conf_file_path, 'w', encoding='utf8') as conf:
+            conf.write("wallet=foo\n")
+        self.nodes[0].assert_start_raises_init_error(expected_msg='Error: Config setting for -wallet only applied on regtest network when in [regtest] section.')
+
         with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
             conf.write('nono\n')
         self.nodes[0].assert_start_raises_init_error(expected_msg='Error reading configuration file: parse error on line 1: nono, if you intended to specify a negated option, use nono=1 instead')
@@ -51,12 +55,20 @@ class ConfArgsTest(BitcoinTestFramework):
             conf.write('server=1\nrpcuser=someuser\n[main]\nrpcpassword=some#pass')
         self.nodes[0].assert_start_raises_init_error(expected_msg='Error reading configuration file: parse error on line 4, using # in rpcpassword can be ambiguous and should be avoided')
 
-        with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
+        inc_conf_file2_path = os.path.join(self.nodes[0].datadir, 'include2.conf')
-            conf.write('testnot.datadir=1\n[testnet]\n')
+        with open(os.path.join(self.nodes[0].datadir, 'dash.conf'), 'a', encoding='utf-8') as conf:
-        self.restart_node(0)
+            conf.write('includeconf={}\n'.format(inc_conf_file2_path))
-        self.nodes[0].stop_node(expected_stderr='Warning: Section [testnet] is not recognized.' + os.linesep + 'Warning: Section [testnot] is not recognized.')
 
         with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
+            conf.write('testnot.datadir=1\n')
+        with open(inc_conf_file2_path, 'w', encoding='utf-8') as conf:
+            conf.write('[testnet]\n')
+        self.restart_node(0)
+        self.nodes[0].stop_node(expected_stderr='Warning: ' + inc_conf_file_path + ':1 Section [testnot] is not recognized.' + os.linesep + 'Warning: ' + inc_conf_file2_path + ':1 Section [testnet] is not recognized.')
+
+        with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
+            conf.write('')  # clear
+        with open(inc_conf_file2_path, 'w', encoding='utf-8') as conf:
             conf.write('')  # clear
 
 

test/functional/feature_pruning.py

@@ -8,12 +8,14 @@ WARNING:
 This test uses 4GB of disk space.
 This test takes 30 mins or more (up to 2 hours)
 """
-
-from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import assert_equal, assert_greater_than, assert_raises_rpc_error, connect_nodes, mine_large_block, wait_until
-
 import os
 
+from test_framework.blocktools import create_coinbase
+from test_framework.messages import CBlock, ToHex
+from test_framework.script import CScript, OP_RETURN, OP_NOP
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_equal, assert_greater_than, assert_raises_rpc_error, connect_nodes, disconnect_nodes, wait_until
+
 MIN_BLOCKS_TO_KEEP = 288
 
 # Rescans start at the earliest block up to 2 hours before a key timestamp, so
@@ -21,19 +23,59 @@ MIN_BLOCKS_TO_KEEP = 288
 # compatible with pruning based on key creation time.
 TIMESTAMP_WINDOW = 2 * 60 * 60
 
+def mine_large_blocks(node, n):
+    # Make a large scriptPubKey for the coinbase transaction. This is OP_RETURN
+    # followed by 950k of OP_NOP. This would be non-standard in a non-coinbase
+    # transaction but is consensus valid.
+
+    # Get the block parameters for the first block
+    big_script = CScript([OP_RETURN] + [OP_NOP] * 950000)
+    best_block = node.getblock(node.getbestblockhash())
+    height = int(best_block["height"]) + 1
+    try:
+        # Static variable ensures that time is monotonicly increasing and is therefore
+        # different for each block created => blockhash is unique.
+        mine_large_blocks.nTime = min(mine_large_blocks.nTime, int(best_block["time"])) + 1
+    except AttributeError:
+        mine_large_blocks.nTime = int(best_block["time"]) + 1
+    previousblockhash = int(best_block["hash"], 16)
+
+    for _ in range(n):
+        # Build the coinbase transaction (with large scriptPubKey)
+        coinbase_tx = create_coinbase(height)
+        coinbase_tx.vin[0].nSequence = 2 ** 32 - 1
+        coinbase_tx.vout[0].scriptPubKey = big_script
+        coinbase_tx.rehash()
+
+        # Build the block
+        block = CBlock()
+        block.nVersion = best_block["version"]
+        block.hashPrevBlock = previousblockhash
+        block.nTime = mine_large_blocks.nTime
+        block.nBits = int('207fffff', 16)
+        block.nNonce = 0
+        block.vtx = [coinbase_tx]
+        block.hashMerkleRoot = block.calc_merkle_root()
+        block.solve()
+
+        # Submit to the node
+        node.submitblock(ToHex(block))
+
+        previousblockhash = block.sha256
+        height += 1
+        mine_large_blocks.nTime += 1
 
 def calc_usage(blockdir):
-    return sum(os.path.getsize(blockdir+f) for f in os.listdir(blockdir) if os.path.isfile(os.path.join(blockdir, f))) / (1024. * 1024.)
+    return sum(os.path.getsize(blockdir + f) for f in os.listdir(blockdir) if os.path.isfile(os.path.join(blockdir, f))) / (1024. * 1024.)
 
 class PruneTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
         self.num_nodes = 6
-        self.rpc_timeout = 900
 
         # Create nodes 0 and 1 to mine.
         # Create node 2 to test pruning.
-        self.full_node_default_args = ["-dip3params=2000:2000", "-dip8params=2000", "-maxreceivebuffer=20000","-blockmaxsize=999000", "-checkblocks=5", "-limitdescendantcount=100", "-limitdescendantsize=5000", "-limitancestorcount=100", "-limitancestorsize=5000"]
+        self.full_node_default_args = ["-dip3params=2000:2000", "-dip8params=2000", "-maxreceivebuffer=20000", "-blockmaxsize=999000", "-checkblocks=5"]
         # Create nodes 3 and 4 to test manual pruning (they will be re-started with manual pruning later)
         # Create nodes 5 to test wallet in prune mode, but do not connect
         self.extra_args = [
@@ -55,7 +97,7 @@ class PruneTest(BitcoinTestFramework):
 
         connect_nodes(self.nodes[0], 1)
         connect_nodes(self.nodes[1], 2)
-        connect_nodes(self.nodes[2], 0)
+        connect_nodes(self.nodes[0], 2)
         connect_nodes(self.nodes[0], 3)
         connect_nodes(self.nodes[0], 4)
         self.sync_blocks(self.nodes[0:5])
@@ -69,21 +111,19 @@ class PruneTest(BitcoinTestFramework):
         self.nodes[1].generate(200)
         self.sync_blocks(self.nodes[0:2])
         self.nodes[0].generate(150)
+
         # Then mine enough full blocks to create more than 550MiB of data
-        for i in range(645):
+        mine_large_blocks(self.nodes[0], 645)
-            mine_large_block(self.nodes[0], self.utxo_cache_0)
 
         self.sync_blocks(self.nodes[0:5])
 
     def test_height_min(self):
-        if not os.path.isfile(os.path.join(self.prunedir, "blk00000.dat")):
+        assert os.path.isfile(os.path.join(self.prunedir, "blk00000.dat")), "blk00000.dat is missing, pruning too early"
-            raise AssertionError("blk00000.dat is missing, pruning too early")
         self.log.info("Success")
         self.log.info("Though we're already using more than 550MiB, current usage: %d" % calc_usage(self.prunedir))
         self.log.info("Mining 25 more blocks should cause the first block file to be pruned")
         # Pruning doesn't run until we're allocating another chunk, 20 full blocks past the height cutoff will ensure this
-        for i in range(25):
+        mine_large_blocks(self.nodes[0], 25)
-            mine_large_block(self.nodes[0], self.utxo_cache_0)
 
         # Wait for blk00000.dat to be pruned
         wait_until(lambda: not os.path.isfile(os.path.join(self.prunedir, "blk00000.dat")), timeout=30)
@@ -91,8 +131,7 @@ class PruneTest(BitcoinTestFramework):
         self.log.info("Success")
         usage = calc_usage(self.prunedir)
         self.log.info("Usage should be below target: %d" % usage)
-        if (usage > 550):
+        assert_greater_than(550, usage)
-            raise AssertionError("Pruning target not being met")
 
     def create_chain_with_staleblocks(self):
         # Create stale blocks in manageable sized chunks
@@ -101,26 +140,17 @@ class PruneTest(BitcoinTestFramework):
         for j in range(12):
             # Disconnect node 0 so it can mine a longer reorg chain without knowing about node 1's soon-to-be-stale chain
             # Node 2 stays connected, so it hears about the stale blocks and then reorg's when node0 reconnects
-            # Stopping node 0 also clears its mempool, so it doesn't have node1's transactions to accidentally mine
+            disconnect_nodes(self.nodes[0], 1)
-            self.stop_node(0)
+            disconnect_nodes(self.nodes[0], 2)
-            self.start_node(0, extra_args=self.full_node_default_args)
             # Mine 24 blocks in node 1
-            for i in range(24):
+            mine_large_blocks(self.nodes[1], 24)
-                if j == 0:
-                    mine_large_block(self.nodes[1], self.utxo_cache_1)
-                else:
-                    # Add node1's wallet transactions back to the mempool, to
-                    # avoid the mined blocks from being too small.
-                    self.nodes[1].resendwallettransactions()
-                    self.nodes[1].generate(1) #tx's already in mempool from previous disconnects
 
             # Reorg back with 25 block chain from node 0
-            for i in range(25):
+            mine_large_blocks(self.nodes[0], 25)
-                mine_large_block(self.nodes[0], self.utxo_cache_0)
 
             # Create connections in the order so both nodes can see the reorg at the same time
-            connect_nodes(self.nodes[1], 0)
+            connect_nodes(self.nodes[0], 1)
-            connect_nodes(self.nodes[2], 0)
+            connect_nodes(self.nodes[0], 2)
             self.sync_blocks(self.nodes[0:3])
 
         self.log.info("Usage can be over target because of high stale rate: %d" % calc_usage(self.prunedir))
@@ -128,65 +158,50 @@ class PruneTest(BitcoinTestFramework):
     def reorg_test(self):
         # Node 1 will mine a 300 block chain starting 287 blocks back from Node 0 and Node 2's tip
         # This will cause Node 2 to do a reorg requiring 288 blocks of undo data to the reorg_test chain
-        # Reboot node 1 to clear its mempool (hopefully make the invalidate faster)
-        # Lower the block max size so we don't keep mining all our big mempool transactions (from disconnected blocks)
-        self.stop_node(1)
-        self.start_node(1, extra_args=["-dip3params=2000:2000", "-dip8params=2000", "-maxreceivebuffer=20000","-blockmaxsize=5000", "-checkblocks=5"])
 
         height = self.nodes[1].getblockcount()
         self.log.info("Current block height: %d" % height)
 
-        invalidheight = height-287
+        self.forkheight = height - 287
-        badhash = self.nodes[1].getblockhash(invalidheight)
+        self.forkhash = self.nodes[1].getblockhash(self.forkheight)
-        self.log.info("Invalidating block %s at height %d" % (badhash,invalidheight))
+        self.log.info("Invalidating block %s at height %d" % (self.forkhash, self.forkheight))
-        self.nodes[1].invalidateblock(badhash)
+        self.nodes[1].invalidateblock(self.forkhash)
 
         # We've now switched to our previously mined-24 block fork on node 1, but that's not what we want
         # So invalidate that fork as well, until we're on the same chain as node 0/2 (but at an ancestor 288 blocks ago)
-        mainchainhash = self.nodes[0].getblockhash(invalidheight - 1)
+        mainchainhash = self.nodes[0].getblockhash(self.forkheight - 1)
-        curhash = self.nodes[1].getblockhash(invalidheight - 1)
+        curhash = self.nodes[1].getblockhash(self.forkheight - 1)
         while curhash != mainchainhash:
             self.nodes[1].invalidateblock(curhash)
-            curhash = self.nodes[1].getblockhash(invalidheight - 1)
+            curhash = self.nodes[1].getblockhash(self.forkheight - 1)
 
-        assert self.nodes[1].getblockcount() == invalidheight - 1
+        assert self.nodes[1].getblockcount() == self.forkheight - 1
         self.log.info("New best height: %d" % self.nodes[1].getblockcount())
 
         # Mine one block to avoid automatic recovery from forks on restart
         self.nodes[1].generate(1)
-        # Reboot node1 to clear those giant tx's from mempool
+        # Disconnect node1 and generate the new chain
-        self.stop_node(1)
+        disconnect_nodes(self.nodes[0], 1)
-        self.start_node(1, extra_args=["-dip3params=2000:2000", "-dip8params=2000", "-maxreceivebuffer=20000","-blockmaxsize=5000", "-checkblocks=5"])
+        disconnect_nodes(self.nodes[1], 2)
 
         self.log.info("Generating new longer chain of 300 more blocks")
         self.nodes[1].generate(299)
 
         self.log.info("Reconnect nodes")
         connect_nodes(self.nodes[0], 1)
-        connect_nodes(self.nodes[2], 1)
+        connect_nodes(self.nodes[1], 2)
         self.sync_blocks(self.nodes[0:3], timeout=120)
 
         self.log.info("Verify height on node 2: %d" % self.nodes[2].getblockcount())
-        self.log.info("Usage possibly still high bc of stale blocks in block files: %d" % calc_usage(self.prunedir))
+        self.log.info("Usage possibly still high because of stale blocks in block files: %d" % calc_usage(self.prunedir))
 
-        self.log.info("Mine 220 more blocks so we have requisite history (some blocks will be big and cause pruning of previous chain)")
+        self.log.info("Mine 220 more large blocks so we have requisite history")
 
-        # Get node0's wallet transactions back in its mempool, to avoid the
+        mine_large_blocks(self.nodes[0], 220)
-        # mined blocks from being too small.
-        self.nodes[0].resendwallettransactions()
-
-        for i in range(22):
-            # This can be slow, so do this in multiple RPC calls to avoid
-            # RPC timeouts.
-            self.nodes[0].generate(10) #node 0 has many large tx's in its mempool from the disconnects
-        self.sync_blocks(self.nodes[0:3], timeout=300)
 
         usage = calc_usage(self.prunedir)
         self.log.info("Usage should be below target: %d" % usage)
-        if (usage > 550):
+        assert_greater_than(550, usage)
-            raise AssertionError("Pruning target not being met")
-
-        return invalidheight,badhash
 
     def reorg_back(self):
         # Verify that a block on the old main chain fork has been pruned away
@@ -219,17 +234,17 @@ class PruneTest(BitcoinTestFramework):
             blocks_to_mine = first_reorg_height + 1 - self.mainchainheight
             self.log.info("Rewind node 0 to prev main chain to mine longer chain to trigger redownload. Blocks needed: %d" % blocks_to_mine)
             self.nodes[0].invalidateblock(curchainhash)
-            assert self.nodes[0].getblockcount() == self.mainchainheight
+            assert_equal(self.nodes[0].getblockcount(), self.mainchainheight)
-            assert self.nodes[0].getbestblockhash() == self.mainchainhash2
+            assert_equal(self.nodes[0].getbestblockhash(), self.mainchainhash2)
             goalbesthash = self.nodes[0].generate(blocks_to_mine)[-1]
             goalbestheight = first_reorg_height + 1
 
         self.log.info("Verify node 2 reorged back to the main chain, some blocks of which it had to redownload")
         # Wait for Node 2 to reorg to proper height
         wait_until(lambda: self.nodes[2].getblockcount() >= goalbestheight, timeout=900)
-        assert self.nodes[2].getbestblockhash() == goalbesthash
+        assert_equal(self.nodes[2].getbestblockhash(), goalbesthash)
         # Verify we can now have the data for a block previously pruned
-        assert self.nodes[2].getblock(self.forkhash)["height"] == self.forkheight
+        assert_equal(self.nodes[2].getblock(self.forkhash)["height"], self.forkheight)
 
     def manual_test(self, node_number, use_timestamp):
         # at this point, node has 995 blocks and has not yet run in prune mode
@@ -287,38 +302,30 @@ class PruneTest(BitcoinTestFramework):
 
         # height=100 too low to prune first block file so this is a no-op
         prune(100)
-        if not has_block(0):
+        assert has_block(0), "blk00000.dat is missing when should still be there"
-            raise AssertionError("blk00000.dat is missing when should still be there")
 
         # Does nothing
         node.pruneblockchain(height(0))
-        if not has_block(0):
+        assert has_block(0), "blk00000.dat is missing when should still be there"
-            raise AssertionError("blk00000.dat is missing when should still be there")
 
         # height=500 should prune first file
         prune(500)
-        if has_block(0):
+        assert not has_block(0), "blk00000.dat is still there, should be pruned by now"
-            raise AssertionError("blk00000.dat is still there, should be pruned by now")
+        assert has_block(1), "blk00001.dat is missing when should still be there"
-        if not has_block(1):
-            raise AssertionError("blk00001.dat is missing when should still be there")
 
         # height=650 should prune second file
         prune(650)
-        if has_block(1):
+        assert not has_block(1), "blk00001.dat is still there, should be pruned by now"
-            raise AssertionError("blk00001.dat is still there, should be pruned by now")
 
         # height=1000 should not prune anything more, because tip-288 is in blk00002.dat.
         prune(1000, 1001 - MIN_BLOCKS_TO_KEEP)
-        if not has_block(2):
+        assert has_block(2), "blk00002.dat is still there, should be pruned by now"
-            raise AssertionError("blk00002.dat is still there, should be pruned by now")
 
         # advance the tip so blk00002.dat and blk00003.dat can be pruned (the last 288 blocks should now be in blk00004.dat)
         node.generate(288)
         prune(1000)
-        if has_block(2):
+        assert not has_block(2), "blk00002.dat is still there, should be pruned by now"
-            raise AssertionError("blk00002.dat is still there, should be pruned by now")
+        assert not has_block(3), "blk00003.dat is still there, should be pruned by now"
-        if has_block(3):
-            raise AssertionError("blk00003.dat is still there, should be pruned by now")
 
         # stop node, start back up with auto-prune at 550 MiB, make sure still runs
         self.stop_node(node_number, expected_stderr='Warning: You are starting with governance validation disabled. This is expected because you are running a pruned node.')
@@ -339,21 +346,14 @@ class PruneTest(BitcoinTestFramework):
         connect_nodes(self.nodes[0], 5)
         nds = [self.nodes[0], self.nodes[5]]
         self.sync_blocks(nds, wait=5, timeout=300)
-        self.stop_node(5, expected_stderr='Warning: You are starting with governance validation disabled. This is expected because you are running a pruned node.') #stop and start to trigger rescan
+        self.stop_node(5, expected_stderr='Warning: You are starting with governance validation disabled. This is expected because you are running a pruned node.') # stop and start to trigger rescan
         self.start_node(5, extra_args=["-dip3params=2000:2000", "-dip8params=2000", "-disablegovernance", "-txindex=0", "-prune=550"])
         self.log.info("Success")
 
     def run_test(self):
-        self.log.info("Warning! This test requires 4GB of disk space and takes over 30 mins (up to 2 hours)")
+        self.log.info("Warning! This test requires 4GB of disk space")
+
         self.log.info("Mining a big blockchain of 995 blocks")
-
-        # Determine default relay fee
-        self.relayfee = self.nodes[0].getnetworkinfo()["relayfee"]
-
-        # Cache for utxos, as the listunspent may take a long time later in the test
-        self.utxo_cache_0 = []
-        self.utxo_cache_1 = []
-
         self.create_big_chain()
         # Chain diagram key:
         # *   blocks on main chain
@@ -394,11 +394,11 @@ class PruneTest(BitcoinTestFramework):
         #                    +...+(1044)  &..                    $...$(1319)
 
         # Save some current chain state for later use
-        self.mainchainheight = self.nodes[2].getblockcount()   #1320
+        self.mainchainheight = self.nodes[2].getblockcount()  # 1320
         self.mainchainhash2 = self.nodes[2].getblockhash(self.mainchainheight)
 
         self.log.info("Check that we can survive a 288 block reorg still")
-        (self.forkheight,self.forkhash) = self.reorg_test() #(1033, )
+        self.reorg_test()  # (1033, )
         # Now create a 288 block reorg by mining a longer chain on N1
         # First disconnect N1
         # Then invalidate 1033 on main chain and 1032 on fork so height is 1032 on main chain

test/functional/rpc_signrawtransaction.py

@@ -12,7 +12,6 @@ class SignRawTransactionsTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
         self.num_nodes = 1
-        self.extra_args = [["-deprecatedrpc=signrawtransaction"]]
 
     def skip_test_if_missing_module(self):
         self.skip_if_no_wallet()

test/functional/test_framework/test_node.py

@@ -380,7 +380,7 @@ class TestNode():
                 stderr=subprocess.DEVNULL, stdout=subprocess.DEVNULL) == 0
 
         if not sys.platform.startswith('linux'):
-            self.log.warning("Can't profile with perf; only availabe on Linux platforms")
+            self.log.warning("Can't profile with perf; only available on Linux platforms")
             return None
 
         if not test_success('which perf'):