From 20e96d1233d125fcd6a1ec5abe8ad05113b190c6 Mon Sep 17 00:00:00 2001
From: Brannon King <countprimes@gmail.com>
Date: Thu, 1 Aug 2019 15:44:05 -0600
Subject: [PATCH] fix unit test crash on OSX

pulled in some fixes from v18
---
 src/test/test_bitcoin.cpp           | 11 ++++++++++-
 src/test/test_bitcoin.h             |  8 ++++----
 src/wallet/db.cpp                   |  6 +++++-
 src/wallet/test/claim_rpc_tests.cpp | 12 ++++++++++--
 4 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp
index 92a0cf4af..9cae2551a 100644
--- a/src/test/test_bitcoin.cpp
+++ b/src/test/test_bitcoin.cpp
@@ -20,6 +20,8 @@
 #include <script/sigcache.h>
 
 #include "claimtrie.h"
+#include <boost/test/unit_test.hpp>
+#include <boost/test/unit_test_parameters.hpp>
 
 void CConnmanTest::AddNode(CNode& node)
 {
@@ -39,7 +41,6 @@ void CConnmanTest::ClearNodes()
 uint256 insecure_rand_seed = GetRandHash();
 FastRandomContext insecure_rand_ctx(insecure_rand_seed);
 
-extern bool fPrintToConsole;
 extern void noui_connect();
 
 std::ostream& operator<<(std::ostream& os, const uint256& num)
@@ -90,6 +91,14 @@ std::ostream& operator<<(std::ostream& os, const CSupportValue& support)
 BasicTestingSetup::BasicTestingSetup(const std::string& chainName)
     : m_path_root(fs::temp_directory_path() / "test_bitcoin" / strprintf("%lu_%i", (unsigned long)GetTime(), (int)(InsecureRandRange(1 << 30))))
 {
+    // for debugging:
+    if (boost::unit_test::runtime_config::get<boost::unit_test::log_level>(boost::unit_test::runtime_config::btrt_log_level)
+            <= boost::unit_test::log_level::log_messages) {
+        g_logger->m_print_to_console = true;
+        g_logger->m_log_time_micros = true;
+        g_logger->EnableCategory(BCLog::ALL);
+    }
+
     SHA256AutoDetect();
     RandomInit();
     ECC_Start();
diff --git a/src/test/test_bitcoin.h b/src/test/test_bitcoin.h
index 059d3ac4a..1d0dd5037 100644
--- a/src/test/test_bitcoin.h
+++ b/src/test/test_bitcoin.h
@@ -44,7 +44,7 @@ struct BasicTestingSetup {
     ECCVerifyHandle globalVerifyHandle;
 
     explicit BasicTestingSetup(const std::string& chainName = CBaseChainParams::MAIN);
-    ~BasicTestingSetup();
+    virtual ~BasicTestingSetup();
 
     fs::path SetDataDir(const std::string& name);
 
@@ -70,7 +70,7 @@ struct TestingSetup: public BasicTestingSetup {
     std::unique_ptr<PeerLogicValidation> peerLogic;
 
     explicit TestingSetup(const std::string& chainName = CBaseChainParams::MAIN);
-    ~TestingSetup();
+    ~TestingSetup() override;
 };
 
 class CBlock;
@@ -89,7 +89,7 @@ struct TestChain100Setup : public TestingSetup {
     CBlock CreateAndProcessBlock(const std::vector<CMutableTransaction>& txns,
                                  const CScript& scriptPubKey);
 
-    ~TestChain100Setup();
+    ~TestChain100Setup() override;
 
     std::vector<CTransactionRef> m_coinbase_txns; // For convenience, coinbase transactions
     CKey coinbaseKey; // private/public key needed to spend coinbase transactions
@@ -97,7 +97,7 @@ struct TestChain100Setup : public TestingSetup {
 
 struct RegTestingSetup: public TestingSetup {
     RegTestingSetup();
-    ~RegTestingSetup();
+    ~RegTestingSetup() override;
 };
 
 class CTxMemPoolEntry;
diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp
index 89b68495a..f7a54f414 100644
--- a/src/wallet/db.cpp
+++ b/src/wallet/db.cpp
@@ -154,6 +154,7 @@ BerkeleyEnvironment::BerkeleyEnvironment(const fs::path& dir_path) : strPath(dir
 
 BerkeleyEnvironment::~BerkeleyEnvironment()
 {
+    LOCK(cs_db);
     g_dbenvs.erase(strPath);
     Close();
 }
@@ -558,6 +559,7 @@ BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const char* pszMode, bo
 
 void BerkeleyBatch::Flush()
 {
+    LOCK(cs_db);
     if (activeTxn)
         return;
 
@@ -566,7 +568,9 @@ void BerkeleyBatch::Flush()
     if (fReadOnly)
         nMinutes = 1;
 
-    env->dbenv->txn_checkpoint(nMinutes ? gArgs.GetArg("-dblogsize", DEFAULT_WALLET_DBLOGSIZE) * 1024 : 0, nMinutes, 0);
+    if (env && env->dbenv) { // env is nullptr for dummy databases (i.e. in tests). Don't actually flush if env is nullptr so we don't segfault
+        env->dbenv->txn_checkpoint(nMinutes ? gArgs.GetArg("-dblogsize", DEFAULT_WALLET_DBLOGSIZE) * 1024 : 0, nMinutes, 0);
+    }
 }
 
 void BerkeleyDatabase::IncrementUpdateCounter()
diff --git a/src/wallet/test/claim_rpc_tests.cpp b/src/wallet/test/claim_rpc_tests.cpp
index 263371cba..46fc8bc39 100644
--- a/src/wallet/test/claim_rpc_tests.cpp
+++ b/src/wallet/test/claim_rpc_tests.cpp
@@ -28,11 +28,11 @@ struct CatchWalletTestSetup: public TestingSetup {
         UniValue results = rpc_method(req);
         BOOST_CHECK_EQUAL(results["name"].get_str(), "tester_wallet");
     }
-    ~CatchWalletTestSetup() {
+    ~CatchWalletTestSetup() override {
         rpcfn_type rpc_method = tableRPC["unloadwallet"]->actor;
         JSONRPCRequest req;
+        req.URI = "/wallet/tester_wallet";
         req.params = UniValue(UniValue::VARR);
-        req.params.push_back("tester_wallet");
         rpc_method(req);
     }
 };
@@ -42,6 +42,7 @@ BOOST_FIXTURE_TEST_SUITE(claim_rpc_tests, CatchWalletTestSetup)
 double AvailableBalance() {
     rpcfn_type rpc_method = tableRPC["getbalance"]->actor;
     JSONRPCRequest req;
+    req.URI = "/wallet/tester_wallet";
     req.params = UniValue(UniValue::VARR);
     UniValue results = rpc_method(req);
     return results.get_real();
@@ -51,6 +52,7 @@ uint256 ClaimAName(const std::string& name, const std::string& data, const std::
     // pass a txid as name for update
     rpcfn_type rpc_method = tableRPC[isUpdate ? "updateclaim" : "claimname"]->actor;
     JSONRPCRequest req;
+    req.URI = "/wallet/tester_wallet";
     req.params = UniValue(UniValue::VARR);
     req.params.push_back(name);
     req.params.push_back(data);
@@ -67,6 +69,7 @@ uint256 SupportAName(const std::string& name, const std::string& claimId, const
     // pass a txid as name for update
     rpcfn_type rpc_method = tableRPC["supportclaim"]->actor;
     JSONRPCRequest req;
+    req.URI = "/wallet/tester_wallet";
     req.params = UniValue(UniValue::VARR);
     req.params.push_back(name);
     req.params.push_back(claimId);
@@ -82,6 +85,7 @@ uint256 SupportAName(const std::string& name, const std::string& claimId, const
 UniValue LookupAllNames() {
     rpcfn_type rpc_method = tableRPC["listnameclaims"]->actor;
     JSONRPCRequest req;
+    req.URI = "/wallet/tester_wallet";
     req.params = UniValue(UniValue::VARR);
     return rpc_method(req);
 }
@@ -89,6 +93,7 @@ UniValue LookupAllNames() {
 std::vector<uint256> generateBlock(int blocks = 1) {
     rpcfn_type rpc_method = tableRPC["generate"]->actor;
     JSONRPCRequest req;
+    req.URI = "/wallet/tester_wallet";
     req.params = UniValue(UniValue::VARR);
     req.params.push_back(blocks);
     UniValue results = rpc_method(req);
@@ -107,6 +112,7 @@ void rollbackBlock(const std::vector<uint256>& ids) {
     rpcfn_type rpc_method = tableRPC["invalidateblock"]->actor;
     for (auto it = ids.rbegin(); it != ids.rend(); ++it) {
         JSONRPCRequest req;
+        req.URI = "/wallet/tester_wallet";
         req.params = UniValue(UniValue::VARR);
         req.params.push_back(it->GetHex());
         rpc_method(req);
@@ -119,6 +125,7 @@ void rollbackBlock(const std::vector<uint256>& ids) {
 uint256 AbandonAClaim(const uint256& txid, bool isSupport = false) {
     rpcfn_type pre_rpc_method = tableRPC["getrawchangeaddress"]->actor;
     JSONRPCRequest pre_req;
+    pre_req.URI = "/wallet/tester_wallet";
     pre_req.params = UniValue(UniValue::VARR);
     pre_req.params.push_back("legacy");
     UniValue adr_hash = pre_rpc_method(pre_req);
@@ -126,6 +133,7 @@ uint256 AbandonAClaim(const uint256& txid, bool isSupport = false) {
     // pass a txid as name for update
     rpcfn_type rpc_method = tableRPC[isSupport ? "abandonsupport" : "abandonclaim"]->actor;
     JSONRPCRequest req;
+    req.URI = "/wallet/tester_wallet";
     req.params = UniValue(UniValue::VARR);
     req.params.push_back(txid.GetHex());
     req.params.push_back(adr_hash.get_str());