From b2410aeb220db3839159661c8ed6f0505e8fff64 Mon Sep 17 00:00:00 2001
From: random-zebra <random.zebra@protonmail.com>
Date: Sun, 2 May 2021 15:27:12 +0200
Subject: [PATCH 01/17] [BUG] Fix assertion in error in assert_debug_log

---
 test/functional/test_framework/test_node.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
index 9fbbfea92d7b..a52a68bba0c9 100755
--- a/test/functional/test_framework/test_node.py
+++ b/test/functional/test_framework/test_node.py
@@ -223,7 +223,7 @@ def assert_debug_log(self, expected_msgs):
             print_log = " - " + "\n - ".join(log.splitlines())
             for expected_msg in expected_msgs:
                 if re.search(re.escape(expected_msg), log, flags=re.MULTILINE) is None:
-                    self._raise_assertion_error('Expected message "{}" does not partially match log:\n\n{}\n\n'.format(expected_msg, print_log))
+                    raise AssertionError('Expected message "{}" does not partially match log:\n\n{}\n\n'.format(expected_msg, print_log))
 
     def node_encrypt_wallet(self, passphrase):
         """"Encrypts the wallet.

From bd6e591ccb1ac80976504eb26814769a074997bc Mon Sep 17 00:00:00 2001
From: random-zebra <random.zebra@protonmail.com>
Date: Mon, 3 May 2021 11:04:07 +0200
Subject: [PATCH 02/17] [Refactoring] remove SaplingActive check in GUI/RPC

---
 src/qt/pivx/send.cpp                | 16 ++--------------
 src/qt/walletmodel.cpp              | 14 +-------------
 src/qt/walletmodel.h                |  1 -
 src/sapling/transaction_builder.cpp | 12 ++----------
 src/wallet/rpcwallet.cpp            | 11 +++++------
 test/functional/sapling_wallet.py   |  2 +-
 6 files changed, 11 insertions(+), 45 deletions(-)

diff --git a/src/qt/pivx/send.cpp b/src/qt/pivx/send.cpp
index ea1e49d39dde..af8407289cd0 100644
--- a/src/qt/pivx/send.cpp
+++ b/src/qt/pivx/send.cpp
@@ -415,11 +415,6 @@ void SendWidget::ProcessSend(QList<SendCoinsRecipient>& recipients, bool hasShie
     // First check SPORK_20 (before unlock)
     bool isShieldedTx = hasShieldedOutput || !isTransparent;
     if (isShieldedTx) {
-        if (!walletModel->isSaplingEnforced()) {
-            inform(tr("Cannot perform shielded operations, v5 upgrade isn't being enforced yet!"));
-            return;
-        }
-
         if (walletModel->isSaplingInMaintenance()) {
             inform(tr("Sapling Protocol temporarily in maintenance. Shielded transactions disabled (SPORK 20)"));
             return;
@@ -702,8 +697,8 @@ void SendWidget::onCoinControlClicked()
 
 void SendWidget::onShieldCoinsClicked()
 {
-    if (!walletModel->isSaplingEnforced()) {
-        inform(tr("Cannot perform shielded operations, v5 upgrade isn't being enforced yet!"));
+    if (walletModel->isSaplingInMaintenance()) {
+        inform(tr("Sapling Protocol temporarily in maintenance. Shielded transactions disabled (SPORK 20)"));
         return;
     }
 
@@ -792,13 +787,6 @@ void SendWidget::onCheckBoxChanged()
 void SendWidget::onPIVSelected(bool _isTransparent)
 {
     isTransparent = _isTransparent;
-
-    if (!isTransparent && !walletModel->isSaplingEnforced()) {
-        ui->pushLeft->setChecked(true);
-        inform(tr("Cannot perform shielded operations, v5 upgrade isn't being enforced yet!"));
-        return;
-    }
-
     resetChangeAddress();
     resetCoinControl();
     tryRefreshAmounts();
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index 50ca2957f36c..796265bf4a99 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -85,11 +85,6 @@ bool WalletModel::isSaplingInMaintenance() const
     return sporkManager.IsSporkActive(SPORK_20_SAPLING_MAINTENANCE);
 }
 
-bool WalletModel::isSaplingEnforced() const
-{
-    return Params().GetConsensus().NetworkUpgradeActive(cachedNumBlocks, Consensus::UPGRADE_V5_0);
-}
-
 bool WalletModel::isV6Enforced() const
 {
     return Params().GetConsensus().NetworkUpgradeActive(cachedNumBlocks, Consensus::UPGRADE_V6_0);
@@ -603,14 +598,6 @@ OperationResult WalletModel::PrepareShieldedTransaction(WalletModelTransaction*
                                                         bool fromTransparent,
                                                         const CCoinControl* coinControl)
 {
-    // Basic checks first
-
-    // Check network status
-    int nextBlockHeight = cachedNumBlocks + 1;
-    if (!Params().GetConsensus().NetworkUpgradeActive(nextBlockHeight, Consensus::UPGRADE_V5_0)) {
-        return errorOut("Error, cannot send transaction. Sapling is not activated");
-    }
-
     // Load shieldedAddrRecipients.
     std::vector<SendManyRecipient> recipients;
     for (const auto& recipient : modelTransaction->getRecipients()) {
@@ -630,6 +617,7 @@ OperationResult WalletModel::PrepareShieldedTransaction(WalletModelTransaction*
     if (!opResult) return opResult;
 
     // Create the operation
+    int nextBlockHeight = cachedNumBlocks + 1;
     SaplingOperation operation(Params().GetConsensus(), nextBlockHeight, wallet);
     auto operationResult = operation.setRecipients(recipients)
              ->setTransparentKeyChange(modelTransaction->getPossibleKeyChange())
diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h
index f7808e111edf..b5977b7ae2c0 100644
--- a/src/qt/walletmodel.h
+++ b/src/qt/walletmodel.h
@@ -155,7 +155,6 @@ class WalletModel : public QObject
     /** Whether cold staking is enabled or disabled in the network **/
     bool isColdStakingNetworkelyEnabled() const;
     bool isSaplingInMaintenance() const;
-    bool isSaplingEnforced() const;
     bool isV6Enforced() const;
     CAmount getMinColdStakingAmount() const;
     /* current staking status from the miner thread **/
diff --git a/src/sapling/transaction_builder.cpp b/src/sapling/transaction_builder.cpp
index 90dba6c01e3c..80acb9547553 100644
--- a/src/sapling/transaction_builder.cpp
+++ b/src/sapling/transaction_builder.cpp
@@ -129,15 +129,6 @@ std::string TransactionBuilderResult::GetError() {
     }
 }
 
-// Set default values of new CMutableTransaction based on consensus rules at given height.
-CMutableTransaction CreateNewContextualCMutableTransaction(const Consensus::Params& consensusParams, int nHeight)
-{
-    CMutableTransaction mtx;
-    bool isSapling = consensusParams.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V5_0);
-    mtx.nVersion = isSapling ? CTransaction::TxVersion::SAPLING : CTransaction::TxVersion::LEGACY;
-    return mtx;
-}
-
 TransactionBuilder::TransactionBuilder(
     const Consensus::Params& _consensusParams,
     int _nHeight,
@@ -151,7 +142,8 @@ TransactionBuilder::TransactionBuilder(
 
 void TransactionBuilder::Clear()
 {
-    mtx = CreateNewContextualCMutableTransaction(consensusParams, nHeight);
+    mtx = CMutableTransaction();
+    mtx.nVersion = CTransaction::TxVersion::SAPLING;
     spends.clear();
     outputs.clear();
     tIns.clear();
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index e75e86d20939..d2af659d3def 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -1168,8 +1168,8 @@ UniValue CreateColdStakeDelegation(const UniValue& params, CTransactionRef& txNe
         const Consensus::Params& consensus = Params().GetConsensus();
         // Check network status
         int nextBlockHeight = chainActive.Height() + 1;
-        if (!consensus.NetworkUpgradeActive(nextBlockHeight, Consensus::UPGRADE_V5_0)) {
-            throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling not active yet");
+        if (sporkManager.IsSporkActive(SPORK_20_SAPLING_MAINTENANCE)) {
+            throw JSONRPCError(RPC_INVALID_PARAMETER, "SHIELD in maintenance (SPORK 20)");
         }
         std::vector<SendManyRecipient> recipients = {SendManyRecipient(ownerKey, *stakeKey, nValue, fV6Enforced)};
         SaplingOperation operation(consensus, nextBlockHeight, pwalletMain);
@@ -1626,11 +1626,10 @@ static SaplingOperation CreateShieldedTransaction(const JSONRPCRequest& request)
     }
 
     // Check network status
-    if (!Params().GetConsensus().NetworkUpgradeActive(nextBlockHeight, Consensus::UPGRADE_V5_0) ||
-            sporkManager.IsSporkActive(SPORK_20_SAPLING_MAINTENANCE)) {
-        // If Sapling is not active, do not allow sending from or sending to Sapling addresses.
+    if (sporkManager.IsSporkActive(SPORK_20_SAPLING_MAINTENANCE)) {
+        // If Sapling is disabled, do not allow sending from or sending to Sapling addresses.
         if (fromSapling || containsSaplingOutput) {
-            throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling not active yet");
+            throw JSONRPCError(RPC_INVALID_PARAMETER, "SHIELD in maintenance (SPORK 20)");
         }
     }
 
diff --git a/test/functional/sapling_wallet.py b/test/functional/sapling_wallet.py
index 9b05430cc947..d57f4a86d656 100755
--- a/test/functional/sapling_wallet.py
+++ b/test/functional/sapling_wallet.py
@@ -113,7 +113,7 @@ def run_test(self):
         self.log.info("Good. Not accepted when SPORK_20 is active.")
 
         # Try with RPC...
-        assert_raises_rpc_error(-8, "Invalid parameter, Sapling not active yet",
+        assert_raises_rpc_error(-8, "SHIELD in maintenance (SPORK 20)",
                                 self.nodes[0].shieldsendmany, "from_transparent", recipients, 1, fee)
 
         # Disable SPORK_20 and retry

From 9d552f94d3a1169afc14491dab0b85ddaa2ebe0e Mon Sep 17 00:00:00 2001
From: random-zebra <random.zebra@protonmail.com>
Date: Mon, 3 May 2021 10:29:33 +0200
Subject: [PATCH 03/17] [Refactor][Tests] Do not change chain when not needed

---
 src/test/librust/sapling_rpc_wallet_tests.cpp | 92 ++++++++-----------
 src/test/script_P2CS_tests.cpp                | 10 +-
 src/test/validation_tests.cpp                 | 21 +----
 3 files changed, 48 insertions(+), 75 deletions(-)

diff --git a/src/test/librust/sapling_rpc_wallet_tests.cpp b/src/test/librust/sapling_rpc_wallet_tests.cpp
index 3e1e1da3e4a3..3e17d198d231 100644
--- a/src/test/librust/sapling_rpc_wallet_tests.cpp
+++ b/src/test/librust/sapling_rpc_wallet_tests.cpp
@@ -85,8 +85,6 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_sapling_validateaddress)
 
 BOOST_AUTO_TEST_CASE(rpc_wallet_getbalance)
 {
-    SelectParams(CBaseChainParams::TESTNET);
-
     {
         LOCK(pwalletMain->cs_wallet);
         pwalletMain->SetMinVersion(FEATURE_SAPLING);
@@ -96,9 +94,9 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_getbalance)
     BOOST_CHECK_THROW(CallRPC("getshieldbalance too many args"), std::runtime_error);
     BOOST_CHECK_THROW(CallRPC("getshieldbalance invalidaddress"), std::runtime_error);
     BOOST_CHECK_THROW(CallRPC("getshieldbalance tmC6YZnCUhm19dEXxh3Jb7srdBJxDawaCab"), std::runtime_error);
-    BOOST_CHECK_NO_THROW(CallRPC("getshieldbalance ptestsapling1h0w73csah2aq0a32h42kr7tq4htlt5wfn4ejxfnm56f6ehjvek7k4e244g6v8v3pgylmz5ea8jh"));
-    BOOST_CHECK_THROW(CallRPC("getshieldbalance ptestsapling1h0w73csah2aq0a32h42kr7tq4htlt5wfn4ejxfnm56f6ehjvek7k4e244g6v8v3pgylmz5ea8jh -1"), std::runtime_error);
-    BOOST_CHECK_NO_THROW(CallRPC("getshieldbalance ptestsapling1nrn6exksuqtpld9gu6fwdz4hwg54h2x37gutdds89pfyg6mtjf63km45a8eare5qla45cj75vs8 0"));
+    BOOST_CHECK_NO_THROW(CallRPC("getshieldbalance ps1u87kylcmn28yclnx2uy0psnvuhs2xn608ukm6n2nshrpg2nzyu3n62ls8j77m9cgp40dx40evej"));
+    BOOST_CHECK_THROW(CallRPC("getshieldbalance ps1u87kylcmn28yclnx2uy0psnvuhs2xn608ukm6n2nshrpg2nzyu3n62ls8j77m9cgp40dx40evej -1"), std::runtime_error);
+    BOOST_CHECK_NO_THROW(CallRPC("getshieldbalance ps1u87kylcmn28yclnx2uy0psnvuhs2xn608ukm6n2nshrpg2nzyu3n62ls8j77m9cgp40dx40evej 0"));
     BOOST_CHECK_THROW(CallRPC("getshieldbalance tnRZ8bPq2pff3xBWhTJhNkVUkm2uhzksDeW5PvEa7aFKGT9Qi3YgTALZfjaY4jU3HLVKBtHdSXxoPoLA3naMPcHBcY88FcF 1"), std::runtime_error);
     BOOST_CHECK_NO_THROW(CallRPC("getshieldbalance *"));
     BOOST_CHECK_NO_THROW(CallRPC("getshieldbalance * 6"));
@@ -106,15 +104,15 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_getbalance)
 
     BOOST_CHECK_THROW(CallRPC("listreceivedbyshieldaddress too many args"), std::runtime_error);
     // negative minconf not allowed
-    BOOST_CHECK_THROW(CallRPC("listreceivedbyshieldaddress yBYhwgzufrZ6F5VVuK9nEChENArq934mqC -1"), std::runtime_error);
+    BOOST_CHECK_THROW(CallRPC("listreceivedbyshieldaddress DMKU6mc52un1MThGCsnNwAtEvncaTdAuaZ -1"), std::runtime_error);
     // invalid zaddr, taddr not allowed
-    BOOST_CHECK_THROW(CallRPC("listreceivedbyshieldaddress yBYhwgzufrZ6F5VVuK9nEChENArq934mqC 0"), std::runtime_error);
+    BOOST_CHECK_THROW(CallRPC("listreceivedbyshieldaddress DMKU6mc52un1MThGCsnNwAtEvncaTdAuaZ 0"), std::runtime_error);
     // don't have the spending key
-    BOOST_CHECK_THROW(CallRPC("listreceivedbyshieldaddress ptestsapling1nrn6exksuqtpld9gu6fwdz4hwg54h2x37gutdds89pfyg6mtjf63km45a8eare5qla45cj75vs8 1"), std::runtime_error);
+    BOOST_CHECK_THROW(CallRPC("listreceivedbyshieldaddress ps1u87kylcmn28yclnx2uy0psnvuhs2xn608ukm6n2nshrpg2nzyu3n62ls8j77m9cgp40dx40evej 1"), std::runtime_error);
 }
 
-BOOST_AUTO_TEST_CASE(rpc_wallet_sapling_importkey_paymentaddress) {
-    SelectParams(CBaseChainParams::MAIN);
+BOOST_AUTO_TEST_CASE(rpc_wallet_sapling_importkey_paymentaddress)
+{
     {
         LOCK(pwalletMain->cs_wallet);
         pwalletMain->SetMinVersion(FEATURE_SAPLING);
@@ -234,8 +232,8 @@ void CheckHaveAddr(const libzcash::PaymentAddress& addr) {
     BOOST_CHECK(pwalletMain->HaveSpendingKeyForPaymentAddress(*addr_of_type));
 }
 
-BOOST_AUTO_TEST_CASE(rpc_wallet_getnewshieldaddress) {
-    UniValue addr;
+BOOST_AUTO_TEST_CASE(rpc_wallet_getnewshieldaddress)
+{
     {
         LOCK(pwalletMain->cs_wallet);
         pwalletMain->SetMinVersion(FEATURE_SAPLING);
@@ -243,7 +241,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_getnewshieldaddress) {
     }
 
     // No parameter defaults to sapling address
-    addr = CallRPC("getnewshieldaddress");
+    UniValue addr = CallRPC("getnewshieldaddress");
     CheckHaveAddr(KeyIO::DecodePaymentAddress(addr.get_str()));
     // Too many arguments will throw with the help
     BOOST_CHECK_THROW(CallRPC("getnewshieldaddress many args"), std::runtime_error);
@@ -251,8 +249,6 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_getnewshieldaddress) {
 
 BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_parameters)
 {
-    SelectParams(CBaseChainParams::TESTNET);
-
     {
         LOCK(pwalletMain->cs_wallet);
         pwalletMain->SetMinVersion(FEATURE_SAPLING);
@@ -265,41 +261,40 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_parameters)
 
     // bad from address
     BOOST_CHECK_THROW(CallRPC("shieldsendmany "
-                              "INVALIDyBYhwgzufrZ6F5VVuK9nEChENArq934mqC []"), std::runtime_error);
+                              "INVALIDDMKU6mc52un1MThGCsnNwAtEvncaTdAuaZ []"), std::runtime_error);
     // empty amounts
     BOOST_CHECK_THROW(CallRPC("shieldsendmany "
-                              "yBYhwgzufrZ6F5VVuK9nEChENArq934mqC []"), std::runtime_error);
+                              "DMKU6mc52un1MThGCsnNwAtEvncaTdAuaZ []"), std::runtime_error);
 
     // don't have the spending key for this address
     BOOST_CHECK_THROW(CallRPC("shieldsendmany "
-                              "ptestsapling1wpurflqllgkcs48m46yu9ktlfe3ahndely20dpaanqq3lw9l5xw7yfehst68yclvlpz7x8cltxe"
-                              "UkJ1oSfbhTJhm72WiZizvkZz5aH1 []"), std::runtime_error);
+                              "ps1u87kylcmn28yclnx2uy0psnvuhs2xn608ukm6n2nshrpg2nzyu3n62ls8j77m9cgp40dx40evej []"), std::runtime_error);
 
     // duplicate address
     BOOST_CHECK_THROW(CallRPC("shieldsendmany "
-                              "yBYhwgzufrZ6F5VVuK9nEChENArq934mqC "
-                              "[{\"address\":\"yAJ4bGeDFcEtx24kbr413fBLpWQcdR5F2z\", \"amount\":50.0},"
-                              " {\"address\":\"yAJ4bGeDFcEtx24kbr413fBLpWQcdR5F2z\", \"amount\":12.0} ]"
+                              "DDTBEPEaub5sk31mUifiv5nHGXtHGnuAJc "
+                              "[{\"address\":\"DMKU6mc52un1MThGCsnNwAtEvncaTdAuaZ\", \"amount\":50.0},"
+                              " {\"address\":\"DMKU6mc52un1MThGCsnNwAtEvncaTdAuaZ\", \"amount\":12.0} ]"
     ), std::runtime_error);
 
     // invalid fee amount, cannot be negative
     BOOST_CHECK_THROW(CallRPC("shieldsendmany "
-                              "yBYhwgzufrZ6F5VVuK9nEChENArq934mqC "
-                              "[{\"address\":\"yAJ4bGeDFcEtx24kbr413fBLpWQcdR5F2z\", \"amount\":50.0}] "
+                              "DDTBEPEaub5sk31mUifiv5nHGXtHGnuAJc "
+                              "[{\"address\":\"DMKU6mc52un1MThGCsnNwAtEvncaTdAuaZ\", \"amount\":50.0}] "
                               "1 -0.0001"
     ), std::runtime_error);
 
     // invalid fee amount, bigger than MAX_MONEY
     BOOST_CHECK_THROW(CallRPC("shieldsendmany "
-                              "yBYhwgzufrZ6F5VVuK9nEChENArq934mqC "
-                              "[{\"address\":\"yAJ4bGeDFcEtx24kbr413fBLpWQcdR5F2z\", \"amount\":50.0}] "
+                              "DDTBEPEaub5sk31mUifiv5nHGXtHGnuAJc "
+                              "[{\"address\":\"DMKU6mc52un1MThGCsnNwAtEvncaTdAuaZ\", \"amount\":50.0}] "
                               "1 21000001"
     ), std::runtime_error);
 
     // fee amount is bigger than sum of outputs
     BOOST_CHECK_THROW(CallRPC("shieldsendmany "
-                              "yBYhwgzufrZ6F5VVuK9nEChENArq934mqC "
-                              "[{\"address\":\"yAJ4bGeDFcEtx24kbr413fBLpWQcdR5F2z\", \"amount\":50.0}] "
+                              "DDTBEPEaub5sk31mUifiv5nHGXtHGnuAJc "
+                              "[{\"address\":\"DMKU6mc52un1MThGCsnNwAtEvncaTdAuaZ\", \"amount\":50.0}] "
                               "1 50.00000001"
     ), std::runtime_error);
 
@@ -309,18 +304,18 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_parameters)
     std::string badmemo(v.begin(), v.end());
     auto pa = pwalletMain->GenerateNewSaplingZKey();
     std::string zaddr1 = KeyIO::EncodePaymentAddress(pa);
-    BOOST_CHECK_THROW(CallRPC(std::string("shieldsendmany yBYhwgzufrZ6F5VVuK9nEChENArq934mqC ")
+    BOOST_CHECK_THROW(CallRPC(std::string("shieldsendmany DMKU6mc52un1MThGCsnNwAtEvncaTdAuaZ ")
                               + "[{\"address\":\"" + zaddr1 + "\", \"amount\":123.456}]"), std::runtime_error);
 }
 
-// TODO: test private methods
-BOOST_AUTO_TEST_CASE(saplingOperationTests) {
-    RegtestActivateSapling();
-    auto consensusParams = Params().GetConsensus();
-
-    LOCK2(cs_main, pwalletMain->cs_wallet);
-    pwalletMain->SetupSPKM(false);
+BOOST_AUTO_TEST_CASE(saplingOperationTests)
+{
+    {
+        LOCK2(cs_main, pwalletMain->cs_wallet);
+        pwalletMain->SetupSPKM(false);
+    }
 
+    auto consensusParams = Params().GetConsensus();
     UniValue retValue;
 
     // add keys manually
@@ -388,17 +383,15 @@ BOOST_AUTO_TEST_CASE(saplingOperationTests) {
         const std::string& errStr = res.getError();
         BOOST_CHECK(errStr.find("too big") != std::string::npos);
     }
-    RegtestDeactivateSapling();
 }
 
 
 BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_taddr_to_sapling)
 {
-    SelectParams(CBaseChainParams::REGTEST);
-    RegtestActivateSapling();
-
-    LOCK2(cs_main, pwalletMain->cs_wallet);
-    pwalletMain->SetupSPKM(false);
+    {
+        LOCK2(cs_main, pwalletMain->cs_wallet);
+        pwalletMain->SetupSPKM(false);
+    }
 
     UniValue retValue;
 
@@ -477,9 +470,6 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_taddr_to_sapling)
     // Tear down
     chainActive.SetTip(nullptr);
     mapBlockIndex.erase(blockHash);
-
-    // Revert to default
-    RegtestDeactivateSapling();
 }
 
 BOOST_AUTO_TEST_CASE(rpc_wallet_encrypted_wallet_sapzkeys)
@@ -546,8 +536,6 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_encrypted_wallet_sapzkeys)
 
 BOOST_AUTO_TEST_CASE(rpc_listshieldunspent_parameters)
 {
-    SelectParams(CBaseChainParams::TESTNET);
-
     {
         LOCK(pwalletMain->cs_wallet);
         pwalletMain->SetupSPKM(false);
@@ -568,19 +556,19 @@ BOOST_AUTO_TEST_CASE(rpc_listshieldunspent_parameters)
     BOOST_CHECK_THROW(CallRPC("listshieldunspent 1 9999999999"), std::runtime_error);
 
     // must be an array of addresses
-    BOOST_CHECK_THROW(CallRPC("listshieldunspent 1 999 false ptestsapling1wpurflqllgkcs48m46yu9ktlfe3ahndely20dpaanqq3lw9l5xw7yfehst68yclvlpz7x8cltxe"), std::runtime_error);
+    BOOST_CHECK_THROW(CallRPC("listshieldunspent 1 999 false ps1u87kylcmn28yclnx2uy0psnvuhs2xn608ukm6n2nshrpg2nzyu3n62ls8j77m9cgp40dx40evej"), std::runtime_error);
 
     // address must be string
     BOOST_CHECK_THROW(CallRPC("listshieldunspent 1 999 false [123456]"), std::runtime_error);
 
     // no spending key
-    BOOST_CHECK_THROW(CallRPC("listshieldunspent 1 999 false [\"ptestsapling1wpurflqllgkcs48m46yu9ktlfe3ahndely20dpaanqq3lw9l5xw7yfehst68yclvlpz7x8cltxe\"]"), std::runtime_error);
+    BOOST_CHECK_THROW(CallRPC("listshieldunspent 1 999 false [\"ps1u87kylcmn28yclnx2uy0psnvuhs2xn608ukm6n2nshrpg2nzyu3n62ls8j77m9cgp40dx40evej\"]"), std::runtime_error);
 
     // allow watch only
-    BOOST_CHECK_NO_THROW(CallRPC("listshieldunspent 1 999 true [\"ptestsapling1wpurflqllgkcs48m46yu9ktlfe3ahndely20dpaanqq3lw9l5xw7yfehst68yclvlpz7x8cltxe\"]"));
+    BOOST_CHECK_NO_THROW(CallRPC("listshieldunspent 1 999 true [\"ps1u87kylcmn28yclnx2uy0psnvuhs2xn608ukm6n2nshrpg2nzyu3n62ls8j77m9cgp40dx40evej\"]"));
 
-    // wrong network, mainnet instead of testnet
-    BOOST_CHECK_THROW(CallRPC("listshieldunspent 1 999 true [\"ps1qenk9kapr0crx7lmdl4yclx78spc36wh7d5hm9hglp85f43k9dupyf0c5836h42wq2ejv0ef2v3\"]"), std::runtime_error);
+    // wrong network, testnet/regtest instead of mainnet
+    BOOST_CHECK_THROW(CallRPC("listshieldunspent 1 999 true [\"ptestsapling1wpurflqllgkcs48m46yu9ktlfe3ahndely20dpaanqq3lw9l5xw7yfehst68yclvlpz7x8cltxe\"]"), std::runtime_error);
 
     // create shielded address so we have the spending key
     BOOST_CHECK_NO_THROW(retValue = CallRPC("getnewshieldaddress"));
diff --git a/src/test/script_P2CS_tests.cpp b/src/test/script_P2CS_tests.cpp
index 45889f4166ea..7385aac42feb 100644
--- a/src/test/script_P2CS_tests.cpp
+++ b/src/test/script_P2CS_tests.cpp
@@ -59,8 +59,8 @@ BOOST_AUTO_TEST_CASE(extract_cold_staking_destination_keys)
 
 static CScript GetNewP2CS(CKey& stakerKey, CKey& ownerKey, bool fLastOutFree)
 {
-    stakerKey = DecodeSecret("91yo52JPHDVUG3jXWLKGyzEdjn1a9nbnurLdmQEf2UzbgzkTc2c");
-    ownerKey = DecodeSecret("92KgNFNfmVVJRQuzssETc7NhwufGuHsLvPQxW9Nwmxs7PB4ByWB");
+    stakerKey = DecodeSecret("YNdsth3BsW53DYmCiR12SofWSAt2utXQUSGoin3PekVQCMbzfS7E");
+    ownerKey = DecodeSecret("YUo8oW3y8cUQdQxQxCdnUJ4Ww5H7nHBEMwD2bNDpBbuLM59t4rvd");
     return fLastOutFree ? GetScriptForStakeDelegationLOF(stakerKey.GetPubKey().GetID(), ownerKey.GetPubKey().GetID())
                         : GetScriptForStakeDelegation(stakerKey.GetPubKey().GetID(), ownerKey.GetPubKey().GetID());
 }
@@ -124,7 +124,6 @@ static bool CheckP2CSScript(const CScript& scriptSig, const CScript& scriptPubKe
 
 BOOST_AUTO_TEST_CASE(coldstake_lof_script)
 {
-    SelectParams(CBaseChainParams::REGTEST);
     CScript scriptP2CS;
     CKey stakerKey, ownerKey;
 
@@ -150,7 +149,7 @@ BOOST_AUTO_TEST_CASE(coldstake_lof_script)
     SignColdStake(tx, 0, scriptP2CS, stakerKey, true);
     BOOST_CHECK(CheckP2CSScript(tx.vin[0].scriptSig, scriptP2CS, tx, err));
 
-    const CKey& dummyKey = DecodeSecret("91t7cwPGevo885Uccg87nVjzUxKhXta9JprHM3R21PQkBFMFg2i");
+    const CKey& dummyKey = DecodeSecret("YNdsth3BsW53DYmCiR12SofWSAt2utXQUSGoin3PekVQCMbzfS7E");
     const CKeyID& dummyKeyID = dummyKey.GetPubKey().GetID();
     const CScript& dummyP2PKH = GetDummyP2PKH(dummyKeyID);
 
@@ -201,7 +200,6 @@ BOOST_AUTO_TEST_CASE(coldstake_lof_script)
 
 BOOST_AUTO_TEST_CASE(coldstake_script)
 {
-    SelectParams(CBaseChainParams::REGTEST);
     CScript scriptP2CS;
     CKey stakerKey, ownerKey;
 
@@ -227,7 +225,7 @@ BOOST_AUTO_TEST_CASE(coldstake_script)
     SignColdStake(tx, 0, scriptP2CS, stakerKey, true);
     BOOST_CHECK(CheckP2CSScript(tx.vin[0].scriptSig, scriptP2CS, tx, err));
 
-    const CKey& dummyKey = DecodeSecret("91t7cwPGevo885Uccg87nVjzUxKhXta9JprHM3R21PQkBFMFg2i");
+    const CKey& dummyKey = DecodeSecret("YNdsth3BsW53DYmCiR12SofWSAt2utXQUSGoin3PekVQCMbzfS7E");
     const CKeyID& dummyKeyID = dummyKey.GetPubKey().GetID();
     const CScript& dummyP2PKH = GetDummyP2PKH(dummyKeyID);
 
diff --git a/src/test/validation_tests.cpp b/src/test/validation_tests.cpp
index 5ea84f423215..5cca80bd8d76 100644
--- a/src/test/validation_tests.cpp
+++ b/src/test/validation_tests.cpp
@@ -13,8 +13,10 @@
 
 BOOST_FIXTURE_TEST_SUITE(validation_tests, TestingSetup)
 
-void test_simple_sapling_invalidity(CMutableTransaction& tx)
+BOOST_AUTO_TEST_CASE(test_simple_shielded_invalid)
 {
+    CMutableTransaction tx;
+    tx.nVersion = CTransaction::TxVersion::SAPLING;
     CAmount nDummyValueOut;
     {
         CMutableTransaction newTx(tx);
@@ -91,22 +93,6 @@ void test_simple_sapling_invalidity(CMutableTransaction& tx)
     }
 }
 
-BOOST_AUTO_TEST_CASE(test_simple_shielded_invalid)
-{
-    // Switch to regtest parameters so we can activate Sapling
-    SelectParams(CBaseChainParams::REGTEST);
-
-    CMutableTransaction mtx;
-    mtx.nVersion = CTransaction::TxVersion::SAPLING;
-
-    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_V5_0, Consensus::NetworkUpgrade::ALWAYS_ACTIVE);
-    test_simple_sapling_invalidity(mtx);
-    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_V5_0, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT);
-
-    // Switch back to mainnet parameters as originally selected in test fixture
-    SelectParams(CBaseChainParams::MAIN);
-}
-
 void CheckBlockZcRejection(const std::shared_ptr<CBlock>& pblock, CMutableTransaction& mtx)
 {
     pblock->vtx.emplace_back(MakeTransactionRef(mtx));
@@ -129,6 +115,7 @@ void CheckMempoolZcRejection(CMutableTransaction& mtx)
 
 BOOST_AUTO_TEST_CASE(zerocoin_rejection_tests)
 {
+    // !TODO: fix me
     SelectParams(CBaseChainParams::REGTEST);
     UpdateNetworkUpgradeParameters(Consensus::UPGRADE_V5_0, Consensus::NetworkUpgrade::ALWAYS_ACTIVE);
     const CChainParams& chainparams = Params();

From f5f72cabab86334872df7ce1d5e14c05702461eb Mon Sep 17 00:00:00 2001
From: random-zebra <random.zebra@protonmail.com>
Date: Sun, 2 May 2021 15:26:42 +0200
Subject: [PATCH 04/17] [Trivial] Update some state logs (tx oversize and
 inputs missing/spent)

---
 src/consensus/tx_verify.cpp | 2 +-
 src/validation.cpp          | 7 +++----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp
index f8d9d0f4115c..58ecb2b1e8b6 100644
--- a/src/consensus/tx_verify.cpp
+++ b/src/consensus/tx_verify.cpp
@@ -74,7 +74,7 @@ bool CheckTransaction(const CTransaction& tx, CValidationState& state, bool fCol
     static_assert(MAX_TX_SIZE_AFTER_SAPLING > MAX_ZEROCOIN_TX_SIZE, "New max TX size must be bigger than old max TX size");  // sanity
     const unsigned int nMaxSize = tx.IsShieldedTx() ? MAX_TX_SIZE_AFTER_SAPLING : MAX_ZEROCOIN_TX_SIZE;
     if (tx.GetTotalSize() > nMaxSize) {
-        return state.DoS(10, false, REJECT_INVALID, "bad-txns-oversize");
+        return state.DoS(10, error("tx oversize: %d > %d", tx.GetTotalSize(), nMaxSize), REJECT_INVALID, "bad-txns-oversize");
     }
 
     // Dispatch to Sapling validator
diff --git a/src/validation.cpp b/src/validation.cpp
index db625a30b883..553eca535262 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1644,10 +1644,9 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
             }
 
         } else if (!tx.IsCoinBase()) {
-            if (!view.HaveInputs(tx))
-                return state.DoS(100, error("ConnectBlock() : inputs missing/spent"),
-                    REJECT_INVALID, "bad-txns-inputs-missingorspent");
-
+            if (!view.HaveInputs(tx)) {
+                return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputs-missingorspent");
+            }
             // Sapling: are the sapling spends' requirements met in tx(valid anchors/nullifiers)?
             if (!view.HaveShieldedRequirements(tx))
                 return state.DoS(100, error("%s: spends requirements not met", __func__),

From 0b670c872d99e848be58914a9215fbe974722d3a Mon Sep 17 00:00:00 2001
From: random-zebra <random.zebra@protonmail.com>
Date: Mon, 3 May 2021 17:55:38 +0200
Subject: [PATCH 05/17] [BUG] Properly recompute coinbase and sapling root hash
 in CreateBlock

In TestChainSetup::CreateBlock, the miner could have been credited with
the fees spent by included mempool txes. Since those transactions are
removed, we need to restore the coinbase out amount to the block value.

Further, since the block could have included shield txes from the
mempool, now removed, and/or shield txes passed from the caller, now
included, we need to update the sapling root.
---
 src/test/test_pivx.cpp | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/src/test/test_pivx.cpp b/src/test/test_pivx.cpp
index 732fce7474d6..7d9e8a3775a1 100644
--- a/src/test/test_pivx.cpp
+++ b/src/test/test_pivx.cpp
@@ -159,15 +159,24 @@ CBlock TestChainSetup::CreateBlock(const std::vector<CMutableTransaction>& txns,
 
     // Replace mempool-selected txns with just coinbase plus passed-in txns:
     pblock->vtx.resize(1);
-    for (const CMutableTransaction& tx : txns)
+    for (const CMutableTransaction& tx : txns) {
         pblock->vtx.push_back(MakeTransactionRef(tx));
+    }
+
+    const CBlockIndex* pindexPrev = WITH_LOCK(cs_main, return chainActive.Tip());
+    const int nHeight = pindexPrev->nHeight + 1;
+
+    // Replace coinbase output amount (could have included fee in CreateNewBlock)
+    CMutableTransaction txCoinbase(*pblock->vtx[0]);
+    txCoinbase.vout[0].nValue = GetBlockValue(nHeight);
+    pblock->vtx[0] = MakeTransactionRef(txCoinbase);
 
     // IncrementExtraNonce creates a valid coinbase and merkleRoot
     unsigned int extraNonce = 0;
-    {
-        LOCK(cs_main);
-        IncrementExtraNonce(pblock, chainActive.Tip(), extraNonce);
-    }
+    IncrementExtraNonce(pblock, pindexPrev, extraNonce);
+
+    // Re-compute sapling root
+    pblock->hashFinalSaplingRoot = CalculateSaplingTreeRoot(pblock.get(), nHeight, Params());
 
     while (!CheckProofOfWork(pblock->GetHash(), pblock->nBits)) ++pblock->nNonce;
     return *pblock;

From 44a1c44aba7b08b56b4e591858c46086d30725b1 Mon Sep 17 00:00:00 2001
From: random-zebra <random.zebra@protonmail.com>
Date: Mon, 3 May 2021 18:07:56 +0200
Subject: [PATCH 06/17] [Refactor] Option to mine mempool txes in
 TestChainSetup::CreateBlock

---
 src/test/test_pivx.cpp | 32 ++++++++++++++++----------------
 src/test/test_pivx.h   |  8 ++++----
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/src/test/test_pivx.cpp b/src/test/test_pivx.cpp
index 7d9e8a3775a1..0d9026628a33 100644
--- a/src/test/test_pivx.cpp
+++ b/src/test/test_pivx.cpp
@@ -133,13 +133,11 @@ TestChainSetup::TestChainSetup(int blockCount) : TestingSetup(CBaseChainParams::
     }
 }
 
-//
-// Create a new block with just given transactions, coinbase paying to
-// scriptPubKey, and try to add it to the current chain.
-//
-CBlock TestChainSetup::CreateAndProcessBlock(const std::vector<CMutableTransaction>& txns, const CScript& scriptPubKey)
+// Create a new block with coinbase paying to scriptPubKey, and try to add it to the current chain.
+// Include given transactions, and, if fNoMempoolTx=true, remove transactions coming from the mempool.
+CBlock TestChainSetup::CreateAndProcessBlock(const std::vector<CMutableTransaction>& txns, const CScript& scriptPubKey, bool fNoMempoolTx)
 {
-    CBlock block = CreateBlock(txns, scriptPubKey);
+    CBlock block = CreateBlock(txns, scriptPubKey, fNoMempoolTx);
     CValidationState state;
     ProcessNewBlock(state, nullptr, std::make_shared<const CBlock>(block), nullptr);
     return block;
@@ -151,26 +149,28 @@ CBlock TestChainSetup::CreateAndProcessBlock(const std::vector<CMutableTransacti
     return CreateAndProcessBlock(txns, scriptPubKey);
 }
 
-CBlock TestChainSetup::CreateBlock(const std::vector<CMutableTransaction>& txns, const CScript& scriptPubKey)
+CBlock TestChainSetup::CreateBlock(const std::vector<CMutableTransaction>& txns, const CScript& scriptPubKey, bool fNoMempoolTx)
 {
     std::unique_ptr<CBlockTemplate> pblocktemplate = BlockAssembler(
             Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, nullptr, false);
     std::shared_ptr<CBlock> pblock = std::make_shared<CBlock>(pblocktemplate->block);
 
+    const CBlockIndex* pindexPrev = WITH_LOCK(cs_main, return chainActive.Tip());
+    const int nHeight = pindexPrev->nHeight + 1;
+
     // Replace mempool-selected txns with just coinbase plus passed-in txns:
-    pblock->vtx.resize(1);
+    if (fNoMempoolTx) {
+        pblock->vtx.resize(1);
+
+        // Replace coinbase output amount (could have included fee in CreateNewBlock)
+        CMutableTransaction txCoinbase(*pblock->vtx[0]);
+        txCoinbase.vout[0].nValue = GetBlockValue(nHeight);
+        pblock->vtx[0] = MakeTransactionRef(txCoinbase);
+    }
     for (const CMutableTransaction& tx : txns) {
         pblock->vtx.push_back(MakeTransactionRef(tx));
     }
 
-    const CBlockIndex* pindexPrev = WITH_LOCK(cs_main, return chainActive.Tip());
-    const int nHeight = pindexPrev->nHeight + 1;
-
-    // Replace coinbase output amount (could have included fee in CreateNewBlock)
-    CMutableTransaction txCoinbase(*pblock->vtx[0]);
-    txCoinbase.vout[0].nValue = GetBlockValue(nHeight);
-    pblock->vtx[0] = MakeTransactionRef(txCoinbase);
-
     // IncrementExtraNonce creates a valid coinbase and merkleRoot
     unsigned int extraNonce = 0;
     IncrementExtraNonce(pblock, pindexPrev, extraNonce);
diff --git a/src/test/test_pivx.h b/src/test/test_pivx.h
index 67ed4d039377..56424a0deb5f 100644
--- a/src/test/test_pivx.h
+++ b/src/test/test_pivx.h
@@ -66,11 +66,11 @@ struct TestChainSetup : public TestingSetup
     TestChainSetup(int blockCount);
     ~TestChainSetup();
 
-    // Create a new block with just given transactions, coinbase paying to
-    // scriptPubKey, and try to add it to the current chain.
-    CBlock CreateAndProcessBlock(const std::vector<CMutableTransaction>& txns, const CScript& scriptPubKey);
+    // Create a new block with coinbase paying to scriptPubKey, and try to add it to the current chain.
+    // Include given transactions, and, if fNoMempoolTx=true, remove transactions coming from the mempool.
+    CBlock CreateAndProcessBlock(const std::vector<CMutableTransaction>& txns, const CScript& scriptPubKey, bool fNoMempoolTx = true);
     CBlock CreateAndProcessBlock(const std::vector<CMutableTransaction>& txns, const CKey& scriptKey);
-    CBlock CreateBlock(const std::vector<CMutableTransaction>& txns, const CScript& scriptPubKey);
+    CBlock CreateBlock(const std::vector<CMutableTransaction>& txns, const CScript& scriptPubKey, bool fNoMempoolTx = true);
     CBlock CreateBlock(const std::vector<CMutableTransaction>& txns, const CKey& scriptKey);
 
     std::vector<CTransaction> coinbaseTxns; // For convenience, coinbase transactions

From 4cf722268cce4b530b81b57ba85892971f1533a4 Mon Sep 17 00:00:00 2001
From: random-zebra <random.zebra@protonmail.com>
Date: Mon, 3 May 2021 18:23:38 +0200
Subject: [PATCH 07/17] [Tests] Refactor
 wallet_sapling_transactions_validations_tests

Extend its fixture from TestChain100Setup, thus avoid changing
chain-params during the test case, and simplify the code.
---
 ...sapling_transactions_validations_tests.cpp | 97 +++++++++++--------
 1 file changed, 56 insertions(+), 41 deletions(-)

diff --git a/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp b/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp
index 779a4b8d6733..672adc74bae1 100644
--- a/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp
+++ b/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp
@@ -13,30 +13,52 @@
 
 #include <boost/test/unit_test.hpp>
 
-BOOST_FIXTURE_TEST_SUITE(wallet_sapling_transactions_validations_tests, WalletTestingSetup)
-
-void setupWallet()
+/*
+ * A text fixture with a preloaded 100-blocks regtest chain, with sapling activating at block 101,
+ * and a wallet containing the key used for the coinbase outputs.
+ */
+struct TestSaplingChainSetup: public TestChain100Setup
 {
-    LOCK(pwalletMain->cs_wallet);
-    pwalletMain->SetMinVersion(FEATURE_SAPLING);
-    gArgs.ForceSetArg("-keypool", "5");
-    pwalletMain->SetupSPKM(true);
-}
+    TestSaplingChainSetup() : TestChain100Setup()
+    {
+        initZKSNARKS(); // init zk-snarks lib
+
+        bitdb.MakeMock();
+        bool fFirstRun;
+        std::unique_ptr<CWalletDBWrapper> dbw(new CWalletDBWrapper(&bitdb, "wallet_test.dat"));
+        pwalletMain = new CWallet(std::move(dbw));
+        pwalletMain->LoadWallet(fFirstRun);
+        RegisterValidationInterface(pwalletMain);
+
+        int SAPLING_ACTIVATION_HEIGHT = 101;
+        UpdateNetworkUpgradeParameters(Consensus::UPGRADE_V5_0, SAPLING_ACTIVATION_HEIGHT);
+
+        // setup wallet
+        {
+            LOCK(pwalletMain->cs_wallet);
+            pwalletMain->SetMinVersion(FEATURE_SAPLING);
+            gArgs.ForceSetArg("-keypool", "5");
+            pwalletMain->SetupSPKM(true);
+
+            // import coinbase key used to generate the 100-blocks chain
+            BOOST_CHECK(pwalletMain->AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey()));
+        }
+        WalletRescanReserver reserver(pwalletMain);
+        BOOST_CHECK(reserver.reserve());
+        pwalletMain->RescanFromTime(0, reserver, true /* update */);
+    }
 
-void generateBlock(const CScript& scriptPubKey, int expectedBlockHeight)
-{
-    std::unique_ptr<CBlockTemplate> pblocktemplate;
-    BOOST_CHECK(pblocktemplate = BlockAssembler(Params(), false).CreateNewBlock(scriptPubKey, pwalletMain, false));
-    std::shared_ptr<CBlock> pblock = std::make_shared<CBlock>(pblocktemplate->block);
-    pblock->nTime = WITH_LOCK(cs_main, return chainActive.Tip()->GetMedianTimePast() + 60);
-    pblock->hashMerkleRoot = BlockMerkleRoot(*pblock);
-    CValidationState state;
-    BOOST_CHECK_MESSAGE(ProcessNewBlock(state, nullptr, pblock, nullptr), strprintf("Failed creating block at height %d", expectedBlockHeight));
-    BOOST_CHECK(state.IsValid());
+    ~TestSaplingChainSetup()
+    {
+        UnregisterValidationInterface(pwalletMain);
+        delete pwalletMain;
+        pwalletMain = nullptr;
+        bitdb.Flush(true);
+        bitdb.Reset();
+    }
+};
 
-    // Let the wallet sync the blocks
-    SyncWithValidationInterfaceQueue();
-}
+BOOST_FIXTURE_TEST_SUITE(wallet_sapling_transactions_validations_tests, TestSaplingChainSetup)
 
 SaplingOperation createOperationAndBuildTx(std::vector<SendManyRecipient> recipients,
                                            int nextBlockHeight,
@@ -60,11 +82,6 @@ SaplingOperation createOperationAndBuildTx(std::vector<SendManyRecipient> recipi
 // Test double spend notes in the mempool and in blocks.
 BOOST_AUTO_TEST_CASE(test_in_block_and_mempool_notes_double_spend)
 {
-    SelectParams(CBaseChainParams::REGTEST);
-    int SAPLING_ACTIVATION_HEIGHT = 99;
-    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_V5_0, SAPLING_ACTIVATION_HEIGHT);
-    setupWallet();
-
     CTxDestination coinbaseDest;
     auto ret = pwalletMain->getNewAddress(coinbaseDest, "coinbase");
     BOOST_ASSERT_MSG(ret.result, "cannot create address");
@@ -72,14 +89,17 @@ BOOST_AUTO_TEST_CASE(test_in_block_and_mempool_notes_double_spend)
     BOOST_ASSERT_MSG(IsMine(*pwalletMain, coinbaseDest), "destination not from wallet");
 
     // create the chain
+    int tipHeight = WITH_LOCK(cs_main, return chainActive.Tip()->nHeight);
+    BOOST_CHECK_EQUAL(tipHeight, 100);
     const CScript& scriptPubKey = GetScriptForDestination(coinbaseDest);
     int nGenerateBlocks = 110;
-    for (int i = 0; i < nGenerateBlocks; ++i) {
-        generateBlock(scriptPubKey, (int) i);
+    for (int i = tipHeight; i < nGenerateBlocks; ++i) {
+        CreateAndProcessBlock({}, scriptPubKey);
+        SyncWithValidationInterfaceQueue();
     }
 
     // Verify that we are at block 110
-    int tipHeight = WITH_LOCK(cs_main, return chainActive.Tip()->nHeight);
+    tipHeight = WITH_LOCK(cs_main, return chainActive.Tip()->nHeight);
     BOOST_CHECK_EQUAL(tipHeight, nGenerateBlocks);
     // Verify that the wallet has all of the coins
     BOOST_CHECK_EQUAL(pwalletMain->GetAvailableBalance(), CAmount(250 * COIN * 10)); // 10 blocks available
@@ -98,7 +118,10 @@ BOOST_AUTO_TEST_CASE(test_in_block_and_mempool_notes_double_spend)
     BOOST_ASSERT_MSG(operation.send(retHash), "error committing and broadcasting the transaction");
 
     // Generate a five blocks to fully confirm the tx and test balance
-    for (int i = 0; i < 5; ++i) { generateBlock(scriptPubKey, WITH_LOCK(cs_main, return chainActive.Tip()->nHeight + 1)); }
+    for (int i = 1; i <= 5; ++i) {
+        CreateAndProcessBlock({}, scriptPubKey, false /*fNoMempoolTx*/);
+    }
+    SyncWithValidationInterfaceQueue();
     BOOST_CHECK_EQUAL(pwalletMain->GetAvailableShieldedBalance(), CAmount(100 * COIN)); // 100 shield PIVs
     BOOST_CHECK_EQUAL(pwalletMain->GetUnconfirmedShieldedBalance(), CAmount(0)); // 0 shield PIVs
 
@@ -133,21 +156,13 @@ BOOST_AUTO_TEST_CASE(test_in_block_and_mempool_notes_double_spend)
 
     // let's now test it inside a block
     // create the block with the two transactions and test validity
-    std::unique_ptr<CBlockTemplate> pblocktemplate;
-    BOOST_CHECK(pblocktemplate = BlockAssembler(Params(), false).CreateNewBlock(scriptPubKey, pwalletMain, false));
-    std::shared_ptr<CBlock> pblock = std::make_shared<CBlock>(pblocktemplate->block);
-    pblock->nTime = WITH_LOCK(cs_main, return chainActive.Tip()->GetMedianTimePast() + 60);
-    pblock->vtx.emplace_back(MakeTransactionRef(operation3.getFinalTx())); // Force the invalid tx append
-    pblock->hashMerkleRoot = BlockMerkleRoot(*pblock);
-    pblock->hashFinalSaplingRoot = CalculateSaplingTreeRoot(pblock.get(), 116, Params());
-    CValidationState state;
-    ProcessNewBlock(state, nullptr, pblock, nullptr);
-    BOOST_CHECK(state.IsValid());
+    const CBlock& block = CreateAndProcessBlock({operation3.getFinalTx()}, scriptPubKey, false /*fNoMempoolTx*/);
+    SyncWithValidationInterfaceQueue();
 
     {
         LOCK(cs_main);
         // Same tip as before, no block connection
-        BOOST_CHECK(chainActive.Tip()->GetBlockHash() != pblock->GetHash());
+        BOOST_CHECK(chainActive.Tip()->GetBlockHash() != block.GetHash());
         BOOST_ASSERT_MSG(chainActive.Tip()->nHeight, 115);
     }
 }

From 93fcc2ebedb33afd59142b0bd995f1227caaf317 Mon Sep 17 00:00:00 2001
From: random-zebra <random.zebra@protonmail.com>
Date: Sun, 2 May 2021 15:23:25 +0200
Subject: [PATCH 08/17] [Params] Fix regtest coinbase - mine it with lower diff

---
 src/chainparams.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/chainparams.cpp b/src/chainparams.cpp
index cb1ba61ad338..4fa8cbca603c 100644
--- a/src/chainparams.cpp
+++ b/src/chainparams.cpp
@@ -393,13 +393,13 @@ class CRegTestParams : public CChainParams
     {
         strNetworkID = "regtest";
 
-        genesis = CreateGenesisBlock(1454124731, 2402015, 0x1e0ffff0, 1, 250 * COIN);
+        genesis = CreateGenesisBlock(1454124731, 1, 0x207fffff, 1, 250 * COIN);
         consensus.hashGenesisBlock = genesis.GetHash();
-        assert(consensus.hashGenesisBlock == uint256S("0x0000041e482b9b9691d98eefb48473405c0b8ec31b76df3797c74a78680ef818"));
+        assert(consensus.hashGenesisBlock == uint256S("0x7445589c4c8e52b105247b13373e5ee325856aa05d53f429e59ea46b7149ae3f"));
         assert(genesis.hashMerkleRoot == uint256S("0x1b2ef6e2f28be914103a277377ae7729dcd125dfeb8bf97bd5964ba72b6dc39b"));
 
         consensus.fPowAllowMinDifficultyBlocks = true;
-        consensus.powLimit   = uint256S("0x00000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
+        consensus.powLimit   = uint256S("0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
         consensus.posLimitV1 = uint256S("0x000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
         consensus.posLimitV2 = uint256S("0x00000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
         consensus.nBudgetCycleBlocks = 144;         // approx 10 cycles per day

From d0777a7eaeb22c7294427b953fe7a645ee745784 Mon Sep 17 00:00:00 2001
From: random-zebra <random.zebra@protonmail.com>
Date: Fri, 30 Apr 2021 23:14:31 +0200
Subject: [PATCH 09/17] [Refactoring] Add fPowNoRetargeting consensus param +
 fix regtest diff

---
 src/chainparams.cpp    | 3 +++
 src/consensus/params.h | 1 +
 src/pow.cpp            | 7 +++----
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/chainparams.cpp b/src/chainparams.cpp
index 4fa8cbca603c..1c75860ff517 100644
--- a/src/chainparams.cpp
+++ b/src/chainparams.cpp
@@ -134,6 +134,7 @@ class CMainParams : public CChainParams
         assert(genesis.hashMerkleRoot == uint256S("0x1b2ef6e2f28be914103a277377ae7729dcd125dfeb8bf97bd5964ba72b6dc39b"));
 
         consensus.fPowAllowMinDifficultyBlocks = false;
+        consensus.fPowNoRetargeting = false;
         consensus.powLimit   = uint256S("0x00000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
         consensus.posLimitV1 = uint256S("0x000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
         consensus.posLimitV2 = uint256S("0x00000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
@@ -275,6 +276,7 @@ class CTestNetParams : public CChainParams
         assert(genesis.hashMerkleRoot == uint256S("0x1b2ef6e2f28be914103a277377ae7729dcd125dfeb8bf97bd5964ba72b6dc39b"));
 
         consensus.fPowAllowMinDifficultyBlocks = true;
+        consensus.fPowNoRetargeting = false;
         consensus.powLimit   = uint256S("0x00000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
         consensus.posLimitV1 = uint256S("0x000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
         consensus.posLimitV2 = uint256S("0x00000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
@@ -399,6 +401,7 @@ class CRegTestParams : public CChainParams
         assert(genesis.hashMerkleRoot == uint256S("0x1b2ef6e2f28be914103a277377ae7729dcd125dfeb8bf97bd5964ba72b6dc39b"));
 
         consensus.fPowAllowMinDifficultyBlocks = true;
+        consensus.fPowNoRetargeting = true;
         consensus.powLimit   = uint256S("0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
         consensus.posLimitV1 = uint256S("0x000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
         consensus.posLimitV2 = uint256S("0x00000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
diff --git a/src/consensus/params.h b/src/consensus/params.h
index 3e2750089ecf..e43ad5484a95 100644
--- a/src/consensus/params.h
+++ b/src/consensus/params.h
@@ -88,6 +88,7 @@ struct NetworkUpgrade {
 struct Params {
     uint256 hashGenesisBlock;
     bool fPowAllowMinDifficultyBlocks;
+    bool fPowNoRetargeting;
     uint256 powLimit;
     uint256 posLimitV1;
     uint256 posLimitV2;
diff --git a/src/pow.cpp b/src/pow.cpp
index 384cb742957c..7bd1422e6708 100644
--- a/src/pow.cpp
+++ b/src/pow.cpp
@@ -18,7 +18,9 @@
 
 unsigned int GetNextWorkRequired(const CBlockIndex* pindexLast, const CBlockHeader* pblock)
 {
-    if (Params().IsRegTestNet())
+    const Consensus::Params& consensus = Params().GetConsensus();
+
+    if (consensus.fPowNoRetargeting)
         return pindexLast->nBits;
 
     /* current difficulty formula, pivx - DarkGravity v3, written by Evan Duffield - evan@dashpay.io */
@@ -31,7 +33,6 @@ unsigned int GetNextWorkRequired(const CBlockIndex* pindexLast, const CBlockHead
     int64_t CountBlocks = 0;
     arith_uint256 PastDifficultyAverage;
     arith_uint256 PastDifficultyAveragePrev;
-    const Consensus::Params& consensus = Params().GetConsensus();
     const arith_uint256& powLimit = UintToArith256(consensus.powLimit);
 
     if (BlockLastSolved == NULL || BlockLastSolved->nHeight == 0 || BlockLastSolved->nHeight < PastBlocksMin) {
@@ -124,8 +125,6 @@ bool CheckProofOfWork(uint256 hash, unsigned int nBits)
     bool fOverflow;
     arith_uint256 bnTarget;
 
-    if (Params().IsRegTestNet()) return true;
-
     bnTarget.SetCompact(nBits, &fNegative, &fOverflow);
 
     // Check range

From 810a880c48a261f97328b10a60b98832811e61c4 Mon Sep 17 00:00:00 2001
From: random-zebra <random.zebra@protonmail.com>
Date: Sun, 2 May 2021 15:29:48 +0200
Subject: [PATCH 10/17] [Tests] Update PIVX specific constants
 bits/block-size/block-version

---
 test/functional/test_framework/blocktools.py | 2 +-
 test/functional/test_framework/messages.py   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py
index 61980e8c015f..92591bf05f1f 100644
--- a/test/functional/test_framework/blocktools.py
+++ b/test/functional/test_framework/blocktools.py
@@ -21,7 +21,7 @@ def create_block(hashprev, coinbase, nTime=None, nVersion=None, hashFinalSapling
     if hashFinalSaplingRoot is not None:
         block.hashFinalSaplingRoot = hashFinalSaplingRoot
     block.hashPrevBlock = hashprev
-    block.nBits = 0x1e0ffff0 # Will break after a difficulty adjustment...
+    block.nBits = 0x207fffff  # difficulty retargeting is disabled in REGTEST chainparams
     block.vtx.append(coinbase)
     block.hashMerkleRoot = block.calc_merkle_root()
     block.calc_sha256()
diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py
index 0796d09344f8..ffd5d42bfe4a 100644
--- a/test/functional/test_framework/messages.py
+++ b/test/functional/test_framework/messages.py
@@ -32,8 +32,8 @@
 MY_RELAY = 1 # from version 70001 onwards, fRelay should be appended to version messages (BIP37)
 
 MAX_INV_SZ = 50000
-MAX_BLOCK_BASE_SIZE = 1000000
-CURRENT_BLK_VERSION = 7
+MAX_BLOCK_BASE_SIZE = 2000000
+CURRENT_BLK_VERSION = 9
 
 COIN = 100000000 # 1 btc in satoshis
 

From b19bee9885b1dc13e6f28179189c1c8a094147bd Mon Sep 17 00:00:00 2001
From: random-zebra <random.zebra@protonmail.com>
Date: Mon, 3 May 2021 19:13:07 +0200
Subject: [PATCH 11/17] [Refactoring][BUG] Never change chain-params after
 setup in unit tests

Otherwise the block index would be no longer valid, should be discarded,
and a new genesis block reloaded.
---
 src/test/librust/sapling_test_fixture.cpp |  2 +-
 src/test/librust/sapling_test_fixture.h   |  5 +++--
 src/test/miner_tests.cpp                  |  4 ++--
 src/test/test_pivx.h                      | 14 +++++++++++---
 src/test/validation_block_tests.cpp       |  6 ++----
 src/test/validation_tests.cpp             | 11 ++++++-----
 src/wallet/test/wallet_test_fixture.cpp   |  3 ++-
 src/wallet/test/wallet_test_fixture.h     | 10 ++++++++--
 8 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/src/test/librust/sapling_test_fixture.cpp b/src/test/librust/sapling_test_fixture.cpp
index dc388352d9d7..740c70341fee 100644
--- a/src/test/librust/sapling_test_fixture.cpp
+++ b/src/test/librust/sapling_test_fixture.cpp
@@ -5,7 +5,7 @@
 #include "test/librust/sapling_test_fixture.h"
 #include "sapling/sapling_util.h"
 
-SaplingTestingSetup::SaplingTestingSetup() : TestingSetup()
+SaplingTestingSetup::SaplingTestingSetup(const std::string& chainName) : TestingSetup(chainName)
 {
     initZKSNARKS(); // init zk-snarks lib
 }
diff --git a/src/test/librust/sapling_test_fixture.h b/src/test/librust/sapling_test_fixture.h
index 7207a23705c4..59ae6927b46d 100644
--- a/src/test/librust/sapling_test_fixture.h
+++ b/src/test/librust/sapling_test_fixture.h
@@ -10,8 +10,9 @@
 /**
  * Testing setup that configures a complete environment for Sapling testing.
  */
-struct SaplingTestingSetup : public TestingSetup {
-    SaplingTestingSetup();
+struct SaplingTestingSetup : public TestingSetup
+{
+    SaplingTestingSetup(const std::string& chainName = CBaseChainParams::MAIN);
     ~SaplingTestingSetup();
 };
 
diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp
index a0924ad7d143..adb10537882b 100644
--- a/src/test/miner_tests.cpp
+++ b/src/test/miner_tests.cpp
@@ -17,7 +17,8 @@
 
 #include <boost/test/unit_test.hpp>
 
-BOOST_FIXTURE_TEST_SUITE(miner_tests, WalletTestingSetup)
+// future: this should be MAINNET.
+BOOST_FIXTURE_TEST_SUITE(miner_tests, WalletRegTestingSetup)
 
 // BOOST_CHECK_EXCEPTION predicates to check the specific validation error
 class HasReason {
@@ -73,7 +74,6 @@ struct {
 BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
 {
     // Note that by default, these tests run with size accounting enabled.
-    SelectParams(CBaseChainParams::REGTEST); // future: this should be MAINNET.
     const CChainParams& chainparams = Params();
     CScript scriptPubKey = CScript() << OP_DUP << OP_HASH160 << ParseHex("8d5b4f83212214d6ef693e02e6d71969fddad976") << OP_EQUALVERIFY << OP_CHECKSIG;
 
diff --git a/src/test/test_pivx.h b/src/test/test_pivx.h
index 56424a0deb5f..02badf2e95a2 100644
--- a/src/test/test_pivx.h
+++ b/src/test/test_pivx.h
@@ -45,7 +45,8 @@ struct BasicTestingSetup {
  * and wallet (if enabled) setup.
  */
 class CConnman;
-struct TestingSetup: public BasicTestingSetup {
+struct TestingSetup: public BasicTestingSetup
+{
     CCoinsViewDB *pcoinsdbview;
     fs::path pathTemp;
     boost::thread_group threadGroup;
@@ -56,6 +57,11 @@ struct TestingSetup: public BasicTestingSetup {
     ~TestingSetup();
 };
 
+struct RegTestingSetup : public TestingSetup
+{
+    RegTestingSetup() : TestingSetup(CBaseChainParams::REGTEST) {}
+};
+
 class CBlock;
 struct CMutableTransaction;
 class CScript;
@@ -78,13 +84,15 @@ struct TestChainSetup : public TestingSetup
 };
 
 // Testing fixture that pre-creates a 100-block REGTEST-mode blockchain
-struct TestChain100Setup : public TestChainSetup {
+struct TestChain100Setup : public TestChainSetup
+{
     TestChain100Setup() : TestChainSetup(100) {}
 };
 
 // Testing fixture that pre-creates a 400-block REGTEST-mode blockchain
 // all 400 blocks are PoW. PoS starts at height 500
-struct TestChain400Setup : public TestChainSetup {
+struct TestChain400Setup : public TestChainSetup
+{
     TestChain400Setup() : TestChainSetup(400) {}
 };
 
diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp
index f27f42e50523..31c3f81c89fb 100644
--- a/src/test/validation_block_tests.cpp
+++ b/src/test/validation_block_tests.cpp
@@ -14,13 +14,11 @@
 #include "validation.h"
 #include "validationinterface.h"
 
+
 #define ASSERT_WITH_MSG(cond, msg) if (!cond) { BOOST_ERROR(msg); }
 
-struct RegtestingSetup : public TestingSetup {
-    RegtestingSetup() : TestingSetup(CBaseChainParams::REGTEST) {}
-};
 
-BOOST_FIXTURE_TEST_SUITE(validation_block_tests, RegtestingSetup)
+BOOST_FIXTURE_TEST_SUITE(validation_block_tests, RegTestingSetup)
 
 struct TestSubscriber : public CValidationInterface {
     uint256 m_expected_tip;
diff --git a/src/test/validation_tests.cpp b/src/test/validation_tests.cpp
index 5cca80bd8d76..78717a8526ed 100644
--- a/src/test/validation_tests.cpp
+++ b/src/test/validation_tests.cpp
@@ -11,9 +11,9 @@
 
 #include <boost/test/unit_test.hpp>
 
-BOOST_FIXTURE_TEST_SUITE(validation_tests, TestingSetup)
+BOOST_AUTO_TEST_SUITE(validation_tests)
 
-BOOST_AUTO_TEST_CASE(test_simple_shielded_invalid)
+BOOST_FIXTURE_TEST_CASE(test_simple_shielded_invalid, TestingSetup)
 {
     CMutableTransaction tx;
     tx.nVersion = CTransaction::TxVersion::SAPLING;
@@ -113,10 +113,11 @@ void CheckMempoolZcRejection(CMutableTransaction& mtx)
     BOOST_CHECK_EQUAL(state.GetRejectReason(), "bad-tx-with-zc");
 }
 
-BOOST_AUTO_TEST_CASE(zerocoin_rejection_tests)
+/*
+ * Running on regtest to have v5 upgrade enforced at block 1 and test in-block zc rejection
+ */
+BOOST_FIXTURE_TEST_CASE(zerocoin_rejection_tests, RegTestingSetup)
 {
-    // !TODO: fix me
-    SelectParams(CBaseChainParams::REGTEST);
     UpdateNetworkUpgradeParameters(Consensus::UPGRADE_V5_0, Consensus::NetworkUpgrade::ALWAYS_ACTIVE);
     const CChainParams& chainparams = Params();
 
diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp
index 6e8a95734a46..cbfeaca0e3fe 100644
--- a/src/wallet/test/wallet_test_fixture.cpp
+++ b/src/wallet/test/wallet_test_fixture.cpp
@@ -19,7 +19,8 @@ void clean()
     bitdb.Reset();
 }
 
-WalletTestingSetup::WalletTestingSetup(): SaplingTestingSetup()
+WalletTestingSetup::WalletTestingSetup(const std::string& chainName):
+        SaplingTestingSetup(chainName)
 {
     clean(); // todo: research why we have an initialized bitdb here.
     bitdb.MakeMock();
diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h
index 704a33df075a..616ce9926214 100644
--- a/src/wallet/test/wallet_test_fixture.h
+++ b/src/wallet/test/wallet_test_fixture.h
@@ -9,10 +9,16 @@
 
 /** Testing setup and teardown for wallet.
  */
-struct WalletTestingSetup : public SaplingTestingSetup {
-    WalletTestingSetup();
+struct WalletTestingSetup : public SaplingTestingSetup
+{
+    WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN);
     ~WalletTestingSetup();
 };
 
+struct WalletRegTestingSetup : public WalletTestingSetup
+{
+    WalletRegTestingSetup() : WalletTestingSetup(CBaseChainParams::REGTEST) {}
+};
+
 #endif
 

From 0f0c97c31df286d8a5762f0f402036dd0e2deb09 Mon Sep 17 00:00:00 2001
From: random-zebra <random.zebra@protonmail.com>
Date: Wed, 5 May 2021 18:55:55 +0200
Subject: [PATCH 12/17] [BUG] Fix block with invalid PoW in
 zerocoin_rejection_tests

---
 src/test/validation_tests.cpp | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/test/validation_tests.cpp b/src/test/validation_tests.cpp
index 78717a8526ed..761dae484639 100644
--- a/src/test/validation_tests.cpp
+++ b/src/test/validation_tests.cpp
@@ -4,7 +4,6 @@
 
 #include "test/test_pivx.h"
 #include "blockassembler.h"
-#include "consensus/merkle.h"
 #include "primitives/transaction.h"
 #include "sapling/sapling_validation.h"
 #include "test/librust/utiltest.h"
@@ -93,10 +92,12 @@ BOOST_FIXTURE_TEST_CASE(test_simple_shielded_invalid, TestingSetup)
     }
 }
 
-void CheckBlockZcRejection(const std::shared_ptr<CBlock>& pblock, CMutableTransaction& mtx)
+void CheckBlockZcRejection(std::shared_ptr<CBlock>& pblock, CMutableTransaction& mtx)
 {
     pblock->vtx.emplace_back(MakeTransactionRef(mtx));
-    pblock->hashMerkleRoot = BlockMerkleRoot(*pblock);
+    unsigned int extraNonce = 0;
+    IncrementExtraNonce(pblock, GetChainTip(), extraNonce);
+    while (!CheckProofOfWork(pblock->GetHash(), pblock->nBits)) ++pblock->nNonce;
     CValidationState state;
     BOOST_CHECK(!ProcessNewBlock(state, nullptr, pblock, nullptr));
     BOOST_CHECK(!state.IsValid());
@@ -137,18 +138,21 @@ BOOST_FIXTURE_TEST_CASE(zerocoin_rejection_tests, RegTestingSetup)
     mtx.vout[0].scriptPubKey = CScript() << OP_ZEROCOINMINT <<
                                          CBigNum::randBignum(chainparams.GetConsensus().Zerocoin_Params(false)->coinCommitmentGroup.groupOrder).getvch();
     mtx.vout[0].nValue = 1 * COIN;
-    CheckBlockZcRejection(std::make_shared<CBlock>(pblocktemplate->block), mtx);
+    std::shared_ptr<CBlock> pblock = std::make_shared<CBlock>(pblocktemplate->block);
+    CheckBlockZcRejection(pblock, mtx);
     CheckMempoolZcRejection(mtx);
 
     // Zerocoin spends rejection test
     mtx.vout[0].scriptPubKey = scriptPubKey;
     mtx.vin[0].scriptSig = CScript() << OP_ZEROCOINSPEND;
-    CheckBlockZcRejection(std::make_shared<CBlock>(pblocktemplate->block), mtx);
+    pblock = std::make_shared<CBlock>(pblocktemplate->block);
+    CheckBlockZcRejection(pblock, mtx);
     CheckMempoolZcRejection(mtx);
 
     // Zerocoin public spends rejection test
     mtx.vin[0].scriptSig = CScript() << OP_ZEROCOINPUBLICSPEND;
-    CheckBlockZcRejection(std::make_shared<CBlock>(pblocktemplate->block), mtx);
+    pblock = std::make_shared<CBlock>(pblocktemplate->block);
+    CheckBlockZcRejection(pblock, mtx);
     CheckMempoolZcRejection(mtx);
 }
 

From 030517b39f5911951c3ab67f22d72056ea4099a4 Mon Sep 17 00:00:00 2001
From: random-zebra <random.zebra@protonmail.com>
Date: Wed, 5 May 2021 19:04:05 +0200
Subject: [PATCH 13/17] [Refactoring] pass next block height to
 IncrementExtraNonce

---
 src/blockassembler.cpp        | 14 ++++++++++++--
 src/blockassembler.h          |  5 +++--
 src/miner.cpp                 |  2 +-
 src/rpc/mining.cpp            | 11 +----------
 src/test/test_pivx.cpp        | 10 +++-------
 src/test/validation_tests.cpp | 12 +++++-------
 6 files changed, 25 insertions(+), 29 deletions(-)

diff --git a/src/blockassembler.cpp b/src/blockassembler.cpp
index e500ffccc1ce..125c67906739 100644
--- a/src/blockassembler.cpp
+++ b/src/blockassembler.cpp
@@ -483,7 +483,18 @@ uint256 CalculateSaplingTreeRoot(CBlock* pblock, int nHeight, const CChainParams
     return UINT256_ZERO;
 }
 
-void IncrementExtraNonce(std::shared_ptr<CBlock>& pblock, const CBlockIndex* pindexPrev, unsigned int& nExtraNonce)
+bool SolveBlock(std::shared_ptr<CBlock>& pblock, int nHeight)
+{
+    unsigned int extraNonce = 0;
+    IncrementExtraNonce(pblock, nHeight, extraNonce);
+    while (pblock->nNonce < std::numeric_limits<uint32_t>::max() &&
+           !CheckProofOfWork(pblock->GetHash(), pblock->nBits)) {
+        ++pblock->nNonce;
+    }
+    return pblock->nNonce != std::numeric_limits<uint32_t>::max();
+}
+
+void IncrementExtraNonce(std::shared_ptr<CBlock>& pblock, int nHeight, unsigned int& nExtraNonce)
 {
     // Update nExtraNonce
     static uint256 hashPrevBlock;
@@ -492,7 +503,6 @@ void IncrementExtraNonce(std::shared_ptr<CBlock>& pblock, const CBlockIndex* pin
         hashPrevBlock = pblock->hashPrevBlock;
     }
     ++nExtraNonce;
-    unsigned int nHeight = pindexPrev->nHeight + 1; // Height first in coinbase required for block.version=2
     CMutableTransaction txCoinbase(*pblock->vtx[0]);
     txCoinbase.vin[0].scriptSig = (CScript() << nHeight << CScriptNum(nExtraNonce)) + COINBASE_FLAGS;
     assert(txCoinbase.vin[0].scriptSig.size() <= 100);
diff --git a/src/blockassembler.h b/src/blockassembler.h
index 753b10048201..9014167326e1 100644
--- a/src/blockassembler.h
+++ b/src/blockassembler.h
@@ -92,8 +92,9 @@ class BlockAssembler
     bool isStillDependent(CTxMemPool::txiter iter);
 };
 
-/** Modify the extranonce in a block */
-void IncrementExtraNonce(std::shared_ptr<CBlock>& pblock, const CBlockIndex* pindexPrev, unsigned int& nExtraNonce);
+/** Modify the nonce/extranonce in a block */
+bool SolveBlock(std::shared_ptr<CBlock>& pblock, int nHeight);
+void IncrementExtraNonce(std::shared_ptr<CBlock>& pblock, int nHeight, unsigned int& nExtraNonce);
 int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev);
 int32_t ComputeBlockVersion(const Consensus::Params& consensusParams, int nHeight);
 
diff --git a/src/miner.cpp b/src/miner.cpp
index 68291fc60ef2..a9f674bfb6b8 100644
--- a/src/miner.cpp
+++ b/src/miner.cpp
@@ -188,7 +188,7 @@ void BitcoinMiner(CWallet* pwallet, bool fProofOfStake)
         }
 
         // POW - miner main
-        IncrementExtraNonce(pblock, pindexPrev, nExtraNonce);
+        IncrementExtraNonce(pblock, pindexPrev->nHeight + 1, nExtraNonce);
 
         LogPrintf("Running PIVXMiner with %u transactions in block (%u bytes)\n", pblock->vtx.size(),
             ::GetSerializeSize(*pblock, SER_NETWORK, PROTOCOL_VERSION));
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index 1d1a7f715f7a..2fe468f85006 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -35,7 +35,6 @@ UniValue generateBlocks(const Consensus::Params& consensus,
                         CScript* coinbaseScript)
 {
     UniValue blockHashes(UniValue::VARR);
-    unsigned int nExtraNonce = 0;
 
     while (nHeight < nHeightEnd && !ShutdownRequested()) {
 
@@ -52,16 +51,8 @@ UniValue generateBlocks(const Consensus::Params& consensus,
         std::shared_ptr<CBlock> pblock = std::make_shared<CBlock>(pblocktemplate->block);
 
         if(!fPoS) {
-            {
-                LOCK(cs_main);
-                IncrementExtraNonce(pblock, chainActive.Tip(), nExtraNonce);
-            }
-            while (pblock->nNonce < std::numeric_limits<uint32_t>::max() &&
-                   !CheckProofOfWork(pblock->GetHash(), pblock->nBits)) {
-                ++pblock->nNonce;
-            }
             if (ShutdownRequested()) break;
-            if (pblock->nNonce == std::numeric_limits<uint32_t>::max()) continue;
+            if (!SolveBlock(pblock, nHeight + 1)) continue;
         }
 
         CValidationState state;
diff --git a/src/test/test_pivx.cpp b/src/test/test_pivx.cpp
index 0d9026628a33..00376f32753b 100644
--- a/src/test/test_pivx.cpp
+++ b/src/test/test_pivx.cpp
@@ -155,8 +155,7 @@ CBlock TestChainSetup::CreateBlock(const std::vector<CMutableTransaction>& txns,
             Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, nullptr, false);
     std::shared_ptr<CBlock> pblock = std::make_shared<CBlock>(pblocktemplate->block);
 
-    const CBlockIndex* pindexPrev = WITH_LOCK(cs_main, return chainActive.Tip());
-    const int nHeight = pindexPrev->nHeight + 1;
+    const int nHeight = WITH_LOCK(cs_main, return chainActive.Height()) + 1;
 
     // Replace mempool-selected txns with just coinbase plus passed-in txns:
     if (fNoMempoolTx) {
@@ -171,14 +170,11 @@ CBlock TestChainSetup::CreateBlock(const std::vector<CMutableTransaction>& txns,
         pblock->vtx.push_back(MakeTransactionRef(tx));
     }
 
-    // IncrementExtraNonce creates a valid coinbase and merkleRoot
-    unsigned int extraNonce = 0;
-    IncrementExtraNonce(pblock, pindexPrev, extraNonce);
-
     // Re-compute sapling root
     pblock->hashFinalSaplingRoot = CalculateSaplingTreeRoot(pblock.get(), nHeight, Params());
 
-    while (!CheckProofOfWork(pblock->GetHash(), pblock->nBits)) ++pblock->nNonce;
+    // Find valid PoW
+    assert(SolveBlock(pblock, nHeight));
     return *pblock;
 }
 
diff --git a/src/test/validation_tests.cpp b/src/test/validation_tests.cpp
index 761dae484639..41b6dca855b8 100644
--- a/src/test/validation_tests.cpp
+++ b/src/test/validation_tests.cpp
@@ -92,12 +92,10 @@ BOOST_FIXTURE_TEST_CASE(test_simple_shielded_invalid, TestingSetup)
     }
 }
 
-void CheckBlockZcRejection(std::shared_ptr<CBlock>& pblock, CMutableTransaction& mtx)
+void CheckBlockZcRejection(std::shared_ptr<CBlock>& pblock, int nHeight, CMutableTransaction& mtx)
 {
     pblock->vtx.emplace_back(MakeTransactionRef(mtx));
-    unsigned int extraNonce = 0;
-    IncrementExtraNonce(pblock, GetChainTip(), extraNonce);
-    while (!CheckProofOfWork(pblock->GetHash(), pblock->nBits)) ++pblock->nNonce;
+    BOOST_CHECK(SolveBlock(pblock, nHeight));
     CValidationState state;
     BOOST_CHECK(!ProcessNewBlock(state, nullptr, pblock, nullptr));
     BOOST_CHECK(!state.IsValid());
@@ -139,20 +137,20 @@ BOOST_FIXTURE_TEST_CASE(zerocoin_rejection_tests, RegTestingSetup)
                                          CBigNum::randBignum(chainparams.GetConsensus().Zerocoin_Params(false)->coinCommitmentGroup.groupOrder).getvch();
     mtx.vout[0].nValue = 1 * COIN;
     std::shared_ptr<CBlock> pblock = std::make_shared<CBlock>(pblocktemplate->block);
-    CheckBlockZcRejection(pblock, mtx);
+    CheckBlockZcRejection(pblock, 1, mtx);
     CheckMempoolZcRejection(mtx);
 
     // Zerocoin spends rejection test
     mtx.vout[0].scriptPubKey = scriptPubKey;
     mtx.vin[0].scriptSig = CScript() << OP_ZEROCOINSPEND;
     pblock = std::make_shared<CBlock>(pblocktemplate->block);
-    CheckBlockZcRejection(pblock, mtx);
+    CheckBlockZcRejection(pblock, 1, mtx);
     CheckMempoolZcRejection(mtx);
 
     // Zerocoin public spends rejection test
     mtx.vin[0].scriptSig = CScript() << OP_ZEROCOINPUBLICSPEND;
     pblock = std::make_shared<CBlock>(pblocktemplate->block);
-    CheckBlockZcRejection(pblock, mtx);
+    CheckBlockZcRejection(pblock, 1, mtx);
     CheckMempoolZcRejection(mtx);
 }
 

From 77645bf35839718c3ecfb8efa83690e1d0eca6fd Mon Sep 17 00:00:00 2001
From: random-zebra <random.zebra@protonmail.com>
Date: Mon, 3 May 2021 23:57:30 +0200
Subject: [PATCH 14/17] [BUG][Tests] Add on_getblocks to P2PDataStore interface

Without this, send_blocks_and_test() fails to send more than one block,
as the GETBLOCKS replies from the node are not passed to on_getdata.
---
 test/functional/test_framework/mininode.py | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py
index 0659811aba72..160be8fc5431 100755
--- a/test/functional/test_framework/mininode.py
+++ b/test/functional/test_framework/mininode.py
@@ -482,6 +482,17 @@ def on_getdata(self, message):
             else:
                 logger.debug('getdata message type {} received.'.format(hex(inv.type)))
 
+    def on_getblocks(self, message):
+        """Check for blocks in our stores, reply with inv messages."""
+        msg_blocks = []
+        blockhashes = self.block_store.keys()
+        blockhash = message.hashstop
+        while blockhash in blockhashes and blockhash not in message.locator.vHave:
+            msg_blocks.append(msg_block(self.block_store[blockhash]))
+            blockhash = self.block_store[blockhash].hashPrevBlock
+        for msg in reversed(msg_blocks):
+            self.send_message(msg)
+
     def send_blocks_and_test(self, blocks, node, success=True, reject_reason=None, expect_disconnect=False, timeout=60):
         """Send blocks to test node and test whether the tip advances.
 

From 3633e42968b65d5ed82143eae1c886838657f2a0 Mon Sep 17 00:00:00 2001
From: random-zebra <random.zebra@protonmail.com>
Date: Sun, 2 May 2021 15:51:23 +0200
Subject: [PATCH 15/17] [Tests] Fix and resurrect feature_block.py

>>> based on bitcoin@ebf053ac6135941907ecfebccc778da34b585fac
---
 test/functional/feature_block.py | 1391 +++++++++++++++---------------
 test/functional/test_runner.py   |    3 +-
 2 files changed, 699 insertions(+), 695 deletions(-)

diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py
index 91adcfc268fe..9a1a5fc477e6 100755
--- a/test/functional/feature_block.py
+++ b/test/functional/feature_block.py
@@ -1,37 +1,61 @@
 #!/usr/bin/env python3
-# Copyright (c) 2015-2017 The Bitcoin Core developers
+# Copyright (c) 2015-2021 The Bitcoin Core developers
+# Copyright (c) 2020-2021 The PIVX developers
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
-"""Test block processing.
-
-This reimplements tests from the bitcoinj/FullBlockTestGenerator used
-by the pull-tester.
-
-We use the testing framework in which we expect a particular answer from
-each test.
-"""
-
-from test_framework.test_framework import ComparisonTestFramework
-from test_framework.util import *
-from test_framework.comptool import TestManager, TestInstance, RejectResult
-from test_framework.blocktools import *
+"""Test block processing."""
+import copy
+import struct
 import time
+
+from test_framework.blocktools import create_block, create_coinbase, create_transaction, get_legacy_sigopcount_block
 from test_framework.key import CECKey
-from test_framework.script import *
-from test_framework.mininode import network_thread_start
-import struct
+from test_framework.messages import (
+    CBlock,
+    COIN,
+    COutPoint,
+    CTransaction,
+    CTxIn,
+    CTxOut,
+    MAX_BLOCK_BASE_SIZE,
+    uint256_from_compact,
+    uint256_from_str,
+)
+from test_framework.mininode import P2PDataStore, network_thread_start, network_thread_join
+from test_framework.script import (
+    CScript,
+    MAX_SCRIPT_ELEMENT_SIZE,
+    OP_2DUP,
+    OP_CHECKMULTISIG,
+    OP_CHECKMULTISIGVERIFY,
+    OP_CHECKSIG,
+    OP_CHECKSIGVERIFY,
+    OP_ELSE,
+    OP_ENDIF,
+    OP_EQUAL,
+    OP_FALSE,
+    OP_HASH160,
+    OP_IF,
+    OP_INVALIDOPCODE,
+    OP_RETURN,
+    OP_TRUE,
+    SIGHASH_ALL,
+    SignatureHash,
+    hash160,
+)
+from test_framework.test_framework import PivxTestFramework
+from test_framework.util import assert_equal, bytes_to_hex_str
+
+MAX_BLOCK_SIGOPS = MAX_BLOCK_BASE_SIZE // 50
 
 class PreviousSpendableOutput():
-    def __init__(self, tx = CTransaction(), n = -1):
+    def __init__(self, tx=CTransaction(), n=-1):
         self.tx = tx
         self.n = n  # the output we're spending
 
 #  Use this class for tests that require behavior other than normal "mininode" behavior.
 #  For now, it is used to serialize a bloated varint (b64).
 class CBrokenBlock(CBlock):
-    def __init__(self, header=None):
-        super(CBrokenBlock, self).__init__(header)
-
     def initialize(self, base_block):
         self.vtx = copy.deepcopy(base_block.vtx)
         self.hashMerkleRoot = self.calc_merkle_root()
@@ -48,381 +72,271 @@ def serialize(self, with_witness=False):
         return r
 
     def normal_serialize(self):
-        r = b""
-        r += super(CBrokenBlock, self).serialize()
-        return r
+        return super().serialize()
 
-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.
+class FullBlockTest(PivxTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
         self.setup_clean_chain = True
+        self.extra_args = [["-acceptnonstdtxn=1"]] # This is a consensus block test, we don't care about tx policy
+
+    def run_test(self):
+        node = self.nodes[0]  # convenience reference to the node
+        # reconnect_p2p() expects the network thread to be running
+        self.log.info("Starting network thread...")
+        network_thread_start()
+        self.reconnect_p2p()
+
         self.block_heights = {}
         self.coinbase_key = CECKey()
         self.coinbase_key.set_secretbytes(b"horsebattery")
         self.coinbase_pubkey = self.coinbase_key.get_pubkey()
         self.tip = None
         self.blocks = {}
-
-    def add_options(self, parser):
-        super().add_options(parser)
-        parser.add_option("--runbarelyexpensive", dest="runbarelyexpensive", default=True)
-
-    def run_test(self):
-        self.test = TestManager(self, self.options.tmpdir)
-        self.test.add_all_connections(self.nodes)
-        network_thread_start()
-        self.test.run()
-
-    def add_transactions_to_block(self, block, tx_list):
-        [ tx.rehash() for tx in tx_list ]
-        block.vtx.extend(tx_list)
-
-    # this is a little handier to use than the version in blocktools.py
-    def create_tx(self, spend_tx, n, value, script=CScript([OP_TRUE, OP_DROP] * 15 + [OP_TRUE])):
-        tx = create_transaction(spend_tx, n, b"", value, script)
-        return tx
-
-    # sign a transaction, using the key we know about
-    # this signs input 0 in tx, which is assumed to be spending output n in spend_tx
-    def sign_tx(self, tx, spend_tx, n):
-        scriptPubKey = bytearray(spend_tx.vout[n].scriptPubKey)
-        if (scriptPubKey[0] == OP_TRUE):  # an anyone-can-spend
-            tx.vin[0].scriptSig = CScript()
-            return
-        (sighash, err) = SignatureHash(spend_tx.vout[n].scriptPubKey, tx, 0, SIGHASH_ALL)
-        tx.vin[0].scriptSig = CScript([self.coinbase_key.sign(sighash) + bytes(bytearray([SIGHASH_ALL]))])
-
-    def create_and_sign_transaction(self, spend_tx, n, value, script=CScript([OP_TRUE])):
-        tx = self.create_tx(spend_tx, n, value, script)
-        self.sign_tx(tx, spend_tx, n)
-        tx.rehash()
-        return tx
-
-    def next_block(self, number, spend=None, additional_coinbase_value=0, script=CScript([OP_TRUE]), solve=True):
-        if self.tip == None:
-            base_block_hash = self.genesis_hash
-            block_time = int(time.time())+1
-        else:
-            base_block_hash = self.tip.sha256
-            block_time = self.tip.nTime + 1
-        # First create the coinbase
-        height = self.block_heights[base_block_hash] + 1
-        coinbase = create_coinbase(height, self.coinbase_pubkey)
-        coinbase.vout[0].nValue += additional_coinbase_value
-        coinbase.rehash()
-        if spend == None:
-            block = create_block(base_block_hash, coinbase, block_time)
-        else:
-            coinbase.vout[0].nValue += spend.tx.vout[spend.n].nValue - 1 # all but one satoshi to fees
-            coinbase.rehash()
-            block = create_block(base_block_hash, coinbase, block_time)
-            tx = create_transaction(spend.tx, spend.n, b"", 1, script)  # spend 1 satoshi
-            self.sign_tx(tx, spend.tx, spend.n)
-            self.add_transactions_to_block(block, [tx])
-            block.hashMerkleRoot = block.calc_merkle_root()
-        if solve:
-            block.solve()
-        self.tip = block
-        self.block_heights[block.sha256] = height
-        assert number not in self.blocks
-        self.blocks[number] = block
-        return block
-
-    def get_tests(self):
         self.genesis_hash = int(self.nodes[0].getbestblockhash(), 16)
         self.block_heights[self.genesis_hash] = 0
-        spendable_outputs = []
-
-        # save the current tip so it can be spent by a later block
-        def save_spendable_output():
-            spendable_outputs.append(self.tip)
-
-        # get an output that we previously marked as spendable
-        def get_spendable_output():
-            return PreviousSpendableOutput(spendable_outputs.pop(0).vtx[0], 0)
-
-        # returns a test case that asserts that the current tip was accepted
-        def accepted():
-            return TestInstance([[self.tip, True]])
-
-        # returns a test case that asserts that the current tip was rejected
-        def rejected(reject = None):
-            if reject is None:
-                return TestInstance([[self.tip, False]])
-            else:
-                return TestInstance([[self.tip, reject]])
-
-        # move the tip back to a previous block
-        def tip(number):
-            self.tip = self.blocks[number]
-
-        # adds transactions to the block and updates state
-        def update_block(block_number, new_transactions):
-            block = self.blocks[block_number]
-            self.add_transactions_to_block(block, new_transactions)
-            old_sha256 = block.sha256
-            block.hashMerkleRoot = block.calc_merkle_root()
-            block.solve()
-            # Update the internal state just like in next_block
-            self.tip = block
-            if block.sha256 != old_sha256:
-                self.block_heights[block.sha256] = self.block_heights[old_sha256]
-                del self.block_heights[old_sha256]
-            self.blocks[block_number] = block
-            return block
-
-        # shorthand for functions
-        block = self.next_block
-        create_tx = self.create_tx
-        create_and_sign_tx = self.create_and_sign_transaction
-
-        # these must be updated if consensus changes
-        MAX_BLOCK_SIGOPS = 20000
-
+        self.spendable_outputs = []
 
         # Create a new block
-        block(0)
-        save_spendable_output()
-        yield accepted()
-
-
-        # Now we need that block to mature so we can spend the coinbase.
-        test = TestInstance(sync_every_block=False)
+        self.log.info("Creating and sending a block...")
+        b0 = self.next_block(0)
+        self.save_spendable_output()
+        self.send_blocks([b0])
+
+        # Allow the block to mature
+        self.log.info("Maturing the block...")
+        blocks = []
         for i in range(99):
-            block(5000 + i)
-            test.blocks_and_transactions.append([self.tip, True])
-            save_spendable_output()
-        yield test
+            blocks.append(self.next_block(4000 + i))
+            self.save_spendable_output()
+        self.log.info("Sending all new blocks")
+        self.send_blocks(blocks)
 
         # collect spendable outputs now to avoid cluttering the code later on
+        self.log.info("Collect spendable outputs...")
         out = []
         for i in range(33):
-            out.append(get_spendable_output())
+            out.append(self.get_spendable_output())
 
         # Start by building a couple of blocks on top (which output is spent is
         # in parentheses):
         #     genesis -> b1 (0) -> b2 (1)
-        block(1, spend=out[0])
-        save_spendable_output()
-        yield accepted()
+        self.log.info("Build b1 and b2")
+        b1 = self.next_block(1, spend=out[0])
+        self.save_spendable_output()
+
+        b2 = self.next_block(2, spend=out[1])
+        self.save_spendable_output()
 
-        block(2, spend=out[1])
-        yield accepted()
-        save_spendable_output()
+        self.send_blocks([b1, b2])
 
-        # so fork like this:
+        # Fork like this:
         #
         #     genesis -> b1 (0) -> b2 (1)
         #                      \-> b3 (1)
         #
         # Nothing should happen at this point. We saw b2 first so it takes priority.
-        tip(1)
-        b3 = block(3, spend=out[1])
+        self.log.info("Don't reorg to a chain of the same length")
+        self.move_tip(1)
+        b3 = self.next_block(3, spend=out[1])
         txout_b3 = PreviousSpendableOutput(b3.vtx[1], 0)
-        yield rejected()
-
+        self.send_blocks([b3], False)
 
         # Now we add another block to make the alternative chain longer.
         #
         #     genesis -> b1 (0) -> b2 (1)
         #                      \-> b3 (1) -> b4 (2)
-        block(4, spend=out[2])
-        yield accepted()
-
+        self.log.info("Reorg to a longer chain")
+        b4 = self.next_block(4, spend=out[2])
+        self.send_blocks([b4])
 
         # ... and back to the first chain.
         #     genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6 (3)
         #                      \-> b3 (1) -> b4 (2)
-        tip(2)
-        block(5, spend=out[2])
-        save_spendable_output()
-        yield rejected()
+        self.move_tip(2)
+        b5 = self.next_block(5, spend=out[2])
+        self.save_spendable_output()
+        self.send_blocks([b5], False)
 
-        block(6, spend=out[3])
-        yield accepted()
+        self.log.info("Reorg back to the original chain")
+        b6 = self.next_block(6, spend=out[3])
+        self.send_blocks([b6], True)
 
         # Try to create a fork that double-spends
         #     genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6 (3)
         #                                          \-> b7 (2) -> b8 (4)
         #                      \-> b3 (1) -> b4 (2)
-        tip(5)
-        block(7, spend=out[2])
-        yield rejected()
+        self.log.info("Reject a chain with a double spend, even if it is longer")
+        self.move_tip(5)
+        b7 = self.next_block(7, spend=out[2])
+        self.send_blocks([b7], False)
 
-        block(8, spend=out[4])
-        yield rejected()
+        b8 = self.next_block(8, spend=out[4])
+        self.send_blocks([b8], False, reconnect=True)
 
         # Try to create a block that has too much fee
         #     genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6 (3)
         #                                                    \-> b9 (4)
         #                      \-> b3 (1) -> b4 (2)
-        tip(6)
-        block(9, spend=out[4], additional_coinbase_value=1)
-        yield rejected(RejectResult(16, b'bad-cb-amount'))
+        self.log.info("Reject a block where the miner creates too much coinbase reward")
+        self.move_tip(6)
+        b9 = self.next_block(9, spend=out[4], additional_coinbase_value=1)
+        # !TODO: fix expect_disconnect
+        self.send_blocks([b9], False, "bad-blk-amount", reconnect=False)
 
         # Create a fork that ends in a block with too much fee (the one that causes the reorg)
         #     genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6  (3)
         #                                          \-> b10 (3) -> b11 (4)
         #                      \-> b3 (1) -> b4 (2)
-        tip(5)
-        block(10, spend=out[3])
-        yield rejected()
-
-        block(11, spend=out[4], additional_coinbase_value=1)
-        yield rejected(RejectResult(16, b'bad-cb-amount'))
+        self.log.info("Reject a chain where the miner creates too much coinbase reward, even if the chain is longer")
+        self.move_tip(5)
+        b10 = self.next_block(10, spend=out[3])
+        self.send_blocks([b10], False)
 
+        b11 = self.next_block(11, spend=out[4], additional_coinbase_value=1)
+        # !TODO: fix expect_disconnect
+        self.send_blocks([b11], False, "bad-blk-amount", reconnect=False)
 
         # Try again, but with a valid fork first
         #     genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6  (3)
         #                                          \-> b12 (3) -> b13 (4) -> b14 (5)
-        #                                              (b12 added last)
         #                      \-> b3 (1) -> b4 (2)
-        tip(5)
-        b12 = block(12, spend=out[3])
-        save_spendable_output()
-        b13 = block(13, spend=out[4])
-        # Deliver the block header for b12, and the block b13.
-        # b13 should be accepted but the tip won't advance until b12 is delivered.
-        yield TestInstance([[CBlockHeader(b12), None], [b13, False]])
-
-        save_spendable_output()
-        # b14 is invalid, but the node won't know that until it tries to connect
-        # Tip still can't advance because b12 is missing
-        block(14, spend=out[5], additional_coinbase_value=1)
-        yield rejected()
-
-        yield TestInstance([[b12, True, b13.sha256]]) # New tip should be b13.
+        self.log.info("Reject a chain where the miner creates too much coinbase reward, even if the chain is longer (on a forked chain)")
+        self.move_tip(5)
+        b12 = self.next_block(12, spend=out[3])
+        self.save_spendable_output()
+        b13 = self.next_block(13, spend=out[4])
+        self.save_spendable_output()
+        b14 = self.next_block(14, spend=out[5], additional_coinbase_value=1)
+        # !TODO: fix expect_disconnect
+        self.send_blocks([b12, b13, b14], False, "bad-blk-amount", reconnect=False)
+
+        # New tip should be b13.
+        assert_equal(node.getbestblockhash(), b13.hash)
 
         # Add a block with MAX_BLOCK_SIGOPS and one with one more sigop
         #     genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6  (3)
         #                                          \-> b12 (3) -> b13 (4) -> b15 (5) -> b16 (6)
         #                      \-> b3 (1) -> b4 (2)
-
-        # Test that a block with a lot of checksigs is okay
+        self.log.info("Accept a block with lots of checksigs")
         lots_of_checksigs = CScript([OP_CHECKSIG] * (MAX_BLOCK_SIGOPS - 1))
-        tip(13)
-        block(15, spend=out[5], script=lots_of_checksigs)
-        yield accepted()
-        save_spendable_output()
-
+        self.move_tip(13)
+        b15 = self.next_block(15, spend=out[5], script=lots_of_checksigs)
+        self.save_spendable_output()
+        self.send_blocks([b15], True)
 
-        # Test that a block with too many checksigs is rejected
+        self.log.info("Reject a block with too many checksigs")
         too_many_checksigs = CScript([OP_CHECKSIG] * (MAX_BLOCK_SIGOPS))
-        block(16, spend=out[6], script=too_many_checksigs)
-        yield rejected(RejectResult(16, b'bad-blk-sigops'))
-
+        b16 = self.next_block(16, spend=out[6], script=too_many_checksigs)
+        self.send_blocks([b16], False, "bad-blk-sigops", reconnect=True)
 
         # Attempt to spend a transaction created on a different fork
         #     genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6  (3)
         #                                          \-> b12 (3) -> b13 (4) -> b15 (5) -> b17 (b3.vtx[1])
         #                      \-> b3 (1) -> b4 (2)
-        tip(15)
-        block(17, spend=txout_b3)
-        yield rejected(RejectResult(16, b'bad-txns-inputs-missingorspent'))
+        self.log.info("Reject a block with a spend from a re-org'ed out tx")
+        self.move_tip(15)
+        b17 = self.next_block(17, spend=txout_b3)
+        # !TODO: fix expect_disconnect
+        self.send_blocks([b17], False, "bad-txns-inputs-missingorspent", reconnect=False)
 
         # Attempt to spend a transaction created on a different fork (on a fork this time)
         #     genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6  (3)
         #                                          \-> b12 (3) -> b13 (4) -> b15 (5)
         #                                                                \-> b18 (b3.vtx[1]) -> b19 (6)
         #                      \-> b3 (1) -> b4 (2)
-        tip(13)
-        block(18, spend=txout_b3)
-        yield rejected()
+        self.log.info("Reject a block with a spend from a re-org'ed out tx (on a forked chain)")
+        self.move_tip(13)
+        b18 = self.next_block(18, spend=txout_b3)
+        self.send_blocks([b18], False)
 
-        block(19, spend=out[6])
-        yield rejected()
+        b19 = self.next_block(19, spend=out[6])
+        self.send_blocks([b19], False, "bad-txns-inputs-missingorspent", reconnect=True)
 
         # Attempt to spend a coinbase at depth too low
         #     genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6  (3)
         #                                          \-> b12 (3) -> b13 (4) -> b15 (5) -> b20 (7)
         #                      \-> b3 (1) -> b4 (2)
-        tip(15)
-        block(20, spend=out[7])
-        yield rejected(RejectResult(16, b'bad-txns-premature-spend-of-coinbase'))
+        self.log.info("Reject a block spending an immature coinbase.")
+        self.move_tip(15)
+        b20 = self.next_block(20, spend=out[7])
+        self.send_blocks([b20], False, "bad-txns-premature-spend-of-coinbase")
 
         # Attempt to spend a coinbase at depth too low (on a fork this time)
         #     genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6  (3)
         #                                          \-> b12 (3) -> b13 (4) -> b15 (5)
         #                                                                \-> b21 (6) -> b22 (5)
         #                      \-> b3 (1) -> b4 (2)
-        tip(13)
-        block(21, spend=out[6])
-        yield rejected()
+        self.log.info("Reject a block spending an immature coinbase (on a forked chain)")
+        self.move_tip(13)
+        b21 = self.next_block(21, spend=out[6])
+        self.send_blocks([b21], False)
 
-        block(22, spend=out[5])
-        yield rejected()
+        b22 = self.next_block(22, spend=out[5])
+        self.send_blocks([b22], False, "bad-txns-premature-spend-of-coinbase")
 
         # Create a block on either side of MAX_BLOCK_BASE_SIZE and make sure its accepted/rejected
         #     genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6  (3)
         #                                          \-> b12 (3) -> b13 (4) -> b15 (5) -> b23 (6)
         #                                                                           \-> b24 (6) -> b25 (7)
         #                      \-> b3 (1) -> b4 (2)
-        tip(15)
-        b23 = block(23, spend=out[6])
-        tx = CTransaction()
-        script_length = MAX_BLOCK_BASE_SIZE - len(b23.serialize()) - 69
-        script_output = CScript([b'\x00' * script_length])
-        tx.vout.append(CTxOut(0, script_output))
-        tx.vin.append(CTxIn(COutPoint(b23.vtx[1].sha256, 0)))
-        b23 = update_block(23, [tx])
-        # Make sure the math above worked out to produce a max-sized block
-        assert_equal(len(b23.serialize()), MAX_BLOCK_BASE_SIZE)
-        yield accepted()
-        save_spendable_output()
-
-        # Make the next block one byte bigger and check that it fails
-        tip(15)
-        b24 = block(24, spend=out[6])
-        script_length = MAX_BLOCK_BASE_SIZE - len(b24.serialize()) - 69
-        script_output = CScript([b'\x00' * (script_length+1)])
-        tx.vout = [CTxOut(0, script_output)]
-        b24 = update_block(24, [tx])
-        assert_equal(len(b24.serialize()), MAX_BLOCK_BASE_SIZE+1)
-        yield rejected(RejectResult(16, b'bad-blk-length'))
-
-        block(25, spend=out[7])
-        yield rejected()
+        self.log.info("Accept a block of size MAX_BLOCK_BASE_SIZE")
+        self.move_tip(15)
+        height = self.block_heights[self.tip.sha256] + 1
+        b23 = self.next_block(23)
+        b23 = self.create_sized_block(b23, out[6], MAX_BLOCK_BASE_SIZE)
+        self.block_heights[b23.sha256] = height
+        self.blocks[23] = b23
+        self.send_blocks([b23], True)
+        self.save_spendable_output()
+
+        self.log.info("Reject a block of size MAX_BLOCK_BASE_SIZE + 2")
+        self.move_tip(15)
+        b24 = self.next_block(24)
+        b24 = self.create_sized_block(b24, out[6], MAX_BLOCK_BASE_SIZE + 2)
+        self.block_heights[b24.sha256] = height
+        self.blocks[24] = b24
+        self.send_blocks([b24], False, "bad-blk-length", reconnect=True)
+
+        b25 = self.next_block(25, spend=out[7])
+        self.send_blocks([b25], False)
 
         # Create blocks with a coinbase input script size out of range
         #     genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6  (3)
         #                                          \-> b12 (3) -> b13 (4) -> b15 (5) -> b23 (6) -> b30 (7)
         #                                                                           \-> ... (6) -> ... (7)
         #                      \-> b3 (1) -> b4 (2)
-        tip(15)
-        b26 = block(26, spend=out[6])
+        self.log.info("Reject a block with coinbase input script size out of range")
+        self.move_tip(15)
+        b26 = self.next_block(26, spend=out[6])
         b26.vtx[0].vin[0].scriptSig = b'\x00'
         b26.vtx[0].rehash()
         # update_block causes the merkle root to get updated, even with no new
         # transactions, and updates the required state.
-        b26 = update_block(26, [])
-        yield rejected(RejectResult(16, b'bad-cb-length'))
+        b26 = self.update_block(26, [])
+        self.send_blocks([b26], False, "bad-cb-length", reconnect=True)
 
         # Extend the b26 chain to make sure pivxd isn't accepting b26
-        block(27, spend=out[7])
-        yield rejected(False)
+        b27 = self.next_block(27, spend=out[7])
+        self.send_blocks([b27], False)
 
         # Now try a too-large-coinbase script
-        tip(15)
-        b28 = block(28, spend=out[6])
-        b28.vtx[0].vin[0].scriptSig = b'\x00' * 101
+        self.move_tip(15)
+        b28 = self.next_block(28, spend=out[6])
+        b28.vtx[0].vin[0].scriptSig += b'\x00' * 151
         b28.vtx[0].rehash()
-        b28 = update_block(28, [])
-        yield rejected(RejectResult(16, b'bad-cb-length'))
+        b28 = self.update_block(28, [])
+        self.send_blocks([b28], False, "bad-cb-length", reconnect=True)
 
         # Extend the b28 chain to make sure pivxd isn't accepting b28
-        block(29, spend=out[7])
-        yield rejected(False)
-
-        # b30 has a max-sized coinbase scriptSig.
-        tip(23)
-        b30 = block(30)
-        b30.vtx[0].vin[0].scriptSig = b'\x00' * 100
-        b30.vtx[0].rehash()
-        b30 = update_block(30, [])
-        yield accepted()
-        save_spendable_output()
+        b29 = self.next_block(29, spend=out[7])
+        self.send_blocks([b29], False)
+        # b30
+        self.move_tip(23)
+        b30 = self.next_block(30)
+        b30 = self.update_block(30, [])
+        self.send_blocks([b30], True)
+        self.save_spendable_output()
 
         # b31 - b35 - check sigops of OP_CHECKMULTISIG / OP_CHECKMULTISIGVERIFY / OP_CHECKSIGVERIFY
         #
@@ -433,42 +347,45 @@ def update_block(block_number, new_transactions):
         #
 
         # MULTISIG: each op code counts as 20 sigops.  To create the edge case, pack another 19 sigops at the end.
-        lots_of_multisigs = CScript([OP_CHECKMULTISIG] * ((MAX_BLOCK_SIGOPS-1) // 20) + [OP_CHECKSIG] * 19)
-        b31 = block(31, spend=out[8], script=lots_of_multisigs)
+        self.log.info("Accept a block with the max number of OP_CHECKMULTISIG sigops")
+        lots_of_multisigs = CScript([OP_CHECKMULTISIG] * ((MAX_BLOCK_SIGOPS - 1) // 20) + [OP_CHECKSIG] * 19)
+        b31 = self.next_block(31, spend=out[8], script=lots_of_multisigs)
         assert_equal(get_legacy_sigopcount_block(b31), MAX_BLOCK_SIGOPS)
-        yield accepted()
-        save_spendable_output()
+        self.send_blocks([b31], True)
+        self.save_spendable_output()
 
         # this goes over the limit because the coinbase has one sigop
+        self.log.info("Reject a block with too many OP_CHECKMULTISIG sigops")
         too_many_multisigs = CScript([OP_CHECKMULTISIG] * (MAX_BLOCK_SIGOPS // 20))
-        b32 = block(32, spend=out[9], script=too_many_multisigs)
+        b32 = self.next_block(32, spend=out[9], script=too_many_multisigs)
         assert_equal(get_legacy_sigopcount_block(b32), MAX_BLOCK_SIGOPS + 1)
-        yield rejected(RejectResult(16, b'bad-blk-sigops'))
-
+        self.send_blocks([b32], False, "bad-blk-sigops", reconnect=True)
 
         # CHECKMULTISIGVERIFY
-        tip(31)
-        lots_of_multisigs = CScript([OP_CHECKMULTISIGVERIFY] * ((MAX_BLOCK_SIGOPS-1) // 20) + [OP_CHECKSIG] * 19)
-        block(33, spend=out[9], script=lots_of_multisigs)
-        yield accepted()
-        save_spendable_output()
-
+        self.log.info("Accept a block with the max number of OP_CHECKMULTISIGVERIFY sigops")
+        self.move_tip(31)
+        lots_of_multisigs = CScript([OP_CHECKMULTISIGVERIFY] * ((MAX_BLOCK_SIGOPS - 1) // 20) + [OP_CHECKSIG] * 19)
+        b33 = self.next_block(33, spend=out[9], script=lots_of_multisigs)
+        self.send_blocks([b33], True)
+        self.save_spendable_output()
+
+        self.log.info("Reject a block with too many OP_CHECKMULTISIGVERIFY sigops")
         too_many_multisigs = CScript([OP_CHECKMULTISIGVERIFY] * (MAX_BLOCK_SIGOPS // 20))
-        block(34, spend=out[10], script=too_many_multisigs)
-        yield rejected(RejectResult(16, b'bad-blk-sigops'))
-
+        b34 = self.next_block(34, spend=out[10], script=too_many_multisigs)
+        self.send_blocks([b34], False, "bad-blk-sigops", reconnect=True)
 
         # CHECKSIGVERIFY
-        tip(33)
+        self.log.info("Accept a block with the max number of OP_CHECKSIGVERIFY sigops")
+        self.move_tip(33)
         lots_of_checksigs = CScript([OP_CHECKSIGVERIFY] * (MAX_BLOCK_SIGOPS - 1))
-        b35 = block(35, spend=out[10], script=lots_of_checksigs)
-        yield accepted()
-        save_spendable_output()
+        b35 = self.next_block(35, spend=out[10], script=lots_of_checksigs)
+        self.send_blocks([b35], True)
+        self.save_spendable_output()
 
+        self.log.info("Reject a block with too many OP_CHECKSIGVERIFY sigops")
         too_many_checksigs = CScript([OP_CHECKSIGVERIFY] * (MAX_BLOCK_SIGOPS))
-        block(36, spend=out[11], script=too_many_checksigs)
-        yield rejected(RejectResult(16, b'bad-blk-sigops'))
-
+        b36 = self.next_block(36, spend=out[11], script=too_many_checksigs)
+        self.send_blocks([b36], False, "bad-blk-sigops", reconnect=True)
 
         # Check spending of a transaction in a block which failed to connect
         #
@@ -479,17 +396,20 @@ def update_block(block_number, new_transactions):
         #
 
         # save 37's spendable output, but then double-spend out11 to invalidate the block
-        tip(35)
-        b37 = block(37, spend=out[11])
+        self.log.info("Reject a block spending transaction from a block which failed to connect")
+        self.move_tip(35)
+        b37 = self.next_block(37, spend=out[11])
         txout_b37 = PreviousSpendableOutput(b37.vtx[1], 0)
-        tx = create_and_sign_tx(out[11].tx, out[11].n, 0)
-        b37 = update_block(37, [tx])
-        yield rejected(RejectResult(16, b'bad-txns-inputs-missingorspent'))
+        tx = self.create_and_sign_transaction(out[11].tx, out[11].n, 0)
+        b37 = self.update_block(37, [tx])
+        # !TODO: fix expect_disconnect
+        self.send_blocks([b37], False, "bad-txns-inputs-missingorspent", reconnect=False)
 
         # attempt to spend b37's first non-coinbase tx, at which point b37 was still considered valid
-        tip(35)
-        block(38, spend=txout_b37)
-        yield rejected(RejectResult(16, b'bad-txns-inputs-missingorspent'))
+        self.move_tip(35)
+        b38 = self.next_block(38, spend=txout_b37)
+        # !TODO: fix expect_disconnect
+        self.send_blocks([b38], False, "bad-txns-inputs-missingorspent", reconnect=False)
 
         # Check P2SH SigOp counting
         #
@@ -502,45 +422,45 @@ def update_block(block_number, new_transactions):
         #           redeem_script = COINBASE_PUBKEY, (OP_2DUP+OP_CHECKSIGVERIFY) * 5, OP_CHECKSIG
         #           p2sh_script = OP_HASH160, ripemd160(sha256(script)), OP_EQUAL
         #
-        tip(35)
-        b39 = block(39)
+        self.log.info("Check P2SH SIGOPS are correctly counted")
+        self.move_tip(35)
+        b39 = self.next_block(39)
         b39_outputs = 0
         b39_sigops_per_output = 6
 
         # Build the redeem script, hash it, use hash to create the p2sh script
-        redeem_script = CScript([self.coinbase_pubkey] + [OP_2DUP, OP_CHECKSIGVERIFY]*5 + [OP_CHECKSIG])
+        redeem_script = CScript([self.coinbase_pubkey] + [OP_2DUP, OP_CHECKSIGVERIFY] * 5 + [OP_CHECKSIG])
         redeem_script_hash = hash160(redeem_script)
         p2sh_script = CScript([OP_HASH160, redeem_script_hash, OP_EQUAL])
 
         # Create a transaction that spends one satoshi to the p2sh_script, the rest to OP_TRUE
         # This must be signed because it is spending a coinbase
         spend = out[11]
-        tx = create_tx(spend.tx, spend.n, 1, p2sh_script)
+        tx = self.create_tx(spend.tx, spend.n, 1, p2sh_script)
         tx.vout.append(CTxOut(spend.tx.vout[spend.n].nValue - 1, CScript([OP_TRUE])))
         self.sign_tx(tx, spend.tx, spend.n)
         tx.rehash()
-        b39 = update_block(39, [tx])
+        b39 = self.update_block(39, [tx])
         b39_outputs += 1
 
         # Until block is full, add tx's with 1 satoshi to p2sh_script, the rest to OP_TRUE
         tx_new = None
         tx_last = tx
-        total_size=len(b39.serialize())
+        total_size = len(b39.serialize())
         while(total_size < MAX_BLOCK_BASE_SIZE):
-            tx_new = create_tx(tx_last, 1, 1, p2sh_script)
+            tx_new = self.create_tx(tx_last, 1, 1, p2sh_script)
             tx_new.vout.append(CTxOut(tx_last.vout[1].nValue - 1, CScript([OP_TRUE])))
             tx_new.rehash()
             total_size += len(tx_new.serialize())
             if total_size >= MAX_BLOCK_BASE_SIZE:
                 break
-            b39.vtx.append(tx_new) # add tx to block
+            b39.vtx.append(tx_new)  # add tx to block
             tx_last = tx_new
             b39_outputs += 1
 
-        b39 = update_block(39, [])
-        yield accepted()
-        save_spendable_output()
-
+        b39 = self.update_block(39, [])
+        self.send_blocks([b39], True)
+        self.save_spendable_output()
 
         # Test sigops in P2SH redeem scripts
         #
@@ -549,15 +469,16 @@ def update_block(block_number, new_transactions):
         #
         # b41 does the same, less one, so it has the maximum sigops permitted.
         #
-        tip(39)
-        b40 = block(40, spend=out[12])
+        self.log.info("Reject a block with too many P2SH sigops")
+        self.move_tip(39)
+        b40 = self.next_block(40, spend=out[12])
         sigops = get_legacy_sigopcount_block(b40)
         numTxes = (MAX_BLOCK_SIGOPS - sigops) // b39_sigops_per_output
         assert_equal(numTxes <= b39_outputs, True)
 
         lastOutpoint = COutPoint(b40.vtx[1].sha256, 0)
         new_txs = []
-        for i in range(1, numTxes+1):
+        for i in range(1, numTxes + 1):
             tx = CTransaction()
             tx.vout.append(CTxOut(1, CScript([OP_TRUE])))
             tx.vin.append(CTxIn(lastOutpoint, b''))
@@ -579,35 +500,35 @@ def update_block(block_number, new_transactions):
         tx.vout.append(CTxOut(1, CScript([OP_CHECKSIG] * b40_sigops_to_fill)))
         tx.rehash()
         new_txs.append(tx)
-        update_block(40, new_txs)
-        yield rejected(RejectResult(16, b'bad-blk-sigops'))
+        self.update_block(40, new_txs)
+        # !TODO: fix expect_disconnect
+        self.send_blocks([b40], False, "bad-blk-sigops", reconnect=False)
 
         # same as b40, but one less sigop
-        tip(39)
-        block(41, spend=None)
-        update_block(41, b40.vtx[1:-1])
+        self.log.info("Accept a block with the max number of P2SH sigops")
+        self.move_tip(39)
+        b41 = self.next_block(41, spend=None)
+        self.update_block(41, b40.vtx[1:-1])
         b41_sigops_to_fill = b40_sigops_to_fill - 1
         tx = CTransaction()
         tx.vin.append(CTxIn(lastOutpoint, b''))
         tx.vout.append(CTxOut(1, CScript([OP_CHECKSIG] * b41_sigops_to_fill)))
         tx.rehash()
-        update_block(41, [tx])
-        yield accepted()
+        self.update_block(41, [tx])
+        self.send_blocks([b41], True)
 
         # Fork off of b39 to create a constant base again
         #
         # b23 (6) -> b30 (7) -> b31 (8) -> b33 (9) -> b35 (10) -> b39 (11) -> b42 (12) -> b43 (13)
         #                                                                  \-> b41 (12)
         #
-        tip(39)
-        block(42, spend=out[12])
-        yield rejected()
-        save_spendable_output()
-
-        block(43, spend=out[13])
-        yield accepted()
-        save_spendable_output()
+        self.move_tip(39)
+        b42 = self.next_block(42, spend=out[12])
+        self.save_spendable_output()
 
+        b43 = self.next_block(43, spend=out[13])
+        self.save_spendable_output()
+        self.send_blocks([b42, b43], True)
 
         # Test a number of really invalid scenarios
         #
@@ -616,6 +537,7 @@ def update_block(block_number, new_transactions):
 
         # The next few blocks are going to be created "by hand" since they'll do funky things, such as having
         # the first transaction be non-coinbase, etc.  The purpose of b44 is to make sure this works.
+        self.log.info("Build block 44 manually")
         height = self.block_heights[self.tip.sha256] + 1
         coinbase = create_coinbase(height, self.coinbase_pubkey)
         b44 = CBlock()
@@ -628,10 +550,10 @@ def update_block(block_number, new_transactions):
         self.tip = b44
         self.block_heights[b44.sha256] = height
         self.blocks[44] = b44
-        yield accepted()
+        self.send_blocks([b44], True)
 
-        # A block with a non-coinbase as the first tx
-        non_coinbase = create_tx(out[15].tx, out[15].n, 1)
+        self.log.info("Reject a block with a non-coinbase as the first tx")
+        non_coinbase = self.create_tx(out[15].tx, out[15].n, 1)
         b45 = CBlock()
         b45.nTime = self.tip.nTime + 1
         b45.hashPrevBlock = self.tip.sha256
@@ -640,97 +562,85 @@ def update_block(block_number, new_transactions):
         b45.hashMerkleRoot = b45.calc_merkle_root()
         b45.calc_sha256()
         b45.solve()
-        self.block_heights[b45.sha256] = self.block_heights[self.tip.sha256]+1
+        self.block_heights[b45.sha256] = self.block_heights[self.tip.sha256] + 1
         self.tip = b45
         self.blocks[45] = b45
-        yield rejected(RejectResult(16, b'bad-cb-missing'))
+        self.send_blocks([b45], False, "bad-cb-missing", reconnect=True)
 
-        # A block with no txns
-        tip(44)
+        self.log.info("Reject a block with no transactions")
+        self.move_tip(44)
         b46 = CBlock()
-        b46.nTime = b44.nTime+1
+        b46.nTime = b44.nTime + 1
         b46.hashPrevBlock = b44.sha256
         b46.nBits = 0x207fffff
         b46.vtx = []
         b46.hashMerkleRoot = 0
         b46.solve()
-        self.block_heights[b46.sha256] = self.block_heights[b44.sha256]+1
+        self.block_heights[b46.sha256] = self.block_heights[b44.sha256] + 1
         self.tip = b46
         assert 46 not in self.blocks
         self.blocks[46] = b46
-        s = ser_uint256(b46.hashMerkleRoot)
-        yield rejected(RejectResult(16, b'bad-blk-length'))
+        self.send_blocks([b46], False, "bad-blk-length", reconnect=True)
 
-        # A block with invalid work
-        tip(44)
-        b47 = block(47, solve=False)
+        self.log.info("Reject a block with invalid work")
+        self.move_tip(44)
+        b47 = self.next_block(47, solve=False)
         target = uint256_from_compact(b47.nBits)
-        while b47.sha256 < target: #changed > to <
+        while b47.sha256 < target:
             b47.nNonce += 1
             b47.rehash()
-        yield rejected(RejectResult(16, b'high-hash'))
-
-        # A block with timestamp > 2 hrs in the future
-        tip(44)
-        b48 = block(48, solve=False)
-        b48.nTime = int(time.time()) + 60 * 60 * 3
-        b48.solve()
-        yield rejected(RejectResult(16, b'time-too-new'))
-
-        # A block with an invalid merkle hash
-        tip(44)
-        b49 = block(49)
+        self.send_blocks([b47], False)
+
+        self.log.info("Reject a block with invalid merkle hash")
+        self.move_tip(44)
+        b49 = self.next_block(49)
         b49.hashMerkleRoot += 1
         b49.solve()
-        yield rejected(RejectResult(16, b'bad-txnmrklroot'))
+        self.send_blocks([b49], False, "bad-txnmrklroot", reconnect=True)
 
-        # A block with an incorrect POW limit
-        tip(44)
-        b50 = block(50)
+        self.log.info("Reject a block with incorrect POW limit")
+        self.move_tip(44)
+        b50 = self.next_block(50)
         b50.nBits = b50.nBits - 1
         b50.solve()
-        yield rejected(RejectResult(16, b'bad-diffbits'))
+        # !TODO: fix expect_disconnect
+        self.send_blocks([b50], False, reconnect=False)
 
-        # A block with two coinbase txns
-        tip(44)
-        b51 = block(51)
+        self.log.info("Reject a block with two coinbase transactions")
+        self.move_tip(44)
+        b51 = self.next_block(51)
         cb2 = create_coinbase(51, self.coinbase_pubkey)
-        b51 = update_block(51, [cb2])
-        yield rejected(RejectResult(16, b'bad-cb-multiple'))
+        b51 = self.update_block(51, [cb2])
+        self.send_blocks([b51], False, "bad-cb-multiple", reconnect=True)
 
-        # A block w/ duplicate txns
+        self.log.info("Reject a block with duplicate transactions")
         # Note: txns have to be in the right position in the merkle tree to trigger this error
-        tip(44)
-        b52 = block(52, spend=out[15])
-        tx = create_tx(b52.vtx[1], 0, 1)
-        b52 = update_block(52, [tx, tx])
-        yield rejected(RejectResult(16, b'bad-txns-duplicate'))
+        self.move_tip(44)
+        b52 = self.next_block(52, spend=out[15])
+        tx = self.create_tx(b52.vtx[1], 0, 1)
+        b52 = self.update_block(52, [tx, tx])
+        self.send_blocks([b52], False, "bad-txns-duplicate", reconnect=True)
 
         # Test block timestamps
         #  -> b31 (8) -> b33 (9) -> b35 (10) -> b39 (11) -> b42 (12) -> b43 (13) -> b53 (14) -> b55 (15)
         #                                                                                   \-> b54 (15)
         #
-        tip(43)
-        block(53, spend=out[14])
-        yield rejected() # rejected since b44 is at same height
-        save_spendable_output()
+        self.move_tip(43)
+        b53 = self.next_block(53, spend=out[14])
+        self.send_blocks([b53], False)
+        self.save_spendable_output()
 
-        # invalid timestamp (b35 is 5 blocks back, so its time is MedianTimePast)
-        b54 = block(54, spend=out[15])
-        b54.nTime = b35.nTime - 1
-        b54.solve()
-        yield rejected(RejectResult(16, b'time-too-old'))
+        # PIVX: timestamp checks disabled for regtest
 
         # valid timestamp
-        tip(53)
-        b55 = block(55, spend=out[15])
+        self.move_tip(53)
+        b55 = self.next_block(55, spend=out[15])
         b55.nTime = b35.nTime
-        update_block(55, [])
-        yield accepted()
-        save_spendable_output()
-
+        self.update_block(55, [])
+        self.send_blocks([b55], True)
+        self.save_spendable_output()
 
-        # Test CVE-2012-2459
+        # Test Merkle tree malleability
         #
         # -> b42 (12) -> b43 (13) -> b53 (14) -> b55 (15) -> b57p2 (16)
         #                                                \-> b57   (16)
@@ -758,46 +668,48 @@ def update_block(block_number, new_transactions):
         #  that the error was caught early, avoiding a DOS vulnerability.)
 
         # b57 - a good block with 2 txs, don't submit until end
-        tip(55)
-        b57 = block(57)
-        tx = create_and_sign_tx(out[16].tx, out[16].n, 1)
-        tx1 = create_tx(tx, 0, 1)
-        b57 = update_block(57, [tx, tx1])
+        self.move_tip(55)
+        b57 = self.next_block(57)
+        tx = self.create_and_sign_transaction(out[16].tx, out[16].n, 1)
+        tx1 = self.create_tx(tx, 0, 1)
+        b57 = self.update_block(57, [tx, tx1])
 
         # b56 - copy b57, add a duplicate tx
-        tip(55)
+        self.log.info("Reject a block with a duplicate transaction in the Merkle Tree (but with a valid Merkle Root)")
+        self.move_tip(55)
         b56 = copy.deepcopy(b57)
         self.blocks[56] = b56
-        assert_equal(len(b56.vtx),3)
-        b56 = update_block(56, [tx1])
+        assert_equal(len(b56.vtx), 3)
+        b56 = self.update_block(56, [tx1])
         assert_equal(b56.hash, b57.hash)
-        yield rejected(RejectResult(16, b'bad-txns-duplicate'))
+        self.send_blocks([b56], False, "bad-txns-duplicate", reconnect=True)
 
         # b57p2 - a good block with 6 tx'es, don't submit until end
-        tip(55)
-        b57p2 = block("57p2")
-        tx = create_and_sign_tx(out[16].tx, out[16].n, 1)
-        tx1 = create_tx(tx, 0, 1)
-        tx2 = create_tx(tx1, 0, 1)
-        tx3 = create_tx(tx2, 0, 1)
-        tx4 = create_tx(tx3, 0, 1)
-        b57p2 = update_block("57p2", [tx, tx1, tx2, tx3, tx4])
+        self.move_tip(55)
+        b57p2 = self.next_block("57p2")
+        tx = self.create_and_sign_transaction(out[16].tx, out[16].n, 1)
+        tx1 = self.create_tx(tx, 0, 1)
+        tx2 = self.create_tx(tx1, 0, 1)
+        tx3 = self.create_tx(tx2, 0, 1)
+        tx4 = self.create_tx(tx3, 0, 1)
+        b57p2 = self.update_block("57p2", [tx, tx1, tx2, tx3, tx4])
 
         # b56p2 - copy b57p2, duplicate two non-consecutive tx's
-        tip(55)
+        self.log.info("Reject a block with two duplicate transactions in the Merkle Tree (but with a valid Merkle Root)")
+        self.move_tip(55)
         b56p2 = copy.deepcopy(b57p2)
         self.blocks["b56p2"] = b56p2
         assert_equal(b56p2.hash, b57p2.hash)
-        assert_equal(len(b56p2.vtx),6)
-        b56p2 = update_block("b56p2", [tx3, tx4])
-        yield rejected(RejectResult(16, b'bad-txns-duplicate'))
+        assert_equal(len(b56p2.vtx), 6)
+        b56p2 = self.update_block("b56p2", [tx3, tx4])
+        self.send_blocks([b56p2], False, "bad-txns-duplicate", reconnect=True)
 
-        tip("57p2")
-        yield accepted()
+        self.move_tip("57p2")
+        self.send_blocks([b57p2], True)
 
-        tip(57)
-        yield rejected()  #rejected because 57p2 seen first
-        save_spendable_output()
+        self.move_tip(57)
+        self.send_blocks([b57], False)  # The tip is not updated because 57p2 seen first
+        self.save_spendable_output()
 
         # Test a few invalid tx types
         #
@@ -806,130 +718,117 @@ def update_block(block_number, new_transactions):
         #
 
         # tx with prevout.n out of range
-        tip(57)
-        b58 = block(58, spend=out[17])
+        self.log.info("Reject a block with a transaction with prevout.n out of range")
+        self.move_tip(57)
+        b58 = self.next_block(58, spend=out[17])
         tx = CTransaction()
         assert(len(out[17].tx.vout) < 42)
         tx.vin.append(CTxIn(COutPoint(out[17].tx.sha256, 42), CScript([OP_TRUE]), 0xffffffff))
-        tx.vout.append(CTxOut(0, b""))
+        tx.vout.append(CTxOut(0, CScript([OP_TRUE])))
         tx.calc_sha256()
-        b58 = update_block(58, [tx])
-        yield rejected(RejectResult(16, b'bad-txns-inputs-missingorspent'))
-
-        # tx with output value > input value out of range
-        tip(57)
-        b59 = block(59)
-        tx = create_and_sign_tx(out[17].tx, out[17].n, 51*COIN)
-        b59 = update_block(59, [tx])
-        yield rejected(RejectResult(16, b'bad-txns-in-belowout'))
+        b58 = self.update_block(58, [tx])
+        # !TODO: fix expect_disconnect
+        self.send_blocks([b58], False, "bad-txns-inputs-missingorspent", reconnect=False)
+
+        # tx with output value > input value
+        self.log.info("Reject a block with a transaction with outputs > inputs")
+        self.move_tip(57)
+        b59 = self.next_block(59)
+        tx = self.create_and_sign_transaction(out[17].tx, out[17].n, 251 * COIN)
+        b59 = self.update_block(59, [tx])
+        # !TODO: fix expect_disconnect
+        self.send_blocks([b59], False, "bad-txns-in-belowout", reconnect=False)
 
         # reset to good chain
-        tip(57)
-        b60 = block(60, spend=out[17])
-        yield accepted()
-        save_spendable_output()
+        self.move_tip(57)
+        b60 = self.next_block(60, spend=out[17])
+        self.send_blocks([b60], True)
+        self.save_spendable_output()
 
+        # Test BIP30
+        #
+        # -> b39 (11) -> b42 (12) -> b43 (13) -> b53 (14) -> b55 (15) -> b57 (16) -> b60 (17)
+        #                                                                                    \-> b61 (18)
+        #
+        # Blocks are not allowed to contain a transaction whose id matches that of an earlier,
+        # not-fully-spent transaction in the same chain. To test, make identical coinbases;
+        # the second one should be rejected.
+        #
+        self.log.info("Reject a block with a transaction with a duplicate hash of a previous transaction (BIP30)")
+        self.move_tip(60)
+        b61 = self.next_block(61, spend=out[18])
+        b61.vtx[0].vin[0].scriptSig = b60.vtx[0].vin[0].scriptSig  # Equalize the coinbases
+        b61.vtx[0].rehash()
+        b61 = self.update_block(61, [])
+        assert_equal(b60.vtx[0].serialize(), b61.vtx[0].serialize())
+        self.send_blocks([b61], False, "bad-cb-height", reconnect=True)
 
         # Test tx.isFinal is properly rejected (not an exhaustive tx.isFinal test, that should be in data-driven transaction tests)
         #
         #   -> b39 (11) -> b42 (12) -> b43 (13) -> b53 (14) -> b55 (15) -> b57 (16) -> b60 (17)
         #                                                                                     \-> b62 (18)
         #
-        tip(60)
-        b62 = block(62)
+        self.log.info("Reject a block with a transaction with a nonfinal locktime")
+        self.move_tip(60)
+        b62 = self.next_block(62)
         tx = CTransaction()
-        tx.nLockTime = 0xffffffff  #this locktime is non-final
+        tx.nLockTime = 0xffffffff  # this locktime is non-final
         assert(out[18].n < len(out[18].tx.vout))
-        tx.vin.append(CTxIn(COutPoint(out[18].tx.sha256, out[18].n))) # don't set nSequence
+        tx.vin.append(CTxIn(COutPoint(out[18].tx.sha256, out[18].n)))  # don't set nSequence
         tx.vout.append(CTxOut(0, CScript([OP_TRUE])))
         assert(tx.vin[0].nSequence < 0xffffffff)
         tx.calc_sha256()
-        b62 = update_block(62, [tx])
-        yield rejected(RejectResult(16, b'bad-txns-nonfinal'))
-
+        b62 = self.update_block(62, [tx])
+        self.send_blocks([b62], False, "bad-txns-nonfinal")
 
         # Test a non-final coinbase is also rejected
         #
         #   -> b39 (11) -> b42 (12) -> b43 (13) -> b53 (14) -> b55 (15) -> b57 (16) -> b60 (17)
         #                                                                                     \-> b63 (-)
         #
-        tip(60)
-        b63 = block(63)
+        self.log.info("Reject a block with a coinbase transaction with a nonfinal locktime")
+        self.move_tip(60)
+        b63 = self.next_block(63)
         b63.vtx[0].nLockTime = 0xffffffff
         b63.vtx[0].vin[0].nSequence = 0xDEADBEEF
         b63.vtx[0].rehash()
-        b63 = update_block(63, [])
-        yield rejected(RejectResult(16, b'bad-txns-nonfinal'))
-
+        b63 = self.update_block(63, [])
+        self.send_blocks([b63], False, "bad-txns-nonfinal")
 
-        #  This checks that a block with a bloated VARINT between the block_header and the array of tx such that
-        #  the block is > MAX_BLOCK_BASE_SIZE with the bloated varint, but <= MAX_BLOCK_BASE_SIZE without the bloated varint,
-        #  does not cause a subsequent, identical block with canonical encoding to be rejected.  The test does not
-        #  care whether the bloated block is accepted or rejected; it only cares that the second block is accepted.
-        #
-        #  What matters is that the receiving node should not reject the bloated block, and then reject the canonical
-        #  block on the basis that it's the same as an already-rejected block (which would be a consensus failure.)
         #
         #  -> b39 (11) -> b42 (12) -> b43 (13) -> b53 (14) -> b55 (15) -> b57 (16) -> b60 (17) -> b64 (18)
-        #                                                                                        \
-        #                                                                                         b64a (18)
-        #  b64a is a bloated block (non-canonical varint)
-        #  b64 is a good block (same as b64 but w/ canonical varint)
-        #
-        tip(60)
-        regular_block = block("64a", spend=out[18])
-
-        # make it a "broken_block," with non-canonical serialization
-        b64a = CBrokenBlock(regular_block)
-        b64a.initialize(regular_block)
-        self.blocks["64a"] = b64a
-        self.tip = b64a
-        tx = CTransaction()
-
-        # use canonical serialization to calculate size
-        script_length = MAX_BLOCK_BASE_SIZE - len(b64a.normal_serialize()) - 69
-        script_output = CScript([b'\x00' * script_length])
-        tx.vout.append(CTxOut(0, script_output))
-        tx.vin.append(CTxIn(COutPoint(b64a.vtx[1].sha256, 0)))
-        b64a = update_block("64a", [tx])
-        assert_equal(len(b64a.serialize()), MAX_BLOCK_BASE_SIZE + 8)
-        yield TestInstance([[self.tip, None]])
-
-        # comptool workaround: to make sure b64 is delivered, manually erase b64a from blockstore
-        self.test.block_store.erase(b64a.sha256)
-
-        tip(60)
-        b64 = CBlock(b64a)
-        b64.vtx = copy.deepcopy(b64a.vtx)
-        assert_equal(b64.hash, b64a.hash)
-        assert_equal(len(b64.serialize()), MAX_BLOCK_BASE_SIZE)
-        self.blocks[64] = b64
-        update_block(64, [])
-        yield accepted()
-        save_spendable_output()
+        #
+        self.move_tip(60)
+        b64 = self.next_block(64)
+        b64 = self.update_block(64, [])
+        self.send_blocks([b64], True)
+        self.save_spendable_output()
 
         # Spend an output created in the block itself
         #
         # -> b42 (12) -> b43 (13) -> b53 (14) -> b55 (15) -> b57 (16) -> b60 (17) -> b64 (18) -> b65 (19)
         #
-        tip(64)
-        block(65)
-        tx1 = create_and_sign_tx(out[19].tx, out[19].n, out[19].tx.vout[0].nValue)
-        tx2 = create_and_sign_tx(tx1, 0, 0)
-        update_block(65, [tx1, tx2])
-        yield accepted()
-        save_spendable_output()
+        self.log.info("Accept a block with a transaction spending an output created in the same block")
+        self.move_tip(64)
+        b65 = self.next_block(65)
+        tx1 = self.create_and_sign_transaction(out[19].tx, out[19].n, out[19].tx.vout[0].nValue)
+        tx2 = self.create_and_sign_transaction(tx1, 0, 0)
+        b65 = self.update_block(65, [tx1, tx2])
+        self.send_blocks([b65], True)
+        self.save_spendable_output()
 
         # Attempt to spend an output created later in the same block
         #
         # -> b43 (13) -> b53 (14) -> b55 (15) -> b57 (16) -> b60 (17) -> b64 (18) -> b65 (19)
         #                                                                                    \-> b66 (20)
-        tip(65)
-        block(66)
-        tx1 = create_and_sign_tx(out[20].tx, out[20].n, out[20].tx.vout[0].nValue)
-        tx2 = create_and_sign_tx(tx1, 0, 1)
-        update_block(66, [tx2, tx1])
-        yield rejected(RejectResult(16, b'bad-txns-inputs-missingorspent'))
+        self.log.info("Reject a block with a transaction spending an output created later in the same block")
+        self.move_tip(65)
+        b66 = self.next_block(66)
+        tx1 = self.create_and_sign_transaction(out[20].tx, out[20].n, out[20].tx.vout[0].nValue)
+        tx2 = self.create_and_sign_transaction(tx1, 0, 1)
+        b66 = self.update_block(66, [tx2, tx1])
+        # !TODO: fix expect_disconnect
+        self.send_blocks([b66], False, "bad-txns-inputs-missingorspent", reconnect=False)
 
         # Attempt to double-spend a transaction created in a block
         #
@@ -937,13 +836,15 @@ def update_block(block_number, new_transactions):
         #                                                                                    \-> b67 (20)
         #
         #
-        tip(65)
-        block(67)
-        tx1 = create_and_sign_tx(out[20].tx, out[20].n, out[20].tx.vout[0].nValue)
-        tx2 = create_and_sign_tx(tx1, 0, 1)
-        tx3 = create_and_sign_tx(tx1, 0, 2)
-        update_block(67, [tx1, tx2, tx3])
-        yield rejected(RejectResult(16, b'bad-txns-inputs-missingorspent'))
+        self.log.info("Reject a block with a transaction double spending a transaction created in the same block")
+        self.move_tip(65)
+        b67 = self.next_block(67)
+        tx1 = self.create_and_sign_transaction(out[20].tx, out[20].n, out[20].tx.vout[0].nValue)
+        tx2 = self.create_and_sign_transaction(tx1, 0, 1)
+        tx3 = self.create_and_sign_transaction(tx1, 0, 2)
+        b67 = self.update_block(67, [tx1, tx2, tx3])
+        # !TODO: fix expect_disconnect
+        self.send_blocks([b67], False, "bad-txns-inputs-missingorspent", reconnect=False)
 
         # More tests of block subsidy
         #
@@ -957,34 +858,38 @@ def update_block(block_number, new_transactions):
         # b69 - coinbase with extra 10 satoshis, and a tx that gives a 10 satoshi fee
         #       this succeeds
         #
-        tip(65)
-        block(68, additional_coinbase_value=10)
-        tx = create_and_sign_tx(out[20].tx, out[20].n, out[20].tx.vout[0].nValue-9)
-        update_block(68, [tx])
-        yield rejected(RejectResult(16, b'bad-cb-amount'))
-
-        tip(65)
-        b69 = block(69, additional_coinbase_value=10)
-        tx = create_and_sign_tx(out[20].tx, out[20].n, out[20].tx.vout[0].nValue-10)
-        update_block(69, [tx])
-        yield accepted()
-        save_spendable_output()
+        self.log.info("Reject a block trying to claim too much subsidy in the coinbase transaction")
+        self.move_tip(65)
+        b68 = self.next_block(68, additional_coinbase_value=10)
+        tx = self.create_and_sign_transaction(out[20].tx, out[20].n, out[20].tx.vout[0].nValue - 9)
+        b68 = self.update_block(68, [tx])
+        # !TODO: fix expect_disconnect
+        self.send_blocks([b68], False, "bad-blk-amount", reconnect=False)
+
+        self.log.info("Accept a block claiming the correct subsidy in the coinbase transaction")
+        self.move_tip(65)
+        b69 = self.next_block(69, additional_coinbase_value=10)
+        tx = self.create_and_sign_transaction(out[20].tx, out[20].n, out[20].tx.vout[0].nValue - 10)
+        self.update_block(69, [tx])
+        self.send_blocks([b69], True)
+        self.save_spendable_output()
 
         # Test spending the outpoint of a non-existent transaction
         #
         # -> b53 (14) -> b55 (15) -> b57 (16) -> b60 (17) -> b64 (18) -> b65 (19) -> b69 (20)
         #                                                                                    \-> b70 (21)
         #
-        tip(69)
-        block(70, spend=out[21])
+        self.log.info("Reject a block containing a transaction spending from a non-existent input")
+        self.move_tip(69)
+        b70 = self.next_block(70, spend=out[21])
         bogus_tx = CTransaction()
         bogus_tx.sha256 = uint256_from_str(b"23c70ed7c0506e9178fc1a987f40a33946d4ad4c962b5ae3a52546da53af0c5c")
         tx = CTransaction()
         tx.vin.append(CTxIn(COutPoint(bogus_tx.sha256, 0), b"", 0xffffffff))
         tx.vout.append(CTxOut(1, b""))
-        update_block(70, [tx])
-        yield rejected(RejectResult(16, b'bad-txns-inputs-missingorspent'))
-
+        b70 = self.update_block(70, [tx])
+        # !TODO: fix expect_disconnect
+        self.send_blocks([b70], False, "bad-txns-inputs-missingorspent", reconnect=False)
 
         # Test accepting an invalid block which has the same hash as a valid one (via merkle tree tricks)
         #
@@ -992,13 +897,13 @@ def update_block(block_number, new_transactions):
         #                                                                                      \-> b71 (21)
         #
         # b72 is a good block.
-        # b71 is a copy of 72, but re-adds one of its transactions.  However, it has the same hash as b71.
-        #
-        tip(69)
-        b72 = block(72)
-        tx1 = create_and_sign_tx(out[21].tx, out[21].n, 2)
-        tx2 = create_and_sign_tx(tx1, 0, 1)
-        b72 = update_block(72, [tx1, tx2])  # now tip is 72
+        # b71 is a copy of 72, but re-adds one of its transactions.  However, it has the same hash as b72.
+        self.log.info("Reject a block containing a duplicate transaction but with the same Merkle root (Merkle tree malleability")
+        self.move_tip(69)
+        b72 = self.next_block(72)
+        tx1 = self.create_and_sign_transaction(out[21].tx, out[21].n, 2)
+        tx2 = self.create_and_sign_transaction(tx1, 0, 1)
+        b72 = self.update_block(72, [tx1, tx2])  # now tip is 72
         b71 = copy.deepcopy(b72)
         b71.vtx.append(tx2)   # add duplicate tx2
         self.block_heights[b71.sha256] = self.block_heights[b69.sha256] + 1  # b71 builds off b69
@@ -1008,12 +913,12 @@ def update_block(block_number, new_transactions):
         assert_equal(len(b72.vtx), 3)
         assert_equal(b72.sha256, b71.sha256)
 
-        tip(71)
-        yield rejected(RejectResult(16, b'bad-txns-duplicate'))
-        tip(72)
-        yield accepted()
-        save_spendable_output()
+        self.move_tip(71)
+        self.send_blocks([b71], False, "bad-txns-duplicate", reconnect=True)
 
+        self.move_tip(72)
+        self.send_blocks([b72], True)
+        self.save_spendable_output()
 
         # Test some invalid scripts and MAX_BLOCK_SIGOPS
         #
@@ -1031,23 +936,23 @@ def update_block(block_number, new_transactions):
         #       bytearray[20,000-20,003]: 521  (max_script_element_size+1, in little-endian format)
         #       bytearray[20,004-20,525]: unread data (script_element)
         #       bytearray[20,526]       : OP_CHECKSIG (this puts us over the limit)
-        #
-        tip(72)
-        b73 = block(73)
+        self.log.info("Reject a block containing too many sigops after a large script element")
+        self.move_tip(72)
+        b73 = self.next_block(73)
         size = MAX_BLOCK_SIGOPS - 1 + MAX_SCRIPT_ELEMENT_SIZE + 1 + 5 + 1
         a = bytearray([OP_CHECKSIG] * size)
-        a[MAX_BLOCK_SIGOPS - 1] = int("4e",16) # OP_PUSHDATA4
+        a[MAX_BLOCK_SIGOPS - 1] = int("4e", 16)  # OP_PUSHDATA4
 
         element_size = MAX_SCRIPT_ELEMENT_SIZE + 1
         a[MAX_BLOCK_SIGOPS] = element_size % 256
-        a[MAX_BLOCK_SIGOPS+1] = element_size // 256
-        a[MAX_BLOCK_SIGOPS+2] = 0
-        a[MAX_BLOCK_SIGOPS+3] = 0
+        a[MAX_BLOCK_SIGOPS + 1] = element_size // 256
+        a[MAX_BLOCK_SIGOPS + 2] = 0
+        a[MAX_BLOCK_SIGOPS + 3] = 0
 
-        tx = create_and_sign_tx(out[22].tx, 0, 1, CScript(a))
-        b73 = update_block(73, [tx])
-        assert_equal(get_legacy_sigopcount_block(b73), MAX_BLOCK_SIGOPS+1)
-        yield rejected(RejectResult(16, b'bad-blk-sigops'))
+        tx = self.create_and_sign_transaction(out[22].tx, 0, 1, CScript(a))
+        b73 = self.update_block(73, [tx])
+        assert_equal(get_legacy_sigopcount_block(b73), MAX_BLOCK_SIGOPS + 1)
+        self.send_blocks([b73], False, 'bad-blk-sigops', reconnect=True)
 
         # b74/75 - if we push an invalid script element, all prevous sigops are counted,
         #          but sigops after the element are not counted.
@@ -1059,45 +964,44 @@ def update_block(block_number, new_transactions):
         #
         #       b74 fails because we put MAX_BLOCK_SIGOPS+1 before the element
         #       b75 succeeds because we put MAX_BLOCK_SIGOPS before the element
-        #
-        #
-        tip(72)
-        b74 = block(74)
-        size = MAX_BLOCK_SIGOPS - 1 + MAX_SCRIPT_ELEMENT_SIZE + 42 # total = 20,561
+        self.log.info("Check sigops are counted correctly after an invalid script element")
+        self.move_tip(72)
+        b74 = self.next_block(74)
+        size = MAX_BLOCK_SIGOPS - 1 + MAX_SCRIPT_ELEMENT_SIZE + 42  # total = 20,561
         a = bytearray([OP_CHECKSIG] * size)
         a[MAX_BLOCK_SIGOPS] = 0x4e
-        a[MAX_BLOCK_SIGOPS+1] = 0xfe
-        a[MAX_BLOCK_SIGOPS+2] = 0xff
-        a[MAX_BLOCK_SIGOPS+3] = 0xff
-        a[MAX_BLOCK_SIGOPS+4] = 0xff
-        tx = create_and_sign_tx(out[22].tx, 0, 1, CScript(a))
-        b74 = update_block(74, [tx])
-        yield rejected(RejectResult(16, b'bad-blk-sigops'))
-
-        tip(72)
-        b75 = block(75)
+        a[MAX_BLOCK_SIGOPS + 1] = 0xfe
+        a[MAX_BLOCK_SIGOPS + 2] = 0xff
+        a[MAX_BLOCK_SIGOPS + 3] = 0xff
+        a[MAX_BLOCK_SIGOPS + 4] = 0xff
+        tx = self.create_and_sign_transaction(out[22].tx, 0, 1, CScript(a))
+        b74 = self.update_block(74, [tx])
+        self.send_blocks([b74], False, 'bad-blk-sigops', reconnect=True)
+
+        self.move_tip(72)
+        b75 = self.next_block(75)
         size = MAX_BLOCK_SIGOPS - 1 + MAX_SCRIPT_ELEMENT_SIZE + 42
         a = bytearray([OP_CHECKSIG] * size)
-        a[MAX_BLOCK_SIGOPS-1] = 0x4e
+        a[MAX_BLOCK_SIGOPS - 1] = 0x4e
         a[MAX_BLOCK_SIGOPS] = 0xff
-        a[MAX_BLOCK_SIGOPS+1] = 0xff
-        a[MAX_BLOCK_SIGOPS+2] = 0xff
-        a[MAX_BLOCK_SIGOPS+3] = 0xff
-        tx = create_and_sign_tx(out[22].tx, 0, 1, CScript(a))
-        b75 = update_block(75, [tx])
-        yield accepted()
-        save_spendable_output()
+        a[MAX_BLOCK_SIGOPS + 1] = 0xff
+        a[MAX_BLOCK_SIGOPS + 2] = 0xff
+        a[MAX_BLOCK_SIGOPS + 3] = 0xff
+        tx = self.create_and_sign_transaction(out[22].tx, 0, 1, CScript(a))
+        b75 = self.update_block(75, [tx])
+        self.send_blocks([b75], True)
+        self.save_spendable_output()
 
         # Check that if we push an element filled with CHECKSIGs, they are not counted
-        tip(75)
-        b76 = block(76)
+        self.move_tip(75)
+        b76 = self.next_block(76)
         size = MAX_BLOCK_SIGOPS - 1 + MAX_SCRIPT_ELEMENT_SIZE + 1 + 5
         a = bytearray([OP_CHECKSIG] * size)
-        a[MAX_BLOCK_SIGOPS-1] = 0x4e # PUSHDATA4, but leave the following bytes as just checksigs
-        tx = create_and_sign_tx(out[23].tx, 0, 1, CScript(a))
-        b76 = update_block(76, [tx])
-        yield accepted()
-        save_spendable_output()
+        a[MAX_BLOCK_SIGOPS - 1] = 0x4e  # PUSHDATA4, but leave the following bytes as just checksigs
+        tx = self.create_and_sign_transaction(out[23].tx, 0, 1, CScript(a))
+        b76 = self.update_block(76, [tx])
+        self.send_blocks([b76], True)
+        self.save_spendable_output()
 
         # Test transaction resurrection
         #
@@ -1108,7 +1012,7 @@ def update_block(block_number, new_transactions):
         #
         #    The tx'es must be unsigned and pass the node's mempool policy.  It is unsigned for the
         #    rather obscure reason that the Python signature code does not distinguish between
-        #    Low-S and High-S values (whereas the bitcoin code has custom code which does so);
+        #    Low-S and High-S values (whereas the pivx code has custom code which does so);
         #    as a result of which, the odds are 50% that the python code will use the right
         #    value and the transaction will be accepted into the mempool. Until we modify the
         #    test framework to support low-S signing, we are out of luck.
@@ -1116,39 +1020,39 @@ def update_block(block_number, new_transactions):
         #    To get around this issue, we construct transactions which are not signed and which
         #    spend to OP_TRUE.  If the standard-ness rules change, this test would need to be
         #    updated.  (Perhaps to spend to a P2SH OP_TRUE script)
-        #
-        tip(76)
-        block(77)
-        tx77 = create_and_sign_tx(out[24].tx, out[24].n, 10*COIN)
-        update_block(77, [tx77])
-        yield accepted()
-        save_spendable_output()
-
-        block(78)
-        tx78 = create_tx(tx77, 0, 9*COIN)
-        update_block(78, [tx78])
-        yield accepted()
-
-        block(79)
-        tx79 = create_tx(tx78, 0, 8*COIN)
-        update_block(79, [tx79])
-        yield accepted()
+        self.log.info("Test transaction resurrection during a re-org")
+        self.move_tip(76)
+        b77 = self.next_block(77)
+        tx77 = self.create_and_sign_transaction(out[24].tx, out[24].n, 10 * COIN)
+        b77 = self.update_block(77, [tx77])
+        self.send_blocks([b77], True)
+        self.save_spendable_output()
+
+        b78 = self.next_block(78)
+        tx78 = self.create_tx(tx77, 0, 9 * COIN)
+        b78 = self.update_block(78, [tx78])
+        self.send_blocks([b78], True)
+
+        b79 = self.next_block(79)
+        tx79 = self.create_tx(tx78, 0, 8 * COIN)
+        b79 = self.update_block(79, [tx79])
+        self.send_blocks([b79], True)
 
         # mempool should be empty
         assert_equal(len(self.nodes[0].getrawmempool()), 0)
 
-        tip(77)
-        block(80, spend=out[25])
-        yield rejected()
-        save_spendable_output()
+        self.move_tip(77)
+        b80 = self.next_block(80, spend=out[25])
+        self.send_blocks([b80], False)
+        self.save_spendable_output()
 
-        block(81, spend=out[26])
-        yield rejected() # other chain is same length
-        save_spendable_output()
+        b81 = self.next_block(81, spend=out[26])
+        self.send_blocks([b81], False)  # other chain is same length
+        self.save_spendable_output()
 
-        block(82, spend=out[27])
-        yield accepted()  # now this chain is longer, triggers re-org
-        save_spendable_output()
+        b82 = self.next_block(82, spend=out[27])
+        self.send_blocks([b82], True)  # now this chain is longer, triggers re-org
+        self.save_spendable_output()
 
         # now check that tx78 and tx79 have been put back into the peer's mempool
         mempool = self.nodes[0].getrawmempool()
@@ -1156,33 +1060,32 @@ def update_block(block_number, new_transactions):
         assert(tx78.hash in mempool)
         assert(tx79.hash in mempool)
 
-
         # Test invalid opcodes in dead execution paths.
         #
         #  -> b81 (26) -> b82 (27) -> b83 (28)
         #
-        block(83)
+        self.log.info("Accept a block with invalid opcodes in dead execution paths")
+        b83 = self.next_block(83)
         op_codes = [OP_IF, OP_INVALIDOPCODE, OP_ELSE, OP_TRUE, OP_ENDIF]
         script = CScript(op_codes)
-        tx1 = create_and_sign_tx(out[28].tx, out[28].n, out[28].tx.vout[0].nValue, script)
+        tx1 = self.create_and_sign_transaction(out[28].tx, out[28].n, out[28].tx.vout[0].nValue, script)
 
-        tx2 = create_and_sign_tx(tx1, 0, 0, CScript([OP_TRUE]))
+        tx2 = self.create_and_sign_transaction(tx1, 0, 0, CScript([OP_TRUE]))
         tx2.vin[0].scriptSig = CScript([OP_FALSE])
         tx2.rehash()
 
-        update_block(83, [tx1, tx2])
-        yield accepted()
-        save_spendable_output()
-
+        b83 = self.update_block(83, [tx1, tx2])
+        self.send_blocks([b83], True)
+        self.save_spendable_output()
 
         # Reorg on/off blocks that have OP_RETURN in them (and try to spend them)
         #
         #  -> b81 (26) -> b82 (27) -> b83 (28) -> b84 (29) -> b87 (30) -> b88 (31)
         #                                    \-> b85 (29) -> b86 (30)            \-> b89a (32)
         #
-        #
-        block(84)
-        tx1 = create_tx(out[29].tx, out[29].n, 0, CScript([OP_RETURN]))
+        self.log.info("Test re-orging blocks with OP_RETURN in them")
+        b84 = self.next_block(84)
+        tx1 = self.create_tx(out[29].tx, out[29].n, 0, CScript([OP_RETURN]))
         tx1.vout.append(CTxOut(0, CScript([OP_TRUE])))
         tx1.vout.append(CTxOut(0, CScript([OP_TRUE])))
         tx1.vout.append(CTxOut(0, CScript([OP_TRUE])))
@@ -1190,86 +1093,186 @@ def update_block(block_number, new_transactions):
         tx1.calc_sha256()
         self.sign_tx(tx1, out[29].tx, out[29].n)
         tx1.rehash()
-        tx2 = create_tx(tx1, 1, 0, CScript([OP_RETURN]))
+        tx2 = self.create_tx(tx1, 1, 0, CScript([OP_RETURN]))
         tx2.vout.append(CTxOut(0, CScript([OP_RETURN])))
-        tx3 = create_tx(tx1, 2, 0, CScript([OP_RETURN]))
+        tx3 = self.create_tx(tx1, 2, 0, CScript([OP_RETURN]))
         tx3.vout.append(CTxOut(0, CScript([OP_TRUE])))
-        tx4 = create_tx(tx1, 3, 0, CScript([OP_TRUE]))
+        tx4 = self.create_tx(tx1, 3, 0, CScript([OP_TRUE]))
         tx4.vout.append(CTxOut(0, CScript([OP_RETURN])))
-        tx5 = create_tx(tx1, 4, 0, CScript([OP_RETURN]))
+        tx5 = self.create_tx(tx1, 4, 0, CScript([OP_RETURN]))
 
-        update_block(84, [tx1,tx2,tx3,tx4,tx5])
-        yield accepted()
-        save_spendable_output()
+        b84 = self.update_block(84, [tx1, tx2, tx3, tx4, tx5])
+        self.send_blocks([b84], True)
+        self.save_spendable_output()
 
-        tip(83)
-        block(85, spend=out[29])
-        yield rejected()
+        self.move_tip(83)
+        b85 = self.next_block(85, spend=out[29])
+        self.send_blocks([b85], False)  # other chain is same length
 
-        block(86, spend=out[30])
-        yield accepted()
+        b86 = self.next_block(86, spend=out[30])
+        self.send_blocks([b86], True)
 
-        tip(84)
-        block(87, spend=out[30])
-        yield rejected()
-        save_spendable_output()
+        self.move_tip(84)
+        b87 = self.next_block(87, spend=out[30])
+        self.send_blocks([b87], False)  # other chain is same length
+        self.save_spendable_output()
 
-        block(88, spend=out[31])
-        yield accepted()
-        save_spendable_output()
+        b88 = self.next_block(88, spend=out[31])
+        self.send_blocks([b88], True)
+        self.save_spendable_output()
 
         # trying to spend the OP_RETURN output is rejected
-        block("89a", spend=out[32])
-        tx = create_tx(tx1, 0, 0, CScript([OP_TRUE]))
-        update_block("89a", [tx])
-        yield rejected()
-
-
-        #  Test re-org of a week's worth of blocks (1088 blocks)
-        #  This test takes a minute or two and can be accomplished in memory
-        #
-        if self.options.runbarelyexpensive:
-            tip(88)
-            LARGE_REORG_SIZE = 1088
-            test1 = TestInstance(sync_every_block=False)
-            spend=out[32]
-            for i in range(89, LARGE_REORG_SIZE + 89):
-                b = block(i, spend)
-                tx = CTransaction()
-                script_length = MAX_BLOCK_BASE_SIZE - len(b.serialize()) - 69
-                script_output = CScript([b'\x00' * script_length])
-                tx.vout.append(CTxOut(0, script_output))
-                tx.vin.append(CTxIn(COutPoint(b.vtx[1].sha256, 0)))
-                b = update_block(i, [tx])
-                assert_equal(len(b.serialize()), MAX_BLOCK_BASE_SIZE)
-                test1.blocks_and_transactions.append([self.tip, True])
-                save_spendable_output()
-                spend = get_spendable_output()
-
-            yield test1
-            chain1_tip = i
-
-            # now create alt chain of same length
-            tip(88)
-            test2 = TestInstance(sync_every_block=False)
-            for i in range(89, LARGE_REORG_SIZE + 89):
-                block("alt"+str(i))
-                test2.blocks_and_transactions.append([self.tip, False])
-            yield test2
-
-            # extend alt chain to trigger re-org
-            block("alt" + str(chain1_tip + 1))
-            yield accepted()
-
-            # ... and re-org back to the first chain
-            tip(chain1_tip)
-            block(chain1_tip + 1)
-            yield rejected()
-            block(chain1_tip + 2)
-            yield accepted()
-
-            chain1_tip += 2
+        b89a = self.next_block("89a", spend=out[32])
+        tx = self.create_tx(tx1, 0, 0, CScript([OP_TRUE]))
+        b89a = self.update_block("89a", [tx])
+        # !TODO: fix expect_disconnect
+        self.send_blocks([b89a], False, 'bad-txns-inputs-missingorspent', reconnect=False)
+
+        # !TODO: add long-reorg test
+
+    # Helper methods
+    ################
+
+    def add_transactions_to_block(self, block, tx_list):
+        [tx.rehash() for tx in tx_list]
+        block.vtx.extend(tx_list)
+
+    # this is a little handier to use than the version in blocktools.py
+    def create_tx(self, spend_tx, n, value, script=CScript([OP_TRUE])):
+        return create_transaction(spend_tx, n, b"", value, script)
+
+    # sign a transaction, using the key we know about
+    # this signs input 0 in tx, which is assumed to be spending output n in spend_tx
+    def sign_tx(self, tx, spend_tx, n):
+        scriptPubKey = bytearray(spend_tx.vout[n].scriptPubKey)
+        if (scriptPubKey[0] == OP_TRUE):  # an anyone-can-spend
+            tx.vin[0].scriptSig = CScript()
+            return
+        (sighash, err) = SignatureHash(spend_tx.vout[n].scriptPubKey, tx, 0, SIGHASH_ALL)
+        tx.vin[0].scriptSig = CScript([self.coinbase_key.sign(sighash) + bytes(bytearray([SIGHASH_ALL]))])
+
+    def create_and_sign_transaction(self, spend_tx, n, value, script=CScript([OP_TRUE])):
+        tx = self.create_tx(spend_tx, n, value, script)
+        self.sign_tx(tx, spend_tx, n)
+        tx.rehash()
+        return tx
+
+    def next_block(self, number, spend=None, additional_coinbase_value=0, script=CScript([OP_TRUE]), solve=True):
+        if self.tip is None:
+            base_block_hash = self.genesis_hash
+            block_time = int(time.time()) + 1
+        else:
+            base_block_hash = self.tip.sha256
+            block_time = self.tip.nTime + 1
+        # First create the coinbase
+        height = self.block_heights[base_block_hash] + 1
+        coinbase = create_coinbase(height, self.coinbase_pubkey)
+        coinbase.vout[0].nValue += additional_coinbase_value
+        coinbase.rehash()
+        if spend is None:
+            block = create_block(base_block_hash, coinbase, block_time)
+        else:
+            coinbase.vout[0].nValue += spend.tx.vout[spend.n].nValue - 1  # all but one satoshi to fees
+            coinbase.rehash()
+            block = create_block(base_block_hash, coinbase, block_time)
+            tx = create_transaction(spend.tx, spend.n, b"", 1, script)  # spend 1 satoshi
+            self.sign_tx(tx, spend.tx, spend.n)
+            self.add_transactions_to_block(block, [tx])
+            block.hashMerkleRoot = block.calc_merkle_root()
+        if solve:
+            block.solve()
+        self.tip = block
+        self.block_heights[block.sha256] = height
+        assert number not in self.blocks
+        self.blocks[number] = block
+        return block
+
+    # save the current tip so it can be spent by a later block
+    def save_spendable_output(self):
+        self.log.debug("saving spendable output %s" % self.tip.vtx[0])
+        self.spendable_outputs.append(self.tip)
+
+    # get an output that we previously marked as spendable
+    def get_spendable_output(self):
+        self.log.debug("getting spendable output %s" % self.spendable_outputs[0].vtx[0])
+        return PreviousSpendableOutput(self.spendable_outputs.pop(0).vtx[0], 0)
+
+    # move the tip back to a previous block
+    def move_tip(self, number):
+        self.tip = self.blocks[number]
+
+    # adds transactions to the block and updates state
+    def update_block(self, block_number, new_transactions):
+        block = self.blocks[block_number]
+        self.add_transactions_to_block(block, new_transactions)
+        old_sha256 = block.sha256
+        block.hashMerkleRoot = block.calc_merkle_root()
+        block.solve()
+        # Update the internal state just like in next_block
+        self.tip = block
+        if block.sha256 != old_sha256:
+            self.block_heights[block.sha256] = self.block_heights[old_sha256]
+            del self.block_heights[old_sha256]
+        self.blocks[block_number] = block
+        return block
+
+    def reconnect_p2p(self):
+        """Add a P2P connection to the node.
+
+        The node gets disconnected several times in this test. This helper
+        method reconnects the p2p and restarts the network thread."""
+
+        network_thread_join()
+        self.nodes[0].disconnect_p2ps()
+        self.nodes[0].add_p2p_connection(P2PDataStore())
+        network_thread_start()
+        self.nodes[0].p2p.wait_for_verack()
+
+    def send_blocks(self, blocks, success=True, reject_reason=None, reconnect=False, timeout=60):
+        """Sends blocks to test node. Syncs and verifies that tip has advanced to most recent block.
+
+        Call with success = False if the tip shouldn't advance to the most recent block."""
+        self.nodes[0].p2p.send_blocks_and_test(blocks, self.nodes[0], success=success, reject_reason=reject_reason, expect_disconnect=reconnect, timeout=timeout)
+
+        if reconnect:
+            self.reconnect_p2p()
+
+    # PIVX
+    # create a block with a tx spending a given out, and lots of txes spending the outputs created
+    # in the first one. Keep the tx size under 150 kB limit.
+    def create_sized_block(self, block, spend, block_size):
+        num_of_txes = int(block_size // 150000) + 1
+        block.vtx[0].vout[0].nValue += spend.tx.vout[spend.n].nValue - num_of_txes
+        block.vtx[0].rehash()
+        tx1 = CTransaction()
+        tx1.vin.append(CTxIn(COutPoint(spend.tx.sha256, spend.n), b"", 0xffffffff))
+        tx1.vout = [CTxOut(1, CScript([OP_TRUE]))] * num_of_txes
+        tx1.calc_sha256()
+        self.sign_tx(tx1, spend.tx, spend.n)
+        self.add_transactions_to_block(block, [tx1])
+
+        available_space = block_size - len(block.serialize())
+        idx = 0
+        while available_space > 138 and idx < num_of_txes:
+            tx = CTransaction()
+            if 149999 < available_space:
+                script_length = 149999 - 141
+            else:
+                script_length = available_space - 138
+            script_output = CScript([b'\x00' * script_length])
+            tx.vout.append(CTxOut(0, script_output))
+            tx.vin.append(CTxIn(COutPoint(block.vtx[1].sha256, idx)))
+            tx.calc_sha256()
+            self.sign_tx(tx, spend.tx, spend.n)
+            self.add_transactions_to_block(block, [tx])
+            available_space = block_size - len(block.serialize())
+            idx += 1
+        block.hashMerkleRoot = block.calc_merkle_root()
+        block.solve()
+        # Make sure the math above worked out to produce a block_size-sized block
+        assert len(block.serialize()) in [block_size - 1, block_size]
+        return block
 
 
 if __name__ == '__main__':
-    FullBlockTest().main()
+    FullBlockTest().main()
\ No newline at end of file
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index c6f1681872dc..7ba290508061 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -77,6 +77,7 @@
     'rpc_spork.py',                             # ~ 144 sec
     'wallet_txn_doublespend.py --mineblock',    # ~ 143 sec
     'wallet_txn_clone.py --mineblock',          # ~ 143 sec
+    'feature_block.py',                         # ~ 140 sec
     'feature_proxy.py',                         # ~ 138 sec
     'rpc_rawtransaction.py',                    # ~ 134 sec
     'mining_pos_reorg.py',                      # ~ 128 sec
@@ -125,7 +126,6 @@
 
     # Don't append tests at the end to avoid merge conflicts
     # Put them in a random line within the section that fits their approximate run-time
-    # 'feature_block.py',
     # 'mempool_limit.py', # We currently don't limit our mempool_reorg
     # 'rpc_getchaintips.py',
     # 'mining_prioritisetransaction.py',
@@ -182,6 +182,7 @@
 
 LEGACY_SKIP_TESTS = [
     # These tests are not run when the flag --legacywallet is used
+    'feature_block.py',
     'feature_blockindexstats.py',
     'feature_config_args.py',
     'feature_help.py',

From fcda6cc2bd42da618974d1a5121b956ad41e22df Mon Sep 17 00:00:00 2001
From: random-zebra <random.zebra@protonmail.com>
Date: Tue, 4 May 2021 00:19:02 +0200
Subject: [PATCH 16/17] [QA][Cleanup] Retire the Comparison Test Framework

Nuke
- ComparisonTestFramework
- comptool
- BlockStore
- TxStore
---
 test/functional/README.md                     |  52 ---
 test/functional/test_framework/blockstore.py  | 170 --------
 test/functional/test_framework/comptool.py    | 397 ------------------
 .../test_framework/test_framework.py          |  30 --
 4 files changed, 649 deletions(-)
 delete mode 100644 test/functional/test_framework/blockstore.py
 delete mode 100755 test/functional/test_framework/comptool.py

diff --git a/test/functional/README.md b/test/functional/README.md
index 1397e664dfb2..b32749f381eb 100644
--- a/test/functional/README.md
+++ b/test/functional/README.md
@@ -112,15 +112,9 @@ Generally useful functions.
 #### [test_framework/mininode.py](test_framework/mininode.py)
 Basic code to support P2P connectivity to a pivxd.
 
-#### [test_framework/comptool.py](test_framework/comptool.py)
-Framework for comparison-tool style, p2p tests.
-
 #### [test_framework/script.py](test_framework/script.py)
 Utilities for manipulating transaction scripts (originally from python-bitcoinlib)
 
-#### [test_framework/blockstore.py](test_framework/blockstore.py)
-Implements disk-backed block and tx storage.
-
 #### [test_framework/key.py](test_framework/key.py)
 Wrapper around OpenSSL EC_Key (originally from python-bitcoinlib)
 
@@ -129,49 +123,3 @@ Helpers for script.py
 
 #### [test_framework/blocktools.py](test_framework/blocktools.py)
 Helper functions for creating blocks and transactions.
-
-### Comptool
-
-* Testing framework for writing tests that compare the block/tx acceptance
-behavior of a pivxd against 1 or more other pivxd instances, or against
-known outcomes, or both.
-
-* Set the ```num_nodes``` variable (defined in ```ComparisonTestFramework```) to start up
-1 or more nodes.  If using 1 node, then ```--testbinary``` can be used as a command line
-option to change the pivxd binary used by the test.  If using 2 or more nodes,
-then ```--refbinary``` can be optionally used to change the pivxd that will be used
-on nodes 2 and up.
-
-* Implement a (generator) function called ```get_tests()``` which yields ```TestInstance```s.
-Each ```TestInstance``` consists of:
-  - a list of ```[object, outcome, hash]``` entries
-    * ```object``` is a ```CBlock```, ```CTransaction```, or
-    ```CBlockHeader```.  ```CBlock```'s and ```CTransaction```'s are tested for
-    acceptance.  ```CBlockHeader```s can be used so that the test runner can deliver
-    complete headers-chains when requested from the pivxd, to allow writing
-    tests where blocks can be delivered out of order but still processed by
-    headers-first pivxd's.
-    * ```outcome``` is ```True```, ```False```, or ```None```.  If ```True```
-    or ```False```, the tip is compared with the expected tip -- either the
-    block passed in, or the hash specified as the optional 3rd entry.  If
-    ```None``` is specified, then the test will compare all the pivxd's
-    being tested to see if they all agree on what the best tip is.
-    * ```hash``` is the block hash of the tip to compare against. Optional to
-    specify; if left out then the hash of the block passed in will be used as
-    the expected tip.  This allows for specifying an expected tip while testing
-    the handling of either invalid blocks or blocks delivered out of order,
-    which complete a longer chain.
-  - ```sync_every_block```: ```True/False```.  If ```False```, then all blocks
-    are inv'ed together, and the test runner waits until the node receives the
-    last one, and tests only the last block for tip acceptance using the
-    outcome and specified tip.  If ```True```, then each block is tested in
-    sequence and synced (this is slower when processing many blocks).
-  - ```sync_every_transaction```: ```True/False```.  Analogous to
-    ```sync_every_block```, except if the outcome on the last tx is "None",
-    then the contents of the entire mempool are compared across all pivxd
-    connections.  If ```True``` or ```False```, then only the last tx's
-    acceptance is tested against the given outcome.
-
-* For examples of tests written in this framework, see
-  ```invalidblockrequest.py``` and ```p2p-fullblocktest.py```.
-
diff --git a/test/functional/test_framework/blockstore.py b/test/functional/test_framework/blockstore.py
deleted file mode 100644
index 592e99fb0d15..000000000000
--- a/test/functional/test_framework/blockstore.py
+++ /dev/null
@@ -1,170 +0,0 @@
-#!/usr/bin/env python3
-# Copyright (c) 2015-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.
-"""BlockStore and TxStore helper classes."""
-
-from .mininode import *
-from io import BytesIO
-import dbm.dumb as dbmd
-
-logger = logging.getLogger("TestFramework.blockstore")
-
-class BlockStore():
-    """BlockStore helper class.
-
-    BlockStore keeps a map of blocks and implements helper functions for
-    responding to getheaders and getdata, and for constructing a getheaders
-    message.
-    """
-
-    def __init__(self, datadir):
-        self.blockDB = dbmd.open(datadir + "/blocks", 'c')
-        self.currentBlock = 0
-        self.headers_map = dict()
-
-    def close(self):
-        self.blockDB.close()
-
-    def erase(self, blockhash):
-        del self.blockDB[repr(blockhash)]
-
-    # lookup an entry and return the item as raw bytes
-    def get(self, blockhash):
-        value = None
-        try:
-            value = self.blockDB[repr(blockhash)]
-        except KeyError:
-            return None
-        return value
-
-    # lookup an entry and return it as a CBlock
-    def get_block(self, blockhash):
-        ret = None
-        serialized_block = self.get(blockhash)
-        if serialized_block is not None:
-            f = BytesIO(serialized_block)
-            ret = CBlock()
-            ret.deserialize(f)
-            ret.calc_sha256()
-        return ret
-
-    def get_header(self, blockhash):
-        try:
-            return self.headers_map[blockhash]
-        except KeyError:
-            return None
-
-    # Note: this pulls full blocks out of the database just to retrieve
-    # the headers -- perhaps we could keep a separate data structure
-    # to avoid this overhead.
-    def headers_for(self, locator, hash_stop, current_tip=None):
-        if current_tip is None:
-            current_tip = self.currentBlock
-        current_block_header = self.get_header(current_tip)
-        if current_block_header is None:
-            return None
-
-        response = msg_headers()
-        headersList = [ current_block_header ]
-        maxheaders = 2000
-        while (headersList[0].sha256 not in locator.vHave):
-            prevBlockHash = headersList[0].hashPrevBlock
-            prevBlockHeader = self.get_header(prevBlockHash)
-            if prevBlockHeader is not None:
-                headersList.insert(0, prevBlockHeader)
-            else:
-                break
-        headersList = headersList[:maxheaders] # truncate if we have too many
-        hashList = [x.sha256 for x in headersList]
-        index = len(headersList)
-        if (hash_stop in hashList):
-            index = hashList.index(hash_stop)+1
-        response.headers = headersList[:index]
-        return response
-
-    def add_block(self, block):
-        block.calc_sha256()
-        try:
-            self.blockDB[repr(block.sha256)] = bytes(block.serialize())
-        except TypeError as e:
-            logger.exception("Unexpected error")
-        self.currentBlock = block.sha256
-        self.headers_map[block.sha256] = CBlockHeader(block)
-
-    def add_header(self, header):
-        self.headers_map[header.sha256] = header
-
-    # lookup the hashes in "inv", and return p2p messages for delivering
-    # blocks found.
-    def get_blocks(self, inv):
-        responses = []
-        for i in inv:
-            if (i.type == 2): # MSG_BLOCK
-                data = self.get(i.hash)
-                if data is not None:
-                    # Use msg_generic to avoid re-serialization
-                    responses.append(msg_generic(b"block", data))
-        return responses
-
-    def get_locator(self, current_tip=None):
-        if current_tip is None:
-            current_tip = self.currentBlock
-        r = []
-        counter = 0
-        step = 1
-        lastBlock = self.get_block(current_tip)
-        while lastBlock is not None:
-            r.append(lastBlock.hashPrevBlock)
-            for i in range(step):
-                lastBlock = self.get_block(lastBlock.hashPrevBlock)
-                if lastBlock is None:
-                    break
-            counter += 1
-            if counter > 10:
-                step *= 2
-        locator = CBlockLocator()
-        locator.vHave = r
-        return locator
-
-class TxStore():
-    def __init__(self, datadir):
-        self.txDB = dbmd.open(datadir + "/transactions", 'c')
-
-    def close(self):
-        self.txDB.close()
-
-    # lookup an entry and return the item as raw bytes
-    def get(self, txhash):
-        value = None
-        try:
-            value = self.txDB[repr(txhash)]
-        except KeyError:
-            return None
-        return value
-
-    def get_transaction(self, txhash):
-        ret = None
-        serialized_tx = self.get(txhash)
-        if serialized_tx is not None:
-            f = BytesIO(serialized_tx)
-            ret = CTransaction()
-            ret.deserialize(f)
-            ret.calc_sha256()
-        return ret
-
-    def add_transaction(self, tx):
-        tx.calc_sha256()
-        try:
-            self.txDB[repr(tx.sha256)] = bytes(tx.serialize())
-        except TypeError as e:
-            logger.exception("Unexpected error")
-
-    def get_transactions(self, inv):
-        responses = []
-        for i in inv:
-            if (i.type == 1): # MSG_TX
-                tx = self.get(i.hash)
-                if tx is not None:
-                    responses.append(msg_generic(b"tx", tx))
-        return responses
diff --git a/test/functional/test_framework/comptool.py b/test/functional/test_framework/comptool.py
deleted file mode 100755
index 2c1e44cf7803..000000000000
--- a/test/functional/test_framework/comptool.py
+++ /dev/null
@@ -1,397 +0,0 @@
-#!/usr/bin/env python3
-# Copyright (c) 2015-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.
-"""Compare two or more pivxds to each other.
-
-To use, create a class that implements get_tests(), and pass it in
-as the test generator to TestManager.  get_tests() should be a python
-generator that returns TestInstance objects.  See below for definition.
-
-TestNode behaves as follows:
-    Configure with a BlockStore and TxStore
-    on_inv: log the message but don't request
-    on_headers: log the chain tip
-    on_pong: update ping response map (for synchronization)
-    on_getheaders: provide headers via BlockStore
-    on_getdata: provide blocks via BlockStore
-"""
-
-from .mininode import *
-from .blockstore import BlockStore, TxStore
-from .util import p2p_port, wait_until
-
-import logging
-
-logger=logging.getLogger("TestFramework.comptool")
-
-global mininode_lock
-
-class RejectResult():
-    """Outcome that expects rejection of a transaction or block."""
-    def __init__(self, code, reason=b''):
-        self.code = code
-        self.reason = reason
-    def match(self, other):
-        if self.code != other.code:
-            return False
-        return other.reason.startswith(self.reason)
-    def __repr__(self):
-        return '%i:%s' % (self.code,self.reason or '*')
-
-class TestNode(P2PInterface):
-
-    def __init__(self, block_store, tx_store):
-        super().__init__()
-        self.bestblockhash = None
-        self.block_store = block_store
-        self.block_request_map = {}
-        self.tx_store = tx_store
-        self.tx_request_map = {}
-        self.block_reject_map = {}
-        self.tx_reject_map = {}
-
-        # When the pingmap is non-empty we're waiting for
-        # a response
-        self.pingMap = {}
-        self.lastInv = []
-        self.closed = False
-
-    def on_close(self):
-        self.closed = True
-
-    def on_headers(self, message):
-        if len(message.headers) > 0:
-            best_header = message.headers[-1]
-            best_header.calc_sha256()
-            self.bestblockhash = best_header.sha256
-
-    def on_getheaders(self, message):
-        response = self.block_store.headers_for(message.locator, message.hashstop)
-        if response is not None:
-            self.send_message(response)
-
-    def on_getdata(self, message):
-        [self.send_message(r) for r in self.block_store.get_blocks(message.inv)]
-        [self.send_message(r) for r in self.tx_store.get_transactions(message.inv)]
-
-        for i in message.inv:
-            if i.type == 1 or i.type == 1 | (1 << 30): # MSG_TX or MSG_WITNESS_TX
-                self.tx_request_map[i.hash] = True
-            elif i.type == 2 or i.type == 2 | (1 << 30): # MSG_BLOCK or MSG_WITNESS_BLOCK
-                self.block_request_map[i.hash] = True
-
-    def on_inv(self, message):
-        self.lastInv = [x.hash for x in message.inv]
-
-    def on_pong(self, message):
-        try:
-            del self.pingMap[message.nonce]
-        except KeyError:
-            raise AssertionError("Got pong for unknown ping [%s]" % repr(message))
-
-    def on_reject(self, message):
-        if message.message == b'tx':
-            self.tx_reject_map[message.data] = RejectResult(message.code, message.reason)
-        if message.message == b'block':
-            self.block_reject_map[message.data] = RejectResult(message.code, message.reason)
-
-    def send_inv(self, obj):
-        mtype = 2 if isinstance(obj, CBlock) else 1
-        self.send_message(msg_inv([CInv(mtype, obj.sha256)]))
-
-    def send_getheaders(self):
-        # We ask for headers from their last tip.
-        m = msg_getheaders()
-        m.locator = self.block_store.get_locator(self.bestblockhash)
-        self.send_message(m)
-
-    def send_header(self, header):
-        m = msg_headers()
-        m.headers.append(header)
-        self.send_message(m)
-
-    # This assumes BIP31
-    def send_ping(self, nonce):
-        self.pingMap[nonce] = True
-        self.send_message(msg_ping(nonce))
-
-    def received_ping_response(self, nonce):
-        return nonce not in self.pingMap
-
-    def send_mempool(self):
-        self.lastInv = []
-        self.send_message(msg_mempool())
-
-# TestInstance:
-#
-# Instances of these are generated by the test generator, and fed into the
-# comptool.
-#
-# "blocks_and_transactions" should be an array of
-#    [obj, True/False/None, hash/None]:
-#  - obj is either a CBlock, CBlockHeader, or a CTransaction, and
-#  - the second value indicates whether the object should be accepted
-#    into the blockchain or mempool (for tests where we expect a certain
-#    answer), or "None" if we don't expect a certain answer and are just
-#    comparing the behavior of the nodes being tested.
-#  - the third value is the hash to test the tip against (if None or omitted,
-#    use the hash of the block)
-#  - NOTE: if a block header, no test is performed; instead the header is
-#    just added to the block_store.  This is to facilitate block delivery
-#    when communicating with headers-first clients (when withholding an
-#    intermediate block).
-# sync_every_block: if True, then each block will be inv'ed, synced, and
-#    nodes will be tested based on the outcome for the block.  If False,
-#    then inv's accumulate until all blocks are processed (or max inv size
-#    is reached) and then sent out in one inv message.  Then the final block
-#    will be synced across all connections, and the outcome of the final
-#    block will be tested.
-# sync_every_tx: analogous to behavior for sync_every_block, except if outcome
-#    on the final tx is None, then contents of entire mempool are compared
-#    across all connections.  (If outcome of final tx is specified as true
-#    or false, then only the last tx is tested against outcome.)
-
-class TestInstance():
-    def __init__(self, objects=None, sync_every_block=True, sync_every_tx=False):
-        self.blocks_and_transactions = objects if objects else []
-        self.sync_every_block = sync_every_block
-        self.sync_every_tx = sync_every_tx
-
-class TestManager():
-
-    def __init__(self, testgen, datadir):
-        self.test_generator = testgen
-        self.p2p_connections= []
-        self.block_store    = BlockStore(datadir)
-        self.tx_store       = TxStore(datadir)
-        self.ping_counter   = 1
-
-    def add_all_connections(self, nodes):
-        for i in range(len(nodes)):
-            # Create a p2p connection to each node
-            node = TestNode(self.block_store, self.tx_store)
-            node.peer_connect('127.0.0.1', p2p_port(i))
-            self.p2p_connections.append(node)
-
-    def clear_all_connections(self):
-        self.p2p_connections = []
-
-    def wait_for_disconnections(self):
-        def disconnected():
-            return all(node.closed for node in self.p2p_connections)
-        wait_until(disconnected, timeout=10, lock=mininode_lock)
-
-    def wait_for_verack(self):
-        return all(node.wait_for_verack() for node in self.p2p_connections)
-
-    def wait_for_pings(self, counter):
-        def received_pongs():
-            return all(node.received_ping_response(counter) for node in self.p2p_connections)
-        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
-    # the response by using a ping (and waiting for pong with same nonce).
-    def sync_blocks(self, blockhash, num_blocks):
-        def blocks_requested():
-            return all(
-                blockhash in node.block_request_map and node.block_request_map[blockhash]
-                for node in self.p2p_connections
-            )
-
-        # --> error if not requested
-        wait_until(blocks_requested, attempts=20*num_blocks, lock=mininode_lock)
-
-        # Send getheaders message
-        [ c.send_getheaders() for c in self.p2p_connections ]
-
-        # Send ping and wait for response -- synchronization hack
-        [ c.send_ping(self.ping_counter) for c in self.p2p_connections ]
-        self.wait_for_pings(self.ping_counter)
-        self.ping_counter += 1
-
-    # Analogous to sync_block (see above)
-    def sync_transaction(self, txhash, num_events):
-        # Wait for nodes to request transaction (50ms sleep * 20 tries * num_events)
-        def transaction_requested():
-            return all(
-                txhash in node.tx_request_map and node.tx_request_map[txhash]
-                for node in self.p2p_connections
-            )
-
-        # --> error if not requested
-        wait_until(transaction_requested, attempts=20*num_events, lock=mininode_lock)
-
-        # Get the mempool
-        [ c.send_mempool() for c in self.p2p_connections ]
-
-        # Send ping and wait for response -- synchronization hack
-        [ c.send_ping(self.ping_counter) for c in self.p2p_connections ]
-        self.wait_for_pings(self.ping_counter)
-        self.ping_counter += 1
-
-        # Sort inv responses from each node
-        with mininode_lock:
-            [ c.lastInv.sort() for c in self.p2p_connections ]
-
-    # Verify that the tip of each connection all agree with each other, and
-    # with the expected outcome (if given)
-    def check_results(self, blockhash, outcome):
-        with mininode_lock:
-            for c in self.p2p_connections:
-                if outcome is None:
-                    if c.bestblockhash != self.p2p_connections[0].bestblockhash:
-                        return False
-                elif isinstance(outcome, RejectResult): # Check that block was rejected w/ code
-                    if c.bestblockhash == blockhash:
-                        return False
-                    if blockhash not in c.block_reject_map:
-                        logger.error('Block not in reject map: %064x' % (blockhash))
-                        return False
-                    if not outcome.match(c.block_reject_map[blockhash]):
-                        logger.error('Block rejected with %s instead of expected %s: %064x' % (c.block_reject_map[blockhash], outcome, blockhash))
-                        return False
-                elif ((c.bestblockhash == blockhash) != outcome):
-                    return False
-            return True
-
-    # Either check that the mempools all agree with each other, or that
-    # txhash's presence in the mempool matches the outcome specified.
-    # This is somewhat of a strange comparison, in that we're either comparing
-    # a particular tx to an outcome, or the entire mempools altogether;
-    # perhaps it would be useful to add the ability to check explicitly that
-    # a particular tx's existence in the mempool is the same across all nodes.
-    def check_mempool(self, txhash, outcome):
-        with mininode_lock:
-            for c in self.p2p_connections:
-                if outcome is None:
-                    # Make sure the mempools agree with each other
-                    if c.lastInv != self.p2p_connections[0].lastInv:
-                        return False
-                elif isinstance(outcome, RejectResult): # Check that tx was rejected w/ code
-                    if txhash in c.lastInv:
-                        return False
-                    if txhash not in c.tx_reject_map:
-                        logger.error('Tx not in reject map: %064x' % (txhash))
-                        return False
-                    if not outcome.match(c.tx_reject_map[txhash]):
-                        logger.error('Tx rejected with %s instead of expected %s: %064x' % (c.tx_reject_map[txhash], outcome, txhash))
-                        return False
-                elif ((txhash in c.lastInv) != outcome):
-                    return False
-            return True
-
-    def run(self):
-        # Wait until verack is received
-        self.wait_for_verack()
-
-        test_number = 0
-        tests = self.test_generator.get_tests()
-        for test_instance in tests:
-            test_number += 1
-            logger.info("Running test %d: %s line %s" % (test_number, tests.gi_code.co_filename, tests.gi_frame.f_lineno))
-            # We use these variables to keep track of the last block
-            # and last transaction in the tests, which are used
-            # if we're not syncing on every block or every tx.
-            [ block, block_outcome, tip ] = [ None, None, None ]
-            [ tx, tx_outcome ] = [ None, None ]
-            invqueue = []
-
-            for test_obj in test_instance.blocks_and_transactions:
-                b_or_t = test_obj[0]
-                outcome = test_obj[1]
-                # Determine if we're dealing with a block or tx
-                if isinstance(b_or_t, CBlock):  # Block test runner
-                    block = b_or_t
-                    block_outcome = outcome
-                    tip = block.sha256
-                    # each test_obj can have an optional third argument
-                    # to specify the tip we should compare with
-                    # (default is to use the block being tested)
-                    if len(test_obj) >= 3:
-                        tip = test_obj[2]
-
-                    # Add to shared block_store, set as current block
-                    # If there was an open getdata request for the block
-                    # previously, and we didn't have an entry in the
-                    # block_store, then immediately deliver, because the
-                    # node wouldn't send another getdata request while
-                    # the earlier one is outstanding.
-                    first_block_with_hash = True
-                    if self.block_store.get(block.sha256) is not None:
-                        first_block_with_hash = False
-                    with mininode_lock:
-                        self.block_store.add_block(block)
-                        for c in self.p2p_connections:
-                            if first_block_with_hash and block.sha256 in c.block_request_map and c.block_request_map[block.sha256] == True:
-                                # There was a previous request for this block hash
-                                # Most likely, we delivered a header for this block
-                                # but never had the block to respond to the getdata
-                                c.send_message(msg_block(block))
-                            else:
-                                c.block_request_map[block.sha256] = False
-                    # Either send inv's to each node and sync, or add
-                    # to invqueue for later inv'ing.
-                    if (test_instance.sync_every_block):
-                        # if we expect success, send inv and sync every block
-                        # if we expect failure, just push the block and see what happens.
-                        if outcome == True:
-                            [ c.send_inv(block) for c in self.p2p_connections ]
-                            self.sync_blocks(block.sha256, 1)
-                        else:
-                            [ c.send_message(msg_block(block)) for c in self.p2p_connections ]
-                            [ c.send_ping(self.ping_counter) for c in self.p2p_connections ]
-                            self.wait_for_pings(self.ping_counter)
-                            self.ping_counter += 1
-                        if (not self.check_results(tip, outcome)):
-                            raise AssertionError("Test failed at test %d" % test_number)
-                    else:
-                        invqueue.append(CInv(2, block.sha256))
-                elif isinstance(b_or_t, CBlockHeader):
-                    block_header = b_or_t
-                    self.block_store.add_header(block_header)
-                    [ c.send_header(block_header) for c in self.p2p_connections ]
-
-                else:  # Tx test runner
-                    assert(isinstance(b_or_t, CTransaction))
-                    tx = b_or_t
-                    tx_outcome = outcome
-                    # Add to shared tx store and clear map entry
-                    with mininode_lock:
-                        self.tx_store.add_transaction(tx)
-                        for c in self.p2p_connections:
-                            c.tx_request_map[tx.sha256] = False
-                    # Again, either inv to all nodes or save for later
-                    if (test_instance.sync_every_tx):
-                        [ c.send_inv(tx) for c in self.p2p_connections ]
-                        self.sync_transaction(tx.sha256, 1)
-                        if (not self.check_mempool(tx.sha256, outcome)):
-                            raise AssertionError("Test failed at test %d" % test_number)
-                    else:
-                        invqueue.append(CInv(1, tx.sha256))
-                # Ensure we're not overflowing the inv queue
-                if len(invqueue) == MAX_INV_SZ:
-                    [ c.send_message(msg_inv(invqueue)) for c in self.p2p_connections ]
-                    invqueue = []
-
-            # Do final sync if we weren't syncing on every block or every tx.
-            if (not test_instance.sync_every_block and block is not None):
-                if len(invqueue) > 0:
-                    [ c.send_message(msg_inv(invqueue)) for c in self.p2p_connections ]
-                    invqueue = []
-                self.sync_blocks(block.sha256, len(test_instance.blocks_and_transactions))
-                if (not self.check_results(tip, block_outcome)):
-                    raise AssertionError("Block test failed at test %d" % test_number)
-            if (not test_instance.sync_every_tx and tx is not None):
-                if len(invqueue) > 0:
-                    [ c.send_message(msg_inv(invqueue)) for c in self.p2p_connections ]
-                    invqueue = []
-                self.sync_transaction(tx.sha256, len(test_instance.blocks_and_transactions))
-                if (not self.check_mempool(tx.sha256, tx_outcome)):
-                    raise AssertionError("Mempool test failed at test %d" % test_number)
-
-        [ c.disconnect_node() for c in self.p2p_connections ]
-        self.wait_for_disconnections()
-        self.block_store.close()
-        self.tx_store.close()
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index dc1135588dd9..64d362bd6112 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -1108,36 +1108,6 @@ def setupMasternode(self,
         # return the collateral id
         return collateralTxId
 
-### ------------------------------------------------------
-
-class ComparisonTestFramework(PivxTestFramework):
-    """Test framework for doing p2p comparison testing
-
-    Sets up some pivxd binaries:
-    - 1 binary: test binary
-    - 2 binaries: 1 test binary, 1 ref binary
-    - n>2 binaries: 1 test binary, n-1 ref binaries"""
-
-    def set_test_params(self):
-        self.num_nodes = 2
-        self.setup_clean_chain = True
-
-    def add_options(self, parser):
-        parser.add_option("--testbinary", dest="testbinary",
-                          default=os.getenv("BITCOIND", "pivxd"),
-                          help="pivxd binary to test")
-        parser.add_option("--refbinary", dest="refbinary",
-                          default=os.getenv("BITCOIND", "pivxd"),
-                          help="pivxd binary to use for reference nodes (if any)")
-
-    def setup_network(self):
-        extra_args = [['-whitelist=127.0.0.1']] * self.num_nodes
-        if hasattr(self, "extra_args"):
-            extra_args = self.extra_args
-        self.add_nodes(self.num_nodes, 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"""

From 48e5d54b5533404e29952f09e53275b2a4d2abde Mon Sep 17 00:00:00 2001
From: random-zebra <random.zebra@protonmail.com>
Date: Wed, 5 May 2021 21:06:06 +0200
Subject: [PATCH 17/17] [Tests][Refactoring] Remove 'magic bytes' in
 p2p_invalid_* tests

---
 test/functional/p2p_invalid_block.py | 16 +++++++---
 test/functional/p2p_invalid_tx.py    | 46 +++++++++++++++-------------
 2 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/test/functional/p2p_invalid_block.py b/test/functional/p2p_invalid_block.py
index 6fda64c59c2f..6aef35f0e35b 100755
--- a/test/functional/p2p_invalid_block.py
+++ b/test/functional/p2p_invalid_block.py
@@ -15,6 +15,10 @@
 from test_framework.blocktools import create_block, create_coinbase, create_transaction
 from test_framework.messages import COIN
 from test_framework.mininode import network_thread_start, P2PDataStore
+from test_framework.script import (
+    CScript,
+    OP_TRUE,
+)
 from test_framework.test_framework import PivxTestFramework
 from test_framework.util import assert_equal
 
@@ -24,6 +28,9 @@ def set_test_params(self):
         self.setup_clean_chain = True
         self.extra_args = [["-whitelist=127.0.0.1"]]
 
+    def create_tx(self, spend_tx, n, value):
+        return create_transaction(spend_tx, n, b"", value, CScript([OP_TRUE]))
+
     def run_test(self):
         # Add p2p connection to node0
         node = self.nodes[0]  # convenience reference to the node
@@ -65,9 +72,8 @@ def run_test(self):
         block2 = create_block(tip, create_coinbase(height), block_time)
         block_time += 1
 
-        # b'0x51' is OP_TRUE
-        tx1 = create_transaction(block1.vtx[0], 0, b'\x51', 50 * COIN)
-        tx2 = create_transaction(tx1, 0, b'\x51', 50 * COIN)
+        tx1 = self.create_tx(block1.vtx[0], 0, 50 * COIN)
+        tx2 = self.create_tx(tx1, 0, 50 * COIN)
 
         block2.vtx.extend([tx1, tx2])
         block2.hashMerkleRoot = block2.calc_merkle_root()
@@ -123,7 +129,7 @@ def run_test(self):
         # Complete testing of CVE-2018-17144, by checking for the inflation bug.
         # Create a block that spends the output of a tx in a previous block.
         block4 = create_block(tip, create_coinbase(height), block_time)
-        tx3 = create_transaction(tx2, 0, b'\x51', 50 * COIN)
+        tx3 = self.create_tx(tx2, 0, 50 * COIN)
 
         # Duplicates input
         tx3.vin.append(tx3.vin[0])
@@ -137,7 +143,7 @@ def run_test(self):
 
         self.log.info("Test output value > input value out of range")
         # Can be removed when 'feature_block.py' is added to the suite.
-        tx4 = create_transaction(tx2, 0, b'\x51', 260 * COIN)
+        tx4 = self.create_tx(tx2, 0, 260 * COIN)
         block4 = create_block(tip, create_coinbase(height), block_time)
         block4.vtx.extend([tx4])
         block4.hashMerkleRoot = block4.calc_merkle_root()
diff --git a/test/functional/p2p_invalid_tx.py b/test/functional/p2p_invalid_tx.py
index 3fcdc9473ffd..bb15f25d82d5 100755
--- a/test/functional/p2p_invalid_tx.py
+++ b/test/functional/p2p_invalid_tx.py
@@ -14,10 +14,14 @@
     CTxOut,
 )
 from test_framework.mininode import network_thread_start, P2PDataStore, network_thread_join
+from test_framework.script import (
+    CScript,
+    OP_NOTIF,
+    OP_TRUE,
+)
 from test_framework.test_framework import PivxTestFramework
 from test_framework.util import (
     assert_equal,
-    wait_until,
 )
 
 
@@ -45,6 +49,18 @@ def reconnect_p2p(self, **kwargs):
         network_thread_join()
         self.bootstrap_p2p(**kwargs)
 
+    def new_spend_tx(self, prev_hash, prev_n, values):
+        """Create a CTransaction spending COutPoint(prev_hash, prev_n)
+
+        each amount specified in the 'values' list is sent to an
+        anyone-can-spend script"""
+        tx = CTransaction()
+        tx.vin.append(CTxIn(outpoint=COutPoint(prev_hash, prev_n)))
+        for value in values:
+            tx.vout.append(CTxOut(nValue=value, scriptPubKey=CScript([OP_TRUE])))
+        tx.calc_sha256()
+        return tx
+
     def run_test(self):
         node = self.nodes[0]  # convenience reference to the node
 
@@ -67,11 +83,10 @@ def run_test(self):
         self.log.info("Mature the block.")
         self.nodes[0].generate(100)
 
-        # b'\x64' is OP_NOTIF
         # Transaction will be rejected with code 16 (REJECT_INVALID)
         # and we get disconnected immediately
         self.log.info('Test a transaction that is rejected')
-        tx1 = create_transaction(block1.vtx[0], 0, b'\x64', 50 * COIN - 12000)
+        tx1 = create_transaction(block1.vtx[0], 0, CScript([OP_NOTIF]), 50 * COIN - 12000)
         node.p2p.send_txs_and_test([tx1], node, success=False, expect_disconnect=True)
 
         # Make two p2p connections to provide the node with orphans
@@ -82,32 +97,19 @@ def run_test(self):
         self.log.info('Test orphan transaction handling ... ')
         # Create a root transaction that we withhold until all dependend transactions
         # are sent out and in the orphan cache
-        tx_withhold = CTransaction()
-        tx_withhold.vin.append(CTxIn(outpoint=COutPoint(block1.vtx[0].sha256, 0)))
-        tx_withhold.vout.append(CTxOut(nValue=50 * COIN - 12000, scriptPubKey=b'\x51'))
-        tx_withhold.calc_sha256()
+        tx_withhold = self.new_spend_tx(block1.vtx[0].sha256, 0, [50 * COIN - 12000])
 
-        # Our first orphan tx with some outputs to create further orphan txs
-        tx_orphan_1 = CTransaction()
-        tx_orphan_1.vin.append(CTxIn(outpoint=COutPoint(tx_withhold.sha256, 0)))
-        tx_orphan_1.vout = [CTxOut(nValue=10 * COIN, scriptPubKey=b'\x51')] * 3
-        tx_orphan_1.calc_sha256()
+        # Our first orphan tx with 3 outputs to create further orphan txs
+        tx_orphan_1 = self.new_spend_tx(tx_withhold.sha256, 0, [10 * COIN] * 3)
 
         # A valid transaction with low fee
-        tx_orphan_2_no_fee = CTransaction()
-        tx_orphan_2_no_fee.vin.append(CTxIn(outpoint=COutPoint(tx_orphan_1.sha256, 0)))
-        tx_orphan_2_no_fee.vout.append(CTxOut(nValue=10 * COIN, scriptPubKey=b'\x51'))
+        tx_orphan_2_no_fee = self.new_spend_tx(tx_orphan_1.sha256, 0, [10 * COIN])
 
         # A valid transaction with sufficient fee
-        tx_orphan_2_valid = CTransaction()
-        tx_orphan_2_valid.vin.append(CTxIn(outpoint=COutPoint(tx_orphan_1.sha256, 1)))
-        tx_orphan_2_valid.vout.append(CTxOut(nValue=10 * COIN - 12000, scriptPubKey=b'\x51'))
-        tx_orphan_2_valid.calc_sha256()
+        tx_orphan_2_valid = self.new_spend_tx(tx_orphan_1.sha256, 1, [10 * COIN - 12000])
 
         # An invalid transaction with negative fee
-        tx_orphan_2_invalid = CTransaction()
-        tx_orphan_2_invalid.vin.append(CTxIn(outpoint=COutPoint(tx_orphan_1.sha256, 2)))
-        tx_orphan_2_invalid.vout.append(CTxOut(nValue=11 * COIN, scriptPubKey=b'\x51'))
+        tx_orphan_2_invalid = self.new_spend_tx(tx_orphan_1.sha256, 2, [11 * COIN])
 
         self.log.info('Send the orphans ... ')
         # Send valid orphan txs from p2ps[0]