net: squashme: a few fixups from review

- Make g_connman a unique_ptr
- Add a note about a NULL connman in ActivateBestChain
- Check for null clientInterface before using it

src/init.cpp

@@ -70,7 +70,7 @@ static const bool DEFAULT_REST_ENABLE = false;
 static const bool DEFAULT_DISABLE_SAFEMODE = false;
 static const bool DEFAULT_STOPAFTERBLOCKIMPORT = false;
 
-std::shared_ptr<CConnman> g_connman;
+std::unique_ptr<CConnman> g_connman;
 
 #if ENABLE_ZMQ
 static CZMQNotificationInterface* pzmqNotificationInterface = NULL;
@@ -1045,7 +1045,7 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
     // ********************************************************* Step 6: network initialization
 
     assert(!g_connman);
-    g_connman = std::make_shared<CConnman>();
+    g_connman = std::unique_ptr<CConnman>(new CConnman());
     CConnman& connman = *g_connman;
 
     RegisterNodeSignals(GetNodeSignals());

src/main.cpp

@@ -2979,6 +2979,9 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams,
                 int nBlockEstimate = 0;
                 if (fCheckpointsEnabled)
                     nBlockEstimate = Checkpoints::GetTotalBlocksEstimate(chainparams.Checkpoints());
+
+                // Only relay the new block hash if a connman has been initialized. That may not be the case
+                // if (for example) we are reindexing.
                 if(connman) {
                     connman->SetBestHeight(nNewHeight);
                     connman->ForEachNode([nNewHeight, nBlockEstimate, &vHashes](CNode* pnode) {

src/net.cpp

@@ -1957,7 +1957,8 @@ bool CConnman::Start(boost::thread_group& threadGroup, CScheduler& scheduler, st
     SetBestHeight(connOptions.nBestHeight);
 
     clientInterface = connOptions.interface;
-    clientInterface->InitMessage(_("Loading addresses..."));
+    if (clientInterface)
+        clientInterface->InitMessage(_("Loading addresses..."));
     // Load addresses from peers.dat
     int64_t nStart = GetTimeMillis();
     {
@@ -1970,8 +1971,8 @@ bool CConnman::Start(boost::thread_group& threadGroup, CScheduler& scheduler, st
             DumpAddresses();
         }
     }
-
-    clientInterface->InitMessage(_("Loading banlist..."));
+    if (clientInterface)
+        clientInterface->InitMessage(_("Loading banlist..."));
     // Load addresses from banlist.dat
     nStart = GetTimeMillis();
     CBanDB bandb;

src/net.h

@@ -362,7 +362,7 @@ class CConnman
     std::atomic<int> nBestHeight;
     CClientUIInterface* clientInterface;
 };
-extern std::shared_ptr<CConnman> g_connman;
+extern std::unique_ptr<CConnman> g_connman;
 
 void Discover(boost::thread_group& threadGroup);
 void MapPort(bool fUseUPnP);

src/test/miner_tests.cpp

@@ -114,7 +114,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
         pblock->hashMerkleRoot = BlockMerkleRoot(*pblock);
         pblock->nNonce = blockinfo[i].nonce;
         CValidationState state;
-        BOOST_CHECK(ProcessNewBlock(state, chainparams, NULL, pblock, true, NULL, connman.get()));
+        BOOST_CHECK(ProcessNewBlock(state, chainparams, NULL, pblock, true, NULL, connman));
         BOOST_CHECK(state.IsValid());
         pblock->hashPrevBlock = pblock->GetHash();
     }

src/test/test_bitcoin.cpp

@@ -26,7 +26,7 @@
 #include <boost/test/unit_test.hpp>
 #include <boost/thread.hpp>
 
-std::shared_ptr<CConnman> g_connman;
+std::unique_ptr<CConnman> g_connman;
 
 extern bool fPrintToConsole;
 extern void noui_connect();
@@ -35,12 +35,12 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName)
 {
         ECC_Start();
         SetupEnvironment();
-        g_connman = std::make_shared<CConnman>();
         SetupNetworking();
         fPrintToDebugLog = false; // don't want to write to debug.log file
         fCheckBlockIndex = true;
         SelectParams(chainName);
         noui_connect();
+        g_connman = std::unique_ptr<CConnman>(new CConnman());
 }
 
 BasicTestingSetup::~BasicTestingSetup()
@@ -54,8 +54,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha
     const CChainParams& chainparams = Params();
         // Ideally we'd move all the RPC tests to the functional testing framework
         // instead of unit tests, but for now we need these here.
-        connman = std::make_shared<CConnman>();
-        g_connman = connman;
+        connman = g_connman.get();
 
         RegisterAllCoreRPCCommands(tableRPC);
         ClearDatadirCache();
@@ -119,7 +118,7 @@ TestChain100Setup::CreateAndProcessBlock(const std::vector<CMutableTransaction>&
     while (!CheckProofOfWork(block.GetHash(), block.nBits, chainparams.GetConsensus())) ++block.nNonce;
 
     CValidationState state;
-    ProcessNewBlock(state, chainparams, NULL, &block, true, NULL, connman.get());
+    ProcessNewBlock(state, chainparams, NULL, &block, true, NULL, connman);
 
     CBlock result = block;
     delete pblocktemplate;

src/test/test_bitcoin.h

@@ -32,7 +32,7 @@ struct TestingSetup: public BasicTestingSetup {
     CCoinsViewDB *pcoinsdbview;
     boost::filesystem::path pathTemp;
     boost::thread_group threadGroup;
-    std::shared_ptr<CConnman> connman;
+    CConnman* connman;
 
     TestingSetup(const std::string& chainName = CBaseChainParams::MAIN);
     ~TestingSetup();