Merge #11445: [qa] 0.15.1 Backports

019c492 qa: Fix lcov for out-of-tree builds (MarcoFalke)
e169349 qa: Restore bitcoin-util-test py2 compatibility (MarcoFalke)
806c78f add functional test for mempoolreplacement command line arg (Gregory Sanders)
a825d4a Fix bip68-sequence rpc test (Johnson Lau)
a36f332 Verify DBWrapper iterators are taking snapshots (Matt Corallo)
8d2e51d qa: Fix bug introduced in p2p-segwit.py (Suhas Daftuar)
2f0b30a qa: Treat mininode p2p exceptions as fatal (Suhas Daftuar)
e4605d9 Tests for zmqpubrawtx and zmqpubrawblock (Andrew Chow)
2c4ff35 [script] Unit tests for IsMine (Jim Posen)
794a80e [script] Unit tests for script/standard functions (Jim Posen)
f9cf7b5 [tests] Check connectivity before sending in assumevalid.py (John Newbery)
f1ced0d [tests] Make p2p-leaktests.py more robust (John Newbery)
2e1ac70 [qa] zapwallettxes: Wait up to 3s for mempool reload (MarcoFalke)
b6468d3 Add listwallets RPC test to multiwallet.py (Cristian Mircea Messel)
d8dd8e7 [tests] fixup dbcrash interaction with add_nodes() (John Newbery)
2b97b36 [test] Replace check_output with low level version (João Barbosa)
e38211f [test] Add assert_raises_process_error to assert process errors (João Barbosa)
e0bfd28 [test] Add support for custom arguments to TestNodeCLI (João Barbosa)
812c870 [test] Improve assert_raises_jsonrpc docstring (João Barbosa)
eeb24a3 [qa] TestNode: Add wait_until_stopped helper method (MarcoFalke)
f3f7891 Stop test_bitcoin-qt touching ~/.bitcoin (MeshCollider)
f0b6795 Remove redundant testutil files (MeshCollider)
4424176 Improve signmessages functional test (Cristian Mircea Messel)
cef0319 [tests] fixups from set_test_params() (John Newbery)
82bf6fc [tests] Functional tests must explicitly set num_nodes (John Newbery)
801d2ae [tests] don't override __init__() in individual tests (John Newbery)
bb5e7cb [tests] Avoid passing around member variables in test_framework (John Newbery)
4d3ba18 [tests] TestNode: separate add_node from start_node (John Newbery)
11a5992 [tests] fix - use rpc_timeout as rpc timeout (John Newbery)
847c75e Add getmininginfo functional test (Cristian Mircea Messel)
2a5d099 RPC: gettxout: Slightly improve doc and tests (Jorge Timón)
716066d [tests] Add bitcoin_cli.py test script (John Newbery)
016b9ad [tests] add TestNodeCLI class for calling bitcoin-cli for a node (John Newbery)
5398f20 qa: Move wait_until to util (MarcoFalke)
1d80d1e [tests] fix timeout issues from TestNode (John Newbery)
c276c1e test: Increase initial RPC timeout to 60 seconds (Wladimir J. van der Laan)
fc2aa09 [tests] Introduce TestNode (John Newbery)

Pull request description:

  This includes test related backports for 0.15.1. The motivation is twofold:

  * Make backporting new tests written for current master easier
  * Fix the most common test issues that happen(ed) frequently on travis

  Even though this includes the new TestNode class, which comes with a lot
  of refactoring, I believe that the issues caused by refactoring are found
  and fixed by now.

Tree-SHA512: 6a0c4e5246da83ff0b3f7d2cb8df358d105ed548fb3857e5d882f26cc336553aa07b39e38c281879bf82f95078298b775334f9a60c0b23140f77c50174bd8347

configure.ac

@@ -1231,6 +1231,7 @@ AC_SUBST(QR_LIBS)
 AC_CONFIG_FILES([Makefile src/Makefile doc/man/Makefile share/setup.nsi share/qt/Info.plist test/config.ini])
 AC_CONFIG_FILES([contrib/devtools/split-debug.sh],[chmod +x contrib/devtools/split-debug.sh])
 AC_CONFIG_FILES([doc/Doxyfile])
+AC_CONFIG_LINKS([contrib/filter-lcov.py:contrib/filter-lcov.py])
 AC_CONFIG_LINKS([test/functional/test_runner.py:test/functional/test_runner.py])
 AC_CONFIG_LINKS([test/util/bitcoin-util-test.py:test/util/bitcoin-util-test.py])
 

src/Makefile.qttest.include

@@ -25,12 +25,10 @@ TEST_QT_H = \
   qt/test/wallettests.h
 
 TEST_BITCOIN_CPP = \
-  test/test_bitcoin.cpp \
+  test/test_bitcoin.cpp
-  test/testutil.cpp
 
 TEST_BITCOIN_H = \
-  test/test_bitcoin.h \
+  test/test_bitcoin.h
-  test/testutil.h
 
 qt_test_test_bitcoin_qt_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BITCOIN_QT_INCLUDES) \
   $(QT_INCLUDES) $(QT_TEST_INCLUDES) $(PROTOBUF_CFLAGS)

src/Makefile.test.include

@@ -65,6 +65,7 @@ BITCOIN_TESTS =\
   test/scheduler_tests.cpp \
   test/script_P2SH_tests.cpp \
   test/script_tests.cpp \
+  test/script_standard_tests.cpp \
   test/scriptnum_tests.cpp \
   test/serialize_tests.cpp \
   test/sighash_tests.cpp \
@@ -74,8 +75,6 @@ BITCOIN_TESTS =\
   test/test_bitcoin.cpp \
   test/test_bitcoin.h \
   test/test_bitcoin_main.cpp \
-  test/testutil.cpp \
-  test/testutil.h \
   test/timedata_tests.cpp \
   test/torcontrol_tests.cpp \
   test/transaction_tests.cpp \
@@ -148,7 +147,7 @@ bitcoin_test_clean : FORCE
 
 check-local:
 	@echo "Running test/util/bitcoin-util-test.py..."
-	$(top_builddir)/test/util/bitcoin-util-test.py
+	$(PYTHON) $(top_builddir)/test/util/bitcoin-util-test.py
 	$(AM_V_at)$(MAKE) $(AM_MAKEFLAGS) -C secp256k1 check
 if EMBEDDED_UNIVALUE
 	$(AM_V_at)$(MAKE) $(AM_MAKEFLAGS) -C univalue check

src/qt/test/rpcnestedtests.cpp

@@ -11,7 +11,6 @@
 #include "rpc/register.h"
 #include "rpc/server.h"
 #include "rpcconsole.h"
-#include "test/testutil.h"
 #include "test/test_bitcoin.h"
 #include "univalue.h"
 #include "util.h"
@@ -37,11 +36,6 @@ void RPCNestedTests::rpcNestedTests()
     // do some test setup
     // could be moved to a more generic place when we add more tests on QT level
     tableRPC.appendCommand("rpcNestedTest", &vRPCCommands[0]);
-    ClearDatadirCache();
-    std::string path = QDir::tempPath().toStdString() + "/" + strprintf("test_bitcoin_qt_%lu_%i", (unsigned long)GetTime(), (int)(GetRand(100000)));
-    QDir dir(QString::fromStdString(path));
-    dir.mkpath(".");
-    gArgs.ForceSetArg("-datadir", path);
     //mempool.setSanityCheck(1.0);
 
     TestingSetup test;
@@ -136,6 +130,4 @@ void RPCNestedTests::rpcNestedTests()
     QVERIFY_EXCEPTION_THROWN(RPCConsole::RPCExecuteCommandLine(result, "rpcNestedTest(abc,,abc)"), std::runtime_error); //don't tollerate empty arguments when using ,
     QVERIFY_EXCEPTION_THROWN(RPCConsole::RPCExecuteCommandLine(result, "rpcNestedTest(abc,,)"), std::runtime_error); //don't tollerate empty arguments when using ,
 #endif
-
-    fs::remove_all(fs::path(path));
 }

src/qt/test/test_main.cpp

@@ -53,6 +53,10 @@ int main(int argc, char *argv[])
     SetupNetworking();
     SelectParams(CBaseChainParams::MAIN);
     noui_connect();
+    ClearDatadirCache();
+    fs::path pathTemp = fs::temp_directory_path() / strprintf("test_bitcoin-qt_%lu_%i", (unsigned long)GetTime(), (int)GetRand(100000));
+    fs::create_directories(pathTemp);
+    gArgs.ForceSetArg("-datadir", pathTemp.string());
 
     bool fInvalid = false;
 
@@ -97,5 +101,7 @@ int main(int argc, char *argv[])
     }
 #endif
 
+    fs::remove_all(pathTemp);
+
     return fInvalid;
 }

src/rpc/blockchain.cpp

@@ -945,8 +945,9 @@ UniValue gettxout(const JSONRPCRequest& request)
             "\nReturns details about an unspent transaction output.\n"
             "\nArguments:\n"
             "1. \"txid\"             (string, required) The transaction id\n"
-            "2. n              (numeric, required) vout number\n"
+            "2. \"n\"                (numeric, required) vout number\n"
-            "3. include_mempool  (boolean, optional) Whether to include the mempool\n"
+            "3. \"include_mempool\"  (boolean, optional) Whether to include the mempool. Default: true."
+            "     Note that an unspent output that is spent in the mempool won't appear.\n"
             "\nResult:\n"
             "{\n"
             "  \"bestblock\" : \"hash\",    (string) the block hash\n"

src/script/ismine.cpp

@@ -46,6 +46,8 @@ isminetype IsMine(const CKeyStore &keystore, const CTxDestination& dest, bool& i
 
 isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey, bool& isInvalid, SigVersion sigversion)
 {
+    isInvalid = false;
+
     std::vector<valtype> vSolutions;
     txnouttype whichType;
     if (!Solver(scriptPubKey, whichType, vSolutions)) {

src/test/dbwrapper_tests.cpp

@@ -204,19 +204,31 @@ BOOST_AUTO_TEST_CASE(iterator_ordering)
     for (int x=0x00; x<256; ++x) {
         uint8_t key = x;
         uint32_t value = x*x;
-        BOOST_CHECK(dbw.Write(key, value));
+        if (!(x & 1)) BOOST_CHECK(dbw.Write(key, value));
     }
 
+    // Check that creating an iterator creates a snapshot
     std::unique_ptr<CDBIterator> it(const_cast<CDBWrapper&>(dbw).NewIterator());
+
+    for (int x=0x00; x<256; ++x) {
+        uint8_t key = x;
+        uint32_t value = x*x;
+        if (x & 1) BOOST_CHECK(dbw.Write(key, value));
+    }
+
     for (int seek_start : {0x00, 0x80}) {
         it->Seek((uint8_t)seek_start);
-        for (int x=seek_start; x<256; ++x) {
+        for (int x=seek_start; x<255; ++x) {
             uint8_t key;
             uint32_t value;
             BOOST_CHECK(it->Valid());
             if (!it->Valid()) // Avoid spurious errors about invalid iterator's key and value in case of failure
                 break;
             BOOST_CHECK(it->GetKey(key));
+            if (x & 1) {
+                BOOST_CHECK_EQUAL(key, x + 1);
+                continue;
+            }
             BOOST_CHECK(it->GetValue(value));
             BOOST_CHECK_EQUAL(key, x);
             BOOST_CHECK_EQUAL(value, x*x);

src/test/multisig_tests.cpp

@@ -16,8 +16,6 @@
 
 #include <boost/test/unit_test.hpp>
 
-typedef std::vector<unsigned char> valtype;
-
 BOOST_FIXTURE_TEST_SUITE(multisig_tests, BasicTestingSetup)
 
 CScript
@@ -173,95 +171,6 @@ BOOST_AUTO_TEST_CASE(multisig_IsStandard)
         BOOST_CHECK(!::IsStandard(malformed[i], whichType));
 }
 
-BOOST_AUTO_TEST_CASE(multisig_Solver1)
-{
-    // Tests Solver() that returns lists of keys that are
-    // required to satisfy a ScriptPubKey
-    //
-    // Also tests IsMine() and ExtractDestination()
-    //
-    // Note: ExtractDestination for the multisignature transactions
-    // always returns false for this release, even if you have
-    // one key that would satisfy an (a|b) or 2-of-3 keys needed
-    // to spend an escrow transaction.
-    //
-    CBasicKeyStore keystore, emptykeystore, partialkeystore;
-    CKey key[3];
-    CTxDestination keyaddr[3];
-    for (int i = 0; i < 3; i++)
-    {
-        key[i].MakeNewKey(true);
-        keystore.AddKey(key[i]);
-        keyaddr[i] = key[i].GetPubKey().GetID();
-    }
-    partialkeystore.AddKey(key[0]);
-
-    {
-        std::vector<valtype> solutions;
-        txnouttype whichType;
-        CScript s;
-        s << ToByteVector(key[0].GetPubKey()) << OP_CHECKSIG;
-        BOOST_CHECK(Solver(s, whichType, solutions));
-        BOOST_CHECK(solutions.size() == 1);
-        CTxDestination addr;
-        BOOST_CHECK(ExtractDestination(s, addr));
-        BOOST_CHECK(addr == keyaddr[0]);
-        BOOST_CHECK(IsMine(keystore, s));
-        BOOST_CHECK(!IsMine(emptykeystore, s));
-    }
-    {
-        std::vector<valtype> solutions;
-        txnouttype whichType;
-        CScript s;
-        s << OP_DUP << OP_HASH160 << ToByteVector(key[0].GetPubKey().GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
-        BOOST_CHECK(Solver(s, whichType, solutions));
-        BOOST_CHECK(solutions.size() == 1);
-        CTxDestination addr;
-        BOOST_CHECK(ExtractDestination(s, addr));
-        BOOST_CHECK(addr == keyaddr[0]);
-        BOOST_CHECK(IsMine(keystore, s));
-        BOOST_CHECK(!IsMine(emptykeystore, s));
-    }
-    {
-        std::vector<valtype> solutions;
-        txnouttype whichType;
-        CScript s;
-        s << OP_2 << ToByteVector(key[0].GetPubKey()) << ToByteVector(key[1].GetPubKey()) << OP_2 << OP_CHECKMULTISIG;
-        BOOST_CHECK(Solver(s, whichType, solutions));
-        BOOST_CHECK_EQUAL(solutions.size(), 4U);
-        CTxDestination addr;
-        BOOST_CHECK(!ExtractDestination(s, addr));
-        BOOST_CHECK(IsMine(keystore, s));
-        BOOST_CHECK(!IsMine(emptykeystore, s));
-        BOOST_CHECK(!IsMine(partialkeystore, s));
-    }
-    {
-        std::vector<valtype> solutions;
-        txnouttype whichType;
-        CScript s;
-        s << OP_1 << ToByteVector(key[0].GetPubKey()) << ToByteVector(key[1].GetPubKey()) << OP_2 << OP_CHECKMULTISIG;
-        BOOST_CHECK(Solver(s, whichType, solutions));
-        BOOST_CHECK_EQUAL(solutions.size(), 4U);
-        std::vector<CTxDestination> addrs;
-        int nRequired;
-        BOOST_CHECK(ExtractDestinations(s, whichType, addrs, nRequired));
-        BOOST_CHECK(addrs[0] == keyaddr[0]);
-        BOOST_CHECK(addrs[1] == keyaddr[1]);
-        BOOST_CHECK(nRequired == 1);
-        BOOST_CHECK(IsMine(keystore, s));
-        BOOST_CHECK(!IsMine(emptykeystore, s));
-        BOOST_CHECK(!IsMine(partialkeystore, s));
-    }
-    {
-        std::vector<valtype> solutions;
-        txnouttype whichType;
-        CScript s;
-        s << OP_2 << ToByteVector(key[0].GetPubKey()) << ToByteVector(key[1].GetPubKey()) << ToByteVector(key[2].GetPubKey()) << OP_3 << OP_CHECKMULTISIG;
-        BOOST_CHECK(Solver(s, whichType, solutions));
-        BOOST_CHECK(solutions.size() == 5);
-    }
-}
-
 BOOST_AUTO_TEST_CASE(multisig_Sign)
 {
     // Test SignSignature() (and therefore the version of Solver() that signs transactions)

src/test/script_standard_tests.cpp

@@ -0,0 +1,740 @@
+// Copyright (c) 2017 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include "key.h"
+#include "keystore.h"
+#include "script/ismine.h"
+#include "script/script.h"
+#include "script/script_error.h"
+#include "script/standard.h"
+#include "test/test_bitcoin.h"
+
+#include <boost/test/unit_test.hpp>
+
+
+BOOST_FIXTURE_TEST_SUITE(script_standard_tests, BasicTestingSetup)
+
+BOOST_AUTO_TEST_CASE(script_standard_Solver_success)
+{
+    CKey keys[3];
+    CPubKey pubkeys[3];
+    for (int i = 0; i < 3; i++) {
+        keys[i].MakeNewKey(true);
+        pubkeys[i] = keys[i].GetPubKey();
+    }
+
+    CScript s;
+    txnouttype whichType;
+    std::vector<std::vector<unsigned char> > solutions;
+
+    // TX_PUBKEY
+    s.clear();
+    s << ToByteVector(pubkeys[0]) << OP_CHECKSIG;
+    BOOST_CHECK(Solver(s, whichType, solutions));
+    BOOST_CHECK_EQUAL(whichType, TX_PUBKEY);
+    BOOST_CHECK_EQUAL(solutions.size(), 1);
+    BOOST_CHECK(solutions[0] == ToByteVector(pubkeys[0]));
+
+    // TX_PUBKEYHASH
+    s.clear();
+    s << OP_DUP << OP_HASH160 << ToByteVector(pubkeys[0].GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
+    BOOST_CHECK(Solver(s, whichType, solutions));
+    BOOST_CHECK_EQUAL(whichType, TX_PUBKEYHASH);
+    BOOST_CHECK_EQUAL(solutions.size(), 1);
+    BOOST_CHECK(solutions[0] == ToByteVector(pubkeys[0].GetID()));
+
+    // TX_SCRIPTHASH
+    CScript redeemScript(s); // initialize with leftover P2PKH script
+    s.clear();
+    s << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL;
+    BOOST_CHECK(Solver(s, whichType, solutions));
+    BOOST_CHECK_EQUAL(whichType, TX_SCRIPTHASH);
+    BOOST_CHECK_EQUAL(solutions.size(), 1);
+    BOOST_CHECK(solutions[0] == ToByteVector(CScriptID(redeemScript)));
+
+    // TX_MULTISIG
+    s.clear();
+    s << OP_1 <<
+        ToByteVector(pubkeys[0]) <<
+        ToByteVector(pubkeys[1]) <<
+        OP_2 << OP_CHECKMULTISIG;
+    BOOST_CHECK(Solver(s, whichType, solutions));
+    BOOST_CHECK_EQUAL(whichType, TX_MULTISIG);
+    BOOST_CHECK_EQUAL(solutions.size(), 4);
+    BOOST_CHECK(solutions[0] == std::vector<unsigned char>({1}));
+    BOOST_CHECK(solutions[1] == ToByteVector(pubkeys[0]));
+    BOOST_CHECK(solutions[2] == ToByteVector(pubkeys[1]));
+    BOOST_CHECK(solutions[3] == std::vector<unsigned char>({2}));
+
+    s.clear();
+    s << OP_2 <<
+        ToByteVector(pubkeys[0]) <<
+        ToByteVector(pubkeys[1]) <<
+        ToByteVector(pubkeys[2]) <<
+        OP_3 << OP_CHECKMULTISIG;
+    BOOST_CHECK(Solver(s, whichType, solutions));
+    BOOST_CHECK_EQUAL(whichType, TX_MULTISIG);
+    BOOST_CHECK_EQUAL(solutions.size(), 5);
+    BOOST_CHECK(solutions[0] == std::vector<unsigned char>({2}));
+    BOOST_CHECK(solutions[1] == ToByteVector(pubkeys[0]));
+    BOOST_CHECK(solutions[2] == ToByteVector(pubkeys[1]));
+    BOOST_CHECK(solutions[3] == ToByteVector(pubkeys[2]));
+    BOOST_CHECK(solutions[4] == std::vector<unsigned char>({3}));
+
+    // TX_NULL_DATA
+    s.clear();
+    s << OP_RETURN <<
+        std::vector<unsigned char>({0}) <<
+        std::vector<unsigned char>({75}) <<
+        std::vector<unsigned char>({255});
+    BOOST_CHECK(Solver(s, whichType, solutions));
+    BOOST_CHECK_EQUAL(whichType, TX_NULL_DATA);
+    BOOST_CHECK_EQUAL(solutions.size(), 0);
+
+    // TX_WITNESS_V0_KEYHASH
+    s.clear();
+    s << OP_0 << ToByteVector(pubkeys[0].GetID());
+    BOOST_CHECK(Solver(s, whichType, solutions));
+    BOOST_CHECK_EQUAL(whichType, TX_WITNESS_V0_KEYHASH);
+    BOOST_CHECK_EQUAL(solutions.size(), 1);
+    BOOST_CHECK(solutions[0] == ToByteVector(pubkeys[0].GetID()));
+
+    // TX_WITNESS_V0_SCRIPTHASH
+    uint256 scriptHash;
+    CSHA256().Write(&redeemScript[0], redeemScript.size())
+        .Finalize(scriptHash.begin());
+
+    s.clear();
+    s << OP_0 << ToByteVector(scriptHash);
+    BOOST_CHECK(Solver(s, whichType, solutions));
+    BOOST_CHECK_EQUAL(whichType, TX_WITNESS_V0_SCRIPTHASH);
+    BOOST_CHECK_EQUAL(solutions.size(), 1);
+    BOOST_CHECK(solutions[0] == ToByteVector(scriptHash));
+
+    // TX_NONSTANDARD
+    s.clear();
+    s << OP_9 << OP_ADD << OP_11 << OP_EQUAL;
+    BOOST_CHECK(!Solver(s, whichType, solutions));
+    BOOST_CHECK_EQUAL(whichType, TX_NONSTANDARD);
+}
+
+BOOST_AUTO_TEST_CASE(script_standard_Solver_failure)
+{
+    CKey key;
+    CPubKey pubkey;
+    key.MakeNewKey(true);
+    pubkey = key.GetPubKey();
+
+    CScript s;
+    txnouttype whichType;
+    std::vector<std::vector<unsigned char> > solutions;
+
+    // TX_PUBKEY with incorrectly sized pubkey
+    s.clear();
+    s << std::vector<unsigned char>(30, 0x01) << OP_CHECKSIG;
+    BOOST_CHECK(!Solver(s, whichType, solutions));
+
+    // TX_PUBKEYHASH with incorrectly sized key hash
+    s.clear();
+    s << OP_DUP << OP_HASH160 << ToByteVector(pubkey) << OP_EQUALVERIFY << OP_CHECKSIG;
+    BOOST_CHECK(!Solver(s, whichType, solutions));
+
+    // TX_SCRIPTHASH with incorrectly sized script hash
+    s.clear();
+    s << OP_HASH160 << std::vector<unsigned char>(21, 0x01) << OP_EQUAL;
+    BOOST_CHECK(!Solver(s, whichType, solutions));
+
+    // TX_MULTISIG 0/2
+    s.clear();
+    s << OP_0 << ToByteVector(pubkey) << OP_1 << OP_CHECKMULTISIG;
+    BOOST_CHECK(!Solver(s, whichType, solutions));
+
+    // TX_MULTISIG 2/1
+    s.clear();
+    s << OP_2 << ToByteVector(pubkey) << OP_1 << OP_CHECKMULTISIG;
+    BOOST_CHECK(!Solver(s, whichType, solutions));
+
+    // TX_MULTISIG n = 2 with 1 pubkey
+    s.clear();
+    s << OP_1 << ToByteVector(pubkey) << OP_2 << OP_CHECKMULTISIG;
+    BOOST_CHECK(!Solver(s, whichType, solutions));
+
+    // TX_MULTISIG n = 1 with 0 pubkeys
+    s.clear();
+    s << OP_1 << OP_1 << OP_CHECKMULTISIG;
+    BOOST_CHECK(!Solver(s, whichType, solutions));
+
+    // TX_NULL_DATA with other opcodes
+    s.clear();
+    s << OP_RETURN << std::vector<unsigned char>({75}) << OP_ADD;
+    BOOST_CHECK(!Solver(s, whichType, solutions));
+
+    // TX_WITNESS with unknown version
+    s.clear();
+    s << OP_1 << ToByteVector(pubkey);
+    BOOST_CHECK(!Solver(s, whichType, solutions));
+
+    // TX_WITNESS with incorrect program size
+    s.clear();
+    s << OP_0 << std::vector<unsigned char>(19, 0x01);
+    BOOST_CHECK(!Solver(s, whichType, solutions));
+}
+
+BOOST_AUTO_TEST_CASE(script_standard_ExtractDestination)
+{
+    CKey key;
+    CPubKey pubkey;
+    key.MakeNewKey(true);
+    pubkey = key.GetPubKey();
+
+    CScript s;
+    CTxDestination address;
+
+    // TX_PUBKEY
+    s.clear();
+    s << ToByteVector(pubkey) << OP_CHECKSIG;
+    BOOST_CHECK(ExtractDestination(s, address));
+    BOOST_CHECK(boost::get<CKeyID>(&address) &&
+                *boost::get<CKeyID>(&address) == pubkey.GetID());
+
+    // TX_PUBKEYHASH
+    s.clear();
+    s << OP_DUP << OP_HASH160 << ToByteVector(pubkey.GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
+    BOOST_CHECK(ExtractDestination(s, address));
+    BOOST_CHECK(boost::get<CKeyID>(&address) &&
+                *boost::get<CKeyID>(&address) == pubkey.GetID());
+
+    // TX_SCRIPTHASH
+    CScript redeemScript(s); // initialize with leftover P2PKH script
+    s.clear();
+    s << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL;
+    BOOST_CHECK(ExtractDestination(s, address));
+    BOOST_CHECK(boost::get<CScriptID>(&address) &&
+                *boost::get<CScriptID>(&address) == CScriptID(redeemScript));
+
+    // TX_MULTISIG
+    s.clear();
+    s << OP_1 << ToByteVector(pubkey) << OP_1 << OP_CHECKMULTISIG;
+    BOOST_CHECK(!ExtractDestination(s, address));
+
+    // TX_NULL_DATA
+    s.clear();
+    s << OP_RETURN << std::vector<unsigned char>({75});
+    BOOST_CHECK(!ExtractDestination(s, address));
+
+    // TX_WITNESS_V0_KEYHASH
+    s.clear();
+    s << OP_0 << ToByteVector(pubkey);
+    BOOST_CHECK(!ExtractDestination(s, address));
+
+    // TX_WITNESS_V0_SCRIPTHASH
+    s.clear();
+    s << OP_0 << ToByteVector(CScriptID(redeemScript));
+    BOOST_CHECK(!ExtractDestination(s, address));
+}
+
+BOOST_AUTO_TEST_CASE(script_standard_ExtractDestinations)
+{
+    CKey keys[3];
+    CPubKey pubkeys[3];
+    for (int i = 0; i < 3; i++) {
+        keys[i].MakeNewKey(true);
+        pubkeys[i] = keys[i].GetPubKey();
+    }
+
+    CScript s;
+    txnouttype whichType;
+    std::vector<CTxDestination> addresses;
+    int nRequired;
+
+    // TX_PUBKEY
+    s.clear();
+    s << ToByteVector(pubkeys[0]) << OP_CHECKSIG;
+    BOOST_CHECK(ExtractDestinations(s, whichType, addresses, nRequired));
+    BOOST_CHECK_EQUAL(whichType, TX_PUBKEY);
+    BOOST_CHECK_EQUAL(addresses.size(), 1);
+    BOOST_CHECK_EQUAL(nRequired, 1);
+    BOOST_CHECK(boost::get<CKeyID>(&addresses[0]) &&
+                *boost::get<CKeyID>(&addresses[0]) == pubkeys[0].GetID());
+
+    // TX_PUBKEYHASH
+    s.clear();
+    s << OP_DUP << OP_HASH160 << ToByteVector(pubkeys[0].GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
+    BOOST_CHECK(ExtractDestinations(s, whichType, addresses, nRequired));
+    BOOST_CHECK_EQUAL(whichType, TX_PUBKEYHASH);
+    BOOST_CHECK_EQUAL(addresses.size(), 1);
+    BOOST_CHECK_EQUAL(nRequired, 1);
+    BOOST_CHECK(boost::get<CKeyID>(&addresses[0]) &&
+                *boost::get<CKeyID>(&addresses[0]) == pubkeys[0].GetID());
+
+    // TX_SCRIPTHASH
+    CScript redeemScript(s); // initialize with leftover P2PKH script
+    s.clear();
+    s << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL;
+    BOOST_CHECK(ExtractDestinations(s, whichType, addresses, nRequired));
+    BOOST_CHECK_EQUAL(whichType, TX_SCRIPTHASH);
+    BOOST_CHECK_EQUAL(addresses.size(), 1);
+    BOOST_CHECK_EQUAL(nRequired, 1);
+    BOOST_CHECK(boost::get<CScriptID>(&addresses[0]) &&
+                *boost::get<CScriptID>(&addresses[0]) == CScriptID(redeemScript));
+
+    // TX_MULTISIG
+    s.clear();
+    s << OP_2 <<
+        ToByteVector(pubkeys[0]) <<
+        ToByteVector(pubkeys[1]) <<
+        OP_2 << OP_CHECKMULTISIG;
+    BOOST_CHECK(ExtractDestinations(s, whichType, addresses, nRequired));
+    BOOST_CHECK_EQUAL(whichType, TX_MULTISIG);
+    BOOST_CHECK_EQUAL(addresses.size(), 2);
+    BOOST_CHECK_EQUAL(nRequired, 2);
+    BOOST_CHECK(boost::get<CKeyID>(&addresses[0]) &&
+                *boost::get<CKeyID>(&addresses[0]) == pubkeys[0].GetID());
+    BOOST_CHECK(boost::get<CKeyID>(&addresses[1]) &&
+                *boost::get<CKeyID>(&addresses[1]) == pubkeys[1].GetID());
+
+    // TX_NULL_DATA
+    s.clear();
+    s << OP_RETURN << std::vector<unsigned char>({75});
+    BOOST_CHECK(!ExtractDestinations(s, whichType, addresses, nRequired));
+
+    // TX_WITNESS_V0_KEYHASH
+    s.clear();
+    s << OP_0 << ToByteVector(pubkeys[0].GetID());
+    BOOST_CHECK(!ExtractDestinations(s, whichType, addresses, nRequired));
+
+    // TX_WITNESS_V0_SCRIPTHASH
+    s.clear();
+    s << OP_0 << ToByteVector(CScriptID(redeemScript));
+    BOOST_CHECK(!ExtractDestinations(s, whichType, addresses, nRequired));
+}
+
+BOOST_AUTO_TEST_CASE(script_standard_GetScriptFor_)
+{
+    CKey keys[3];
+    CPubKey pubkeys[3];
+    for (int i = 0; i < 3; i++) {
+        keys[i].MakeNewKey(true);
+        pubkeys[i] = keys[i].GetPubKey();
+    }
+
+    CScript expected, result;
+
+    // CKeyID
+    expected.clear();
+    expected << OP_DUP << OP_HASH160 << ToByteVector(pubkeys[0].GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
+    result = GetScriptForDestination(pubkeys[0].GetID());
+    BOOST_CHECK(result == expected);
+
+    // CScriptID
+    CScript redeemScript(result);
+    expected.clear();
+    expected << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL;
+    result = GetScriptForDestination(CScriptID(redeemScript));
+    BOOST_CHECK(result == expected);
+
+    // CNoDestination
+    expected.clear();
+    result = GetScriptForDestination(CNoDestination());
+    BOOST_CHECK(result == expected);
+
+    // GetScriptForRawPubKey
+    expected.clear();
+    expected << ToByteVector(pubkeys[0]) << OP_CHECKSIG;
+    result = GetScriptForRawPubKey(pubkeys[0]);
+    BOOST_CHECK(result == expected);
+
+    // GetScriptForMultisig
+    expected.clear();
+    expected << OP_2 <<
+        ToByteVector(pubkeys[0]) <<
+        ToByteVector(pubkeys[1]) <<
+        ToByteVector(pubkeys[2]) <<
+        OP_3 << OP_CHECKMULTISIG;
+    result = GetScriptForMultisig(2, std::vector<CPubKey>(pubkeys, pubkeys + 3));
+    BOOST_CHECK(result == expected);
+
+    // GetScriptForWitness
+    CScript witnessScript;
+
+    witnessScript << ToByteVector(pubkeys[0]) << OP_CHECKSIG;
+    expected.clear();
+    expected << OP_0 << ToByteVector(pubkeys[0].GetID());
+    result = GetScriptForWitness(witnessScript);
+    BOOST_CHECK(result == expected);
+
+    witnessScript.clear();
+    witnessScript << OP_DUP << OP_HASH160 << ToByteVector(pubkeys[0].GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
+    result = GetScriptForWitness(witnessScript);
+    BOOST_CHECK(result == expected);
+
+    witnessScript.clear();
+    witnessScript << OP_1 << ToByteVector(pubkeys[0]) << OP_1 << OP_CHECKMULTISIG;
+
+    uint256 scriptHash;
+    CSHA256().Write(&witnessScript[0], witnessScript.size())
+        .Finalize(scriptHash.begin());
+
+    expected.clear();
+    expected << OP_0 << ToByteVector(scriptHash);
+    result = GetScriptForWitness(witnessScript);
+    BOOST_CHECK(result == expected);
+}
+
+BOOST_AUTO_TEST_CASE(script_standard_IsMine)
+{
+    CKey keys[2];
+    CPubKey pubkeys[2];
+    for (int i = 0; i < 2; i++) {
+        keys[i].MakeNewKey(true);
+        pubkeys[i] = keys[i].GetPubKey();
+    }
+
+    CKey uncompressedKey;
+    uncompressedKey.MakeNewKey(false);
+    CPubKey uncompressedPubkey = uncompressedKey.GetPubKey();
+
+    CScript scriptPubKey;
+    isminetype result;
+    bool isInvalid;
+
+    // P2PK compressed
+    {
+        CBasicKeyStore keystore;
+        scriptPubKey.clear();
+        scriptPubKey << ToByteVector(pubkeys[0]) << OP_CHECKSIG;
+
+        // Keystore does not have key
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(!isInvalid);
+
+        // Keystore has key
+        keystore.AddKey(keys[0]);
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+        BOOST_CHECK(!isInvalid);
+    }
+
+    // P2PK uncompressed
+    {
+        CBasicKeyStore keystore;
+        scriptPubKey.clear();
+        scriptPubKey << ToByteVector(uncompressedPubkey) << OP_CHECKSIG;
+
+        // Keystore does not have key
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(!isInvalid);
+
+        // Keystore has key
+        keystore.AddKey(uncompressedKey);
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+        BOOST_CHECK(!isInvalid);
+    }
+
+    // P2PKH compressed
+    {
+        CBasicKeyStore keystore;
+        scriptPubKey.clear();
+        scriptPubKey << OP_DUP << OP_HASH160 << ToByteVector(pubkeys[0].GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
+
+        // Keystore does not have key
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(!isInvalid);
+
+        // Keystore has key
+        keystore.AddKey(keys[0]);
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+        BOOST_CHECK(!isInvalid);
+    }
+
+    // P2PKH uncompressed
+    {
+        CBasicKeyStore keystore;
+        scriptPubKey.clear();
+        scriptPubKey << OP_DUP << OP_HASH160 << ToByteVector(uncompressedPubkey.GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
+
+        // Keystore does not have key
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(!isInvalid);
+
+        // Keystore has key
+        keystore.AddKey(uncompressedKey);
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+        BOOST_CHECK(!isInvalid);
+    }
+
+    // P2SH
+    {
+        CBasicKeyStore keystore;
+
+        CScript redeemScript;
+        redeemScript << OP_DUP << OP_HASH160 << ToByteVector(pubkeys[0].GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
+
+        scriptPubKey.clear();
+        scriptPubKey << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL;
+
+        // Keystore does not have redeemScript or key
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(!isInvalid);
+
+        // Keystore has redeemScript but no key
+        keystore.AddCScript(redeemScript);
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(!isInvalid);
+
+        // Keystore has redeemScript and key
+        keystore.AddKey(keys[0]);
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+        BOOST_CHECK(!isInvalid);
+    }
+
+    // P2WPKH compressed
+    {
+        CBasicKeyStore keystore;
+        keystore.AddKey(keys[0]);
+
+        scriptPubKey.clear();
+        scriptPubKey << OP_0 << ToByteVector(pubkeys[0].GetID());
+
+        // Keystore has key, but no P2SH redeemScript
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(!isInvalid);
+
+        // Keystore has key and P2SH redeemScript
+        keystore.AddCScript(scriptPubKey);
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+        BOOST_CHECK(!isInvalid);
+    }
+
+    // P2WPKH uncompressed
+    {
+        CBasicKeyStore keystore;
+        keystore.AddKey(uncompressedKey);
+
+        scriptPubKey.clear();
+        scriptPubKey << OP_0 << ToByteVector(uncompressedPubkey.GetID());
+
+        // Keystore has key, but no P2SH redeemScript
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(!isInvalid);
+
+        // Keystore has key and P2SH redeemScript
+        keystore.AddCScript(scriptPubKey);
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(isInvalid);
+    }
+
+    // scriptPubKey multisig
+    {
+        CBasicKeyStore keystore;
+
+        scriptPubKey.clear();
+        scriptPubKey << OP_2 <<
+            ToByteVector(uncompressedPubkey) <<
+            ToByteVector(pubkeys[1]) <<
+            OP_2 << OP_CHECKMULTISIG;
+
+        // Keystore does not have any keys
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(!isInvalid);
+
+        // Keystore has 1/2 keys
+        keystore.AddKey(uncompressedKey);
+
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(!isInvalid);
+
+        // Keystore has 2/2 keys
+        keystore.AddKey(keys[1]);
+
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+        BOOST_CHECK(!isInvalid);
+    }
+
+    // P2SH multisig
+    {
+        CBasicKeyStore keystore;
+        keystore.AddKey(uncompressedKey);
+        keystore.AddKey(keys[1]);
+
+        CScript redeemScript;
+        redeemScript << OP_2 <<
+            ToByteVector(uncompressedPubkey) <<
+            ToByteVector(pubkeys[1]) <<
+            OP_2 << OP_CHECKMULTISIG;
+
+        scriptPubKey.clear();
+        scriptPubKey << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL;
+
+        // Keystore has no redeemScript
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(!isInvalid);
+
+        // Keystore has redeemScript
+        keystore.AddCScript(redeemScript);
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+        BOOST_CHECK(!isInvalid);
+    }
+
+    // P2WSH multisig with compressed keys
+    {
+        CBasicKeyStore keystore;
+        keystore.AddKey(keys[0]);
+        keystore.AddKey(keys[1]);
+
+        CScript witnessScript;
+        witnessScript << OP_2 <<
+            ToByteVector(pubkeys[0]) <<
+            ToByteVector(pubkeys[1]) <<
+            OP_2 << OP_CHECKMULTISIG;
+
+        uint256 scriptHash;
+        CSHA256().Write(&witnessScript[0], witnessScript.size())
+            .Finalize(scriptHash.begin());
+
+        scriptPubKey.clear();
+        scriptPubKey << OP_0 << ToByteVector(scriptHash);
+
+        // Keystore has keys, but no witnessScript or P2SH redeemScript
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(!isInvalid);
+
+        // Keystore has keys and witnessScript, but no P2SH redeemScript
+        keystore.AddCScript(witnessScript);
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(!isInvalid);
+
+        // Keystore has keys, witnessScript, P2SH redeemScript
+        keystore.AddCScript(scriptPubKey);
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+        BOOST_CHECK(!isInvalid);
+    }
+
+    // P2WSH multisig with uncompressed key
+    {
+        CBasicKeyStore keystore;
+        keystore.AddKey(uncompressedKey);
+        keystore.AddKey(keys[1]);
+
+        CScript witnessScript;
+        witnessScript << OP_2 <<
+            ToByteVector(uncompressedPubkey) <<
+            ToByteVector(pubkeys[1]) <<
+            OP_2 << OP_CHECKMULTISIG;
+
+        uint256 scriptHash;
+        CSHA256().Write(&witnessScript[0], witnessScript.size())
+            .Finalize(scriptHash.begin());
+
+        scriptPubKey.clear();
+        scriptPubKey << OP_0 << ToByteVector(scriptHash);
+
+        // Keystore has keys, but no witnessScript or P2SH redeemScript
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(!isInvalid);
+
+        // Keystore has keys and witnessScript, but no P2SH redeemScript
+        keystore.AddCScript(witnessScript);
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(!isInvalid);
+
+        // Keystore has keys, witnessScript, P2SH redeemScript
+        keystore.AddCScript(scriptPubKey);
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(isInvalid);
+    }
+
+    // P2WSH multisig wrapped in P2SH
+    {
+        CBasicKeyStore keystore;
+
+        CScript witnessScript;
+        witnessScript << OP_2 <<
+            ToByteVector(pubkeys[0]) <<
+            ToByteVector(pubkeys[1]) <<
+            OP_2 << OP_CHECKMULTISIG;
+
+        uint256 scriptHash;
+        CSHA256().Write(&witnessScript[0], witnessScript.size())
+            .Finalize(scriptHash.begin());
+
+        CScript redeemScript;
+        redeemScript << OP_0 << ToByteVector(scriptHash);
+
+        scriptPubKey.clear();
+        scriptPubKey << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL;
+
+        // Keystore has no witnessScript, P2SH redeemScript, or keys
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(!isInvalid);
+
+        // Keystore has witnessScript and P2SH redeemScript, but no keys
+        keystore.AddCScript(redeemScript);
+        keystore.AddCScript(witnessScript);
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(!isInvalid);
+
+        // Keystore has keys, witnessScript, P2SH redeemScript
+        keystore.AddKey(keys[0]);
+        keystore.AddKey(keys[1]);
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+        BOOST_CHECK(!isInvalid);
+    }
+
+    // OP_RETURN
+    {
+        CBasicKeyStore keystore;
+        keystore.AddKey(keys[0]);
+
+        scriptPubKey.clear();
+        scriptPubKey << OP_RETURN << ToByteVector(pubkeys[0]);
+
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(!isInvalid);
+    }
+
+    // Nonstandard
+    {
+        CBasicKeyStore keystore;
+        keystore.AddKey(keys[0]);
+
+        scriptPubKey.clear();
+        scriptPubKey << OP_9 << OP_ADD << OP_11 << OP_EQUAL;
+
+        result = IsMine(keystore, scriptPubKey, isInvalid);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+        BOOST_CHECK(!isInvalid);
+    }
+}
+
+BOOST_AUTO_TEST_SUITE_END()

src/test/test_bitcoin.cpp

@@ -22,8 +22,6 @@
 #include "rpc/register.h"
 #include "script/sigcache.h"
 
-#include "test/testutil.h"
-
 #include <memory>
 
 uint256 insecure_rand_seed = GetRandHash();
@@ -61,7 +59,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha
 
         RegisterAllCoreRPCCommands(tableRPC);
         ClearDatadirCache();
-        pathTemp = GetTempPath() / strprintf("test_bitcoin_%lu_%i", (unsigned long)GetTime(), (int)(InsecureRandRange(100000)));
+        pathTemp = fs::temp_directory_path() / strprintf("test_bitcoin_%lu_%i", (unsigned long)GetTime(), (int)(InsecureRandRange(100000)));
         fs::create_directories(pathTemp);
         gArgs.ForceSetArg("-datadir", pathTemp.string());
 

src/test/testutil.cpp

@@ -1,15 +0,0 @@
-// Copyright (c) 2009-2016 The Bitcoin Core developers
-// Distributed under the MIT software license, see the accompanying
-// file COPYING or http://www.opensource.org/licenses/mit-license.php.
-
-#include "testutil.h"
-
-#ifdef WIN32
-#include <shlobj.h>
-#endif
-
-#include "fs.h"
-
-fs::path GetTempPath() {
-    return fs::temp_directory_path();
-}

src/test/testutil.h

@@ -1,15 +0,0 @@
-// Copyright (c) 2009-2016 The Bitcoin Core developers
-// Distributed under the MIT software license, see the accompanying
-// file COPYING or http://www.opensource.org/licenses/mit-license.php.
-
-/**
- * Utility functions shared by unit tests
- */
-#ifndef BITCOIN_TEST_TESTUTIL_H
-#define BITCOIN_TEST_TESTUTIL_H
-
-#include "fs.h"
-
-fs::path GetTempPath();
-
-#endif // BITCOIN_TEST_TESTUTIL_H

test/functional/README.md

@@ -24,8 +24,8 @@ don't have test cases for.
 - Use a module-level docstring to describe what the test is testing, and how it
   is testing it.
 - When subclassing the BitcoinTestFramwork, place overrides for the
-  `__init__()`, and `setup_xxxx()` methods at the top of the subclass, then
+  `set_test_params()`, `add_options()` and `setup_xxxx()` methods at the top of
-  locally-defined helper methods, then the `run_test()` method.
+  the subclass, then locally-defined helper methods, then the `run_test()` method.
 
 #### General test-writing advice
 
@@ -36,7 +36,7 @@ don't have test cases for.
 - Avoid stop-starting the nodes multiple times during the test if possible. A
   stop-start takes several seconds, so doing it several times blows up the
   runtime of the test.
-- Set the `self.setup_clean_chain` variable in `__init__()` to control whether
+- Set the `self.setup_clean_chain` variable in `set_test_params()` to control whether
   or not to use the cached data directories. The cached data directories
   contain a 200-block pre-mined blockchain and wallets for four nodes. Each node
   has 25 mature blocks (25x50=1250 BTC) in its wallet.

test/functional/abandonconflict.py

@@ -14,10 +14,8 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
 class AbandonConflictTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.num_nodes = 2
-        self.setup_clean_chain = False
         self.extra_args = [["-minrelaytxfee=0.00001"], []]
 
     def run_test(self):
@@ -74,7 +72,7 @@ class AbandonConflictTest(BitcoinTestFramework):
         # Restart the node with a higher min relay fee so the parent tx is no longer in mempool
         # TODO: redo with eviction
         self.stop_node(0)
-        self.nodes[0] = self.start_node(0, self.options.tmpdir, ["-minrelaytxfee=0.0001"])
+        self.start_node(0, extra_args=["-minrelaytxfee=0.0001"])
 
         # Verify txs no longer in either node's mempool
         assert_equal(len(self.nodes[0].getrawmempool()), 0)
@@ -101,7 +99,7 @@ class AbandonConflictTest(BitcoinTestFramework):
 
         # Verify that even with a low min relay fee, the tx is not reaccepted from wallet on startup once abandoned
         self.stop_node(0)
-        self.nodes[0] = self.start_node(0, self.options.tmpdir, ["-minrelaytxfee=0.00001"])
+        self.start_node(0, extra_args=["-minrelaytxfee=0.00001"])
         assert_equal(len(self.nodes[0].getrawmempool()), 0)
         assert_equal(self.nodes[0].getbalance(), balance)
 
@@ -121,7 +119,7 @@ class AbandonConflictTest(BitcoinTestFramework):
 
         # Remove using high relay fee again
         self.stop_node(0)
-        self.nodes[0] = self.start_node(0, self.options.tmpdir, ["-minrelaytxfee=0.0001"])
+        self.start_node(0, extra_args=["-minrelaytxfee=0.0001"])
         assert_equal(len(self.nodes[0].getrawmempool()), 0)
         newbalance = self.nodes[0].getbalance()
         assert_equal(newbalance, balance - Decimal("24.9996"))

test/functional/assumevalid.py

@@ -54,20 +54,22 @@ class BaseNode(NodeConnCB):
         self.send_message(headers_message)
 
 class AssumeValidTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 3
 
     def setup_network(self):
+        self.add_nodes(3)
         # Start node0. We don't start the other nodes yet since
         # we need to pre-mine a block with an invalid transaction
         # signature so we can pass in the block hash as assumevalid.
-        self.nodes = [self.start_node(0, self.options.tmpdir)]
+        self.start_node(0)
 
     def send_blocks_until_disconnected(self, node):
         """Keep sending blocks to the node until we're disconnected."""
         for i in range(len(self.blocks)):
+            if not node.connection:
+                break
             try:
                 node.send_message(msg_block(self.blocks[i]))
             except IOError as e:
@@ -162,15 +164,13 @@ class AssumeValidTest(BitcoinTestFramework):
             height += 1
 
         # Start node1 and node2 with assumevalid so they accept a block with a bad signature.
-        self.nodes.append(self.start_node(1, self.options.tmpdir,
+        self.start_node(1, extra_args=["-assumevalid=" + hex(block102.sha256)])
-                                     ["-assumevalid=" + hex(block102.sha256)]))
         node1 = BaseNode()  # connects to node1
         connections.append(NodeConn('127.0.0.1', p2p_port(1), self.nodes[1], node1))
         node1.add_connection(connections[1])
         node1.wait_for_verack()
 
-        self.nodes.append(self.start_node(2, self.options.tmpdir,
+        self.start_node(2, extra_args=["-assumevalid=" + hex(block102.sha256)])
-                                     ["-assumevalid=" + hex(block102.sha256)]))
         node2 = BaseNode()  # connects to node2
         connections.append(NodeConn('127.0.0.1', p2p_port(2), self.nodes[2], node2))
         node2.add_connection(connections[2])

test/functional/bip65-cltv-p2p.py

@@ -60,9 +60,7 @@ def create_transaction(node, coinbase, to_address, amount):
     return tx
 
 class BIP65Test(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.num_nodes = 1
         self.extra_args = [['-promiscuousmempoolflags=1', '-whitelist=127.0.0.1']]
         self.setup_clean_chain = True
@@ -109,7 +107,7 @@ class BIP65Test(BitcoinTestFramework):
         node0.send_and_ping(msg_block(block))
         assert_equal(int(self.nodes[0].getbestblockhash(), 16), tip)
 
-        assert wait_until(lambda: "reject" in node0.last_message.keys())
+        wait_until(lambda: "reject" in node0.last_message.keys(), lock=mininode_lock)
         with mininode_lock:
             assert_equal(node0.last_message["reject"].code, REJECT_OBSOLETE)
             assert_equal(node0.last_message["reject"].reason, b'bad-version(0x00000003)')
@@ -138,7 +136,7 @@ class BIP65Test(BitcoinTestFramework):
         node0.send_and_ping(msg_block(block))
         assert_equal(int(self.nodes[0].getbestblockhash(), 16), tip)
 
-        assert wait_until (lambda: "reject" in node0.last_message.keys())
+        wait_until(lambda: "reject" in node0.last_message.keys(), lock=mininode_lock)
         with mininode_lock:
             assert node0.last_message["reject"].code in [REJECT_INVALID, REJECT_NONSTANDARD]
             assert_equal(node0.last_message["reject"].data, block.sha256)

test/functional/bip68-112-113-p2p.py

@@ -92,9 +92,9 @@ def all_rlt_txs(txarray):
     return txs
 
 class BIP68_112_113Test(ComparisonTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.num_nodes = 1
+        self.setup_clean_chain = True
         self.extra_args = [['-whitelist=127.0.0.1', '-blockversion=4']]
 
     def run_test(self):

test/functional/bip68-sequence.py

@@ -17,10 +17,8 @@ SEQUENCE_LOCKTIME_MASK = 0x0000ffff
 NOT_FINAL_ERROR = "64: non-BIP68-final"
 
 class BIP68Test(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.num_nodes = 2
-        self.setup_clean_chain = False
         self.extra_args = [[], ["-acceptnonstdtxn=0"]]
 
     def run_test(self):
@@ -371,11 +369,14 @@ class BIP68Test(BitcoinTestFramework):
 
     def activateCSV(self):
         # activation should happen at block height 432 (3 periods)
+        # getblockchaininfo will show CSV as active at block 431 (144 * 3 -1) since it's returning whether CSV is active for the next block.
         min_activation_height = 432
         height = self.nodes[0].getblockcount()
-        assert(height < min_activation_height)
+        assert_greater_than(min_activation_height - height, 2)
-        self.nodes[0].generate(min_activation_height-height)
+        self.nodes[0].generate(min_activation_height - height - 2)
-        assert(get_bip9_status(self.nodes[0], 'csv')['status'] == 'active')
+        assert_equal(get_bip9_status(self.nodes[0], 'csv')['status'], "locked_in")
+        self.nodes[0].generate(1)
+        assert_equal(get_bip9_status(self.nodes[0], 'csv')['status'], "active")
         sync_blocks(self.nodes)
 
     # Use self.nodes[1] to test that version 2 transactions are standard.

test/functional/bip9-softforks.py

@@ -28,11 +28,10 @@ from test_framework.comptool import TestInstance, TestManager
 from test_framework.script import CScript, OP_1NEGATE, OP_CHECKSEQUENCEVERIFY, OP_DROP
 
 class BIP9SoftForksTest(ComparisonTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.num_nodes = 1
         self.extra_args = [['-whitelist=127.0.0.1']]
+        self.setup_clean_chain = True
 
     def run_test(self):
         self.test = TestManager(self, self.options.tmpdir)
@@ -241,6 +240,7 @@ class BIP9SoftForksTest(ComparisonTestFramework):
         # Restart all
         self.test.clear_all_connections()
         self.stop_nodes()
+        self.nodes = []
         shutil.rmtree(self.options.tmpdir + "/node0")
         self.setup_chain()
         self.setup_network()

test/functional/bipdersig-p2p.py

@@ -48,9 +48,7 @@ def create_transaction(node, coinbase, to_address, amount):
     return tx
 
 class BIP66Test(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.num_nodes = 1
         self.extra_args = [['-promiscuousmempoolflags=1', '-whitelist=127.0.0.1']]
         self.setup_clean_chain = True
@@ -98,7 +96,7 @@ class BIP66Test(BitcoinTestFramework):
         node0.send_and_ping(msg_block(block))
         assert_equal(int(self.nodes[0].getbestblockhash(), 16), tip)
 
-        assert wait_until(lambda: "reject" in node0.last_message.keys())
+        wait_until(lambda: "reject" in node0.last_message.keys(), lock=mininode_lock)
         with mininode_lock:
             assert_equal(node0.last_message["reject"].code, REJECT_OBSOLETE)
             assert_equal(node0.last_message["reject"].reason, b'bad-version(0x00000002)')
@@ -128,7 +126,7 @@ class BIP66Test(BitcoinTestFramework):
         node0.send_and_ping(msg_block(block))
         assert_equal(int(self.nodes[0].getbestblockhash(), 16), tip)
 
-        assert wait_until (lambda: "reject" in node0.last_message.keys())
+        wait_until(lambda: "reject" in node0.last_message.keys(), lock=mininode_lock)
         with mininode_lock:
             # We can receive different reject messages depending on whether
             # bitcoind is running with multiple script check threads. If script

test/functional/bitcoin_cli.py

@@ -0,0 +1,25 @@
+#!/usr/bin/env python3
+# Copyright (c) 2017 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 bitcoin-cli"""
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_equal
+
+class TestBitcoinCli(BitcoinTestFramework):
+
+    def set_test_params(self):
+        self.setup_clean_chain = True
+        self.num_nodes = 1
+
+    def run_test(self):
+        """Main test logic"""
+
+        self.log.info("Compare responses from getinfo RPC and `bitcoin-cli getinfo`")
+        cli_get_info = self.nodes[0].cli.getinfo()
+        rpc_get_info = self.nodes[0].getinfo()
+
+        assert_equal(cli_get_info, rpc_get_info)
+
+if __name__ == '__main__':
+    TestBitcoinCli().main()

test/functional/blockchain.py

@@ -21,7 +21,7 @@ from decimal import Decimal
 import http.client
 import subprocess
 
-from test_framework.test_framework import (BitcoinTestFramework, BITCOIND_PROC_WAIT_TIMEOUT)
+from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
     assert_raises,
@@ -30,12 +30,8 @@ from test_framework.util import (
     assert_is_hash_string,
 )
 
-
 class BlockchainTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
-        self.setup_clean_chain = False
         self.num_nodes = 1
         self.extra_args = [['-stopatheight=207']]
 
@@ -139,14 +135,14 @@ class BlockchainTest(BitcoinTestFramework):
         self.nodes[0].generate(6)
         assert_equal(self.nodes[0].getblockcount(), 206)
         self.log.debug('Node should not stop at this height')
-        assert_raises(subprocess.TimeoutExpired, lambda: self.bitcoind_processes[0].wait(timeout=3))
+        assert_raises(subprocess.TimeoutExpired, lambda: self.nodes[0].process.wait(timeout=3))
         try:
             self.nodes[0].generate(1)
         except (ConnectionError, http.client.BadStatusLine):
             pass  # The node already shut down before response
         self.log.debug('Node should stop at this height...')
-        self.bitcoind_processes[0].wait(timeout=BITCOIND_PROC_WAIT_TIMEOUT)
+        self.nodes[0].wait_until_stopped()
-        self.nodes[0] = self.start_node(0, self.options.tmpdir)
+        self.start_node(0)
         assert_equal(self.nodes[0].getblockcount(), 207)
 
 

test/functional/bumpfee.py

@@ -30,26 +30,21 @@ WALLET_PASSPHRASE_TIMEOUT = 3600
 
 
 class BumpFeeTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.num_nodes = 2
         self.setup_clean_chain = True
-
+        self.extra_args = [["-prematurewitness", "-walletprematurewitness", "-walletrbf={}".format(i)]
-    def setup_network(self, split=False):
-        extra_args = [["-prematurewitness", "-walletprematurewitness", "-walletrbf={}".format(i)]
                            for i in range(self.num_nodes)]
-        self.nodes = self.start_nodes(self.num_nodes, self.options.tmpdir, extra_args)
 
+    def run_test(self):
         # Encrypt wallet for test_locked_wallet_fails test
-        self.nodes[1].encryptwallet(WALLET_PASSPHRASE)
+        self.nodes[1].node_encrypt_wallet(WALLET_PASSPHRASE)
-        self.bitcoind_processes[1].wait()
+        self.start_node(1)
-        self.nodes[1] = self.start_node(1, self.options.tmpdir, extra_args[1])
         self.nodes[1].walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT)
 
         connect_nodes_bi(self.nodes, 0, 1)
         self.sync_all()
 
-    def run_test(self):
         peer_node, rbf_node = self.nodes
         rbf_node_address = rbf_node.getnewaddress()
 

test/functional/create_cache.py

@@ -12,13 +12,10 @@ tests are being run in parallel.
 from test_framework.test_framework import BitcoinTestFramework
 
 class CreateCache(BitcoinTestFramework):
-
-    def __init__(self):
-        super().__init__()
-
     # Test network and test nodes are not required:
+
+    def set_test_params(self):
         self.num_nodes = 0
-        self.nodes = []
 
     def setup_network(self):
         pass

test/functional/dbcrash.py

@@ -43,8 +43,7 @@ except AttributeError:
     pass
 
 class ChainstateWriteCrashTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.num_nodes = 4
         self.setup_clean_chain = False
 
@@ -64,7 +63,9 @@ class ChainstateWriteCrashTest(BitcoinTestFramework):
         self.extra_args = [self.node0_args, self.node1_args, self.node2_args, self.node3_args]
 
     def setup_network(self):
-        self.setup_nodes()
+        # Need a bit of extra time for the nodes to start up for this test
+        self.add_nodes(self.num_nodes, extra_args=self.extra_args, timewait=90)
+        self.start_nodes()
         # Leave them unconnected, we'll use submitblock directly in this test
 
     def restart_node(self, node_index, expected_tip):
@@ -74,10 +75,10 @@ class ChainstateWriteCrashTest(BitcoinTestFramework):
         after 60 seconds. Returns the utxo hash of the given node."""
 
         time_start = time.time()
-        while time.time() - time_start < 60:
+        while time.time() - time_start < 120:
             try:
                 # Any of these RPC calls could throw due to node crash
-                self.nodes[node_index] = self.start_node(node_index, self.options.tmpdir, self.extra_args[node_index])
+                self.start_node(node_index)
                 self.nodes[node_index].waitforblock(expected_tip)
                 utxo_hash = self.nodes[node_index].gettxoutsetinfo()['hash_serialized_2']
                 return utxo_hash

test/functional/decodescript.py

@@ -10,9 +10,7 @@ from test_framework.mininode import *
 from io import BytesIO
 
 class DecodeScriptTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 1
 

test/functional/disablewallet.py

@@ -11,11 +11,8 @@
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
-
 class DisableWalletTest (BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 1
         self.extra_args = [["-disablewallet"]]

test/functional/disconnect_ban.py

@@ -5,18 +5,17 @@
 """Test node disconnect and ban behavior"""
 import time
 
-from test_framework.mininode import wait_until
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import (assert_equal,
+from test_framework.util import (
+    assert_equal,
     assert_raises_jsonrpc,
-                                 connect_nodes_bi)
+    connect_nodes_bi,
+    wait_until,
+)
 
 class DisconnectBanTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.num_nodes = 2
-        self.setup_clean_chain = False
 
     def run_test(self):
         self.log.info("Test setban and listbanned RPCs")
@@ -24,7 +23,7 @@ class DisconnectBanTest(BitcoinTestFramework):
         self.log.info("setban: successfully ban single IP address")
         assert_equal(len(self.nodes[1].getpeerinfo()), 2)  # node1 should have 2 connections to node0 at this point
         self.nodes[1].setban("127.0.0.1", "add")
-        assert wait_until(lambda: len(self.nodes[1].getpeerinfo()) == 0, timeout=10)
+        wait_until(lambda: len(self.nodes[1].getpeerinfo()) == 0, timeout=10)
         assert_equal(len(self.nodes[1].getpeerinfo()), 0)  # all nodes must be disconnected at this point
         assert_equal(len(self.nodes[1].listbanned()), 1)
 
@@ -66,8 +65,8 @@ class DisconnectBanTest(BitcoinTestFramework):
         assert_equal(len(self.nodes[1].listbanned()), 3)
 
         self.stop_node(1)
+        self.start_node(1)
 
-        self.nodes[1] = self.start_node(1, self.options.tmpdir)
         listAfterShutdown = self.nodes[1].listbanned()
         assert_equal("127.0.0.0/24", listAfterShutdown[0]['address'])
         assert_equal("127.0.0.0/32", listAfterShutdown[1]['address'])
@@ -90,7 +89,7 @@ class DisconnectBanTest(BitcoinTestFramework):
         self.log.info("disconnectnode: successfully disconnect node by address")
         address1 = self.nodes[0].getpeerinfo()[0]['addr']
         self.nodes[0].disconnectnode(address=address1)
-        assert wait_until(lambda: len(self.nodes[0].getpeerinfo()) == 1, timeout=10)
+        wait_until(lambda: len(self.nodes[0].getpeerinfo()) == 1, timeout=10)
         assert not [node for node in self.nodes[0].getpeerinfo() if node['addr'] == address1]
 
         self.log.info("disconnectnode: successfully reconnect node")
@@ -101,7 +100,7 @@ class DisconnectBanTest(BitcoinTestFramework):
         self.log.info("disconnectnode: successfully disconnect node by node id")
         id1 = self.nodes[0].getpeerinfo()[0]['id']
         self.nodes[0].disconnectnode(nodeid=id1)
-        assert wait_until(lambda: len(self.nodes[0].getpeerinfo()) == 1, timeout=10)
+        wait_until(lambda: len(self.nodes[0].getpeerinfo()) == 1, timeout=10)
         assert not [node for node in self.nodes[0].getpeerinfo() if node['id'] == id1]
 
 if __name__ == '__main__':

test/functional/example_test.py

@@ -23,13 +23,13 @@ from test_framework.mininode import (
     mininode_lock,
     msg_block,
     msg_getdata,
-    wait_until,
 )
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
     connect_nodes,
     p2p_port,
+    wait_until,
 )
 
 # NodeConnCB is a class containing callbacks to be executed when a P2P
@@ -73,21 +73,19 @@ def custom_function():
 class ExampleTest(BitcoinTestFramework):
     # Each functional test is a subclass of the BitcoinTestFramework class.
 
-    # Override the __init__(), add_options(), setup_chain(), setup_network()
+    # Override the set_test_params(), add_options(), setup_chain(), setup_network()
     # and setup_nodes() methods to customize the test setup as required.
 
-    def __init__(self):
+    def set_test_params(self):
-        """Initialize the test
+        """Override test parameters for your individual test.
 
-        Call super().__init__() first, and then override any test parameters
+        This method must be overridden and num_nodes must be exlicitly set."""
-        for your individual test."""
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 3
         # Use self.extra_args to change command-line arguments for the nodes
         self.extra_args = [[], ["-logips"], []]
 
-        # self.log.info("I've finished __init__")  # Oops! Can't run self.log before run_test()
+        # self.log.info("I've finished set_test_params")  # Oops! Can't run self.log before run_test()
 
     # Use add_options() to add specific command-line options for your test.
     # In practice this is not used very much, since the tests are mostly written
@@ -209,7 +207,7 @@ class ExampleTest(BitcoinTestFramework):
 
         # wait_until() will loop until a predicate condition is met. Use it to test properties of the
         # NodeConnCB objects.
-        assert wait_until(lambda: sorted(blocks) == sorted(list(node2.block_receive_map.keys())), timeout=5)
+        wait_until(lambda: sorted(blocks) == sorted(list(node2.block_receive_map.keys())), timeout=5, lock=mininode_lock)
 
         self.log.info("Check that each block was received only once")
         # The network thread uses a global lock on data access to the NodeConn objects when sending and receiving

test/functional/forknotify.py

@@ -7,28 +7,18 @@ import os
 import time
 
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import *
 
 class ForkNotifyTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.num_nodes = 2
-        self.setup_clean_chain = False
 
     def setup_network(self):
-        self.nodes = []
         self.alert_filename = os.path.join(self.options.tmpdir, "alert.txt")
         with open(self.alert_filename, 'w', encoding='utf8'):
             pass  # Just open then close to create zero-length file
-        self.nodes.append(self.start_node(0, self.options.tmpdir,
+        self.extra_args = [["-blockversion=2", "-alertnotify=echo %s >> \"" + self.alert_filename + "\""],
-                            ["-blockversion=2", "-alertnotify=echo %s >> \"" + self.alert_filename + "\""]))
+                           ["-blockversion=211"]]
-        # Node1 mines block.version=211 blocks
+        super().setup_network()
-        self.nodes.append(self.start_node(1, self.options.tmpdir,
-                                ["-blockversion=211"]))
-        connect_nodes(self.nodes[1], 0)
-
-        self.sync_all()
 
     def run_test(self):
         # Mine 51 up-version blocks

test/functional/fundrawtransaction.py

@@ -4,7 +4,7 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test the fundrawtransaction RPC."""
 
-from test_framework.test_framework import BitcoinTestFramework, BITCOIND_PROC_WAIT_TIMEOUT
+from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
 
@@ -14,13 +14,10 @@ def get_unspent(listunspent, amount):
             return utx
     raise AssertionError('Could not find unspent with amount={}'.format(amount))
 
-
 class RawTransactionsTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
-        self.setup_clean_chain = True
         self.num_nodes = 4
+        self.setup_clean_chain = True
 
     def setup_network(self, split=False):
         self.setup_nodes()
@@ -449,12 +446,11 @@ class RawTransactionsTest(BitcoinTestFramework):
         ############################################################
         # locked wallet test
         self.stop_node(0)
+        self.nodes[1].node_encrypt_wallet("test")
         self.stop_node(2)
         self.stop_node(3)
-        self.nodes[1].encryptwallet("test")
-        self.bitcoind_processes[1].wait(timeout=BITCOIND_PROC_WAIT_TIMEOUT)
 
-        self.nodes = self.start_nodes(self.num_nodes, self.options.tmpdir)
+        self.start_nodes()
         # This test is not meant to test fee estimation and we'd like
         # to be sure all txs are sent at a consistent desired feerate
         for node in self.nodes:

test/functional/getblocktemplate_longpoll.py

@@ -17,16 +17,14 @@ class LongpollThread(threading.Thread):
         self.longpollid = templat['longpollid']
         # create a new connection to the node, we can't use the same
         # connection from two threads
-        self.node = get_rpc_proxy(node.url, 1, timeout=600)
+        self.node = get_rpc_proxy(node.url, 1, timeout=600, coveragedir=node.coverage_dir)
 
     def run(self):
         self.node.getblocktemplate({'longpollid':self.longpollid})
 
 class GetBlockTemplateLPTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
+        self.num_nodes = 2
-        self.num_nodes = 4
-        self.setup_clean_chain = False
 
     def run_test(self):
         self.log.info("Warning: this test will take about 70 seconds in the best case. Be patient.")

test/functional/getchaintips.py

@@ -14,13 +14,10 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal
 
 class GetChainTipsTest (BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.num_nodes = 4
-        self.setup_clean_chain = False
 
     def run_test (self):
-
         tips = self.nodes[0].getchaintips ()
         assert_equal (len (tips), 1)
         assert_equal (tips[0]['branchlen'], 0)

test/functional/httpbasics.py

@@ -11,10 +11,8 @@ import http.client
 import urllib.parse
 
 class HTTPBasicsTest (BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.num_nodes = 3
-        self.setup_clean_chain = False
 
     def setup_network(self):
         self.setup_nodes()

test/functional/import-rescan.py

@@ -111,8 +111,7 @@ TIMESTAMP_WINDOW = 2 * 60 * 60
 
 
 class ImportRescanTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.num_nodes = 2 + len(IMPORT_NODES)
 
     def setup_network(self):
@@ -121,7 +120,8 @@ class ImportRescanTest(BitcoinTestFramework):
             if import_node.prune:
                 extra_args[i] += ["-prune=1"]
 
-        self.nodes = self.start_nodes(self.num_nodes, self.options.tmpdir, extra_args)
+        self.add_nodes(self.num_nodes, extra_args)
+        self.start_nodes()
         for i in range(1, self.num_nodes):
             connect_nodes(self.nodes[i], 0)
 

test/functional/importmulti.py

@@ -7,8 +7,7 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
 class ImportMultiTest (BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.num_nodes = 2
         self.setup_clean_chain = True
 
@@ -429,7 +428,7 @@ class ImportMultiTest (BitcoinTestFramework):
 
         # restart nodes to check for proper serialization/deserialization of watch only address
         self.stop_nodes()
-        self.nodes = self.start_nodes(2, self.options.tmpdir)
+        self.start_nodes()
         address_assert = self.nodes[1].validateaddress(watchonly_address)
         assert_equal(address_assert['iswatchonly'], True)
         assert_equal(address_assert['ismine'], False)

test/functional/importprunedfunds.py

@@ -6,11 +6,8 @@
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
-
 class ImportPrunedFundsTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 2
 

test/functional/invalidateblock.py

@@ -8,9 +8,7 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
 class InvalidateTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 3
 

test/functional/invalidblockrequest.py

@@ -23,9 +23,9 @@ class InvalidBlockRequestTest(ComparisonTestFramework):
 
     ''' Can either run this test as 1 node with expected answers, or two and compare them. 
         Change the "outcome" variable from each TestInstance object to only do the comparison. '''
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.num_nodes = 1
+        self.setup_clean_chain = True
 
     def run_test(self):
         test = TestManager(self, self.options.tmpdir)

test/functional/invalidtxrequest.py

@@ -19,9 +19,9 @@ class InvalidTxRequestTest(ComparisonTestFramework):
 
     ''' Can either run this test as 1 node with expected answers, or two and compare them. 
         Change the "outcome" variable from each TestInstance object to only do the comparison. '''
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.num_nodes = 1
+        self.setup_clean_chain = True
 
     def run_test(self):
         test = TestManager(self, self.options.tmpdir)

test/functional/keypool-topup.py

@@ -20,8 +20,7 @@ from test_framework.util import (
 )
 
 class KeypoolRestoreTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 2
         self.extra_args = [['-usehd=0'], ['-usehd=1', '-keypool=100', '-keypoolmin=20']]
@@ -35,7 +34,7 @@ class KeypoolRestoreTest(BitcoinTestFramework):
         self.stop_node(1)
 
         shutil.copyfile(self.tmpdir + "/node1/regtest/wallet.dat", self.tmpdir + "/wallet.bak")
-        self.nodes[1] = self.start_node(1, self.tmpdir, self.extra_args[1])
+        self.start_node(1, self.extra_args[1])
         connect_nodes_bi(self.nodes, 0, 1)
 
         self.log.info("Generate keys for wallet")
@@ -61,7 +60,7 @@ class KeypoolRestoreTest(BitcoinTestFramework):
 
         self.log.info("Verify keypool is restored and balance is correct")
 
-        self.nodes[1] = self.start_node(1, self.tmpdir, self.extra_args[1])
+        self.start_node(1, self.extra_args[1])
         connect_nodes_bi(self.nodes, 0, 1)
         self.sync_all()
 

test/functional/keypool.py

@@ -8,6 +8,8 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
 class KeyPoolTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 1
 
     def run_test(self):
         nodes = self.nodes
@@ -17,10 +19,9 @@ class KeyPoolTest(BitcoinTestFramework):
         assert(addr_before_encrypting_data['hdmasterkeyid'] == wallet_info_old['hdmasterkeyid'])
         
         # Encrypt wallet and wait to terminate
-        nodes[0].encryptwallet('test')
+        nodes[0].node_encrypt_wallet('test')
-        self.bitcoind_processes[0].wait()
         # Restart node 0
-        nodes[0] = self.start_node(0, self.options.tmpdir)
+        self.start_node(0)
         # Keep creating keys
         addr = nodes[0].getnewaddress()
         addr_data = nodes[0].validateaddress(addr)
@@ -79,10 +80,5 @@ class KeyPoolTest(BitcoinTestFramework):
         assert_equal(wi['keypoolsize_hd_internal'], 100)
         assert_equal(wi['keypoolsize'], 100)
 
-    def __init__(self):
-        super().__init__()
-        self.setup_clean_chain = False
-        self.num_nodes = 1
-
 if __name__ == '__main__':
     KeyPoolTest().main()

test/functional/listsinceblock.py

@@ -8,11 +8,9 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal
 
 class ListSinceBlockTest (BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
-        self.setup_clean_chain = True
         self.num_nodes = 4
+        self.setup_clean_chain = True
 
     def run_test(self):
         self.nodes[2].generate(101)

test/functional/listtransactions.py

@@ -16,15 +16,9 @@ def txFromHex(hexstring):
     return tx
 
 class ListTransactionsTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
+        self.num_nodes = 2
-        self.num_nodes = 4
-        self.setup_clean_chain = False
-
-    def setup_nodes(self):
-        #This test requires mocktime
         self.enable_mocktime()
-        self.nodes = self.start_nodes(self.num_nodes, self.options.tmpdir)
 
     def run_test(self):
         # Simple send, 0 to 1:

test/functional/maxuploadtarget.py

@@ -31,8 +31,7 @@ class TestNode(NodeConnCB):
 
 class MaxUploadTest(BitcoinTestFramework):
  
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 1
         self.extra_args = [["-maxuploadtarget=800", "-blockmaxsize=999000"]]
@@ -147,7 +146,7 @@ class MaxUploadTest(BitcoinTestFramework):
         #stop and start node 0 with 1MB maxuploadtarget, whitelist 127.0.0.1
         self.log.info("Restarting nodes with -whitelist=127.0.0.1")
         self.stop_node(0)
-        self.nodes[0] = self.start_node(0, self.options.tmpdir, ["-whitelist=127.0.0.1", "-maxuploadtarget=1", "-blockmaxsize=999000"])
+        self.start_node(0, ["-whitelist=127.0.0.1", "-maxuploadtarget=1", "-blockmaxsize=999000"])
 
         #recreate/reconnect a test node
         test_nodes = [TestNode()]

test/functional/mempool_limit.py

@@ -8,9 +8,7 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
 class MempoolLimitTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 1
         self.extra_args = [["-maxmempool=5", "-spendzeroconfchange=0"]]

test/functional/mempool_packages.py

@@ -12,10 +12,8 @@ MAX_ANCESTORS = 25
 MAX_DESCENDANTS = 25
 
 class MempoolPackagesTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.num_nodes = 2
-        self.setup_clean_chain = False
         self.extra_args = [["-maxorphantx=1000"], ["-maxorphantx=1000", "-limitancestorcount=5"]]
 
     # Build a transaction that spends parent_txid:vout

test/functional/mempool_persist.py

@@ -32,17 +32,12 @@ Test is as follows:
 """
 import time
 
-from test_framework.mininode import wait_until
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
 class MempoolPersistTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
-        # We need 3 nodes for this test. Node1 does not have a persistent mempool.
         self.num_nodes = 3
-        self.setup_clean_chain = False
         self.extra_args = [[], ["-persistmempool=0"], []]
 
     def run_test(self):
@@ -64,27 +59,24 @@ class MempoolPersistTest(BitcoinTestFramework):
 
         self.log.debug("Stop-start node0 and node1. Verify that node0 has the transactions in its mempool and node1 does not.")
         self.stop_nodes()
-        self.nodes = []
+        self.start_node(0)
-        self.nodes.append(self.start_node(0, self.options.tmpdir))
+        self.start_node(1)
-        self.nodes.append(self.start_node(1, self.options.tmpdir))
         # Give bitcoind a second to reload the mempool
         time.sleep(1)
-        assert wait_until(lambda: len(self.nodes[0].getrawmempool()) == 5)
+        wait_until(lambda: len(self.nodes[0].getrawmempool()) == 5)
         assert_equal(len(self.nodes[1].getrawmempool()), 0)
 
         self.log.debug("Stop-start node0 with -persistmempool=0. Verify that it doesn't load its mempool.dat file.")
         self.stop_nodes()
-        self.nodes = []
+        self.start_node(0, extra_args=["-persistmempool=0"])
-        self.nodes.append(self.start_node(0, self.options.tmpdir, ["-persistmempool=0"]))
         # Give bitcoind a second to reload the mempool
         time.sleep(1)
         assert_equal(len(self.nodes[0].getrawmempool()), 0)
 
         self.log.debug("Stop-start node0. Verify that it has the transactions in its mempool.")
         self.stop_nodes()
-        self.nodes = []
+        self.start_node(0)
-        self.nodes.append(self.start_node(0, self.options.tmpdir))
+        wait_until(lambda: len(self.nodes[0].getrawmempool()) == 5)
-        assert wait_until(lambda: len(self.nodes[0].getrawmempool()) == 5)
 
 if __name__ == '__main__':
     MempoolPersistTest().main()

test/functional/mempool_reorg.py

@@ -13,10 +13,8 @@ from test_framework.util import *
 
 # Create one-input, one-output, no-fee transaction:
 class MempoolCoinbaseTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.num_nodes = 2
-        self.setup_clean_chain = False
         self.extra_args = [["-checkmempool"]] * 2
 
     alert_filename = None  # Set by setup_network

test/functional/mempool_resurrect_test.py

@@ -9,12 +9,8 @@ from test_framework.util import *
 
 # Create one-input, one-output, no-fee transaction:
 class MempoolCoinbaseTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.num_nodes = 1
-        self.setup_clean_chain = False
-        # Just need one node for this test
         self.extra_args = [["-checkmempool"]]
 
     def run_test(self):

test/functional/mempool_spendcoinbase.py

@@ -17,11 +17,8 @@ from test_framework.util import *
 
 # Create one-input, one-output, no-fee transaction:
 class MempoolSpendCoinbaseTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.num_nodes = 1
-        self.setup_clean_chain = False
         self.extra_args = [["-checkmempool"]]
 
     def run_test(self):

test/functional/merkle_blocks.py

@@ -8,11 +8,9 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
 class MerkleBlockTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
-        self.setup_clean_chain = True
         self.num_nodes = 4
+        self.setup_clean_chain = True
         # Nodes 0/1 are "wallet" nodes, Nodes 2/3 are used for testing
         self.extra_args = [[], [], [], ["-txindex"]]
 

test/functional/mining.py

@@ -4,16 +4,18 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test mining RPCs
 
+- getmininginfo
 - getblocktemplate proposal mode
 - submitblock"""
 
-from binascii import b2a_hex
 import copy
+from binascii import b2a_hex
+from decimal import Decimal
 
 from test_framework.blocktools import create_coinbase
-from test_framework.test_framework import BitcoinTestFramework
 from test_framework.mininode import CBlock
-from test_framework.util import *
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_equal, assert_raises_jsonrpc
 
 def b2x(b):
     return b2a_hex(b).decode('ascii')
@@ -25,14 +27,24 @@ def assert_template(node, block, expect, rehash=True):
     assert_equal(rsp, expect)
 
 class MiningTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.num_nodes = 2
         self.setup_clean_chain = False
 
     def run_test(self):
         node = self.nodes[0]
+
+        self.log.info('getmininginfo')
+        mining_info = node.getmininginfo()
+        assert_equal(mining_info['blocks'], 200)
+        assert_equal(mining_info['chain'], 'regtest')
+        assert_equal(mining_info['currentblocksize'], 0)
+        assert_equal(mining_info['currentblocktx'], 0)
+        assert_equal(mining_info['currentblockweight'], 0)
+        assert_equal(mining_info['difficulty'], Decimal('4.656542373906925E-10'))
+        assert_equal(mining_info['networkhashps'], Decimal('0.003333333333333334'))
+        assert_equal(mining_info['pooledtx'], 0)
+
         # Mine a block to leave initial block download
         node.generate(1)
         tmpl = node.getblocktemplate()

test/functional/multi_rpc.py

@@ -12,10 +12,7 @@ import http.client
 import urllib.parse
 
 class HTTPBasicsTest (BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
-        self.setup_clean_chain = False
         self.num_nodes = 2
 
     def setup_chain(self):

test/functional/multiwallet.py

@@ -12,34 +12,38 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal, assert_raises_jsonrpc
 
 class MultiWalletTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 1
         self.extra_args = [['-wallet=w1', '-wallet=w2', '-wallet=w3']]
 
     def run_test(self):
+        assert_equal(set(self.nodes[0].listwallets()), {"w1", "w2", "w3"})
+
         self.stop_node(0)
 
         # should not initialize if there are duplicate wallets
-        self.assert_start_raises_init_error(0, self.options.tmpdir, ['-wallet=w1', '-wallet=w1'], 'Error loading wallet w1. Duplicate -wallet filename specified.')
+        self.assert_start_raises_init_error(0, ['-wallet=w1', '-wallet=w1'], 'Error loading wallet w1. Duplicate -wallet filename specified.')
 
         # should not initialize if wallet file is a directory
         os.mkdir(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w11'))
-        self.assert_start_raises_init_error(0, self.options.tmpdir, ['-wallet=w11'], 'Error loading wallet w11. -wallet filename must be a regular file.')
+        self.assert_start_raises_init_error(0, ['-wallet=w11'], 'Error loading wallet w11. -wallet filename must be a regular file.')
 
         # should not initialize if wallet file is a symlink
         os.symlink(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w1'), os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w12'))
-        self.assert_start_raises_init_error(0, self.options.tmpdir, ['-wallet=w12'], 'Error loading wallet w12. -wallet filename must be a regular file.')
+        self.assert_start_raises_init_error(0, ['-wallet=w12'], 'Error loading wallet w12. -wallet filename must be a regular file.')
 
-        self.nodes[0] = self.start_node(0, self.options.tmpdir, self.extra_args[0])
+        self.start_node(0, self.extra_args[0])
+
+        w1 = self.nodes[0].get_wallet_rpc("w1")
+        w2 = self.nodes[0].get_wallet_rpc("w2")
+        w3 = self.nodes[0].get_wallet_rpc("w3")
+        wallet_bad = self.nodes[0].get_wallet_rpc("bad")
 
-        w1 = self.nodes[0] / "wallet/w1"
         w1.generate(1)
 
         # accessing invalid wallet fails
-        assert_raises_jsonrpc(-18, "Requested wallet does not exist or is not loaded", (self.nodes[0] / "wallet/bad").getwalletinfo)
+        assert_raises_jsonrpc(-18, "Requested wallet does not exist or is not loaded", wallet_bad.getwalletinfo)
 
         # accessing wallet RPC without using wallet endpoint fails
         assert_raises_jsonrpc(-19, "Wallet file not specified", self.nodes[0].getwalletinfo)
@@ -50,14 +54,12 @@ class MultiWalletTest(BitcoinTestFramework):
         w1_name = w1_info['walletname']
         assert_equal(w1_name, "w1")
 
-        # check w1 wallet balance
+        # check w2 wallet balance
-        w2 = self.nodes[0] / "wallet/w2"
         w2_info = w2.getwalletinfo()
         assert_equal(w2_info['immature_balance'], 0)
         w2_name = w2_info['walletname']
         assert_equal(w2_name, "w2")
 
-        w3 = self.nodes[0] / "wallet/w3"
         w3_name = w3.getwalletinfo()['walletname']
         assert_equal(w3_name, "w3")
 

test/functional/net.py

@@ -17,10 +17,8 @@ from test_framework.util import (
     p2p_port,
 )
 
-
 class NetTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 2
 

test/functional/nulldummy.py

@@ -37,8 +37,7 @@ def trueDummy(tx):
 
 class NULLDUMMYTest(BitcoinTestFramework):
 
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.num_nodes = 1
         self.setup_clean_chain = True
         self.extra_args = [['-whitelist=127.0.0.1', '-walletprematurewitness']]

test/functional/p2p-acceptblock.py

@@ -60,8 +60,7 @@ class AcceptBlockTest(BitcoinTestFramework):
                           default=os.getenv("BITCOIND", "bitcoind"),
                           help="bitcoind binary to test")
 
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 2
         self.extra_args = [[], ["-whitelist=127.0.0.1"]]

test/functional/p2p-compactblocks.py

@@ -70,7 +70,7 @@ class TestNode(NodeConnCB):
     def request_headers_and_sync(self, locator, hashstop=0):
         self.clear_block_announcement()
         self.get_headers(locator, hashstop)
-        assert wait_until(self.received_block_announcement, timeout=30)
+        wait_until(self.received_block_announcement, timeout=30, lock=mininode_lock)
         self.clear_block_announcement()
 
     # Block until a block announcement for a particular block hash is
@@ -78,7 +78,7 @@ class TestNode(NodeConnCB):
     def wait_for_block_announcement(self, block_hash, timeout=30):
         def received_hash():
             return (block_hash in self.announced_blockhashes)
-        return wait_until(received_hash, timeout=timeout)
+        wait_until(received_hash, timeout=timeout, lock=mininode_lock)
 
     def send_await_disconnect(self, message, timeout=30):
         """Sends a message to the node and wait for disconnect.
@@ -86,15 +86,10 @@ class TestNode(NodeConnCB):
         This is used when we want to send a message into the node that we expect
         will get us disconnected, eg an invalid block."""
         self.send_message(message)
-        success = wait_until(lambda: not self.connected, timeout=timeout)
+        wait_until(lambda: not self.connected, timeout=timeout, lock=mininode_lock)
-        if not success:
-            logger.error("send_await_disconnect failed!")
-            raise AssertionError("send_await_disconnect failed!")
-        return success
 
 class CompactBlocksTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.setup_clean_chain = True
         # Node0 = pre-segwit, node1 = segwit-aware
         self.num_nodes = 2
@@ -150,9 +145,7 @@ class CompactBlocksTest(BitcoinTestFramework):
         # Make sure we get a SENDCMPCT message from our peer
         def received_sendcmpct():
             return (len(test_node.last_sendcmpct) > 0)
-        got_message = wait_until(received_sendcmpct, timeout=30)
+        wait_until(received_sendcmpct, timeout=30, lock=mininode_lock)
-        assert(received_sendcmpct())
-        assert(got_message)
         with mininode_lock:
             # Check that the first version received is the preferred one
             assert_equal(test_node.last_sendcmpct[0].version, preferred_version)
@@ -167,7 +160,6 @@ class CompactBlocksTest(BitcoinTestFramework):
             block_hash = int(node.generate(1)[0], 16)
             peer.wait_for_block_announcement(block_hash, timeout=30)
             assert(peer.block_announced)
-            assert(got_message)
 
             with mininode_lock:
                 assert predicate(peer), (
@@ -282,7 +274,7 @@ class CompactBlocksTest(BitcoinTestFramework):
 
         # Wait until we've seen the block announcement for the resulting tip
         tip = int(node.getbestblockhash(), 16)
-        assert(test_node.wait_for_block_announcement(tip))
+        test_node.wait_for_block_announcement(tip)
 
         # Make sure we will receive a fast-announce compact block
         self.request_cb_announcements(test_node, node, version)
@@ -297,8 +289,7 @@ class CompactBlocksTest(BitcoinTestFramework):
         block.rehash()
 
         # Wait until the block was announced (via compact blocks)
-        wait_until(test_node.received_block_announcement, timeout=30)
+        wait_until(test_node.received_block_announcement, timeout=30, lock=mininode_lock)
-        assert(test_node.received_block_announcement())
 
         # Now fetch and check the compact block
         header_and_shortids = None
@@ -314,8 +305,7 @@ class CompactBlocksTest(BitcoinTestFramework):
             inv = CInv(4, block_hash)  # 4 == "CompactBlock"
             test_node.send_message(msg_getdata([inv]))
 
-        wait_until(test_node.received_block_announcement, timeout=30)
+        wait_until(test_node.received_block_announcement, timeout=30, lock=mininode_lock)
-        assert(test_node.received_block_announcement())
 
         # Now fetch and check the compact block
         header_and_shortids = None
@@ -386,13 +376,11 @@ class CompactBlocksTest(BitcoinTestFramework):
 
             if announce == "inv":
                 test_node.send_message(msg_inv([CInv(2, block.sha256)]))
-                success = wait_until(lambda: "getheaders" in test_node.last_message, timeout=30)
+                wait_until(lambda: "getheaders" in test_node.last_message, timeout=30, lock=mininode_lock)
-                assert(success)
                 test_node.send_header_for_blocks([block])
             else:
                 test_node.send_header_for_blocks([block])
-            success = wait_until(lambda: "getdata" in test_node.last_message, timeout=30)
+            wait_until(lambda: "getdata" in test_node.last_message, timeout=30, lock=mininode_lock)
-            assert(success)
             assert_equal(len(test_node.last_message["getdata"].inv), 1)
             assert_equal(test_node.last_message["getdata"].inv[0].type, 4)
             assert_equal(test_node.last_message["getdata"].inv[0].hash, block.sha256)
@@ -571,8 +559,7 @@ class CompactBlocksTest(BitcoinTestFramework):
         assert_equal(int(node.getbestblockhash(), 16), block.hashPrevBlock)
 
         # We should receive a getdata request
-        success = wait_until(lambda: "getdata" in test_node.last_message, timeout=10)
+        wait_until(lambda: "getdata" in test_node.last_message, timeout=10, lock=mininode_lock)
-        assert(success)
         assert_equal(len(test_node.last_message["getdata"].inv), 1)
         assert(test_node.last_message["getdata"].inv[0].type == 2 or test_node.last_message["getdata"].inv[0].type == 2|MSG_WITNESS_FLAG)
         assert_equal(test_node.last_message["getdata"].inv[0].hash, block.sha256)
@@ -599,8 +586,7 @@ class CompactBlocksTest(BitcoinTestFramework):
             num_to_request = random.randint(1, len(block.vtx))
             msg.block_txn_request.from_absolute(sorted(random.sample(range(len(block.vtx)), num_to_request)))
             test_node.send_message(msg)
-            success = wait_until(lambda: "blocktxn" in test_node.last_message, timeout=10)
+            wait_until(lambda: "blocktxn" in test_node.last_message, timeout=10, lock=mininode_lock)
-            assert(success)
 
             [tx.calc_sha256() for tx in block.vtx]
             with mininode_lock:
@@ -639,22 +625,20 @@ class CompactBlocksTest(BitcoinTestFramework):
         for i in range(MAX_CMPCTBLOCK_DEPTH + 1):
             test_node.clear_block_announcement()
             new_blocks.append(node.generate(1)[0])
-            wait_until(test_node.received_block_announcement, timeout=30)
+            wait_until(test_node.received_block_announcement, timeout=30, lock=mininode_lock)
 
         test_node.clear_block_announcement()
         test_node.send_message(msg_getdata([CInv(4, int(new_blocks[0], 16))]))
-        success = wait_until(lambda: "cmpctblock" in test_node.last_message, timeout=30)
+        wait_until(lambda: "cmpctblock" in test_node.last_message, timeout=30, lock=mininode_lock)
-        assert(success)
 
         test_node.clear_block_announcement()
         node.generate(1)
-        wait_until(test_node.received_block_announcement, timeout=30)
+        wait_until(test_node.received_block_announcement, timeout=30, lock=mininode_lock)
         test_node.clear_block_announcement()
         with mininode_lock:
             test_node.last_message.pop("block", None)
         test_node.send_message(msg_getdata([CInv(4, int(new_blocks[0], 16))]))
-        success = wait_until(lambda: "block" in test_node.last_message, timeout=30)
+        wait_until(lambda: "block" in test_node.last_message, timeout=30, lock=mininode_lock)
-        assert(success)
         with mininode_lock:
             test_node.last_message["block"].block.calc_sha256()
             assert_equal(test_node.last_message["block"].block.sha256, int(new_blocks[0], 16))
@@ -705,7 +689,7 @@ class CompactBlocksTest(BitcoinTestFramework):
         node.submitblock(ToHex(block))
 
         for l in listeners:
-            wait_until(lambda: l.received_block_announcement(), timeout=30)
+            wait_until(lambda: l.received_block_announcement(), timeout=30, lock=mininode_lock)
         with mininode_lock:
             for l in listeners:
                 assert "cmpctblock" in l.last_message

test/functional/p2p-feefilter.py

@@ -37,11 +37,8 @@ class TestNode(NodeConnCB):
             self.txinvs = []
 
 class FeeFilterTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.num_nodes = 2
-        self.setup_clean_chain = False
 
     def run_test(self):
         node1 = self.nodes[1]

test/functional/p2p-fullblocktest.py

@@ -49,12 +49,11 @@ class CBrokenBlock(CBlock):
         return r
 
 class FullBlockTest(ComparisonTestFramework):
-
     # Can either run this test as 1 node with expected answers, or two and compare them.
     # Change the "outcome" variable from each TestInstance object to only do the comparison.
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.num_nodes = 1
+        self.setup_clean_chain = True
         self.block_heights = {}
         self.coinbase_key = CECKey()
         self.coinbase_key.set_secretbytes(b"horsebattery")

test/functional/p2p-leaktests.py

@@ -92,8 +92,7 @@ class CNodeNoVerackIdle(CLazyNode):
         conn.send_message(msg_getaddr())
 
 class P2PLeakTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.num_nodes = 1
         self.extra_args = [['-banscore='+str(banscore)]]
 
@@ -119,11 +118,11 @@ class P2PLeakTest(BitcoinTestFramework):
 
         NetworkThread().start()  # Start up network handling in another thread
 
-        assert wait_until(lambda: no_version_bannode.ever_connected, timeout=10)
+        wait_until(lambda: no_version_bannode.ever_connected, timeout=10, lock=mininode_lock)
-        assert wait_until(lambda: no_version_idlenode.ever_connected, timeout=10)
+        wait_until(lambda: no_version_idlenode.ever_connected, timeout=10, lock=mininode_lock)
-        assert wait_until(lambda: no_verack_idlenode.version_received, timeout=10)
+        wait_until(lambda: no_verack_idlenode.version_received, timeout=10, lock=mininode_lock)
-        assert wait_until(lambda: unsupported_service_bit5_node.ever_connected, timeout=10)
+        wait_until(lambda: unsupported_service_bit5_node.ever_connected, timeout=10, lock=mininode_lock)
-        assert wait_until(lambda: unsupported_service_bit7_node.ever_connected, timeout=10)
+        wait_until(lambda: unsupported_service_bit7_node.ever_connected, timeout=10, lock=mininode_lock)
 
         # Mine a block and make sure that it's not sent to the connected nodes
         self.nodes[0].generate(1)
@@ -140,6 +139,9 @@ class P2PLeakTest(BitcoinTestFramework):
 
         [conn.disconnect_node() for conn in connections]
 
+        # Wait until all connections are closed
+        wait_until(lambda: len(self.nodes[0].getpeerinfo()) == 0)
+
         # Make sure no unexpected messages came in
         assert(no_version_bannode.unexpected_msg == False)
         assert(no_version_idlenode.unexpected_msg == False)
@@ -158,8 +160,10 @@ class P2PLeakTest(BitcoinTestFramework):
         allowed_service_bit5_node.add_connection(connections[5])
         allowed_service_bit7_node.add_connection(connections[6])
 
-        assert wait_until(lambda: allowed_service_bit5_node.message_count["verack"], timeout=10)
+        NetworkThread().start()  # Network thread stopped when all previous NodeConnCBs disconnected. Restart it
-        assert wait_until(lambda: allowed_service_bit7_node.message_count["verack"], timeout=10)
+
+        wait_until(lambda: allowed_service_bit5_node.message_count["verack"], lock=mininode_lock)
+        wait_until(lambda: allowed_service_bit7_node.message_count["verack"], lock=mininode_lock)
 
 if __name__ == '__main__':
     P2PLeakTest().main()

test/functional/p2p-mempool.py

@@ -13,9 +13,7 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
 class P2PMempoolTests(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 1
         self.extra_args = [["-peerbloomfilters=0"]]

test/functional/p2p-segwit.py

@@ -109,9 +109,7 @@ def sign_P2PK_witness_input(script, txTo, inIdx, hashtype, value, key):
 
 
 class SegWitTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 3
         self.extra_args = [["-whitelist=127.0.0.1"], ["-whitelist=127.0.0.1", "-acceptnonstdtxn=0"], ["-whitelist=127.0.0.1", "-vbparams=segwit:0:0"]]
@@ -1496,7 +1494,7 @@ class SegWitTest(BitcoinTestFramework):
 
         # Restart with the new binary
         self.stop_node(node_id)
-        self.nodes[node_id] = self.start_node(node_id, self.options.tmpdir)
+        self.start_node(node_id, extra_args=[])
         connect_nodes(self.nodes[0], node_id)
 
         sync_blocks(self.nodes)

test/functional/p2p-timeouts.py

@@ -33,8 +33,7 @@ class TestNode(NodeConnCB):
         pass
 
 class TimeoutsTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 1
 

test/functional/p2p-versionbits-warning.py

@@ -28,8 +28,7 @@ class TestNode(NodeConnCB):
         pass
 
 class VersionBitsWarningTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 1
 
@@ -112,7 +111,7 @@ class VersionBitsWarningTest(BitcoinTestFramework):
         # Empty out the alert file
         with open(self.alert_filename, 'w', encoding='utf8') as _:
             pass
-        self.nodes = self.start_nodes(self.num_nodes, self.options.tmpdir, self.extra_args)
+        self.start_nodes()
 
         # Connecting one block should be enough to generate an error.
         self.nodes[0].generate(1)
@@ -123,7 +122,7 @@ class VersionBitsWarningTest(BitcoinTestFramework):
         self.test_versionbits_in_alert_file()
 
         # Test framework expects the node to still be running...
-        self.nodes = self.start_nodes(self.num_nodes, self.options.tmpdir, self.extra_args)
+        self.start_nodes()
 
 if __name__ == '__main__':
     VersionBitsWarningTest().main()

test/functional/preciousblock.py

@@ -35,8 +35,7 @@ def node_sync_via_rpc(nodes):
             unidirectional_node_sync_via_rpc(node_src, node_dest)
 
 class PreciousTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 3
 

test/functional/prioritise_transaction.py

@@ -9,9 +9,7 @@ from test_framework.util import *
 from test_framework.mininode import COIN, MAX_BLOCK_BASE_SIZE
 
 class PrioritiseTransactionTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 2
         self.extra_args = [["-printpriority=1"], ["-printpriority=1"]]

test/functional/proxy_test.py

@@ -41,12 +41,9 @@ from test_framework.netutil import test_ipv6_local
 
 RANGE_BEGIN = PORT_MIN + 2 * PORT_RANGE  # Start after p2p and rpc ports
 
-
 class ProxyTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.num_nodes = 4
-        self.setup_clean_chain = False
 
     def setup_nodes(self):
         self.have_ipv6 = test_ipv6_local()
@@ -89,7 +86,8 @@ class ProxyTest(BitcoinTestFramework):
             ]
         if self.have_ipv6:
             args[3] = ['-listen', '-proxy=[%s]:%i' % (self.conf3.addr),'-proxyrandomize=0', '-noonion']
-        self.nodes = self.start_nodes(self.num_nodes, self.options.tmpdir, extra_args=args)
+        self.add_nodes(self.num_nodes, extra_args=args)
+        self.start_nodes()
 
     def node_test(self, node, proxies, auth, test_onion=True):
         rv = []

test/functional/pruning.py

@@ -26,9 +26,7 @@ def calc_usage(blockdir):
     return sum(os.path.getsize(blockdir+f) for f in os.listdir(blockdir) if os.path.isfile(blockdir+f)) / (1024. * 1024.)
 
 class PruneTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 6
 
@@ -56,6 +54,10 @@ class PruneTest(BitcoinTestFramework):
         connect_nodes(self.nodes[0], 4)
         sync_blocks(self.nodes[0:5])
 
+    def setup_nodes(self):
+        self.add_nodes(self.num_nodes, self.extra_args, timewait=900)
+        self.start_nodes()
+
     def create_big_chain(self):
         # Start by creating some coinbases we can spend later
         self.nodes[1].generate(200)
@@ -98,7 +100,7 @@ class PruneTest(BitcoinTestFramework):
             # 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
             self.stop_node(0)
-            self.nodes[0]=self.start_node(0, self.options.tmpdir, self.full_node_default_args, timewait=900)
+            self.start_node(0, extra_args=self.full_node_default_args)
             # Mine 24 blocks in node 1
             for i in range(24):
                 if j == 0:
@@ -126,7 +128,7 @@ class PruneTest(BitcoinTestFramework):
         # 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.nodes[1] = self.start_node(1, self.options.tmpdir, ["-maxreceivebuffer=20000","-blockmaxsize=5000", "-checkblocks=5", "-disablesafemode"], timewait=900)
+        self.start_node(1, extra_args=["-maxreceivebuffer=20000","-blockmaxsize=5000", "-checkblocks=5", "-disablesafemode"])
 
         height = self.nodes[1].getblockcount()
         self.log.info("Current block height: %d" % height)
@@ -149,7 +151,7 @@ class PruneTest(BitcoinTestFramework):
 
         # Reboot node1 to clear those giant tx's from mempool
         self.stop_node(1)
-        self.nodes[1] = self.start_node(1, self.options.tmpdir, ["-maxreceivebuffer=20000","-blockmaxsize=5000", "-checkblocks=5", "-disablesafemode"], timewait=900)
+        self.start_node(1, extra_args=["-maxreceivebuffer=20000","-blockmaxsize=5000", "-checkblocks=5", "-disablesafemode"])
 
         self.log.info("Generating new longer chain of 300 more blocks")
         self.nodes[1].generate(300)
@@ -227,13 +229,15 @@ class PruneTest(BitcoinTestFramework):
 
     def manual_test(self, node_number, use_timestamp):
         # at this point, node has 995 blocks and has not yet run in prune mode
-        node = self.nodes[node_number] = self.start_node(node_number, self.options.tmpdir, timewait=900)
+        self.start_node(node_number)
+        node = self.nodes[node_number]
         assert_equal(node.getblockcount(), 995)
         assert_raises_jsonrpc(-1, "not in prune mode", node.pruneblockchain, 500)
-        self.stop_node(node_number)
 
         # now re-start in manual pruning mode
-        node = self.nodes[node_number] = self.start_node(node_number, self.options.tmpdir, ["-prune=1"], timewait=900)
+        self.stop_node(node_number)
+        self.start_node(node_number, extra_args=["-prune=1"])
+        node = self.nodes[node_number]
         assert_equal(node.getblockcount(), 995)
 
         def height(index):
@@ -307,7 +311,7 @@ class PruneTest(BitcoinTestFramework):
 
         # stop node, start back up with auto-prune at 550MB, make sure still runs
         self.stop_node(node_number)
-        self.nodes[node_number] = self.start_node(node_number, self.options.tmpdir, ["-prune=550"], timewait=900)
+        self.start_node(node_number, extra_args=["-prune=550"])
 
         self.log.info("Success")
 
@@ -315,7 +319,7 @@ class PruneTest(BitcoinTestFramework):
         # check that the pruning node's wallet is still in good shape
         self.log.info("Stop and start pruning node to trigger wallet rescan")
         self.stop_node(2)
-        self.nodes[2] = self.start_node(2, self.options.tmpdir, ["-prune=550"])
+        self.start_node(2, extra_args=["-prune=550"])
         self.log.info("Success")
 
         # check that wallet loads successfully when restarting a pruned node after IBD.
@@ -325,7 +329,7 @@ class PruneTest(BitcoinTestFramework):
         nds = [self.nodes[0], self.nodes[5]]
         sync_blocks(nds, wait=5, timeout=300)
         self.stop_node(5) #stop and start to trigger rescan
-        self.nodes[5] = self.start_node(5, self.options.tmpdir, ["-prune=550"])
+        self.start_node(5, extra_args=["-prune=550"])
         self.log.info("Success")
 
     def run_test(self):

test/functional/rawtransactions.py

@@ -17,9 +17,7 @@ from test_framework.util import *
 
 # Create one-input, one-output, no-fee transaction:
 class RawTransactionsTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 3
 

test/functional/receivedby.py

@@ -23,16 +23,9 @@ def get_sub_array_from_array(object_array, to_match):
     return []
 
 class ReceivedByTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
+        self.num_nodes = 2
-        super().__init__()
-        self.num_nodes = 4
-        self.setup_clean_chain = False
-
-    def setup_nodes(self):
-        #This test requires mocktime
         self.enable_mocktime()
-        self.nodes = self.start_nodes(self.num_nodes, self.options.tmpdir)
 
     def run_test(self):
         '''

test/functional/reindex.py

@@ -15,8 +15,7 @@ import time
 
 class ReindexTest(BitcoinTestFramework):
 
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 1
 
@@ -25,7 +24,7 @@ class ReindexTest(BitcoinTestFramework):
         blockcount = self.nodes[0].getblockcount()
         self.stop_nodes()
         extra_args = [["-reindex-chainstate" if justchainstate else "-reindex", "-checkblockindex=1"]]
-        self.nodes = self.start_nodes(self.num_nodes, self.options.tmpdir, extra_args)
+        self.start_nodes(extra_args)
         while self.nodes[0].getblockcount() < blockcount:
             time.sleep(0.1)
         assert_equal(self.nodes[0].getblockcount(), blockcount)

test/functional/replace-by-fee.py

@@ -61,16 +61,15 @@ def make_utxo(node, amount, confirmed=True, scriptPubKey=CScript([1])):
 
 class ReplaceByFeeTest(BitcoinTestFramework):
 
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
+        self.num_nodes = 2
-        self.num_nodes = 1
-        self.setup_clean_chain = False
         self.extra_args= [["-maxorphantx=1000",
                            "-whitelist=127.0.0.1",
                            "-limitancestorcount=50",
                            "-limitancestorsize=101",
                            "-limitdescendantcount=200",
-                           "-limitdescendantsize=101"]]
+                           "-limitdescendantsize=101"],
+                           ["-mempoolreplacement=0"]]
 
     def run_test(self):
         make_utxo(self.nodes[0], 1*COIN)
@@ -117,6 +116,8 @@ class ReplaceByFeeTest(BitcoinTestFramework):
         tx1a_hex = txToHex(tx1a)
         tx1a_txid = self.nodes[0].sendrawtransaction(tx1a_hex, True)
 
+        self.sync_all([self.nodes])
+
         # Should fail because we haven't changed the fee
         tx1b = CTransaction()
         tx1b.vin = [CTxIn(tx0_outpoint, nSequence=0)]
@@ -125,12 +126,17 @@ class ReplaceByFeeTest(BitcoinTestFramework):
 
         # This will raise an exception due to insufficient fee
         assert_raises_jsonrpc(-26, "insufficient fee", self.nodes[0].sendrawtransaction, tx1b_hex, True)
+        # This will raise an exception due to transaction replacement being disabled
+        assert_raises_jsonrpc(-26, "txn-mempool-conflict", self.nodes[1].sendrawtransaction, tx1b_hex, True)
 
         # Extra 0.1 BTC fee
         tx1b = CTransaction()
         tx1b.vin = [CTxIn(tx0_outpoint, nSequence=0)]
         tx1b.vout = [CTxOut(int(0.9*COIN), CScript([b'b']))]
         tx1b_hex = txToHex(tx1b)
+        # Replacement still disabled even with "enough fee"
+        assert_raises_jsonrpc(-26, "txn-mempool-conflict", self.nodes[1].sendrawtransaction, tx1b_hex, True)
+        # Works when enabled
         tx1b_txid = self.nodes[0].sendrawtransaction(tx1b_hex, True)
 
         mempool = self.nodes[0].getrawmempool()
@@ -140,6 +146,11 @@ class ReplaceByFeeTest(BitcoinTestFramework):
 
         assert_equal(tx1b_hex, self.nodes[0].getrawtransaction(tx1b_txid))
 
+        # Second node is running mempoolreplacement=0, will not replace originally-seen txn
+        mempool = self.nodes[1].getrawmempool()
+        assert tx1a_txid in mempool
+        assert tx1b_txid not in mempool
+
     def test_doublespend_chain(self):
         """Doublespend of a long chain"""
 

test/functional/resendwallettransactions.py

@@ -8,11 +8,9 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal, assert_raises_jsonrpc
 
 class ResendWalletTransactionsTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
-        self.extra_args = [['--walletbroadcast=false']]
         self.num_nodes = 1
+        self.extra_args = [['--walletbroadcast=false']]
 
     def run_test(self):
         # Should raise RPC_WALLET_ERROR (-4) if walletbroadcast is disabled.
@@ -20,7 +18,7 @@ class ResendWalletTransactionsTest(BitcoinTestFramework):
 
         # Should return an empty array if there aren't unconfirmed wallet transactions.
         self.stop_node(0)
-        self.nodes[0] = self.start_node(0, self.options.tmpdir)
+        self.start_node(0, extra_args=[])
         assert_equal(self.nodes[0].resendwallettransactions(), [])
 
         # Should return an array with the unconfirmed wallet transaction.

test/functional/rest.py

@@ -43,8 +43,7 @@ def http_post_call(host, port, path, requestdata = '', response_object = 0):
 class RESTTest (BitcoinTestFramework):
     FORMAT_SEPARATOR = "."
 
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 3
 

test/functional/rpcbind_test.py

@@ -11,19 +11,13 @@ from test_framework.test_framework import BitcoinTestFramework, SkipTest
 from test_framework.util import *
 from test_framework.netutil import *
 
-
 class RPCBindTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 1
 
     def setup_network(self):
-        pass
+        self.add_nodes(self.num_nodes, None)
-
-    def setup_nodes(self):
-        pass
 
     def run_bind_test(self, allow_ips, connect_to, addresses, expected):
         '''
@@ -31,13 +25,15 @@ class RPCBindTest(BitcoinTestFramework):
         then try to connect, and check if the set of bound addresses
         matches the expected set.
         '''
+        self.log.info("Bind test for %s" % str(addresses))
         expected = [(addr_to_hex(addr), port) for (addr, port) in expected]
         base_args = ['-disablewallet', '-nolisten']
         if allow_ips:
             base_args += ['-rpcallowip=' + x for x in allow_ips]
         binds = ['-rpcbind='+addr for addr in addresses]
-        self.nodes = self.start_nodes(self.num_nodes, self.options.tmpdir, [base_args + binds], connect_to)
+        self.nodes[0].rpchost = connect_to
-        pid = self.bitcoind_processes[0].pid
+        self.start_node(0, base_args + binds)
+        pid = self.nodes[0].process.pid
         assert_equal(set(get_bind_addrs(pid)), set(expected))
         self.stop_nodes()
 
@@ -46,10 +42,12 @@ class RPCBindTest(BitcoinTestFramework):
         Start a node with rpcallow IP, and request getnetworkinfo
         at a non-localhost IP.
         '''
+        self.log.info("Allow IP test for %s:%d" % (rpchost, rpcport))
         base_args = ['-disablewallet', '-nolisten'] + ['-rpcallowip='+x for x in allow_ips]
-        self.nodes = self.start_nodes(self.num_nodes, self.options.tmpdir, [base_args])
+        self.nodes[0].rpchost = None
+        self.start_nodes([base_args])
         # connect to node through non-loopback interface
-        node = get_rpc_proxy(rpc_url(get_datadir_path(self.options.tmpdir, 0), 0, "%s:%d" % (rpchost, rpcport)), 0)
+        node = get_rpc_proxy(rpc_url(get_datadir_path(self.options.tmpdir, 0), 0, "%s:%d" % (rpchost, rpcport)), 0, coveragedir=self.options.coveragedir)
         node.getnetworkinfo()
         self.stop_nodes()
 

test/functional/rpcnamedargs.py

@@ -10,15 +10,8 @@ from test_framework.util import (
     assert_raises_jsonrpc,
 )
 
-
 class NamedArgumentTest(BitcoinTestFramework):
-    """
+    def set_test_params(self):
-    Test named arguments on RPC calls.
-    """
-
-    def __init__(self):
-        super().__init__()
-        self.setup_clean_chain = False
         self.num_nodes = 1
 
     def run_test(self):

test/functional/segwit.py

@@ -75,9 +75,7 @@ def find_unspent(node, min_value):
             return utxo
 
 class SegWitTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 3
         self.extra_args = [["-walletprematurewitness", "-rpcserialversion=0"],

test/functional/sendheaders.py

@@ -128,7 +128,7 @@ class TestNode(NodeConnCB):
         expect_headers = headers if headers != None else []
         expect_inv = inv if inv != None else []
         test_function = lambda: self.block_announced
-        assert(wait_until(test_function, timeout=60))
+        wait_until(test_function, timeout=60, lock=mininode_lock)
         with mininode_lock:
             self.block_announced = False
 
@@ -155,12 +155,12 @@ class TestNode(NodeConnCB):
             return
 
         test_function = lambda: "getdata" in self.last_message and [x.hash for x in self.last_message["getdata"].inv] == hash_list
-        assert(wait_until(test_function, timeout=timeout))
+        wait_until(test_function, timeout=timeout, lock=mininode_lock)
         return
 
     def wait_for_block_announcement(self, block_hash, timeout=60):
         test_function = lambda: self.last_blockhash_announced == block_hash
-        assert(wait_until(test_function, timeout=timeout))
+        wait_until(test_function, timeout=timeout, lock=mininode_lock)
         return
 
     def send_header_for_blocks(self, new_blocks):
@@ -174,8 +174,7 @@ class TestNode(NodeConnCB):
         self.send_message(getblocks_message)
 
 class SendHeadersTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 2
 

test/functional/signmessages.py

@@ -5,31 +5,34 @@
 """Test RPC commands for signing and verifying messages."""
 
 from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_equal
 
 class SignMessagesTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 1
 
     def run_test(self):
         message = 'This is just a test message'
 
-        # Test the signing with a privkey
+        self.log.info('test signing with priv_key')
-        privKey = 'cUeKHd5orzT3mz8P9pxyREHfsWtVfgsfDjiZZBcjUBAaGk1BTj7N'
+        priv_key = 'cUeKHd5orzT3mz8P9pxyREHfsWtVfgsfDjiZZBcjUBAaGk1BTj7N'
         address = 'mpLQjfK79b7CCV4VMJWEWAj5Mpx8Up5zxB'
-        signature = self.nodes[0].signmessagewithprivkey(privKey, message)
+        expected_signature = 'INbVnW4e6PeRmsv2Qgu8NuopvrVjkcxob+sX8OcZG0SALhWybUjzMLPdAsXI46YZGb0KQTRii+wWIQzRpG/U+S0='
-
+        signature = self.nodes[0].signmessagewithprivkey(priv_key, message)
-        # Verify the message
+        assert_equal(expected_signature, signature)
         assert(self.nodes[0].verifymessage(address, signature, message))
 
-        # Test the signing with an address with wallet
+        self.log.info('test signing with an address with wallet')
         address = self.nodes[0].getnewaddress()
         signature = self.nodes[0].signmessage(address, message)
-
-        # Verify the message
         assert(self.nodes[0].verifymessage(address, signature, message))
 
+        self.log.info('test verifying with another address should not work')
+        other_address = self.nodes[0].getnewaddress()
+        other_signature = self.nodes[0].signmessage(other_address, message)
+        assert(not self.nodes[0].verifymessage(other_address, signature, message))
+        assert(not self.nodes[0].verifymessage(address, other_signature, message))
+
 if __name__ == '__main__':
     SignMessagesTest().main()

test/functional/signrawtransactions.py

@@ -9,8 +9,7 @@ from test_framework.util import *
 
 
 class SignRawTransactionsTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 1
 

test/functional/smartfees.py

@@ -141,11 +141,8 @@ def check_estimates(node, fees_seen, max_invalid, print_estimates = True):
 
 
 class EstimateFeeTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.num_nodes = 3
-        self.setup_clean_chain = False
 
     def setup_network(self):
         """
@@ -153,57 +150,16 @@ class EstimateFeeTest(BitcoinTestFramework):
         But first we need to use one node to create a lot of outputs
         which we will use to generate our transactions.
         """
-        self.nodes = []
+        self.add_nodes(3, extra_args=[["-maxorphantx=1000", "-whitelist=127.0.0.1"],
+                                      ["-blockmaxsize=17000", "-maxorphantx=1000"],
+                                      ["-blockmaxsize=8000", "-maxorphantx=1000"]])
         # Use node0 to mine blocks for input splitting
-        self.nodes.append(self.start_node(0, self.options.tmpdir, ["-maxorphantx=1000",
-                                                              "-whitelist=127.0.0.1"]))
-
-        self.log.info("This test is time consuming, please be patient")
-        self.log.info("Splitting inputs so we can generate tx's")
-        self.txouts = []
-        self.txouts2 = []
-        # Split a coinbase into two transaction puzzle outputs
-        split_inputs(self.nodes[0], self.nodes[0].listunspent(0), self.txouts, True)
-
-        # Mine
-        while (len(self.nodes[0].getrawmempool()) > 0):
-            self.nodes[0].generate(1)
-
-        # Repeatedly split those 2 outputs, doubling twice for each rep
-        # Use txouts to monitor the available utxo, since these won't be tracked in wallet
-        reps = 0
-        while (reps < 5):
-            #Double txouts to txouts2
-            while (len(self.txouts)>0):
-                split_inputs(self.nodes[0], self.txouts, self.txouts2)
-            while (len(self.nodes[0].getrawmempool()) > 0):
-                self.nodes[0].generate(1)
-            #Double txouts2 to txouts
-            while (len(self.txouts2)>0):
-                split_inputs(self.nodes[0], self.txouts2, self.txouts)
-            while (len(self.nodes[0].getrawmempool()) > 0):
-                self.nodes[0].generate(1)
-            reps += 1
-        self.log.info("Finished splitting")
-
-        # Now we can connect the other nodes, didn't want to connect them earlier
-        # so the estimates would not be affected by the splitting transactions
         # Node1 mines small blocks but that are bigger than the expected transaction rate.
         # NOTE: the CreateNewBlock code starts counting block size at 1,000 bytes,
         # (17k is room enough for 110 or so transactions)
-        self.nodes.append(self.start_node(1, self.options.tmpdir,
-                                     ["-blockmaxsize=17000", "-maxorphantx=1000"]))
-        connect_nodes(self.nodes[1], 0)
-
         # Node2 is a stingy miner, that
         # produces too small blocks (room for only 55 or so transactions)
-        node2args = ["-blockmaxsize=8000", "-maxorphantx=1000"]
 
-        self.nodes.append(self.start_node(2, self.options.tmpdir, node2args))
-        connect_nodes(self.nodes[0], 2)
-        connect_nodes(self.nodes[2], 1)
-
-        self.sync_all()
 
     def transact_and_mine(self, numblocks, mining_node):
         min_fee = Decimal("0.00001")
@@ -232,9 +188,51 @@ class EstimateFeeTest(BitcoinTestFramework):
             self.memutxo = newmem
 
     def run_test(self):
+        self.log.info("This test is time consuming, please be patient")
+        self.log.info("Splitting inputs so we can generate tx's")
+
         # Make log handler available to helper functions
         global log
         log = self.log
+
+        # Start node0
+        self.start_node(0)
+        self.txouts = []
+        self.txouts2 = []
+        # Split a coinbase into two transaction puzzle outputs
+        split_inputs(self.nodes[0], self.nodes[0].listunspent(0), self.txouts, True)
+
+        # Mine
+        while (len(self.nodes[0].getrawmempool()) > 0):
+            self.nodes[0].generate(1)
+
+        # Repeatedly split those 2 outputs, doubling twice for each rep
+        # Use txouts to monitor the available utxo, since these won't be tracked in wallet
+        reps = 0
+        while (reps < 5):
+            #Double txouts to txouts2
+            while (len(self.txouts)>0):
+                split_inputs(self.nodes[0], self.txouts, self.txouts2)
+            while (len(self.nodes[0].getrawmempool()) > 0):
+                self.nodes[0].generate(1)
+            #Double txouts2 to txouts
+            while (len(self.txouts2)>0):
+                split_inputs(self.nodes[0], self.txouts2, self.txouts)
+            while (len(self.nodes[0].getrawmempool()) > 0):
+                self.nodes[0].generate(1)
+            reps += 1
+        self.log.info("Finished splitting")
+
+        # Now we can connect the other nodes, didn't want to connect them earlier
+        # so the estimates would not be affected by the splitting transactions
+        self.start_node(1)
+        self.start_node(2)
+        connect_nodes(self.nodes[1], 0)
+        connect_nodes(self.nodes[0], 2)
+        connect_nodes(self.nodes[2], 1)
+
+        self.sync_all()
+
         self.fees_per_kb = []
         self.memutxo = []
         self.confutxo = self.txouts # Start with the set of confirmed txouts after splitting

test/functional/test_framework/comptool.py

@@ -19,7 +19,7 @@ TestNode behaves as follows:
 
 from .mininode import *
 from .blockstore import BlockStore, TxStore
-from .util import p2p_port
+from .util import p2p_port, wait_until
 
 import logging
 
@@ -189,7 +189,7 @@ class TestManager(object):
     def wait_for_disconnections(self):
         def disconnected():
             return all(node.closed for node in self.test_nodes)
-        return wait_until(disconnected, timeout=10)
+        wait_until(disconnected, timeout=10, lock=mininode_lock)
 
     def wait_for_verack(self):
         return all(node.wait_for_verack() for node in self.test_nodes)
@@ -197,7 +197,7 @@ class TestManager(object):
     def wait_for_pings(self, counter):
         def received_pongs():
             return all(node.received_ping_response(counter) for node in self.test_nodes)
-        return wait_until(received_pongs)
+        wait_until(received_pongs, lock=mininode_lock)
 
     # sync_blocks: Wait for all connections to request the blockhash given
     # then send get_headers to find out the tip of each node, and synchronize
@@ -210,8 +210,7 @@ class TestManager(object):
             )
 
         # --> error if not requested
-        if not wait_until(blocks_requested, attempts=20*num_blocks):
+        wait_until(blocks_requested, attempts=20*num_blocks, lock=mininode_lock)
-            raise AssertionError("Not all nodes requested block")
 
         # Send getheaders message
         [ c.cb.send_getheaders() for c in self.connections ]
@@ -231,8 +230,7 @@ class TestManager(object):
             )
 
         # --> error if not requested
-        if not wait_until(transaction_requested, attempts=20*num_events):
+        wait_until(transaction_requested, attempts=20*num_events, lock=mininode_lock)
-            raise AssertionError("Not all nodes requested transaction")
 
         # Get the mempool
         [ c.cb.send_mempool() for c in self.connections ]

test/functional/test_framework/mininode.py

@@ -35,7 +35,7 @@ import time
 from threading import RLock, Thread
 
 from test_framework.siphash import siphash256
-from test_framework.util import hex_str_to_bytes, bytes_to_hex_str
+from test_framework.util import hex_str_to_bytes, bytes_to_hex_str, wait_until
 
 BIP0031_VERSION = 60000
 MY_VERSION = 70014  # past bip-31 for ping/pong
@@ -1358,23 +1358,6 @@ class msg_reject(object):
         return "msg_reject: %s %d %s [%064x]" \
             % (self.message, self.code, self.reason, self.data)
 
-# Helper function
-def wait_until(predicate, *, attempts=float('inf'), timeout=float('inf')):
-    if attempts == float('inf') and timeout == float('inf'):
-        timeout = 60
-    attempt = 0
-    elapsed = 0
-
-    while attempt < attempts and elapsed < timeout:
-        with mininode_lock:
-            if predicate():
-                return True
-        attempt += 1
-        elapsed += 0.05
-        time.sleep(0.05)
-
-    return False
-
 class msg_feefilter(object):
     command = b"feefilter"
 
@@ -1522,6 +1505,7 @@ class NodeConnCB(object):
             except:
                 print("ERROR delivering %s (%s)" % (repr(message),
                                                     sys.exc_info()[0]))
+                raise
 
     def set_deliver_sleep_time(self, value):
         with mininode_lock:
@@ -1591,21 +1575,21 @@ class NodeConnCB(object):
 
     def wait_for_disconnect(self, timeout=60):
         test_function = lambda: not self.connected
-        assert wait_until(test_function, timeout=timeout)
+        wait_until(test_function, timeout=timeout, lock=mininode_lock)
 
     # Message receiving helper methods
 
     def wait_for_block(self, blockhash, timeout=60):
         test_function = lambda: self.last_message.get("block") and self.last_message["block"].block.rehash() == blockhash
-        assert wait_until(test_function, timeout=timeout)
+        wait_until(test_function, timeout=timeout, lock=mininode_lock)
 
     def wait_for_getdata(self, timeout=60):
         test_function = lambda: self.last_message.get("getdata")
-        assert wait_until(test_function, timeout=timeout)
+        wait_until(test_function, timeout=timeout, lock=mininode_lock)
 
     def wait_for_getheaders(self, timeout=60):
         test_function = lambda: self.last_message.get("getheaders")
-        assert wait_until(test_function, timeout=timeout)
+        wait_until(test_function, timeout=timeout, lock=mininode_lock)
 
     def wait_for_inv(self, expected_inv, timeout=60):
         """Waits for an INV message and checks that the first inv object in the message was as expected."""
@@ -1614,11 +1598,11 @@ class NodeConnCB(object):
         test_function = lambda: self.last_message.get("inv") and \
                                 self.last_message["inv"].inv[0].type == expected_inv[0].type and \
                                 self.last_message["inv"].inv[0].hash == expected_inv[0].hash
-        assert wait_until(test_function, timeout=timeout)
+        wait_until(test_function, timeout=timeout, lock=mininode_lock)
 
     def wait_for_verack(self, timeout=60):
         test_function = lambda: self.message_count["verack"]
-        assert wait_until(test_function, timeout=timeout)
+        wait_until(test_function, timeout=timeout, lock=mininode_lock)
 
     # Message sending helper functions
 
@@ -1636,7 +1620,7 @@ class NodeConnCB(object):
     def sync_with_ping(self, timeout=60):
         self.send_message(msg_ping(nonce=self.ping_counter))
         test_function = lambda: self.last_message.get("pong") and self.last_message["pong"].nonce == self.ping_counter
-        assert wait_until(test_function, timeout=timeout)
+        wait_until(test_function, timeout=timeout, lock=mininode_lock)
         self.ping_counter += 1
         return True
 
@@ -1725,13 +1709,10 @@ class NodeConn(asyncore.dispatcher):
         self.cb.on_close(self)
 
     def handle_read(self):
-        try:
         t = self.recv(8192)
         if len(t) > 0:
             self.recvbuf += t
             self.got_data()
-        except:
-            pass
 
     def readable(self):
         return True
@@ -1797,8 +1778,10 @@ class NodeConn(asyncore.dispatcher):
                     self.got_message(t)
                 else:
                     logger.warning("Received unknown command from %s:%d: '%s' %s" % (self.dstaddr, self.dstport, command, repr(msg)))
+                    raise ValueError("Unknown command: '%s'" % (command))
         except Exception as e:
             logger.exception('got_data:', repr(e))
+            raise
 
     def send_message(self, message, pushbuf=False):
         if self.state != "connected" and not pushbuf:
@@ -1854,6 +1837,7 @@ class NetworkThread(Thread):
                     disconnected.append(obj)
             [ obj.handle_close() for obj in disconnected ]
             asyncore.loop(0.1, use_poll=True, map=mininode_socket_map, count=1)
+        logger.debug("Network thread closing")
 
 
 # An exception we can raise if we detect a potential disconnect

test/functional/test_framework/test_framework.py

@@ -5,15 +5,12 @@
 """Base class for RPC testing."""
 
 from collections import deque
-import errno
 from enum import Enum
-import http.client
 import logging
 import optparse
 import os
 import pdb
 import shutil
-import subprocess
 import sys
 import tempfile
 import time
@@ -21,6 +18,7 @@ import traceback
 
 from .authproxy import JSONRPCException
 from . import coverage
+from .test_node import TestNode
 from .util import (
     MAX_NODES,
     PortSeed,
@@ -28,12 +26,9 @@ from .util import (
     check_json_precision,
     connect_nodes_bi,
     disconnect_nodes,
-    get_rpc_proxy,
     initialize_datadir,
-    get_datadir_path,
     log_filename,
     p2p_port,
-    rpc_url,
     set_node_times,
     sync_blocks,
     sync_mempools,
@@ -48,63 +43,33 @@ TEST_EXIT_PASSED = 0
 TEST_EXIT_FAILED = 1
 TEST_EXIT_SKIPPED = 77
 
-BITCOIND_PROC_WAIT_TIMEOUT = 60
-
 class BitcoinTestFramework(object):
     """Base class for a bitcoin test script.
 
-    Individual bitcoin test scripts should subclass this class and override the following methods:
+    Individual bitcoin test scripts should subclass this class and override the set_test_params() and run_test() methods.
+
+    Individual tests can also override the following methods to customize the test setup:
 
-    - __init__()
     - add_options()
     - setup_chain()
     - setup_network()
-    - run_test()
+    - setup_nodes()
 
-    The main() method should not be overridden.
+    The __init__() and main() methods should not be overridden.
 
     This class also contains various public and private helper methods."""
 
-    # Methods to override in subclass test scripts.
     def __init__(self):
-        self.num_nodes = 4
+        """Sets test framework defaults. Do not override this method. Instead, override the set_test_params() method"""
         self.setup_clean_chain = False
         self.nodes = []
-        self.bitcoind_processes = {}
         self.mocktime = 0
+        self.set_test_params()
 
-    def add_options(self, parser):
+        assert hasattr(self, "num_nodes"), "Test must set self.num_nodes in set_test_params()"
-        pass
-
-    def setup_chain(self):
-        self.log.info("Initializing test directory " + self.options.tmpdir)
-        if self.setup_clean_chain:
-            self._initialize_chain_clean(self.options.tmpdir, self.num_nodes)
-        else:
-            self._initialize_chain(self.options.tmpdir, self.num_nodes, self.options.cachedir)
-
-    def setup_network(self):
-        self.setup_nodes()
-
-        # Connect the nodes as a "chain".  This allows us
-        # to split the network between nodes 1 and 2 to get
-        # two halves that can work on competing chains.
-        for i in range(self.num_nodes - 1):
-            connect_nodes_bi(self.nodes, i, i + 1)
-        self.sync_all()
-
-    def setup_nodes(self):
-        extra_args = None
-        if hasattr(self, "extra_args"):
-            extra_args = self.extra_args
-        self.nodes = self.start_nodes(self.num_nodes, self.options.tmpdir, extra_args)
-
-    def run_test(self):
-        raise NotImplementedError
-
-    # Main function. This should not be overridden by the subclass test scripts.
 
     def main(self):
+        """Main function. This should not be overridden by the subclass test scripts."""
 
         parser = optparse.OptionParser(usage="%prog [options]")
         parser.add_option("--nocleanup", dest="nocleanup", default=False, action="store_true",
@@ -208,77 +173,115 @@ class BitcoinTestFramework(object):
             logging.shutdown()
             sys.exit(TEST_EXIT_FAILED)
 
-    # Public helper methods. These can be accessed by the subclass test scripts.
+    # Methods to override in subclass test scripts.
+    def set_test_params(self):
+        """Tests must this method to change default values for number of nodes, topology, etc"""
+        raise NotImplementedError
 
-    def start_node(self, i, dirname, extra_args=None, rpchost=None, timewait=None, binary=None, stderr=None):
+    def add_options(self, parser):
-        """Start a bitcoind and return RPC connection to it"""
+        """Override this method to add command-line options to the test"""
+        pass
 
-        datadir = os.path.join(dirname, "node" + str(i))
+    def setup_chain(self):
-        if binary is None:
+        """Override this method to customize blockchain setup"""
-            binary = os.getenv("BITCOIND", "bitcoind")
+        self.log.info("Initializing test directory " + self.options.tmpdir)
-        args = [binary, "-datadir=" + datadir, "-server", "-keypool=1", "-discover=0", "-rest", "-logtimemicros", "-debug", "-debugexclude=libevent", "-debugexclude=leveldb", "-mocktime=" + str(self.mocktime), "-uacomment=testnode%d" % i]
+        if self.setup_clean_chain:
-        if extra_args is not None:
+            self._initialize_chain_clean()
-            args.extend(extra_args)
+        else:
-        self.bitcoind_processes[i] = subprocess.Popen(args, stderr=stderr)
+            self._initialize_chain()
-        self.log.debug("initialize_chain: bitcoind started, waiting for RPC to come up")
-        self._wait_for_bitcoind_start(self.bitcoind_processes[i], datadir, i, rpchost)
-        self.log.debug("initialize_chain: RPC successfully started")
-        proxy = get_rpc_proxy(rpc_url(datadir, i, rpchost), i, timeout=timewait)
 
-        if self.options.coveragedir:
+    def setup_network(self):
-            coverage.write_all_rpc_commands(self.options.coveragedir, proxy)
+        """Override this method to customize test network topology"""
+        self.setup_nodes()
 
-        return proxy
+        # Connect the nodes as a "chain".  This allows us
+        # to split the network between nodes 1 and 2 to get
+        # two halves that can work on competing chains.
+        for i in range(self.num_nodes - 1):
+            connect_nodes_bi(self.nodes, i, i + 1)
+        self.sync_all()
 
-    def start_nodes(self, num_nodes, dirname, extra_args=None, rpchost=None, timewait=None, binary=None):
+    def setup_nodes(self):
-        """Start multiple bitcoinds, return RPC connections to them"""
+        """Override this method to customize test node setup"""
+        extra_args = None
+        if hasattr(self, "extra_args"):
+            extra_args = self.extra_args
+        self.add_nodes(self.num_nodes, extra_args)
+        self.start_nodes()
+
+    def run_test(self):
+        """Tests must override this method to define test logic"""
+        raise NotImplementedError
+
+    # Public helper methods. These can be accessed by the subclass test scripts.
+
+    def add_nodes(self, num_nodes, extra_args=None, rpchost=None, timewait=None, binary=None):
+        """Instantiate TestNode objects"""
 
         if extra_args is None:
-            extra_args = [None] * num_nodes
+            extra_args = [[]] * num_nodes
         if binary is None:
             binary = [None] * num_nodes
         assert_equal(len(extra_args), num_nodes)
         assert_equal(len(binary), num_nodes)
-        rpcs = []
-        try:
         for i in range(num_nodes):
-                rpcs.append(self.start_node(i, dirname, extra_args[i], rpchost, timewait=timewait, binary=binary[i]))
+            self.nodes.append(TestNode(i, self.options.tmpdir, extra_args[i], rpchost, timewait=timewait, binary=binary[i], stderr=None, mocktime=self.mocktime, coverage_dir=self.options.coveragedir))
+
+    def start_node(self, i, extra_args=None, stderr=None):
+        """Start a bitcoind"""
+
+        node = self.nodes[i]
+
+        node.start(extra_args, stderr)
+        node.wait_for_rpc_connection()
+
+        if self.options.coveragedir is not None:
+            coverage.write_all_rpc_commands(self.options.coveragedir, node.rpc)
+
+    def start_nodes(self, extra_args=None):
+        """Start multiple bitcoinds"""
+
+        if extra_args is None:
+            extra_args = [None] * self.num_nodes
+        assert_equal(len(extra_args), self.num_nodes)
+        try:
+            for i, node in enumerate(self.nodes):
+                node.start(extra_args[i])
+            for node in self.nodes:
+                node.wait_for_rpc_connection()
         except:
             # If one node failed to start, stop the others
-            # TODO: abusing self.nodes in this way is a little hacky.
-            # Eventually we should do a better job of tracking nodes
-            self.nodes.extend(rpcs)
             self.stop_nodes()
-            self.nodes = []
             raise
-        return rpcs
+
+        if self.options.coveragedir is not None:
+            for node in self.nodes:
+                coverage.write_all_rpc_commands(self.options.coveragedir, node.rpc)
 
     def stop_node(self, i):
         """Stop a bitcoind test node"""
-
+        self.nodes[i].stop_node()
-        self.log.debug("Stopping node %d" % i)
+        self.nodes[i].wait_until_stopped()
-        try:
-            self.nodes[i].stop()
-        except http.client.CannotSendRequest as e:
-            self.log.exception("Unable to stop node")
-        return_code = self.bitcoind_processes[i].wait(timeout=BITCOIND_PROC_WAIT_TIMEOUT)
-        del self.bitcoind_processes[i]
-        assert_equal(return_code, 0)
 
     def stop_nodes(self):
         """Stop multiple bitcoind test nodes"""
+        for node in self.nodes:
+            # Issue RPC to stop nodes
+            node.stop_node()
 
-        for i in range(len(self.nodes)):
+        for node in self.nodes:
-            self.stop_node(i)
+            # Wait for nodes to stop
-        assert not self.bitcoind_processes.values()  # All connections must be gone now
+            node.wait_until_stopped()
 
-    def assert_start_raises_init_error(self, i, dirname, extra_args=None, expected_msg=None):
+    def assert_start_raises_init_error(self, i, extra_args=None, expected_msg=None):
         with tempfile.SpooledTemporaryFile(max_size=2**16) as log_stderr:
             try:
-                self.start_node(i, dirname, extra_args, stderr=log_stderr)
+                self.start_node(i, extra_args, stderr=log_stderr)
                 self.stop_node(i)
             except Exception as e:
                 assert 'bitcoind exited' in str(e)  # node must have shutdown
+                self.nodes[i].running = False
+                self.nodes[i].process = None
                 if expected_msg is not None:
                     log_stderr.seek(0)
                     stderr = log_stderr.read().decode('utf-8')
@@ -292,7 +295,7 @@ class BitcoinTestFramework(object):
                 raise AssertionError(assert_msg)
 
     def wait_for_node_exit(self, i, timeout):
-        self.bitcoind_processes[i].wait(timeout)
+        self.nodes[i].process.wait(timeout)
 
     def split_network(self):
         """
@@ -362,16 +365,16 @@ class BitcoinTestFramework(object):
             rpc_handler.setLevel(logging.DEBUG)
             rpc_logger.addHandler(rpc_handler)
 
-    def _initialize_chain(self, test_dir, num_nodes, cachedir):
+    def _initialize_chain(self):
         """Initialize a pre-mined blockchain for use by the test.
 
         Create a cache of a 200-block-long chain (with wallet) for MAX_NODES
         Afterward, create num_nodes copies from the cache."""
 
-        assert num_nodes <= MAX_NODES
+        assert self.num_nodes <= MAX_NODES
         create_cache = False
         for i in range(MAX_NODES):
-            if not os.path.isdir(os.path.join(cachedir, 'node' + str(i))):
+            if not os.path.isdir(os.path.join(self.options.cachedir, 'node' + str(i))):
                 create_cache = True
                 break
 
@@ -380,27 +383,22 @@ class BitcoinTestFramework(object):
 
             # find and delete old cache directories if any exist
             for i in range(MAX_NODES):
-                if os.path.isdir(os.path.join(cachedir, "node" + str(i))):
+                if os.path.isdir(os.path.join(self.options.cachedir, "node" + str(i))):
-                    shutil.rmtree(os.path.join(cachedir, "node" + str(i)))
+                    shutil.rmtree(os.path.join(self.options.cachedir, "node" + str(i)))
 
             # Create cache directories, run bitcoinds:
             for i in range(MAX_NODES):
-                datadir = initialize_datadir(cachedir, i)
+                datadir = initialize_datadir(self.options.cachedir, i)
                 args = [os.getenv("BITCOIND", "bitcoind"), "-server", "-keypool=1", "-datadir=" + datadir, "-discover=0"]
                 if i > 0:
                     args.append("-connect=127.0.0.1:" + str(p2p_port(0)))
-                self.bitcoind_processes[i] = subprocess.Popen(args)
+                self.nodes.append(TestNode(i, self.options.cachedir, extra_args=[], rpchost=None, timewait=None, binary=None, stderr=None, mocktime=self.mocktime, coverage_dir=None))
-                self.log.debug("initialize_chain: bitcoind started, waiting for RPC to come up")
+                self.nodes[i].args = args
-                self._wait_for_bitcoind_start(self.bitcoind_processes[i], datadir, i)
+                self.start_node(i)
-                self.log.debug("initialize_chain: RPC successfully started")
 
-            self.nodes = []
+            # Wait for RPC connections to be ready
-            for i in range(MAX_NODES):
+            for node in self.nodes:
-                try:
+                node.wait_for_rpc_connection()
-                    self.nodes.append(get_rpc_proxy(rpc_url(get_datadir_path(cachedir, i), i), i))
-                except:
-                    self.log.exception("Error connecting to node %d" % i)
-                    sys.exit(1)
 
             # Create a 200-block-long chain; each of the 4 first nodes
             # gets 25 mature blocks and 25 immature.
@@ -425,48 +423,24 @@ class BitcoinTestFramework(object):
             self.nodes = []
             self.disable_mocktime()
             for i in range(MAX_NODES):
-                os.remove(log_filename(cachedir, i, "debug.log"))
+                os.remove(log_filename(self.options.cachedir, i, "debug.log"))
-                os.remove(log_filename(cachedir, i, "db.log"))
+                os.remove(log_filename(self.options.cachedir, i, "db.log"))
-                os.remove(log_filename(cachedir, i, "peers.dat"))
+                os.remove(log_filename(self.options.cachedir, i, "peers.dat"))
-                os.remove(log_filename(cachedir, i, "fee_estimates.dat"))
+                os.remove(log_filename(self.options.cachedir, i, "fee_estimates.dat"))
-
+
-        for i in range(num_nodes):
+        for i in range(self.num_nodes):
-            from_dir = os.path.join(cachedir, "node" + str(i))
+            from_dir = os.path.join(self.options.cachedir, "node" + str(i))
-            to_dir = os.path.join(test_dir, "node" + str(i))
+            to_dir = os.path.join(self.options.tmpdir, "node" + str(i))
             shutil.copytree(from_dir, to_dir)
-            initialize_datadir(test_dir, i)  # Overwrite port/rpcport in bitcoin.conf
+            initialize_datadir(self.options.tmpdir, i)  # Overwrite port/rpcport in bitcoin.conf
 
-    def _initialize_chain_clean(self, test_dir, num_nodes):
+    def _initialize_chain_clean(self):
         """Initialize empty blockchain for use by the test.
 
         Create an empty blockchain and num_nodes wallets.
         Useful if a test case wants complete control over initialization."""
-        for i in range(num_nodes):
+        for i in range(self.num_nodes):
-            initialize_datadir(test_dir, i)
+            initialize_datadir(self.options.tmpdir, i)
-
-    def _wait_for_bitcoind_start(self, process, datadir, i, rpchost=None):
-        """Wait for bitcoind to start.
-
-        This means that RPC is accessible and fully initialized.
-        Raise an exception if bitcoind exits during initialization."""
-        while True:
-            if process.poll() is not None:
-                raise Exception('bitcoind exited with status %i during initialization' % process.returncode)
-            try:
-                # Check if .cookie file to be created
-                rpc = get_rpc_proxy(rpc_url(datadir, i, rpchost), i, coveragedir=self.options.coveragedir)
-                rpc.getblockcount()
-                break  # break out of loop on success
-            except IOError as e:
-                if e.errno != errno.ECONNREFUSED:  # Port not yet open?
-                    raise  # unknown IO error
-            except JSONRPCException as e:  # Initialization phase
-                if e.error['code'] != -28:  # RPC in warmup?
-                    raise  # unknown JSON RPC exception
-            except ValueError as e:  # cookie file not found and no rpcuser or rpcassword. bitcoind still starting
-                if "No RPC credentials" not in str(e):
-                    raise
-            time.sleep(0.25)
 
 class ComparisonTestFramework(BitcoinTestFramework):
     """Test framework for doing p2p comparison testing
@@ -476,8 +450,7 @@ class ComparisonTestFramework(BitcoinTestFramework):
     - 2 binaries: 1 test binary, 1 ref binary
     - n>2 binaries: 1 test binary, n-1 ref binaries"""
 
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
         self.num_nodes = 2
         self.setup_clean_chain = True
 
@@ -490,13 +463,13 @@ class ComparisonTestFramework(BitcoinTestFramework):
                           help="bitcoind binary to use for reference nodes (if any)")
 
     def setup_network(self):
-        extra_args = [['-whitelist=127.0.0.1']]*self.num_nodes
+        extra_args = [['-whitelist=127.0.0.1']] * self.num_nodes
         if hasattr(self, "extra_args"):
             extra_args = self.extra_args
-        self.nodes = self.start_nodes(
+        self.add_nodes(self.num_nodes, extra_args,
-            self.num_nodes, self.options.tmpdir, extra_args,
                        binary=[self.options.testbinary] +
                        [self.options.refbinary] * (self.num_nodes - 1))
+        self.start_nodes()
 
 class SkipTest(Exception):
     """This exception is raised to skip a test"""

test/functional/test_framework/test_node.py

@@ -0,0 +1,190 @@
+#!/usr/bin/env python3
+# Copyright (c) 2017 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Class for bitcoind node under test"""
+
+import decimal
+import errno
+import http.client
+import json
+import logging
+import os
+import subprocess
+import time
+
+from .util import (
+    assert_equal,
+    get_rpc_proxy,
+    rpc_url,
+    wait_until,
+)
+from .authproxy import JSONRPCException
+
+BITCOIND_PROC_WAIT_TIMEOUT = 60
+
+class TestNode():
+    """A class for representing a bitcoind node under test.
+
+    This class contains:
+
+    - state about the node (whether it's running, etc)
+    - a Python subprocess.Popen object representing the running process
+    - an RPC connection to the node
+
+    To make things easier for the test writer, a bit of magic is happening under the covers.
+    Any unrecognised messages will be dispatched to the RPC connection."""
+
+    def __init__(self, i, dirname, extra_args, rpchost, timewait, binary, stderr, mocktime, coverage_dir):
+        self.index = i
+        self.datadir = os.path.join(dirname, "node" + str(i))
+        self.rpchost = rpchost
+        if timewait:
+            self.rpc_timeout = timewait
+        else:
+            # Wait for up to 60 seconds for the RPC server to respond
+            self.rpc_timeout = 60
+        if binary is None:
+            self.binary = os.getenv("BITCOIND", "bitcoind")
+        else:
+            self.binary = binary
+        self.stderr = stderr
+        self.coverage_dir = coverage_dir
+        # Most callers will just need to add extra args to the standard list below. For those callers that need more flexibity, they can just set the args property directly.
+        self.extra_args = extra_args
+        self.args = [self.binary, "-datadir=" + self.datadir, "-server", "-keypool=1", "-discover=0", "-rest", "-logtimemicros", "-debug", "-debugexclude=libevent", "-debugexclude=leveldb", "-mocktime=" + str(mocktime), "-uacomment=testnode%d" % i]
+
+        self.cli = TestNodeCLI(os.getenv("BITCOINCLI", "bitcoin-cli"), self.datadir)
+
+        self.running = False
+        self.process = None
+        self.rpc_connected = False
+        self.rpc = None
+        self.url = None
+        self.log = logging.getLogger('TestFramework.node%d' % i)
+
+    def __getattr__(self, *args, **kwargs):
+        """Dispatches any unrecognised messages to the RPC connection."""
+        assert self.rpc_connected and self.rpc is not None, "Error: no RPC connection"
+        return self.rpc.__getattr__(*args, **kwargs)
+
+    def start(self, extra_args=None, stderr=None):
+        """Start the node."""
+        if extra_args is None:
+            extra_args = self.extra_args
+        if stderr is None:
+            stderr = self.stderr
+        self.process = subprocess.Popen(self.args + extra_args, stderr=stderr)
+        self.running = True
+        self.log.debug("bitcoind started, waiting for RPC to come up")
+
+    def wait_for_rpc_connection(self):
+        """Sets up an RPC connection to the bitcoind process. Returns False if unable to connect."""
+        # Poll at a rate of four times per second
+        poll_per_s = 4
+        for _ in range(poll_per_s * self.rpc_timeout):
+            assert self.process.poll() is None, "bitcoind exited with status %i during initialization" % self.process.returncode
+            try:
+                self.rpc = get_rpc_proxy(rpc_url(self.datadir, self.index, self.rpchost), self.index, timeout=self.rpc_timeout, coveragedir=self.coverage_dir)
+                self.rpc.getblockcount()
+                # If the call to getblockcount() succeeds then the RPC connection is up
+                self.rpc_connected = True
+                self.url = self.rpc.url
+                self.log.debug("RPC successfully started")
+                return
+            except IOError as e:
+                if e.errno != errno.ECONNREFUSED:  # Port not yet open?
+                    raise  # unknown IO error
+            except JSONRPCException as e:  # Initialization phase
+                if e.error['code'] != -28:  # RPC in warmup?
+                    raise  # unknown JSON RPC exception
+            except ValueError as e:  # cookie file not found and no rpcuser or rpcassword. bitcoind still starting
+                if "No RPC credentials" not in str(e):
+                    raise
+            time.sleep(1.0 / poll_per_s)
+        raise AssertionError("Unable to connect to bitcoind")
+
+    def get_wallet_rpc(self, wallet_name):
+        assert self.rpc_connected
+        assert self.rpc
+        wallet_path = "wallet/%s" % wallet_name
+        return self.rpc / wallet_path
+
+    def stop_node(self):
+        """Stop the node."""
+        if not self.running:
+            return
+        self.log.debug("Stopping node")
+        try:
+            self.stop()
+        except http.client.CannotSendRequest:
+            self.log.exception("Unable to stop node.")
+
+    def is_node_stopped(self):
+        """Checks whether the node has stopped.
+
+        Returns True if the node has stopped. False otherwise.
+        This method is responsible for freeing resources (self.process)."""
+        if not self.running:
+            return True
+        return_code = self.process.poll()
+        if return_code is None:
+            return False
+
+        # process has stopped. Assert that it didn't return an error code.
+        assert_equal(return_code, 0)
+        self.running = False
+        self.process = None
+        self.rpc_connected = False
+        self.rpc = None
+        self.log.debug("Node stopped")
+        return True
+
+    def wait_until_stopped(self, timeout=BITCOIND_PROC_WAIT_TIMEOUT):
+        wait_until(self.is_node_stopped, timeout=timeout)
+
+    def node_encrypt_wallet(self, passphrase):
+        """"Encrypts the wallet.
+
+        This causes bitcoind to shutdown, so this method takes
+        care of cleaning up resources."""
+        self.encryptwallet(passphrase)
+        self.wait_until_stopped()
+
+class TestNodeCLI():
+    """Interface to bitcoin-cli for an individual node"""
+
+    def __init__(self, binary, datadir):
+        self.args = []
+        self.binary = binary
+        self.datadir = datadir
+        self.input = None
+
+    def __call__(self, *args, input=None):
+        # TestNodeCLI is callable with bitcoin-cli command-line args
+        self.args = [str(arg) for arg in args]
+        self.input = input
+        return self
+
+    def __getattr__(self, command):
+        def dispatcher(*args, **kwargs):
+            return self.send_cli(command, *args, **kwargs)
+        return dispatcher
+
+    def send_cli(self, command, *args, **kwargs):
+        """Run bitcoin-cli command. Deserializes returned string as python object."""
+
+        pos_args = [str(arg) for arg in args]
+        named_args = [str(key) + "=" + str(value) for (key, value) in kwargs.items()]
+        assert not (pos_args and named_args), "Cannot use positional arguments and named arguments in the same bitcoin-cli call"
+        p_args = [self.binary, "-datadir=" + self.datadir] + self.args
+        if named_args:
+            p_args += ["-named"]
+        p_args += [command] + pos_args + named_args
+        process = subprocess.Popen(p_args, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
+        cli_stdout, cli_stderr = process.communicate(input=self.input)
+        returncode = process.poll()
+        if returncode:
+            # Ignore cli_stdout, raise with cli_stderr
+            raise subprocess.CalledProcessError(returncode, self.binary, output=cli_stderr)
+        return json.loads(cli_stdout, parse_float=decimal.Decimal)

test/functional/test_framework/util.py

@@ -7,11 +7,13 @@
 from base64 import b64encode
 from binascii import hexlify, unhexlify
 from decimal import Decimal, ROUND_DOWN
+import hashlib
 import json
 import logging
 import os
 import random
 import re
+from subprocess import CalledProcessError
 import time
 
 from . import coverage
@@ -57,18 +59,42 @@ def assert_raises_message(exc, message, fun, *args, **kwds):
     else:
         raise AssertionError("No exception raised")
 
+def assert_raises_process_error(returncode, output, fun, *args, **kwds):
+    """Execute a process and asserts the process return code and output.
+
+    Calls function `fun` with arguments `args` and `kwds`. Catches a CalledProcessError
+    and verifies that the return code and output are as expected. Throws AssertionError if
+    no CalledProcessError was raised or if the return code and output are not as expected.
+
+    Args:
+        returncode (int): the process return code.
+        output (string): [a substring of] the process output.
+        fun (function): the function to call. This should execute a process.
+        args*: positional arguments for the function.
+        kwds**: named arguments for the function.
+    """
+    try:
+        fun(*args, **kwds)
+    except CalledProcessError as e:
+        if returncode != e.returncode:
+            raise AssertionError("Unexpected returncode %i" % e.returncode)
+        if output not in e.output:
+            raise AssertionError("Expected substring not found:" + e.output)
+    else:
+        raise AssertionError("No exception raised")
+
 def assert_raises_jsonrpc(code, message, fun, *args, **kwds):
     """Run an RPC and verify that a specific JSONRPC exception code and message is raised.
 
     Calls function `fun` with arguments `args` and `kwds`. Catches a JSONRPCException
     and verifies that the error code and message are as expected. Throws AssertionError if
-    no JSONRPCException was returned or if the error code/message are not as expected.
+    no JSONRPCException was raised or if the error code/message are not as expected.
 
     Args:
         code (int), optional: the error code returned by the RPC call (defined
             in src/rpc/protocol.h). Set to None if checking the error code is not required.
         message (string), optional: [a substring of] the error string returned by the
-            RPC call. Set to None if checking the error string is not required
+            RPC call. Set to None if checking the error string is not required.
         fun (function): the function to call. This should be the name of an RPC.
         args*: positional arguments for the function.
         kwds**: named arguments for the function.
@@ -148,6 +174,13 @@ def count_bytes(hex_string):
 def bytes_to_hex_str(byte_str):
     return hexlify(byte_str).decode('ascii')
 
+def hash256(byte_str):
+    sha256 = hashlib.sha256()
+    sha256.update(byte_str)
+    sha256d = hashlib.sha256()
+    sha256d.update(sha256.digest())
+    return sha256d.digest()[::-1]
+
 def hex_str_to_bytes(hex_str):
     return unhexlify(hex_str.encode('ascii'))
 
@@ -157,6 +190,28 @@ def str_to_b64str(string):
 def satoshi_round(amount):
     return Decimal(amount).quantize(Decimal('0.00000001'), rounding=ROUND_DOWN)
 
+def wait_until(predicate, *, attempts=float('inf'), timeout=float('inf'), lock=None):
+    if attempts == float('inf') and timeout == float('inf'):
+        timeout = 60
+    attempt = 0
+    timeout += time.time()
+
+    while attempt < attempts and time.time() < timeout:
+        if lock:
+            with lock:
+                if predicate():
+                    return
+        else:
+            if predicate():
+                return
+        attempt += 1
+        time.sleep(0.05)
+
+    # Print the cause of the timeout
+    assert_greater_than(attempts, attempt)
+    assert_greater_than(timeout, time.time())
+    raise RuntimeError('Unreachable')
+
 # RPC/P2P connection constants and functions
 ############################################
 
@@ -204,7 +259,7 @@ def rpc_port(n):
     return PORT_MIN + PORT_RANGE + n + (MAX_NODES * PortSeed.n) % (PORT_RANGE - 1 - MAX_NODES)
 
 def rpc_url(datadir, i, rpchost=None):
-    rpc_u, rpc_p = get_auth_cookie(datadir, i)
+    rpc_u, rpc_p = get_auth_cookie(datadir)
     host = '127.0.0.1'
     port = rpc_port(i)
     if rpchost:
@@ -232,7 +287,7 @@ def initialize_datadir(dirname, n):
 def get_datadir_path(dirname, n):
     return os.path.join(dirname, "node" + str(n))
 
-def get_auth_cookie(datadir, n):
+def get_auth_cookie(datadir):
     user = None
     password = None
     if os.path.isfile(os.path.join(datadir, "bitcoin.conf")):

test/functional/test_runner.py

@@ -81,6 +81,7 @@ BASE_SCRIPTS= [
     # vv Tests less than 30s vv
     'keypool-topup.py',
     'zmq_test.py',
+    'bitcoin_cli.py',
     'mempool_resurrect_test.py',
     'txn_doublespend.py --mineblock',
     'txn_clone.py',
@@ -279,6 +280,7 @@ def run_tests(test_list, src_dir, build_dir, exeext, tmpdir, jobs=1, enable_cove
     #Set env vars
     if "BITCOIND" not in os.environ:
         os.environ["BITCOIND"] = build_dir + '/src/bitcoind' + exeext
+        os.environ["BITCOINCLI"] = build_dir + '/src/bitcoin-cli' + exeext
 
     tests_dir = src_dir + '/test/functional/'
 

test/functional/txn_clone.py

@@ -8,11 +8,8 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
 class TxnMallTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.num_nodes = 4
-        self.setup_clean_chain = False
 
     def add_options(self, parser):
         parser.add_option("--mineblock", dest="mine_block", default=False, action="store_true",

test/functional/txn_doublespend.py

@@ -8,11 +8,8 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
 class TxnMallTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.num_nodes = 4
-        self.setup_clean_chain = False
 
     def add_options(self, parser):
         parser.add_option("--mineblock", dest="mine_block", default=False, action="store_true",

test/functional/uptime.py

@@ -13,9 +13,7 @@ from test_framework.test_framework import BitcoinTestFramework
 
 
 class UptimeTest(BitcoinTestFramework):
-    def __init__(self):
+    def set_test_params(self):
-        super().__init__()
-
         self.num_nodes = 1
         self.setup_clean_chain = True
 

test/functional/wallet-accounts.py

@@ -17,9 +17,7 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal
 
 class WalletAccountsTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 1
         self.extra_args = [[]]

test/functional/wallet-dump.py

@@ -56,10 +56,7 @@ def read_dump(file_name, addrs, hd_master_addr_old):
 
 
 class WalletDumpTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
-        self.setup_clean_chain = False
         self.num_nodes = 1
         self.extra_args = [["-keypool=90"]]
 
@@ -68,7 +65,8 @@ class WalletDumpTest(BitcoinTestFramework):
         # longer than the default 30 seconds due to an expensive
         # CWallet::TopUpKeyPool call, and the encryptwallet RPC made later in
         # the test often takes even longer.
-        self.nodes = self.start_nodes(self.num_nodes, self.options.tmpdir, self.extra_args, timewait=60)
+        self.add_nodes(self.num_nodes, self.extra_args, timewait=60)
+        self.start_nodes()
 
     def run_test (self):
         tmpdir = self.options.tmpdir
@@ -94,9 +92,8 @@ class WalletDumpTest(BitcoinTestFramework):
         assert_equal(found_addr_rsv, 90*2) # 90 keys plus 100% internal keys
 
         #encrypt wallet, restart, unlock and dump
-        self.nodes[0].encryptwallet('test')
+        self.nodes[0].node_encrypt_wallet('test')
-        self.bitcoind_processes[0].wait()
+        self.start_node(0)
-        self.nodes[0] = self.start_node(0, self.options.tmpdir, self.extra_args[0])
         self.nodes[0].walletpassphrase('test', 10)
         # Should be a no-op:
         self.nodes[0].keypoolrefill()

test/functional/wallet-encryption.py

@@ -6,16 +6,14 @@
 
 import time
 
-from test_framework.test_framework import BitcoinTestFramework, BITCOIND_PROC_WAIT_TIMEOUT
+from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
     assert_raises_jsonrpc,
 )
 
 class WalletEncryptionTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 1
 
@@ -30,9 +28,8 @@ class WalletEncryptionTest(BitcoinTestFramework):
         assert_equal(len(privkey), 52)
 
         # Encrypt the wallet
-        self.nodes[0].encryptwallet(passphrase)
+        self.nodes[0].node_encrypt_wallet(passphrase)
-        self.bitcoind_processes[0].wait(timeout=BITCOIND_PROC_WAIT_TIMEOUT)
+        self.start_node(0)
-        self.nodes[0] = self.start_node(0, self.options.tmpdir)
 
         # Test that the wallet is encrypted
         assert_raises_jsonrpc(-13, "Please enter the wallet passphrase with walletpassphrase first", self.nodes[0].dumpprivkey, address)

test/functional/wallet-hd.py

@@ -11,11 +11,8 @@ from test_framework.util import (
 )
 import shutil
 
-
 class WalletHDTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 2
         self.extra_args = [['-usehd=0'], ['-usehd=1', '-keypool=0']]
@@ -25,8 +22,8 @@ class WalletHDTest(BitcoinTestFramework):
 
         # Make sure can't switch off usehd after wallet creation
         self.stop_node(1)
-        self.assert_start_raises_init_error(1, self.options.tmpdir, ['-usehd=0'], 'already existing HD wallet')
+        self.assert_start_raises_init_error(1, ['-usehd=0'], 'already existing HD wallet')
-        self.nodes[1] = self.start_node(1, self.options.tmpdir, self.extra_args[1])
+        self.start_node(1)
         connect_nodes_bi(self.nodes, 0, 1)
 
         # Make sure we use hd, keep masterkeyid
@@ -76,7 +73,7 @@ class WalletHDTest(BitcoinTestFramework):
         shutil.rmtree(tmpdir + "/node1/regtest/blocks")
         shutil.rmtree(tmpdir + "/node1/regtest/chainstate")
         shutil.copyfile(tmpdir + "/hd.bak", tmpdir + "/node1/regtest/wallet.dat")
-        self.nodes[1] = self.start_node(1, self.options.tmpdir, self.extra_args[1])
+        self.start_node(1)
 
         # Assert that derivation is deterministic
         hd_add_2 = None
@@ -91,7 +88,7 @@ class WalletHDTest(BitcoinTestFramework):
 
         # Needs rescan
         self.stop_node(1)
-        self.nodes[1] = self.start_node(1, self.options.tmpdir, self.extra_args[1] + ['-rescan'])
+        self.start_node(1, extra_args=self.extra_args[1] + ['-rescan'])
         assert_equal(self.nodes[1].getbalance(), num_hd_adds + 1)
 
         # send a tx and make sure its using the internal chain for the changeoutput

test/functional/wallet.py

@@ -7,28 +7,28 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
 class WalletTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def check_fee_amount(self, curr_balance, balance_with_fee, fee_per_byte, tx_size):
-        """Return curr_balance after asserting the fee was in range"""
-        fee = balance_with_fee - curr_balance
-        assert_fee_amount(fee, tx_size, fee_per_byte * 1000)
-        return curr_balance
-
-    def __init__(self):
-        super().__init__()
-        self.setup_clean_chain = True
         self.num_nodes = 4
+        self.setup_clean_chain = True
         self.extra_args = [['-usehd={:d}'.format(i%2==0)] for i in range(4)]
 
     def setup_network(self):
-        self.nodes = self.start_nodes(3, self.options.tmpdir, self.extra_args[:3])
+        self.add_nodes(4, self.extra_args)
+        self.start_node(0)
+        self.start_node(1)
+        self.start_node(2)
         connect_nodes_bi(self.nodes,0,1)
         connect_nodes_bi(self.nodes,1,2)
         connect_nodes_bi(self.nodes,0,2)
-        self.sync_all()
+        self.sync_all([self.nodes[0:3]])
 
-    def run_test(self):
+    def check_fee_amount(self, curr_balance, balance_with_fee, fee_per_byte, tx_size):
+        """Return curr_balance after asserting the fee was in range"""
+        fee = balance_with_fee - curr_balance
+        assert_fee_amount(fee, tx_size, fee_per_byte * 1000)
+        return curr_balance
 
+    def run_test(self):
         # Check that there's no UTXO on none of the nodes
         assert_equal(len(self.nodes[0].listunspent()), 0)
         assert_equal(len(self.nodes[1].listunspent()), 0)
@@ -42,9 +42,9 @@ class WalletTest(BitcoinTestFramework):
         assert_equal(walletinfo['immature_balance'], 50)
         assert_equal(walletinfo['balance'], 0)
 
-        self.sync_all()
+        self.sync_all([self.nodes[0:3]])
         self.nodes[1].generate(101)
-        self.sync_all()
+        self.sync_all([self.nodes[0:3]])
 
         assert_equal(self.nodes[0].getbalance(), 50)
         assert_equal(self.nodes[1].getbalance(), 50)
@@ -56,6 +56,15 @@ class WalletTest(BitcoinTestFramework):
         assert_equal(len(self.nodes[1].listunspent()), 1)
         assert_equal(len(self.nodes[2].listunspent()), 0)
 
+        self.log.info("test gettxout")
+        confirmed_txid, confirmed_index = utxos[0]["txid"], utxos[0]["vout"]
+        # First, outputs that are unspent both in the chain and in the
+        # mempool should appear with or without include_mempool
+        txout = self.nodes[0].gettxout(txid=confirmed_txid, n=confirmed_index, include_mempool=False)
+        assert_equal(txout['value'], 50)
+        txout = self.nodes[0].gettxout(txid=confirmed_txid, n=confirmed_index, include_mempool=True)
+        assert_equal(txout['value'], 50)
+        
         # Send 21 BTC from 0 to 2 using sendtoaddress call.
         # Locked memory should use at least 32 bytes to sign each transaction
         self.log.info("test getmemoryinfo")
@@ -65,10 +74,9 @@ class WalletTest(BitcoinTestFramework):
         memory_after = self.nodes[0].getmemoryinfo()
         assert(memory_before['locked']['used'] + 64 <= memory_after['locked']['used'])
 
-        self.log.info("test gettxout")
+        self.log.info("test gettxout (second part)")
         # utxo spent in mempool should be visible if you exclude mempool
         # but invisible if you include mempool
-        confirmed_txid, confirmed_index = utxos[0]["txid"], utxos[0]["vout"]
         txout = self.nodes[0].gettxout(confirmed_txid, confirmed_index, False)
         assert_equal(txout['value'], 50)
         txout = self.nodes[0].gettxout(confirmed_txid, confirmed_index, True)
@@ -88,7 +96,7 @@ class WalletTest(BitcoinTestFramework):
 
         # Have node0 mine a block, thus it will collect its own fee.
         self.nodes[0].generate(1)
-        self.sync_all()
+        self.sync_all([self.nodes[0:3]])
 
         # Exercise locking of unspent outputs
         unspent_0 = self.nodes[2].listunspent()[0]
@@ -101,7 +109,7 @@ class WalletTest(BitcoinTestFramework):
 
         # Have node1 generate 100 blocks (so node0 can recover the fee)
         self.nodes[1].generate(100)
-        self.sync_all()
+        self.sync_all([self.nodes[0:3]])
 
         # node0 should end up with 100 btc in block rewards plus fees, but
         # minus the 21 plus fees sent to node2
@@ -130,7 +138,7 @@ class WalletTest(BitcoinTestFramework):
 
         # Have node1 mine a block to confirm transactions:
         self.nodes[1].generate(1)
-        self.sync_all()
+        self.sync_all([self.nodes[0:3]])
 
         assert_equal(self.nodes[0].getbalance(), 0)
         assert_equal(self.nodes[2].getbalance(), 94)
@@ -142,14 +150,14 @@ class WalletTest(BitcoinTestFramework):
         self.nodes[2].settxfee(fee_per_byte * 1000)
         txid = self.nodes[2].sendtoaddress(address, 10, "", "", False)
         self.nodes[2].generate(1)
-        self.sync_all()
+        self.sync_all([self.nodes[0:3]])
         node_2_bal = self.check_fee_amount(self.nodes[2].getbalance(), Decimal('84'), fee_per_byte, count_bytes(self.nodes[2].getrawtransaction(txid)))
         assert_equal(self.nodes[0].getbalance(), Decimal('10'))
 
         # Send 10 BTC with subtract fee from amount
         txid = self.nodes[2].sendtoaddress(address, 10, "", "", True)
         self.nodes[2].generate(1)
-        self.sync_all()
+        self.sync_all([self.nodes[0:3]])
         node_2_bal -= Decimal('10')
         assert_equal(self.nodes[2].getbalance(), node_2_bal)
         node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), Decimal('20'), fee_per_byte, count_bytes(self.nodes[2].getrawtransaction(txid)))
@@ -157,7 +165,7 @@ class WalletTest(BitcoinTestFramework):
         # Sendmany 10 BTC
         txid = self.nodes[2].sendmany('from1', {address: 10}, 0, "", [])
         self.nodes[2].generate(1)
-        self.sync_all()
+        self.sync_all([self.nodes[0:3]])
         node_0_bal += Decimal('10')
         node_2_bal = self.check_fee_amount(self.nodes[2].getbalance(), node_2_bal - Decimal('10'), fee_per_byte, count_bytes(self.nodes[2].getrawtransaction(txid)))
         assert_equal(self.nodes[0].getbalance(), node_0_bal)
@@ -165,7 +173,7 @@ class WalletTest(BitcoinTestFramework):
         # Sendmany 10 BTC with subtract fee from amount
         txid = self.nodes[2].sendmany('from1', {address: 10}, 0, "", [address])
         self.nodes[2].generate(1)
-        self.sync_all()
+        self.sync_all([self.nodes[0:3]])
         node_2_bal -= Decimal('10')
         assert_equal(self.nodes[2].getbalance(), node_2_bal)
         node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), node_0_bal + Decimal('10'), fee_per_byte, count_bytes(self.nodes[2].getrawtransaction(txid)))
@@ -176,9 +184,9 @@ class WalletTest(BitcoinTestFramework):
         # EXPECT: nodes[3] should have those transactions in its mempool.
         txid1 = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 1)
         txid2 = self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1)
-        sync_mempools(self.nodes)
+        sync_mempools(self.nodes[0:2])
 
-        self.nodes.append(self.start_node(3, self.options.tmpdir, self.extra_args[3]))
+        self.start_node(3)
         connect_nodes_bi(self.nodes, 0, 3)
         sync_blocks(self.nodes)
 
@@ -222,22 +230,24 @@ class WalletTest(BitcoinTestFramework):
 
         #do some -walletbroadcast tests
         self.stop_nodes()
-        self.nodes = self.start_nodes(3, self.options.tmpdir, [["-walletbroadcast=0"],["-walletbroadcast=0"],["-walletbroadcast=0"]])
+        self.start_node(0, ["-walletbroadcast=0"])
+        self.start_node(1, ["-walletbroadcast=0"])
+        self.start_node(2, ["-walletbroadcast=0"])
         connect_nodes_bi(self.nodes,0,1)
         connect_nodes_bi(self.nodes,1,2)
         connect_nodes_bi(self.nodes,0,2)
-        self.sync_all()
+        self.sync_all([self.nodes[0:3]])
 
         txIdNotBroadcasted  = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 2)
         txObjNotBroadcasted = self.nodes[0].gettransaction(txIdNotBroadcasted)
         self.nodes[1].generate(1) #mine a block, tx should not be in there
-        self.sync_all()
+        self.sync_all([self.nodes[0:3]])
         assert_equal(self.nodes[2].getbalance(), node_2_bal) #should not be changed because tx was not broadcasted
 
         #now broadcast from another node, mine a block, sync, and check the balance
         self.nodes[1].sendrawtransaction(txObjNotBroadcasted['hex'])
         self.nodes[1].generate(1)
-        self.sync_all()
+        self.sync_all([self.nodes[0:3]])
         node_2_bal += 2
         txObjNotBroadcasted = self.nodes[0].gettransaction(txIdNotBroadcasted)
         assert_equal(self.nodes[2].getbalance(), node_2_bal)
@@ -247,14 +257,16 @@ class WalletTest(BitcoinTestFramework):
 
         #restart the nodes with -walletbroadcast=1
         self.stop_nodes()
-        self.nodes = self.start_nodes(3, self.options.tmpdir)
+        self.start_node(0)
+        self.start_node(1)
+        self.start_node(2)
         connect_nodes_bi(self.nodes,0,1)
         connect_nodes_bi(self.nodes,1,2)
         connect_nodes_bi(self.nodes,0,2)
-        sync_blocks(self.nodes)
+        sync_blocks(self.nodes[0:3])
 
         self.nodes[0].generate(1)
-        sync_blocks(self.nodes)
+        sync_blocks(self.nodes[0:3])
         node_2_bal += 2
 
         #tx should be added to balance because after restarting the nodes tx should be broadcastet
@@ -285,7 +297,7 @@ class WalletTest(BitcoinTestFramework):
         address_to_import = self.nodes[2].getnewaddress()
         txid = self.nodes[0].sendtoaddress(address_to_import, 1)
         self.nodes[0].generate(1)
-        self.sync_all()
+        self.sync_all([self.nodes[0:3]])
 
         # 2. Import address from node2 to node1
         self.nodes[1].importaddress(address_to_import)
@@ -311,15 +323,15 @@ class WalletTest(BitcoinTestFramework):
         cbAddr = self.nodes[1].getnewaddress()
         blkHash = self.nodes[0].generatetoaddress(1, cbAddr)[0]
         cbTxId = self.nodes[0].getblock(blkHash)['tx'][0]
-        self.sync_all()
+        self.sync_all([self.nodes[0:3]])
 
         # Check that the txid and balance is found by node1
         self.nodes[1].gettransaction(cbTxId)
 
         # check if wallet or blockchain maintenance changes the balance
-        self.sync_all()
+        self.sync_all([self.nodes[0:3]])
         blocks = self.nodes[0].generate(2)
-        self.sync_all()
+        self.sync_all([self.nodes[0:3]])
         balance_nodes = [self.nodes[i].getbalance() for i in range(3)]
         block_count = self.nodes[0].getblockcount()
 
@@ -350,7 +362,9 @@ class WalletTest(BitcoinTestFramework):
             self.log.info("check " + m)
             self.stop_nodes()
             # set lower ancestor limit for later
-            self.nodes = self.start_nodes(3, self.options.tmpdir, [[m, "-limitancestorcount="+str(chainlimit)]] * 3)
+            self.start_node(0, [m, "-limitancestorcount="+str(chainlimit)])
+            self.start_node(1, [m, "-limitancestorcount="+str(chainlimit)])
+            self.start_node(2, [m, "-limitancestorcount="+str(chainlimit)])
             while m == '-reindex' and [block_count] * 3 != [self.nodes[i].getblockcount() for i in range(3)]:
                 # reindex will leave rpc warm up "early"; Wait for it to finish
                 time.sleep(0.1)
@@ -398,7 +412,7 @@ class WalletTest(BitcoinTestFramework):
         # Try with walletrejectlongchains
         # Double chain limit but require combining inputs, so we pass SelectCoinsMinConf
         self.stop_node(0)
-        self.nodes[0] = self.start_node(0, self.options.tmpdir, ["-walletrejectlongchains", "-limitancestorcount="+str(2*chainlimit)])
+        self.start_node(0, extra_args=["-walletrejectlongchains", "-limitancestorcount="+str(2*chainlimit)])
 
         # wait for loadmempool
         timeout = 10

test/functional/walletbackup.py

@@ -37,11 +37,9 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
 class WalletBackupTest(BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
-        self.setup_clean_chain = True
         self.num_nodes = 4
+        self.setup_clean_chain = True
         # nodes 1, 2,3 are spenders, let's give them a keypool=100
         self.extra_args = [["-keypool=100"], ["-keypool=100"], ["-keypool=100"], []]
 
@@ -78,9 +76,9 @@ class WalletBackupTest(BitcoinTestFramework):
 
     # As above, this mirrors the original bash test.
     def start_three(self):
-        self.nodes[0] = self.start_node(0, self.options.tmpdir)
+        self.start_node(0)
-        self.nodes[1] = self.start_node(1, self.options.tmpdir)
+        self.start_node(1)
-        self.nodes[2] = self.start_node(2, self.options.tmpdir)
+        self.start_node(2)
         connect_nodes(self.nodes[0], 3)
         connect_nodes(self.nodes[1], 3)
         connect_nodes(self.nodes[2], 3)

test/functional/zapwallettxes.py

@@ -15,14 +15,14 @@
   been zapped.
 """
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import (assert_equal,
+from test_framework.util import (
+    assert_equal,
     assert_raises_jsonrpc,
-                                 )
+    wait_until,
+)
 
 class ZapWalletTXesTest (BitcoinTestFramework):
-
+    def set_test_params(self):
-    def __init__(self):
-        super().__init__()
         self.setup_clean_chain = True
         self.num_nodes = 2
 
@@ -48,7 +48,7 @@ class ZapWalletTXesTest (BitcoinTestFramework):
 
         # Stop-start node0. Both confirmed and unconfirmed transactions remain in the wallet.
         self.stop_node(0)
-        self.nodes[0] = self.start_node(0, self.options.tmpdir)
+        self.start_node(0)
 
         assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1)
         assert_equal(self.nodes[0].gettransaction(txid2)['txid'], txid2)
@@ -56,7 +56,9 @@ class ZapWalletTXesTest (BitcoinTestFramework):
         # Stop node0 and restart with zapwallettxes and persistmempool. The unconfirmed
         # transaction is zapped from the wallet, but is re-added when the mempool is reloaded.
         self.stop_node(0)
-        self.nodes[0] = self.start_node(0, self.options.tmpdir, ["-persistmempool=1", "-zapwallettxes=2"])
+        self.start_node(0, ["-persistmempool=1", "-zapwallettxes=2"])
+
+        wait_until(lambda: self.nodes[0].getmempoolinfo()['size'] == 1, timeout=3)
 
         assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1)
         assert_equal(self.nodes[0].gettransaction(txid2)['txid'], txid2)
@@ -64,7 +66,7 @@ class ZapWalletTXesTest (BitcoinTestFramework):
         # Stop node0 and restart with zapwallettxes, but not persistmempool.
         # The unconfirmed transaction is zapped and is no longer in the wallet.
         self.stop_node(0)
-        self.nodes[0] = self.start_node(0, self.options.tmpdir, ["-zapwallettxes=2"])
+        self.start_node(0, ["-zapwallettxes=2"])
 
         # tx1 is still be available because it was confirmed
         assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1)